Skip to content

Commit

Permalink
Fix: Events API null total (#27)
Browse files Browse the repository at this point in the history
* fixes #13, event api null total

* shift to promise all

* add e2e tests

* fix format

* add e2e tests for total props

* add e2e test for non-existent event
  • Loading branch information
tuminzee authored Dec 9, 2024
1 parent 8a1e6d1 commit dd2d4e3
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 25 deletions.
9 changes: 7 additions & 2 deletions apps/trench/src/events/events.dao.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,21 @@ export class EventsDao {
if (!isReadOnlyQuery(clickhouseQuery)) {
throw new BadRequestException('The provided query is not read-only')
}
const totalQuery = `SELECT COUNT(*) AS count FROM events ${whereClause}`

try {
const result = await this.clickhouse.queryResults(clickhouseQuery, workspace.databaseName)
const [result, total] = await Promise.all([
this.clickhouse.queryResults(clickhouseQuery, workspace.databaseName),
this.clickhouse.queryResults(totalQuery, workspace.databaseName),
])
const results = result.map((row: any) => mapRowToEvent(row))
const totalCount = +total[0].count

return {
results: results,
limit: maxRecords,
offset: +offset || 0,
total: null,
total: totalCount,
}
} catch (error) {
throw new BadRequestException(`Error querying events: ${error.message}`)
Expand Down
6 changes: 3 additions & 3 deletions apps/trench/src/services/data/click-house/click-house.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function escapeString(str: string) {
* The date will also be escaped to be safe to use in a ClickHouse query.
*/
export function formatToClickhouseDate(date: Date): string {
const isoString = date.toISOString();
const clickhouseDate = isoString.replace('Z', '');
return escapeString(clickhouseDate);
const isoString = date.toISOString()
const clickhouseDate = isoString.replace('Z', '')
return escapeString(clickhouseDate)
}
37 changes: 23 additions & 14 deletions apps/trench/test/e2e/events.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ describe('events/', () => {
})
expect(createRes.statusCode).toEqual(201)
expect(createRes.body.results).toHaveLength(1)
expect(createRes.body.total).toEqual(1)
expect(createRes.body.results[0].uuid).toBeDefined()
const uuid = createRes.body.results[0].uuid

// Fetch the created event using the newly created util function
const results = await waitForQueryResults(`uuid=${uuid}`)
expect(results).toHaveLength(1)
expect(results[0].uuid).toEqual(uuid)
const queryResults = await waitForQueryResults(`uuid=${uuid}`)
expect(queryResults.results).toHaveLength(1)
expect(queryResults.results[0].uuid).toEqual(uuid)
})

test('should create a new event with instanceId, event name with spaces, and userId, then fetch it', async () => {
Expand All @@ -39,18 +40,19 @@ describe('events/', () => {
})
expect(createRes.statusCode).toEqual(201)
expect(createRes.body.results).toHaveLength(1)
expect(createRes.body.total).toEqual(1)
expect(createRes.body.results[0].uuid).toBeDefined()
const eventUuid = createRes.body.results[0].uuid

// Fetch the created event using the instanceId, event name, and userId
const results = await waitForQueryResults(
`uuid=${eventUuid}&event=User%20Logged%20In&userId=user-123&instanceId=instance-456`
)
expect(results).toHaveLength(1)
expect(results[0].uuid).toEqual(eventUuid)
expect(results[0].event).toEqual('User Logged In')
expect(results[0].userId).toEqual('user-123')
expect(results[0].instanceId).toEqual('instance-456')
expect(results.total).toEqual(1)
expect(results.results[0].uuid).toEqual(eventUuid)
expect(results.results[0].event).toEqual('User Logged In')
expect(results.results[0].userId).toEqual('user-123')
expect(results.results[0].instanceId).toEqual('instance-456')
})

test('should create a new event with properties and fetch it using properties', async () => {
Expand All @@ -71,17 +73,24 @@ describe('events/', () => {
})
expect(createRes.statusCode).toEqual(201)
expect(createRes.body.results).toHaveLength(1)
expect(createRes.body.total).toEqual(1)
expect(createRes.body.results[0].uuid).toBeDefined()
const eventUuid = createRes.body.results[0].uuid

// Fetch the created event using the properties
const results = await waitForQueryResults(
const queryResults = await waitForQueryResults(
`uuid=${eventUuid}&properties.plan=premium&properties.country=USA`
)
expect(results).toHaveLength(1)
expect(results[0].uuid).toEqual(eventUuid)
expect(results[0].event).toEqual('User Updated Profile')
expect(results[0].properties.plan).toEqual('premium')
expect(results[0].properties.country).toEqual('USA')
expect(queryResults.total).toEqual(1)
expect(queryResults.results[0].uuid).toEqual(eventUuid)
expect(queryResults.results[0].event).toEqual('User Updated Profile')
expect(queryResults.results[0].properties.plan).toEqual('premium')
expect(queryResults.results[0].properties.country).toEqual('USA')
})

test('should return an error when querying for a non-existent event', async () => {
await expect(waitForQueryResults('event=NonExistentEvent')).rejects.toThrow(
'Timeout: No results found'
)
})
})
7 changes: 4 additions & 3 deletions apps/trench/test/e2e/queries.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ describe('queries/', () => {
})
expect(createRes.statusCode).toEqual(201)
expect(createRes.body.results).toHaveLength(1)
expect(createRes.body.total).toEqual(1)
const eventUuid = createRes.body.results[0].uuid
// Wait for the event to be created
const results = await waitForQueryResults(`uuid=${eventUuid}`)
expect(results).toHaveLength(1)
expect(results[0].uuid).toEqual(eventUuid)
const queryResults = await waitForQueryResults(`uuid=${eventUuid}`)
expect(queryResults.results).toHaveLength(1)
expect(queryResults.results[0].uuid).toEqual(eventUuid)

// Execute the query
const query = `SELECT * FROM events WHERE uuid = '${eventUuid}'`
Expand Down
2 changes: 1 addition & 1 deletion apps/trench/test/e2e/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export async function waitForQueryResults(query: string, privateApiKey?: string)
while (true) {
const res = await authenticatedGet(`/events?${query}`, privateApiKey)
if (res.body.results && res.body.results.length > 0) {
return res.body.results
return res.body
}
if (Date.now() - startTime > maxWaitTime) {
throw new Error(`Timeout: No results found within ${maxWaitTime / 1000} seconds`)
Expand Down
4 changes: 2 additions & 2 deletions apps/trench/test/e2e/workspaces.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ describe('workspaces/', () => {

// Query the created event using the public API key
const queryResults = await waitForQueryResults(`uuid=${eventUuid}`, newPrivateApiKey)
expect(queryResults).toHaveLength(1)
expect(queryResults[0].uuid).toEqual(eventUuid)
expect(queryResults.results).toHaveLength(1)
expect(queryResults.results[0].uuid).toEqual(eventUuid)

// Ensure the new private api key cannot be used to create new workspaces
const createWorkspaceRes = await authenticatedPost('/workspaces', newPrivateApiKey).send({
Expand Down

0 comments on commit dd2d4e3

Please sign in to comment.