FIX: move something else flag to the bottom (#27366)

The mistake was made when flags were moved to the database. The `notify_moderators` (something else) flag should be the last position on the list.

This commit contains 3 changes:
- update fixtures order;
- remove position and enable from fixtures (they can be overridden by admin and we don't want seed to restore them);
- migration to fix data if the order was not changed by admin.

db/fixtures/003_flags.rb

@@ -1,9 +1,16 @@
 # frozen_string_literal: true
 
+Flag.seed do |s|
+  s.id = 6
+  s.name = "notify_user"
+  s.notify_type = false
+  s.auto_action_type = false
+  s.custom_type = true
+  s.applies_to = %w[Post Chat::Message]
+end
 Flag.seed do |s|
   s.id = 3
   s.name = "off_topic"
-  s.position = 2
   s.notify_type = true
   s.auto_action_type = true
   s.custom_type = false
@@ -12,7 +19,6 @@ end
 Flag.seed do |s|
   s.id = 4
   s.name = "inappropriate"
-  s.position = 3
   s.notify_type = true
   s.auto_action_type = true
   s.custom_type = false
@@ -21,34 +27,22 @@ end
 Flag.seed do |s|
   s.id = 8
   s.name = "spam"
-  s.position = 4
   s.notify_type = true
   s.auto_action_type = true
   s.custom_type = false
   s.applies_to = %w[Post Topic Chat::Message]
 end
 Flag.seed do |s|
-  s.id = 6
+  s.id = 10
-  s.name = "notify_user"
+  s.name = "illegal"
-  s.position = 0
-  s.notify_type = false
-  s.auto_action_type = false
-  s.custom_type = true
-  s.applies_to = %w[Post Chat::Message]
-end
-Flag.seed do |s|
-  s.id = 7
-  s.name = "notify_moderators"
-  s.position = 1
   s.notify_type = true
   s.auto_action_type = false
   s.custom_type = true
   s.applies_to = %w[Post Topic Chat::Message]
 end
 Flag.seed do |s|
-  s.id = 10
+  s.id = 7
-  s.name = "illegal"
+  s.name = "notify_moderators"
-  s.position = 5
   s.notify_type = true
   s.auto_action_type = false
   s.custom_type = true
@@ -57,7 +51,6 @@ end
 Flag.unscoped.seed do |s|
   s.id = 9
   s.name = "needs_approval"
-  s.position = 6
   s.notify_type = false
   s.auto_action_type = false
   s.custom_type = false

db/migrate/20240606003822_reorder_flags.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ReorderFlags < ActiveRecord::Migration[7.0]
+  def up
+    current_order = DB.query(<<~SQL)
+      SELECT name FROM flags
+      WHERE score_type IS FALSE
+      ORDER BY position ASC
+    SQL
+
+    if current_order.map(&:name) ==
+         %w[notify_user notify_moderators off_topic inappropriate spam illegal]
+      execute "UPDATE flags SET position = 0 WHERE name = 'notify_user'"
+      execute "UPDATE flags SET position = 1 WHERE name = 'off_topic'"
+      execute "UPDATE flags SET position = 2 WHERE name = 'inappropriate'"
+      execute "UPDATE flags SET position = 3 WHERE name = 'spam'"
+      execute "UPDATE flags SET position = 4 WHERE name = 'illegal'"
+      execute "UPDATE flags SET position = 5 WHERE name = 'notify_moderators'"
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

spec/models/flag_spec.rb

@@ -31,32 +31,32 @@ RSpec.describe Flag, type: :model do
 
   it "updates post action types when created, modified or destroyed" do
     expect(PostActionType.flag_types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators],
     )
     expect(ReviewableScore.types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal needs_approval],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators needs_approval],
     )
 
     flag = Fabricate(:flag, name: "custom")
     expect(PostActionType.flag_types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal custom],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators custom],
     )
     expect(ReviewableScore.types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal custom needs_approval],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators custom needs_approval],
     )
 
     flag.update!(name: "edited_custom")
     expect(PostActionType.flag_types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal edited_custom],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators edited_custom],
     )
     expect(ReviewableScore.types.keys).to eq(
       %i[
         notify_user
-        notify_moderators
         off_topic
         inappropriate
         spam
         illegal
+        notify_moderators
         edited_custom
         needs_approval
       ],
@@ -64,10 +64,10 @@ RSpec.describe Flag, type: :model do
 
     flag.destroy!
     expect(PostActionType.flag_types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators],
     )
     expect(ReviewableScore.types.keys).to eq(
-      %i[notify_user notify_moderators off_topic inappropriate spam illegal needs_approval],
+      %i[notify_user off_topic inappropriate spam illegal notify_moderators needs_approval],
     )
   end
 end

