From fa185046a66273338237f77a8dfa283cac6deb3e Mon Sep 17 00:00:00 2001 From: Sean Dickinson Date: Sat, 29 Aug 2026 10:32:57 -0400 Subject: [PATCH 1/3] fix: remove implicit ordering --- app/controllers/content_modules_controller.rb | 2 +- app/models/concerns/ordered.rb | 7 +++++++ app/models/content_module.rb | 6 +++--- app/models/link.rb | 3 ++- test/models/content_module_test.rb | 8 ++++++++ 5 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 app/models/concerns/ordered.rb diff --git a/app/controllers/content_modules_controller.rb b/app/controllers/content_modules_controller.rb index 4dfbb1c..3720eb6 100644 --- a/app/controllers/content_modules_controller.rb +++ b/app/controllers/content_modules_controller.rb @@ -5,7 +5,7 @@ def index @programs = Program.order(:name) @active_program = @programs.find { |p| p.id.to_s == params[:program_id] } || @programs.first @modules_by_level = @active_program - .content_modules + .content_modules.ordered .includes(:links) .group_by(&:level) end diff --git a/app/models/concerns/ordered.rb b/app/models/concerns/ordered.rb new file mode 100644 index 0000000..2e56b03 --- /dev/null +++ b/app/models/concerns/ordered.rb @@ -0,0 +1,7 @@ +module Ordered + extend ActiveSupport::Concern + + included do + scope :ordered, -> { order(:position) } + end +end diff --git a/app/models/content_module.rb b/app/models/content_module.rb index 810add8..0f01694 100644 --- a/app/models/content_module.rb +++ b/app/models/content_module.rb @@ -1,11 +1,11 @@ class ContentModule < ApplicationRecord + include Ordered + belongs_to :program - has_many :links, dependent: :destroy + has_many :links, -> { ordered }, dependent: :destroy has_many :classroom_modules, dependent: :restrict_with_error enum :level, { basic: "basic", moderate: "moderate", advanced: "advanced" }, validate: true validates :name, :level, presence: true - - default_scope { order(:position) } end diff --git a/app/models/link.rb b/app/models/link.rb index 995a7c4..b1f7a2a 100644 --- a/app/models/link.rb +++ b/app/models/link.rb @@ -1,4 +1,6 @@ class Link < ApplicationRecord + include Ordered + belongs_to :content_module enum :link_type, { survey: "survey", game: "game" }, validate: true @@ -6,5 +8,4 @@ class Link < ApplicationRecord validates :title, :url, :link_type, presence: true validates :url, format: { with: /\Ahttps?:\/\/.+\z/i, message: "must start with http:// or https://" }, allow_blank: true - default_scope { order(:position) } end diff --git a/test/models/content_module_test.rb b/test/models/content_module_test.rb index 95a4039..dcbc2a9 100644 --- a/test/models/content_module_test.rb +++ b/test/models/content_module_test.rb @@ -35,6 +35,14 @@ class ContentModuleTest < ActiveSupport::TestCase end end + test "links are returned in position order" do + content_module = ContentModule.create!(program: @program, level: "basic", name: "Ordered Module") + last = content_module.links.create!(title: "Last", url: "https://example.com/2", link_type: "survey", position: 2) + first = content_module.links.create!(title: "First", url: "https://example.com/1", link_type: "survey", position: 1) + + assert_equal [ first, last ], content_module.links.to_a + end + test "cannot be destroyed when classroom modules exist" do content_module = ContentModule.create!(program: @program, level: "basic", name: "Assigned Module") classroom_program = classroom_programs(:one) From 7a91ef6056689949bf500b59b1016417ecca7a0b Mon Sep 17 00:00:00 2001 From: Sean Dickinson Date: Sat, 29 Aug 2026 10:37:55 -0400 Subject: [PATCH 2/3] test: add tests for content ordering --- test/controllers/content_modules_controller_test.rb | 11 +++++++++++ test/models/content_module_test.rb | 8 -------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/test/controllers/content_modules_controller_test.rb b/test/controllers/content_modules_controller_test.rb index 8dd91d5..d50d3dd 100644 --- a/test/controllers/content_modules_controller_test.rb +++ b/test/controllers/content_modules_controller_test.rb @@ -45,6 +45,17 @@ class ContentModulesControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "edit lists links ordered by position" do + @content_module.links.destroy_all + @content_module.links.create!(title: "Last Link", url: "https://example.com/2", link_type: "survey", position: 2) + @content_module.links.create!(title: "First Link", url: "https://example.com/1", link_type: "survey", position: 1) + + get edit_content_module_url(@content_module) + + hrefs = css_select("table tbody tr td:first-child a").map { |link| link["href"] } + assert_equal [ "https://example.com/1", "https://example.com/2" ], hrefs + end + test "should update content module" do patch content_module_url(@content_module), params: { content_module: { name: "Updated Name" } diff --git a/test/models/content_module_test.rb b/test/models/content_module_test.rb index dcbc2a9..95a4039 100644 --- a/test/models/content_module_test.rb +++ b/test/models/content_module_test.rb @@ -35,14 +35,6 @@ class ContentModuleTest < ActiveSupport::TestCase end end - test "links are returned in position order" do - content_module = ContentModule.create!(program: @program, level: "basic", name: "Ordered Module") - last = content_module.links.create!(title: "Last", url: "https://example.com/2", link_type: "survey", position: 2) - first = content_module.links.create!(title: "First", url: "https://example.com/1", link_type: "survey", position: 1) - - assert_equal [ first, last ], content_module.links.to_a - end - test "cannot be destroyed when classroom modules exist" do content_module = ContentModule.create!(program: @program, level: "basic", name: "Assigned Module") classroom_program = classroom_programs(:one) From 656df30c5cb3053fb858bab9e90a07a18f2022d7 Mon Sep 17 00:00:00 2001 From: Sean Dickinson Date: Sat, 29 Aug 2026 10:49:37 -0400 Subject: [PATCH 3/3] chore: lint --- app/models/link.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/link.rb b/app/models/link.rb index b1f7a2a..c24ba84 100644 --- a/app/models/link.rb +++ b/app/models/link.rb @@ -7,5 +7,4 @@ class Link < ApplicationRecord validates :title, :url, :link_type, presence: true validates :url, format: { with: /\Ahttps?:\/\/.+\z/i, message: "must start with http:// or https://" }, allow_blank: true - end