contentable drop module columns

app/controllers/admin/content_modules_controller.rb

@@ -26,7 +26,7 @@ def create
 
   def edit
     @contents = @content_module.contents.includes(:contentable)
-    @available_games = Game.where.not(id: @content_module.games).order(:title)
+    @available_games = Game.not_in(@content_module).order(:title)
   end
 
   def update

app/controllers/admin/links_controller.rb

@@ -1,16 +1,15 @@
 class Admin::LinksController < AdminController
   before_action :set_content_module, only: %i[new create]
-  before_action :set_link, only: %i[edit update destroy]
+  before_action :set_link, only: %i[edit update]
 
   def new
-    @link = Link.new(content_module: @content_module)
+    @link = Link.new
   end
 
   def create
-    @link = Link.new(link_params.merge(content_module: @content_module))
+    @link = Link.new(link_params)
 
-    if @link.save
-      @content_module.contents.create!(contentable: @link)
+    if save_with_content
       redirect_to edit_admin_content_module_path(@content_module), notice: "Link was successfully created."
     else
       render :new, status: :unprocessable_entity
@@ -28,11 +27,6 @@ def update
     end
   end
 
-  def destroy
-    @link.destroy!
-    redirect_to edit_admin_content_module_path(@link.content_module), notice: "Link was successfully deleted.", status: :see_other
-  end
-
   private
     def set_content_module
       @content_module = ContentModule.find(params.expect(:content_module_id))
@@ -45,4 +39,10 @@ def set_link
     def link_params
       params.expect(link: [ :title, :url, :link_type ])
     end
+
+    def save_with_content
+      Link.transaction do
+        @link.save && @content_module.contents.create!(contentable: @link)
+      end
+    end
 end

app/models/content_module.rb

@@ -3,14 +3,6 @@ class ContentModule < ApplicationRecord
 
   belongs_to :program
   has_many :contents, -> { ordered }, dependent: :destroy
-  has_many :links, -> { ordered },
-    through: :contents,
-    source: :contentable,
-    source_type: "Link"
-  has_many :games,
-    through: :contents,
-    source: :contentable,
-    source_type: "Game"
   has_many :classroom_modules, dependent: :restrict_with_error
 
   enum :level, { basic: "basic", moderate: "moderate", advanced: "advanced" }, validate: true

app/models/game.rb

@@ -1,11 +1,12 @@
 class Game < ApplicationRecord
-  belongs_to :content_module, optional: true
   has_many :contents, as: :contentable, dependent: :destroy
   has_many :game_attempts, dependent: :destroy
 
   validates :title, :slug, presence: true
   validates :title, :slug, uniqueness: true
 
+  scope :not_in, ->(content_module) { where.not(id: content_module.contents.games.select(:contentable_id)) }
+
   def destroy_with_attachable?
     false
   end

app/models/link.rb

@@ -1,9 +1,7 @@
 class Link < ApplicationRecord
-  include Ordered
-
-  belongs_to :content_module
   # :delete skips Content callbacks, so Content#destroy_owned_record cannot loop back into Link#destroy
   has_one :content, as: :contentable, dependent: :delete
+  has_one :content_module, through: :content
 
   enum :link_type, { survey: "survey" }, validate: true
 

app/views/admin/links/_form.html.erb

@@ -1,4 +1,4 @@
-<%= form_with(model: link.persisted? ? [:admin, link] : [:admin, link.content_module, link]) do |form| %>
+<%= form_with(model: link.persisted? ? [:admin, link] : [:admin, content_module, link]) do |form| %>
   <div class="flex flex-col gap-2">
     <% if link.errors.any? %>
       <div style="color: red">

app/views/admin/links/edit.html.erb

@@ -11,7 +11,7 @@
   <div class="card card-border bg-base-100 shadow-md w-96">
     <div class="card-body">
       <h1 class="card-title">Edit Link</h1>
-      <%= render "form", link: @link %>
+      <%= render "form", link: @link, content_module: @link.content_module %>
     </div>
   </div>
 </section>

app/views/admin/links/new.html.erb

@@ -11,7 +11,7 @@
   <div class="card card-border bg-base-100 shadow-md w-96">
     <div class="card-body">
       <h1 class="card-title">New Link</h1>
-      <%= render "form", link: @link %>
+      <%= render "form", link: @link, content_module: @content_module %>
     </div>
   </div>
 </section>

config/routes.rb

@@ -24,7 +24,7 @@
     end
     resources :content_modules, except: [ :show ] do
       resource :position, only: :update, module: :content_modules
-      resources :links, shallow: true, except: %i[index show]
+      resources :links, shallow: true, except: %i[index show destroy]
       resources :contents, shallow: true, only: %i[create destroy] do
         resource :position, only: :update, module: :contents
       end

db/migrate/20261031181544_remove_content_module_from_links_and_games.rb

@@ -0,0 +1,27 @@
+class RemoveContentModuleFromLinksAndGames < ActiveRecord::Migration[8.1]
+  def up
+    remove_reference :links, :content_module, foreign_key: true, index: true
+    remove_column :links, :position
+    remove_reference :games, :content_module, foreign_key: true, index: true
+  end
+
+  def down
+    add_reference :links, :content_module, foreign_key: true, index: true
+    add_column :links, :position, :integer
+    add_reference :games, :content_module, foreign_key: true, index: true
+
+    execute <<~SQL
+      UPDATE links SET
+        content_module_id = (SELECT content_module_id FROM contents WHERE contentable_type = 'Link' AND contentable_id = links.id),
+        position = (SELECT position FROM contents WHERE contentable_type = 'Link' AND contentable_id = links.id)
+    SQL
+
+    # A game can be in many modules, but the old column held one, so pick the lowest content id.
+    execute <<~SQL
+      UPDATE games SET
+        content_module_id = (SELECT content_module_id FROM contents WHERE contentable_type = 'Game' AND contentable_id = games.id ORDER BY id LIMIT 1)
+    SQL
+
+    change_column_null :links, :content_module_id, false
+  end
+end

db/seeds.rb

@@ -20,9 +20,8 @@ def maybe(&block)
   programs.each do |program|
     2.times do |i|
       mod = program.content_modules.create!(name: "#{program.name} #{level.humanize} Module #{i + 1}", level: level)
-      # TODO: Update this when the delegated types migration is completed
-      link = Link.new(title: "Survey #{i + 1}", url: "https://example.com/survey/#{program.id}-#{level}-#{i}", link_type: "survey", position: 1, content_module: mod)
-      mod.contents.create! contentable: link
+      link = Link.new(title: "Survey #{i + 1}", url: "https://example.com/survey/#{program.id}-#{level}-#{i}", link_type: "survey")
+      mod.contents.create!(contentable: link)
     end
   end
 end

test/controllers/admin/links_controller_test.rb

@@ -21,8 +21,17 @@ class Admin::LinksControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to edit_admin_content_module_url(@mod)
   end
 
+  test "should create a content in the module along with the link" do
+    assert_difference -> { @mod.contents.count }, 1 do
+      post admin_content_module_links_url(@mod), params: {
+        link: { title: "New Survey", url: "https://example.com/new", link_type: "survey" }
+      }
+    end
+    assert_equal "New Survey", @mod.contents.last.contentable.title
+  end
+
   test "should not create with missing title" do
-    assert_no_difference "Link.count" do
+    assert_no_difference [ "Link.count", "Content.count" ] do
       post admin_content_module_links_url(@mod), params: {
         link: { title: "", url: "https://example.com", link_type: "survey" }
       }
@@ -40,11 +49,4 @@ class Admin::LinksControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to edit_admin_content_module_url(@mod)
     assert_equal "Updated Title", @link.reload.title
   end
-
-  test "should destroy link" do
-    assert_difference "Link.count", -1 do
-      delete admin_link_url(@link)
-    end
-    assert_redirected_to edit_admin_content_module_url(@mod)
-  end
 end

test/fixtures/games.yml

@@ -3,11 +3,9 @@
 one:
   title: Game 1
   slug: game-1
-  content_module: one
   description: MyText
 
 two:
   title: Game 2
   slug: game-2
-  content_module: one
   description: MyText

test/fixtures/links.yml

@@ -1,6 +1,4 @@
 one:
-  content_module: one
   title: Health Survey
   url: https://example.com/survey/1
   link_type: survey
-  position: 1

test/models/content_module_test.rb

@@ -28,19 +28,14 @@ class ContentModuleTest < ActiveSupport::TestCase
 
   test "destroys associated links" do
     content_module = ContentModule.create!(program: @program, level: "basic", name: "With Links")
