FEATURE: admin can disable flags (#27171)

UI for admins to disable system flags.
This commit is contained in:
Krzysztof Kotlarek 2024-05-29 14:39:58 +10:00 committed by GitHub
parent e9c8e182d3
commit 963b9fd157
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
31 changed files with 350 additions and 7 deletions

View File

@ -0,0 +1,43 @@
import Component from "@glimmer/component";
import { tracked } from "@glimmer/tracking";
import { fn } from "@ember/helper";
import { on } from "@ember/modifier";
import { action } from "@ember/object";
import { htmlSafe } from "@ember/template";
import DToggleSwitch from "discourse/components/d-toggle-switch";
import { ajax } from "discourse/lib/ajax";
import { popupAjaxError } from "discourse/lib/ajax-error";
export default class AdminFlagItem extends Component {
@tracked enabled = this.args.flag.enabled;
@action
toggleFlagEnabled(flag) {
this.enabled = !this.enabled;
return ajax(`/admin/config/flags/${flag.id}/toggle`, {
type: "PUT",
}).catch((error) => {
this.enabled = !this.enabled;
return popupAjaxError(error);
});
}
<template>
<tr class="admin-flag-item">
<td>
<p class="admin-flag-item__name">{{@flag.name}}</p>
<p class="admin-flag-item__description">{{htmlSafe
@flag.description
}}</p>
</td>
<td>
<DToggleSwitch
@state={{this.enabled}}
class="admin-flag-item__toggle {{@flag.name_key}}"
{{on "click" (fn this.toggleFlagEnabled @flag)}}
/>
</td>
</tr>
</template>
}

View File

@ -0,0 +1,26 @@
import Component from "@glimmer/component";
import { inject as service } from "@ember/service";
import i18n from "discourse-common/helpers/i18n";
import AdminFlagItem from "admin/components/admin-flag-item";
export default class AdminFlags extends Component {
@service site;
flags = this.site.flagTypes;
<template>
<div class="container admin-flags">
<h1>{{i18n "admin.flags.title"}}</h1>
<table class="flags grid">
<thead>
<th>{{i18n "admin.flags.description"}}</th>
<th>{{i18n "admin.flags.enabled"}}</th>
</thead>
<tbody>
{{#each this.flags as |flag|}}
<AdminFlagItem @flag={{flag}} />
{{/each}}
</tbody>
</table>
</div>
</template>
}

View File

@ -209,6 +209,14 @@ export default function () {
}
);
this.route(
"adminConfigFlags",
{ path: "/config/flags", resetNamespace: true },
function () {
this.route("index", { path: "/" });
}
);
this.route(
"adminPlugins",
{ path: "/plugins", resetNamespace: true },

View File

@ -0,0 +1 @@
<AdminFlags />

View File

@ -0,0 +1 @@
<AdminFlags />

View File

@ -85,7 +85,7 @@ export default class Flag extends Component {
}
get flagsAvailable() {
return this.args.model.flagTarget.flagsAvailable(this);
return this.args.model.flagTarget.flagsAvailable(this).filterBy("enabled");
}
get staffFlagsAvailable() {

View File

@ -110,6 +110,12 @@ export const ADMIN_NAV_MAP = [
label: "admin.community.sidebar_link.legal",
icon: "gavel",
},
{
name: "admin_moderation_flags",
route: "adminConfigFlags",
label: "admin.community.sidebar_link.moderation_flags",
icon: "flag",
},
],
},
{

View File

@ -590,6 +590,7 @@ export default {
icon: null,
id: 3,
is_custom_flag: false,
enabled: true
},
{
name_key: "inappropriate",
@ -602,6 +603,7 @@ export default {
icon: null,
id: 4,
is_custom_flag: false,
enabled: true
},
{
name_key: "vote",
@ -612,6 +614,7 @@ export default {
icon: null,
id: 5,
is_custom_flag: false,
enabled: true
},
{
name_key: "spam",
@ -623,6 +626,7 @@ export default {
icon: null,
id: 8,
is_custom_flag: false,
enabled: true
},
{
name_key: "notify_user",
@ -635,6 +639,7 @@ export default {
icon: null,
id: 6,
is_custom_flag: true,
enabled: true
},
{
name_key: "notify_moderators",
@ -646,6 +651,7 @@ export default {
icon: null,
id: 7,
is_custom_flag: true,
enabled: true
},
],
topic_flag_types: [
@ -658,6 +664,7 @@ export default {
icon: null,
id: 4,
is_custom_flag: false,
enabled: true
},
{
name_key: "spam",
@ -668,6 +675,7 @@ export default {
icon: null,
id: 8,
is_custom_flag: false,
enabled: true
},
{
name_key: "notify_moderators",
@ -678,6 +686,7 @@ export default {
icon: null,
id: 7,
is_custom_flag: true,
enabled: true
},
],
archetypes: [

View File

@ -1056,6 +1056,7 @@ a.inline-editable-field {
@import "common/admin/penalty";
@import "common/admin/badges";
@import "common/admin/emails";
@import "common/admin/flags";
@import "common/admin/json_schema_editor";
@import "common/admin/schema_field";
@import "common/admin/staff_logs";

View File

@ -0,0 +1,11 @@
.admin-flag-item {
&__name {
font-weight: bold;
color: var(--tertiary);
padding-bottom: 0;
margin-bottom: 0;
}
&__description {
margin-top: 0.5em;
}
}

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true
class Admin::AdminController < ApplicationController
include WithServiceHelper
requires_login
before_action :ensure_admin

View File

@ -0,0 +1,21 @@
# frozen_string_literal: true
class Admin::Config::FlagsController < Admin::AdminController
def toggle
with_service(ToggleFlag) 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_contract do |contract|
render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
end
end
end
def index
end
end

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true
class Admin::StaffController < ApplicationController
include WithServiceHelper
requires_login
before_action :ensure_staff
end

View File

@ -62,4 +62,5 @@ end
# enabled :boolean default(TRUE), not null
# created_at :datetime not null
# updated_at :datetime not null
# score_type :boolean default(FALSE), not null
#

View File

@ -37,6 +37,7 @@ class PostActionType < ActiveRecord::Base
@all_flags = nil
@flag_settings = FlagSettings.new
ReviewableScore.reload_types
PostActionType.new.expire_cache
end
def overridden_by_plugin_or_skipped_db?
@ -67,7 +68,18 @@ class PostActionType < ActiveRecord::Base
def flag_types
return flag_settings.flag_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags)
# Once replace_flag API is fully deprecated, then we can drop respond_to. It is needed right now for migration to be evaluated.
# TODO (krisk)
flag_enum(all_flags.reject { |flag| flag.respond_to?(:score_type) && flag.score_type })
end
def score_types
return flag_settings.flag_types if overridden_by_plugin_or_skipped_db?
# Once replace_flag API is fully deprecated, then we can drop respond_to. It is needed right now for migration to be evaluated.
# TODO (krisk)
flag_enum(all_flags.filter { |flag| flag.respond_to?(:score_type) && flag.score_type })
end
# flags resulting in mod notifications
@ -88,6 +100,14 @@ class PostActionType < ActiveRecord::Base
end
end
def disabled_flag_types
flag_enum(all_flags.reject(&:enabled))
end
def enabled_flag_types
flag_enum(all_flags.filter(&:enabled))
end
def custom_types
return flag_settings.custom_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.select(&:custom_type))

View File

@ -11,7 +11,7 @@ class ReviewableScore < ActiveRecord::Base
# To keep things simple the types correspond to `PostActionType` for backwards
# compatibility, but we can add extra reasons for scores.
def self.types
@types ||= PostActionType.flag_types.merge(needs_approval: 9)
@types ||= PostActionType.flag_types.merge(PostActionType.score_types)
end
# When extending post action flags, we need to call this method in order to

View File

@ -1,7 +1,16 @@
# frozen_string_literal: true
class PostActionTypeSerializer < ApplicationSerializer
attributes(:id, :name_key, :name, :description, :short_description, :is_flag, :is_custom_flag)
attributes(
:id,
:name_key,
:name,
:description,
:short_description,
:is_flag,
:is_custom_flag,
:enabled,
)
include ConfigurableUrls
@ -29,6 +38,10 @@ class PostActionTypeSerializer < ApplicationSerializer
PostActionType.types[object.id].to_s
end
def enabled
!!PostActionType.enabled_flag_types[object.id]
end
protected
def i18n(field, default: nil, vars: nil)

View File

@ -0,0 +1,40 @@
# frozen_string_literal: true
class ToggleFlag
include Service::Base
contract
model :flag
policy :invalid_access
transaction do
step :toggle
step :log
end
class Contract
attribute :flag_id, :integer
validates :flag_id, presence: true
end
private
def fetch_flag(flag_id:)
Flag.find(flag_id)
end
def invalid_access(guardian:)
guardian.can_toggle_flag?
end
def toggle(flag:)
flag.update!(enabled: !flag.enabled)
end
def log(guardian:, flag:)
StaffActionLogger.new(guardian.user).log_custom(
"toggle_flag",
{ flag: flag.name, enabled: flag.enabled },
)
end
end

View File

@ -5023,6 +5023,10 @@ en:
label: Category
include_subcategories:
label: "Include Subcategories"
flags:
title: "Moderation Flags"
description: "Description"
enabled: "Enabled?"
groups:
new:
@ -5362,6 +5366,8 @@ en:
user_fields: "User Fields"
watched_words: "Watched Words"
legal: "Legal"
moderation_flags: "Moderation Flags"
appearance:
title: "Appearance"
@ -6080,6 +6086,7 @@ en:
create_watched_word_group: "create watched word group"
update_watched_word_group: "update watched word group"
delete_watched_word_group: "delete watched word group"
toggle_flag: "toggle 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."

View File

@ -393,6 +393,11 @@ Discourse::Application.routes.draw do
post "preview" => "badges#preview"
end
end
namespace :config, constraints: StaffConstraint.new do
resources :flags, only: %i[index] do
put "toggle"
end
end
end # admin namespace
get "email/unsubscribe/:key" => "email#unsubscribe", :as => "email_unsubscribe"

View File

@ -54,3 +54,13 @@ Flag.seed do |s|
s.custom_type = true
s.applies_to = %w[Post Topic Chat::Message]
end
Flag.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
s.score_type = true
s.applies_to = %w[]
end

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddScoreTypeToFlags < ActiveRecord::Migration[7.0]
def change
add_column(:flags, :score_type, :boolean, default: false, null: false)
end
end

View File

@ -4,4 +4,8 @@ module FlagGuardian
def can_edit_flag?(flag)
@user.admin? && !flag.system? && !flag.used?
end
def can_toggle_flag?
@user.admin?
end
end

View File

@ -56,6 +56,8 @@ module PostGuardian
# post made by staff, but we don't allow staff flags
return false if is_flag && (!SiteSetting.allow_flagging_staff?) && post&.user&.staff?
return false if is_flag && PostActionType.disabled_flag_types.keys.include?(action_key)
if action_key == :notify_user &&
!@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)
# The modifier below is used to add additional permissions for notifying users.

View File

@ -3,13 +3,15 @@
RSpec.describe FlagGuardian do
fab!(:user)
fab!(:admin)
fab!(:moderator)
after(:each) { Flag.reset_flag_settings! }
describe "#can_edit_flag?" do
it "returns true for admin and false for regular user" do
it "returns true for admin and false for moderator and regular user" do
flag = Fabricate(:flag)
expect(Guardian.new(admin).can_edit_flag?(flag)).to eq(true)
expect(Guardian.new(moderator).can_edit_flag?(flag)).to eq(false)
expect(Guardian.new(user).can_edit_flag?(flag)).to eq(false)
flag.destroy!
end
@ -32,4 +34,11 @@ RSpec.describe FlagGuardian do
flag.destroy!
end
end
describe "#can_toggle_flag?" do
it "returns true for admin and false for regular user" do
expect(Guardian.new(admin).can_toggle_flag?).to eq(true)
expect(Guardian.new(user).can_toggle_flag?).to eq(false)
end
end
end

View File

@ -154,6 +154,15 @@ RSpec.describe Guardian do
expect(Guardian.new(admin).post_can_act?(post, :notify_user)).to be_truthy
end
it "returns false if flag is disabled" do
expect(Guardian.new(admin).post_can_act?(post, :spam)).to be true
Flag.where(name: "spam").update!(enabled: false)
expect(Guardian.new(admin).post_can_act?(post, :spam)).to be false
Flag.where(name: "spam").update!(enabled: true)
ensure
Flag.reset_flag_settings!
end
it "works as expected for silenced users" do
UserSilencer.silence(user, admin)

View File

@ -353,6 +353,9 @@
},
"is_custom_flag": {
"type": "boolean"
},
"enabled": {
"type": "boolean"
}
},
"required": [
@ -362,7 +365,8 @@
"description",
"short_description",
"is_flag",
"is_custom_flag"
"is_custom_flag",
"enabled"
]
}
},
@ -393,6 +397,9 @@
},
"is_custom_flag": {
"type": "boolean"
},
"enabled": {
"type": "boolean"
}
},
"required": [
@ -402,7 +409,8 @@
"description",
"short_description",
"is_flag",
"is_custom_flag"
"is_custom_flag",
"enabled"
]
}
},

View File

@ -0,0 +1,33 @@
# frozen_string_literal: true
RSpec.describe(ToggleFlag) do
subject(:result) { described_class.call(flag_id: flag.id, guardian: current_user.guardian) }
let(:flag) { Flag.system.last }
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 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 "toggles the flag" do
expect(result[:flag].enabled).to be false
end
it "logs the action" do
expect { result }.to change { UserHistory.count }.by(1)
expect(UserHistory.last).to have_attributes(
custom_type: "toggle_flag",
details: "flag: #{result[:flag].name}\nenabled: #{result[:flag].enabled}",
)
end
end
end

View File

@ -0,0 +1,29 @@
# frozen_string_literal: true
describe "Admin Flags Page", type: :system do
fab!(:admin)
fab!(:topic)
fab!(:post) { Fabricate(:post, topic: topic) }
let(:topic_page) { PageObjects::Pages::Topic.new }
let(:admin_flags_page) { PageObjects::Pages::AdminFlags.new }
before { sign_in(admin) }
it "allows admin to disable 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.toggle("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"],
)
end
end

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
module PageObjects
module Pages
class AdminFlags < PageObjects::Pages::Base
def toggle(key)
PageObjects::Components::DToggleSwitch.new(".admin-flag-item__toggle.#{key}").toggle
end
end
end
end

View File

@ -244,6 +244,10 @@ module PageObjects
find(".modal.convert-to-public-topic")
end
def open_flag_topic_modal
find(".flag-topic").click
end
private
def within_post(post)