Return actual value for roster last_updated #6913
Open
+77
−61
Conversation
This file contains 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
marcospri
force-pushed
the
roster-dashboard-api-last-updated
branch
2 times, most recently
from
95caeb1 to
4d5d4de
Compare
marcospri
changed the title
marcospri
commented
Dec 16, 2024
lms/services/roster.py
Outdated
| .where(AssignmentRoster.assignment_id == assignment.id) | ||
| .limit(1) | ||
| ) | ||
| def assignment_roster_exists(self, assignment: Assignment) -> datetime | None: |
There was a problem hiding this comment.
Changing this from returning a boolean to returning a date or None.
| @@ -84,7 +85,7 @@ def __init__(self, request) -> None: | |||
| schema=ListUsersSchema, | |||
| ) | |||
| def students(self) -> APIStudents: | |||
| students_query = self._students_query( | |||
| _, students_query = self._students_query( | |||
There was a problem hiding this comment.
We don't care about last_udpated for the dropdowns, ignoring the value there.
| @@ -211,7 +210,7 @@ def _students_query( | |||
| assignment_ids: list[int], | |||
| segment_authority_provided_ids: list[str], | |||
| h_userids: list[str] | None = None, | |||
| ): | |||
| ) -> tuple[datetime | None, Select]: | |||
There was a problem hiding this comment.
This type anno helped find where we need to start returning a tuple
acelaya
reviewed
Dec 16, 2024
marcospri
force-pushed
the
roster-dashboard-api-last-updated
branch
from
9168cf6 to
7d79381
Compare
Return the greatest date we have for a given roster or None for cases where we still base the roster data on launches.
Only return the last_update when the roster data is returned to the client. Avoid returning it when we do have that roster data but the feature is disabled.
From assignment_roster_exists to assignment_roster_last_updated Co-authored-by: Alejandro Celaya <[email protected]>
marcospri
force-pushed
the
roster-dashboard-api-last-updated
branch
from
7d79381 to
a9adfab
Compare
marcospri
commented
Dec 16, 2024
| @@ -217,46 +217,53 @@ def test_get_request_admin_organizations_for_staff( | |||
|
|
|||
| assert svc.get_request_admin_organizations(pyramid_request) == [organization] | |||
|
|
|||
| @pytest.mark.parametrize("rosters_enabled", [True, False]) | |||
| @pytest.mark.parametrize("roster_available", [True, False]) | |||
There was a problem hiding this comment.
Combined a couple of test into this one, adding all the combinations of enabled/disabled with the data available/missing.
acelaya
approved these changes
Dec 16, 2024
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.
We have been returning
Nonefor all entries until now.Actually return the value on the DB for roster entries and
Nonefor the ones based on launches.Testing
https://hypothesis.instructure.com/courses/125/assignments/873
You'll see the
Full roster data for this assignment is not available. This only shows students who have previously launched it.message.
http://localhost:8001/admin/instances/102/settings
Enable roster data.https://hypothesis.instructure.com/courses/319/assignments/3336
make shell:Open the dashboard, you'll get the roster updated date on the top right corner.
Repeat with the AI setting disabled:
http://localhost:8001/admin/instances/102/settings
The top right notice will be gone and you will fallback to launches data.