Conversation
There was a problem hiding this comment.
Pull request overview
Updates Learning Path presentation to better support (a) single-topic learning paths and (b) “unordered” learning paths where topic ordering labels are hidden.
Changes:
- Auto-expands the topic contents when a learning path has exactly one topic (CSS/JS + view classes).
- Adds an
unorderedflag onlearning_pathswith a form checkbox and applies an.unorderedclass in the show view. - Adds controller/view tests for single-topic display.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/controllers/learning_paths_controller_test.rb | Adds assertions for multi-topic vs single-topic rendering. |
| db/schema.rb | Reflects new unordered column on learning_paths. |
| db/migrate/20260421144919_add_unordered_to_learning_paths.rb | Adds unordered boolean column. |
| config/locales/en.yml | Adds hint text for the new form option. |
| app/views/learning_paths/show.html.erb | Adds class logic for single-topic + unordered display modes. |
| app/views/learning_paths/_form.html.erb | Adds checkbox to edit unordered. |
| app/controllers/learning_paths_controller.rb | Permits :unordered via strong params. |
| app/assets/stylesheets/learning-paths.scss | Adds styling for .single-topic and .unordered. |
| app/assets/javascripts/learning_paths.js | Disables expand/collapse click behavior for single-topic view. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,5 @@ | |||
| class AddUnorderedToLearningPaths < ActiveRecord::Migration[7.2] | |||
| def change | |||
| add_column :learning_paths, :unordered, :boolean | |||
There was a problem hiding this comment.
This adds a new boolean column without a default, leaving existing rows as NULL (tri-state) until they’re edited. If NULL and false are meant to behave the same, consider setting default: false (and optionally backfilling existing records in the migration) so the DB state matches the intended semantics and queries like where(unordered: false) behave predictably.
| add_column :learning_paths, :unordered, :boolean | |
| add_column :learning_paths, :unordered, :boolean, default: false, null: false |
| assert assigns(:learning_path) | ||
| assert_select '.learning-path-topic', count: 1 | ||
| assert_select '.learning-path-topic.single-topic', count: 1 | ||
| end |
There was a problem hiding this comment.
The PR introduces the unordered display option, but the controller/view tests added here only cover the single-topic styling. It would be good to add a test case that sets a fixture learning path to unordered: true and asserts the rendered topic container has the .unordered class (or otherwise verifies the unordered mode is applied).
| end | |
| end | |
| test 'displays unordered view when learning path is unordered' do | |
| learning_path = learning_paths(:one) | |
| learning_path.update!(unordered: true) | |
| get :show, params: { id: learning_path } | |
| assert_response :success | |
| assert assigns(:learning_path) | |
| assert_select '.learning-path-topic.unordered', minimum: 1 | |
| end |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary of changes
Motivation and context
#1169
#1247
Screenshots
LP with only 1 topic:
Unordered LP:
Form:
Display:
Checklist