Split Course Settings Into Multiple Pages - #453
Merged
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
…omachead/145-basic-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1 * 'main' of github.com:berkeley-cdss/flextensions: chore: autocorrect RSpec/MatchWithSimpleRegex offenses chore: use Rails.env.local? for developer provider gate refactor: simplify developer login gate and trim comments fix: prevent account takeover by splitting user lookup paths refactor: remove redundant TokenRefreshable from SessionController feat: update assignments table styling and sync functionality feat: extend user sessions via token refresh and persistent cookies tweak postion of the page size dropdown style(test): fix rubocop layout offenses in staging interceptor spec feat: implement email interceptor for staging environment Bump json from 2.19.5 to 2.20.0 Bump rubocop-rspec from 3.9.0 to 3.10.2 Bump sentry-rails from 6.5.0 to 6.6.2 Bump axe-core-rspec from 4.11.3 to 4.12.0 Bump brakeman from 8.0.4 to 8.0.5 feat: update pagination and page size options for pending requests refactor: trust top-level base dates, drop all_dates parsing refactor: simplify Canvas base-date handling and document the API feat: group Canvas overrides by extension length fix: audit and resolve Canvas assignment override bugs
- 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>
- "Reset to Default" is now a client-side action: a Stimulus controller fills the subject/body fields from defaults sent down in the HTML. The user must still Save to persist, so the reset click is non-destructive. Removed the reset_email_template server branch and its route flow. - sidebar_nav_item sets aria-current="page" on the active link. - Drop the redundant current_nav_page comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
20 tasks
…or-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1
…-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1 # Conflicts: # app/controllers/course_settings_controller.rb # app/controllers/courses_controller.rb # app/controllers/requests_controller.rb # app/javascript/controllers/course_settings_controller.js # app/models/course.rb # app/models/course_settings.rb # app/views/courses/edit.html.erb # app/views/layouts/_sidebar.html.erb # db/schema.rb # spec/controllers/course_settings_controller_spec.rb # spec/features/accessibility_spec.rb
Post-merge fixes integrating main: - Course Details update uses params.expect and staff_user?/require_course_staff!; drop the removed @role/determine_user_role path. - Carry main's pending-notification fields onto the Course Details Notifications section and permit them in courses#update; keep the approvals/emails split trimmed to their own fields. - Keep togglePendingNotificationEmail in the Stimulus controller; drop the obsolete tab logic. - Move pending-notification controller specs onto CoursesController; fix Enrollment/lms_id usage in the settings specs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
oauth2 >= 2.0 raises when OAuth2::AccessToken is built with a blank token, which the developer omniauth provider always has. Fall back to a stub token so local developer login works again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nges warning - Rename the "Extensions" card to "Enable Extensions"; when student requests are off, highlight it with an info border + explanatory notice, and render the toggle as an enlarged .checkbox-xl switch. - Delete Course button now reads exactly "Delete Course"; when extensions are enabled (delete not allowed) it is disabled and a Bootstrap tooltip (new tooltip Stimulus controller) explains why. - Add an unsaved-changes Stimulus controller: any settings change shows a toast and arms a beforeunload prompt; saving clears both. - Add Capybara feature specs (rack_test for the server-rendered states, a :js/:a11y example for the toast + save-clears-warning flow). 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 # Conflicts: # spec/Helpers/application_helper_spec.rb
- Add a Slack webhook instructions link + "individual Slack message" note below the webhook box on Course Details. - Move the Enable email notifications toggle and Course Reply Email Address off Course Details onto the Email Templates page (below an hr), and permit those fields on course_settings#update instead of courses#update. - Rename the Course Details "Notifications" card to "Staff Notifications". - Update controller + feature specs for the new field placement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Reorder the Email Templates card: the guidance note, Enable email notifications toggle, and Course Reply Email Address now sit above the subject/body template (separated by an hr). - Add an info note at the top explaining what the notifications do, the recommendation to keep them enabled with a monitored reply-to, the upcoming approve/deny template split, and the noreply guidance. - Extend the feature spec to assert the guidance text and the ordering. 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 # Conflicts: # app/views/courses/edit.html.erb # spec/Helpers/application_helper_spec.rb
…tings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1' into cycomachead/145-basic-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1
…bar-for-settings-to-be-a-links-under-a-collapsible-heading/1' of github.com:berkeley-cdss/flextensions into cycomachead/145-basic-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1 * 'cycomachead/145-basic-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1' of github.com:berkeley-cdss/flextensions: (236 commits) Show staff notes via a click popover on a button, not a hover tooltip Fix enrollments page tooltips/popovers not loading feat: refine instructor request review display Email Templates: move email settings above the template and add guidance Add text about shared notes status show notes on the enrollments page via tooltip db:migrate undo file rename (migrations were already run) feat: add student review context and history to instructor view Tidy the footer a bit Reorganize the sync buttons on the assignments page Update cron job times to 4:00PM feat: add instructor review context to request show page feat: add hourly email notifications and enable GoodJob cron Move email settings to Email Templates; add Slack webhook instructions refactor: left-align emails and add copy-to-clipboard functionality Course Details: enable-extensions emphasis, delete guard, unsaved-changes warning feat: add deployment timestamp and commit SHA to footer feat: add release date to assignments feat: implement Null Object Pattern for current_user ...
…or-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1 * main: fix: restrict API write endpoints to test environment perf: preload and cache Canvas LMS entry at boot
…aved-changes - Seed DEFAULT_EMAIL_SUBJECT/TEMPLATE on the course_settings row at creation (before_create) and drop the email_subject DB column default. - Sidebar: rename 'Request for Student' to 'New Request' and pin the sidebar to a fixed 250px so opening/closing the submenu no longer shifts its width. - Course Details: move the Enable Extensions card to the top of the page. - Add the unsaved-changes toast + beforeunload warning to the Automatic Approvals, Email Templates, and Request Form pages via a shared partial. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cycomachead
deleted the
cycomachead/145-basic-course-settings-refactor-the-sidebar-for-settings-to-be-a-links-under-a-collapsible-heading/1
branch
August 6, 2026 09:33
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