Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide support for groupby #10277

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aaqilniz
Copy link
Contributor

@aaqilniz aaqilniz commented Dec 28, 2023

Currently, loopback 4 doesn't support Groupby. This PR provides support for that.

Related PRs:

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@@ -50,8 +50,42 @@ export function writeResultToResponse(
// TODO(ritch) remove this, should be configurable
// See https://github.com/loopbackio/loopback-next/issues/436
response.setHeader('Content-Type', 'application/json');
// let customResult = result;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this commented ? can you check please ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a mistake. I'd fix it.

Copy link
Contributor

@samarpanB samarpanB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not able to figure out what will the type returned for this method. groupBy is tricky because it can't return model array anymore so it will fail typings. Also I dont see repository related changes. Can you please add an end to end test here to confirm, its working end to end with repository ?

@aaqilniz aaqilniz force-pushed the feat/support-groupby branch from b721132 to d344824 Compare September 26, 2024 16:57
Signed-off-by: Muhammad Aaqil <[email protected]>
@aaqilniz aaqilniz force-pushed the feat/support-groupby branch from d344824 to 834539b Compare September 28, 2024 06:56
Signed-off-by: Muhammad Aaqil <[email protected]>
@aaqilniz
Copy link
Contributor Author

Hi, @samarpanB. The returned response of a group by filter should be an array of the models.

I have added a test in one of the examples (todo-list). But the test fails due to missing changes from this and this PR. The test works fine locally if the changes are there from these PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants