DEV: move post flags into database (#26951)

This is preparation for a feature that will allow admins to define their custom flags. Current behaviour should stay untouched.
This commit is contained in:
Krzysztof Kotlarek 2024-05-21 13:15:32 +10:00 committed by GitHub
parent 34c527d694
commit 7aff9806eb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
28 changed files with 346 additions and 83 deletions

View File

@ -14,7 +14,9 @@ export default Component.extend({
@discourseComputed("flag.name_key")
customPlaceholder(nameKey) {
return I18n.t("flagging.custom_placeholder_" + nameKey);
return I18n.t("flagging.custom_placeholder_" + nameKey, {
defaultValue: I18n.t("flagging.custom_placeholder_notify_moderators"),
});
},
@discourseComputed("flag.name", "flag.name_key", "username")
@ -22,7 +24,9 @@ export default Component.extend({
if (["notify_user", "notify_moderators"].includes(nameKey)) {
return name.replace(/{{username}}|%{username}/, username);
} else {
return I18n.t("flagging.formatted_name." + nameKey);
return I18n.t("flagging.formatted_name." + nameKey, {
defaultValue: name,
});
}
},

View File

@ -254,7 +254,11 @@ export default function transformPost(
postId: post.id,
action,
canUndo: a.can_undo,
description: I18n.t(`post.actions.by_you.${action}`),
description: I18n.t(`post.actions.by_you.${action}`, {
defaultValue: I18n.t(`post.actions.by_you.custom`, {
custom: a.actionType.name,
}),
}),
};
});
}

61
app/models/flag.rb Normal file
View File

@ -0,0 +1,61 @@
# frozen_string_literal: true
class Flag < ActiveRecord::Base
scope :enabled, -> { where(enabled: true) }
scope :system, -> { where("id < 1000") }
before_save :set_position
before_save :set_name_key
after_save :reset_flag_settings!
after_destroy :reset_flag_settings!
def used?
PostAction.exists?(post_action_type_id: self.id) ||
ReviewableScore.exists?(reviewable_score_type: self.id)
end
def self.reset_flag_settings!
PostActionType.reload_types
ReviewableScore.reload_types
end
def system?
self.id < 1000
end
def applies_to?(type)
self.applies_to.include?(type)
end
private
def reset_flag_settings!
self.class.reset_flag_settings!
end
def set_position
self.position = Flag.maximum(:position).to_i + 1 if !self.position
end
def set_name_key
self.name_key = self.name.squeeze(" ").gsub(" ", "_").gsub(/[^\w]/, "").downcase
end
end
# == Schema Information
#
# Table name: flags
#
# id :bigint not null, primary key
# name :string
# name_key :string
# description :text
# notify_type :boolean default(FALSE), not null
# auto_action_type :boolean default(FALSE), not null
# custom_type :boolean default(FALSE), not null
# applies_to :string not null, is an Array
# position :integer not null
# enabled :boolean default(TRUE), not null
# created_at :datetime not null
# updated_at :datetime not null
#

View File

@ -18,7 +18,7 @@ class PostActionType < ActiveRecord::Base
if settings
@flag_settings = settings
else
initialize_flag_settings
reload_types
end
@types = nil
end
@ -79,34 +79,27 @@ class PostActionType < ActiveRecord::Base
flag_types.valid?(sym)
end
private
def initialize_flag_settings
def reload_types
@types = nil
@flag_settings = FlagSettings.new
@flag_settings.add(3, :off_topic, notify_type: true, auto_action_type: true)
@flag_settings.add(
4,
:inappropriate,
topic_type: true,
notify_type: true,
auto_action_type: true,
)
@flag_settings.add(8, :spam, topic_type: true, notify_type: true, auto_action_type: true)
@flag_settings.add(6, :notify_user, topic_type: false, notify_type: false, custom_type: true)
@flag_settings.add(
7,
:notify_moderators,
topic_type: true,
notify_type: true,
custom_type: true,
)
@flag_settings.add(10, :illegal, topic_type: true, notify_type: true, custom_type: true)
# When adding a new ID here, check that it doesn't clash with any added in
# `ReviewableScore.types`. You can thank me later.
Flag
.enabled
.order(:position)
.each do |flag|
@flag_settings.add(
flag.id,
flag.name_key.to_sym,
topic_type: flag.applies_to?("Topic"),
notify_type: flag.notify_type,
auto_action_type: flag.auto_action_type,
custom_type: flag.custom_type,
name: flag.name,
)
end
end
end
initialize_flag_settings
reload_types
end
# == Schema Information

View File

@ -14,25 +14,25 @@ class PostActionTypeSerializer < ApplicationSerializer
end
def name
i18n("title")
i18n("title", default: object.class.flag_settings.names[object.id])
end
def description
i18n("description", tos_url: tos_url, base_path: Discourse.base_path)
i18n("description", vars: { tos_url: tos_url, base_path: Discourse.base_path })
end
def short_description
i18n("short_description", tos_url: tos_url, base_path: Discourse.base_path)
i18n("short_description", vars: { tos_url: tos_url, base_path: Discourse.base_path })
end
def name_key
PostActionType.types[object.id]
PostActionType.types[object.id].to_s
end
protected
def i18n(field, vars = nil)
def i18n(field, default: nil, vars: nil)
key = "post_action_types.#{name_key}.#{field}"
vars ? I18n.t(key, vars) : I18n.t(key)
vars ? I18n.t(key, vars, default: default) : I18n.t(key, default: default)
end
end

View File

@ -9,8 +9,9 @@ class ReviewableScoreTypeSerializer < ApplicationSerializer
# Allow us to share post action type translations for backwards compatibility
def title
I18n.t("post_action_types.#{ReviewableScore.types[id]}.title", default: nil) ||
I18n.t("reviewable_score_types.#{ReviewableScore.types[id]}.title")
I18n.t("post_action_types.#{type}.title", default: nil) ||
I18n.t("reviewable_score_types.#{type}.title", default: nil) ||
PostActionType.flag_settings.names[id]
end
def reviewable_priority

View File

@ -350,13 +350,6 @@ class SiteSerializer < ApplicationSerializer
private
def ordered_flags(flags)
notify_moderators_type = PostActionType.flag_types[:notify_moderators]
types = flags
if notify_moderators_flag = types.index(notify_moderators_type)
types.insert(types.length, types.delete_at(notify_moderators_flag))
end
types.map { |id| PostActionType.new(id: id) }
flags.map { |id| PostActionType.new(id: id) }
end
end

View File

@ -3,8 +3,8 @@
class TopicFlagTypeSerializer < PostActionTypeSerializer
protected
def i18n(field, vars = nil)
def i18n(field, default: nil, vars: nil)
key = "topic_flag_types.#{name_key}.#{field}"
vars ? I18n.t(key, vars) : I18n.t(key)
vars ? I18n.t(key, vars, default: default) : I18n.t(key, default: default)
end
end

View File

@ -3748,6 +3748,7 @@ en:
illegal: "You flagged this as illegal"
notify_moderators: "You flagged this for moderation"
notify_user: "You sent a message to this user"
custom: "You flagged this topic as %{custom}"
delete:
confirm:

56
db/fixtures/003_flags.rb Normal file
View File

@ -0,0 +1,56 @@
# frozen_string_literal: true
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
s.applies_to = %w[Post Chat::Message]
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
s.applies_to = %w[Post Topic Chat::Message]
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.name = "notify_user"
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.name = "illegal"
s.position = 5
s.notify_type = true
s.auto_action_type = false
s.custom_type = true
s.applies_to = %w[Post Topic Chat::Message]
end

View File

@ -18,7 +18,7 @@ class AddFlagStatsToUser < ActiveRecord::Migration[5.2]
SUM(CASE WHEN pa.deferred_at IS NOT NULL THEN 1 ELSE 0 END) as flags_ignored
FROM post_actions AS pa
INNER JOIN users AS u ON u.id = pa.user_id
WHERE pa.post_action_type_id IN (#{PostActionType.notify_flag_types.values.join(", ")})
WHERE pa.post_action_type_id IN (3, 4, 8, 7, 10)
AND pa.user_id > 0
GROUP BY u.id
) AS x

View File

@ -0,0 +1,19 @@
# frozen_string_literal: true
class CreateFlags < ActiveRecord::Migration[7.0]
def change
create_table :flags do |t|
t.string :name, unique: true
t.string :name_key, unique: true
t.text :description
t.boolean :notify_type, default: false, null: false
t.boolean :auto_action_type, default: false, null: false
t.boolean :custom_type, default: false, null: false
t.string :applies_to, array: true, null: false
t.integer :position, null: false
t.boolean :enabled, default: true, null: false
t.timestamps
end
DB.exec("SELECT setval('flags_id_seq', 1001, FALSE);")
end
end

View File

@ -7,6 +7,7 @@ class FlagSettings
:topic_flag_types,
:auto_action_types,
:custom_types,
:names,
)
def initialize
@ -16,18 +17,28 @@ class FlagSettings
@auto_action_types = Enum.new
@custom_types = Enum.new
@without_custom_types = Enum.new
@names = Enum.new
end
def add(id, name, topic_type: nil, notify_type: nil, auto_action_type: nil, custom_type: nil)
@all_flag_types[name] = id
@topic_flag_types[name] = id if !!topic_type
@notify_types[name] = id if !!notify_type
@auto_action_types[name] = id if !!auto_action_type
def add(
id,
name_key,
topic_type: nil,
notify_type: nil,
auto_action_type: nil,
custom_type: nil,
name: nil
)
@all_flag_types[name_key] = id
@topic_flag_types[name_key] = id if !!topic_type
@notify_types[name_key] = id if !!notify_type
@auto_action_types[name_key] = id if !!auto_action_type
@names[id] = name if name
if !!custom_type
@custom_types[name] = id
@custom_types[name_key] = id
else
@without_custom_types[name] = id
@without_custom_types[name_key] = id
end
end

