From a8c3ff8c5aadfa3e7b571041b9f5cd1fbff8b4cb Mon Sep 17 00:00:00 2001 From: Andy Zhao <17884966+Zhao-Andy@users.noreply.github.com> Date: Thu, 5 Nov 2020 10:36:26 -0500 Subject: [PATCH] Add trusted role to new tag moderators (#11242) * Don't add to mailchimp for N/A circumstance * Use class << self over self. methods * Refactor and make add/remove tag mod idempotent * Add missing param wiki_body_markdown * Add errors_as_sentence b/c Tag not inheriting from AppRecord * Refactor and Crayonsify tag page * Use proper for attr to make label checkable * Crayonsify moderator section * Make placeholder white * Remove remote: false for sync form submit * Fix more merge conflicts * Minor styling adjustments * Fix color field value * Don't add to Mailchimp community mod list if not enabled * Add tests for new tag mod routes * Add missing line oops * Use self.method over class << self for new Rubyists * Use more efficient query for getting tag mods * Use parentheses to follow convention * Use cleaner way of routing and controller * Update tests to match new configuration --- .../admin/tags/moderators_controller.rb | 44 +++++++ app/controllers/admin/tags_controller.rb | 32 ++--- app/labor/assign_tag_moderator.rb | 16 ++- app/models/tag.rb | 4 + app/views/admin/tags/_form.html.erb | 57 +++++++++ app/views/admin/tags/show.html.erb | 109 +++++------------- config/routes.rb | 4 +- spec/requests/admin/tags/moderators_spec.rb | 35 ++++++ spec/requests/admin/tags_spec.rb | 8 +- 9 files changed, 200 insertions(+), 109 deletions(-) create mode 100644 app/controllers/admin/tags/moderators_controller.rb create mode 100644 app/views/admin/tags/_form.html.erb create mode 100644 spec/requests/admin/tags/moderators_spec.rb diff --git a/app/controllers/admin/tags/moderators_controller.rb b/app/controllers/admin/tags/moderators_controller.rb new file mode 100644 index 000000000..f114405c9 --- /dev/null +++ b/app/controllers/admin/tags/moderators_controller.rb @@ -0,0 +1,44 @@ +module Admin + module Tags + class ModeratorsController < Admin::ApplicationController + def authorization_resource + Tag + end + + after_action only: %i[create destroy] do + Audit::Logger.log(:moderator, current_user, params.dup) + end + + def create + user = User.find_by(id: tag_params[:user_id]) + if user&.update(email_tag_mod_newsletter: true) + AssignTagModerator.add_tag_moderators([user.id], [params[:tag_id]]) + flash[:success] = "#{user.username} was added as a tag moderator!" + else + flash[:error] = "Error: User ID ##{tag_params[:user_id]} was not found, + or their account has errors: #{user&.errors_as_sentence}" + end + redirect_to admin_tag_path(params[:tag_id]) + end + + def destroy + user = User.find_by(id: tag_params[:user_id]) + tag = Tag.find_by(id: params[:tag_id]) + if user&.update(email_tag_mod_newsletter: false) + AssignTagModerator.remove_tag_moderator(user, tag) + flash[:success] = "@#{user.username} - ID ##{user.id} was removed as a tag moderator." + else + flash[:error] = "Error: User ID ##{tag_params[:user_id]} was not found, + or their account has errors: #{user&.errors_as_sentence}" + end + redirect_to admin_tag_path(tag.id) + end + + private + + def tag_params + params.require(:tag).permit(:user_id) + end + end + end +end diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index 2abc1b943..9745b50a1 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/app/controllers/admin/tags_controller.rb @@ -20,37 +20,27 @@ module Admin def show @tag = Tag.find(params[:id]) + @tag_moderators = User.with_role(:tag_moderator, @tag).select(:id, :username) + @badges_for_options = Badge.pluck(:title, :id) end def update @tag = Tag.find(params[:id]) - @add_user_id = params[:tag][:tag_moderator_id] - @remove_user_id = params[:tag][:remove_moderator_id] - add_moderator if @add_user_id - remove_moderator if @remove_user_id - @tag.update!(tag_params) - - redirect_to "/admin/tags/#{params[:id]}" + if @tag.update(tag_params) + flash[:success] = "#{@tag.name} tag successfully updated!" + else + flash[:error] = "The tag update failed: #{@tag.errors_as_sentence}" + end + redirect_to admin_tag_path(@tag.id) end private - def remove_moderator - user = User.find(@remove_user_id) - user.update(email_tag_mod_newsletter: false) - AssignTagModerator.remove_tag_moderator(user, @tag) - end - - def add_moderator - User.find(@add_user_id).update(email_tag_mod_newsletter: true) - AssignTagModerator.add_tag_moderators([@add_user_id], [@tag.id]) - end - def tag_params allowed_params = %i[ - supported rules_markdown short_summary pretty_name bg_color_hex - text_color_hex tag_moderator_id remove_moderator_id alias_for badge_id - category social_preview_template + id supported rules_markdown short_summary pretty_name bg_color_hex + text_color_hex user_id alias_for badge_id + category social_preview_template wiki_body_markdown ] params.require(:tag).permit(allowed_params) end diff --git a/app/labor/assign_tag_moderator.rb b/app/labor/assign_tag_moderator.rb index 087856a3b..500534883 100644 --- a/app/labor/assign_tag_moderator.rb +++ b/app/labor/assign_tag_moderator.rb @@ -3,9 +3,9 @@ module AssignTagModerator return if user.has_role?(:trusted) return if user.has_role?(:banned) - user.add_role :trusted + user.add_role(:trusted) user.update(email_community_mod_newsletter: true) - MailchimpBot.new(user).manage_community_moderator_list + MailchimpBot.new(user).manage_community_moderator_list if community_mod_newsletter_enabled? Rails.cache.delete("user-#{user.id}/has_trusted_role") NotifyMailer.with(user: user).trusted_role_email.deliver_now end @@ -47,17 +47,25 @@ module AssignTagModerator user.update(email_tag_mod_newsletter: true) if user.email_tag_mod_newsletter == false user.add_role(:tag_moderator, tag) Rails.cache.delete("user-#{user.id}/tag_moderators_list") - MailchimpBot.new(user).manage_tag_moderator_list + MailchimpBot.new(user).manage_tag_moderator_list if tag_mod_newsletter_enabled? end def self.remove_tag_moderator(user, tag) user.remove_role(:tag_moderator, tag) user.update(email_tag_mod_newsletter: false) if user.email_tag_mod_newsletter == true Rails.cache.delete("user-#{user.id}/tag_moderators_list") - MailchimpBot.new(user).manage_tag_moderator_list + MailchimpBot.new(user).manage_tag_moderator_list if tag_mod_newsletter_enabled? end def self.chat_channel_slug(tag) tag.mod_chat_channel&.slug end + + def self.community_mod_newsletter_enabled? + SiteConfig.mailchimp_api_key.present? && SiteConfig.mailchimp_community_moderators_id.present? + end + + def self.tag_mod_newsletter_enabled? + SiteConfig.mailchimp_api_key.present? && SiteConfig.mailchimp_tag_moderators_id.present? + end end diff --git a/app/models/tag.rb b/app/models/tag.rb index b399202dd..57ce967b0 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -89,6 +89,10 @@ class Tag < ActsAsTaggableOn::Tag errors.add(:name, "contains non-ASCII characters") unless name.match?(/\A[[a-z0-9]]+\z/i) end + def errors_as_sentence + errors.full_messages.to_sentence + end + private def evaluate_markdown diff --git a/app/views/admin/tags/_form.html.erb b/app/views/admin/tags/_form.html.erb new file mode 100644 index 000000000..6061fc1b1 --- /dev/null +++ b/app/views/admin/tags/_form.html.erb @@ -0,0 +1,57 @@ +
+

Edit details

+ <%= form_with model: [:admin, tag], data: { remote: false } do |f| %> +
+ <%= f.check_box :supported, class: "crayons-checkbox" %> + +
+
+ <%= f.label :badge_id, class: "crayons-field__label" %> + <%= f.select(:badge_id, options_for_select(badges_for_options, tag.badge_id), { include_blank: true }, { class: "crayons-select" }) %> +
+
+ <%= f.label :category, class: "crayons-field__label" %> + <%= f.select(:category, options_for_select(Tag.valid_categories, tag.category), {}, { class: "crayons-select" }) %> +
+
+ <%= f.label :social_preview_template, class: "crayons-field__label" %> + <%= f.select(:social_preview_template, Tag.social_preview_templates, {}, { class: "crayons-select" }) %> +
+
+ <%= f.label :alias_for, class: "crayons-field__label" %> + <%= f.text_field :alias_for, value: tag.alias_for, class: "crayons-textfield" %> +
+
+ <%= f.label :pretty_name, class: "crayons-field__label" %> + <%= f.text_field :pretty_name, value: tag.pretty_name, class: "crayons-textfield" %> +
+
+ <%= f.label :short_summary, class: "crayons-field__label" %> + <%= f.text_field :short_summary, value: tag.short_summary, class: "crayons-textfield" %> +
+
+ <%= f.label :rules_markdown, class: "crayons-field__label" %> +
+ <%= f.text_area :rules_markdown, value: tag.rules_markdown, class: "crayons-textfield" %> +
+
+ <%= f.label :wiki_body_markdown, class: "crayons-field__label" %> +
+ <%= f.text_area :wiki_body_markdown, value: tag.wiki_body_markdown, class: "crayons-textfield" %> +
+ <%= f.label :bg_color_hex, class: "crayons-field__label" %> +
+ <%= f.text_field :bg_color_hex, class: "crayons-textfield", value: tag.bg_color_hex.presence || "#000000", placeholder: "#000000" %> + <%= f.color_field :bg_color_hex, class: "crayons-color-selector ml-2", required: true, value: tag.bg_color_hex.presence || "#000000", placeholder: "#000000" %> +
+ <%= f.label :text_color_hex, class: "crayons-field__label" %> +
+ <%= f.text_field :text_color_hex, class: "crayons-textfield", value: tag.text_color_hex.presence || "#ffffff", placeholder: "#ffffff" %> + <%= f.color_field :text_color_hex, class: "crayons-color-selector ml-2", required: true, value: tag.text_color_hex.presence || "#ffffff", placeholder: "#ffffff" %> +
+ <%= f.submit class: "crayons-btn mt-2" %> + <% end %> +
diff --git a/app/views/admin/tags/show.html.erb b/app/views/admin/tags/show.html.erb index 0423973c1..018402b1a 100644 --- a/app/views/admin/tags/show.html.erb +++ b/app/views/admin/tags/show.html.erb @@ -11,84 +11,35 @@
-

Moderators

- <% if @tag.tag_moderator_ids.any? %> - - <% else %> -

This tag currently has no moderators.

- <% end %> - <%= form_for [:admin, @tag], html: { class: "form-inline justify-content-between" } do |f| %> -
- <%= f.label :tag_moderator_id, "Add Moderator (id):", class: "mr-3" %> - <%= f.text_field :tag_moderator_id, class: "form-control" %> -
-
- <%= f.submit "Add Moderator", class: "btn btn-primary" %> -
- <% end %> -
- -
-

Edit details

- <%= form_for [:admin, @tag] do |f| %> -
- <%= f.label :supported %> - <%= f.check_box :supported %> -
-
- <%= f.label :badge_id %> - <% badges_array = Badge.pluck(:title, :id) %> - <%= f.select(:badge_id, options_for_select([["None", nil]] + badges_array, @tag.badge_id)) %> -
-
- <%= f.label :category %> - <%= f.select(:category, options_for_select(Tag.valid_categories, @tag.category)) %> -
-
- <%= f.label :social_preview_template %> - <%= f.select(:social_preview_template, Tag.social_preview_templates) %> -
-
- <%= f.label :alias_for %> - <%= f.text_field :alias_for, class: "form-control" %> -
-
- <%= f.label :pretty_name %> - <%= f.text_field :pretty_name, class: "form-control" %> -
-
- <%= f.label :short_summary %> - <%= f.text_field :short_summary, class: "form-control" %> -
-
- <%= f.label :rules_markdown %> -
- <%= f.text_area :rules_markdown, class: "form-control" %> -
-
- <%= f.label :wiki_body_markdown %> -
- <%= f.text_area :wiki_body_markdown, class: "form-control" %> -
-
- <%= f.label :bg_color_hex %> - <%= f.color_field :bg_color_hex, class: "form-control", required: true %> -
-
- <%= f.label :text_color_hex %> - <%= f.color_field :text_color_hex, class: "form-control", required: true, value: @tag.text_color_hex.presence || "#ffffff" %> -
- <%= f.submit class: "btn btn-primary float-right" %> - <% end %> +
+

Moderators

+
+
+ <% if @tag_moderators.exists? %> +
    + <% @tag_moderators.each do |user| %> +
  • + @<%= user.username %> + <%= form_with url: admin_tag_moderator_path(@tag.id), model: [:admin, @tag], method: :delete, data: { remote: false } do |f| %> + <%= f.hidden_field :user_id, value: user.id %> + <%= f.submit "Remove", class: "crayons-btn crayons-btn--danger crayons-btn--s" %> + <% end %> +
    • + <% end %> +
+ <% else %> +
+ This tag currently has no moderators. +
+ <% end %> + <%= form_with url: admin_tag_moderator_path(@tag.id), model: [:admin, @tag], method: :post, data: { remote: false } do |f| %> +
+ <%= f.label :user_id, "Add Moderator (ID):", class: "crayons-field__label" %> + <%= f.text_field :user_id, class: "crayons-textfield" %> + <%= f.submit "Add Moderator", class: "crayons-btn" %> +
+ <% end %> +
+ <%= render "form", tag: @tag, badges_for_options: @badges_for_options %>
diff --git a/config/routes.rb b/config/routes.rb index 6f9d4e5c8..66052ad39 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,7 +103,9 @@ Rails.application.routes.draw do post "save_status" end end - resources :tags, only: %i[index update show] + resources :tags, only: %i[index update show] do + resource :moderator, only: %i[create destroy], module: "tags" + end resources :users, only: %i[index show edit update] do member do post "banish" diff --git a/spec/requests/admin/tags/moderators_spec.rb b/spec/requests/admin/tags/moderators_spec.rb new file mode 100644 index 000000000..ae754cb48 --- /dev/null +++ b/spec/requests/admin/tags/moderators_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +RSpec.describe "/admin/tags/:id/moderator", type: :request do + let(:super_admin) { create(:user, :super_admin) } + let(:user) { create(:user) } + let(:tag) { create(:tag) } + + describe "POST /admin/tags/:id/moderator" do + before { sign_in super_admin } + + it "adds the given user as trusted and as a tag moderator" do + post admin_tag_moderator_path(tag.id), params: { tag_id: tag.id, tag: { user_id: user.id } } + expect(user.tag_moderator?).to be true + expect(user.trusted).to be true + end + end + + describe "DELETE /admin/tags/:id/moderator" do + before do + sign_in super_admin + user.add_role :trusted + user.add_role :tag_moderator, tag + end + + it "removes the tag moderator role from the user" do + delete admin_tag_moderator_path(tag.id), params: { tag_id: tag.id, tag: { user_id: user.id } } + expect(user.tag_moderator?).to be false + end + + it "does not remove the trusted role from the user" do + delete admin_tag_moderator_path(tag.id), params: { tag_id: tag.id, tag: { user_id: user.id } } + expect(user.trusted).to be true + end + end +end diff --git a/spec/requests/admin/tags_spec.rb b/spec/requests/admin/tags_spec.rb index ee9a72f5c..7c2860b72 100644 --- a/spec/requests/admin/tags_spec.rb +++ b/spec/requests/admin/tags_spec.rb @@ -2,23 +2,23 @@ require "rails_helper" RSpec.describe "/admin/tags", type: :request do let(:super_admin) { create(:user, :super_admin) } - let(:tag) { create(:tag) } + let(:user) { create(:user) } + let!(:tag) { create(:tag) } before do - tag sign_in super_admin end describe "GET /admin/tags" do it "responds with 200 OK" do - get "/admin/tags" + get admin_tags_path expect(response.status).to eq 200 end end describe "GET /admin/tags/:id" do it "responds with 200 OK" do - get "/admin/tags/#{tag.id}" + get admin_tag_path(tag.id) expect(response.status).to eq 200 end end