-
Notifications
You must be signed in to change notification settings - Fork 364
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: Get Groups with basic aggregation #10086
feat: Get Groups with basic aggregation #10086
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/row-grouping #10086 +/- ##
========================================================
- Coverage 59.19% 54.61% -4.59%
========================================================
Files 756 1260 +504
Lines 105076 157696 +52620
Branches 3631 3632 +1
========================================================
+ Hits 62201 86127 +23926
- Misses 42741 71435 +28694
Partials 134 134
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good overall, my only comment isn't even a nit, just a flat remark.
Join("LEFT JOIN runs_metadata AS rm ON r.id=rm.run_id"). | ||
Join("LEFT JOIN users u ON e.owner_id = u.id"). | ||
Join("LEFT JOIN projects p ON r.project_id = p.id"). | ||
Join("LEFT JOIN workspaces w ON p.workspace_id = w.id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's beginning to get a little confusing and disjointed, having all these extra table and column aliases off in their own functions just because some other function (filterRunQuery
) might do something with rm.metadata
. Not sure what we can do about it just now, but I think it's worth pointing out now and then so we keep thinking about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will be returning the metadata for groups in following PRs. For now we are just doing the default columns.
Ticket
ET-831
Description
Create GetGroups endpoint for grouping runs in the run table by a provided column. For this ticket we are only grouping by default columns.
Test Plan
integration tests
Checklist
docs/release-notes/
See Release Note for details.