Added aggregate methods #42

StemateF · 2023-12-10T12:00:58Z

Hi there,

I added missing aggregate functions.

Unfortunately I will need some help with the tests, I couldn't figure out how the whole test setup actually works 😢.

@alexzarbn Please let me know what you think, if you have suggestions on improving this.

Thanks

Bumps [@babel/traverse](https://github.com/babel/babel/tree/HEAD/packages/babel-traverse) from 7.13.13 to 7.23.2. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.23.2/packages/babel-traverse) --- updated-dependencies: - dependency-name: "@babel/traverse" dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [axios](https://github.com/axios/axios) from 0.21.2 to 1.6.0. - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md) - [Commits](axios/axios@v0.21.2...v1.6.0) --- updated-dependencies: - dependency-name: axios dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>

query.withCount() query.withExists() query.withSum() query.withAvg() query.withMin() query.withMax()

alexzarbn · 2023-12-13T13:02:16Z

Hi @StemateF,

Thank you for the PR!

Overall it looks good, as for the tests - integration tests are built on top of MirageJS, you can take a look at their docs to get yourself familiar with the concept, as well as check out the retrieving a paginated list resources with included relations test in queryBuilder.test.ts and try to implement a similar logic for the newly introduced methods on QueryBuilder.

Merge pull request tailflow#41 from tailflow/dependabot/npm_and_yarn/axios-1.6.0

Merge pull request tailflow#40 from tailflow/dependabot/npm_and_yarn/babel/traverse-7.23.2

StemateF · 2023-12-14T08:26:09Z

@alexzarbn Done.
I also fixed a typing issue on the new type I created (oops 😅 )
Added some basic testing following the test you referenced.
Let me know if there are more changes required, will be happy to make them.

alexzarbn · 2024-01-07T06:26:29Z

Hi @StemateF,

Awesome, looking great overall! One fix is required before I merge this - TS complains about missing information in runtime.

Added types to with(), .withCount(), .withExists()

query.withCount() query.withExists() query.withSum() query.withAvg() query.withMin() query.withMax()

Added types to with(), .withCount(), .withExists()

# Conflicts: # tests/integration/orion.test.ts

StemateF · 2024-01-07T11:49:33Z

Hi @alexzarbn, I've addressed the TypeScript errors in the code, funny enough I had to update my ts for the errors to come up on my end.

Additionally, I have included types for the .with() method. Please note that this might be a breaking change, as it could disrupt builds when attempting to include a relation that hasn't been defined. Similarly, trying to include a nested relation with a depth greater than 2 (e.g., users.posts.tags) will not work; only users.posts will be valid.

I'm open to increasing the depth to a larger value if needed, but implementing recursion could pose challenges. This is particularly true when two models reference each other, resulting in a scenario where the type becomes something like users.posts.user.posts.user.... Handling this recursive situation may be intricate. Let me know your thoughts on these changes.

alexzarbn · 2024-01-07T17:29:12Z

@StemateF Thank you for the quick update!

I would suggest not to introduce breaking changes for now and leave the with method as is and just have the types working for the aggregates.

Also, I'm not sure if using string & keyof Relations construct would actually preserve the type string in the runtime, were you able to confirm it works?

StemateF · 2024-01-11T09:49:25Z

@alexzarbn I undid the changes to the with method.

I am not sure I understand what you mean by "runtim" you mean the transpiled version of the sdk? The one in lib?
If so it does work, this is what I get in phpstorm after changing the imports to lib.

StemateF · 2024-01-11T09:57:47Z

Now that i look at this, I realized that the new methods will suffer from the same issue I highlighted in my previous comment "#42 (comment)".

Might be best to just give up on types for the relations names as it might be limiting for some users.

alexzarbn · 2024-01-20T12:34:36Z

@StemateF What I mean is the js output that, when used, might not work correctly, since it does not have the type information.

I will merge it as is, and will see, if any issues pop up.

StemateF · 2024-01-21T07:18:17Z

@alexzarbn Sounds good, thanks. Is there a planned release for the new version of the sdk on NPM ?

alexzarbn · 2024-02-15T15:47:37Z

@alexzarbn Sounds good, thanks. Is there a planned release for the new version of the sdk on NPM ?

Published v4.1.0 just now

dependabot bot and others added 3 commits October 17, 2023 02:59

Added aggregated methods:

54f4ace

query.withCount() query.withExists() query.withSum() query.withAvg() query.withMin() query.withMax()

alexzarbn and others added 10 commits December 13, 2023 18:03

chore(deps): bump axios from 0.21.2 to 1.6.0

6e56081

Merge pull request tailflow#41 from tailflow/dependabot/npm_and_yarn/axios-1.6.0

chore(deps-dev): bump @babel/traverse from 7.13.13 to 7.23.2

6bacd21

Merge pull request tailflow#40 from tailflow/dependabot/npm_and_yarn/babel/traverse-7.23.2

fix: running tests on files utilizing axios

41b73fd

fix: eslint errors

d992ea3

chore: vendors upgrade

e28187f

feat(ci): upgrade actions

09ca0c2

feat(ci): upgrade actions

8d69902

fixed relation type

9df9349

Added missing relations in test models

ed3cea6

Added test for aggregate methods

8dbbdf8

StemateF added 7 commits January 7, 2024 13:16

Fixed ts errors,

71a8f06

Added types to with(), .withCount(), .withExists()

Added aggregated methods:

69162e6

query.withCount() query.withExists() query.withSum() query.withAvg() query.withMin() query.withMax()

fixed relation type

39ab5a5

Added missing relations in test models

1e284b1

Added test for aggregate methods

be86a6c

Fixed ts errors,

0a7f6e6

Added types to with(), .withCount(), .withExists()

Merge remote-tracking branch 'origin/main'

f85f778

# Conflicts: # tests/integration/orion.test.ts

Undo changes on ".with()" method

30ffa18

alexzarbn merged commit 884dcc1 into tailflow:main Jan 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added aggregate methods #42

Added aggregate methods #42

Uh oh!

StemateF commented Dec 10, 2023

Uh oh!

alexzarbn commented Dec 13, 2023

Uh oh!

StemateF commented Dec 14, 2023

Uh oh!

alexzarbn commented Jan 7, 2024 •

edited

Loading

Uh oh!

StemateF commented Jan 7, 2024

Uh oh!

alexzarbn commented Jan 7, 2024

Uh oh!

StemateF commented Jan 11, 2024

Uh oh!

StemateF commented Jan 11, 2024

Uh oh!

alexzarbn commented Jan 20, 2024

Uh oh!

StemateF commented Jan 21, 2024

Uh oh!

alexzarbn commented Feb 15, 2024

Uh oh!

Uh oh!

Added aggregate methods #42

Added aggregate methods #42

Uh oh!

Conversation

StemateF commented Dec 10, 2023

Uh oh!

alexzarbn commented Dec 13, 2023

Uh oh!

StemateF commented Dec 14, 2023

Uh oh!

alexzarbn commented Jan 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StemateF commented Jan 7, 2024

Uh oh!

alexzarbn commented Jan 7, 2024

Uh oh!

StemateF commented Jan 11, 2024

Uh oh!

StemateF commented Jan 11, 2024

Uh oh!

alexzarbn commented Jan 20, 2024

Uh oh!

StemateF commented Jan 21, 2024

Uh oh!

alexzarbn commented Feb 15, 2024

Uh oh!

Uh oh!

alexzarbn commented Jan 7, 2024 •

edited

Loading