Skip to content

Improve Request#Show page for instructors with review information#455

Open
cycomachead wants to merge 1 commit into
mainfrom
cycomachead/151-improve-request-show-instructor/1
Open

Improve Request#Show page for instructors with review information#455
cycomachead wants to merge 1 commit into
mainfrom
cycomachead/151-improve-request-show-instructor/1

Conversation

@cycomachead

Copy link
Copy Markdown
Contributor

Improve Request#Show Page for Instructors with Review Information

Surfaces the context an instructor needs to make an informed approval/denial decision on an extension request, and cleans up the underlying role-checking pattern across the app.

Changes

New instructor review context on Request#Show:

  • Student email (mailto link) and an "Extended requests allowed" badge
  • Request submission timestamp with a before/after-deadline badge
  • Direct LMS link to the assignment
  • Projected late due date that will be applied on approval (for pending requests)
  • Student's full extension history in the course (assignment, requested date, days, submission date, status) in a compact table with approved/pending/denied summary counts
  • Decision provenance for already-decided requests (who processed it and when)

RequestReviewPresenter (app/presenters/request_review_presenter.rb) — encapsulates all the review-context data computation (student history, status counts, enrollment, projected dates) so the controller stays thin and the view reads from a single @review object.

_student_extension_history partial — the history table extracted into its own partial for clarity.

staff_user? helper — replaces every @role == 'instructor' permission check across controllers and views with a semantic, memoized predicate that delegates to Course#course_staff?. Available in both controllers and views via helper_method.

Security fix in AssignmentsController#toggle_enabled — previously trusted a client-supplied role param to determine authorization. Now uses the server-side staff_user? check exclusively.

Testing

  • Added spec/presenters/request_review_presenter_spec.rb (8 examples) covering all presenter methods including edge cases (no enrollment, submitted after deadline, status counts excluding current request).
  • Added controller specs for the instructor show action asserting template rendering and correct @review presenter assignment.
  • Added a regression test asserting AssignmentsController ignores a client-supplied role: 'instructor' param.
  • Added #staff_user? specs covering staff-true, student-false, nil guards, and memoization correctness.
  • Fixed a latent test isolation bug: the users factory generated bare numeric canvas_uid sequences ("1", "2", …) that would eventually collide with hardcoded UIDs in specs. Prefixed the sequence ("canvas-uid-#{n}") to prevent collisions across full suite runs.
  • Fixed course_settings_controller_spec enrollments that used a fictitious role: 'instructor' (not a real DB role); switched to role: 'teacher'.

Full suite: 446 examples, 0 failures.

Documentation

No additional documentation required.

Checklist

  • Name of branch corresponds to story

Superconductor Ticket Implementation | App Preview | Guided Review

- Introduce `RequestReviewPresenter` to encapsulate student history and approval metadata
- Add `_student_extension_history` partial to display prior requests and status counts
- Surface student email, LMS links, and projected late due dates on the show page
- Replace string-based `@role == 'instructor'` checks with a cached `staff_user?` helper
- Fix a security vulnerability in `AssignmentsController` by enforcing server-side permission checks
- Resolve a latent test isolation bug by prefixing factory `canvas_uid` sequences

Co-authored-by: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant