From c62363fca5e4f8cc8f5ed3ff20b12998520c8955 Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Fri, 1 Oct 2021 16:22:07 +0100 Subject: [PATCH 1/4] Update site updates when exercises change and add wip guard --- app/commands/concept_exercise/create.rb | 5 +-- app/commands/practice_exercise/create.rb | 5 +-- .../process_new_exercise_update.rb | 40 +++++++++++++++++++ app/models/concerns/is_paramaterised_sti.rb | 4 ++ ...index_to_uniqueness_key_on_site_updates.rb | 5 +++ db/schema.rb | 3 +- test/commands/concept_exercise/create_test.rb | 11 ++--- .../commands/practice_exercise/create_test.rb | 11 ++--- .../process_new_exercise_update_test.rb | 40 +++++++++++++++++++ 9 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 app/commands/site_updates/process_new_exercise_update.rb create mode 100644 db/migrate/20211001151540_add_index_to_uniqueness_key_on_site_updates.rb create mode 100644 test/commands/site_updates/process_new_exercise_update_test.rb diff --git a/app/commands/concept_exercise/create.rb b/app/commands/concept_exercise/create.rb index ea359ec7c6..b0bb1fd3e9 100644 --- a/app/commands/concept_exercise/create.rb +++ b/app/commands/concept_exercise/create.rb @@ -10,10 +10,7 @@ def call track: track, **attributes ).tap do |exercise| - SiteUpdates::NewExerciseUpdate.create!( - exercise: exercise, - track: track - ) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) end rescue ActiveRecord::RecordNotUnique ConceptExercise.find_by!(uuid: uuid, track: track) diff --git a/app/commands/practice_exercise/create.rb b/app/commands/practice_exercise/create.rb index 73fdc92605..ec39283275 100644 --- a/app/commands/practice_exercise/create.rb +++ b/app/commands/practice_exercise/create.rb @@ -10,10 +10,7 @@ def call track: track, **attributes ).tap do |exercise| - SiteUpdates::NewExerciseUpdate.create!( - exercise: exercise, - track: track - ) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) end rescue ActiveRecord::RecordNotUnique PracticeExercise.find_by!(uuid: uuid, track: track) diff --git a/app/commands/site_updates/process_new_exercise_update.rb b/app/commands/site_updates/process_new_exercise_update.rb new file mode 100644 index 0000000000..1d2f6a71fc --- /dev/null +++ b/app/commands/site_updates/process_new_exercise_update.rb @@ -0,0 +1,40 @@ +module SiteUpdates + class ProcessNewExerciseUpdate + include Mandate + + initialize_with :exercise + + def call + if exercise.wip? + destroy! + else + begin + create! + rescue ActiveRecord::RecordNotUnique + update! + end + end + end + + def destroy! + SiteUpdates::NewExerciseUpdate.where( + exercise: exercise, + track: exercise.track + ).destroy_all + end + + def create! + SiteUpdates::NewExerciseUpdate.create!( + exercise: exercise, + track: exercise.track + ) + end + + def update! + SiteUpdates::NewExerciseUpdate.find_by!( + exercise: exercise, + track: exercise.track + ).regenerate_rendering_data! + end + end +end diff --git a/app/models/concerns/is_paramaterised_sti.rb b/app/models/concerns/is_paramaterised_sti.rb index f9f6ad5d08..e1ad363288 100644 --- a/app/models/concerns/is_paramaterised_sti.rb +++ b/app/models/concerns/is_paramaterised_sti.rb @@ -108,6 +108,10 @@ def params(*keys) end end + def regenerate_rendering_data! + update!(rendering_data_cache: {}) + end + def rendering_data data = rendering_data_cache if data.blank? diff --git a/db/migrate/20211001151540_add_index_to_uniqueness_key_on_site_updates.rb b/db/migrate/20211001151540_add_index_to_uniqueness_key_on_site_updates.rb new file mode 100644 index 0000000000..498711d2fb --- /dev/null +++ b/db/migrate/20211001151540_add_index_to_uniqueness_key_on_site_updates.rb @@ -0,0 +1,5 @@ +class AddIndexToUniquenessKeyOnSiteUpdates < ActiveRecord::Migration[6.1] + def change + add_index :site_updates, :uniqueness_key, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index f91c04de13..24814c66f6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_09_25_111900) do +ActiveRecord::Schema.define(version: 2021_10_01_151540) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "name", null: false @@ -449,6 +449,7 @@ t.index ["exercise_id"], name: "index_site_updates_on_exercise_id" t.index ["pull_request_id"], name: "index_site_updates_on_pull_request_id" t.index ["track_id"], name: "index_site_updates_on_track_id" + t.index ["uniqueness_key"], name: "index_site_updates_on_uniqueness_key", unique: true end create_table "solution_comments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| diff --git a/test/commands/concept_exercise/create_test.rb b/test/commands/concept_exercise/create_test.rb index abbba5c141..6b6a1e8a89 100644 --- a/test/commands/concept_exercise/create_test.rb +++ b/test/commands/concept_exercise/create_test.rb @@ -66,15 +66,12 @@ class ConceptExercise::CreateTest < ActiveSupport::TestCase end test "creates site_update" do - track = create :track - exercise = ConceptExercise::Create.( + SiteUpdates::ProcessNewExerciseUpdate.expects(:call) + + ConceptExercise::Create.( SecureRandom.uuid, - track, + create(:track), build(:concept_exercise).attributes.symbolize_keys ) - - update = SiteUpdate.first - assert_equal exercise, update.exercise - assert_equal track, update.track end end diff --git a/test/commands/practice_exercise/create_test.rb b/test/commands/practice_exercise/create_test.rb index 1f5195525b..5ee47fec88 100644 --- a/test/commands/practice_exercise/create_test.rb +++ b/test/commands/practice_exercise/create_test.rb @@ -61,15 +61,12 @@ class PracticeExercise::CreateTest < ActiveSupport::TestCase end test "creates site_update" do - track = create :track - exercise = PracticeExercise::Create.( + SiteUpdates::ProcessNewExerciseUpdate.expects(:call) + + PracticeExercise::Create.( SecureRandom.uuid, - track, + create(:track), build(:practice_exercise).attributes.symbolize_keys ) - - update = SiteUpdate.first - assert_equal exercise, update.exercise - assert_equal track, update.track end end diff --git a/test/commands/site_updates/process_new_exercise_update_test.rb b/test/commands/site_updates/process_new_exercise_update_test.rb new file mode 100644 index 0000000000..87f598b971 --- /dev/null +++ b/test/commands/site_updates/process_new_exercise_update_test.rb @@ -0,0 +1,40 @@ +require "test_helper" + +class ConceptExercise::CreateTest < ActiveSupport::TestCase + test "no-op if wip and not present" do + exercise = create :concept_exercise, status: :wip + + SiteUpdates::ProcessNewExerciseUpdate.(exercise) + + refute SiteUpdate.exists? + end + + test "deletes if wip and was present" do + exercise = create :concept_exercise, status: :wip + + create :new_exercise_site_update, exercise: exercise + + SiteUpdates::ProcessNewExerciseUpdate.(exercise) + + refute SiteUpdate.exists? + end + + test "creates if not wip" do + exercise = create :concept_exercise + + SiteUpdates::ProcessNewExerciseUpdate.(exercise) + + assert SiteUpdate.where(exercise: exercise).exists? + end + + test "updates if not wip and exists" do + exercise = create :concept_exercise + update = create :new_exercise_site_update, exercise: exercise + update.update_column(:rendering_data_cache, { "foo" => "bar" }) + assert_equal({ "foo" => "bar" }, update.rendering_data_cache) # Sanity + + SiteUpdates::ProcessNewExerciseUpdate.(exercise) + + assert_equal JSON.parse(update.cacheable_rendering_data.to_json), update.reload.rendering_data_cache + end +end From 3e36c118c2ac6cadbc1b9af7d5d967ecfbe102a6 Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Fri, 1 Oct 2021 16:28:18 +0100 Subject: [PATCH 2/4] Call out during sync --- app/commands/git/sync_concept_exercise.rb | 1 + app/commands/git/sync_practice_exercise.rb | 1 + test/commands/git/sync_concept_exercise_test.rb | 7 +++++++ test/commands/git/sync_practice_exercise_test.rb | 7 +++++++ 4 files changed, 16 insertions(+) diff --git a/app/commands/git/sync_concept_exercise.rb b/app/commands/git/sync_concept_exercise.rb index 74fa8c5d80..08fd001567 100644 --- a/app/commands/git/sync_concept_exercise.rb +++ b/app/commands/git/sync_concept_exercise.rb @@ -27,6 +27,7 @@ def call SyncExerciseAuthors.(exercise) SyncExerciseContributors.(exercise) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) end private diff --git a/app/commands/git/sync_practice_exercise.rb b/app/commands/git/sync_practice_exercise.rb index 7fbfcca802..e8f4c30892 100644 --- a/app/commands/git/sync_practice_exercise.rb +++ b/app/commands/git/sync_practice_exercise.rb @@ -28,6 +28,7 @@ def call SyncExerciseAuthors.(exercise) SyncExerciseContributors.(exercise) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) end private diff --git a/test/commands/git/sync_concept_exercise_test.rb b/test/commands/git/sync_concept_exercise_test.rb index f9a9b9b85f..2db9f59e8c 100644 --- a/test/commands/git/sync_concept_exercise_test.rb +++ b/test/commands/git/sync_concept_exercise_test.rb @@ -352,4 +352,11 @@ class Git::SyncConceptExerciseTest < ActiveSupport::TestCase assert_equal 'log-levels', exercise.slug end + + test "updates site_update" do + exercise = create :concept_exercise, uuid: 'f4f7de13-a9ee-4251-8796-006ed85b3f70', slug: 'logs', git_sha: "c75486b75db8012646b0e1c667cb1db47ff5a9d5", synced_to_git_sha: "c75486b75db8012646b0e1c667cb1db47ff5a9d5" # rubocop:disable Layout/LineLength + SiteUpdates::ProcessNewExerciseUpdate.expects(:call).with(exercise) + + Git::SyncConceptExercise.(exercise, force_sync: true) + end end diff --git a/test/commands/git/sync_practice_exercise_test.rb b/test/commands/git/sync_practice_exercise_test.rb index 39e7087a37..0c12b5372c 100644 --- a/test/commands/git/sync_practice_exercise_test.rb +++ b/test/commands/git/sync_practice_exercise_test.rb @@ -333,4 +333,11 @@ class Git::SyncPracticeExerciseTest < ActiveSupport::TestCase assert exercise.reload.has_test_runner? end + + test "updates site_update" do + exercise = create :practice_exercise, uuid: '185b964c-1ec1-4d60-b9b9-fa20b9f57b4a' + SiteUpdates::ProcessNewExerciseUpdate.expects(:call).with(exercise) + + Git::SyncPracticeExercise.(exercise, force_sync: true) + end end From c0eaa17857066be9f64bbe9db263bceb2cce76e6 Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Tue, 5 Oct 2021 13:26:39 +0100 Subject: [PATCH 3/4] Add beta and deprecated checks --- .../process_new_exercise_update.rb | 2 +- .../process_new_exercise_update_test.rb | 50 ++++++++++--------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/app/commands/site_updates/process_new_exercise_update.rb b/app/commands/site_updates/process_new_exercise_update.rb index 1d2f6a71fc..b595156785 100644 --- a/app/commands/site_updates/process_new_exercise_update.rb +++ b/app/commands/site_updates/process_new_exercise_update.rb @@ -5,7 +5,7 @@ class ProcessNewExerciseUpdate initialize_with :exercise def call - if exercise.wip? + if exercise.wip? || exercise.deprecated? destroy! else begin diff --git a/test/commands/site_updates/process_new_exercise_update_test.rb b/test/commands/site_updates/process_new_exercise_update_test.rb index 87f598b971..ccbe911eda 100644 --- a/test/commands/site_updates/process_new_exercise_update_test.rb +++ b/test/commands/site_updates/process_new_exercise_update_test.rb @@ -1,40 +1,44 @@ require "test_helper" class ConceptExercise::CreateTest < ActiveSupport::TestCase - test "no-op if wip and not present" do - exercise = create :concept_exercise, status: :wip + %i[active beta].each do |status| + test "creates if #{status}" do + exercise = create :concept_exercise, status: status - SiteUpdates::ProcessNewExerciseUpdate.(exercise) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) - refute SiteUpdate.exists? - end - - test "deletes if wip and was present" do - exercise = create :concept_exercise, status: :wip + assert SiteUpdate.where(exercise: exercise).exists? + end - create :new_exercise_site_update, exercise: exercise + test "updates if #{status} and exists" do + exercise = create :concept_exercise, status: status + update = create :new_exercise_site_update, exercise: exercise + update.update_column(:rendering_data_cache, { "foo" => "bar" }) + assert_equal({ "foo" => "bar" }, update.rendering_data_cache) # Sanity - SiteUpdates::ProcessNewExerciseUpdate.(exercise) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) - refute SiteUpdate.exists? + assert_equal JSON.parse(update.cacheable_rendering_data.to_json), update.reload.rendering_data_cache + end end - test "creates if not wip" do - exercise = create :concept_exercise + %i[wip deprecated].each do |status| + test "does not create if #{status}" do + exercise = create :concept_exercise, status: status - SiteUpdates::ProcessNewExerciseUpdate.(exercise) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) - assert SiteUpdate.where(exercise: exercise).exists? - end + refute SiteUpdate.exists? + end + + test "deletes if #{status} and was present" do + exercise = create :concept_exercise, status: status - test "updates if not wip and exists" do - exercise = create :concept_exercise - update = create :new_exercise_site_update, exercise: exercise - update.update_column(:rendering_data_cache, { "foo" => "bar" }) - assert_equal({ "foo" => "bar" }, update.rendering_data_cache) # Sanity + create :new_exercise_site_update, exercise: exercise - SiteUpdates::ProcessNewExerciseUpdate.(exercise) + SiteUpdates::ProcessNewExerciseUpdate.(exercise) - assert_equal JSON.parse(update.cacheable_rendering_data.to_json), update.reload.rendering_data_cache + refute SiteUpdate.exists? + end end end From 621b071d66d80fbab6be0c9f56daccde2f974cb1 Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Tue, 5 Oct 2021 13:32:27 +0100 Subject: [PATCH 4/4] Fix test class name --- test/commands/site_updates/process_new_exercise_update_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/commands/site_updates/process_new_exercise_update_test.rb b/test/commands/site_updates/process_new_exercise_update_test.rb index ccbe911eda..cded472ddc 100644 --- a/test/commands/site_updates/process_new_exercise_update_test.rb +++ b/test/commands/site_updates/process_new_exercise_update_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class ConceptExercise::CreateTest < ActiveSupport::TestCase +class SiteUpdates::ProcessNewExerciseUpdateTest < ActiveSupport::TestCase %i[active beta].each do |status| test "creates if #{status}" do exercise = create :concept_exercise, status: status