Split Course Settings Into Multiple Pages#453
Open
cycomachead wants to merge 13 commits into
Open
Conversation
Replace the single tabbed Settings page (?tab=general|email) with two distinct, bookmarkable pages served by CourseSettingsController: - GET /courses/:id/course_settings/approvals - GET /courses/:id/course_settings/emails - PATCH /courses/:id/course_settings (update, redirects back via :page) Distinct routes are more idiomatic than query-param tabs: each page has its own URL and focused view, the brittle JS history/?tab= hack is gone, and it mirrors the existing nested form_setting resource. The old /courses/:id/edit now redirects to the Approvals page; its tabbed view is removed and the unused Stimulus tab code is dropped. Sidebar and in-page links are repointed at the new routes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Repurpose CoursesController#edit (and add #update) to edit the course itself: name, code and semester. Editing the Course model belongs on the courses resource, so this reuses the conventional edit/update routes (GET /courses/:id/edit, PATCH /courses/:id). Semester is entered as a season + year pair. Years run from 2012 through next year (Course.semester_year_options); Course.parse_semester splits a stored value into [season, year] when it is well formed, so the dropdowns pre-select valid values and stay blank for anything in an unexpected format, while the raw value is still shown beneath the picker. The stored semester is only overwritten when both dropdowns are set, preserving a malformed value the picker could not represent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the single Settings link (and the separate Form link) with a collapsible "Settings" heading that expands to four pages: Course Details, Approvals, Email Templates and Request Form. The group is expanded and highlighted whenever the current page is one of them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the per-action @side_nav instance variable (set in four controllers) with ApplicationHelper#sidebar_section, which maps the current controller_name/action_name to a sidebar key. The view layer now owns navigation-highlight logic instead of every controller action having to remember to set an ivar. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The collapsible group is now labelled "Course Settings". Only the selected sub-page is highlighted — the parent header no longer gets the active (yellow) treatment, so a single item reads as current. The header keeps its collapse toggle and stays expanded on any settings page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Route the settings pages under a clean /courses/:id/settings/ prefix (approvals, emails, form) and drop the redundant "course" from the URL; path helpers are unchanged. - Move Course Details instructor check into a require_course_instructor before_action for consistent permissions. - Order semester seasons Winter→Spring→Summer→Fall. - Drop explanatory comments flagged in review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Boolean column (default false) marking sandbox/demo courses, e.g. the developer-login test course seeded in development. It only helps track usage so these can be excluded from real metrics and has no effect on behavior. Seed the DEV101 course with it enabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1
cycomachead
changed the title
- Rename the "Approvals" page/link to "Automatic Approvals" and leave only the auto-approval rules on it. - Move the extensions toggle, Gradescope, and notification (email + Slack) sections onto Course Details, along with the delete-course button. courses#update now saves the Course and its CourseSettings together and sends the Slack ping when the webhook is newly enabled; the Slack logic is removed from course_settings#update. - Show "Unknown Semester" on Course Details when the course has no semester. - Reduce the sidebar sub-item indentation so titles no longer wrap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
This is close, but will maybe wait until after I merge in the background job settings? |
Contributor
Author
|
- Consolidate the default email subject/template onto CourseSettings (heredoc) and reference it from the reset action, the Canvas import, and the Email Templates view; drop the duplicate controller constants. - Set @course_settings via a single set_course_settings before_action. - Redirect settings updates back to the originating page with redirect_back_or_to instead of a page-param helper; drop the page field. - Authorize Course Details with course.course_staff? rather than a role string, and drop the leftover action comments. - Add a "Demo course" checkbox to Course Details (permit demo_course). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Move slack_webhook_just_enabled? and slack_enabled_message onto CourseSettings, where the state they read lives; the controller just orchestrates the notify call. - Rename ApplicationHelper#sidebar_section to current_nav_page. - Replace the _sidebar_menu_item partial with a sidebar_nav_item helper that derives active state from current_nav_page and takes either a text: label or a block (used for the requests badge). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
17 tasks
…or-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1
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
1 participant
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
Refactors the course settings UI from a single tabbed page into focused, bookmarkable pages grouped under a collapsible "Course Settings" sidebar heading. Adds a new Course Details page for editing course name, code, and semester.
Approach: Course Details lives on
courses#edit/update(it edits theCourserecord itself, so it belongs on the conventional resource route). Approvals and Email Templates shareCourseSettingsControllersince they both editCourseSettings— mirroring the existingFormSettingsControllerpattern. All settings URLs now live under/courses/:id/settings/.Key changes:
/settings/approvalsand/settings/emails. Removes the?tab=query param + JShistory.pushStatehack.courses#edit) exposes name, code, and semester. Semester uses season + year dropdowns (Winter/Spring/Summer/Fall, 2012–next year). If the stored value doesn't match the expected format, it's displayed as-is with the dropdowns left blank.@side_nav = '...'ivar assignments with a singleApplicationHelper#sidebar_sectionhelper that derives the active nav key fromcontroller_name/action_name.unless @role == 'instructor'checks with abefore_action :require_course_instructoronCoursesController.demo_courseboolean column (defaultfalse) toCourseto flag sandbox/demo courses for usage tracking. No behavioral effect.Testing
course_settings,form_settingsfeatures): 7 scenarios, 59 steps, all passingCoursesController#update(name/code/semester update, blank-dropdown preservation, non-instructor redirect, validation failure) andCourseSettingsController#approvals/#emailssidebar_sectionmappingsCourse.semester_year_optionsandCourse.parse_semester(valid input, unrecognized season, out-of-range year, malformed input)Screenshots
Documentation
No documentation changes required.
Checklist
Superconductor Ticket Implementation | App Preview | Guided Review