From eaf0c172422e7875fd9a8b762f8802c70460827b Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 17 Sep 2026 16:01:57 +0800 Subject: [PATCH 1/8] feat: coursecode section restyled + js to clear/hide the generate/coursecode if coursecode off --- CONTEXT.md | 2 +- app/controllers/courses_controller.rb | 5 +- .../coursecode_form_handler_controller.js | 50 ++++++++-- app/views/courses/_course_code_form.html.erb | 92 +++++++++++++------ app/views/courses/settings.html.erb | 9 +- docs/adr/0010-formless-coursecode-widget.md | 10 +- test/integration/update_coursecode_test.rb | 20 ++++ .../courses/settings_coursecode_test.rb | 66 ++++++++++--- test/system/courses/settings_save_test.rb | 8 +- 9 files changed, 201 insertions(+), 61 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 22704774..6a8e9015 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -22,7 +22,7 @@ definitions. - **course settings** — the coordinator's settings page for a course: a takeover-layout form (`no_sidebar` + sticky top nav, Save inside the form) whose section cards are Class Details · General · Permissions and Rules · Grouping. General holds the **coursecode widget** and the Supervisor Capacity subsections. - **settings sections** — the section cards on course settings: Class Details · General · Permissions and Rules · Grouping (headers styled `text-3xl font-normal text-gray-800 tracking-tight`, per the mockup). General's subsection cards are Coursecode and Supervisor Capacity. Grouping's two-mode card set is driven by `@course.grouping_enabled?` / `student_list_finalised?`. -- **coursecode widget** — the formless control inside General (ADR-0010): no `
` in the static DOM, so it lives inside `#course-settings-form` without nesting one. Generate/Re-Generate is a `data-turbo-method: :post` link to `update_coursecode_course_path(course, generate: true)`; the toggle fires its own `fetch` + `Turbo.renderStreamMessage`. Both keep `course_code_form`, `coursecode-form-handler`, and `Course#coursecode` semantics; `coursecode` and `coursecode_enabled` stay out of `handle_settings`' whitelist. Its partial's single top-level node must be the `course_code_form` turbo frame. +- **coursecode widget** — the formless control inside General (ADR-0010): no `` in the static DOM, so it lives inside `#course-settings-form` without nesting one. Both Generate/Re-Generate and the toggle fire their own `fetch` + `Turbo.renderStreamMessage` via `coursecode-form-handler` (Generate sends `generate=true`, the toggle sends `course[coursecode_enabled]=1|0`), both to `update_coursecode_course_path(course)`. The code input and Generate/Re-Generate button render only while joining is enabled; switching to No wipes the stored `coursecode` (and so the controls disappear). A copy button inside the code input writes the code to the clipboard (`content_copy`→`check` feedback); the toggle header carries a muted "(only solo courses)" hint beside the Beta badge. Both keep `course_code_form`, `coursecode-form-handler`, and `Course#coursecode` semantics; `coursecode` and `coursecode_enabled` stay out of `handle_settings`' whitelist. Its partial's single top-level node must be the `course_code_form` turbo frame. ## Template fields diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 3166482d..e831d08c 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -452,7 +452,10 @@ def update_coursecode end if params[:course]&.key?(:coursecode_enabled) - @course.update!(coursecode_enabled: params[:course][:coursecode_enabled]) + enabled = ActiveModel::Type::Boolean.new.cast(params[:course][:coursecode_enabled]) + attributes = { coursecode_enabled: enabled } + attributes[:coursecode] = nil unless enabled + @course.update!(attributes) flash.now[:notice] ||= 'Course join code settings updated' end rescue StandardError => e diff --git a/app/javascript/controllers/coursecode_form_handler_controller.js b/app/javascript/controllers/coursecode_form_handler_controller.js index fe66e8f3..ef11a430 100644 --- a/app/javascript/controllers/coursecode_form_handler_controller.js +++ b/app/javascript/controllers/coursecode_form_handler_controller.js @@ -1,17 +1,16 @@ import { Controller } from "@hotwired/stimulus"; // Connects to data-controller="coursecode-form-handler" -// Fires the coursecode_enabled toggle independently of any : fetch() the -// update_coursecode endpoint with a form-encoded body, then render its turbo -// stream (replaces the course_code_form frame and the flash). +// Formless coursecode widget (ADR-0010): no in the static DOM, so it can +// live inside #course-settings-form without nesting one. Generate/toggle fetch() +// the update_coursecode endpoint with a form-encoded body, then render its +// turbo stream (replaces the course_code_form frame and the flash). Copy writes +// the current join code to the clipboard. export default class extends Controller { + static targets = ["codeInput", "copyIcon"]; static values = { url: String }; - async toggle(event) { - const body = new URLSearchParams({ - "course[coursecode_enabled]": event.target.checked ? "1" : "0", - }); - + async post(body) { const response = await fetch(this.urlValue, { method: "POST", headers: { @@ -26,4 +25,39 @@ export default class extends Controller { window.Turbo.renderStreamMessage(await response.text()); } } + + generate() { + return this.post(new URLSearchParams({ generate: "true" })); + } + + toggle(event) { + return this.post( + new URLSearchParams({ + "course[coursecode_enabled]": event.target.value === "true" ? "1" : "0", + }), + ); + } + + async copy() { + const code = this.codeInputTarget.value; + if (!code) return; + + try { + await navigator.clipboard.writeText(code); + this.flashCopied(); + } catch { + // Clipboard unavailable (insecure context / permission) — no-op. + } + } + + flashCopied() { + if (!this.hasCopyIconTarget) return; + + const icon = this.copyIconTarget; + icon.textContent = "check"; + clearTimeout(this.copyResetTimer); + this.copyResetTimer = setTimeout(() => { + icon.textContent = "content_copy"; + }, 1500); + } } diff --git a/app/views/courses/_course_code_form.html.erb b/app/views/courses/_course_code_form.html.erb index fb1bf6f3..81e29522 100644 --- a/app/views/courses/_course_code_form.html.erb +++ b/app/views/courses/_course_code_form.html.erb @@ -1,35 +1,69 @@ <%# locals: (course:) %> -<%# Formless: Generate/Re-Generate is a data-turbo-method link (Turbo synthesizes a throwaway - form at click time) and the toggle fires its own fetch. There is no in the static - DOM, so this can live anywhere inside #course-settings-form. The top-level node of this - partial must stay exactly — Turbo Streams replace() - swaps in the WHOLE template content, so any wrapper would get re-nested on every replace - (see ADR-0010). %> +<%# Formless: both Generate/Re-Generate and the toggle fire their own fetch via + coursecode-form-handler (the same pattern as the sibling email-domain widget). There is no + in the static DOM, so this can live anywhere inside #course-settings-form. The + top-level node of this partial must stay exactly — + Turbo Streams replace() swaps in the WHOLE template content, so any wrapper would get + re-nested on every replace (see ADR-0010). %> <%= turbo_frame_tag "course_code_form" do %> -
- -

- Use this only for individual projects. For groups, use CSV imports. Group formation in ProPro coming in a future update. -

-
- - <%= link_to (course.coursecode.present? ? 'Re-Generate Join Code' : 'Generate Join Code'), - update_coursecode_course_path(course, generate: true), - id: "regenerate-code-btn", - data: { turbo_method: :post }, - class: "px-4 py-2 bg-blue-200 hover:bg-blue-300 text-gray-800 text-sm font-medium rounded shadow-sm cursor-pointer" %> + + +
+
+ + Allow joining via course code + Beta + (only solo courses) + + + Use this only for individual projects. Group formation via code is coming in a future update. + +
+
+ + +
- + + + <% if course.coursecode_enabled %> +
+
+ + + +
+ +
+ <% end %>
-<% end %> +<% end %> \ No newline at end of file diff --git a/app/views/courses/settings.html.erb b/app/views/courses/settings.html.erb index d6270f71..e68e4146 100644 --- a/app/views/courses/settings.html.erb +++ b/app/views/courses/settings.html.erb @@ -51,7 +51,7 @@ <%= render "copy_course_overlay", course: @course, mode: "settings", courses: @courses %> -
+

Class Details

@@ -78,7 +78,7 @@
-
+

General

@@ -112,7 +112,7 @@
-
+

Permissions and Rules

@@ -202,7 +202,7 @@
-
+

Student Self-Grouping Beta

-

Group formation in ProPro is still in progress!

