feat: give contents ownership of module membership and order

app/controllers/admin/content_modules_controller.rb

@@ -49,6 +49,6 @@ def set_content_module
     end
 
     def content_module_params
-      params.expect(content_module: [ :name, :program_id, :level, :position ])
+      params.expect(content_module: [ :name, :program_id, :level ])
     end
 end

app/controllers/admin/games_controller.rb

@@ -6,8 +6,7 @@ def index
   end
 
   def new
-    @game = Game.new(content_module_id: params[:content_module_id])
-    @content_module = @game.content_module
+    @game = Game.new
   end
 
   def edit
@@ -18,7 +17,6 @@ def create
 
     respond_to do |format|
       if @game.save
-        Content.create!(content_module: @game.content_module, contentable: @game) if @game.content_module
         format.html { redirect_back(fallback_location: admin_games_path, notice: "Game was successfully created.") }
       else
         format.html { render :new, status: :unprocessable_content }
@@ -53,6 +51,6 @@ def set_game
     end
 
     def game_params
-      params.expect(game: [ :title, :slug, :content_module_id, :description ])
+      params.expect(game: [ :title, :slug, :description ])
     end
 end

app/controllers/admin/links_controller.rb

@@ -3,14 +3,15 @@ class Admin::LinksController < AdminController
   before_action :set_link, only: %i[edit update destroy]
 
   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)
+
+    @link.build_content(content_module: @content_module)
 
     if @link.save
-      @content_module.contents.create!(contentable: @link)
       redirect_to edit_admin_content_module_path(@content_module), notice: "Link was successfully created."
     else
       render :new, status: :unprocessable_entity
@@ -29,8 +30,9 @@ def update
   end
 
   def destroy
+    content_module = @link.content_module
     @link.destroy!
-    redirect_to edit_admin_content_module_path(@link.content_module), notice: "Link was successfully deleted.", status: :see_other
+    redirect_to edit_admin_content_module_path(content_module), notice: "Link was successfully deleted.", status: :see_other
   end
 
   private
@@ -43,6 +45,6 @@ def set_link
     end
 
     def link_params
-      params.expect(link: [ :title, :url, :link_type, :position ])
+      params.expect(link: [ :title, :url, :link_type ])
     end
 end

app/models/concerns/contentable.rb

@@ -1,7 +1,10 @@
 module Contentable
   extend ActiveSupport::Concern
 
-  included do
-    has_one :content, as: :contentable
+  # Whether the content record owns this contentable, so the contentable is destroyed together with it.
+  #
+  # @return [Boolean]
+  def owned_by_content?
+    false
   end
 end

app/models/concerns/ordered.rb

@@ -1,7 +1,24 @@
+# Orders records by an integer +position+ column. New records go to the end of their siblings.
 module Ordered
   extend ActiveSupport::Concern
 
   included do
     scope :ordered, -> { order(:position) }
+
+    before_create :append_to_siblings
+  end
+
+  private
+
+  # This method is required to override. It is the relation of records this record is
+  # ordered among (for example, the contents of one module).
+  #
+  # @return [ActiveRecord::Relation]
+  def position_siblings
+    raise NotImplementedError, "#{self.class} must define #position_siblings to be ordered"
+  end
+
+  def append_to_siblings
+    self.position = (position_siblings.maximum(:position) || 0) + 1
   end
 end

app/models/content.rb

@@ -1,15 +1,26 @@
 class Content < ApplicationRecord
+  include Ordered
+
   belongs_to :content_module
 
   delegated_type :contentable, types: %w[Link Game]
 
+  validates :contentable_id, uniqueness: {
+    scope: [ :content_module_id, :contentable_type ],
+    message: "is already in this module"
+  }
+
   # Ensure the associated contentable record is destroyed if allowed before destroying this content.
   # This allows us to have dependent: :destroy behavior for contentable records.
   before_destroy :destroy_owned_record
 
   private
 
+  def position_siblings
+    content_module.contents
+  end
+
   def destroy_owned_record
-    contentable.destroy if contentable.destroy_with_attachable?
+    contentable.destroy if contentable.owned_by_content?
   end
 end

app/models/content_module.rb

@@ -2,18 +2,23 @@ class ContentModule < ApplicationRecord
   include Ordered
 
   belongs_to :program
-  has_many :contents, 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 :contents, -> { ordered }, dependent: :destroy
+  has_many :games, through: :contents, source: :contentable, source_type: "Game"
+  has_many :links, through: :contents, source: :contentable, source_type: "Link"
   has_many :classroom_modules, dependent: :restrict_with_error
 
   enum :level, { basic: "basic", moderate: "moderate", advanced: "advanced" }, validate: true
 
   validates :name, :level, presence: true
+
+  # A module that joins another program or level goes to the end of it.
+  # TODO: Consider if we just don't allow these types of edits to avoid this hidden callback behavior
+  before_update :append_to_siblings, if: -> { program_id_changed? || level_changed? }
+
+  private
+
+  # Modules are ordered within one program and level, the same grouping the admin index shows.
+  def position_siblings
+    program.content_modules.where(level: level)
+  end
 end

app/models/game.rb

@@ -1,15 +1,12 @@
 class Game < ApplicationRecord
   include Contentable
 
-  belongs_to :content_module, optional: true
-  has_many :game_attempts, dependent: :destroy
-
+  has_many :contents, as: :contentable, dependent: :destroy
+  has_many :content_modules, through: :contents
   has_many :game_attempts, dependent: :destroy
 
   validates :title, :slug, presence: true
   validates :title, :slug, uniqueness: true
 
-  def destroy_with_attachable?
-    false
-  end
+  scope :available_for, ->(content_module) { where.not(id: content_module.games).order(:title) }
 end

app/models/link.rb

@@ -1,15 +1,16 @@
 class Link < ApplicationRecord
-  include Ordered
   include Contentable
 
-  belongs_to :content_module
+  # :delete skips Content callbacks to avoid a destroy loop
+  has_one :content, as: :contentable, dependent: :delete
+  has_one :content_module, through: :content
 
   enum :link_type, { survey: "survey" }, validate: true
 
   validates :title, :url, :link_type, presence: true
   validates :url, format: { with: /\Ahttps?:\/\/.+\z/i, message: "must start with http:// or https://" }, allow_blank: true
 
-  def destroy_with_attachable?
+  def owned_by_content?
     true
   end
 end

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

@@ -22,9 +22,6 @@
     <%= form.select :level, ContentModule.levels.keys.map { |l| [ l.humanize, l ] },
           { include_blank: "Select a level" }, class: "select w-full" %>
 
-    <%= form.label :position, class: "label" %>
-    <%= form.number_field :position, class: "input w-full" %>
-
     <%= form.submit class: "btn btn-primary" %>
   </div>
 <% end %>

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

@@ -57,7 +57,6 @@
       <div class="card-body">
         <div class="flex justify-between items-center mb-2">
           <h2 class="card-title text-lg">Games</h2>
-          <%= link_to "Add game", new_admin_game_path(content_module_id: @content_module.id), class: "btn btn-sm btn-primary" %>
         </div>
         <% if @content_module.games.any? %>
           <table class="table">

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

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

app/views/admin/games/_game.html.erb

@@ -7,10 +7,6 @@
     <strong class="block font-medium mb-1">Slug:</strong>
     <%= game.slug %>
   </div>
-  <div>
-    <strong class="block font-medium mb-1">Content module:</strong>
-    <%= game.content_module_id %>
-  </div>
   <div>
     <strong class="block font-medium mb-1">Description:</strong>
     <%= game.description %>

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

@@ -3,9 +3,6 @@
 <nav class="breadcrumbs text-sm">
   <ul>
     <li><%= link_to "Content Modules", admin_content_modules_path %></li>
-    <% if @content_module %>
-      <li><%= link_to @content_module.name, edit_admin_content_module_path(@content_module) %></li>
-    <% end %>
     <li>New Game</li>
   </ul>
 </nav>

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">
@@ -21,9 +21,6 @@
     <%= form.select :link_type, Link.link_types.keys.map { |t| [ t.humanize, t ] },
           { include_blank: "Select a type" }, class: "select w-full" %>
 
-    <%= form.label :position, class: "label" %>
-    <%= form.number_field :position, class: "input w-full" %>
-
     <%= form.submit class: "btn btn-primary" %>
   </div>
 <% end %>

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>

db/migrate/20261031181034_add_position_to_contents.rb

@@ -0,0 +1,7 @@
+class AddPositionToContents < ActiveRecord::Migration[8.1]
+  def change
+    add_column :contents, :position, :integer
+    add_index :contents, [ :content_module_id, :position ]
+    add_index :contents, [ :content_module_id, :contentable_type, :contentable_id ], unique: true, name: "index_contents_on_module_and_contentable"
+  end
+end

db/migrate/20261031181544_remove_content_module_from_links_and_games.rb

@@ -0,0 +1,11 @@
+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
+    raise ActiveRecord::IrreversibleMigration
+  end
+end