Store approach and article order (#6690)

* Add position to approaches and articles

* Add scope for sorted articles and approaches

* Fix factories

* Sync article and approach order as found in config

* Dig deeper page shows articles and approaches in order

* Make position ordering default scope

app/commands/git/sync_exercise_approach.rb

@@ -1,7 +1,7 @@
 class Git::SyncExerciseApproach
   include Mandate
 
-  initialize_with :exercise, :config
+  initialize_with :exercise, :config, :position
 
   def call
     find_or_create!.tap do |approach|
@@ -20,7 +20,10 @@ def find_or_create!
     end
   end
 
-  def attributes_for_create = config.slice(:slug, :title, :blurb).merge({ exercise:, synced_to_git_sha: exercise.git.head_sha })
+  def attributes_for_create
+    config.slice(:slug, :title, :blurb).
+      merge({ exercise:, position:, synced_to_git_sha: exercise.git.head_sha })
+  end
 
   def attributes_for_update(approach)
     attributes_for_create.merge({

app/commands/git/sync_exercise_approaches.rb

@@ -15,7 +15,11 @@ def call
   private
   attr_reader :exercise
 
-  def approaches = approaches_config.map { |approach| Git::SyncExerciseApproach.(exercise, approach) }
+  def approaches
+    approaches_config.map.with_index do |approach, index|
+      Git::SyncExerciseApproach.(exercise, approach, index + 1)
+    end
+  end
 
   def introduction_config = head_git_approaches.config_introduction
 

app/commands/git/sync_exercise_article.rb

@@ -1,7 +1,7 @@
 class Git::SyncExerciseArticle
   include Mandate
 
-  initialize_with :exercise, :config
+  initialize_with :exercise, :config, :position
 
   def call
     find_or_create!.tap do |article|
@@ -16,7 +16,10 @@ def find_or_create!
     end
   end
 
-  def attributes_for_create = config.slice(:slug, :title, :blurb).merge({ exercise:, synced_to_git_sha: exercise.git.head_sha })
+  def attributes_for_create
+    config.slice(:slug, :title, :blurb).
+      merge({ exercise:, position:, synced_to_git_sha: exercise.git.head_sha })
+  end
 
   def attributes_for_update(article)
     attributes_for_create.merge({

app/commands/git/sync_exercise_articles.rb

@@ -6,6 +6,11 @@ class Git::SyncExerciseArticles
   def call = exercise.update(articles:)
 
   private
-  def articles = git_articles.articles.map { |article| Git::SyncExerciseArticle.(exercise, article) }
+  def articles
+    git_articles.articles.map.with_index do |article, index|
+      Git::SyncExerciseArticle.(exercise, article, index + 1)
+    end
+  end
+
   def git_articles = Git::Exercise::Articles.new(exercise.slug, exercise.git_type, "HEAD", repo_url: exercise.track.repo_url)
 end

app/controllers/tracks/dig_deeper_controller.rb

@@ -7,8 +7,8 @@ class Tracks::DigDeeperController < ApplicationController
 
   def show
     @videos = @exercise.community_videos.approved
-    @approaches = @exercise.approaches.random
-    @articles = @exercise.articles.random
+    @approaches = @exercise.approaches
+    @articles = @exercise.articles
     @introduction = introduction
     @links = links
   end

app/models/exercise/approach.rb

@@ -27,6 +27,7 @@ class Exercise::Approach < ApplicationRecord
   has_one :track, through: :exercise
 
   scope :random, -> { order('RAND()') }
+  default_scope { order(:position) }
 
   delegate :content, :snippet, to: :git
 

app/models/exercise/article.rb

@@ -23,6 +23,7 @@ class Exercise::Article < ApplicationRecord
   has_one :track, through: :exercise
 
   scope :random, -> { order('RAND()') }
+  default_scope -> { order(:position) }
 
   delegate :content, :snippet, to: :git
 

db/migrate/20240118102648_add_position_to_exercise_approaches.rb

@@ -0,0 +1,13 @@
+class AddPositionToExerciseApproaches < ActiveRecord::Migration[7.0]
+  def change
+    return if Rails.env.production?
+
+    add_column :exercise_approaches, :position, :integer, null: true, limit: 1
+    add_index :exercise_approaches, [:exercise_id, :position]
+
+    # The positions will be fixed manually by running Git::SyncTrack.()
+    Exercise::Approach.update_all(position: 0)
+
+    change_column_null :exercise_approaches, :position, false
+  end
+end

db/migrate/20240118102652_add_position_to_exercise_articles.rb

@@ -0,0 +1,13 @@
+class AddPositionToExerciseArticles < ActiveRecord::Migration[7.0]
+  def change
+    return if Rails.env.production?
+
+    add_column :exercise_articles, :position, :integer, null: true, limit: 1
+    add_index :exercise_articles, [:exercise_id, :position]
+
+    # The positions will be fixed manually by running Git::SyncTrack.()
+    Exercise::Article.update_all(position: 0)
+
+    change_column_null :exercise_articles, :position, false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_12_01_091435) do
+ActiveRecord::Schema[7.0].define(version: 2024_01_18_102652) do
   create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
     t.string "name", null: false
     t.string "record_type", null: false
@@ -239,6 +239,8 @@
     t.datetime "updated_at", null: false
     t.json "tags"
     t.integer "num_solutions", default: 0, null: false
+    t.integer "position", limit: 1, null: false
+    t.index ["exercise_id", "position"], name: "index_exercise_approaches_on_exercise_id_and_position"
     t.index ["exercise_id", "uuid"], name: "index_exercise_approaches_on_exercise_id_and_uuid", unique: true
     t.index ["exercise_id"], name: "index_exercise_approaches_on_exercise_id"
     t.index ["uuid"], name: "index_exercise_approaches_on_uuid"
@@ -273,6 +275,8 @@
     t.string "synced_to_git_sha", null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.integer "position", limit: 1, null: false
+    t.index ["exercise_id", "position"], name: "index_exercise_articles_on_exercise_id_and_position"
     t.index ["exercise_id", "uuid"], name: "index_exercise_articles_on_exercise_id_and_uuid", unique: true
     t.index ["exercise_id"], name: "index_exercise_articles_on_exercise_id"
     t.index ["uuid"], name: "index_exercise_articles_on_uuid"

test/commands/git/sync_exercise_approach_test.rb

@@ -5,13 +5,14 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
     exercise = create :practice_exercise
     config = { uuid: SecureRandom.uuid, slug: "performance", title: "Performance", blurb: "Speed up!" }
 
-    approach = Git::SyncExerciseApproach.(exercise, config)
+    approach = Git::SyncExerciseApproach.(exercise, config, 1)
 
     assert_equal exercise, approach.exercise
     assert_equal config[:slug], approach.slug
     assert_equal config[:title], approach.title
     assert_equal config[:blurb], approach.blurb
     assert_equal approach.git.head_sha, approach.synced_to_git_sha
+    assert_equal 1, approach.position
     assert_nil approach.tags
   end
 
@@ -29,7 +30,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
       contributors: [contributor.github_username]
     }
 
-    approach = Git::SyncExerciseApproach.(exercise, config)
+    approach = Git::SyncExerciseApproach.(exercise, config, 1)
 
     assert_equal [author_1, author_2], approach.authors
     assert_equal [contributor], approach.contributors
@@ -40,13 +41,14 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
     approach = create(:exercise_approach, exercise:)
     config = { uuid: approach.uuid, slug: "new slug", title: "new title", blurb: "new blurb" }
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 2)
 
     approach.reload
     assert_equal exercise, approach.exercise
     assert_equal config[:slug], approach.slug
     assert_equal config[:title], approach.title
     assert_equal config[:blurb], approach.blurb
+    assert_equal 2, approach.position
     assert_equal approach.git.head_sha, approach.synced_to_git_sha
   end
 
@@ -63,7 +65,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
       contributors: [contributor_2.github_username]
     })
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
 
     approach.reload
     assert_equal [author_1, author_2], approach.authors