View File

@ -4,6 +4,7 @@ require "guardian/bookmark_guardian"
require "guardian/category_guardian"
require "guardian/ensure_magic"
require "guardian/group_guardian"
require "guardian/flag_guardian"
require "guardian/post_guardian"
require "guardian/post_revision_guardian"
require "guardian/sidebar_guardian"
@ -16,6 +17,7 @@ class Guardian
include BookmarkGuardian
include CategoryGuardian
include EnsureMagic
include FlagGuardian
include GroupGuardian
include PostGuardian
include PostRevisionGuardian

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
module FlagGuardian
def can_edit_flag?(flag)
@user.admin? && !flag.system? && !flag.used?
end
end

View File

@ -35,7 +35,10 @@ export default class ChatMessageFlag {
return flags.map((flag) => {
flag.set(
"description",
I18n.t(`chat.flags.${flag.name_key}`, { basePath: getURL("") })
I18n.t(`chat.flags.${flag.name_key}`, {
basePath: getURL(""),
defaultValue: "",
})
);
return flag;
});
@ -45,9 +48,8 @@ export default class ChatMessageFlag {
let flagsAvailable = flagModal.site.flagTypes;
flagsAvailable = flagsAvailable.filter((flag) => {
return flagModal.args.model.flagModel.availableFlags.includes(
flag.name_key
);
const availableFlags = flagModal.args.model.flagModel.availableFlags;
return availableFlags.includes(flag.name_key);
});
// "message user" option should be at the top

View File

@ -22,7 +22,7 @@ RSpec.describe Chat::FlagMessage do
let(:guardian) { Guardian.new(current_user) }
let(:channel_id) { channel_1.id }
let(:message_id) { message_1.id }
let(:flag_type_id) { ReviewableScore.types.values.first }
let(:flag_type_id) { ReviewableScore.types[:off_topic] }
let(:message) { nil }
let(:is_warning) { nil }
let(:take_action) { nil }

View File

@ -1032,7 +1032,7 @@ RSpec.describe DiscourseNarrativeBot::NewUserNarrative do
let(:another_post) { Fabricate(:post, user: discobot_user, topic: topic) }
let(:flag) do
Fabricate(
:flag,
:flag_post_action,
post: post,
user: user,
post_action_type_id: PostActionType.types[:inappropriate],
@ -1040,7 +1040,7 @@ RSpec.describe DiscourseNarrativeBot::NewUserNarrative do
end
let(:other_flag) do
Fabricate(
:flag,
:flag_post_action,
post: another_post,
user: user,
post_action_type_id: PostActionType.types[:spam],
@ -1166,7 +1166,7 @@ RSpec.describe DiscourseNarrativeBot::NewUserNarrative do
describe "when post contain the right answer" do
let(:post) { Fabricate(:post, user: discobot_user, topic: topic) }
let(:flag) { Fabricate(:flag, post: post, user: user) }
let(:flag) { Fabricate(:flag_post_action, post: post, user: user) }
before do
narrative.set_data(user, state: :tutorial_flag, topic_id: topic.id)

View File

@ -1,7 +1,3 @@
# frozen_string_literal: true
Fabricator(:flag, from: :post_action) do
user
post
post_action_type_id PostActionType.types[:spam]
end
Fabricator(:flag) { name "offtopic", applies_to { %w[Post Chat::Message] } }

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
Fabricator(:flag_post_action, from: :post_action) do
user
post
post_action_type_id PostActionType.types[:spam]
end

View File

@ -360,7 +360,7 @@ RSpec.describe ComposerMessagesFinder do
end
it "shows a message when the replier has already flagged the post" do
Fabricate(:flag, post: self_flagged_post, user: user)
Fabricate(:flag_post_action, post: self_flagged_post, user: user)
finder =
ComposerMessagesFinder.new(
user,
@ -372,13 +372,13 @@ RSpec.describe ComposerMessagesFinder do
end
it "shows a message when replying to flagged topic (first post)" do
Fabricate(:flag, post: original_post, user: user)
Fabricate(:flag_post_action, post: original_post, user: user)
finder = ComposerMessagesFinder.new(user, composer_action: "reply", topic_id: topic.id)
expect(finder.check_dont_feed_the_trolls).to be_present
end
it "does not show a message when not enough others have flagged the post" do
Fabricate(:flag, post: under_flagged_post, user: other_user)
Fabricate(:flag_post_action, post: under_flagged_post, user: other_user)
finder =
ComposerMessagesFinder.new(
user,
@ -392,7 +392,12 @@ RSpec.describe ComposerMessagesFinder do
it "does not show a message when the flag has already been resolved" do
SiteSetting.dont_feed_the_trolls_threshold = 1
Fabricate(:flag, post: resolved_flag_post, user: other_user, disagreed_at: 1.hour.ago)
Fabricate(
:flag_post_action,
post: resolved_flag_post,
user: other_user,
disagreed_at: 1.hour.ago,
)
finder =
ComposerMessagesFinder.new(
user,
@ -404,8 +409,8 @@ RSpec.describe ComposerMessagesFinder do
end
it "shows a message when enough others have already flagged the post" do
Fabricate(:flag, post: over_flagged_post, user: other_user)
Fabricate(:flag, post: over_flagged_post, user: third_user)
Fabricate(:flag_post_action, post: over_flagged_post, user: other_user)
Fabricate(:flag_post_action, post: over_flagged_post, user: third_user)
finder =
ComposerMessagesFinder.new(
user,

View File

@ -0,0 +1,28 @@
# frozen_string_literal: true
RSpec.describe FlagGuardian do
fab!(:user)
fab!(:admin)
fab!(:flag)
describe "#can_edit_flag?" do
it "returns true for admin and false for regular user" do
expect(Guardian.new(admin).can_edit_flag?(flag)).to eq(true)
expect(Guardian.new(user).can_edit_flag?(flag)).to eq(false)
end
it "returns false when flag is system" do
expect(Guardian.new(admin).can_edit_flag?(Flag.system.first)).to eq(false)
end
it "returns false when flag was already used with post action" do
Fabricate(:post_action, post_action_type_id: flag.id)
expect(Guardian.new(admin).can_edit_flag?(flag)).to eq(false)
end
it "returns false when flag was already used with reviewable" do
Fabricate(:reviewable_score, reviewable_score_type: flag.id)
expect(Guardian.new(admin).can_edit_flag?(flag)).to eq(false)
end
end
end

View File

@ -484,7 +484,7 @@ RSpec.describe PostRevisor do
post = Fabricate(:post, raw: "hello world")
Fabricate(:flag, post: post, user: user)
Fabricate(:flag_post_action, post: post, user: user)
revisor = PostRevisor.new(post)
revisor.revise!(

69
spec/models/flag_spec.rb Normal file
View File

@ -0,0 +1,69 @@
# frozen_string_literal: true
RSpec.describe Flag, type: :model do
before { Flag.reset_flag_settings! }
it "has id lower than 1000 for system flags" do
flag = Fabricate(:flag, id: 1)
expect(flag.system?).to be true
end
it "has id greater than 1000 for non-system flags" do
flag = Fabricate(:flag)
expect(flag.system?).to be false
expect(flag.id).to be > 1000
end
it "has correct name key" do
flag = Fabricate(:flag, name: "CuStOm Flag!!!")
expect(flag.name_key).to eq("custom_flag")
flag.update!(name: "It's Illegal")
expect(flag.name_key).to eq("its_illegal")
flag.update!(name: "THIS IS SPaM!+)(*&^%$#@@@!)")
expect(flag.name_key).to eq("this_is_spam")
end
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],
)
expect(ReviewableScore.types.keys).to eq(
%i[notify_user notify_moderators off_topic inappropriate spam illegal 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],
)
expect(ReviewableScore.types.keys).to eq(
%i[notify_user notify_moderators off_topic inappropriate spam illegal 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],
)
expect(ReviewableScore.types.keys).to eq(
%i[
notify_user
notify_moderators
off_topic
inappropriate
spam
illegal
edited_custom
needs_approval
],
)
flag.destroy!
expect(PostActionType.flag_types.keys).to eq(
%i[notify_user notify_moderators off_topic inappropriate spam illegal],
)
expect(ReviewableScore.types.keys).to eq(
%i[notify_user notify_moderators off_topic inappropriate spam illegal needs_approval],
)
end
end

View File

@ -359,7 +359,7 @@ RSpec.describe TrustLevel3Requirements do
flags =
%i[off_topic inappropriate notify_user notify_moderators spam].map do |t|
Fabricate(
:flag,
:flag_post_action,
post: Fabricate(:post, user: user),
post_action_type_id: PostActionType.types[t],
agreed_at: 1.minute.ago,
@ -369,7 +369,7 @@ RSpec.describe TrustLevel3Requirements do
_deferred_flags =
%i[off_topic inappropriate notify_user notify_moderators spam].map do |t|
Fabricate(
:flag,
:flag_post_action,
post: Fabricate(:post, user: user),
post_action_type_id: PostActionType.types[t],
deferred_at: 1.minute.ago,
@ -379,7 +379,7 @@ RSpec.describe TrustLevel3Requirements do
_deleted_flags =
%i[off_topic inappropriate notify_user notify_moderators spam].map do |t|
Fabricate(
:flag,
:flag_post_action,
post: Fabricate(:post, user: user),
post_action_type_id: PostActionType.types[t],
deleted_at: 1.minute.ago,
@ -388,7 +388,7 @@ RSpec.describe TrustLevel3Requirements do
# Same post, different user:
Fabricate(
:flag,
:flag_post_action,
post: flags[1].post,
post_action_type_id: PostActionType.types[:spam],
agreed_at: 1.minute.ago,
@ -396,7 +396,7 @@ RSpec.describe TrustLevel3Requirements do
# Flagged their own post:
Fabricate(
:flag,
:flag_post_action,
user: user,
post: Fabricate(:post, user: user),
post_action_type_id: PostActionType.types[:spam],
@ -405,7 +405,7 @@ RSpec.describe TrustLevel3Requirements do
# More than 100 days ago:
Fabricate(
:flag,
:flag_post_action,
post: Fabricate(:post, user: user, created_at: 101.days.ago),
post_action_type_id: PostActionType.types[:spam],
created_at: 101.days.ago,

View File

@ -72,7 +72,11 @@ RSpec.describe SpamRule::AutoSilence do
end
it "returns 0 when there is one flag that has a reason other than spam" do
Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:off_topic])
Fabricate(
:flag_post_action,
post: post,
post_action_type_id: PostActionType.types[:off_topic],
)
expect(count).to eq(0)
end

View File

@ -6,7 +6,7 @@ describe "Composer don't feed the trolls popup", type: :system do
fab!(:topic) { Fabricate(:topic, user: user) }
fab!(:post) { Fabricate(:post, user: user, topic: topic) }
fab!(:reply) { Fabricate(:post, user: troll, topic: topic) }
fab!(:flag) { Fabricate(:flag, post: reply, user: user) }
fab!(:flag) { Fabricate(:flag_post_action, post: reply, user: user) }
let(:topic_page) { PageObjects::Pages::Topic.new }
before { sign_in user }

View File

@ -12,7 +12,7 @@ describe "Flagging post", type: :system do
describe "Using Take Action" do
it "can select the default action to hide the post, agree with other flags, and reach the flag threshold" do
other_flag = Fabricate(:flag, post: post_to_flag, user: Fabricate(:moderator))
other_flag = Fabricate(:flag_post_action, post: post_to_flag, user: Fabricate(:moderator))
other_flag_reviewable =
Fabricate(:reviewable_flagged_post, target: post_to_flag, created_by: other_flag.user)
expect(other_flag.reload.agreed_at).to be_nil