diff --git a/docs/adr/0010-formless-coursecode-widget.md b/docs/adr/0010-formless-coursecode-widget.md index 4c22dc1d..5981a86d 100644 --- a/docs/adr/0010-formless-coursecode-widget.md +++ b/docs/adr/0010-formless-coursecode-widget.md @@ -66,4 +66,12 @@ mockup and like every well-known regenerate/invite-code control: code, toggle persists independently). - **Invariant:** a partial served by a Turbo Stream `replace` must have exactly one top-level node, and it must *be* the replacement target frame — - any wrapper gets cloned and re-nested with every stream. \ No newline at end of file + any wrapper gets cloned and re-nested with every stream. +- **Amendment 2026-09-17:** Generate / Re-Generate no longer rides a + `data-turbo-method` link. It calls the same `coursecode-form-handler` + `fetch()` path as the toggle, posting `generate=true`, so a real click can + never race Turbo's synthesized throwaway form. The code input and + Generate/Re-Generate controls render only while `coursecode_enabled` is true, + and switching joining to No wipes the stored `coursecode` in + `update_coursecode`. Endpoint, params, the `course_code_form` frame contract, + and `handle_settings`' whitelist are unchanged. \ No newline at end of file diff --git a/test/integration/update_coursecode_test.rb b/test/integration/update_coursecode_test.rb index 5fc52462..4277a4e5 100644 --- a/test/integration/update_coursecode_test.rb +++ b/test/integration/update_coursecode_test.rb @@ -14,6 +14,10 @@ class UpdateCoursecodeTest < ActionDispatch::IntegrationTest assert_nil @course.coursecode + # The UI only renders the code/regenerate controls while joining via code + # is enabled, so enable before generating. + post update_coursecode_course_path(@course), params: { course: { coursecode_enabled: true } }, headers: { 'Accept' => 'text/vnd.turbo-stream.html' } + post update_coursecode_course_path(@course), params: { generate: true }, headers: { 'Accept' => 'text/vnd.turbo-stream.html' } # The request should succeed and return turbo stream content @@ -47,4 +51,20 @@ class UpdateCoursecodeTest < ActionDispatch::IntegrationTest @course.reload assert_equal @course.coursecode_enabled, true end + + test 'turning joining off wipes the stored coursecode' do + post session_path, params: { email_address: @lecturer.email_address, password: 'password' } + assert_redirected_to root_path + + post update_coursecode_course_path(@course), params: { course: { coursecode_enabled: true } }, headers: { 'Accept' => 'text/vnd.turbo-stream.html' } + post update_coursecode_course_path(@course), params: { generate: true }, headers: { 'Accept' => 'text/vnd.turbo-stream.html' } + assert_not_nil @course.reload.coursecode + + post update_coursecode_course_path(@course), params: { course: { coursecode_enabled: false } }, headers: { 'Accept' => 'text/vnd.turbo-stream.html' } + + assert_response :success + @course.reload + assert_not @course.coursecode_enabled + assert_nil @course.coursecode + end end diff --git a/test/system/courses/settings_coursecode_test.rb b/test/system/courses/settings_coursecode_test.rb index 188ad55e..91eb6373 100644 --- a/test/system/courses/settings_coursecode_test.rb +++ b/test/system/courses/settings_coursecode_test.rb @@ -1,8 +1,7 @@ require 'application_system_test_case' # Real-browser guard for the generate/toggle coursecode path. The widget is -# formless (ADR-0010): Generate/Re-Generate is a data-turbo-method link (Turbo -# synthesizes a throwaway form at click time) and the toggle fires its own +# formless (ADR-0010): both Generate/Re-Generate and the toggle fire their own # fetch() + Turbo.renderStreamMessage. Both land on the unchanged # update_coursecode action, whose turbo streams re-render the course_code_form # frame and the flash. rack_test cannot run the JS, so this exercises the same @@ -18,10 +17,10 @@ class SettingsCoursecodeTest < BrowserSystemTestCase @coordinator = create(:enrolment, :coordinator, course: @course).user end - # The Generate link must be clicked scripted, not natively: this page's + # The Generate button must be clicked scripted, not natively: this page's # headless Chrome intermittently swallows the first trusted (Selenium pointer) # click with no event, no request, and no navigation — reproduced both -# sequentially and under parallel load, with native `click_link`, scripted +# sequentially and under parallel load, with native `click_button`, scripted # `element.click()`, and `page.execute_script`. The only reliable path is # `page.execute_script` *without* `wait_for_turbo` guards: Turbo already # marked `aria-busy` by the time the script runs, and post-click @@ -32,11 +31,16 @@ def click_generate page.execute_script("document.getElementById('regenerate-code-btn').click()") end - # The toggle checkbox is visually hidden (sr-only peer), so a native click - # would fail intermittently on this page — dispatch the click via script, - # which is the same change event a real click fires. - def toggle_enabled - page.execute_script("document.getElementById('course_coursecode_enabled').click()") + # The toggle is a Yes/No radio pair, and a native click (Selenium pointer) on + # a radio inside this page's form can be swallowed intermittently — dispatch + # the click via script, which fires the same change event a real click does. + def select_coursecode_enabled(value) + page.execute_script(%(document.querySelector('input[name="coursecode_enabled"][value="#{value}"]').click())) + end + + def enable_coursecode + select_coursecode_enabled('true') + assert_text 'Course join code settings updated' end test 'generating a coursecode persists it and reflects it back in the frame' do @@ -45,6 +49,11 @@ def toggle_enabled assert_nil @course.reload.coursecode + # The code controls render only while joining via code is on. + assert_no_selector '#regenerate-code-btn' + enable_coursecode + assert_selector '#regenerate-code-btn' + click_generate assert_text 'Course join code successfully generated' @@ -58,6 +67,8 @@ def toggle_enabled login_as @coordinator visit settings_course_path(@course) + enable_coursecode + click_generate assert_text 'Course join code successfully generated' first_code = @course.reload.coursecode @@ -68,20 +79,47 @@ def toggle_enabled assert_not_equal first_code, @course.reload.coursecode end - test 'toggling join code access persists independently of the settings form' do + test 'toggling join code access persists and wipes the code when turned off' do login_as @coordinator visit settings_course_path(@course) + enable_coursecode click_generate assert_text 'Course join code successfully generated' assert_not_nil @course.reload.coursecode - toggle_enabled + select_coursecode_enabled('false') assert_text 'Course join code settings updated' - assert @course.reload.coursecode_enabled + @course.reload + assert_not @course.coursecode_enabled + assert_nil @course.coursecode, 'turning joining off must wipe the stored code' + assert_no_selector '#regenerate-code-btn' - toggle_enabled + select_coursecode_enabled('true') assert_text 'Course join code settings updated' - assert_not @course.reload.coursecode_enabled + assert @course.reload.coursecode_enabled + assert_selector '#regenerate-code-btn' + end + + test 'copy button copies the current join code to the clipboard' do + login_as @coordinator + visit settings_course_path(@course) + + enable_coursecode + click_generate + assert_text 'Course join code successfully generated' + code = @course.reload.coursecode + + # Stub the clipboard so the test proves our controller wiring rather than + # depending on headless Chrome's clipboard permission. + page.execute_script(<<~JS) + Object.defineProperty(navigator, 'clipboard', { + configurable: true, + value: { writeText: (text) => { window.__copied = text; return Promise.resolve(); } }, + }); + JS + page.execute_script("document.getElementById('copy-code-btn').click()") + + assert_equal code, page.evaluate_script('window.__copied') end end diff --git a/test/system/courses/settings_save_test.rb b/test/system/courses/settings_save_test.rb index 3a571bc4..9df3be9a 100644 --- a/test/system/courses/settings_save_test.rb +++ b/test/system/courses/settings_save_test.rb @@ -5,7 +5,8 @@ class SettingsSaveTest < ApplicationSystemTestCase @course = create(:course, course_name: 'Original Course Name', starting_week: 1, - course_description: 'Old description') + course_description: 'Old description', + coursecode_enabled: true) coordinator = create(:user) create(:enrolment, :coordinator, user: coordinator, course: @course) @coordinator = coordinator @@ -29,7 +30,10 @@ class SettingsSaveTest < ApplicationSystemTestCase # nest a form or roll a settings save back on generate. assert_selector '#course-settings-form' assert_selector '#course_code_form' - assert_selector "#course_code_form a[href*='update_coursecode']" + assert_selector '#course_code_form #regenerate-code-btn' + assert_selector '#course_code_form #copy-code-btn' + assert_selector '#course_code_form [data-coursecode-form-handler-url-value*="update_coursecode"]' + assert_text 'only solo courses' assert_equal 0, page.all('#course_code_form form').count, 'the coursecode widget must never render a ' assert_equal 0, page.all('#course-settings-form form').count, From da880b1934692f21c58af3f95a9abd9237100d29 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 20 Sep 2026 15:25:11 +0800 Subject: [PATCH 2/8] feat: redesigned edit/profile courses/profiles and minor tweaks to group-tab and minor fixes --- CONTEXT.md | 24 + app/assets/tailwind/application.css | 4 + app/controllers/lecturers_controller.rb | 3 + app/controllers/user_controller.rb | 2 + .../controllers/sortable_controller.js | 5 + .../controllers/textarea_resize_controller.js | 31 +- app/javascript/project_template_fields.js | 129 +- app/models/project_template_field.rb | 4 +- .../courses/_copy_course_overlay.html.erb | 2 +- app/views/courses/_course_code_form.html.erb | 2 +- app/views/courses/_groups_tab.html.erb | 47 +- app/views/courses/_lecturers_section.html.erb | 2 +- app/views/courses/_project_card.html.erb | 59 - .../courses/_project_card_contents.html.erb | 95 -- app/views/courses/_student_row.html.erb | 6 +- app/views/courses/profile.html.erb | 446 +++--- app/views/courses/show.html.erb | 2 +- app/views/lecturers/show.html.erb | 600 ++++---- .../project_templates/_field_row.html.erb | 189 +-- .../_option_row_dropdown.html.erb | 19 +- .../_option_row_radio.html.erb | 28 +- .../_option_section.html.erb | 9 +- app/views/project_templates/edit.html.erb | 246 +-- .../projects/_proposal_list_item.html.erb | 30 +- app/views/user/profile.html.erb | 242 +-- ...profile-participants-courses-show-shell.md | 69 + docs/checklist.md | 148 -- docs/correctness_plan.md | 130 -- docs/course_tab_plan.md | 493 ------ docs/courses_overview_refactor_plan.md | 1013 ------------- docs/courses_show_refactor_plan.md | 1338 ----------------- docs/design_refactor_plan.md | 336 ----- docs/merge_conflicts_refactor_design.md | 93 -- docs/people_tab_refactor_plan.md | 497 ------ docs/projects_forms_refactor_plan.md | 153 -- docs/projects_show_refactor_plan.md | 474 ------ docs/settings_refactor_plan.md | 209 --- docs/shortcut_group_search_plan.md | 335 ----- docs/sidebar_collapsable_plan.md | 583 ------- docs/tabs_to_cookies_refactor_plan.md | 705 --------- docs/tailwindcss_audit_plan.md | 460 ------ docs/topics_directory_refactor_plan.md | 668 -------- docs/topics_forms_refactor_plan.md | 287 ---- docs/topics_show_refactor_plan.md | 249 --- test/controllers/courses_controller_test.rb | 87 +- test/controllers/lecturers_controller_test.rb | 83 + .../project_templates_controller_test.rb | 52 + test/controllers/user_controller_test.rb | 37 + .../courses/settings_coursecode_test.rb | 18 +- 49 files changed, 1250 insertions(+), 9493 deletions(-) delete mode 100644 app/views/courses/_project_card.html.erb delete mode 100644 app/views/courses/_project_card_contents.html.erb create mode 100644 docs/adr/0016-profile-participants-courses-show-shell.md delete mode 100644 docs/checklist.md delete mode 100644 docs/correctness_plan.md delete mode 100644 docs/course_tab_plan.md delete mode 100644 docs/courses_overview_refactor_plan.md delete mode 100644 docs/courses_show_refactor_plan.md delete mode 100644 docs/design_refactor_plan.md delete mode 100644 docs/merge_conflicts_refactor_design.md delete mode 100644 docs/people_tab_refactor_plan.md delete mode 100644 docs/projects_forms_refactor_plan.md delete mode 100644 docs/projects_show_refactor_plan.md delete mode 100644 docs/settings_refactor_plan.md delete mode 100644 docs/shortcut_group_search_plan.md delete mode 100644 docs/sidebar_collapsable_plan.md delete mode 100644 docs/tabs_to_cookies_refactor_plan.md delete mode 100644 docs/tailwindcss_audit_plan.md delete mode 100644 docs/topics_directory_refactor_plan.md delete mode 100644 docs/topics_forms_refactor_plan.md delete mode 100644 docs/topics_show_refactor_plan.md create mode 100644 test/controllers/lecturers_controller_test.rb create mode 100644 test/controllers/project_templates_controller_test.rb create mode 100644 test/controllers/user_controller_test.rb diff --git a/CONTEXT.md b/CONTEXT.md index 6a8e9015..5996617f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -60,6 +60,30 @@ definitions. - **pinned (You) group** — the current viewer's own supervisor group in the topics directory, always first with a gray "(You)" suffix. Staff (lecturer/coordinator) only, pinned even at 0 topics; replaces the old "My Topics" section. Students browsing see plain A→Z with no pinned group. - **available badge** — on topics-directory rows only, an approved-and-unclaimed topic (`Topic#available?` = `approved? && proposed_project_instances.none?`) renders its pill as green "Available" *in place of* the "Approved" status pill (same approved-green palette — the swap is just the shared `pill_label` local, driven by `_topic_card`'s optional `available_pill:` local, default `false`). Non-available rows keep their real status pill. Always green — the mockup's blue "(You)" rows are an artifact. Display-only; never a substitute authorization gate (ADR 014). +## Participant profile + +- **participant profile** — the `courses#profile` page for one student or group, reached as `courses/profile/:participant_id/:participant_type` from the People/Groups rows (ADR 0016). Rendered in the **courses/show shell** (`bg-surface-tint`, inline `shared/sidebar` in the page's own `.flex`, `rounded-tl-panel` main — no `border-t border-l`) with content in a centered `max-w-5xl` reading column. +- **solo individual profile** — the participant profile for one person when the target is an individual (not a group): the student branch of `courses/profile`, i.e. what opens when you link a student name from the Students table or a Group Members name. "Solo" here is the solo-format course sense (a non-grouped course where each student works individually), not the solo-supervisor sense — but the profile itself is just the student branch; in solo/individual courses every student reaches it. +- **profile entry points** — every student/group name that opens a participant profile: the Students table name cell, the People tab rows, and the Group Members name inside a group profile. A name that does not yet link is a regression of the browse path. Each link is a plain `participant_profile_course_path(course, id, 'student'|'group')` visit — no `from_participant` breadcrumb context — and unregistered (invited) students' names link exactly like registered ones. Only the name is clickable; avatar, email, and STUDENT ID stay static. Chrome-free main-column chrome: the old `bg-gray-50`/`w-[95%] mx-auto` layout is gone. +- **profile hero** — the banner band (`bg-primary-container-alt`) + identity row section per participant type. Student: overlapping avatar (`-mt-12`, `sm:pt-14` in the action column), name, email, coordinator-only **Remove From Course** pill top-right. Group: banner band + group name only. The hero carries the **facts strip**. +- **facts strip** — the Project Status pill + Supervisor row under the profile hero, split by a hairline (`border-t`, a pseudo-element `w-px` divider between the two `sm:w-1/2` cells). Status pill uses the shared palette (approved-green/pending-blue/redo-amber/rejected-red, `not_submitted` gray). Supervisor shows a muted "—" when absent. +- **participant-scoped project link** — the Current Project row's path, `course_project_path(course, project, from_participant:, participant_type:)`, preserving the profile as the project show's breadcrumb parent. Passed into `_proposal_list_item` as its `path:` local alongside `linkable: policy(project).show?`. +- **profile empty state** — the `shared/_empty_state` ("No project has been submitted yet.") shown when a participant has no project, or a project without any `project_instance` (the old card silently rendered nothing in that case; the flat row requires a title from the instance, so the guard is `@project.present? && @current_instance.present?`). + +## Lecturer profile + +- **lecturer profile** — the `lecturers#show` page for one lecturer within a course, reached from the People tab rows (`course_lecturer_path`). Same chrome as the **participant profile**: `bg-surface-tint`, inline sidebar, `rounded-tl-panel` main, centered max-w-5xl reading column. "View Supervisor" eyebrow, banner band + overlapping avatar hero, then the **capacity strip**, the **Profile Information** card, four collapsible **lecturer profile sections**, and a Back to Course footer. The old "Propose To Lecturer" profile button is gone (the proposal method itself is untouched). +- **lecturer profile sections** — the four collapsible blocks under the lecturer hero, in round-1 order: Supervised Projects → Topics → Pending Proposals → Reviewed Proposals. Wired exactly like Overview (section header `section:` mode + `collapsible-sections`, load expanded, one Collapse all/Expand all switch, no persistence, sentence empty states). Pending/Reviewed proposals split by `current_status == 'pending'` vs `%w[redo rejected]`. +- **Topics (lecturer profile)** — the single flat Topics section on the lecturer profile. `unrestricted_lecturer_access?` viewers see all of the lecturer's topics (approved + draft) interleaved newest-first with real status pills; a student who only passes `student_can_view_lecturer?` sees approved topics only. Not the grouped topics-directory view. +- **unrestricted lecturer view** — a viewer who may see the full lecturer profile: the coordinator, the lecturer themselves, or anyone while the course's `lecturer_access` is on (then *every* enrolled user, students included). Students see the full sections only when the old gates change; when `lecturer_access` is off, a student branch shows approved Topics only. +- **capacity strip (lecturer)** — the Supervisor Capacity facts cell under the lecturer hero. Bar-first, unlike the People tab: the `w-24 h-1.5` bar starts flush under the "Supervisor Capacity" label (colored `--color-error/warning/success`), then approved/cap, then `(n pending)`, and the exceeding `error` icon sits at the end so it never shifts the bar off the label. Hidden for **solo-supervisor courses**, where only the muted "—" shows. +- **Profile Information (lecturer)** — the card below the hero: Role pill (from the enrolment), Email (`mailto:`), Description (`whitespace-pre-line`, "No description provided." fallback), Portfolio/MMU Directory (`web_link`, "No web link provided." fallback). Coordinator-only Promote To Coordinator / Demote to Lecturer links live in the card's heading row. + +## Edit profile + +- **edit profile** — the personal account form, `user#profile` / `user_profile_path`, "Edit Profile". Rendered in the **takeover layout** chrome copied verbatim from **course settings**: `no_sidebar`/`no_header`/`no_footer`, `bg-surface-tint`, a sticky white header (close X + "Edit Profile" DM Sans title + Discard Changes/Save that stay visible at every width — no mobile bottom bar), a `max-w-4xl` centered column, and two **settings sections** cards (Account Details, Change Password). Both close X and Discard Changes go to `/` (home). Underline fields throughout; Email/instid stay read-only and unrendered. +- **underline field** — the shared settings input treatment (first used by **course settings**, reused by **edit profile**): `bg-surface-variant rounded-t-[4px] border-b-2 border-on-surface-variant focus-within:border-primary`, a floating `absolute top-2 left-4 text-xs` label, and `text-[14px] mt-1` value text. Full-width by default, restricted to `sm:w-1/2` (or a `grid sm:grid-cols-2`) where the mockup wants a half field. The helper paragraph under Description stays as muted `text-[13px] text-on-surface-variant` text beneath the field. + ## App chrome - **app sidebar** — `shared/_sidebar`: app-wide nav (Home / courses / Edit profile), plus the `render_sidebar` helper widget for legacy pages. One element, two presentations: a static left column at ≥`lg` (1024px), an off-canvas drawer below it opened by the header menu icon. Controlled by `sidebar`. Its breakpoint is chrome-level and deliberately independent of content breakpoints (e.g., the `min-[1245px]` comments drawer). diff --git a/app/assets/tailwind/application.css b/app/assets/tailwind/application.css index ef0a3a89..696c8991 100644 --- a/app/assets/tailwind/application.css +++ b/app/assets/tailwind/application.css @@ -98,6 +98,10 @@ overflow: visible; } +/* Checkbox accents without extra markup */ +input[type="checkbox"].accent-primary { accent-color: var(--color-primary); } +input[type="checkbox"].accent-success { accent-color: var(--color-success); } + /* Force EasyMDE Preview Headings to restore their size */ .editor-preview h1 { font-size: 2em !important; diff --git a/app/controllers/lecturers_controller.rb b/app/controllers/lecturers_controller.rb index 7d794808..7f03d0ad 100644 --- a/app/controllers/lecturers_controller.rb +++ b/app/controllers/lecturers_controller.rb @@ -22,6 +22,9 @@ def show @is_student = @current_user_enrolment&.student? @is_lecturer = @current_user_enrolment&.lecturer? + @capacity_result = SupervisorCapacityCalculator.new(@course).calculate + @lecturer_capacity_info = @capacity_result.lecturer_capacities.index_by { |lc| lc.enrolment.user_id } + unless @enrolment&.role.in?(%w[lecturer coordinator]) redirect_to course_lecturers_path(@course), alert: 'Not a lecturer.' return diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 81db949b..1051c105 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -24,6 +24,8 @@ def new end def edit + @user = Current.user + if params[:user][:name].blank? redirect_back_or_to '/', alert: 'Name cannot be empty' return diff --git a/app/javascript/controllers/sortable_controller.js b/app/javascript/controllers/sortable_controller.js index 06f423fa..3f9683a2 100644 --- a/app/javascript/controllers/sortable_controller.js +++ b/app/javascript/controllers/sortable_controller.js @@ -8,6 +8,8 @@ export default class extends Controller { this.sortable = Sortable.create(this.element, { draggable: ".field-row", handle: ".drag-handle", + filter: ".sortable-disabled", + preventOnFilter: true, animation: 150, ghostClass: "opacity-25", chosenClass: "sortable-chosen", @@ -20,6 +22,9 @@ export default class extends Controller { scrollSensitivity: 100, scrollSpeed: 20, forceFallback: true, + onMove: (evt) => { + if (evt.related && evt.related.closest(".sortable-disabled")) return false; + }, onEnd: this.updatePositions.bind(this) }) } diff --git a/app/javascript/controllers/textarea_resize_controller.js b/app/javascript/controllers/textarea_resize_controller.js index 775bbdf0..56642bae 100644 --- a/app/javascript/controllers/textarea_resize_controller.js +++ b/app/javascript/controllers/textarea_resize_controller.js @@ -2,16 +2,39 @@ import { Controller } from "@hotwired/stimulus"; export default class extends Controller { connect() { - requestAnimationFrame(() => { + this.lastWidth = null; + this.resize(); + + // Webfonts swap in after connect and change text metrics. + document.fonts?.ready.then(() => { + this.resize(); + }); + + // Re-measure when the column width changes (rotation, zoom, becoming + // visible). Guard on width: our own height change also fires the observer. + this.observer = new ResizeObserver(([entry]) => { + const width = entry.contentRect.width; + if (width === this.lastWidth) return; + this.lastWidth = width; this.resize(); }); + this.observer.observe(this.element); + } + + disconnect() { + this.observer?.disconnect(); } resize() { - if (this.element.offsetParent === null) return; + const el = this.element; + if (el.offsetParent === null) return; + + const cs = getComputedStyle(el); + const borders = + parseFloat(cs.borderTopWidth) + parseFloat(cs.borderBottomWidth); - this.element.style.height = "auto"; - this.element.style.height = `${this.element.scrollHeight}px`; + el.style.height = "auto"; + el.style.height = `${el.scrollHeight + borders}px`; } commentResize() { diff --git a/app/javascript/project_template_fields.js b/app/javascript/project_template_fields.js index dedf0262..0a37d964 100644 --- a/app/javascript/project_template_fields.js +++ b/app/javascript/project_template_fields.js @@ -12,8 +12,8 @@ document.addEventListener("turbo:load", function () { const fieldTypeOptions = [ { value: "shorttext", label: "Short Text" }, { value: "textarea", label: "Paragraph" }, - { value: "dropdown", label: "Dropdown Selection" }, - { value: "radio", label: "Selection" }, + { value: "dropdown", label: "Dropdown" }, + { value: "radio", label: "Radio" }, ]; const applicableToOptions = [ @@ -44,29 +44,26 @@ document.addEventListener("turbo:load", function () { function createNewFieldHTML(index) { return ` - - - Field Label +
- +
+
+ + \ No newline at end of file diff --git a/app/views/project_templates/_field_row.html.erb b/app/views/project_templates/_field_row.html.erb index cb20fa3c..a3242ad8 100644 --- a/app/views/project_templates/_field_row.html.erb +++ b/app/views/project_templates/_field_row.html.erb @@ -1,42 +1,38 @@ +<% is_title = field_form.object.is_project_title? %> " data-controller="project-template-fields" - data-is-project-title="<%= field_form.object.is_project_title? %>" + data-is-project-title="<%= is_title %>" > <%= field_form.hidden_field :position, class: "position-input" %> <%# --- Label --- %> - - Field Label - +
- \ No newline at end of file diff --git a/app/views/project_templates/_option_row_radio.html.erb b/app/views/project_templates/_option_row_radio.html.erb index 786ea2bc..86425a91 100644 --- a/app/views/project_templates/_option_row_radio.html.erb +++ b/app/views/project_templates/_option_row_radio.html.erb @@ -1,18 +1,36 @@
- + + + - -
+ + + +
\ No newline at end of file diff --git a/app/views/project_templates/_option_section.html.erb b/app/views/project_templates/_option_section.html.erb index 16e27f1b..f4944878 100644 --- a/app/views/project_templates/_option_section.html.erb +++ b/app/views/project_templates/_option_section.html.erb @@ -30,10 +30,9 @@ - + \ No newline at end of file diff --git a/app/views/project_templates/edit.html.erb b/app/views/project_templates/edit.html.erb index 8680b4b4..74a082f2 100644 --- a/app/views/project_templates/edit.html.erb +++ b/app/views/project_templates/edit.html.erb @@ -1,133 +1,135 @@ -<% provide :no_sidebar, true %> +<% content_for :no_sidebar, true %> +<% content_for :no_header, true %> +<% content_for :no_footer, true %> +<% content_for :hide_toggler, true %> +<% content_for :hide_breadcrumbs, true %> +<% content_for :body_class, "bg-surface-tint" %> <% breadcrumb :edit_project_template, @course %> <% content_for :title, "Edit Project Template - #{@course.course_name.truncate(TITLE_NAME_LIMIT)} | ProPro" %> -
-
- - <%= render "courses/flash" %> - <%# --- Page Header --- %> -
-

