From e82c3b5692c30dd9ca636a4823445dc90301a303 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 23 Jun 2026 01:36:36 +0000 Subject: [PATCH 01/10] Split course settings into Approvals and Email Templates pages 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 --- app/controllers/course_settings_controller.rb | 28 +- app/controllers/courses_controller.rb | 5 +- .../controllers/course_settings_controller.js | 14 +- app/views/course_settings/approvals.html.erb | 236 ++++++++++++ app/views/course_settings/emails.html.erb | 77 ++++ app/views/courses/edit.html.erb | 335 ------------------ app/views/courses/enrollments.html.erb | 2 +- app/views/layouts/_sidebar.html.erb | 4 +- .../shared/_extension_status_warning.html.erb | 2 +- config/routes.rb | 9 +- features/support/paths.rb | 2 +- .../course_settings_controller_spec.rb | 68 ++-- 12 files changed, 395 insertions(+), 387 deletions(-) create mode 100644 app/views/course_settings/approvals.html.erb create mode 100644 app/views/course_settings/emails.html.erb delete mode 100644 app/views/courses/edit.html.erb diff --git a/app/controllers/course_settings_controller.rb b/app/controllers/course_settings_controller.rb index 3abe25e3..6ff25157 100644 --- a/app/controllers/course_settings_controller.rb +++ b/app/controllers/course_settings_controller.rb @@ -17,14 +17,25 @@ class CourseSettingsController < ApplicationController \n\nBest regards, \n{{course_name}} Staff".freeze + # Approval and notification settings (formerly the "General Settings" tab). + def approvals + @side_nav = 'approvals' + @course_settings = @course.course_settings || @course.build_course_settings + end + + # Email subject/body templates (formerly the "Email Settings" tab). + def emails + @side_nav = 'emails' + @course_settings = @course.course_settings || @course.build_course_settings + end + # rubocop:disable Metrics/AbcSize def update - @side_nav = 'course_settings' @course_settings = @course.course_settings || @course.build_course_settings if params[:reset_email_template].present? reset_email_templates - redirect_to course_settings_path(@course, tab: 'email'), notice: 'Email templates reset to defaults.' + redirect_to emails_course_settings_path(@course), notice: 'Email templates reset to defaults.' elsif @course_settings.update(course_settings_params) if @course_settings.enable_slack_webhook_url && @course_settings.slack_webhook_url.present? && @@ -35,22 +46,27 @@ def update @course_settings.slack_webhook_url ) unless success - redirect_to course_settings_path(@course, tab: params[:tab]), alert: 'Failed to send Slack notification. Please check the webhook URL.' + redirect_to settings_redirect_path, alert: 'Failed to send Slack notification. Please check the webhook URL.' return end - redirect_to course_settings_path(@course, tab: params[:tab]), notice: 'Course settings updated successfully. Check your Slack channel for Notifications.' + redirect_to settings_redirect_path, notice: 'Course settings updated successfully. Check your Slack channel for Notifications.' return end - redirect_to course_settings_path(@course, tab: params[:tab]), notice: 'Course settings updated successfully.' + redirect_to settings_redirect_path, notice: 'Course settings updated successfully.' else flash[:alert] = "Failed to update course settings: #{@course_settings.errors.full_messages.to_sentence}" - redirect_to course_settings_path(@course, tab: params[:tab]) + redirect_to settings_redirect_path end end # rubocop:enable Metrics/AbcSize private + # Redirects back to the settings page the form was submitted from. + def settings_redirect_path + params[:page] == 'emails' ? emails_course_settings_path(@course) : approvals_course_settings_path(@course) + end + def reset_email_templates @course_settings.update( email_subject: DEFAULT_EMAIL_SUBJECT, diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index aa5cd469..45f95c5b 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -62,8 +62,9 @@ def new end def edit - @side_nav = 'edit' - redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' + return redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' + + redirect_to approvals_course_settings_path(@course) end def create diff --git a/app/javascript/controllers/course_settings_controller.js b/app/javascript/controllers/course_settings_controller.js index 19a4ff03..bcc037a9 100644 --- a/app/javascript/controllers/course_settings_controller.js +++ b/app/javascript/controllers/course_settings_controller.js @@ -1,7 +1,7 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = ["emailField", "tab", "gradescopeField", "slackWebhookField"]; + static targets = ["emailField", "gradescopeField", "slackWebhookField"]; connect() { this.toggleEmailFields(); @@ -52,16 +52,4 @@ export default class extends Controller { slackWebhookField.disabled = !slackToggle.checked; } } - - updateUrlParam(event) { - const tabName = event.currentTarget.dataset.tab; - const url = new URL(window.location); - url.searchParams.set('tab', tabName); - window.history.pushState({}, '', url); - - const tabInput = document.getElementById('tab'); - if (tabInput) { - tabInput.value = tabName; - } - } } diff --git a/app/views/course_settings/approvals.html.erb b/app/views/course_settings/approvals.html.erb new file mode 100644 index 00000000..d84ee74a --- /dev/null +++ b/app/views/course_settings/approvals.html.erb @@ -0,0 +1,236 @@ +
+
+
+ <%= render "courses/course_title" %> +

Approvals

+ <%= form_with url: course_settings_path(@course), method: :patch do |form| %> + <%= hidden_field_tag :page, 'approvals' %> +
+
+

Approvals

+
+
+
+
+ <%= hidden_field_tag 'course_settings[enable_extensions]', false %> + <%= check_box_tag 'course_settings[enable_extensions]', + true, + @course.course_settings&.enable_extensions, + class: 'form-check-input', + id: 'course-enable' %> + +
+
+ +
+ +
+ <%= number_field_tag 'course_settings[auto_approve_days]', + @course.course_settings&.auto_approve_days, + min: 0, + step: 1, + class: 'form-control', + id: 'auto-approve', + placeholder: 'Number of days' %> + Extensions requested within this many days will be automatically approved, subject to the maximum limit below. +
+
+ +
+ +
+ <%= number_field_tag 'course_settings[auto_approve_extended_request_days]', + @course.course_settings&.auto_approve_extended_request_days, + min: 0, + step: 1, + class: 'form-control', + id: 'auto-approve-dsp', + placeholder: 'Number of days' %> + + This allows students who have 'Allow Extended Requests' checked to have requests automatically approved within an extended timeframe. Use this for students who have extenuating circumstances. + +
+
+ +
+ +
+ <%= number_field_tag 'course_settings[max_auto_approve]', + @course.course_settings&.max_auto_approve, + min: 0, + step: 1, + class: 'form-control', + id: 'max-auto-approve', + placeholder: 'Number' %> + This limit applies per student per course. Set to zero for no limit on auto-approved requests. +
+
+ + +
+
+ <%= hidden_field_tag 'course_settings[enable_min_hours_before_deadline]', false %> + <%= check_box_tag 'course_settings[enable_min_hours_before_deadline]', + true, + @course.course_settings&.enable_min_hours_before_deadline.nil? ? true : @course.course_settings&.enable_min_hours_before_deadline, + class: 'form-check-input', + id: 'enable-min-hours-before-deadline' %> + +
+
+ +
+ +
+ <%= number_field_tag 'course_settings[min_hours_before_deadline]', + @course.course_settings&.min_hours_before_deadline || 0, + min: 0, + step: 1, + class: 'form-control', + id: 'min-hours-before-deadline', + placeholder: 'Number of hours' %> + + When enabled, a request is only auto-approved if it is made at least this many hours before the assignment's deadline; otherwise it is held for manual review. + A value of 0 still requires the deadline to not have passed. Recommended values are 0 or 24. + +
+
+ +
+
+ <%= hidden_field_tag 'course_settings[extend_late_due_date]', false %> + <%= check_box_tag 'course_settings[extend_late_due_date]', + true, + @course.course_settings&.extend_late_due_date.nil? ? true : @course.course_settings&.extend_late_due_date, + class: 'form-check-input', + id: 'extend-late-due-date' %> + +
+ + When enabled, the late due date (if present) will be shifted by the same amount as the extension. + When disabled, the new late due date will be set to the later of the original late due date and the extended due date. + +
+ +
+
+ <%= hidden_field_tag 'course_settings[enable_gradescope]', false %> + <%= check_box_tag 'course_settings[enable_gradescope]', + true, + @course.course_settings&.enable_gradescope, + class: 'form-check-input', + id: 'enable-gradescope' %> + +
+
+ +
+ +
+ <%= url_field_tag 'course_settings[gradescope_course_url]', + @course.course_settings&.gradescope_course_url, + class: 'form-control', + id: 'gradescope-course-url', + placeholder: 'https://www.gradescope.com/courses/123456', + data: { course_settings_target: "gradescopeField" }, + disabled: !@course.course_settings&.enable_gradescope, + pattern: 'https://(www\.)?gradescope\.com/courses/\d+/?', + title: 'Must be a valid Gradescope course URL (e.g. https://www.gradescope.com/courses/123456)' %> + + Please invite the user <%= ENV.fetch('GRADESCOPE_EMAIL') { 'gradescope-bot@berkeley.edu' } %> + as a TA to your Gradescope course for integration to work. + + + +
+
+ +
+
+ <%= hidden_field_tag 'course_settings[enable_emails]', false %> + <%= check_box_tag 'course_settings[enable_emails]', + true, + @course.course_settings&.enable_emails, + class: 'form-check-input', + id: 'enable-email' %> + +
+
+ +
+ +
+ <%= email_field_tag 'course_settings[reply_email]', + @course.course_settings&.reply_email, + class: 'form-control', + id: 'reply-email', + placeholder: 'example@example.com', + data: { course_settings_target: "emailField" }, + disabled: !@course.course_settings&.enable_emails %> +
+
+ +
+
+ <%= hidden_field_tag 'course_settings[enable_slack_webhook_url]', false %> + <%= check_box_tag 'course_settings[enable_slack_webhook_url]', + true, + @course.course_settings&.enable_slack_webhook_url, + class: 'form-check-input', + id: 'enable-slack' %> + +
+
+ +
+ +
+ <%= text_field_tag 'course_settings[slack_webhook_url]', + @course.course_settings&.slack_webhook_url, + class: 'form-control', + id: 'slack-webhook', + placeholder: 'https://hooks.slack.com/services/...', + data: { course_settings_target: "slackWebhookField" }, + disabled: !@course.course_settings&.enable_slack_webhook_url %> +
+
+ +
+
+ <% if @course.course_settings&.enable_extensions %> + <%= link_to 'Delete Course (must have student extension requests disabled)', '#', class: 'btn btn-outline-danger disable me-3', aria_disabled: 'true' %> + <% else %> + <%= link_to 'Delete Course', + delete_course_path(@course), + method: :delete, + class: 'btn btn-danger me-3', + data: { + confirm: 'Are you sure you want to delete this course? This action cannot be undone.' + } %> + <% end %> +
+
+
+
+ +
+ <%= form.submit 'Save Settings', class: 'btn btn-success px-4' %> +
+ <% end %> +
+
+
diff --git a/app/views/course_settings/emails.html.erb b/app/views/course_settings/emails.html.erb new file mode 100644 index 00000000..3ed9da25 --- /dev/null +++ b/app/views/course_settings/emails.html.erb @@ -0,0 +1,77 @@ +
+
+
+ <%= render "courses/course_title" %> +

Email Templates

+ <%= form_with url: course_settings_path(@course), method: :patch do |form| %> + <%= hidden_field_tag :page, 'emails' %> +
+
+

Email Template

+ <%= button_tag "Reset to Default", + name: "reset_email_template", + value: "true", + class: "btn btn-secondary btn-sm", + type: "submit" %> +
+
+
+ + <%= text_field_tag 'course_settings[email_subject]', + @course.course_settings&.email_subject || "Extension Request Status: {{status}} - {{course_code}}", + class: 'form-control', + id: 'email-subject' %> + Use {{variable_name}} to insert dynamic values +
+ +
+ + <%= text_area_tag 'course_settings[email_template]', + @course.course_settings&.email_template || "Dear {{student_name}},\n\nYour extension request for {{assignment_name}} in {{course_name}} ({{course_code}}) has been {{status}}.\n\nExtension Details:\n- Original Due Date: {{original_due_date}}\n- New Due Date: {{new_due_date}}\n- Extension Days: {{extension_days}}\n\nIf you have any questions, please contact the course staff.\n\nBest regards,\n{{course_name}} Staff", + class: 'form-control', + id: 'email-template', + rows: 10 %> +
+ +
+
+

Available Variables

+
+
+

Student Information:

+
    +
  • {{student_name}} - Full name
    • +
  • {{student_email}} - Email
    • +
  • {{student_id}} - ID number
    • +
+
+
+

Course Information:

+
    +
  • {{course_name}} - Course name
    • +
  • {{course_code}} - Course code
    • +
  • {{assignment_name}} - Assignment
    • +
+
+
+

Extension Information:

+
    +
  • {{original_due_date}} - Original due date
    • +
  • {{new_due_date}} - New due date
    • +
  • {{extension_days}} - Days extended
    • +
  • {{status}} - Status (Approved/Denied)
    • +
+
+
+
+
+
+
+ +
+ <%= form.submit 'Save Settings', class: 'btn btn-success px-4' %> +
+ <% end %> +
+
+
diff --git a/app/views/courses/edit.html.erb b/app/views/courses/edit.html.erb deleted file mode 100644 index 64d90796..00000000 --- a/app/views/courses/edit.html.erb +++ /dev/null @@ -1,335 +0,0 @@ -
-
-
- <%= render "courses/course_title" %> -

Course Settings

-
    - - -
- <%= form_with url: course_settings_update_path, method: :post do |form| %> - <%= hidden_field_tag :course_id, @course.id %> - <%= hidden_field_tag :tab, params[:tab] || 'general' %> -
-
-
-
-

General Settings

-
-
-
-
- <%= hidden_field_tag 'course_settings[enable_extensions]', false %> - <%= check_box_tag 'course_settings[enable_extensions]', - true, - @course.course_settings&.enable_extensions, - class: 'form-check-input', - id: 'course-enable' %> - -
-
- -
- -
- <%= number_field_tag 'course_settings[auto_approve_days]', - @course.course_settings&.auto_approve_days, - min: 0, - step: 1, - class: 'form-control', - id: 'auto-approve', - placeholder: 'Number of days' %> - Extensions requested within this many days will be automatically approved, subject to the maximum limit below. -
-
- -
- -
- <%= number_field_tag 'course_settings[auto_approve_extended_request_days]', - @course.course_settings&.auto_approve_extended_request_days, - min: 0, - step: 1, - class: 'form-control', - id: 'auto-approve-dsp', - placeholder: 'Number of days' %> - - This allows students who have 'Allow Extended Requests' checked to have requests automatically approved within an extended timeframe. Use this for students who have extenuating circumstances. - -
-
- -
- -
- <%= number_field_tag 'course_settings[max_auto_approve]', - @course.course_settings&.max_auto_approve, - min: 0, - step: 1, - class: 'form-control', - id: 'max-auto-approve', - placeholder: 'Number' %> - This limit applies per student per course. Set to zero for no limit on auto-approved requests. -
-
- - -
-
- <%= hidden_field_tag 'course_settings[enable_min_hours_before_deadline]', false %> - <%= check_box_tag 'course_settings[enable_min_hours_before_deadline]', - true, - @course.course_settings&.enable_min_hours_before_deadline.nil? ? true : @course.course_settings&.enable_min_hours_before_deadline, - class: 'form-check-input', - id: 'enable-min-hours-before-deadline' %> - -
-
- -
- -
- <%= number_field_tag 'course_settings[min_hours_before_deadline]', - @course.course_settings&.min_hours_before_deadline || 0, - min: 0, - step: 1, - class: 'form-control', - id: 'min-hours-before-deadline', - placeholder: 'Number of hours' %> - - When enabled, a request is only auto-approved if it is made at least this many hours before the assignment's deadline; otherwise it is held for manual review. - A value of 0 still requires the deadline to not have passed. Recommended values are 0 or 24. - -
-
- -
-
- <%= hidden_field_tag 'course_settings[extend_late_due_date]', false %> - <%= check_box_tag 'course_settings[extend_late_due_date]', - true, - @course.course_settings&.extend_late_due_date.nil? ? true : @course.course_settings&.extend_late_due_date, - class: 'form-check-input', - id: 'extend-late-due-date' %> - -
- - When enabled, the late due date (if present) will be shifted by the same amount as the extension. - When disabled, the new late due date will be set to the later of the original late due date and the extended due date. - -
- -
-
- <%= hidden_field_tag 'course_settings[enable_gradescope]', false %> - <%= check_box_tag 'course_settings[enable_gradescope]', - true, - @course.course_settings&.enable_gradescope, - class: 'form-check-input', - id: 'enable-gradescope' %> - -
-
- -
- -
- <%= url_field_tag 'course_settings[gradescope_course_url]', - @course.course_settings&.gradescope_course_url, - class: 'form-control', - id: 'gradescope-course-url', - placeholder: 'https://www.gradescope.com/courses/123456', - data: { course_settings_target: "gradescopeField" }, - disabled: !@course.course_settings&.enable_gradescope, - pattern: 'https://(www\.)?gradescope\.com/courses/\d+/?', - title: 'Must be a valid Gradescope course URL (e.g. https://www.gradescope.com/courses/123456)' %> - - Please invite the user <%= ENV.fetch('GRADESCOPE_EMAIL') { 'gradescope-bot@berkeley.edu' } %> - as a TA to your Gradescope course for integration to work. - - - -
-
- -
-
- <%= hidden_field_tag 'course_settings[enable_emails]', false %> - <%= check_box_tag 'course_settings[enable_emails]', - true, - @course.course_settings&.enable_emails, - class: 'form-check-input', - id: 'enable-email' %> - -
-
- -
- -
- <%= email_field_tag 'course_settings[reply_email]', - @course.course_settings&.reply_email, - class: 'form-control', - id: 'reply-email', - placeholder: 'example@example.com', - data: { course_settings_target: "emailField" }, - disabled: !@course.course_settings&.enable_emails %> -
-
- -
-
- <%= hidden_field_tag 'course_settings[enable_slack_webhook_url]', false %> - <%= check_box_tag 'course_settings[enable_slack_webhook_url]', - true, - @course.course_settings&.enable_slack_webhook_url, - class: 'form-check-input', - id: 'enable-slack' %> - -
-
- -
- -
- <%= text_field_tag 'course_settings[slack_webhook_url]', - @course.course_settings&.slack_webhook_url, - class: 'form-control', - id: 'slack-webhook', - placeholder: 'https://hooks.slack.com/services/...', - data: { course_settings_target: "slackWebhookField" }, - disabled: !@course.course_settings&.enable_slack_webhook_url %> -
-
- -
-
- <% if @course.course_settings&.enable_extensions %> - <%= link_to 'Delete Course (must have student extension requests disabled)', '#', class: 'btn btn-outline-danger disable me-3', aria_disabled: 'true' %> - <% else %> - <%= link_to 'Delete Course', - delete_course_path, - method: :delete, - class: 'btn btn-danger me-3', - data: { - confirm: 'Are you sure you want to delete this course? This action cannot be undone.' - } %> - <% end %> -
-
-
-
-
- - -
-
-
-

Email Template

- <%= button_tag "Reset to Default", - name: "reset_email_template", - value: "true", - class: "btn btn-secondary btn-sm", - type: "submit" %> -
-
-
- - <%= text_field_tag 'course_settings[email_subject]', - @course.course_settings&.email_subject || "Extension Request Status: {{status}} - {{course_code}}", - class: 'form-control', - id: 'email-subject' %> - Use {{variable_name}} to insert dynamic values -
- -
- - <%= text_area_tag 'course_settings[email_template]', - @course.course_settings&.email_template || "Dear {{student_name}},\n\nYour extension request for {{assignment_name}} in {{course_name}} ({{course_code}}) has been {{status}}.\n\nExtension Details:\n- Original Due Date: {{original_due_date}}\n- New Due Date: {{new_due_date}}\n- Extension Days: {{extension_days}}\n\nIf you have any questions, please contact the course staff.\n\nBest regards,\n{{course_name}} Staff", - class: 'form-control', - id: 'email-template', - rows: 10 %> -
- -
-
-

Available Variables

-
-
-

Student Information:

-
    -
  • {{student_name}} - Full name
    • -
  • {{student_email}} - Email
    • -
  • {{student_id}} - ID number
    • -
-
-
-

Course Information:

-
    -
  • {{course_name}} - Course name
    • -
  • {{course_code}} - Course code
    • -
  • {{assignment_name}} - Assignment
    • -
-
-
-

Extension Information:

-
    -
  • {{original_due_date}} - Original due date
    • -
  • {{new_due_date}} - New due date
    • -
  • {{extension_days}} - Days extended
    • -
  • {{status}} - Status (Approved/Denied)
    • -
-
-
-
-
-
-
-
-
- -
- <%= form.submit 'Save Settings', class: 'btn btn-success px-4' %> -
- <% end %> -
-
-
diff --git a/app/views/courses/enrollments.html.erb b/app/views/courses/enrollments.html.erb index 12e577bd..6f52158b 100644 --- a/app/views/courses/enrollments.html.erb +++ b/app/views/courses/enrollments.html.erb @@ -22,7 +22,7 @@ data-bs-toggle="popover" data-bs-trigger="focus" data-bs-html="true" - data-bs-content="Allow students to have an additional window for automatically approved requests, such as for extenuating circumstances. Set the limit from the settings page." + data-bs-content="Allow students to have an additional window for automatically approved requests, such as for extenuating circumstances. Set the limit from the settings page." aria-label="Help: additional request window settings"> diff --git a/app/views/layouts/_sidebar.html.erb b/app/views/layouts/_sidebar.html.erb index f984823c..187e6090 100644 --- a/app/views/layouts/_sidebar.html.erb +++ b/app/views/layouts/_sidebar.html.erb @@ -35,10 +35,10 @@ active: @side_nav == 'enrollments' %> <%= render 'layouts/components/sidebar_menu_item', - path: course_settings_path(@course.id), + path: approvals_course_settings_path(@course.id), icon: 'fas fa-cog', text: 'Settings', - active: @side_nav == 'edit' %> + active: %w[approvals emails].include?(@side_nav) %> <%= render 'layouts/components/sidebar_menu_item', path: edit_course_form_setting_path(@course.id), diff --git a/app/views/shared/_extension_status_warning.html.erb b/app/views/shared/_extension_status_warning.html.erb index 2d33be2d..ba7270e3 100644 --- a/app/views/shared/_extension_status_warning.html.erb +++ b/app/views/shared/_extension_status_warning.html.erb @@ -5,6 +5,6 @@ <% unless extensions_enabled %>
- Flextensions is disabled for this course. To allow to students to see this course, please enable extensions in Course Settings. + Flextensions is disabled for this course. To allow to students to see this course, please enable extensions in Approvals settings.
<% end %> \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 2a0448d9..a2e7f40c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -24,7 +24,6 @@ get '/courses', to: 'courses#index', as: 'courses' get '/courses/new', to: 'courses#new', as: :new_course get '/courses/:id', to: 'courses#show', as: :course - get '/courses/:id/edit', to: 'courses#edit', as: :course_settings resources :courses do member do @@ -53,8 +52,14 @@ end end resource :form_setting, only: [:edit, :update] + + # Course-level settings split into focused, bookmarkable pages. They all + # edit the single CourseSettings record, so they live on one controller. + resource :course_settings, only: [:update], controller: :course_settings, as: :settings do + get :approvals + get :emails + end end - post 'course_settings/update' resources :assignments do member do diff --git a/features/support/paths.rb b/features/support/paths.rb index 5b82fa87..41bcab86 100644 --- a/features/support/paths.rb +++ b/features/support/paths.rb @@ -24,7 +24,7 @@ def path_to(page_name) "courses/#{@course.id}/enrollments" when /^Course Settings page$/ - "/courses/#{@course.id}/edit" + "/courses/#{@course.id}/course_settings/approvals" when /^Form Settings page$/ "/courses/#{@course.id}/form_setting/edit" diff --git a/spec/controllers/course_settings_controller_spec.rb b/spec/controllers/course_settings_controller_spec.rb index c6ac03a8..25dddf7b 100644 --- a/spec/controllers/course_settings_controller_spec.rb +++ b/spec/controllers/course_settings_controller_spec.rb @@ -25,7 +25,7 @@ it 'creates new course settings when none exist' do expect(CourseSettings.where(course_id: course.id).count).to eq(0) - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true', @@ -33,12 +33,12 @@ auto_approve_extended_request_days: '5', enable_emails: 'true' }, - tab: 'general' + page: 'approvals' } # Now verify a new settings record was created expect(CourseSettings.where(course_id: course.id).count).to eq(1) - expect(response).to redirect_to(course_settings_path(course.id, tab: 'general')) + expect(response).to redirect_to(approvals_course_settings_path(course.id)) expect(flash[:notice]).to eq('Course settings updated successfully.') settings = CourseSettings.find_by(course_id: course.id) expect(settings.enable_extensions).to be true @@ -58,17 +58,17 @@ expect(CourseSettings.where(course_id: course.id).count).to eq(1) - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true', auto_approve_days: '3' }, - tab: 'general' + page: 'approvals' } expect(CourseSettings.where(course_id: course.id).count).to eq(1) # Still only 1 record - expect(response).to redirect_to(course_settings_path(course.id, tab: 'general')) + expect(response).to redirect_to(approvals_course_settings_path(course.id)) expect(flash[:notice]).to eq('Course settings updated successfully.') # Force reload to get updated values @@ -87,13 +87,13 @@ expect(CourseSettings.where(course_id: course.id).count).to eq(1) allow_any_instance_of(CourseSettings).to receive(:update).and_return(false) - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true' }, - tab: 'general' + page: 'approvals' } - expect(response).to redirect_to(course_settings_path(course.id, tab: 'general')) + expect(response).to redirect_to(approvals_course_settings_path(course.id)) expect(flash[:alert]).to include('Failed to update course settings:') end @@ -105,17 +105,37 @@ email_template: 'Custom Template' ) - post :update, params: { + patch :update, params: { course_id: course.id, reset_email_template: true, - tab: 'email' + page: 'emails' } - expect(response).to redirect_to(course_settings_path(course.id, tab: 'email')) + expect(response).to redirect_to(emails_course_settings_path(course.id)) expect(flash[:notice]).to eq('Email templates reset to defaults.') # We won't test the exact content since that requires knowledge of the constants end end + + describe 'GET #approvals' do + it 'renders the approvals page' do + get :approvals, params: { course_id: course.id } + + expect(response).to have_http_status(:ok) + expect(response).to render_template(:approvals) + expect(assigns(:side_nav)).to eq('approvals') + end + end + + describe 'GET #emails' do + it 'renders the email templates page' do + get :emails, params: { course_id: course.id } + + expect(response).to have_http_status(:ok) + expect(response).to render_template(:emails) + expect(assigns(:side_nav)).to eq('emails') + end + end end describe 'pending requests count' do @@ -162,10 +182,10 @@ requests = instance_double(ActiveRecord::Relation, count: 1) allow(Request).to receive(:where).with(course_id: course.id, status: 'pending').and_return(requests) - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true' }, - tab: 'general' + page: 'approvals' } expect(assigns(:pending_requests_count)).to eq(1) @@ -176,10 +196,10 @@ requests = instance_double(ActiveRecord::Relation, count: 0) allow(Request).to receive(:where).with(course_id: course.id, status: 'pending').and_return(requests) - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true' }, - tab: 'general' + page: 'approvals' } expect(assigns(:pending_requests_count)).to eq(0) @@ -207,13 +227,13 @@ end it 'denies access to update course settings' do - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true', auto_approve_days: '99' }, - tab: 'general' + page: 'approvals' } expect(response).to redirect_to(courses_path) @@ -225,10 +245,10 @@ end it 'denies access to reset email templates' do - post :update, params: { + patch :update, params: { course_id: course.id, reset_email_template: true, - tab: 'email' + page: 'emails' } expect(response).to redirect_to(courses_path) @@ -240,10 +260,10 @@ it 'redirects to root path when user is not authenticated' do session[:user_id] = 'non_existent_id' - post :update, params: { + patch :update, params: { course_id: course.id, course_settings: { enable_extensions: 'true' }, - tab: 'general' + page: 'approvals' } expect(response).to redirect_to(root_path) @@ -254,10 +274,10 @@ session[:user_id] = instructor.canvas_uid UserToCourse.create!(user: instructor, course: course, role: 'instructor') - post :update, params: { + patch :update, params: { course_id: 999, course_settings: { enable_extensions: 'true' }, - tab: 'general' + page: 'approvals' } expect(response).to redirect_to(courses_path) From 16dfc254d3a850f4511de1f0f3849af7ae27520d Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 23 Jun 2026 01:40:50 +0000 Subject: [PATCH 02/10] Add Course Details settings page 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 --- app/controllers/courses_controller.rb | 40 +++++++++-- app/models/course.rb | 22 ++++++ app/views/courses/edit.html.erb | 66 ++++++++++++++++++ spec/controllers/courses_controller_spec.rb | 75 +++++++++++++++++++++ spec/models/course_spec.rb | 34 ++++++++++ spec/rails_helper.rb | 3 + 6 files changed, 236 insertions(+), 4 deletions(-) create mode 100644 app/views/courses/edit.html.erb diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 45f95c5b..c5470511 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -1,6 +1,6 @@ class CoursesController < ApplicationController before_action :authenticate_user - before_action :set_course, only: %i[show edit sync_assignments sync_enrollments enrollments delete] + before_action :set_course, only: %i[show edit update sync_assignments sync_enrollments enrollments delete] before_action :set_pending_request_count before_action :determine_user_role @@ -61,10 +61,10 @@ def new end end + # Course Details: edit the course name, code and semester. def edit - return redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' - - redirect_to approvals_course_settings_path(@course) + @side_nav = 'course_details' + redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' end def create @@ -75,6 +75,24 @@ def create redirect_to courses_path, notice: 'Selected courses and their assignments have been imported successfully.' end + def update + @side_nav = 'course_details' + return redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' + + attrs = course_params.to_h + # Only overwrite the semester when both dropdowns are set; this preserves a + # value stored in an unexpected format that the picker left blank. + semester = combined_semester + attrs[:semester] = semester if semester.present? + + if @course.update(attrs) + redirect_to edit_course_path(@course), notice: 'Course details updated successfully.' + else + flash.now[:alert] = "Failed to update course details: #{@course.errors.full_messages.to_sentence}" + render :edit, status: :unprocessable_content + end + end + def sync_assignments return render json: { error: 'Course not found.' }, status: :not_found unless @course @@ -118,6 +136,20 @@ def delete private + def course_params + params.require(:course).permit(:course_name, :course_code) + end + + # Combines the season + year dropdowns into a "Season Year" string, or nil + # when either is blank. + def combined_semester + season = params.dig(:course, :semester_season) + year = params.dig(:course, :semester_year) + return nil if season.blank? || year.blank? + + "#{season} #{year}" + end + # Returns the time the roster was last synced from Canvas, or nil if never synced. def enrollments_last_synced_at synced_at = @course.course_to_lms&.recent_roster_sync&.dig('synced_at') diff --git a/app/models/course.rb b/app/models/course.rb index 9548512f..0151b286 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -62,6 +62,28 @@ def self.sort_semesters(semesters) semesters.sort_by { |s| semester_sort_key(s) }.reverse end + # Seasons offered in the Course Details semester picker. + SEMESTER_SEASONS = %w[Spring Summer Fall Winter].freeze + # Earliest selectable academic year in the semester picker. + FIRST_SEMESTER_YEAR = 2012 + + # Years offered in the semester picker: FIRST_SEMESTER_YEAR through next year. + def self.semester_year_options(today = Date.current) + (FIRST_SEMESTER_YEAR..today.year + 1).to_a + end + + # Splits a stored semester string (e.g. "Spring 2026") into [season, year] + # when it is a recognized season paired with an in-range year, otherwise + # [nil, nil]. This lets the Course Details form pre-select valid values and + # leave the dropdowns empty for anything stored in an unexpected format. + def self.parse_semester(semester) + season, year = semester.to_s.split + return [ nil, nil ] unless SEMESTER_SEASONS.include?(season) + return [ nil, nil ] unless year&.match?(/\A\d{4}\z/) && semester_year_options.include?(year.to_i) + + [ season, year.to_i ] + end + # Month a term starts in maps to its Berkeley season. # Spring starts in January, Summer in late May, Fall in late August. SEASON_BY_START_MONTH = { diff --git a/app/views/courses/edit.html.erb b/app/views/courses/edit.html.erb new file mode 100644 index 00000000..0a6773d9 --- /dev/null +++ b/app/views/courses/edit.html.erb @@ -0,0 +1,66 @@ +<% selected_season, selected_year = Course.parse_semester(@course.semester) %> +
+
+
+ <%= render "courses/course_title" %> +

Course Details

+ <%= form_with model: @course, url: course_path(@course), method: :patch do |form| %> +
+
+

Course Details

+
+
+
+ +
+ <%= form.text_field :course_name, + class: 'form-control', + id: 'course-name', + placeholder: 'e.g. Introduction to Computer Science' %> +
+
+ +
+ +
+ <%= form.text_field :course_code, + class: 'form-control', + id: 'course-code', + placeholder: 'e.g. COMPSCI 61A' %> +
+
+ +
+ +
+
+
+ <%= select_tag 'course[semester_season]', + options_for_select([ [ 'Season', '' ] ] + Course::SEMESTER_SEASONS.map { |s| [ s, s ] }, selected_season), + class: 'form-select', + id: 'semester-season', + 'aria-label': 'Semester season' %> +
+
+ <%= select_tag 'course[semester_year]', + options_for_select([ [ 'Year', '' ] ] + Course.semester_year_options.reverse.map { |y| [ y, y ] }, selected_year), + class: 'form-select', + id: 'semester-year', + 'aria-label': 'Semester year' %> +
+
+ <% if @course.semester.present? %> + Current semester: <%= @course.semester %> + <% end %> +
+
+
+
+ +
+ <%= form.submit 'Save Course Details', class: 'btn btn-success px-4' %> +
+ <% end %> +
+
+
diff --git a/spec/controllers/courses_controller_spec.rb b/spec/controllers/courses_controller_spec.rb index baa03839..196b7542 100644 --- a/spec/controllers/courses_controller_spec.rb +++ b/spec/controllers/courses_controller_spec.rb @@ -107,6 +107,81 @@ expect(response).to redirect_to(course_path(course)) expect(flash[:alert]).to eq('You do not have access to this page.') end + + context 'as an instructor' do + let(:instructor) { User.create!(email: 'teacher@example.com', canvas_uid: '999', name: 'Teacher') } + + before do + session[:user_id] = instructor.canvas_uid + instructor.lms_credentials.create!( + lms_name: 'canvas', token: 't', refresh_token: 'r', expire_time: 1.hour.from_now + ) + UserToCourse.create!(user: instructor, course: course, role: 'teacher') + end + + it 'renders the Course Details page' do + get :edit, params: { id: course.id } + + expect(response).to render_template(:edit) + expect(assigns(:side_nav)).to eq('course_details') + end + end + end + + describe 'PATCH #update' do + let(:instructor) { User.create!(email: 'teacher@example.com', canvas_uid: '999', name: 'Teacher') } + + before do + session[:user_id] = instructor.canvas_uid + instructor.lms_credentials.create!( + lms_name: 'canvas', token: 't', refresh_token: 'r', expire_time: 1.hour.from_now + ) + UserToCourse.create!(user: instructor, course: course, role: 'teacher') + end + + it 'updates the name, code and semester from the dropdowns' do + patch :update, params: { + id: course.id, + course: { course_name: 'New Name', course_code: 'NEW1', semester_season: 'Fall', semester_year: '2025' } + } + + expect(response).to redirect_to(edit_course_path(course)) + expect(flash[:notice]).to eq('Course details updated successfully.') + course.reload + expect(course.course_name).to eq('New Name') + expect(course.course_code).to eq('NEW1') + expect(course.semester).to eq('Fall 2025') + end + + it 'leaves the semester unchanged when the dropdowns are blank' do + course.update!(semester: 'weird-format') + + patch :update, params: { + id: course.id, + course: { course_name: 'Kept Name', semester_season: '', semester_year: '' } + } + + course.reload + expect(course.course_name).to eq('Kept Name') + expect(course.semester).to eq('weird-format') + end + + it 'redirects non-instructor users' do + session[:user_id] = user.canvas_uid + + patch :update, params: { id: course.id, course: { course_name: 'Nope' } } + + expect(response).to redirect_to(course_path(course)) + expect(flash[:alert]).to eq('You do not have access to this page.') + expect(course.reload.course_name).to eq('Test Course') + end + + it 're-renders with an alert when validation fails' do + patch :update, params: { id: course.id, course: { course_name: '' } } + + expect(response).to have_http_status(:unprocessable_content) + expect(flash[:alert]).to include('Failed to update course details') + end end describe 'POST #create' do diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index f62234ac..5223f930 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -244,6 +244,40 @@ end end + describe '.semester_year_options' do + it 'spans 2012 through next year relative to the given date' do + options = described_class.semester_year_options(Date.new(2026, 6, 23)) + expect(options.first).to eq(2012) + expect(options.last).to eq(2027) + end + end + + describe '.parse_semester' do + around do |example| + travel_to(Date.new(2026, 6, 23)) { example.run } + end + + it 'splits a well-formed semester into [season, year]' do + expect(described_class.parse_semester('Spring 2026')).to eq([ 'Spring', 2026 ]) + expect(described_class.parse_semester('Fall 2025')).to eq([ 'Fall', 2025 ]) + end + + it 'returns [nil, nil] for an unrecognized season' do + expect(described_class.parse_semester('Autumn 2026')).to eq([ nil, nil ]) + end + + it 'returns [nil, nil] for an out-of-range year' do + expect(described_class.parse_semester('Spring 2011')).to eq([ nil, nil ]) + expect(described_class.parse_semester('Spring 2099')).to eq([ nil, nil ]) + end + + it 'returns [nil, nil] for blank or malformed input' do + expect(described_class.parse_semester(nil)).to eq([ nil, nil ]) + expect(described_class.parse_semester('')).to eq([ nil, nil ]) + expect(described_class.parse_semester('Spring2026')).to eq([ nil, nil ]) + end + end + describe '.semester_from_term' do it 'returns the term name when present' do expect(described_class.semester_from_term({ 'name' => 'Spring 2026' })).to eq('Spring 2026') diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 2749c91b..d76cbbfe 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -64,6 +64,9 @@ Rails.root.join('spec/fixtures') ] + # Enables travel_to/freeze_time for deterministic, time-dependent specs. + config.include ActiveSupport::Testing::TimeHelpers + config.before(:suite) do Rails.application.load_seed end From c57f039660105ebe4104d3dfe79477ca86e90133 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 23 Jun 2026 01:42:26 +0000 Subject: [PATCH 03/10] Make sidebar Settings a collapsible heading with links 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 --- app/views/layouts/_sidebar.html.erb | 50 +++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/app/views/layouts/_sidebar.html.erb b/app/views/layouts/_sidebar.html.erb index 187e6090..f591e65a 100644 --- a/app/views/layouts/_sidebar.html.erb +++ b/app/views/layouts/_sidebar.html.erb @@ -34,17 +34,47 @@ text: 'Enrollments', active: @side_nav == 'enrollments' %> - <%= render 'layouts/components/sidebar_menu_item', - path: approvals_course_settings_path(@course.id), - icon: 'fas fa-cog', - text: 'Settings', - active: %w[approvals emails].include?(@side_nav) %> + <% settings_pages = %w[course_details approvals emails form_settings] %> + <% settings_open = settings_pages.include?(@side_nav) %> +
  • + +
    + +
    + Settings + +     + +
    • <%= render 'layouts/components/sidebar_menu_item', path: new_course_request_path(@course.id), From de5ec22bf425988acd5934e67f2125367dcd14d0 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 23 Jun 2026 05:55:22 +0000 Subject: [PATCH 04/10] Derive sidebar active state from controller/action, not @side_nav 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 --- app/controllers/course_settings_controller.rb | 2 -- app/controllers/courses_controller.rb | 4 --- app/controllers/form_settings_controller.rb | 2 -- app/controllers/requests_controller.rb | 4 --- app/helpers/application_helper.rb | 23 +++++++++++++ app/views/layouts/_sidebar.html.erb | 21 ++++++------ spec/Helpers/application_helper_spec.rb | 32 +++++++++++++++++++ .../course_settings_controller_spec.rb | 2 -- spec/controllers/courses_controller_spec.rb | 1 - 9 files changed, 66 insertions(+), 25 deletions(-) create mode 100644 spec/Helpers/application_helper_spec.rb diff --git a/app/controllers/course_settings_controller.rb b/app/controllers/course_settings_controller.rb index 6ff25157..bb2a6467 100644 --- a/app/controllers/course_settings_controller.rb +++ b/app/controllers/course_settings_controller.rb @@ -19,13 +19,11 @@ class CourseSettingsController < ApplicationController # Approval and notification settings (formerly the "General Settings" tab). def approvals - @side_nav = 'approvals' @course_settings = @course.course_settings || @course.build_course_settings end # Email subject/body templates (formerly the "Email Settings" tab). def emails - @side_nav = 'emails' @course_settings = @course.course_settings || @course.build_course_settings end diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index c5470511..49593609 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -25,7 +25,6 @@ def show return redirect_to courses_path, alert: 'Course not found.' unless @course return redirect_to courses_path, alert: 'No Canvas LMS data found for this course.' unless @course.has_canvas_linked? - @side_nav = 'show' @course.regenerate_readonly_api_token_if_blank if @role == 'student' @@ -63,7 +62,6 @@ def new # Course Details: edit the course name, code and semester. def edit - @side_nav = 'course_details' redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' end @@ -76,7 +74,6 @@ def create end def update - @side_nav = 'course_details' return redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor' attrs = course_params.to_h @@ -109,7 +106,6 @@ def sync_enrollments end def enrollments - @side_nav = 'enrollments' return redirect_to courses_path, alert: 'You do not have access to this page.' unless @role == 'instructor' @enrollments = @course.user_to_courses.includes(:user) diff --git a/app/controllers/form_settings_controller.rb b/app/controllers/form_settings_controller.rb index 8b14e53f..ed78c4e2 100644 --- a/app/controllers/form_settings_controller.rb +++ b/app/controllers/form_settings_controller.rb @@ -6,12 +6,10 @@ class FormSettingsController < ApplicationController before_action :set_pending_request_count def edit - @side_nav = 'form_settings' @form_setting = @course.form_setting end def update - @side_nav = 'form_settings' @form_setting = @course.form_setting || @course.build_form_setting permitted = form_setting_params.to_h diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 463f809d..3fbf5955 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -14,7 +14,6 @@ class RequestsController < ApplicationController before_action :check_instructor_permission, only: %i[approve reject mass_approve mass_reject] def index - @side_nav = 'requests' if @role == 'student' @requests = @course.requests.for_user(@user) elsif params[:show_all] == 'true' @@ -36,7 +35,6 @@ def show end def new - @side_nav = 'form' # course_to_lms = @course.course_to_lms(1) course_to_lmss = @course.all_linked_lmss.pluck(:id) return redirect_to courses_path, alert: 'No Canvas LMS data found for this course.' unless course_to_lmss.any? @@ -55,7 +53,6 @@ def new end def new_for_student - @side_nav = 'form' return redirect_to course_requests_path(@course), alert: 'You do not have permission to access this page.' unless @role == 'instructor' course_to_lmss = @course.all_linked_lmss.pluck(:id) @@ -191,7 +188,6 @@ def export private def set_request - @side_nav = 'requests' @request = @course.requests.includes(:assignment).find_by(id: params[:id]) redirect_to course_path(@course), alert: 'Request not found.' unless @request end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b83449b1..7822d8bf 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,4 +1,27 @@ module ApplicationHelper + # Identifies which sidebar entry is active for the current request, derived + # from the controller and action rather than a per-action @side_nav ivar. + def sidebar_section + case "#{controller_name}##{action_name}" + when 'courses#show' + 'show' + when 'courses#edit', 'courses#update' + 'course_details' + when 'courses#enrollments' + 'enrollments' + when 'course_settings#approvals' + 'approvals' + when 'course_settings#emails' + 'emails' + when 'form_settings#edit', 'form_settings#update' + 'form_settings' + when 'requests#new', 'requests#new_for_student' + 'form' + when /\Arequests#/ + 'requests' + end + end + def assignment_link_for(assignment, course) case assignment.course_to_lms.lms_id when 1 diff --git a/app/views/layouts/_sidebar.html.erb b/app/views/layouts/_sidebar.html.erb index f591e65a..2a777863 100644 --- a/app/views/layouts/_sidebar.html.erb +++ b/app/views/layouts/_sidebar.html.erb @@ -5,6 +5,7 @@ + <% nav = sidebar_section %> @@ -80,7 +81,7 @@ path: new_course_request_path(@course.id), icon: 'fas fa-user-plus', text: 'Request for Student', - active: @side_nav == 'form' %> + active: nav == 'form' %> <% end %> <% if @role == 'student' %> @@ -88,7 +89,7 @@ path: new_course_request_path(@course.id), icon: 'fas fa-file-pen', text: 'Request Extension', - active: @side_nav == 'form' %> + active: nav == 'form' %> <% end %> \ No newline at end of file diff --git a/spec/Helpers/application_helper_spec.rb b/spec/Helpers/application_helper_spec.rb new file mode 100644 index 00000000..ce5d9fb7 --- /dev/null +++ b/spec/Helpers/application_helper_spec.rb @@ -0,0 +1,32 @@ +require 'rails_helper' + +RSpec.describe ApplicationHelper, type: :helper do + describe '#sidebar_section' do + def section_for(controller, action) + allow(helper).to receive_messages(controller_name: controller, action_name: action) + helper.sidebar_section + end + + it 'maps course and settings pages to their sidebar keys' do + expect(section_for('courses', 'show')).to eq('show') + expect(section_for('courses', 'edit')).to eq('course_details') + expect(section_for('courses', 'update')).to eq('course_details') + expect(section_for('courses', 'enrollments')).to eq('enrollments') + expect(section_for('course_settings', 'approvals')).to eq('approvals') + expect(section_for('course_settings', 'emails')).to eq('emails') + expect(section_for('form_settings', 'edit')).to eq('form_settings') + expect(section_for('form_settings', 'update')).to eq('form_settings') + end + + it 'distinguishes new-request pages from the requests list' do + expect(section_for('requests', 'new')).to eq('form') + expect(section_for('requests', 'new_for_student')).to eq('form') + expect(section_for('requests', 'index')).to eq('requests') + expect(section_for('requests', 'show')).to eq('requests') + end + + it 'returns nil for pages without a sidebar entry' do + expect(section_for('home', 'index')).to be_nil + end + end +end diff --git a/spec/controllers/course_settings_controller_spec.rb b/spec/controllers/course_settings_controller_spec.rb index 25dddf7b..02b96c47 100644 --- a/spec/controllers/course_settings_controller_spec.rb +++ b/spec/controllers/course_settings_controller_spec.rb @@ -123,7 +123,6 @@ expect(response).to have_http_status(:ok) expect(response).to render_template(:approvals) - expect(assigns(:side_nav)).to eq('approvals') end end @@ -133,7 +132,6 @@ expect(response).to have_http_status(:ok) expect(response).to render_template(:emails) - expect(assigns(:side_nav)).to eq('emails') end end end diff --git a/spec/controllers/courses_controller_spec.rb b/spec/controllers/courses_controller_spec.rb index 196b7542..93d9db2e 100644 --- a/spec/controllers/courses_controller_spec.rb +++ b/spec/controllers/courses_controller_spec.rb @@ -123,7 +123,6 @@ get :edit, params: { id: course.id } expect(response).to render_template(:edit) - expect(assigns(:side_nav)).to eq('course_details') end end end From 12f69136ec2381f5484305e530584685ec7ef0b2 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Wed, 24 Jun 2026 03:14:32 +0000 Subject: [PATCH 05/10] Rename sidebar group to "Course Settings"; don't highlight the header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- app/views/layouts/_sidebar.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/_sidebar.html.erb b/app/views/layouts/_sidebar.html.erb index 2a777863..0cae7b70 100644 --- a/app/views/layouts/_sidebar.html.erb +++ b/app/views/layouts/_sidebar.html.erb @@ -38,7 +38,7 @@ <% settings_pages = %w[course_details approvals emails form_settings] %> <% settings_open = settings_pages.include?(nav) %>
  • - - Settings + Course Settings