@@ -76,7 +78,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
     approach = create :exercise_approach, exercise:, updated_at:, synced_to_git_sha: exercise.git.head_sha
     config = approach.slice(:uuid, :slug, :title, :blurb)
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
 
     approach.reload
     assert_equal updated_at, approach.updated_at
@@ -97,7 +99,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
       tags:
     }
 
-    approach = Git::SyncExerciseApproach.(exercise, config)
+    approach = Git::SyncExerciseApproach.(exercise, config, 1)
 
     assert_equal tags, approach.tags
   end
@@ -125,7 +127,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
       tags: new_tags
     }
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
 
     assert_equal new_tags, approach.reload.tags
   end
@@ -144,7 +146,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
 
     Exercise::Approach::LinkMatchingSubmissions.expects(:call).once
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
   end
 
   test "don't link submissions when creating approach without tags" do
@@ -153,7 +155,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
 
     Exercise::Approach::LinkMatchingSubmissions.expects(:call).never
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
   end
 
   test "link submissions when tags are updated from config" do
@@ -166,7 +168,7 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
 
     Exercise::Approach::LinkMatchingSubmissions.expects(:call).with(approach)
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
   end
 
   test "don't link submissions when tags haven't changed" do
@@ -178,6 +180,6 @@ class Git::SyncExerciseApproachTest < ActiveSupport::TestCase
 
     Exercise::Approach::LinkMatchingSubmissions.expects(:call).with(approach).never
 
-    Git::SyncExerciseApproach.(exercise, config)
+    Git::SyncExerciseApproach.(exercise, config, 1)
   end
 end

test/commands/git/sync_exercise_article_test.rb

@@ -5,13 +5,14 @@ class Git::SyncExerciseArticleTest < ActiveSupport::TestCase
     exercise = create :practice_exercise
     config = { uuid: SecureRandom.uuid, slug: "performance", title: "Performance", blurb: "Speed up!" }
 
-    article = Git::SyncExerciseArticle.(exercise, config)
+    article = Git::SyncExerciseArticle.(exercise, config, 1)
 
     assert_equal exercise, article.exercise
     assert_equal config[:slug], article.slug
     assert_equal config[:title], article.title
     assert_equal config[:blurb], article.blurb
     assert_equal article.git.head_sha, article.synced_to_git_sha
+    assert_equal 1, article.position
   end
 
   test "creates authors and contributors from config" do
@@ -28,7 +29,7 @@ class Git::SyncExerciseArticleTest < ActiveSupport::TestCase
       contributors: [contributor.github_username]
     }
 
-    article = Git::SyncExerciseArticle.(exercise, config)
+    article = Git::SyncExerciseArticle.(exercise, config, 1)
 
     assert_equal [author_1, author_2], article.authors
     assert_equal [contributor], article.contributors
@@ -39,13 +40,14 @@ class Git::SyncExerciseArticleTest < ActiveSupport::TestCase
     article = create(:exercise_article, exercise:)
     config = { uuid: article.uuid, slug: "new slug", title: "new title", blurb: "new blurb" }
 
-    Git::SyncExerciseArticle.(exercise, config)
+    Git::SyncExerciseArticle.(exercise, config, 2)
 
     article.reload
     assert_equal exercise, article.exercise
     assert_equal config[:slug], article.slug
     assert_equal config[:title], article.title
     assert_equal config[:blurb], article.blurb
+    assert_equal 2, article.position
     assert_equal article.git.head_sha, article.synced_to_git_sha
   end
 
@@ -62,7 +64,7 @@ class Git::SyncExerciseArticleTest < ActiveSupport::TestCase
       contributors: [contributor_2.github_username]
     })
 
-    Git::SyncExerciseArticle.(exercise, config)
+    Git::SyncExerciseArticle.(exercise, config, 1)
 
     article.reload
     assert_equal [author_1, author_2], article.authors
@@ -75,7 +77,7 @@ class Git::SyncExerciseArticleTest < ActiveSupport::TestCase
     article = create :exercise_article, exercise:, updated_at:, synced_to_git_sha: exercise.git.head_sha
     config = article.slice(:uuid, :slug, :title, :blurb)
 
-    Git::SyncExerciseArticle.(exercise, config)
+    Git::SyncExerciseArticle.(exercise, config, 1)
 
     article.reload
     assert_equal updated_at, article.updated_at

test/factories/exercise/approaches.rb

@@ -9,6 +9,7 @@
     blurb { "Learn all about #{slug}" }
     title { slug.to_s.titleize }
     synced_to_git_sha { "HEAD" }
+    position { 1 }
 
     trait :random do
       slug { SecureRandom.hex }

test/factories/exercise/articles.rb

@@ -6,6 +6,7 @@
     blurb { "Learn all about #{slug}" }
     title { slug.to_s.titleize }
     synced_to_git_sha { "HEAD" }
+    position { 1 }
 
     trait :random do
       slug { SecureRandom.hex }