-
Notifications
You must be signed in to change notification settings - Fork 5
Lab Manager Promotion and test fixes #348
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
Lab Manager Promotion and test fixes #348
Conversation
- rewrote serialize_course - fixed "departments" test case
- started lab manager registration route - fixed a general test case
…132123/LabConnect-Backend into labconnect-backend-aniket
- still need to test
- tests run successfully!
|
Here's the code health analysis summary for commits Analysis Summary
|
RafaelCenzano
left a comment
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.
Some general comments, overall things look good. Also there some ruff linting issues. You can fix these with ruff lint to view the mistakes, ruff lint --fix can resolve some and ruff format as well. Most are lines too long easy enough to just wrap lines around. Also since the pytest github action isn't working, can you confirm your new local tests are working?
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.
Pull request overview
This PR adds a Lab Manager promotion feature and fixes several existing tests. The promotion feature allows super admins to promote users to Lab Manager status by updating their management permissions.
- Added new promotion endpoint for super admins to promote users to Lab Manager
- Updated test suite with comprehensive promotion tests and fixed existing test data
- Modified serializers and route responses to return structured data instead of strings
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_manager_promotion.py | New test suite for user promotion feature with fixtures and comprehensive test cases |
| tests/test_general.py | Fixed test data, removed obsolete discover test, updated years list, and added JWT authentication to profile test |
| tests/test_departments.py | Updated test assertions to match corrected spelling, field names, and data structures |
| tests/test_authentication.py | Uncommented imports for future test development |
| tests/conftest.py | Disabled JWT CSRF protection for testing environment |
| labconnect/serializers.py | Changed serialize_course to return dictionary instead of formatted string |
| labconnect/main/routes.py | Updated departments endpoint to return more fields with standardized field names |
| labconnect/main/auth_routes.py | Added new promotion endpoint with permission checks and validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- used bearer tokens to help authenticate tests along with cookies - ruff fixes - TODO: try to simulate csrf token for testing
…132123/LabConnect-Backend into labconnect-backend-aniket
RafaelCenzano
left a comment
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.
Looks good few small comments. Also check the ruff linting errors besides that should be good to go.
RafaelCenzano
left a comment
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.
LGTM
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Ran tests that check whether the promoter has proper perms, if lab manager to be promoted actually exists, and a variety of other tests to ensure integrity of the route. I also fixed some of the previous tests.
Checklist: