feat: add items count column to datasets list #276
Conversation
- Added SQL query to count datapoints per dataset - Added items count to dataset API response - Added Items column to datasets table in UI
|
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to d63d167 in 1 minute and 17 seconds
More details
- Looked at
135lines of code in4files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. app-server/src/db/datasets.rs:82
- Draft comment:
The use ofCOALESCE(dc.count, 0)is unnecessary here because theLEFT JOINwithdataset_countsalready ensures that datasets without datapoints will have a count of 0. You can directly usedc.count. - Reason this comment was not posted:
Confidence changes required:50%
The SQL query inget_datasets_with_countsis correct, but the use ofCOALESCEis unnecessary since theLEFT JOINwithdataset_countsalready ensures that datasets without datapoints will have a count of 0.
2. app-server/src/routes/datasets.rs:250
- Draft comment:
Theget_datasetsfunction is missing a route parameter forproject_id. The path should be#[get("projects/{project_id}/datasets")]to match the SQL query requirements. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_QKxjfwVn4zAh8297
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
app-server/src/routes/datasets.rs
Outdated
| @@ -244,6 +244,43 @@ async fn get_datapoints( | |||
| Ok(HttpResponse::Ok().json(response)) | |||
| } | |||
|
|
|||
| #[get("datasets")] | |||
There was a problem hiding this comment.
The get_datasets function is missing a route parameter for project_id. The path should be #[get("projects/{project_id}/datasets")] to match the SQL query requirements.
There was a problem hiding this comment.
Hey @raj921! Thanks for your PR! This looks good, but I think we actually fetch datasets from the frontend using drizzle, so there was no need to alter this in app-server. You should probably modify https://github.com/lmnr-ai/lmnr/blob/main/frontend/app/api/projects/%5BprojectId%5D/datasets/route.ts#L34 instead
|
Closing in favour of #288 |
Important
Add item count column to datasets list by updating backend and frontend components.
get_datasets_with_counts()indatasets.rsto fetch datasets with item counts.get_datasets()indatasets.rsto useget_datasets_with_counts().get_datasets()inroutes/datasets.rsto includeitemsCountin API response.itemsCounttoDatasetinterface intypes.ts.datasets.tsxto display item counts.This description was created by
for d63d167. It will automatically update as commits are pushed.