Revert "DEV: move post flags into database (#26951)" (#27102)

This reverts commit 7aff9806eb.
This commit is contained in:
Krzysztof Kotlarek 2024-05-21 16:21:07 +10:00 committed by GitHub
parent 67a7b28096
commit 40d65dddf8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
28 changed files with 83 additions and 346 deletions

View File

@ -14,9 +14,7 @@ export default Component.extend({
@discourseComputed("flag.name_key")
customPlaceholder(nameKey) {
return I18n.t("flagging.custom_placeholder_" + nameKey, {
defaultValue: I18n.t("flagging.custom_placeholder_notify_moderators"),
});
return I18n.t("flagging.custom_placeholder_" + nameKey);
},
@discourseComputed("flag.name", "flag.name_key", "username")
@ -24,9 +22,7 @@ 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, {
defaultValue: name,
});
return I18n.t("flagging.formatted_name." + nameKey);
}
},

View File

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

View File

@ -1,61 +0,0 @@
# 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
reload_types
initialize_flag_settings
end
@types = nil
end
@ -79,27 +79,34 @@ class PostActionType < ActiveRecord::Base
flag_types.valid?(sym)
end
def reload_types
@types = nil
private
def initialize_flag_settings
@flag_settings = FlagSettings.new
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
@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.
end
end
reload_types
initialize_flag_settings
end
# == Schema Information

View File

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

View File

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

View File

@ -350,6 +350,13 @@ class SiteSerializer < ApplicationSerializer
private
def ordered_flags(flags)
flags.map { |id| PostActionType.new(id: id) }
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) }
end
end

View File

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

View File

@ -3748,7 +3748,6 @@ 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:

View File

@ -1,56 +0,0 @@
# 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 (3, 4, 8, 7, 10)
WHERE pa.post_action_type_id IN (#{PostActionType.notify_flag_types.values.join(", ")})
AND pa.user_id > 0
GROUP BY u.id
) AS x

View File

@ -1,19 +0,0 @@
# 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,7 +7,6 @@ class FlagSettings
:topic_flag_types,
:auto_action_types,
:custom_types,
:names,
)
def initialize
@ -17,28 +16,18 @@ class FlagSettings
@auto_action_types = Enum.new
@custom_types = Enum.new
@without_custom_types = Enum.new
@names = Enum.new
end
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
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
if !!custom_type
@custom_types[name_key] = id
@custom_types[name] = id
else
@without_custom_types[name_key] = id
@without_custom_types[name] = id
end
end

View File

@ -4,7 +4,6 @@ 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"
@ -17,7 +16,6 @@ class Guardian
include BookmarkGuardian
include CategoryGuardian
include EnsureMagic
include FlagGuardian
include GroupGuardian
include PostGuardian
include PostRevisionGuardian

View File

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

View File

@ -35,10 +35,7 @@ export default class ChatMessageFlag {
return flags.map((flag) => {
flag.set(
"description",
I18n.t(`chat.flags.${flag.name_key}`, {
basePath: getURL(""),
defaultValue: "",
})
I18n.t(`chat.flags.${flag.name_key}`, { basePath: getURL("") })
);
return flag;
});
@ -48,8 +45,9 @@ export default class ChatMessageFlag {
let flagsAvailable = flagModal.site.flagTypes;
flagsAvailable = flagsAvailable.filter((flag) => {
const availableFlags = flagModal.args.model.flagModel.availableFlags;
return availableFlags.includes(flag.name_key);
return flagModal.args.model.flagModel.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[:off_topic] }
let(:flag_type_id) { ReviewableScore.types.values.first }
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_post_action,
:flag,
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_post_action,
:flag,
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_action, post: post, user: user) }
let(:flag) { Fabricate(:flag, post: post, user: user) }
before do
narrative.set_data(user, state: :tutorial_flag, topic_id: topic.id)

View File

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

View File

@ -1,7 +0,0 @@
# 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_action, post: self_flagged_post, user: user)
Fabricate(:flag, 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_action, post: original_post, user: user)
Fabricate(:flag, 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_action, post: under_flagged_post, user: other_user)
Fabricate(:flag, post: under_flagged_post, user: other_user)
finder =
ComposerMessagesFinder.new(
user,
@ -392,12 +392,7 @@ 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_action,
post: resolved_flag_post,
user: other_user,
disagreed_at: 1.hour.ago,
)
Fabricate(:flag, post: resolved_flag_post, user: other_user, disagreed_at: 1.hour.ago)
finder =
ComposerMessagesFinder.new(
user,
@ -409,8 +404,8 @@ RSpec.describe ComposerMessagesFinder do
end
it "shows a message when enough others have already flagged the post" do
Fabricate(:flag_post_action, post: over_flagged_post, user: other_user)
Fabricate(:flag_post_action, post: over_flagged_post, user: third_user)
Fabricate(:flag, post: over_flagged_post, user: other_user)
Fabricate(:flag, post: over_flagged_post, user: third_user)
finder =
ComposerMessagesFinder.new(
user,

View File

@ -1,28 +0,0 @@
# 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_action, post: post, user: user)
Fabricate(:flag, post: post, user: user)
revisor = PostRevisor.new(post)
revisor.revise!(

View File

@ -1,69 +0,0 @@
# 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_post_action,
:flag,
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_post_action,
:flag,
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_post_action,
:flag,
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_post_action,
:flag,
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_post_action,
:flag,
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_post_action,
:flag,
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,11 +72,7 @@ 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_action,
post: post,
post_action_type_id: PostActionType.types[:off_topic],
)
Fabricate(:flag, 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_action, post: reply, user: user) }
fab!(:flag) { Fabricate(:flag, 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_action, post: post_to_flag, user: Fabricate(:moderator))
other_flag = Fabricate(:flag, 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