fix(amber, frontend): serve created datasets to the admin quota modal - #7318
Merged
xuang7 merged 4 commits intoAug 5, 2026
Merged
Conversation
The admin user-quota modal requested /admin/user/created_datasets, but AdminUserResource never exposed that route, so the created-datasets panel 404'd. The only created_datasets endpoint lives on UserQuotaResource under /quota and derives the uid from the session, so it cannot serve the datasets of the user an admin is inspecting. Add the ADMIN-only endpoint alongside the sibling created_workflows route, and send the uid that AdminUserService.getCreatedDatasets already accepted but dropped. Closes apache#7317
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
Backport auto-label reportThis
|
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
The new backend endpoint should validate missing/invalid user_id and return a clear 400 instead of silently proceeding with a null query parameter.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes the admin user-quota modal’s “created datasets” panel by aligning the frontend request with a newly added backend admin endpoint that can query datasets for an inspected user (by user_id), instead of relying on a session-derived user.
Changes:
- Added
GET /admin/user/created_datasets?user_id=<uid>toAdminUserResource(ADMIN-only) backed byDatasetStatisticsUtils.getUserCreatedDatasets. - Updated
AdminUserService.getCreatedDatasets(uid)to actually senduser_idas a query parameter. - Added/updated backend and frontend tests to cover the new endpoint wiring and request shape.
File summaries
| File | Description |
|---|---|
| frontend/src/app/dashboard/service/admin/user/admin-user.service.ts | Sends user_id query param when fetching created datasets for an inspected user. |
| frontend/src/app/dashboard/service/admin/user/admin-user.service.spec.ts | Updates unit test to assert user_id is included in the request. |
| amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResource.scala | Adds the missing admin route for created_datasets with user_id query support. |
| amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala | Adds regression tests ensuring datasets returned are scoped to the requested user. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 348 | 0.212 | 28,344/38,641/38,641 us | 🔴 +15.5% / 🔴 +148.7% |
| ⚪ | bs=100 sw=10 sl=64 | 786 | 0.479 | 124,303/153,246/153,246 us | ⚪ within ±5% / 🔴 +40.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 901 | 0.55 | 1,098,413/1,238,504/1,238,504 us | ⚪ within ±5% / 🔴 +19.2% |
Baseline details
Latest main 86865f3 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 348 tuples/sec | 384 tuples/sec | 767.32 tuples/sec | -9.4% | -54.6% |
| bs=10 sw=10 sl=64 | MB/s | 0.212 MB/s | 0.234 MB/s | 0.468 MB/s | -9.4% | -54.7% |
| bs=10 sw=10 sl=64 | p50 | 28,344 us | 26,423 us | 12,772 us | +7.3% | +121.9% |
| bs=10 sw=10 sl=64 | p95 | 38,641 us | 33,446 us | 15,538 us | +15.5% | +148.7% |
| bs=10 sw=10 sl=64 | p99 | 38,641 us | 33,446 us | 18,948 us | +15.5% | +103.9% |
| bs=100 sw=10 sl=64 | throughput | 786 tuples/sec | 821 tuples/sec | 972.51 tuples/sec | -4.3% | -19.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.479 MB/s | 0.501 MB/s | 0.594 MB/s | -4.4% | -19.3% |
| bs=100 sw=10 sl=64 | p50 | 124,303 us | 120,444 us | 103,020 us | +3.2% | +20.7% |
| bs=100 sw=10 sl=64 | p95 | 153,246 us | 155,382 us | 109,070 us | -1.4% | +40.5% |
| bs=100 sw=10 sl=64 | p99 | 153,246 us | 155,382 us | 118,964 us | -1.4% | +28.8% |
| bs=1000 sw=10 sl=64 | throughput | 901 tuples/sec | 907 tuples/sec | 1,005 tuples/sec | -0.7% | -10.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.55 MB/s | 0.553 MB/s | 0.613 MB/s | -0.5% | -10.3% |
| bs=1000 sw=10 sl=64 | p50 | 1,098,413 us | 1,102,079 us | 1,002,400 us | -0.3% | +9.6% |
| bs=1000 sw=10 sl=64 | p95 | 1,238,504 us | 1,211,598 us | 1,039,228 us | +2.2% | +19.2% |
| bs=1000 sw=10 sl=64 | p99 | 1,238,504 us | 1,211,598 us | 1,069,081 us | +2.2% | +15.8% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,575.20,200,128000,348,0.212,28344.26,38641.34,38641.34
1,100,10,64,20,2546.08,2000,1280000,786,0.479,124302.53,153246.03,153246.03
2,1000,10,64,20,22187.64,20000,12800000,901,0.550,1098412.80,1238503.98,1238503.98
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7318 +/- ##
============================================
- Coverage 83.37% 83.35% -0.03%
+ Complexity 4129 4128 -1
============================================
Files 1166 1166
Lines 46424 46404 -20
Branches 5174 5171 -3
============================================
- Hits 38707 38681 -26
- Misses 6000 6003 +3
- Partials 1717 1720 +3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
The auto-committed guard was indented at column 0, which failed
scalafmtCheckAll and stopped the amber job before any test ran.
Reindent it, and raise BadRequestException("user_id is required") to match
the existing admin-only FeedbackResource.listUserFeedback guard rather than
a bare WebApplicationException. Cover the 400 path in the spec.
Contributor
|
Backport PR opened: draft #7328 (#7328) to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
The admin user-quota modal (
UserQuotaComponentopened from the admin user list) fails to load its "created datasets" panel: the request 404s.Root cause:
AdminUserService.getCreatedDatasets()targets/admin/user/created_datasets, butAdminUserResourcenever exposed that endpoint. The onlycreated_datasetsendpoint lives onUserQuotaResource(/quota/created_datasets), and it derives the uid from the authenticated session (@Auth current_user) with nouser_idparameter — so it cannot serve "datasets of the user an admin is inspecting" (it would return the admin's own datasets instead).GET /admin/user/created_datasets(no params)GET /admin/user/created_datasets?user_id=<uid>AdminUserResource.getCreatedDatasets(user_id)→ datasets owned byuser_idChanges:
AdminUserResource.scala: addGET /admin/user/created_datasets?user_id=N(ADMIN-only, like the siblingcreated_workflows/access_workflows/user_quota_sizeendpoints), delegating toDatasetStatisticsUtils.getUserCreatedDatasets.admin-user.service.ts:getCreatedDatasets(uid)now sends the uid as theuser_idquery param (it previously accepted the uid and silently dropped it).Any related issues, documentation, discussions?
Closes #7317
How was this PR tested?
Written test-first (both new tests were confirmed failing before the fix):
AdminUserResourceSpec: two new tests forgetCreatedDatasets— empty list for a user with no datasets, and only the queried user's datasets are returned (a second user's dataset is excluded).admin-user.service.spec.ts: thegetCreatedDatasetstest now asserts theuser_idquery param is sent (previously asserted no params were sent).Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)