Edit Project Template

-

Define the fields students must fill out for their project proposals.

-
- - <%= form_with model: [@course, @project_template], url: course_project_template_path(@course), local: true, class: "flex flex-col bg-white shadow-sm rounded-xl border border-gray-200 overflow-hidden" do |form| %> - - <%# --- Errors Section --- %> - <% if @project_template.errors.any? %> -
-
-
- - - -
-
-

- <%= pluralize(@project_template.errors.count, "error") %> - prevented saving: -

-
    - <% @project_template.errors.full_messages.each do |msg| %> -
  • <%= msg %>
    • - <% end %> -
-
-
-
+
+ +
+
+ <%= link_to course_path(@course), + title: "Close and return to course settings", + class: "p-2 -ml-2 rounded-full hover:bg-surface-variant text-on-surface-variant hover:text-on-surface transition-colors flex items-center justify-center shrink-0" do %> + close <% end %> - <%= render "courses/copy_course_overlay", course: @course, mode: "template", courses: @courses %> - -
- - - <%# --- Table Header --- %> - - - - - - - - - - - - - <%# --- Table Body --- %> - - <%= form.fields_for :project_template_fields do |field_form| %> - <%= render "field_row", field_form: field_form %> - <% end %> - -
-
+
- <%# --- Add Field Button --- %> -
- +
+

