@@ -17,7 +56,12 @@ export default class AdminFlags extends Component {
{{#each this.flags as |flag|}}
-
+
{{/each}}
diff --git a/app/assets/javascripts/discourse/app/lib/constants.js b/app/assets/javascripts/discourse/app/lib/constants.js
index dd0830974ab..7cd9f15d8c1 100644
--- a/app/assets/javascripts/discourse/app/lib/constants.js
+++ b/app/assets/javascripts/discourse/app/lib/constants.js
@@ -80,3 +80,13 @@ export const TOPIC_VISIBILITY_REASONS = {
bulk_action: 5,
unknown: 99,
};
+
+export const SYSTEM_FLAG_IDS = {
+ like: 2,
+ notify_user: 6,
+ notify_moderators: 7,
+ off_topic: 3,
+ inappropriate: 4,
+ spam: 8,
+ illegal: 10,
+};
diff --git a/app/assets/stylesheets/common/admin/flags.scss b/app/assets/stylesheets/common/admin/flags.scss
index 1adf148b8a4..d63ef74129e 100644
--- a/app/assets/stylesheets/common/admin/flags.scss
+++ b/app/assets/stylesheets/common/admin/flags.scss
@@ -8,4 +8,14 @@
&__description {
margin-top: 0.5em;
}
+ &__options {
+ display: flex;
+ align-items: center;
+ }
+ .d-toggle-switch--label {
+ margin-bottom: 0;
+ }
+ .flag-menu-trigger {
+ padding: 0.25em 0.325em;
+ }
}
diff --git a/app/assets/stylesheets/mobile/_index.scss b/app/assets/stylesheets/mobile/_index.scss
index 651925d76bb..f521737a703 100644
--- a/app/assets/stylesheets/mobile/_index.scss
+++ b/app/assets/stylesheets/mobile/_index.scss
@@ -1,5 +1,6 @@
@import "admin_badges";
@import "admin_customize";
+@import "admin_flags";
@import "admin_report_counters";
@import "admin_report_table";
@import "admin_report";
diff --git a/app/assets/stylesheets/mobile/admin_flags.scss b/app/assets/stylesheets/mobile/admin_flags.scss
new file mode 100644
index 00000000000..dda3b4f055f
--- /dev/null
+++ b/app/assets/stylesheets/mobile/admin_flags.scss
@@ -0,0 +1,3 @@
+.admin-contents table.grid tr.admin-flag-item {
+ grid-template-columns: auto min-content;
+}
diff --git a/app/controllers/admin/config/flags_controller.rb b/app/controllers/admin/config/flags_controller.rb
index 7eb69d50fff..3f1bccdde7e 100644
--- a/app/controllers/admin/config/flags_controller.rb
+++ b/app/controllers/admin/config/flags_controller.rb
@@ -18,4 +18,20 @@ class Admin::Config::FlagsController < Admin::AdminController
def index
end
+
+ def reorder
+ with_service(ReorderFlag) do
+ on_success do
+ Discourse.request_refresh!
+ render(json: success_json)
+ end
+ on_failure { render(json: failed_json, status: 422) }
+ on_model_not_found(:message) { raise Discourse::NotFound }
+ on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
+ on_failed_policy(:invalid_move) { render_json_error(I18n.t("flags.errors.wrong_move")) }
+ on_failed_contract do |contract|
+ render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
+ end
+ end
+ end
end
diff --git a/app/models/flag.rb b/app/models/flag.rb
index 1312dfbf3d6..99e1ce5f6d4 100644
--- a/app/models/flag.rb
+++ b/app/models/flag.rb
@@ -10,7 +10,7 @@ class Flag < ActiveRecord::Base
after_save :reset_flag_settings!
after_destroy :reset_flag_settings!
- default_scope { order(:position) }
+ default_scope { order(:position).where(score_type: false) }
def used?
PostAction.exists?(post_action_type_id: self.id) ||
diff --git a/app/models/post_action_type.rb b/app/models/post_action_type.rb
index 611a011c4df..ff132794c3a 100644
--- a/app/models/post_action_type.rb
+++ b/app/models/post_action_type.rb
@@ -45,7 +45,7 @@ class PostActionType < ActiveRecord::Base
end
def all_flags
- @all_flags ||= Flag.all
+ @all_flags ||= Flag.unscoped.order(:position).all
end
def auto_action_flag_types
diff --git a/app/services/reorder_flag.rb b/app/services/reorder_flag.rb
new file mode 100644
index 00000000000..c0e8033f552
--- /dev/null
+++ b/app/services/reorder_flag.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+VALID_DIRECTIONS = %w[up down]
+
+class ReorderFlag
+ include Service::Base
+
+ contract
+ model :flag
+ policy :invalid_access
+ policy :invalid_move
+
+ transaction do
+ step :move
+ step :log
+ end
+
+ class Contract
+ attribute :flag_id, :integer
+ attribute :direction, :string
+ validates :flag_id, presence: true
+ validates :direction, inclusion: { in: VALID_DIRECTIONS }
+ end
+
+ private
+
+ def fetch_flag(flag_id:)
+ Flag.find(flag_id)
+ end
+
+ def invalid_access(guardian:, flag:)
+ guardian.can_reorder_flag?(flag)
+ end
+
+ def all_flags
+ @all_flags ||= Flag.where.not(name_key: "notify_user").order(:position)
+ end
+
+ def invalid_move(flag:, direction:)
+ return false if all_flags.first == flag && direction == "up"
+ return false if all_flags.last == flag && direction == "down"
+ true
+ end
+
+ def move(flag:, direction:)
+ old_position = flag.position
+ index = all_flags.index(flag)
+ target_flag = all_flags[direction == "up" ? index - 1 : index + 1]
+
+ flag.update!(position: target_flag.position)
+ target_flag.update!(position: old_position)
+ end
+
+ def log(guardian:, flag:, direction:)
+ StaffActionLogger.new(guardian.user).log_custom(
+ "move_flag",
+ { flag: flag.name, direction: direction },
+ )
+ end
+end
diff --git a/app/serializers/toggle_flag.rb b/app/services/toggle_flag.rb
similarity index 100%
rename from app/serializers/toggle_flag.rb
rename to app/services/toggle_flag.rb
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 69a0ca726eb..483aead157c 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -5033,6 +5033,10 @@ en:
title: "Moderation Flags"
description: "Description"
enabled: "Enabled?"
+ more_options:
+ title: "More options"
+ move_up: "Move up"
+ move_down: "Move down"
groups:
new:
@@ -6093,6 +6097,7 @@ en:
update_watched_word_group: "update watched word group"
delete_watched_word_group: "delete watched word group"
toggle_flag: "toggle flag"
+ move_flag: "move flag"
screened_emails:
title: "Screened Emails"
description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 5d961dbea6e..52e26eef59f 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1250,6 +1250,7 @@ en:
flags:
errors:
already_handled: "Flag was already handled"
+ wrong_move: "Flag cannot be moved"
reports:
default:
labels:
diff --git a/config/routes.rb b/config/routes.rb
index 3bd79656827..6058895335b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -388,6 +388,7 @@ Discourse::Application.routes.draw do
namespace :config, constraints: StaffConstraint.new do
resources :flags, only: %i[index] do
put "toggle"
+ put "reorder/:direction" => "flags#reorder"
end
resources :about, constraints: AdminConstraint.new, only: %i[index] do
diff --git a/db/fixtures/003_flags.rb b/db/fixtures/003_flags.rb
index 3e8fef259c8..2b7e19f3659 100644
--- a/db/fixtures/003_flags.rb
+++ b/db/fixtures/003_flags.rb
@@ -54,7 +54,7 @@ Flag.seed do |s|
s.custom_type = true
s.applies_to = %w[Post Topic Chat::Message]
end
-Flag.seed do |s|
+Flag.unscoped.seed do |s|
s.id = 9
s.name = "needs_approval"
s.position = 6
diff --git a/lib/guardian/flag_guardian.rb b/lib/guardian/flag_guardian.rb
index 72df18ff822..89cd218bb6a 100644
--- a/lib/guardian/flag_guardian.rb
+++ b/lib/guardian/flag_guardian.rb
@@ -8,4 +8,8 @@ module FlagGuardian
def can_toggle_flag?
@user.admin?
end
+
+ def can_reorder_flag?(flag)
+ @user.admin? && flag.name_key != "notify_user"
+ end
end
diff --git a/lib/tasks/javascript.rake b/lib/tasks/javascript.rake
index 3587384ca33..ba68b791aa0 100644
--- a/lib/tasks/javascript.rake
+++ b/lib/tasks/javascript.rake
@@ -164,6 +164,8 @@ task "javascript:update_constants" => :environment do
export const MAX_NOTIFICATIONS_LIMIT_PARAMS = #{NotificationsController::INDEX_LIMIT};
export const TOPIC_VISIBILITY_REASONS = #{Topic.visibility_reasons.to_json};
+
+ export const SYSTEM_FLAG_IDS = #{PostActionType.types.to_json}
JS
pretty_notifications = Notification.types.map { |n| " #{n[0]}: #{n[1]}," }.join("\n")
diff --git a/spec/lib/guardian/flag_guardian_spec.rb b/spec/lib/guardian/flag_guardian_spec.rb
index 951af910582..f221084f6fc 100644
--- a/spec/lib/guardian/flag_guardian_spec.rb
+++ b/spec/lib/guardian/flag_guardian_spec.rb
@@ -41,4 +41,14 @@ RSpec.describe FlagGuardian do
expect(Guardian.new(user).can_toggle_flag?).to eq(false)
end
end
+
+ describe "#can_reorder_flag?" do
+ it "returns true for admin and false for regular user and notify_user" do
+ expect(Guardian.new(admin).can_reorder_flag?(Flag.system.last)).to eq(true)
+ expect(
+ Guardian.new(admin).can_reorder_flag?(Flag.system.find_by(name_key: "notify_user")),
+ ).to eq(false)
+ expect(Guardian.new(user).can_reorder_flag?(Flag.system.last)).to eq(false)
+ end
+ end
end
diff --git a/spec/services/reorder_flag_spec.rb b/spec/services/reorder_flag_spec.rb
new file mode 100644
index 00000000000..4351f62ed57
--- /dev/null
+++ b/spec/services/reorder_flag_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+RSpec.describe(ReorderFlag) do
+ subject(:result) do
+ described_class.call(flag_id: flag.id, guardian: current_user.guardian, direction: direction)
+ end
+
+ let(:flag) { Flag.order(:position).last }
+ let(:direction) { "up" }
+
+ context "when user is not allowed to perform the action" do
+ fab!(:current_user) { Fabricate(:user) }
+
+ it { is_expected.to fail_a_policy(:invalid_access) }
+ end
+
+ context "when direction is invalid" do
+ fab!(:current_user) { Fabricate(:admin) }
+ let(:direction) { "side" }
+
+ it { is_expected.to fail_a_contract }
+ end
+
+ context "when move is invalid" do
+ fab!(:current_user) { Fabricate(:admin) }
+ let(:direction) { "down" }
+
+ it { is_expected.to fail_a_policy(:invalid_move) }
+ end
+
+ context "when user is allowed to perform the action" do
+ fab!(:current_user) { Fabricate(:admin) }
+
+ it "sets the service result as successful" do
+ expect(result).to be_a_success
+ end
+
+ it "moves the flag" do
+ expect(Flag.order(:position).map(&:name)).to eq(
+ %w[notify_user notify_moderators off_topic inappropriate spam illegal],
+ )
+ result
+ expect(Flag.order(:position).map(&:name)).to eq(
+ %w[notify_user notify_moderators off_topic inappropriate illegal spam],
+ )
+ end
+
+ it "logs the action" do
+ expect { result }.to change { UserHistory.count }.by(1)
+ expect(UserHistory.last).to have_attributes(
+ custom_type: "move_flag",
+ details: "flag: #{result[:flag].name}\ndirection: up",
+ )
+ end
+ end
+end
diff --git a/spec/system/admin_flags_spec.rb b/spec/system/admin_flags_spec.rb
index fc6888725ad..4098fa4f167 100644
--- a/spec/system/admin_flags_spec.rb
+++ b/spec/system/admin_flags_spec.rb
@@ -26,4 +26,47 @@ describe "Admin Flags Page", type: :system do
["Something Else", "It's Inappropriate", "It's Illegal"],
)
end
+
+ it "allows admin to change order of flags" 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"],
+ )
+
+ visit "/admin/config/flags"
+ admin_flags_page.move_down("spam")
+
+ 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"],
+ )
+
+ visit "/admin/config/flags"
+ admin_flags_page.move_up("spam")
+
+ 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"],
+ )
+ end
+
+ it "does not allow to move notify user flag" do
+ visit "/admin/config/flags"
+ expect(page).not_to have_css(".notify_user .flag-menu-trigger")
+ end
+
+ it "does not allow bottom flag to move down" do
+ visit "/admin/config/flags"
+ admin_flags_page.open_flag_menu("illegal")
+ 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")
+ expect(page).not_to have_css(".dropdown-menu__item .move-up")
+ end
end
diff --git a/spec/system/page_objects/pages/admin_flags.rb b/spec/system/page_objects/pages/admin_flags.rb
index 72748128918..6e481c09c59 100644
--- a/spec/system/page_objects/pages/admin_flags.rb
+++ b/spec/system/page_objects/pages/admin_flags.rb
@@ -6,6 +6,20 @@ module PageObjects
def toggle(key)
PageObjects::Components::DToggleSwitch.new(".admin-flag-item__toggle.#{key}").toggle
end
+
+ def open_flag_menu(key)
+ find(".#{key} .flag-menu-trigger").click
+ end
+
+ def move_down(key)
+ open_flag_menu(key)
+ find(".dropdown-menu__item .move-down").click
+ end
+
+ def move_up(key)
+ open_flag_menu(key)
+ find(".dropdown-menu__item .move-up").click
+ end
end
end
end