Fix: Events API null total

[tuminzee]

Fixes #13
Earlier the total was returning a hardcoded null value, I changed it to get count on the where clause

const total = await this.clickhouse.queryResults(totalQuery, workspace.databaseName)
const totalCount = total?.[0]?.count || null

[tuminzee]

@christianmat I am not sure if I am supposed to write tests for it. Anyways do let me know what you think about it.

[christianmat]

Hey @tuminzee, thanks for this PR. Please see the guide on contributing re: tests: https://docs.trench.dev/contributing

One thing we need to fix here is that the total is not meant to simply equal the length of the array, it's supposed to be the total amount of records matching the query string. It is meant to be used for pagination so you know how many results you have. The real solution should be to do a separate query to clickhouse in parallel that counts the number of rows that matches the query.

[tuminzee]

@christianmat I have made a separate query for calculating the total. It is not a length fn.

I will check the contribution guidelines

[christianmat]

Please add e2e tests as well that test that this field works. See other e2e tests for examples.

[tuminzee]

Please add e2e tests as well that test that this field works. See other e2e tests for examples.

Yes I will do that

[tuminzee]

waitForQueryResults function should return the complete res.body instead of just res.body.results since we need to test other response properties like total

Questions for review:

1. Should I update all affected tests to include total property validation, or should we keep this as a separate enhancement?
2. If no results are found the query would always return a timeout error, shouldn't it just return a plain response?

[christianmat]

waitForQueryResults function should return the complete res.body instead of just res.body.results since we need to test other response properties like total

Questions for review:

1. Should I update all affected tests to include total property validation, or should we keep this as a separate enhancement?
2. If no results are found the query would always return a timeout error, shouldn't it just return a plain response?

1. Yes, please update all the existing tests to account for the new total behavior.
2. Not sure what you mean. It currently returns an empty array if no results are returned, it doesn't throw an error (see test screenshot below)
   

[tuminzee]

• I have updated the tests to include the total property.
• Regarding the timeout error: I apologize for not clarifying earlier—this issue occurs only in the e2e tests. waitForQueryResults: It currently throws an error if no results are found. Should this be the expected behavior, or should we modify it to return an empty array instead? Let me know your preference, and I can adjust accordingly. I have added a test to demonstrate this here