-    link = Link.create!(content_module: content_module, title: "A Link", url: "https://example.com", link_type: "survey")
-    Content.create!(content_module: content_module, contentable: link)
+    link = Link.new(title: "A Link", url: "https://example.com", link_type: "survey")
+    content_module.contents.create!(contentable: link)
 
     assert_difference "Link.count", -1 do
       content_module.destroy!
     end
   end
 
-  test "exposes delegated contentables" do
-    assert_equal [ links(:one) ], content_modules(:one).links.to_a
-    assert_equal [ games(:one) ], content_modules(:one).games.to_a
-  end
-
   test "returns contents ordered by position" do
     contents(:one).update!(position: 2)
     contents(:two).update!(position: 1)

test/models/game_test.rb

@@ -2,19 +2,18 @@
 
 class GameTest < ActiveSupport::TestCase
   test "should not save game without title" do
-    game = Game.new(slug: "test-slug", content_module: ContentModule.new)
+    game = Game.new(slug: "test-slug")
     assert_not game.save, "Saved the game without a title"
   end
 
   test "should not save game without slug" do
-    game = Game.new(title: "Test Game", content_module: ContentModule.new)
+    game = Game.new(title: "Test Game")
     assert_not game.save, "Saved the game without a slug"
   end
 
   test "should not save game with duplicate title" do
-    content_module = ContentModule.create!(program: programs(:one), level: "basic", name: "Test Module")
-    Game.create!(title: "Test Game", slug: "test-slug", content_module: content_module)
-    duplicate_game = Game.new(title: "Test Game", slug: "another-slug", content_module: content_module)
+    Game.create!(title: "Test Game", slug: "test-slug")
+    duplicate_game = Game.new(title: "Test Game", slug: "another-slug")
     assert_not duplicate_game.save, "Saved the game with a duplicate title"
   end
 
@@ -28,10 +27,16 @@ class GameTest < ActiveSupport::TestCase
     assert_empty Content.where(contentable: games(:one))
   end
 
+  test "not_in excludes games attached to the module and includes unattached games" do
+    available = Game.not_in(content_modules(:one))
+
+    assert_not_includes available, games(:one)
+    assert_includes available, games(:two)
+  end
+
   test "should not save game with duplicate slug" do
-    content_module = ContentModule.create!(program: programs(:one), level: "basic", name: "Test Module")
-    Game.create!(title: "Test Game", slug: "test-slug", content_module: content_module)
-    duplicate_game = Game.new(title: "Another Game", slug: "test-slug", content_module: content_module)
+    Game.create!(title: "Test Game", slug: "test-slug")
+    duplicate_game = Game.new(title: "Another Game", slug: "test-slug")
     assert_not duplicate_game.save, "Saved the game with a duplicate slug"
   end
 end

test/models/link_test.rb

@@ -2,18 +2,18 @@
 
 class LinkTest < ActiveSupport::TestCase
   test "is valid with required fields" do
-    link = Link.new(content_module: content_modules(:one), title: "My Link", url: "https://example.com", link_type: "survey")
+    link = Link.new(title: "My Link", url: "https://example.com", link_type: "survey")
     assert link.valid?
   end
 
   test "is invalid without a title" do
-    link = Link.new(content_module: content_modules(:one), url: "https://example.com", link_type: "survey")
+    link = Link.new(url: "https://example.com", link_type: "survey")
     assert_not link.valid?
     assert_includes link.errors[:title], "can't be blank"
   end
 
   test "is invalid without a url" do
-    link = Link.new(content_module: content_modules(:one), title: "My Link", link_type: "survey")
+    link = Link.new(title: "My Link", link_type: "survey")
     assert_not link.valid?
     assert_includes link.errors[:url], "can't be blank"
   end
@@ -27,8 +27,12 @@ class LinkTest < ActiveSupport::TestCase
     assert_nil Content.find_by(id: content_id)
   end
 
+  test "finds its content module through its content" do
+    assert_equal content_modules(:one), links(:one).content_module
+  end
+
   test "is invalid without a link_type" do
-    link = Link.new(content_module: content_modules(:one), title: "My Link", url: "https://example.com")
+    link = Link.new(title: "My Link", url: "https://example.com")
     assert_not link.valid?
   end
 end