+ Edit Project Template +

+
+
- <%# --- Footer Actions --- %> -
+ <%= link_to "Discard Changes", + course_path(@course), + class: "px-5 py-2 rounded-full text-on-surface-variant hover:bg-surface-variant hover:text-on-surface font-medium text-[14px] transition-colors whitespace-nowrap" %> +
+ check + Save Template + +
+
+ +
+
+ + <%= render "courses/flash" %> + + <%= form_with model: [@course, @project_template], + url: course_project_template_path(@course), + local: true, + html: { id: "template-form" }, + class: "space-y-6" do |form| %> + + <%# --- Errors Section --- %> + <% if @project_template.errors.any? %> +
+

+ <%= pluralize(@project_template.errors.count, "error") %> + prevented saving: +

+
    + <% @project_template.errors.full_messages.each do |msg| %> +
  • <%= msg %>
    • + <% end %> +
+
+ <% end %> + +
+ +
+ + + <%# --- Table Header --- %> + <%# TODO: refactor to pinned columns (17/20/13/15/20/7/8) %> + + + + + + + + + + + + + <%# --- Table Body --- %> + + <%= form.fields_for :project_template_fields do |field_form| %> + <%= render "field_row", field_form: field_form %> + <% end %> + +
Field LabelDescriptionTypeApplicable ToOptionsRequiredFree Edit
+
+ + <%# --- Bottom Action Row --- %> +
+ + + <%= render "courses/copy_course_overlay", course: @course, mode: "template", courses: @courses %> +
+
+ + <%# --- Desktop-only Footer Actions --- %> + + + <% end %> - <% end %> -
-
+ <%= render "shared/form_action_bar", + discard_path: course_path(@course), + form_id: "template-form", + discard_label: "Discard", + submit_label: "Save Template" %> + + + \ No newline at end of file diff --git a/app/views/projects/_proposal_list_item.html.erb b/app/views/projects/_proposal_list_item.html.erb index ee383d12..a13a9bb7 100644 --- a/app/views/projects/_proposal_list_item.html.erb +++ b/app/views/projects/_proposal_list_item.html.erb @@ -1,16 +1,26 @@ -<% icon, verb = - case proposal.current_status - when "approved" then ["assignment_turned_in", "Updated"] - when "pending" then ["assignment", "Submitted"] - when "redo" then ["history", "Returned"] - when "rejected" then ["block", "Rejected"] - else ["assignment", "Updated"] - end %> +<%# Locals: proposal (required), path (optional, defaults to the plain + # course→project link), linkable (optional, default true). The caller may + # override path/linkable to keep participant-scoped params (from_participant, + # participant_type) or gate the row behind a policy. %> + +<% path = local_assigns[:path] || course_project_path(@course, proposal) %> +<% linkable = local_assigns.fetch(:linkable, true) %> + +<% + icon, verb = + case proposal.current_status + when "approved" then ["assignment_turned_in", "Updated"] + when "pending" then ["assignment", "Submitted"] + when "redo" then ["history", "Returned"] + when "rejected" then ["block", "Rejected"] + else ["assignment", "Updated"] + end %> <%= render "shared/row_item", - path: course_project_path(@course, proposal), + path: path, icon: icon, title: proposal.current_title, meta_parts: [proposal.owner_name], time_meta: "#{verb} #{time_ago_in_words(proposal.current_instance&.updated_at || proposal.updated_at)} ago", - status: proposal.current_status %> + status: proposal.current_status, + linkable: linkable %> diff --git a/app/views/user/profile.html.erb b/app/views/user/profile.html.erb index 085b7eb3..861c495a 100644 --- a/app/views/user/profile.html.erb +++ b/app/views/user/profile.html.erb @@ -1,193 +1,97 @@ +<% content_for :no_sidebar, true %> +<% content_for :no_header, true %> +<% content_for :no_footer, true %> +<% content_for :hide_toggler, true %> +<% content_for :hide_breadcrumbs, true %> +<% content_for :body_class, "bg-surface-tint" %> <% breadcrumb :user_profile %> + <% content_for :title, "Edit Profile | ProPro" %> -
-
+
+ + <%= form_with url: user_edit_path, model: @user, method: :post, id: "edit-profile-form", class: "flex flex-col min-h-screen" do |form| %> - <%# --- Flash Messages --- %> - <% if flash[:alert] %> -
- - <%= flash[:alert] %> + +
+
+ <%= link_to "/", title: "Close and return home", class: "p-2 -ml-2 rounded-full hover:bg-surface-variant text-on-surface-variant hover:text-on-surface transition-colors flex items-center justify-center" do %> + close + <% end %> +

Edit Profile

- <% end %> - <% if flash[:notice] %> -
- - <%= flash[:notice] %> +
+ <%= link_to "Discard Changes", "/", class: "px-5 py-2 rounded-full text-on-surface-variant hover:bg-surface-variant hover:text-on-surface font-medium text-[14px] transition-colors" %> + <%= form.submit "Save", class: "bg-primary hover:bg-primary-hover text-white px-6 py-2 rounded-full font-medium text-[14px] transition-all shadow-sm cursor-pointer" %>
- <% end %> - - <%# --- Page Header --- %> -
- <%= link_to "/", class: "group inline-flex items-center gap-3 text-sm font-medium text-gray-500 hover:text-blue-600 transition-colors mb-6" do %> -
- -
- Back to Dashboard - <% end %> +
-

- Edit Profile - <%= image_tag("edit.svg", class: "w-6 h-6 sm:w-8 sm:h-8 opacity-40") %> -

-

- Update your personal details and account settings below. -

-
+ +
+
-
-
- <%= form_with url: user_edit_path, model: @user, method: :post, local: true, class: "space-y-10" do |form| %> - - <%# Shared Input Classes %> - <% input_classes = - "w-full px-4 py-3 border border-gray-200 rounded-xl text-gray-700 bg-gray-50 focus:outline-none focus:ring-4 focus:ring-blue-500/10 focus:border-blue-500 transition-all font-medium placeholder-gray-400 text-sm sm:text-base" %> - <% label_classes = - "block text-sm font-bold text-gray-700 uppercase tracking-wide mb-2" %> +
+ <%= render "courses/flash" %> +
- <%# --- Section 1: Account Details --- %> -
-

- Account Details + <% if @user.errors.any? %> +
+

+ <%= pluralize(@user.errors.count, "error") %> prohibited saving:

+
    + <% @user.errors.full_messages.each do |message| %> +
  • <%= message %>
    • + <% end %> +
+
+ <% end %> + + +
+

Account Details

-
- <%= form.label :name, "Name", class: label_classes %> - <%= form.text_field :name, class: input_classes, placeholder: "e.g. jdoe123" %> +
+
+ <%= form.label :name, "Name", class: "absolute top-2 left-4 text-xs font-medium text-on-surface-variant" %> + <%= form.text_field :name, placeholder: "e.g. jdoe123", class: "w-full bg-transparent border-none p-0 text-on-surface focus:ring-0 focus:outline-none text-[14px] mt-1" %>
-
- <%= form.label :web_link, "Link to MMU Directory or Portfolio", class: label_classes %> - <%= form.text_field :web_link, class: input_classes, placeholder: "https://..." %> +
+ <%= form.label :web_link, "Link to MMU Directory or Portfolio", class: "absolute top-2 left-4 text-xs font-medium text-on-surface-variant" %> + <%= form.text_field :web_link, placeholder: "https://...", class: "w-full bg-transparent border-none p-0 text-primary focus:ring-0 focus:outline-none text-[14px] mt-1 underline" %>
-
- <%= form.label :description, "Description", class: label_classes %> -

- Describe yourself to indicate to students what project topics - you may accept for the courses you are teaching. Also indicate - Consultation Hours (days of week and time) when students can - meet you to request you as supervisor in case of FYP/Mini IT - Project. +

+
+ <%= form.label :description, "Description", class: "absolute top-2 left-4 text-xs font-medium text-on-surface-variant" %> + <%= form.text_area :description, rows: 5, class: "w-full bg-transparent border-none p-0 text-on-surface focus:ring-0 focus:outline-none text-[14px] mt-1 resize-none overflow-hidden", data: { controller: "textarea-resize", action: "input->textarea-resize#resize" } %> +
+

+ Describe yourself to indicate to students what project topics you may accept for the courses you are teaching. Also indicate Consultation Hours (days of week and time) when students can meet you to request you as supervisor in case of FYP/Mini IT Project.

- <%= form.text_area :description, - class: - "#{input_classes} min-h-[140px] overflow-hidden resize-none", - data: { - controller: "textarea-resize", - action: "input->textarea-resize#resize", - } %>
-
- - <%# --- Section 2: Password --- %> -
-

- Change Password -

+

+ -
-
- <%= form.label :new_password, "New Password", class: label_classes %> - <%= form.password_field :new_password, - class: input_classes, - placeholder: "Leave blank to keep current" %> -
+ +
+

Change Password

