-
Notifications
You must be signed in to change notification settings - Fork 7
Consider the code for updating / posting an assignment override to Canvas * Audit for any bugs * Know that the Canvas API returns the wrong due da...#439
Open
cycomachead wants to merge 2 commits into
Conversation
- Fix shared overrides being destroyed by removing students from groups instead of deleting the entire override. - Fix update_assignment_override no-oping by nesting params correctly. - Fix override lookups failing when > 25 overrides exist by implementing depagination and requesting 100 items per page. - Fix incorrect base due dates by explicitly querying the date_details endpoint when Canvas omits all_dates. - Ensure override renames are applied during updates. - Prevent auth token leakage into paginated URLs. - Improve error handling by raising CanvasAPIError for failed requests. Co-authored-by: Claude Code <noreply@anthropic.com>
Implement logic to title and group assignment overrides by "N day(s) extension" instead of individual student IDs. This reduces the total override count to mitigate Canvas API limitations where date reporting becomes unreliable after 25 overrides. - Add `get_base_dates` lookup to ensure accurate day calculation. - Update `provision_extension` to merge students into existing matching groups. - Handle student removal from shared overrides when extension lengths change. - Ensure timezone-tolerant date matching for group identification. Co-authored-by: Claude Code <noreply@anthropic.com>
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.
General Info
Changes
Audited and fixed 7 bugs in the Canvas assignment override update/post flow, addressing correctness issues around shared overrides, pagination, base due dates, and error handling.
Critical fixes:
Shared overrides were being destroyed — the old flow deleted a student's entire override before re-creating one, wiping extensions for all other students in a shared group (e.g. "1 day extension"). Now: single-student overrides are updated in place; shared overrides have only the one student removed, preserving the title and dates for remaining students.
update_assignment_overridesilently no-oped — params weren't nested underassignment_overrideas Canvas requires. Un-nested params are silently ignored by Canvas, meaning override updates (including renames) never applied. Override renames now go through correctly.Override lookups only read page 1 — Canvas paginates overrides (default 10/page), so with many overrides a student's existing override was never found, leading to duplicate create attempts and 400 errors.
get_assignment_overridesnow requests 100 items/page and a newget_all_assignment_overridesdepaginates fully.Base due date fix (>25 overrides):
get_all_assignmentsrelied onall_dates, which Canvas omits when an assignment has more than 25 dates. The top-leveldue_atcan then reflect an override's date. A newget_base_datesmethod uses thedate_detailsendpoint (whose top-level dates are always the base/"Everyone" dates) and is called wheneverall_datesis missing andhas_overridesis true. The controller now uses this forinitial_due_dateas well.Other fixes:
Overridewith a nil id and the request was marked approved.provision_extensionnow raisesCanvasAPIErroron failure.depaginate_responsewas passingheaders: auth_headeras Faraday query params, appendingAuthorization=Bearer ...to follow-up page URLs. The connection's default headers already carry auth.remove_student_from_overrideno longer mutates the caller's array; updates no longer resetunlock_atto "now";override_has_errors?renamed tooverride_taken_error?with correct semantics;Lmss::Canvas::Assignmentno longer falls back to a potentially override-tainted top-level date when base date info is present;Lmss::Canvas::Overrideexposes the full override fields needed by the controller.Testing
delete_assignment_overridecalls, correctunlock_atpreservation).get_base_dates(success/failure/bad JSON),date_detailsfallback inget_all_assignments, failure-status raising, and non-mutation ofstudent_ids.Documentation
No documentation changes required.
Checklist
Superconductor Ticket Implementation | App Preview | Guided Review