From aa201060e8c57c3756fe7e805d5bc157479bd5ba Mon Sep 17 00:00:00 2001 From: Ridhwana Date: Fri, 15 Sep 2023 15:06:58 +0200 Subject: [PATCH] Override Tag Moderation Actions (#20088) * feat: remove the validation for the article_id and tag_name to be unique on a tag_adjustment * chore: remove unique index * feat: update the validate_tag method to align with the removal of unique indexes on the table * feat: do not allow removal and addition of tags that were added/removed by admins * spec: add tests for the tag_adjustment_spec * fix: update the validation of tags * spec: article validation * spec: test the action_panel helper * chore: amend language * add styles * Empty commit --- app/assets/stylesheets/views/mod-actions.scss | 24 +++++++ .../moderations/actions_panel_helper.rb | 10 +++ app/models/article.rb | 25 +++++--- app/models/tag_adjustment.rb | 3 +- app/views/moderations/actions_panel.html.erb | 63 +++++++++++++------ config/locales/views/moderations/en.yml | 6 +- config/locales/views/moderations/fr.yml | 4 +- ..._adjustments_on_tag_name_and_article_id.rb | 19 ++++++ db/schema.rb | 3 +- .../moderations/actions_panel_helper_spec.rb | 26 ++++++++ spec/models/article_spec.rb | 58 +++++++++++------ spec/models/tag_adjustment_spec.rb | 46 ++++++++++---- 12 files changed, 222 insertions(+), 65 deletions(-) create mode 100644 app/helpers/moderations/actions_panel_helper.rb create mode 100644 db/migrate/20230910135738_remove_index_tag_adjustments_on_tag_name_and_article_id.rb create mode 100644 spec/helpers/moderations/actions_panel_helper_spec.rb diff --git a/app/assets/stylesheets/views/mod-actions.scss b/app/assets/stylesheets/views/mod-actions.scss index 58ad40dab..79b1e85c7 100644 --- a/app/assets/stylesheets/views/mod-actions.scss +++ b/app/assets/stylesheets/views/mod-actions.scss @@ -224,6 +224,21 @@ } } + .adjustment-restriction__container--text { + color: var(--color-secondary); + } + + .adjustment-restriction__container--tag { + font-family: var(--ff-monospace); + line-height: var(--lh-base); + letter-spacing: -0.32px; + color: var(--tag-name); + + .num-sign { + color: var(--tag-hash); + } + } + .additional-subtext-section { font-weight: var(--fw-medium); font-size: var(--fs-xs); @@ -290,6 +305,7 @@ .reaction-vomit-button.reacted .reacted-emoji { display: block; } + .other-things-container { border-top: 1px solid var(--base-20); overflow-y: auto; @@ -335,6 +351,7 @@ } } } + .toggle-chevron-container { width: 16px; height: 16px; @@ -350,6 +367,7 @@ transform: rotateZ(-180deg); } } + &:hover { background-color: var(--base-inverted); .label-wrapper > .icon { @@ -362,11 +380,13 @@ color: var(--accent-brand); } } + &.active { header h2 { font-weight: var(--fw-bold); } } + &.inactive:not(.active) { header h2 { color: var(--base-50); @@ -385,6 +405,7 @@ } } } + .dropdown-options { flex-direction: column; padding: 0 var(--su-4); @@ -394,6 +415,7 @@ display: flex; } } + .adjust-tags-options { .add-tag-container { padding: var(--su-2) var(--su-1); @@ -482,6 +504,7 @@ } } } + button.add-tag { background-color: inherit; border: none; @@ -552,6 +575,7 @@ } } } + .set-experience-options { button.level-rating-button { background: inherit; diff --git a/app/helpers/moderations/actions_panel_helper.rb b/app/helpers/moderations/actions_panel_helper.rb new file mode 100644 index 000000000..77983b440 --- /dev/null +++ b/app/helpers/moderations/actions_panel_helper.rb @@ -0,0 +1,10 @@ +module Moderations + module ActionsPanelHelper + def last_adjusted_by_admin?(article, tag, adjustment_type) + last_user_id = TagAdjustment.where(article_id: article.id, adjustment_type: adjustment_type, status: "committed", + tag_id: tag.id) + .last&.user_id + User.find_by(id: last_user_id)&.any_admin? + end + end +end diff --git a/app/models/article.rb b/app/models/article.rb index e27bb6f4d..ecf2d8041 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -817,10 +817,20 @@ class Article < ApplicationRecord published_at || date || Time.current end + # When an article is saved, it ensures that the tags that were adjusted by moderators and admins + # remain adjusted. We do not allow the author to add or remove tags that were previously added or + # removed by moderators and admins. + # + # This method is called before validation, so that the tag_list can be validated. + # + # @return [String] an array of tag names. def validate_tag - # remove adjusted tags - remove_tag_adjustments_from_tag_list - add_tag_adjustments_to_tag_list + distinct_tag_adjustments = TagAdjustment.where(article_id: id, status: "committed") + .select('DISTINCT ON ("tag_id") *') + .order(:tag_id, updated_at: :desc, id: :desc) + + remove_tag_adjustments_from_tag_list(distinct_tag_adjustments) + add_tag_adjustments_to_tag_list(distinct_tag_adjustments) # check there are not too many tags return errors.add(:tag_list, I18n.t("models.article.too_many_tags")) if tag_list.size > MAX_TAG_LIST_SIZE @@ -828,14 +838,13 @@ class Article < ApplicationRecord validate_tag_name(tag_list) end - def remove_tag_adjustments_from_tag_list - tags_to_remove = TagAdjustment.where(article_id: id, adjustment_type: "removal", - status: "committed").pluck(:tag_name) + def remove_tag_adjustments_from_tag_list(distinct_adjustments) + tags_to_remove = distinct_adjustments.select { |adj| adj.adjustment_type == "removal" }.pluck(:tag_name) tag_list.remove(tags_to_remove, parse: true) if tags_to_remove.present? end - def add_tag_adjustments_to_tag_list - tags_to_add = TagAdjustment.where(article_id: id, adjustment_type: "addition", status: "committed").pluck(:tag_name) + def add_tag_adjustments_to_tag_list(distinct_adjustments) + tags_to_add = distinct_adjustments.select { |adj| adj.adjustment_type == "addition" }.pluck(:tag_name) return if tags_to_add.blank? tag_list.add(tags_to_add, parse: true) diff --git a/app/models/tag_adjustment.rb b/app/models/tag_adjustment.rb index aec115d09..9b1d928d5 100644 --- a/app/models/tag_adjustment.rb +++ b/app/models/tag_adjustment.rb @@ -1,6 +1,5 @@ class TagAdjustment < ApplicationRecord - validates :tag_name, presence: true, - uniqueness: { scope: :article_id, message: I18n.t("models.tag_adjustment.unique") } + validates :tag_name, presence: true validates :adjustment_type, inclusion: { in: %w[removal addition] }, presence: true validates :status, inclusion: { in: %w[committed pending committed_and_resolvable resolved] }, presence: true has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all diff --git a/app/views/moderations/actions_panel.html.erb b/app/views/moderations/actions_panel.html.erb index cec38aad1..21aff8793 100644 --- a/app/views/moderations/actions_panel.html.erb +++ b/app/views/moderations/actions_panel.html.erb @@ -110,7 +110,12 @@