-
- <%= form.label :new_password_confirmation, "Confirm Password", class: label_classes %> - <%= form.password_field :new_password_confirmation, - class: input_classes, - placeholder: "Confirm new password" %> -
+
+
+ <%= form.label :new_password, "New Password", class: "absolute top-2 left-4 text-xs font-medium text-on-surface-variant" %> + <%= form.password_field :new_password, placeholder: "Leave blank to keep current", class: "w-full bg-transparent border-none p-0 text-on-surface focus:ring-0 focus:outline-none text-[14px] mt-1" %>
-
- -
- <%= link_to "Cancel", - "/", - class: - "w-full sm:w-auto text-center text-sm font-bold text-gray-500 hover:text-gray-800 px-6 py-3.5 rounded-xl hover:bg-gray-100 transition-colors" %> - <%= form.submit "Save Changes", - class: - "w-full sm:w-auto cursor-pointer bg-slate-600 hover:bg-slate-700 text-white font-bold py-3.5 px-8 rounded-xl shadow-lg shadow-slate-500/20 hover:shadow-slate-500/30 transition-all transform active:scale-95 text-sm sm:text-base flex justify-center items-center" %> +
+ <%= form.label :new_password_confirmation, "Confirm Password", class: "absolute top-2 left-4 text-xs font-medium text-on-surface-variant" %> + <%= form.password_field :new_password_confirmation, placeholder: "Confirm new password", class: "w-full bg-transparent border-none p-0 text-on-surface focus:ring-0 focus:outline-none text-[14px] mt-1" %> +
+ - <% end %>
-
- -
-
+
+ <% end %> + \ No newline at end of file diff --git a/docs/adr/0016-profile-participants-courses-show-shell.md b/docs/adr/0016-profile-participants-courses-show-shell.md new file mode 100644 index 00000000..c0978bd6 --- /dev/null +++ b/docs/adr/0016-profile-participants-courses-show-shell.md @@ -0,0 +1,69 @@ +# ADR 016 — Course participant profiles mirror the courses/show shell and reuse the flat proposal row + +Date: 2026-09-19 +Status: Accepted + +## Context + +`courses/profile.html.erb` (the student/group profile reached from the course's +People/Groups tables) had drifted into a distinct visual language from the rest +of the app: it lived on `bg-gray-50`, used its own `w-[95%] mx-auto` main +sections, and rendered the Current Project through the old full project card +(`courses/_project_card` + `_project_card_contents`). That card also powered the +two project/proposal sections on `lecturers/show`. + +At the same time `courses/show` had been rebuilt as a white internal-scroll panel +(`main.flex-1.bg-white.rounded-tl-panel.h-[calc(100vh-var(--spacing-header))]`) +with an inline `shared/sidebar` in its own `
`, and a flat row +list item (`projects/_proposal_list_item` → `shared/_row_item`) had become the +standard way to render a proposal/project in a list. The design mockup +(`ProPro_Design/profile_mockup.html.erb`) specified a Classroom-style centered +column (`max-w-5xl`) with a hero (banner band + overlapping avatar), a two-cell +facts strip (Project Status | Supervisor) split by a hairline, and Group Members +as one divided list. + +## Decision + +- **Profile page chrome** = the `courses/show` shell: `content_for :body_class, + "bg-surface-tint"`, `shared/sidebar` rendered inline inside the page's own + `
`, and `main.flex-1.bg-white.rounded-tl-panel. + overflow-x-hidden.overflow-y-auto.h-[calc(100vh-var(--spacing-header))]` + (no `border-t border-l` — the mockup's artificial border pokes out of the + rounded top-left corner, so it is not reproduced). Content lives in a centered + `w-[95%] max-w-5xl mx-auto pt-8 pb-16` reading column. +- **Hero** mirrors the mockup per scenario: group = banner band + group name from + the batched hero; student = banner band + overlapping avatar (`-mt-12`, then + `sm:pt-14` in the identity row), name, email, and the coordinator-only Remove + From Course pill top-right in the identity row. The hero itself carries the + facts strip (Project Status pill + Supervisor), not a separate Profile + Information card. +- **Group Members** is a single divided list (`divide-y divide-outline-variant`) + with avatar + name + STUDENT ID (kept from the old view) + email, Classroom + "People" style. +- **Current Project** reuses `projects/_proposal_list_item` (the same flat row as + `courses/show` overview and `lecturers/show`) instead of `_project_card`. The + caller passes `path:` (preserving `from_participant` / `participant_type` so + the project-show breadcrumb still resolves to this profile) and + `linkable: policy(project).show?`. The row renders only when the project has a + `project_instance` (the old card was silently empty without one); otherwise + `shared/_empty_state` shows "No project has been submitted yet." +- **`_project_card` removal** is global, not profile-only: the two remaining + `_project_card` call sites on `lecturers/show` (Projects and Incoming Proposals) + now render `projects/_proposal_list_item` in a divided list, keeping the + lecturer-scoped path (`course_lecturer_project_path`) and the same policy gate. + `_project_card` and `_project_card_contents` are deleted (zero call sites). +- Both partials are unchanged in behavior and keep their locals-only, ivar-free + contracts so they stay reusable from any presenter or controller context. + +## Consequences + +- Profile and lecturers/project rows share one visual grammar with `courses/show` + (flat rows, status-pill palette, white panel chrome); the old card grid is gone + everywhere. +- Participant-scoped deep-links into the project show survive because the row's + `path` is caller-supplied rather than derived inside the partial. +- Removing a project's `project_instance` history no longer crashes the profile + render (guarded on `@current_instance`, matching the old silent-empty + behavior). +- System/controller coverage was added for both profile scenarios and both + `lecturers/show` sections, which previously had none. \ No newline at end of file diff --git a/docs/checklist.md b/docs/checklist.md deleted file mode 100644 index bf2fbf82..00000000 --- a/docs/checklist.md +++ /dev/null @@ -1,148 +0,0 @@ -# `refactor/design` — QA Sanity Check List - -For manual pass, not a replacement for the automated suite (list of what's already automated is at the bottom — don't duplicate effort there, go deeper where automation can't see: layout, responsiveness, visual state). Built from the actual `main`...`refactor/design` diff, so this should cover everything touched, not just the pages that look different. - ---- - -## 0. Read this before starting — merge-base gap - -`refactor/design` branched off a commit that's now **16 commits behind `main`**. That's not just churn — three of those commits are full features `main` has that this branch doesn't: - -| On `main`, not in `refactor/design` | What it means for testing | -|---|---| -| **Email domain restriction** (course settings) — full feature: migration, controller action, form partial | `refactor/design`'s rewritten `courses/settings.html.erb` has **no email-restriction UI at all** — the column doesn't even exist in this branch's schema. Testing settings on this branch today can't catch this; it'll surface as a missing feature the moment this merges into `main` unless someone re-adds the form to the new design first. | -| **OTP removed** from sign-in | This branch's `db/schema.rb` still has the `otps` table and `User has_one :otp`. Sign-in itself looks fine here (no OTP field in the form either way), but this is a live merge-conflict risk in the auth flow — worth a full sign-in retest right after merge, not just now. | -| **Self-service sign-up + `is_staff` removed** | This branch still has `is_staff` in 7 files (`courses_controller`, `user_controller`, mailers, `course_policy`, schema). `user/new_staff.html.erb` / `user/new_student.html.erb` on this branch reflect the *old* registration model. | - -**Recommendation: don't treat a pass on `refactor/design` today as a pass on what actually ships.** Either get `main` merged/rebased into this branch before the full QA pass, or plan on a second, shorter pass focused specifically on settings/sign-in/sign-up right after that merge lands. - ---- - -## 1. High-priority — specific regressions to verify, not just "does it look right" - -These aren't visual — they're the diff calling out actual behavior changes. Test these deliberately, they won't show up from casually clicking around. - -1. **Enrolment removal — authorization fix.** `EnrolmentsController#destroy` used to trust a client-submitted `coordinator_id` param to decide who's allowed to remove someone from a course; it now checks the actual logged-in user against the course's real coordinators, and scopes the enrolment lookup to that course. This is a real fix, so verify it didn't overcorrect: - - As a coordinator: remove a student/enrolment from **your own** course → should still succeed. - - As a lecturer or student (non-coordinator): attempt the same action (via UI, and via direct request with a crafted `coordinator_id` if you're comfortable poking at it with devtools) → should be rejected/redirected, not succeed. - - As a coordinator of Course A, try to affect an enrolment id belonging to Course B → should fail (previously this may have worked with the right `coordinator_id`). -2. **Progress update redirect no longer passes `tab: 'progress'`.** Create, edit, and delete a progress update → confirm you land back on the **Progress Updates tab**, not silently reset to Overview/Project Details. If tab state is meant to come from the cookie-persistence mechanism now instead of the query param, confirm that actually happens — don't assume it does. -3. **Dangling breadcrumb reference.** `config/breadcrumbs.rb`'s `crumb :topic` block was deleted, but `topics/show.html.erb` still calls `breadcrumb :topic, @topic`. Load **any** topic's show page and confirm: no error, and the breadcrumb trail at top renders sensibly (even if it's now just blank/generic — check it's not throwing). -4. **`/courses/:id/participants` route is gone entirely** (controller, route, and view all deleted) — confirm nothing in the app still links to it. Check the People tab on `courses/show` is a complete replacement, not a partial one (filters, sorting, search all present). - ---- - -## 2. Global chrome — test once, applies to (almost) every page - -The header, sidebar, and footer are shared partials rendered on ~30 views. Bugs here have the widest blast radius, so give this section real time. - -- **Header:** desktop shows ProPro wordmark + breadcrumb trail + Log out; mobile (<640px) shows the current page name only, no wordmark/breadcrumb/Log out. On mobile, the settings-gear and edit-template icons should appear **only** for a coordinator, and only in the header — never doubled up with a desktop-tab version at the same width. -- **Sidebar:** hamburger toggle opens/closes it as a drawer on mobile/tablet; on desktop it's persistent and collapsible (icon-rail mode) — check the collapse/expand toggle, and that it **remembers its collapsed state across a page navigation** (cookie-based). - - Confirm the "Enrolled" course list, "Edit profile", and "Log out" nav items all work from the collapsed (icon-only) state — hover tooltips, not just icons with no label anywhere. -- **Footer:** now rendered globally from `application.html.erb` instead of per-page. Confirm it does **not** appear on pages that are supposed to suppress it (`project/edit`, `project/new`, `topics/edit`, `topics/new`, `courses/settings`, `project_groups/index`, `sessions/new`, `passwords/new`, `passwords/edit`, `project_templates/edit`) and **does** appear everywhere else. -- **Pages with no `@course` in scope** (homescreen, static pages, `user/profile`, sign-in, password reset): confirm the header renders without error and without any course-specific icons — this was flagged in code review as guarded, but worth eyeballing directly. -- **Pages that render `shared/_sidebar` but aren't one of the three fully-redesigned show pages** (`courses/profile.html.erb`, `lecturers/show.html.erb`) — confirm the sidebar actually appears there (it's wired via `content_for(:sidebar)` rather than the inline render the redesigned pages use — different mechanism, same partial, worth checking it actually fires). -- **Pages with no sidebar at all** (`participants` doesn't exist anymore, but `progress_updates/*`, `project_groups/index`, `static_pages/*`, `user/profile` currently render with no nav chrome) — confirm that's the intended interim state and not something that reads as broken navigation to a real user. - ---- - -## 3. Responsive matrix - -Test breakpoints actually in use in this branch: **`sm` (640px)**, **`lg` (1024px)**, and a custom **`1245px`** breakpoint (`--breakpoint-comments` — governs when the comments drawer docks permanently vs. slides in as an overlay). Test at minimum these widths: **360px** (smallest supported per the docs), **639px**, **640px**, **1023px**, **1024px**, **1244px**, **1245px**, and a normal desktop width (≥1440px). - -- `courses/show` — 4 tabs (Overview/Topics/People/Groups) stay visible at every width, no overflow/"More" menu, no horizontal page scroll, at 360px specifically (called out in the code as the design floor). -- `projects/show` and `topics/show` — content tabs scroll horizontally on narrow widths with a fade mask on the trailing edge that appears **only** when tabs are actually cut off (not permanently visible). -- Comments drawer: static docked column above 1245px; below that, off-canvas slide-in triggered by the chat-bubble icon in the tab bar, with a backdrop that dismisses it on tap and closes on Escape. -- Mobile review action bar (bottom-pinned version switcher + actions) — visible on mobile only, thumb-reachable, doesn't overlap the comments trigger. -- Course settings page and new/edit project & topic forms — these are full-screen "takeover" layouts (no sidebar/header/footer). Confirm they're still usable and non-overflowing down to 360px. - ---- - -## 4. Per-role critical paths - -Run each of these end to end as the actual role, not just by eyeballing the page as an admin. - -**Student** -- Browse Topics Directory, propose a new project (both "Propose to Lecturer" and "Base on a Topic" paths via the method picker modal), edit a pending proposal, view own project status/history, add a comment, view/download the diff between versions, submit a progress update if the course has them enabled. -- Join a course via course code. -- Confirm a student **cannot** see coordinator-only controls anywhere in the redesigned chrome (settings gear, edit-templates icon, review-approve actions). - -**Lecturer / Supervisor** -- Review a pending proposal (approve / reject / request redo), leave a comment, use the version switcher on a project with multiple versions. -- View own supervision capacity indicator where shown (People tab, lecturer card). -- `lecturers/show.html.erb` (Follow-up page, untouched styling) — confirm it still works end-to-end wrapped in the new header/sidebar. - -**Coordinator** -- Course settings: every section (Class Details, General incl. coursecode widget, Permissions and Rules, Grouping) — save each independently, confirm no cross-section data loss. -- Add students / add lecturers (CSV and manual), remove a student/enrolment (see §1.1), toggle "Base on a Topic", copy a course, copy a topic between courses. -- People / Groups / Topics-directory tabs: search, sort, filter, "show all" expansion — all of these are htmx-driven partial swaps, see §6. -- Export CSV. - ---- - -## 5. Feature-by-feature checklist - -- **Course tabs (Overview/Topics/People/Groups):** switching tabs persists the active tab via a per-course cookie — reload the page and confirm it reopens on the last tab, not always Overview. Same check on `projects/show` and `topics/show` content tabs (separate cookie key). -- **Topics Directory:** supervisor groups collapse/expand individually and via "Collapse all"/"Expand all"; the current viewer's own group (staff only) is always pinned first, even at zero topics; students see plain A→Z with no pinned group. -- **`/` search shortcut on `courses/show`:** focuses the current tab's search box; if the current tab has none, jumps to the Groups search as fallback (and that jump itself gets written to the tab-persistence cookie). Confirm it's a no-op while typing in any input/textarea/select/contenteditable, and while a dialog/overlay is open. -- **Copy Topic / Copy Course overlays (Turbo Frame):** open, submit, cancel, and error paths (e.g. submitting with a required field missing) all render inside the frame correctly — confirm a validation error doesn't blow away the whole page or leave the frame in a broken state. -- **Coursecode widget:** Generate and Re-generate (this is a formless control living inside the settings form without nesting a `` — worth specifically confirming the containing settings form doesn't get submitted accidentally by clicking these), and the enable/disable toggle (fires its own request outside the main form). -- **Template fields (project templates):** add/remove/reorder fields, all field types (short text, textarea, dropdown, radio), the "free-edit" flag, "is project title" flag, required/hint text — both in the template editor and reflected correctly on the actual proposal form. -- **Markdown editor (EasyMDE)** on template textareas and progress-update feedback fields — toolbar, preview mode, and that content round-trips correctly through create/edit. -- **Grouping settings / grouping preview (htmx):** changing group size min/max live-updates the preview count without a full page reload; confirm it doesn't silently show a stale preview if you change the inputs rapidly. -- **Record Progress Update modal** — open, submit, validation errors, close/cancel without submitting. -- **Empty states:** each redesigned section (My Submission with nothing submitted, no topics, no groups, no pending proposals, etc.) — confirm the actual empty-state illustration/copy shows, not a blank gap or a stray leftover element from the old styling. -- **Flash messages** — trigger a notice and an alert (e.g. failed sign-in for alert, a successful save for notice) — you mentioned these move to the bottom and expire; confirm timing/position/dismissal actually work as intended and don't stack/overlap if two fire close together. - ---- - -## 6. JS controller interaction checklist - -Each of these is new or touched this branch — exercise the actual interaction, not just page load: - -| Controller | What to click/do | -|---|---| -| `sidebar` | Toggle open/close on mobile; collapse/expand on desktop; refresh and confirm collapsed state persisted | -| `tabs` / `tab-fade` | Switch every tab on `courses/show`, `projects/show`, `topics/show`; narrow the window until tabs overflow and confirm the fade mask appears/disappears correctly | -| `comments-drawer` | Open/close via trigger button and Escape key, at widths above and below 1245px | -| `modal` | Every modal-driven flow (method picker, record update modal, copy overlays where applicable) — open, close via backdrop click, close via explicit cancel | -| `dropdown` | Any dropdown/context menu introduced or restyled | -| `collapsible-sections` / `expandable-rows` | Overview section collapse, Topics-directory supervisor-group collapse — individually and via "collapse/expand all" | -| `search-shortcut` | `/` key behavior described in §5 | -| `students-select`, `expandable-rows` (post-htmx-swap) | After an htmx table swap (search/sort/filter), confirm selection state and row-expand state don't silently break — these controllers use delegated listeners specifically because the DOM gets replaced under them | -| `copy-topic` | Full copy-topic dialog flow | -| `coursecode-form-handler` | Generate/Re-generate/toggle, see §5 | -| `method-picker` | Choosing "Propose to Lecturer" vs "Base on a Topic" and confirming the right fields show | -| `version-select` | Jumping between project/topic versions from the review card and the mobile review action bar | -| `record-update-modal` | See §5 | -| `textarea-resize` | Comment box and progress-update feedback box auto-grow as you type | -| `scroll-spy` (if still wired anywhere) | Confirm intentional — this was on `main`'s old layout; check it wasn't left half-wired | - ---- - -## 7. Regression check — Follow-up (intentionally untouched) pages - -These pages weren't restyled, but they now sit inside the **new** header/sidebar/footer, so they need a smoke test even though nobody touched their content: - -- Topics index (`topics/index.html.erb`) — flagged for removal later, but should still function today -- `lecturers/show.html.erb` (Lecture profile) -- `courses/profile.html.erb` (Groups profile) -- `user/profile.html.erb` (Edit profile) -- Static pages: About, Privacy Policy, Terms of Service -- `project_templates/edit.html.erb` -- Course settings' Grouping section (restyle explicitly deferred) -- Coursecode component styling (restyle explicitly deferred, functionality above is in scope) -- Flash message positioning (old behavior expected here until the restyle lands) -- "Reuse details from another topic" flow — confirm current (pre-fix) behavior is at least not worse than on `main` - -For each: load it, confirm no console errors, confirm the new sidebar/header/footer don't clip or overlap its (unrestyled) content, and confirm nothing in the old markup was relying on something this branch removed from the layout (old topbar, old font stack, old breadcrumb structure). - ---- - -## Appendix — already covered by the automated suite - -Don't spend manual time re-verifying these in detail; skim them instead and go deep on gaps. - -`test/system/courses/{course_tab_persistence,course_tabs,overview_tab,settings_coursecode,settings_save,solo_topics_toggle}_test.rb`, `test/system/projects/{change_status,mobile_overflow,project_form,project_show_responsive,project_versioning}_test.rb`, `test/system/topics/{change_status,copy_topic_dialog,topic_versioning}_test.rb`, `test/system/{sidebar_collapse,tabs_sticky,form_overflow}_test.rb`. - -Not covered by any system test today, worth flagging to the team as an automation gap once manual QA confirms behavior: the enrolment-authorization fix (§1.1), the progress-update redirect/tab behavior (§1.2), and the People/Groups/Topics-directory htmx search-sort-filter-paginate flows (§6). \ No newline at end of file diff --git a/docs/correctness_plan.md b/docs/correctness_plan.md deleted file mode 100644 index 308e2837..00000000 --- a/docs/correctness_plan.md +++ /dev/null @@ -1,130 +0,0 @@ -# `refactor/design` Audit — vs `main` - -Scope: `app/views`, `app/javascript/controllers`, `app/helpers`, `app/presenters`, `app/assets` on `refactor/design` (merge-base `88506851`), diffed against `main`, cross-checked against *Sustainable Web Development with Ruby on Rails* (Copeland) ch. 7–8 and 10–11. Not an implementation pass — findings and recommendations only. - -This branch touches 241 files (+29,053/‑11,420). It's a huge, genuinely disciplined redesign — good ADRs, a real glossary (`CONTEXT.md`), a presenter with its own test, an internal Tailwind audit doc, no jQuery, no framework sprawl in JS. The findings below are the gaps in an otherwise strong pass, not a verdict on the branch. - ---- - -## 1. Unused partials (the "backup left behind" check) - -Cross-referenced every `_*.html.erb` against every `render` call in `app/` (including controller-level `render partial:` for AJAX/Turbo responses). **9 partials are dead** — zero render call anywhere: - -| Partial | Verdict | -|---|---| -| `app/views/courses/_course_card.html.erb` | Orphaned. No caller anywhere in the repo. | -| `app/views/lecturers/_lecturer_card.html.erb` | Orphaned. **Also** sets `content_for :no_footer` — see §4. | -| `app/views/project_templates/_new_field.html.erb` | Orphaned. | -| `app/views/projects/_comment.html.erb` | Orphaned — superseded. `_project_comments.html.erb` and `_topic_comments.html.erb` now inline the comment-row markup directly instead of calling `render "comment"`. Confirm that was deliberate (dropping the abstraction) and not an accidental fork; if deliberate, delete this file so the next person doesn't "fix" `_project_comments` by pointing it back at a partial two different views no longer agree on. | -| `app/views/topics/_topbar.html.erb` | Orphaned, and worth deleting first. CRLF line endings, pre-Tailwind classes (`topbar-icon`, `topbar-info`, `topbar-actions`) that no longer exist in any stylesheet — this is a fossil from before the CSS migration in §5, not just before this branch. | -| `app/views/project_groups/_group_list.html.erb` | Orphaned. | -| `app/views/project_groups/_grouping_closed.html.erb` | Orphaned. | -| `app/views/project_groups/_my_group.html.erb` | Orphaned — superseded by `_my_group_panel.html.erb`, which duplicates its purpose and *is* live. | -| `app/views/project_groups/_no_group.html.erb` | Orphaned. | -| `app/views/project_groups/_ungrouped_students.html.erb` | Orphaned. | - -**Chain note:** `_group_card.html.erb` is only rendered by `_group_list.html.erb`, and `_group_list.html.erb` is itself never rendered — so `_group_card` is dead too even though a naive one-hop check would call it "used." `project_groups/index.html.erb` renders its whole browse/filter grid inline now and only calls out to `_my_group_panel`. That leaves **6 of the 8 partials in `app/views/project_groups/` dead**, with only `index.html.erb` and `_my_group_panel.html.erb` live. - -This lines up with your backend note: grouping isn't merged to `main` yet, and the DB has zero confirmed groups in production. My read is `project_groups/` is exactly the kind of directory that accumulates this — a UI was built ahead of the backend, the backend direction changed (confirmed-vs-all-groups), and the index page got rewritten inline without anyone deleting the partials it used to call. Worth sweeping this directory in the same pass as the group-table decision, since whoever resolves that will already be in this code. - -Also flag `app/javascript/controllers/hello_controller.js` — zero `data-controller="hello"` anywhere, it's the Stimulus scaffold default (present on `main` too, so not this branch's doing, but it's dead weight sitting next to real controllers and worth a one-line deletion whenever someone's next in that folder). - -**Not flagged, confirmed live:** `_project_card.html.erb` (0 hits in this branch's diff, but rendered from `courses/profile.html.erb` and `lecturers/show.html.erb` — both Follow-up pages, correctly untouched). - ---- - -## 2. Partials vs. Copeland 7.3 ("Use Locals to Pass Parameters") - -34 of 78 partials document their interface with a `<%# locals: (...) %>` header — good, that's the right convention and more than half the codebase doesn't have it, so it's clearly an intentional practice, not an accident. - -The gap: **25 partials reach into controller instance variables instead of receiving locals**, and a meaningful chunk of those are partials this branch wrote or rewrote, not legacy carryover: - -- `shared/_header.html.erb`, `shared/_sidebar.html.erb` — both use `@course` -- `projects/_project_overview.html.erb`, `_project_details.html.erb`, `_proposal_list_item.html.erb`, `_project_comments.html.erb` -- `topics/_topic_comments.html.erb`, `_copy_topic_details.html.erb`, `_review_actions.html.erb`, `_copy_topic_overlay.html.erb` -- `courses/_project_details_card.html.erb`, `_topic_card.html.erb`, `_overview_tab.html.erb`, `_people_tab.html.erb`, `_topic_directory_tab.html.erb`, `_groups_tab.html.erb` - -The sharper version of this finding: **four of those — `_project_overview`, `_project_comments`, `_topic_comments`, `_review_actions` — already have a `locals:` strict-locals header, and still reach for `@course`/`@window`/`@is_coordinator` in the body.** That's not just missing the convention, it's declaring the convention and then quietly opting back out of it. A partial with a `locals:` header reads as "here is the complete interface" — a reviewer or a future editor has no reason to grep the body for hidden `@` dependencies once that header is there. Right now they'd need to. - -`shared/_header.html.erb` and `_sidebar.html.erb` are the two highest-leverage ones to fix, precisely because they're rendered on ~30 views (per the header's own comment) — every one of those call sites has an implicit, undeclared dependency on `@course` being set (or nil-safe, which it correctly is here — see §6). - -## 3. Helpers vs. Copeland 8.1 ("Don't Conflate Helpers with Your Domain") - -Mixed. `ApplicationHelper` and `BreadcrumbHelper` are good examples of the chapter's positive case — `status_badge_classes`, `format_timestamp`, `render_custom_breadcrumbs` are pure markup/formatting, exactly what 8.2 asks for. Two others aren't: - -- **`ProjectsHelper#name(user_id)`** runs `User.find_by(id: user_id)` — a database query inside a view helper. This is the chapter's core warning case verbatim, and it's an N+1 risk if it's called per-row anywhere. -- **`CoursesHelper#group_project_for` / `#student_project_for`** reach into `@projects_by_owner` (a controller ivar) and do a hash lookup — domain-shaped logic, and it inherits the same undeclared-dependency problem as §2 since it's not an argument. -- **`ProjectsHelper#show_progress_tab?` and `TopicsHelper#show_progress_tab?`** are byte-for-byte identical (`@course.use_progress_updates && @current_instance.status == 'approved'`), duplicated across two helper modules instead of living in one place — a small DRY miss, but an easy one to fix since it's a single method. - -## 4. Presenters vs. Copeland 8.3 - -`OverviewPresenter` is a good instance of the pattern the chapter actually endorses: plain Ruby object, explicit keyword-arg constructor, intention-revealing query methods (`show_pending_topics?`, `any_sections?`), no view-rendering logic bleeding in, and it has its own unit test (`test/presenters/overview_presenter_test.rb`). This is worth calling out as a positive pattern to repeat elsewhere, not just a pass/fail. - -The catch: `courses/_overview_tab.html.erb` receives it as `@presenter`, not as a local. The whole value of building an explicit object here is that a partial's dependency on it becomes visible and testable at the call site — stuffing it into an ivar and reaching for it inside the partial throws that away and makes `_overview_tab` no different from any other ivar-coupled partial in §2. - -## 5. `content_for` called from inside a partial - -`app/views/lecturers/_lecturer_card.html.erb` calls `content_for :no_footer, true` in its own body. A partial reaching up to flip page-level layout flags breaks the "partial is an isolated, reusable component" contract from 7.3.1/7.3.2 — render this partial twice, or from a page that already made its own footer decision, and the outcome depends on render order. (It's also one of the 9 dead partials from §1, so it's not live today — but it's the kind of pattern that should get caught in review before it ships again, not just deleted this once.) - -Smaller consistency note: `sessions/new.html.erb` and `passwords/{new,edit}.html.erb` use `<% provide :no_footer, true %>` while every other page uses `<% content_for :no_footer, true %>`. They're aliases, so nothing's broken, but two spellings of the same call in one codebase is exactly the kind of needless inconsistency the book's "Just Use ERB" chapter argues against — pick one. - ---- - -## 6. Correctness vs. `main` — shared header/sidebar/footer - -`application.html.erb`'s `unless content_for?(:no_sidebar/:no_header/:no_footer)` gating is unchanged from `main` — this branch didn't introduce that mechanism, it's inherited. What changed is what gets rendered inside it: `shared/_header.html.erb` and `_sidebar.html.erb` are new, and both touch `@course`. - -Checked this specifically because `_header`/`_sidebar` now render globally on pages that may have no `@course` — static pages, `user/profile`, the homescreen. **Both partials guard correctly**: the header's coordinator-icons block is behind `@course.present? && policy(@course).update?` (with a comment noting exactly why: "header is shared by ~30 views"), and the sidebar's only `@course` use is an equality check (`course == @course`) that's nil-safe by construction. No regression here — flagging it as checked, not as a bug. - -One pre-existing (not new) gap worth a note while you're in this area: the sidebar only actually paints on pages that either render `shared/_sidebar` directly inline (`courses/show`, `projects/show`, `topics/show` — the three fully-rewritten pages) or explicitly populate `content_for(:sidebar)` (`courses/profile.html.erb`, `lecturers/show.html.erb` — both bridged to the new sidebar as part of this branch, good). Everything else that doesn't opt out via `no_sidebar` — `participants/index`, `progress_updates/*`, `project_groups/index`, `static_pages/*`, `user/profile` — just silently renders without a sidebar, because nothing populates the yield. That's the same architecture `main` had (I diffed `main`'s layout to confirm), so it's not something this branch broke. But several of those pages are on your Follow-up list, so it's worth deciding now whether "no sidebar" is the intended interim state for them or just an artifact of the old layout nobody's revisited — otherwise whoever redesigns `user/profile.html.erb` next will inherit an ambiguous default. - -CSS cleanup: confirmed the entire legacy `app/assets/stylesheets/` directory — including the literally-named `backup_courses.css` — was deleted in this branch in favor of `app/assets/tailwind/application.css`. That's exactly the cleanup this audit is checking for, done correctly. Good sign that the team does clean up when it gets to it; the partials in §1 are just what didn't get swept yet. - ---- - -## 7. JavaScript vs. Copeland ch. 10–11 - -Stimulus-only, no jQuery, controllers eager-loaded via the standard `eagerLoadControllersFrom` importmap convention — this is squarely "carefully choose one framework" done right, and it's a clean fit for "embrace server-rendered Rails views." - -One thing worth a deliberate look: **htmx and Turbo Frames are both in active use for the same job** — swapping a fragment of HTML in response to a request. Turbo Frames handle the copy-topic and copy-course overlays and the coursecode widget; htmx (extensively — 6+ partials) handles the courses/show People, Groups, and Topics-directory search/sort/filter/paginate. If htmx was picked deliberately for that corner because `hx-include`/`hx-vals` composes multiple sibling filter inputs into one request more declaratively than Turbo Frames does out of the box, that's a legitimate, defensible reason — worth writing down as an ADR next to the others in `docs/adr/`, the same way the coursecode widget and sidebar-collapse decisions are. Right now it reads as two different answers to "how do we swap HTML" living in different corners of the same app, and the next contributor won't know which one to reach for on the next table. - -`_topic_picker.html.erb`'s `