Skip to content

Conversation

BenjaminCharmes
Copy link
Contributor

Closes #1348

Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 71.69811% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.02%. Comparing base (b121182) to head (af47bb4).

Files with missing lines Patch % Lines
pydatalab/src/pydatalab/routes/v0_1/admin.py 71.69% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1352      +/-   ##
==========================================
- Coverage   80.10%   80.02%   -0.08%     
==========================================
  Files          70       70              
  Lines        4755     4807      +52     
==========================================
+ Hits         3809     3847      +38     
- Misses        946      960      +14     
Files with missing lines Coverage Δ
pydatalab/src/pydatalab/routes/v0_1/admin.py 70.45% <71.69%> (+3.78%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@BenjaminCharmes BenjaminCharmes marked this pull request as ready for review September 18, 2025 12:43
Copy link

cypress bot commented Sep 18, 2025

datalab    Run #3919

Run Properties:  status check passed Passed #3919  •  git commit 7d33fc31ca ℹ️: Merge af47bb4486001f23f724699e7dc3d956cc0888b9 into b121182e7a2c4ca1eebd268ed0b8...
Project datalab
Branch Review bc/admin-manager
Run status status check passed Passed #3919
Run duration 07m 41s
Commit git commit 7d33fc31ca ℹ️: Merge af47bb4486001f23f724699e7dc3d956cc0888b9 into b121182e7a2c4ca1eebd268ed0b8...
Committer Ben Charmes
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 336
View all changes introduced in this branch ↗︎

@BenjaminCharmes BenjaminCharmes changed the title Added manager management to the admin dashboard Added manager to the admin dashboard Sep 19, 2025
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Thanks for this @BenjaminCharmes, just tried it out, whilst it works quite nicely in usual cases, I seemed to be able to crash it by making user A manager of another user B, and then making that user B manager of user A, where I get "cyclic object value" as an error.

This seems to happen at arbitrary depths too, i.e., A -> B -> C -> A manager loops.

Finally, it would be good to add some tests for this new API route, i.e., make a user another user's manager, check that they have to be a manager/admin themselves, and make sure they can see that user's items once they are their manager.

@BenjaminCharmes BenjaminCharmes requested a review from ml-evs October 1, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add ability to assign managers in Admin panel

2 participants