spec/services/reorder_flag_spec.rb

@@ -41,11 +41,11 @@ RSpec.describe(ReorderFlag) do
 
     it "moves the flag" do
       expect(Flag.order(:position).map(&:name)).to eq(
-        %w[notify_user notify_moderators off_topic inappropriate spam illegal],
+        %w[notify_user off_topic inappropriate spam illegal notify_moderators],
       )
       result
       expect(Flag.order(:position).map(&:name)).to eq(
-        %w[notify_user notify_moderators off_topic inappropriate illegal spam],
+        %w[notify_user off_topic inappropriate spam notify_moderators illegal],
       )
     end
 

spec/system/admin_flags_spec.rb

@@ -14,7 +14,7 @@ describe "Admin Flags Page", type: :system do
     topic_page.visit_topic(post.topic)
     topic_page.open_flag_topic_modal
     expect(all(".flag-action-type-details strong").map(&:text)).to eq(
-      ["Something Else", "It's Inappropriate", "It's Spam", "It's Illegal"],
+      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
     )
 
     visit "/admin/config/flags"
@@ -23,7 +23,7 @@ describe "Admin Flags Page", type: :system do
     topic_page.visit_topic(post.topic)
     topic_page.open_flag_topic_modal
     expect(all(".flag-action-type-details strong").map(&:text)).to eq(
-      ["Something Else", "It's Inappropriate", "It's Illegal"],
+      ["It's Inappropriate", "It's Illegal", "Something Else"],
     )
 
     Flag.system.where(name: "spam").update!(enabled: true)
@@ -33,7 +33,7 @@ describe "Admin Flags Page", type: :system do
     topic_page.visit_topic(post.topic)
     topic_page.open_flag_topic_modal
     expect(all(".flag-action-type-details strong").map(&:text)).to eq(
-      ["Something Else", "It's Inappropriate", "It's Spam", "It's Illegal"],
+      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
     )
 
     visit "/admin/config/flags"
@@ -42,7 +42,7 @@ describe "Admin Flags Page", type: :system do
     topic_page.visit_topic(post.topic)
     topic_page.open_flag_topic_modal
     expect(all(".flag-action-type-details strong").map(&:text)).to eq(
-      ["Something Else", "It's Inappropriate", "It's Illegal", "It's Spam"],
+      ["It's Inappropriate", "It's Illegal", "It's Spam", "Something Else"],
     )
 
     visit "/admin/config/flags"
@@ -51,7 +51,7 @@ describe "Admin Flags Page", type: :system do
     topic_page.visit_topic(post.topic)
     topic_page.open_flag_topic_modal
     expect(all(".flag-action-type-details strong").map(&:text)).to eq(
-      ["Something Else", "It's Inappropriate", "It's Spam", "It's Illegal"],
+      ["It's Inappropriate", "It's Spam", "It's Illegal", "Something Else"],
     )
   end
 
@@ -62,13 +62,13 @@ describe "Admin Flags Page", type: :system do
 
   it "does not allow bottom flag to move down" do
     visit "/admin/config/flags"
-    admin_flags_page.open_flag_menu("illegal")
+    admin_flags_page.open_flag_menu("notify_moderators")
     expect(page).not_to have_css(".dropdown-menu__item .move-down")
   end
 
   it "does not allow top flag to move up" do
     visit "/admin/config/flags"
-    admin_flags_page.open_flag_menu("notify_moderators")
+    admin_flags_page.open_flag_menu("off_topic")
     expect(page).not_to have_css(".dropdown-menu__item .move-up")
   end
 end