From 23aeef52cc4540b4514e9f3b935b21f0530a3746 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sat, 6 Jul 2019 23:26:16 +0200
Subject: Remove Salmon and PubSubHubbub (#11205)

* Remove Salmon and PubSubHubbub endpoints

* Add error when trying to follow OStatus accounts

* Fix new accounts not being created in ResolveAccountService
---
 spec/controllers/admin/accounts_controller_spec.rb | 38 ----------------------
 .../admin/subscriptions_controller_spec.rb         | 32 ------------------
 2 files changed, 70 deletions(-)
 delete mode 100644 spec/controllers/admin/subscriptions_controller_spec.rb

(limited to 'spec/controllers/admin')

diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb
index a348ab3d7..608606ff9 100644
--- a/spec/controllers/admin/accounts_controller_spec.rb
+++ b/spec/controllers/admin/accounts_controller_spec.rb
@@ -75,44 +75,6 @@ RSpec.describe Admin::AccountsController, type: :controller do
     end
   end
 
-  describe 'POST #subscribe' do
-    subject { post :subscribe, params: { id: account.id } }
-
-    let(:current_user) { Fabricate(:user, admin: admin) }
-    let(:account) { Fabricate(:account) }
-
-    context 'when user is admin' do
-      let(:admin) { true }
-
-      it { is_expected.to redirect_to admin_account_path(account.id) }
-    end
-
-    context 'when user is not admin' do
-      let(:admin) { false }
-
-      it { is_expected.to have_http_status :forbidden }
-    end
-  end
-
-  describe 'POST #unsubscribe' do
-    subject { post :unsubscribe, params: { id: account.id } }
-
-    let(:current_user) { Fabricate(:user, admin: admin) }
-    let(:account) { Fabricate(:account) }
-
-    context 'when user is admin' do
-      let(:admin) { true }
-
-      it { is_expected.to redirect_to admin_account_path(account.id) }
-    end
-
-    context 'when user is not admin' do
-      let(:admin) { false }
-
-      it { is_expected.to have_http_status :forbidden }
-    end
-  end
-
   describe 'POST #memorialize' do
     subject { post :memorialize, params: { id: account.id } }
 
diff --git a/spec/controllers/admin/subscriptions_controller_spec.rb b/spec/controllers/admin/subscriptions_controller_spec.rb
deleted file mode 100644
index 967152abe..000000000
--- a/spec/controllers/admin/subscriptions_controller_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-require 'rails_helper'
-
-RSpec.describe Admin::SubscriptionsController, type: :controller do
-  render_views
-
-  describe 'GET #index' do
-    around do |example|
-      default_per_page = Subscription.default_per_page
-      Subscription.paginates_per 1
-      example.run
-      Subscription.paginates_per default_per_page
-    end
-
-    before do
-      sign_in Fabricate(:user, admin: true), scope: :user
-    end
-
-    it 'renders subscriptions' do
-      Fabricate(:subscription)
-      specified = Fabricate(:subscription)
-
-      get :index
-
-      subscriptions = assigns(:subscriptions)
-      expect(subscriptions.count).to eq 1
-      expect(subscriptions[0]).to eq specified
-
-      expect(response).to have_http_status(200)
-    end
-  end
-end
-- 
cgit 


From 115dab78f1cc5357281dcb593f04ac8b2629cec6 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Mon, 5 Aug 2019 19:54:29 +0200
Subject: Change admin UI for hashtags and add back whitelisted trends (#11490)

Fix #271

Add back the `GET /api/v1/trends` API with the caveat that it does
not return tags that have not been allowed to trend by the staff.

When a hashtag begins to trend (internally) and that hashtag has
not been previously reviewed by the staff, the staff is notified.

The new admin UI for hashtags allows filtering hashtags by where
they are used (e.g. in the profile directory), whether they have
been reviewed or are pending reviewal, they show by how many people
the hashtag is used in the directory, how many people used it
today, how many statuses with it have been created today, and it
allows fixing the name of the hashtag to make it more readable.

The disallowed hashtags feature has been reworked. It is now
controlled from the admin UI for hashtags instead of from
the file `config/settings.yml`
---
 app/controllers/admin/dashboard_controller.rb      |  2 +-
 app/controllers/admin/tags_controller.rb           | 36 ++++++++-----
 app/controllers/api/v1/trends_controller.rb        | 17 ++++++
 app/controllers/settings/preferences_controller.rb |  2 +-
 app/helpers/admin/filter_helper.rb                 |  5 +-
 app/mailers/admin_mailer.rb                        | 10 ++++
 app/models/application_record.rb                   | 11 ++++
 app/models/tag.rb                                  | 60 +++++++++++++++++++---
 app/models/trending_tags.rb                        | 48 ++++++++---------
 app/models/user.rb                                 |  4 ++
 app/policies/tag_policy.rb                         |  4 +-
 app/validators/disallowed_hashtags_validator.rb    | 21 +-------
 app/views/admin/dashboard/index.html.haml          |  2 +-
 app/views/admin/tags/_tag.html.haml                | 24 +++++----
 app/views/admin/tags/index.html.haml               | 26 +++++-----
 app/views/admin/tags/show.html.haml                | 16 ++++++
 app/views/admin_mailer/new_trending_tag.text.erb   |  5 ++
 .../preferences/notifications/show.html.haml       |  1 +
 config/locales/en.yml                              | 18 ++++---
 config/locales/simple_form.en.yml                  |  7 +++
 config/navigation.rb                               |  2 +-
 config/routes.rb                                   |  9 +---
 config/settings.yml                                |  1 +
 .../20190805123746_add_capabilities_to_tags.rb     |  9 ++++
 db/schema.rb                                       |  7 ++-
 spec/controllers/admin/tags_controller_spec.rb     | 56 ++------------------
 spec/policies/tag_policy_spec.rb                   |  2 +-
 .../disallowed_hashtags_validator_spec.rb          | 26 +++++-----
 28 files changed, 258 insertions(+), 173 deletions(-)
 create mode 100644 app/controllers/api/v1/trends_controller.rb
 create mode 100644 app/views/admin/tags/show.html.haml
 create mode 100644 app/views/admin_mailer/new_trending_tag.text.erb
 create mode 100644 db/migrate/20190805123746_add_capabilities_to_tags.rb

(limited to 'spec/controllers/admin')

diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb
index e74e4755f..70afdedd7 100644
--- a/app/controllers/admin/dashboard_controller.rb
+++ b/app/controllers/admin/dashboard_controller.rb
@@ -27,7 +27,7 @@ module Admin
       @saml_enabled          = ENV['SAML_ENABLED'] == 'true'
       @pam_enabled           = ENV['PAM_ENABLED'] == 'true'
       @hidden_service        = ENV['ALLOW_ACCESS_TO_HIDDEN_SERVICE'] == 'true'
-      @trending_hashtags     = TrendingTags.get(7)
+      @trending_hashtags     = TrendingTags.get(10, filtered: false)
       @profile_directory     = Setting.profile_directory
       @timeline_preview      = Setting.timeline_preview
       @spam_check_enabled    = Setting.spam_check_enabled
diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb
index e9f4f2cfa..0e9dda302 100644
--- a/app/controllers/admin/tags_controller.rb
+++ b/app/controllers/admin/tags_controller.rb
@@ -4,41 +4,49 @@ module Admin
   class TagsController < BaseController
     before_action :set_tags, only: :index
     before_action :set_tag, except: :index
-    before_action :set_filter_params
 
     def index
       authorize :tag, :index?
     end
 
-    def hide
-      authorize @tag, :hide?
-      @tag.account_tag_stat.update!(hidden: true)
-      redirect_to admin_tags_path(@filter_params)
+    def show
+      authorize @tag, :show?
     end
 
-    def unhide
-      authorize @tag, :unhide?
-      @tag.account_tag_stat.update!(hidden: false)
-      redirect_to admin_tags_path(@filter_params)
+    def update
+      authorize @tag, :update?
+
+      if @tag.update(tag_params.merge(reviewed_at: Time.now.utc))
+        redirect_to admin_tag_path(@tag.id)
+      else
+        render :show
+      end
     end
 
     private
 
     def set_tags
-      @tags = Tag.discoverable
-      @tags.merge!(Tag.hidden) if filter_params[:hidden]
+      @tags = filtered_tags.page(params[:page])
     end
 
     def set_tag
       @tag = Tag.find(params[:id])
     end
 
-    def set_filter_params
-      @filter_params = filter_params.to_hash.symbolize_keys
+    def filtered_tags
+      scope = Tag
+      scope = scope.discoverable if filter_params[:context] == 'directory'
+      scope = scope.reviewed if filter_params[:review] == 'reviewed'
+      scope = scope.pending_review if filter_params[:review] == 'pending_review'
+      scope.reorder(score: :desc)
     end
 
     def filter_params
-      params.permit(:hidden)
+      params.slice(:context, :review).permit(:context, :review)
+    end
+
+    def tag_params
+      params.require(:tag).permit(:name, :trendable, :usable, :listable)
     end
   end
 end
diff --git a/app/controllers/api/v1/trends_controller.rb b/app/controllers/api/v1/trends_controller.rb
new file mode 100644
index 000000000..bcea9857e
--- /dev/null
+++ b/app/controllers/api/v1/trends_controller.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class Api::V1::TrendsController < Api::BaseController
+  before_action :set_tags
+
+  respond_to :json
+
+  def index
+    render json: @tags, each_serializer: REST::TagSerializer
+  end
+
+  private
+
+  def set_tags
+    @tags = TrendingTags.get(limit_param(10))
+  end
+end
diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb
index 742c97cdb..d548072a8 100644
--- a/app/controllers/settings/preferences_controller.rb
+++ b/app/controllers/settings/preferences_controller.rb
@@ -56,7 +56,7 @@ class Settings::PreferencesController < Settings::BaseController
       :setting_advanced_layout,
       :setting_use_blurhash,
       :setting_use_pending_items,
-      notification_emails: %i(follow follow_request reblog favourite mention digest report pending_account),
+      notification_emails: %i(follow follow_request reblog favourite mention digest report pending_account trending_tag),
       interactions: %i(must_be_follower must_be_following must_be_following_dm)
     )
   end
diff --git a/app/helpers/admin/filter_helper.rb b/app/helpers/admin/filter_helper.rb
index 0bda25974..506429e10 100644
--- a/app/helpers/admin/filter_helper.rb
+++ b/app/helpers/admin/filter_helper.rb
@@ -5,15 +5,16 @@ module Admin::FilterHelper
   REPORT_FILTERS       = %i(resolved account_id target_account_id).freeze
   INVITE_FILTER        = %i(available expired).freeze
   CUSTOM_EMOJI_FILTERS = %i(local remote by_domain shortcode).freeze
-  TAGS_FILTERS         = %i(hidden).freeze
+  TAGS_FILTERS         = %i(context review).freeze
   INSTANCES_FILTERS    = %i(limited by_domain).freeze
   FOLLOWERS_FILTERS    = %i(relationship status by_domain activity order).freeze
 
   FILTERS = ACCOUNT_FILTERS + REPORT_FILTERS + INVITE_FILTER + CUSTOM_EMOJI_FILTERS + TAGS_FILTERS + INSTANCES_FILTERS + FOLLOWERS_FILTERS
 
   def filter_link_to(text, link_to_params, link_class_params = link_to_params)
-    new_url = filtered_url_for(link_to_params)
+    new_url   = filtered_url_for(link_to_params)
     new_class = filtered_url_for(link_class_params)
+
     link_to text, new_url, class: filter_link_class(new_class)
   end
 
diff --git a/app/mailers/admin_mailer.rb b/app/mailers/admin_mailer.rb
index 9ab3e2bbd..8abce5f05 100644
--- a/app/mailers/admin_mailer.rb
+++ b/app/mailers/admin_mailer.rb
@@ -24,4 +24,14 @@ class AdminMailer < ApplicationMailer
       mail to: @me.user_email, subject: I18n.t('admin_mailer.new_pending_account.subject', instance: @instance, username: @account.username)
     end
   end
+
+  def new_trending_tag(recipient, tag)
+    @tag      = tag
+    @me       = recipient
+    @instance = Rails.configuration.x.local_domain
+
+    locale_for_account(@me) do
+      mail to: @me.user_email, subject: I18n.t('admin_mailer.new_trending_tag.subject', instance: @instance, name: @tag.name)
+    end
+  end
 end
diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 83134d41a..c1b873da6 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -2,5 +2,16 @@
 
 class ApplicationRecord < ActiveRecord::Base
   self.abstract_class = true
+
   include Remotable
+
+  def boolean_with_default(key, default_value)
+    value = attributes[key]
+
+    if value.nil?
+      default_value
+    else
+      value
+    end
+  end
 end
diff --git a/app/models/tag.rb b/app/models/tag.rb
index c7f0af86d..6a02581fa 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -3,11 +3,16 @@
 #
 # Table name: tags
 #
-#  id         :bigint(8)        not null, primary key
-#  name       :string           default(""), not null
-#  created_at :datetime         not null
-#  updated_at :datetime         not null
-#  score      :integer
+#  id                  :bigint(8)        not null, primary key
+#  name                :string           default(""), not null
+#  created_at          :datetime         not null
+#  updated_at          :datetime         not null
+#  score               :integer
+#  usable              :boolean
+#  trendable           :boolean
+#  listable            :boolean
+#  reviewed_at         :datetime
+#  requested_review_at :datetime
 #
 
 class Tag < ApplicationRecord
@@ -22,16 +27,17 @@ class Tag < ApplicationRecord
   HASHTAG_RE = /(?:^|[^\/\)\w])#(#{HASHTAG_NAME_RE})/i
 
   validates :name, presence: true, format: { with: /\A(#{HASHTAG_NAME_RE})\z/i }
+  validate :validate_name_change, if: -> { !new_record? && name_changed? }
 
-  scope :discoverable, -> { joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).where(account_tag_stats: { hidden: false }).order(Arel.sql('account_tag_stats.accounts_count desc')) }
-  scope :hidden, -> { where(account_tag_stats: { hidden: true }) }
+  scope :reviewed, -> { where.not(reviewed_at: nil) }
+  scope :pending_review, -> { where(reviewed_at: nil).where.not(requested_review_at: nil) }
+  scope :discoverable, -> { where.not(listable: false).joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) }
   scope :most_used, ->(account) { joins(:statuses).where(statuses: { account: account }).group(:id).order(Arel.sql('count(*) desc')) }
 
   delegate :accounts_count,
            :accounts_count=,
            :increment_count!,
            :decrement_count!,
-           :hidden?,
            to: :account_tag_stat
 
   after_save :save_account_tag_stat
@@ -48,6 +54,40 @@ class Tag < ApplicationRecord
     name
   end
 
+  def usable
+    boolean_with_default('usable', true)
+  end
+
+  alias usable? usable
+
+  def listable
+    boolean_with_default('listable', true)
+  end
+
+  alias listable? listable
+
+  def trendable
+    boolean_with_default('trendable', false)
+  end
+
+  alias trendable? trendable
+
+  def requires_review?
+    reviewed_at.nil?
+  end
+
+  def reviewed?
+    reviewed_at.present?
+  end
+
+  def requested_review?
+    requested_review_at.present?
+  end
+
+  def trending?
+    TrendingTags.trending?(self)
+  end
+
   def history
     days = []
 
@@ -117,4 +157,8 @@ class Tag < ApplicationRecord
     return unless account_tag_stat&.changed?
     account_tag_stat.save
   end
+
+  def validate_name_change
+    errors.add(:name, I18n.t('tags.does_not_match_previous_name')) unless name_was.mb_chars.casecmp(name.mb_chars).zero?
+  end
 end
diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb
index 211c8f1dc..e9b9b25e3 100644
--- a/app/models/trending_tags.rb
+++ b/app/models/trending_tags.rb
@@ -10,20 +10,28 @@ class TrendingTags
     include Redisable
 
     def record_use!(tag, account, at_time = Time.now.utc)
-      return if disallowed_hashtags.include?(tag.name) || account.silenced? || account.bot?
+      return if account.silenced? || account.bot? || !tag.usable? || !(tag.trendable? || tag.requires_review?)
 
       increment_historical_use!(tag.id, at_time)
       increment_unique_use!(tag.id, account.id, at_time)
-      increment_vote!(tag.id, at_time)
+      increment_vote!(tag, at_time)
     end
 
-    def get(limit)
-      key     = "#{KEY}:#{Time.now.utc.beginning_of_day.to_i}"
-      tag_ids = redis.zrevrange(key, 0, limit - 1).map(&:to_i)
-      tags    = Tag.where(id: tag_ids).to_a.each_with_object({}) { |tag, h| h[tag.id] = tag }
+    def get(limit, filtered: true)
+      tag_ids = redis.zrevrange("#{KEY}:#{Time.now.utc.beginning_of_day.to_i}", 0, limit - 1).map(&:to_i)
+
+      tags = Tag.where(id: tag_ids)
+      tags = tags.where(trendable: true) if filtered
+      tags = tags.each_with_object({}) { |tag, h| h[tag.id] = tag }
+
       tag_ids.map { |tag_id| tags[tag_id] }.compact
     end
 
+    def trending?(tag)
+      rank = redis.zrevrank("#{KEY}:#{Time.now.utc.beginning_of_day.to_i}", tag.id)
+      rank.present? && rank <= 10
+    end
+
     private
 
     def increment_historical_use!(tag_id, at_time)
@@ -38,33 +46,27 @@ class TrendingTags
       redis.expire(key, EXPIRE_HISTORY_AFTER)
     end
 
-    def increment_vote!(tag_id, at_time)
+    def increment_vote!(tag, at_time)
       key      = "#{KEY}:#{at_time.beginning_of_day.to_i}"
-      expected = redis.pfcount("activity:tags:#{tag_id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts").to_f
+      expected = redis.pfcount("activity:tags:#{tag.id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts").to_f
       expected = 1.0 if expected.zero?
-      observed = redis.pfcount("activity:tags:#{tag_id}:#{at_time.beginning_of_day.to_i}:accounts").to_f
+      observed = redis.pfcount("activity:tags:#{tag.id}:#{at_time.beginning_of_day.to_i}:accounts").to_f
 
       if expected > observed || observed < THRESHOLD
-        redis.zrem(key, tag_id.to_s)
+        redis.zrem(key, tag.id)
       else
-        score = ((observed - expected)**2) / expected
-        added = redis.zadd(key, score, tag_id.to_s)
-        bump_tag_score!(tag_id) if added
+        score    = ((observed - expected)**2) / expected
+        old_rank = redis.zrevrank(key, tag.id)
+
+        redis.zadd(key, score, tag.id)
+        request_review!(tag) if (old_rank.nil? || old_rank > 10) && redis.zrevrank(key, tag.id) <= 10 && !tag.trendable? && tag.requires_review? && !tag.requested_review?
       end
 
       redis.expire(key, EXPIRE_TRENDS_AFTER)
     end
 
-    def bump_tag_score!(tag_id)
-      Tag.where(id: tag_id).update_all('score = COALESCE(score, 0) + 1')
-    end
-
-    def disallowed_hashtags
-      return @disallowed_hashtags if defined?(@disallowed_hashtags)
-
-      @disallowed_hashtags = Setting.disallowed_hashtags.nil? ? [] : Setting.disallowed_hashtags
-      @disallowed_hashtags = @disallowed_hashtags.split(' ') if @disallowed_hashtags.is_a? String
-      @disallowed_hashtags = @disallowed_hashtags.map(&:downcase)
+    def request_review!(tag)
+      User.staff.includes(:account).find_each { |u| AdminMailer.new_trending_tag(u.account, tag).deliver_later! if u.allows_trending_tag_emails? }
     end
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6806c0362..b83e26af3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -207,6 +207,10 @@ class User < ApplicationRecord
     settings.notification_emails['pending_account']
   end
 
+  def allows_trending_tag_emails?
+    settings.notification_emails['trending_tag']
+  end
+
   def hides_network?
     @hides_network ||= settings.hide_network
   end
diff --git a/app/policies/tag_policy.rb b/app/policies/tag_policy.rb
index c63de01db..aaf70fcab 100644
--- a/app/policies/tag_policy.rb
+++ b/app/policies/tag_policy.rb
@@ -5,11 +5,11 @@ class TagPolicy < ApplicationPolicy
     staff?
   end
 
-  def hide?
+  def show?
     staff?
   end
 
-  def unhide?
+  def update?
     staff?
   end
 end
diff --git a/app/validators/disallowed_hashtags_validator.rb b/app/validators/disallowed_hashtags_validator.rb
index ee06b20f6..d745b767f 100644
--- a/app/validators/disallowed_hashtags_validator.rb
+++ b/app/validators/disallowed_hashtags_validator.rb
@@ -4,24 +4,7 @@ class DisallowedHashtagsValidator < ActiveModel::Validator
   def validate(status)
     return unless status.local? && !status.reblog?
 
-    @status = status
-    tags    = select_tags
-
-    status.errors.add(:text, I18n.t('statuses.disallowed_hashtags', tags: tags.join(', '), count: tags.size)) unless tags.empty?
-  end
-
-  private
-
-  def select_tags
-    tags = Extractor.extract_hashtags(@status.text)
-    tags.keep_if { |tag| disallowed_hashtags.include? tag.downcase }
-  end
-
-  def disallowed_hashtags
-    return @disallowed_hashtags if @disallowed_hashtags
-
-    @disallowed_hashtags = Setting.disallowed_hashtags.nil? ? [] : Setting.disallowed_hashtags
-    @disallowed_hashtags = @disallowed_hashtags.split(' ') if @disallowed_hashtags.is_a? String
-    @disallowed_hashtags = @disallowed_hashtags.map(&:downcase)
+    disallowed_hashtags = Tag.matching_name(Extractor.extract_hashtags(status.text)).reject(&:usable?)
+    status.errors.add(:text, I18n.t('statuses.disallowed_hashtags', tags: disallowed_hashtags.map(&:name).join(', '), count: disallowed_hashtags.size)) unless disallowed_hashtags.empty?
   end
 end
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 77cc1a2a0..910896075 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -107,5 +107,5 @@
       %ul
         - @trending_hashtags.each do |tag|
           %li
-            = link_to "##{tag.name}", web_url("timelines/tag/#{tag.name}")
+            = link_to content_tag(:span, "##{tag.name}", class: !tag.trendable? && !tag.reviewed? ? 'warning-hint' : (!tag.trendable? ? 'negative-hint' : nil)), admin_tag_path(tag.id)
             %span.pull-right= number_with_delimiter(tag.history[0][:accounts].to_i)
diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml
index 961b83f93..91af8e492 100644
--- a/app/views/admin/tags/_tag.html.haml
+++ b/app/views/admin/tags/_tag.html.haml
@@ -1,12 +1,16 @@
-%tr
-  %td
-    = link_to explore_hashtag_path(tag) do
+.directory__tag
+  = link_to admin_tag_path(tag.id) do
+    %h4
       = fa_icon 'hashtag'
       = tag.name
-  %td
-    = t('directories.people', count: tag.accounts_count)
-  %td
-    - if tag.hidden?
-      = table_link_to 'eye', t('admin.tags.unhide'), unhide_admin_tag_path(tag.id, **@filter_params), method: :post
-    - else
-      = table_link_to 'eye-slash', t('admin.tags.hide'), hide_admin_tag_path(tag.id, **@filter_params), method: :post
+
+      %small
+        = t('admin.tags.in_directory', count: tag.accounts_count)
+        &bull;
+        = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts])
+
+        - if tag.trending?
+          = fa_icon 'fire fw'
+          = t('admin.tags.trending_right_now')
+
+    .trends__item__current= number_to_human tag.history.first[:uses], strip_insignificant_zeros: true
diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml
index 4ba395860..5e4ee21f5 100644
--- a/app/views/admin/tags/index.html.haml
+++ b/app/views/admin/tags/index.html.haml
@@ -3,17 +3,19 @@
 
 .filters
   .filter-subset
-    %strong= t('admin.reports.status')
+    %strong= t('admin.tags.context')
     %ul
-      %li= filter_link_to t('admin.tags.visible'), hidden: nil
-      %li= filter_link_to t('admin.tags.hidden'), hidden: '1'
+      %li= filter_link_to t('generic.all'), context: nil
+      %li= filter_link_to t('admin.tags.directory'), context: 'directory'
 
-.table-wrapper
-  %table.table
-    %thead
-      %tr
-        %th= t('admin.tags.name')
-        %th= t('admin.tags.accounts')
-        %th
-    %tbody
-      = render @tags
+  .filter-subset
+    %strong= t('admin.tags.review')
+    %ul
+      %li= filter_link_to t('generic.all'), review: nil
+      %li= filter_link_to t('admin.tags.reviewed'), review: 'reviewed'
+      %li= filter_link_to safe_join([t('admin.accounts.moderation.pending'), "(#{Tag.pending_review.count})"], ' '), review: 'pending_review'
+
+%hr.spacer/
+
+= render @tags
+= paginate @tags
diff --git a/app/views/admin/tags/show.html.haml b/app/views/admin/tags/show.html.haml
new file mode 100644
index 000000000..27c8dc92b
--- /dev/null
+++ b/app/views/admin/tags/show.html.haml
@@ -0,0 +1,16 @@
+- content_for :page_title do
+  = "##{@tag.name}"
+
+= simple_form_for @tag, url: admin_tag_path(@tag.id) do |f|
+  = render 'shared/error_messages', object: @tag
+
+  .fields-group
+    = f.input :name, wrapper: :with_block_label
+
+  .fields-group
+    = f.input :usable, as: :boolean, wrapper: :with_label
+    = f.input :trendable, as: :boolean, wrapper: :with_label
+    = f.input :listable, as: :boolean, wrapper: :with_label
+
+  .actions
+    = f.button :button, t('generic.save_changes'), type: :submit
diff --git a/app/views/admin_mailer/new_trending_tag.text.erb b/app/views/admin_mailer/new_trending_tag.text.erb
new file mode 100644
index 000000000..f3087df37
--- /dev/null
+++ b/app/views/admin_mailer/new_trending_tag.text.erb
@@ -0,0 +1,5 @@
+<%= raw t('application_mailer.salutation', name: display_name(@me)) %>
+
+<%= raw t('admin_mailer.new_trending_tag.body', name: @tag.name) %>
+
+<%= raw t('application_mailer.view')%> <%= admin_tags_url(review: 'pending_review') %>
diff --git a/app/views/settings/preferences/notifications/show.html.haml b/app/views/settings/preferences/notifications/show.html.haml
index acc646fc3..f666ae4ff 100644
--- a/app/views/settings/preferences/notifications/show.html.haml
+++ b/app/views/settings/preferences/notifications/show.html.haml
@@ -15,6 +15,7 @@
       - if current_user.staff?
         = ff.input :report, as: :boolean, wrapper: :with_label
         = ff.input :pending_account, as: :boolean, wrapper: :with_label
+        = ff.input :trending_tag, as: :boolean, wrapper: :with_label
 
   .fields-group
     = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff|
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 6c1a34300..9b62aac3a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -483,13 +483,14 @@ en:
       title: Account statuses
       with_media: With media
     tags:
-      accounts: Accounts
-      hidden: Hidden
-      hide: Hide from directory
-      name: Hashtag
+      context: Context
+      directory: In directory
+      in_directory: "%{count} in directory"
+      review: Review status
+      reviewed: Reviewed
       title: Hashtags
-      unhide: Show in directory
-      visible: Visible
+      trending_right_now: Trending right now
+      unique_uses_today: "%{count} posting today"
     title: Administration
     warning_presets:
       add_new: Add new
@@ -505,6 +506,9 @@ en:
       body: "%{reporter} has reported %{target}"
       body_remote: Someone from %{domain} has reported %{target}
       subject: New report for %{instance} (#%{id})
+    new_trending_tag:
+      body: 'The hashtag #%{name} is trending today, but has not been previously reviewed. It will not be displayed publicly unless you allow it to, or just save the form as it is to never hear about it again.'
+      subject: New hashtag up for review on %{instance} (#%{name})
   appearance:
     advanced_web_interface: Advanced web interface
     advanced_web_interface_hint: 'If you want to make use of your entire screen width, the advanced web interface allows you to configure many different columns to see as much information at the same time as you want: Home, notifications, federated timeline, any number of lists and hashtags.'
@@ -939,6 +943,8 @@ en:
     pinned: Pinned toot
     reblogged: boosted
     sensitive_content: Sensitive content
+  tags:
+    does_not_match_previous_name: does not match the previous name
   terms:
     body_html: |
       <h2>Privacy Policy</h2>
diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml
index 10b30e627..6fdfc9d7b 100644
--- a/config/locales/simple_form.en.yml
+++ b/config/locales/simple_form.en.yml
@@ -48,6 +48,8 @@ en:
         text: This will help us review your application
       sessions:
         otp: 'Enter the two-factor code generated by your phone app or use one of your recovery codes:'
+      tag:
+        name: You can only change the casing of the letters, for example, to make it more readable
       user:
         chosen_languages: When checked, only toots in selected languages will be displayed in public timelines
     labels:
@@ -137,6 +139,11 @@ en:
         pending_account: Send e-mail when a new account needs review
         reblog: Send e-mail when someone boosts your status
         report: Send e-mail when a new report is submitted
+        trending_tag: Send e-mail when an unreviewed hashtag is trending
+      tag:
+        listable: Allow this hashtag to appear on the profile directory
+        trendable: Allow this hashtag to appear under trends
+        usable: Allow toots to use this hashtag
     'no': 'No'
     recommended: Recommended
     required:
diff --git a/config/navigation.rb b/config/navigation.rb
index 9b46da603..38668bbf7 100644
--- a/config/navigation.rb
+++ b/config/navigation.rb
@@ -38,7 +38,7 @@ SimpleNavigation::Configuration.run do |navigation|
       s.item :reports, safe_join([fa_icon('flag fw'), t('admin.reports.title')]), admin_reports_url, highlights_on: %r{/admin/reports}
       s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_url, highlights_on: %r{/admin/accounts|/admin/pending_accounts}
       s.item :invites, safe_join([fa_icon('user-plus fw'), t('admin.invites.title')]), admin_invites_path
-      s.item :tags, safe_join([fa_icon('tag fw'), t('admin.tags.title')]), admin_tags_path
+      s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.tags.title')]), admin_tags_path, highlights_on: %r{/admin/tags}
       s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_url(limited: whitelist_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.admin? }
       s.item :email_domain_blocks, safe_join([fa_icon('envelope fw'), t('admin.email_domain_blocks.title')]), admin_email_domain_blocks_url, highlights_on: %r{/admin/email_domain_blocks}, if: -> { current_user.admin? }
     end
diff --git a/config/routes.rb b/config/routes.rb
index 04424bbbd..60f7d2e05 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -243,13 +243,7 @@ Rails.application.routes.draw do
     end
 
     resources :account_moderation_notes, only: [:create, :destroy]
-
-    resources :tags, only: [:index] do
-      member do
-        post :hide
-        post :unhide
-      end
-    end
+    resources :tags, only: [:index, :show, :update]
   end
 
   get '/admin', to: redirect('/admin/dashboard', status: 302)
@@ -311,6 +305,7 @@ Rails.application.routes.draw do
       resources :mutes,        only: [:index]
       resources :favourites,   only: [:index]
       resources :reports,      only: [:create]
+      resources :trends,       only: [:index]
       resources :filters,      only: [:index, :create, :show, :update, :destroy]
       resources :endorsements, only: [:index]
 
diff --git a/config/settings.yml b/config/settings.yml
index ad2970bb7..10180201f 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -43,6 +43,7 @@ defaults: &defaults
     digest: true
     report: true
     pending_account: true
+    trending_tag: true
   interactions:
     must_be_follower: false
     must_be_following: false
diff --git a/db/migrate/20190805123746_add_capabilities_to_tags.rb b/db/migrate/20190805123746_add_capabilities_to_tags.rb
new file mode 100644
index 000000000..43c7763b1
--- /dev/null
+++ b/db/migrate/20190805123746_add_capabilities_to_tags.rb
@@ -0,0 +1,9 @@
+class AddCapabilitiesToTags < ActiveRecord::Migration[5.2]
+  def change
+    add_column :tags, :usable, :boolean
+    add_column :tags, :trendable, :boolean
+    add_column :tags, :listable, :boolean
+    add_column :tags, :reviewed_at, :datetime
+    add_column :tags, :requested_review_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index e3af9c31a..d1b6825b4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_07_29_185330) do
+ActiveRecord::Schema.define(version: 2019_08_05_123746) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -660,6 +660,11 @@ ActiveRecord::Schema.define(version: 2019_07_29_185330) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.integer "score"
+    t.boolean "usable"
+    t.boolean "trendable"
+    t.boolean "listable"
+    t.datetime "reviewed_at"
+    t.datetime "requested_review_at"
     t.index "lower((name)::text)", name: "index_tags_on_name_lower", unique: true
   end
 
diff --git a/spec/controllers/admin/tags_controller_spec.rb b/spec/controllers/admin/tags_controller_spec.rb
index 3af994071..5c1944fc7 100644
--- a/spec/controllers/admin/tags_controller_spec.rb
+++ b/spec/controllers/admin/tags_controller_spec.rb
@@ -10,62 +10,14 @@ RSpec.describe Admin::TagsController, type: :controller do
   end
 
   describe 'GET #index' do
-    before do
-      account_tag_stat = Fabricate(:tag).account_tag_stat
-      account_tag_stat.update(hidden: hidden, accounts_count: 1)
-      get :index, params: { hidden: hidden }
-    end
-
-    context 'with hidden tags' do
-      let(:hidden) { true }
-
-      it 'returns status 200' do
-        expect(response).to have_http_status(200)
-      end
-    end
-
-    context 'without hidden tags' do
-      let(:hidden) { false }
-
-      it 'returns status 200' do
-        expect(response).to have_http_status(200)
-      end
-    end
-  end
-
-  describe 'POST #hide' do
-    let(:tag) { Fabricate(:tag) }
+    let!(:tag) { Fabricate(:tag) }
 
     before do
-      tag.account_tag_stat.update(hidden: false)
-      post :hide, params: { id: tag.id }
-    end
-
-    it 'hides tag' do
-      tag.reload
-      expect(tag).to be_hidden
-    end
-
-    it 'redirects to admin_tags_path' do
-      expect(response).to redirect_to(admin_tags_path(controller.instance_variable_get(:@filter_params)))
-    end
-  end
-
-  describe 'POST #unhide' do
-    let(:tag) { Fabricate(:tag) }
-
-    before do
-      tag.account_tag_stat.update(hidden: true)
-      post :unhide, params: { id: tag.id }
-    end
-
-    it 'unhides tag' do
-      tag.reload
-      expect(tag).not_to be_hidden
+      get :index
     end
 
-    it 'redirects to admin_tags_path' do
-      expect(response).to redirect_to(admin_tags_path(controller.instance_variable_get(:@filter_params)))
+    it 'returns status 200' do
+      expect(response).to have_http_status(200)
     end
   end
 end
diff --git a/spec/policies/tag_policy_spec.rb b/spec/policies/tag_policy_spec.rb
index c7afaa7c9..c63875dc0 100644
--- a/spec/policies/tag_policy_spec.rb
+++ b/spec/policies/tag_policy_spec.rb
@@ -8,7 +8,7 @@ RSpec.describe TagPolicy do
   let(:admin)   { Fabricate(:user, admin: true).account }
   let(:john)    { Fabricate(:user).account }
 
-  permissions :index?, :hide?, :unhide? do
+  permissions :index?, :show?, :update? do
     context 'staff?' do
       it 'permits' do
         expect(subject).to permit(admin, Tag)
diff --git a/spec/validators/disallowed_hashtags_validator_spec.rb b/spec/validators/disallowed_hashtags_validator_spec.rb
index 8ec1302ab..9deec0bb9 100644
--- a/spec/validators/disallowed_hashtags_validator_spec.rb
+++ b/spec/validators/disallowed_hashtags_validator_spec.rb
@@ -3,42 +3,44 @@
 require 'rails_helper'
 
 RSpec.describe DisallowedHashtagsValidator, type: :validator do
+  let(:disallowed_tags) { [] }
+
   describe '#validate' do
     before do
-      allow_any_instance_of(described_class).to receive(:select_tags) { tags }
+      disallowed_tags.each { |name| Fabricate(:tag, name: name, usable: false) }
       described_class.new.validate(status)
     end
 
-    let(:status) { double(errors: errors, local?: local, reblog?: reblog, text: '') }
+    let(:status) { double(errors: errors, local?: local, reblog?: reblog, text: disallowed_tags.map { |x| '#' + x }.join(' ')) }
     let(:errors) { double(add: nil) }
 
-    context 'unless status.local? && !status.reblog?' do
+    context 'for a remote reblog' do
       let(:local)  { false }
       let(:reblog) { true }
 
-      it 'not calls errors.add' do
+      it 'does not add errors' do
         expect(errors).not_to have_received(:add).with(:text, any_args)
       end
     end
 
-    context 'status.local? && !status.reblog?' do
+    context 'for a local original status' do
       let(:local)  { true }
       let(:reblog) { false }
 
-      context 'tags.empty?' do
-        let(:tags) { [] }
+      context 'when does not contain any disallowed hashtags' do
+        let(:disallowed_tags) { [] }
 
-        it 'not calls errors.add' do
+        it 'does not add errors' do
           expect(errors).not_to have_received(:add).with(:text, any_args)
         end
       end
 
-      context '!tags.empty?' do
-        let(:tags) { %w(a b c) }
+      context 'when contains disallowed hashtags' do
+        let(:disallowed_tags) { %w(a b c) }
 
-        it 'calls errors.add' do
+        it 'adds an error' do
           expect(errors).to have_received(:add)
-            .with(:text, I18n.t('statuses.disallowed_hashtags', tags: tags.join(', '), count: tags.size))
+            .with(:text, I18n.t('statuses.disallowed_hashtags', tags: disallowed_tags.join(', '), count: disallowed_tags.size))
         end
       end
     end
-- 
cgit 


From 97192d9a77c0b4b68afe50d6a94d87110a8adbcd Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Thu, 22 Aug 2019 04:17:12 +0200
Subject: Fix remote and staff-removed statuses leaving media behind for a day
 (#11638)

The reason for unattaching media instead of removing it is to support
delete & redraft functionality, but remote or staff-removed statuses
will never be redrafted, so the media should be deleted immediately
---
 app/controllers/api/v1/statuses_controller.rb               |  2 +-
 app/lib/activitypub/activity/delete.rb                      |  2 +-
 app/models/form/status_batch.rb                             |  2 +-
 app/services/batched_remove_status_service.rb               |  2 +-
 app/services/remove_status_service.rb                       | 12 ++++++++++++
 app/workers/removal_worker.rb                               |  4 ++--
 spec/controllers/admin/reported_statuses_controller_spec.rb |  2 +-
 spec/controllers/admin/statuses_controller_spec.rb          |  2 +-
 spec/models/form/status_batch_spec.rb                       |  4 ++--
 9 files changed, 22 insertions(+), 10 deletions(-)

(limited to 'spec/controllers/admin')

diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb
index 71a505c26..39ca56482 100644
--- a/app/controllers/api/v1/statuses_controller.rb
+++ b/app/controllers/api/v1/statuses_controller.rb
@@ -53,7 +53,7 @@ class Api::V1::StatusesController < Api::BaseController
     @status = Status.where(account_id: current_user.account).find(params[:id])
     authorize @status, :destroy?
 
-    RemovalWorker.perform_async(@status.id)
+    RemovalWorker.perform_async(@status.id, redraft: true)
 
     render json: @status, serializer: REST::StatusSerializer, source_requested: true
   end
diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb
index 1f2b40c15..345060462 100644
--- a/app/lib/activitypub/activity/delete.rb
+++ b/app/lib/activitypub/activity/delete.rb
@@ -70,7 +70,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity
   end
 
   def delete_now!
-    RemoveStatusService.new.call(@status)
+    RemoveStatusService.new.call(@status, redraft: false)
   end
 
   def payload
diff --git a/app/models/form/status_batch.rb b/app/models/form/status_batch.rb
index 933dfdaca..831d8b7c5 100644
--- a/app/models/form/status_batch.rb
+++ b/app/models/form/status_batch.rb
@@ -34,7 +34,7 @@ class Form::StatusBatch
 
   def delete_statuses
     Status.where(id: status_ids).reorder(nil).find_each do |status|
-      RemovalWorker.perform_async(status.id)
+      RemovalWorker.perform_async(status.id, redraft: false)
       Tombstone.find_or_create_by(uri: status.uri, account: status.account, by_moderator: true)
       log_action :destroy, status
     end
diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb
index 6df8d4769..3638134be 100644
--- a/app/services/batched_remove_status_service.rb
+++ b/app/services/batched_remove_status_service.rb
@@ -8,7 +8,7 @@ class BatchedRemoveStatusService < BaseService
   # Dispatch Salmon deletes, unique per domain, of the deleted statuses, but only local ones
   # Remove statuses from home feeds
   # Push delete events to streaming API for home feeds and public feeds
-  # @param [Status] statuses A preferably batched array of statuses
+  # @param [Enumerable<Status>] statuses A preferably batched array of statuses
   # @param [Hash] options
   # @option [Boolean] :skip_side_effects
   def call(statuses, **options)
diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb
index 91c934181..685c1d4bf 100644
--- a/app/services/remove_status_service.rb
+++ b/app/services/remove_status_service.rb
@@ -4,6 +4,11 @@ class RemoveStatusService < BaseService
   include Redisable
   include Payloadable
 
+  # Delete a status
+  # @param   [Status] status
+  # @param   [Hash] options
+  # @option  [Boolean] :redraft
+  # @options [Boolean] :original_removed
   def call(status, **options)
     @payload  = Oj.dump(event: :delete, payload: status.id.to_s)
     @status   = status
@@ -24,6 +29,7 @@ class RemoveStatusService < BaseService
         remove_from_public
         remove_from_media if status.media_attachments.any?
         remove_from_spam_check
+        remove_media
 
         @status.destroy!
       else
@@ -143,6 +149,12 @@ class RemoveStatusService < BaseService
     redis.publish('timeline:public:local:media', @payload) if @status.local?
   end
 
+  def remove_media
+    return if @options[:redraft]
+
+    @status.media_attachments.destroy_all
+  end
+
   def remove_from_spam_check
     redis.zremrangebyscore("spam_check:#{@status.account_id}", @status.id, @status.id)
   end
diff --git a/app/workers/removal_worker.rb b/app/workers/removal_worker.rb
index 19a660dd3..14423a4fb 100644
--- a/app/workers/removal_worker.rb
+++ b/app/workers/removal_worker.rb
@@ -3,8 +3,8 @@
 class RemovalWorker
   include Sidekiq::Worker
 
-  def perform(status_id)
-    RemoveStatusService.new.call(Status.find(status_id))
+  def perform(status_id, options = {})
+    RemoveStatusService.new.call(Status.find(status_id), **options.symbolize_keys)
   rescue ActiveRecord::RecordNotFound
     true
   end
diff --git a/spec/controllers/admin/reported_statuses_controller_spec.rb b/spec/controllers/admin/reported_statuses_controller_spec.rb
index c358506d6..bd146b795 100644
--- a/spec/controllers/admin/reported_statuses_controller_spec.rb
+++ b/spec/controllers/admin/reported_statuses_controller_spec.rb
@@ -47,7 +47,7 @@ describe Admin::ReportedStatusesController do
       it 'removes a status' do
         allow(RemovalWorker).to receive(:perform_async)
         subject.call
-        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first)
+        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, redraft: false)
       end
     end
 
diff --git a/spec/controllers/admin/statuses_controller_spec.rb b/spec/controllers/admin/statuses_controller_spec.rb
index 1a08c10b7..6b06343ef 100644
--- a/spec/controllers/admin/statuses_controller_spec.rb
+++ b/spec/controllers/admin/statuses_controller_spec.rb
@@ -65,7 +65,7 @@ describe Admin::StatusesController do
       it 'removes a status' do
         allow(RemovalWorker).to receive(:perform_async)
         subject.call
-        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first)
+        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, redraft: false)
       end
     end
 
diff --git a/spec/models/form/status_batch_spec.rb b/spec/models/form/status_batch_spec.rb
index 00c790a11..f9c58c90f 100644
--- a/spec/models/form/status_batch_spec.rb
+++ b/spec/models/form/status_batch_spec.rb
@@ -41,12 +41,12 @@ describe Form::StatusBatch do
 
     it 'call RemovalWorker' do
       form.save
-      expect(RemovalWorker).to have_received(:perform_async).with(status.id)
+      expect(RemovalWorker).to have_received(:perform_async).with(status.id, redraft: false)
     end
 
     it 'do not call RemovalWorker' do
       form.save
-      expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id)
+      expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id, redraft: false)
     end
   end
 end
-- 
cgit 


From 1110ea1a9162d5488e1ed5dbccd0803618e713f8 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Mon, 9 Sep 2019 22:44:17 +0200
Subject: Add batch actions and categories to admin UI for custom emojis
 (#11793)

---
 app/controllers/admin/custom_emojis_controller.rb  | 102 +++++++-------------
 app/javascript/styles/mastodon/tables.scss         |  41 ++++++++
 app/models/custom_emoji.rb                         |   6 ++
 app/models/custom_emoji_category.rb                |   2 +
 app/models/custom_emoji_filter.rb                  |   8 +-
 app/models/form/custom_emoji_batch.rb              | 106 +++++++++++++++++++++
 .../admin/custom_emojis/_custom_emoji.html.haml    |  55 ++++++-----
 app/views/admin/custom_emojis/index.html.haml      |  66 ++++++++++---
 config/locales/en.yml                              |   3 +
 config/routes.rb                                   |   8 +-
 .../admin/custom_emojis_controller_spec.rb         |  60 ------------
 11 files changed, 281 insertions(+), 176 deletions(-)
 create mode 100644 app/models/form/custom_emoji_batch.rb

(limited to 'spec/controllers/admin')

diff --git a/app/controllers/admin/custom_emojis_controller.rb b/app/controllers/admin/custom_emojis_controller.rb
index f77699166..2af90f051 100644
--- a/app/controllers/admin/custom_emojis_controller.rb
+++ b/app/controllers/admin/custom_emojis_controller.rb
@@ -2,19 +2,20 @@
 
 module Admin
   class CustomEmojisController < BaseController
-    before_action :set_custom_emoji, except: [:index, :new, :create]
-    before_action :set_filter_params
-
     include ObfuscateFilename
+
     obfuscate_filename [:custom_emoji, :image]
 
     def index
       authorize :custom_emoji, :index?
+
       @custom_emojis = filtered_custom_emojis.eager_load(:local_counterpart).page(params[:page])
+      @form          = Form::CustomEmojiBatch.new
     end
 
     def new
       authorize :custom_emoji, :create?
+
       @custom_emoji = CustomEmoji.new
     end
 
@@ -31,69 +32,17 @@ module Admin
       end
     end
 
-    def update
-      authorize @custom_emoji, :update?
-
-      if @custom_emoji.update(resource_params)
-        log_action :update, @custom_emoji
-        flash[:notice] = I18n.t('admin.custom_emojis.updated_msg')
-      else
-        flash[:alert] =  I18n.t('admin.custom_emojis.update_failed_msg')
-      end
-      redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params)
-    end
-
-    def destroy
-      authorize @custom_emoji, :destroy?
-      @custom_emoji.destroy!
-      log_action :destroy, @custom_emoji
-      flash[:notice] = I18n.t('admin.custom_emojis.destroyed_msg')
-      redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params)
-    end
-
-    def copy
-      authorize @custom_emoji, :copy?
-
-      emoji = CustomEmoji.find_or_initialize_by(domain: nil,
-                                                shortcode: @custom_emoji.shortcode)
-      emoji.image = @custom_emoji.image
-
-      if emoji.save
-        log_action :create, emoji
-        flash[:notice] = I18n.t('admin.custom_emojis.copied_msg')
-      else
-        flash[:alert] = I18n.t('admin.custom_emojis.copy_failed_msg')
-      end
-
-      redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params)
-    end
-
-    def enable
-      authorize @custom_emoji, :enable?
-      @custom_emoji.update!(disabled: false)
-      log_action :enable, @custom_emoji
-      flash[:notice] = I18n.t('admin.custom_emojis.enabled_msg')
-      redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params)
-    end
-
-    def disable
-      authorize @custom_emoji, :disable?
-      @custom_emoji.update!(disabled: true)
-      log_action :disable, @custom_emoji
-      flash[:notice] = I18n.t('admin.custom_emojis.disabled_msg')
-      redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params)
+    def batch
+      @form = Form::CustomEmojiBatch.new(form_custom_emoji_batch_params.merge(current_account: current_account, action: action_from_button))
+      @form.save
+    rescue ActionController::ParameterMissing
+      flash[:alert] = I18n.t('admin.accounts.no_account_selected')
+    ensure
+      redirect_to admin_custom_emojis_path(filter_params)
     end
 
     private
 
-    def set_custom_emoji
-      @custom_emoji = CustomEmoji.find(params[:id])
-    end
-
-    def set_filter_params
-      @filter_params = filter_params.to_hash.symbolize_keys
-    end
-
     def resource_params
       params.require(:custom_emoji).permit(:shortcode, :image, :visible_in_picker)
     end
@@ -103,12 +52,29 @@ module Admin
     end
 
     def filter_params
-      params.permit(
-        :local,
-        :remote,
-        :by_domain,
-        :shortcode
-      )
+      params.slice(:local, :remote, :by_domain, :shortcode, :page).permit(:local, :remote, :by_domain, :shortcode, :page)
+    end
+
+    def action_from_button
+      if params[:update]
+        'update'
+      elsif params[:list]
+        'list'
+      elsif params[:unlist]
+        'unlist'
+      elsif params[:enable]
+        'enable'
+      elsif params[:disable]
+        'disable'
+      elsif params[:copy]
+        'copy'
+      elsif params[:delete]
+        'delete'
+      end
+    end
+
+    def form_custom_emoji_batch_params
+      params.require(:form_custom_emoji_batch).permit(:action, :category_id, :category_name, custom_emoji_ids: [])
     end
   end
 end
diff --git a/app/javascript/styles/mastodon/tables.scss b/app/javascript/styles/mastodon/tables.scss
index 2aef099e6..d6403986f 100644
--- a/app/javascript/styles/mastodon/tables.scss
+++ b/app/javascript/styles/mastodon/tables.scss
@@ -180,6 +180,18 @@ a.table-action-link {
     }
   }
 
+  &__form {
+    padding: 16px;
+    border: 1px solid darken($ui-base-color, 8%);
+    border-top: 0;
+    background: $ui-base-color;
+
+    .fields-row {
+      padding-top: 0;
+      margin-bottom: 0;
+    }
+  }
+
   &__row {
     border: 1px solid darken($ui-base-color, 8%);
     border-top: 0;
@@ -210,6 +222,35 @@ a.table-action-link {
       &--unpadded {
         padding: 0;
       }
+
+      &--with-image {
+        display: flex;
+        align-items: center;
+      }
+
+      &__image {
+        flex: 0 0 auto;
+        display: flex;
+        justify-content: center;
+        align-items: center;
+        margin-right: 10px;
+
+        .emojione {
+          width: 32px;
+          height: 32px;
+        }
+      }
+
+      &__text {
+        flex: 1 1 auto;
+      }
+
+      &__extra {
+        flex: 0 0 auto;
+        text-align: right;
+        color: $darker-text-color;
+        font-weight: 500;
+      }
     }
 
     .directory__tag {
diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb
index b21ad9042..0a4201a14 100644
--- a/app/models/custom_emoji.rb
+++ b/app/models/custom_emoji.rb
@@ -59,6 +59,12 @@ class CustomEmoji < ApplicationRecord
     :emoji
   end
 
+  def copy!
+    copy = self.class.find_or_initialize_by(domain: nil, shortcode: shortcode)
+    copy.image = image
+    copy.save!
+  end
+
   class << self
     def from_text(text, domain)
       return [] if text.blank?
diff --git a/app/models/custom_emoji_category.rb b/app/models/custom_emoji_category.rb
index 7d8c0ee2d..3c87f2b2e 100644
--- a/app/models/custom_emoji_category.rb
+++ b/app/models/custom_emoji_category.rb
@@ -12,4 +12,6 @@
 
 class CustomEmojiCategory < ApplicationRecord
   has_many :emojis, class_name: 'CustomEmoji', foreign_key: 'category_id', inverse_of: :category
+
+  validates :name, presence: true, uniqueness: true
 end
diff --git a/app/models/custom_emoji_filter.rb b/app/models/custom_emoji_filter.rb
index 7649055d2..15b8da1d1 100644
--- a/app/models/custom_emoji_filter.rb
+++ b/app/models/custom_emoji_filter.rb
@@ -11,6 +11,8 @@ class CustomEmojiFilter
     scope = CustomEmoji.alphabetic
 
     params.each do |key, value|
+      next if key.to_s == 'page'
+
       scope.merge!(scope_for(key, value)) if value.present?
     end
 
@@ -22,13 +24,13 @@ class CustomEmojiFilter
   def scope_for(key, value)
     case key.to_s
     when 'local'
-      CustomEmoji.local
+      CustomEmoji.local.left_joins(:category).reorder(Arel.sql('custom_emoji_categories.name ASC NULLS FIRST, custom_emojis.shortcode ASC'))
     when 'remote'
       CustomEmoji.remote
     when 'by_domain'
-      CustomEmoji.where(domain: value.downcase)
+      CustomEmoji.where(domain: value.strip.downcase)
     when 'shortcode'
-      CustomEmoji.search(value)
+      CustomEmoji.search(value.strip)
     else
       raise "Unknown filter: #{key}"
     end
diff --git a/app/models/form/custom_emoji_batch.rb b/app/models/form/custom_emoji_batch.rb
new file mode 100644
index 000000000..076e8c9e3
--- /dev/null
+++ b/app/models/form/custom_emoji_batch.rb
@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+class Form::CustomEmojiBatch
+  include ActiveModel::Model
+  include Authorization
+  include AccountableConcern
+
+  attr_accessor :custom_emoji_ids, :action, :current_account,
+                :category_id, :category_name, :visible_in_picker
+
+  def save
+    case action
+    when 'update'
+      update!
+    when 'list'
+      list!
+    when 'unlist'
+      unlist!
+    when 'enable'
+      enable!
+    when 'disable'
+      disable!
+    when 'copy'
+      copy!
+    when 'delete'
+      delete!
+    end
+  end
+
+  private
+
+  def custom_emojis
+    CustomEmoji.where(id: custom_emoji_ids)
+  end
+
+  def update!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) }
+
+    category = begin
+      if category_id.present?
+        CustomEmojiCategory.find(category_id)
+      elsif category_name.present?
+        CustomEmojiCategory.create!(name: category_name)
+      end
+    end
+
+    custom_emojis.each do |custom_emoji|
+      custom_emoji.update(category_id: category&.id)
+      log_action :update, custom_emoji
+    end
+  end
+
+  def list!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) }
+
+    custom_emojis.each do |custom_emoji|
+      custom_emoji.update(visible_in_picker: true)
+      log_action :update, custom_emoji
+    end
+  end
+
+  def unlist!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) }
+
+    custom_emojis.each do |custom_emoji|
+      custom_emoji.update(visible_in_picker: false)
+      log_action :update, custom_emoji
+    end
+  end
+
+  def enable!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :enable?) }
+
+    custom_emojis.each do |custom_emoji|
+      custom_emoji.update(disabled: false)
+      log_action :enable, custom_emoji
+    end
+  end
+
+  def disable!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :disable?) }
+
+    custom_emojis.each do |custom_emoji|
+      custom_emoji.update(disabled: true)
+      log_action :disable, custom_emoji
+    end
+  end
+
+  def copy!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :copy?) }
+
+    custom_emojis.each do |custom_emoji|
+      copied_custom_emoji = custom_emoji.copy!
+      log_action :create, copied_custom_emoji
+    end
+  end
+
+  def delete!
+    custom_emojis.each { |custom_emoji| authorize(custom_emoji, :destroy?) }
+
+    custom_emojis.each do |custom_emoji|
+      custom_emoji.destroy
+      log_action :destroy, custom_emoji
+    end
+  end
+end
diff --git a/app/views/admin/custom_emojis/_custom_emoji.html.haml b/app/views/admin/custom_emojis/_custom_emoji.html.haml
index fbaa9a174..9e06a3b42 100644
--- a/app/views/admin/custom_emojis/_custom_emoji.html.haml
+++ b/app/views/admin/custom_emojis/_custom_emoji.html.haml
@@ -1,28 +1,31 @@
-%tr
-  %td
-    = custom_emoji_tag(custom_emoji)
-  %td
-    %samp= ":#{custom_emoji.shortcode}:"
-  %td
-    - if custom_emoji.local?
-      = t('admin.accounts.location.local')
-    - else
-      = link_to custom_emoji.domain, admin_custom_emojis_path(by_domain: custom_emoji.domain)
-  %td
-    - if custom_emoji.local?
-      - if custom_emoji.visible_in_picker
-        = table_link_to 'eye', t('admin.custom_emojis.listed'), admin_custom_emoji_path(custom_emoji, custom_emoji: { visible_in_picker: false }, page: params[:page], **@filter_params), method: :patch
+.batch-table__row
+  %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
+    = f.check_box :custom_emoji_ids, { multiple: true, include_hidden: false }, custom_emoji.id
+  .batch-table__row__content.batch-table__row__content--with-image
+    .batch-table__row__content__image
+      = custom_emoji_tag(custom_emoji)
+
+    .batch-table__row__content__text
+      %samp= ":#{custom_emoji.shortcode}:"
+
+      - if custom_emoji.local?
+        %span.account-role.bot= custom_emoji.category&.name || t('admin.custom_emojis.uncategorized')
+
+    .batch-table__row__content__extra
+      - if custom_emoji.local?
+        = t('admin.accounts.location.local')
       - else
-        = table_link_to 'eye-slash', t('admin.custom_emojis.unlisted'), admin_custom_emoji_path(custom_emoji, custom_emoji: { visible_in_picker: true }, page: params[:page], **@filter_params), method: :patch
-    - else
-      - if custom_emoji.local_counterpart.present?
-        = link_to safe_join([custom_emoji_tag(custom_emoji.local_counterpart), t('admin.custom_emojis.overwrite')]), copy_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post, class: 'table-action-link'
+        = custom_emoji.domain
+
+      %br/
+
+      - if custom_emoji.disabled?
+        = t('admin.custom_emojis.disabled')
       - else
-        = table_link_to 'copy', t('admin.custom_emojis.copy'), copy_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post
-  %td
-    - if custom_emoji.disabled?
-      = table_link_to 'power-off', t('admin.custom_emojis.enable'), enable_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }
-    - else
-      = table_link_to 'power-off', t('admin.custom_emojis.disable'), disable_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }
-  %td
-    = table_link_to 'times', t('admin.custom_emojis.delete'), admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :delete, data: { confirm: t('admin.accounts.are_you_sure') }
+        = t('admin.custom_emojis.enabled')
+      - if custom_emoji.local?
+        &bull;
+        - if custom_emoji.visible_in_picker?
+          = t('admin.custom_emojis.listed')
+        - else
+          = t('admin.custom_emojis.unlisted')
diff --git a/app/views/admin/custom_emojis/index.html.haml b/app/views/admin/custom_emojis/index.html.haml
index 3a119276c..7320ce1bb 100644
--- a/app/views/admin/custom_emojis/index.html.haml
+++ b/app/views/admin/custom_emojis/index.html.haml
@@ -1,6 +1,9 @@
 - content_for :page_title do
   = t('admin.custom_emojis.title')
 
+- content_for :header_tags do
+  = javascript_pack_tag 'admin', integrity: true, async: true, crossorigin: 'anonymous'
+
 .filters
   .filter-subset
     %strong= t('admin.accounts.location.title')
@@ -20,8 +23,7 @@
 = form_tag admin_custom_emojis_url, method: 'GET', class: 'simple_form' do
   .fields-group
     - Admin::FilterHelper::CUSTOM_EMOJI_FILTERS.each do |key|
-      - if params[key].present?
-        = hidden_field_tag key, params[key]
+      = hidden_field_tag key, params[key] if params[key].present?
 
     - %i(shortcode by_domain).each do |key|
       .input.string.optional
@@ -31,18 +33,54 @@
       %button= t('admin.accounts.search')
       = link_to t('admin.accounts.reset'), admin_custom_emojis_path, class: 'button negative'
 
-.table-wrapper
-  %table.table
-    %thead
-      %tr
-        %th= t('admin.custom_emojis.emoji')
-        %th= t('admin.custom_emojis.shortcode')
-        %th= t('admin.accounts.domain')
-        %th
-        %th
-        %th
-    %tbody
-      = render @custom_emojis
+= form_for(@form, url: batch_admin_custom_emojis_path) do |f|
+  = hidden_field_tag :page, params[:page] || 1
+
+  - Admin::FilterHelper::CUSTOM_EMOJI_FILTERS.each do |key|
+    = hidden_field_tag key, params[key] if params[key].present?
+
+  .batch-table
+    .batch-table__toolbar
+      %label.batch-table__toolbar__select.batch-checkbox-all
+        = check_box_tag :batch_checkbox_all, nil, false
+      .batch-table__toolbar__actions
+        - if params[:local] == '1'
+          = f.button safe_join([fa_icon('save'), t('generic.save_changes')]), name: :update, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+          = f.button safe_join([fa_icon('eye'), t('admin.custom_emojis.list')]), name: :list, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+          = f.button safe_join([fa_icon('eye-slash'), t('admin.custom_emojis.unlist')]), name: :unlist, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+        = f.button safe_join([fa_icon('power-off'), t('admin.custom_emojis.enable')]), name: :enable, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+        = f.button safe_join([fa_icon('power-off'), t('admin.custom_emojis.disable')]), name: :disable, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+        = f.button safe_join([fa_icon('times'), t('admin.custom_emojis.delete')]), name: :delete, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+        - unless params[:local] == '1'
+          = f.button safe_join([fa_icon('copy'), t('admin.custom_emojis.copy')]), name: :copy, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+    - if params[:local] == '1'
+      .batch-table__form.simple_form
+        .fields-row
+          .fields-group.fields-row__column.fields-row__column-6
+            .input.select.optional
+              .label_input
+                = f.select :category_id, options_from_collection_for_select(CustomEmojiCategory.all, 'id', 'name'), prompt: t('admin.custom_emojis.assign_category'), class: 'select optional', 'aria-label': t('admin.custom_emojis.assign_category')
+
+          .fields-group.fields-row__column.fields-row__column-6
+            .input.string.optional
+              .label_input
+                = f.text_field :category_name, class: 'string optional', placeholder: t('admin.custom_emojis.create_new_category'), 'aria-label': t('admin.custom_emojis.create_new_category')
+
+    .batch-table__body
+      - if @custom_emojis.empty?
+        = nothing_here 'nothing-here--under-tabs'
+      - else
+        = render partial: 'custom_emoji', collection: @custom_emojis, locals: { f: f }
 
 = paginate @custom_emojis
+
+%hr.spacer/
+
 = link_to t('admin.custom_emojis.upload'), new_admin_custom_emoji_path, class: 'button'
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 42d8e0eb8..52cb4a269 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -225,10 +225,12 @@ en:
       deleted_status: "(deleted status)"
       title: Audit log
     custom_emojis:
+      assign_category: Assign category
       by_domain: Domain
       copied_msg: Successfully created local copy of the emoji
       copy: Copy
       copy_failed_msg: Could not make a local copy of that emoji
+      create_new_category: Create new category
       created_msg: Emoji successfully created!
       delete: Delete
       destroyed_msg: Emojo successfully destroyed!
@@ -245,6 +247,7 @@ en:
       shortcode: Shortcode
       shortcode_hint: At least 2 characters, only alphanumeric characters and underscores
       title: Custom emojis
+      uncategorized: Uncategorized
       unlisted: Unlisted
       update_failed_msg: Could not update that emoji
       updated_msg: Emoji successfully updated!
diff --git a/config/routes.rb b/config/routes.rb
index 534e68814..d22a9e56a 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -242,11 +242,9 @@ Rails.application.routes.draw do
       resource :two_factor_authentication, only: [:destroy]
     end
 
-    resources :custom_emojis, only: [:index, :new, :create, :update, :destroy] do
-      member do
-        post :copy
-        post :enable
-        post :disable
+    resources :custom_emojis, only: [:index, :new, :create] do
+      collection do
+        post :batch
       end
     end
 
diff --git a/spec/controllers/admin/custom_emojis_controller_spec.rb b/spec/controllers/admin/custom_emojis_controller_spec.rb
index b7e2894e9..a8d96948c 100644
--- a/spec/controllers/admin/custom_emojis_controller_spec.rb
+++ b/spec/controllers/admin/custom_emojis_controller_spec.rb
@@ -52,64 +52,4 @@ describe Admin::CustomEmojisController do
       end
     end
   end
-
-  describe 'PUT #update' do
-    let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test') }
-    let(:image) { fixture_file_upload(Rails.root.join('spec', 'fixtures', 'files', 'emojo.png'), 'image/png') }
-
-    before do
-      put :update, params: { id: custom_emoji.id, custom_emoji: params }
-    end
-
-    context 'when parameter is valid' do
-      let(:params) { { shortcode: 'updated', image: image } }
-
-      it 'succeeds in updating custom emoji' do
-        expect(flash[:notice]).to eq I18n.t('admin.custom_emojis.updated_msg')
-        expect(custom_emoji.reload).to have_attributes(shortcode: 'updated')
-      end
-    end
-
-    context 'when parameter is invalid' do
-      let(:params) { { shortcode: 'u', image: image } }
-
-      it 'fails to update custom emoji' do
-        expect(flash[:alert]).to eq I18n.t('admin.custom_emojis.update_failed_msg')
-        expect(custom_emoji.reload).to have_attributes(shortcode: 'test')
-      end
-    end
-  end
-
-  describe 'POST #copy' do
-    subject { post :copy, params: { id: custom_emoji.id } }
-
-    let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test') }
-
-    it 'copies custom emoji' do
-      expect { subject }.to change { CustomEmoji.where(shortcode: 'test').count }.by(1)
-      expect(flash[:notice]).to eq I18n.t('admin.custom_emojis.copied_msg')
-    end
-  end
-
-  describe 'POST #enable' do
-    let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test', disabled: true) }
-
-    before { post :enable, params: { id: custom_emoji.id } }
-
-    it 'enables custom emoji' do
-      expect(response).to redirect_to admin_custom_emojis_path
-      expect(custom_emoji.reload).to have_attributes(disabled: false)
-    end
-  end
-
-  describe 'POST #disable' do
-    let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test', disabled: false) }
-
-    before { post :disable, params: { id: custom_emoji.id } }
-
-    it 'enables custom emoji' do
-      expect(response).to redirect_to admin_custom_emojis_path
-      expect(custom_emoji.reload).to have_attributes(disabled: true)
-    end
-  end
 end
-- 
cgit 


From c5d37f18cb3f4d6212fb8f3e1c4e1e027f677ec5 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Wed, 11 Sep 2019 16:32:44 +0200
Subject: Change deletes to preserve soft-deleted statuses in unresolved
 reports (#11805)

Change all account actions except "none" to resolve all unresolved reports

Refactor `SuspendAccountService` to be more readable
---
 app/controllers/admin/accounts_controller.rb       |  2 +-
 app/controllers/admin/report_notes_controller.rb   |  9 ++--
 .../api/v1/admin/accounts_controller.rb            |  2 +-
 app/lib/activitypub/activity/delete.rb             |  3 +-
 app/models/account.rb                              |  1 +
 app/models/admin/account_action.rb                 | 24 +++++++--
 app/models/form/account_batch.rb                   |  2 +-
 app/models/form/status_batch.rb                    |  2 +-
 app/models/report.rb                               |  1 +
 app/models/status.rb                               |  4 ++
 app/models/user.rb                                 |  4 ++
 app/services/block_domain_service.rb               |  2 +-
 app/services/remove_status_service.rb              |  7 +--
 app/services/suspend_account_service.rb            | 62 ++++++++++++++++------
 app/services/unallow_domain_service.rb             |  2 +-
 app/workers/admin/suspension_worker.rb             |  2 +-
 lib/mastodon/accounts_cli.rb                       |  4 +-
 lib/mastodon/domains_cli.rb                        |  2 +-
 .../admin/reported_statuses_controller_spec.rb     |  2 +-
 spec/controllers/admin/statuses_controller_spec.rb |  2 +-
 spec/models/form/status_batch_spec.rb              |  4 +-
 21 files changed, 98 insertions(+), 45 deletions(-)

(limited to 'spec/controllers/admin')

diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb
index 2fa1dfe5f..68b6352f8 100644
--- a/app/controllers/admin/accounts_controller.rb
+++ b/app/controllers/admin/accounts_controller.rb
@@ -41,7 +41,7 @@ module Admin
 
     def reject
       authorize @account.user, :reject?
-      SuspendAccountService.new.call(@account, including_user: true, destroy: true, skip_distribution: true)
+      SuspendAccountService.new.call(@account, reserve_email: false, reserve_username: false)
       redirect_to admin_pending_accounts_path
     end
 
diff --git a/app/controllers/admin/report_notes_controller.rb b/app/controllers/admin/report_notes_controller.rb
index bcb3f2026..b816c5b5d 100644
--- a/app/controllers/admin/report_notes_controller.rb
+++ b/app/controllers/admin/report_notes_controller.rb
@@ -5,10 +5,10 @@ module Admin
     before_action :set_report_note, only: [:destroy]
 
     def create
-      authorize ReportNote, :create?
+      authorize :report_note, :create?
 
       @report_note = current_account.report_notes.new(resource_params)
-      @report = @report_note.report
+      @report      = @report_note.report
 
       if @report_note.save
         if params[:create_and_resolve]
@@ -26,9 +26,8 @@ module Admin
 
         redirect_to admin_report_path(@report), notice: I18n.t('admin.report_notes.created_msg')
       else
-        @report_notes = @report.notes.latest
-        @report_history = @report.history
-        @form = Form::StatusBatch.new
+        @report_notes = (@report.notes.latest + @report.history + @report.target_account.targeted_account_warnings.latest.custom).sort_by(&:created_at)
+        @form         = Form::StatusBatch.new
 
         render template: 'admin/reports/show'
       end
diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb
index c306180ca..c35ea5ab2 100644
--- a/app/controllers/api/v1/admin/accounts_controller.rb
+++ b/app/controllers/api/v1/admin/accounts_controller.rb
@@ -58,7 +58,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController
 
   def reject
     authorize @account.user, :reject?
-    SuspendAccountService.new.call(@account, including_user: true, destroy: true, skip_distribution: true)
+    SuspendAccountService.new.call(@account, reserve_email: false, reserve_username: false)
     render json: @account, serializer: REST::Admin::AccountSerializer
   end
 
diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb
index 345060462..dc9ff580c 100644
--- a/app/lib/activitypub/activity/delete.rb
+++ b/app/lib/activitypub/activity/delete.rb
@@ -13,8 +13,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity
 
   def delete_person
     lock_or_return("delete_in_progress:#{@account.id}") do
-      SuspendAccountService.new.call(@account)
-      @account.destroy!
+      SuspendAccountService.new.call(@account, reserve_username: false)
     end
   end
 
diff --git a/app/models/account.rb b/app/models/account.rb
index 8c9388b95..55fe53fae 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -115,6 +115,7 @@ class Account < ApplicationRecord
            :approved?,
            :pending?,
            :disabled?,
+           :unconfirmed_or_pending?,
            :role,
            :admin?,
            :moderator?,
diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb
index c7da8b52c..b30a82369 100644
--- a/app/models/admin/account_action.rb
+++ b/app/models/admin/account_action.rb
@@ -83,19 +83,23 @@ class Admin::AccountAction
 
     # A log entry is only interesting if the warning contains
     # custom text from someone. Otherwise it's just noise.
+
     log_action(:create, warning) if warning.text.present?
   end
 
   def process_reports!
-    return if report_id.blank?
+    # If we're doing "mark as resolved" on a single report,
+    # then we want to keep other reports open in case they
+    # contain new actionable information.
+    #
+    # Otherwise, we will mark all unresolved reports about
+    # the account as resolved.
 
-    authorize(report, :update?)
+    reports.each { |report| authorize(report, :update?) }
 
-    if type == 'none'
+    reports.each do |report|
       log_action(:resolve, report)
       report.resolve!(current_account)
-    else
-      Report.where(target_account: target_account).unresolved.update_all(action_taken: true, action_taken_by_account_id: current_account.id)
     end
   end
 
@@ -141,6 +145,16 @@ class Admin::AccountAction
     @report.status_ids if @report && include_statuses
   end
 
+  def reports
+    @reports ||= begin
+      if type == 'none' && with_report?
+        [report]
+      else
+        Report.where(target_account: target_account).unresolved
+      end
+    end
+  end
+
   def warning_preset
     @warning_preset ||= AccountWarningPreset.find(warning_preset_id) if warning_preset_id.present?
   end
diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb
index f1b7a4566..0b285fde9 100644
--- a/app/models/form/account_batch.rb
+++ b/app/models/form/account_batch.rb
@@ -69,6 +69,6 @@ class Form::AccountBatch
     records = accounts.includes(:user)
 
     records.each { |account| authorize(account.user, :reject?) }
-           .each { |account| SuspendAccountService.new.call(account, including_user: true, destroy: true, skip_distribution: true) }
+           .each { |account| SuspendAccountService.new.call(account, reserve_email: false, reserve_username: false) }
   end
 end
diff --git a/app/models/form/status_batch.rb b/app/models/form/status_batch.rb
index e09cc2594..c4943a7ea 100644
--- a/app/models/form/status_batch.rb
+++ b/app/models/form/status_batch.rb
@@ -35,7 +35,7 @@ class Form::StatusBatch
   def delete_statuses
     Status.where(id: status_ids).reorder(nil).find_each do |status|
       status.discard
-      RemovalWorker.perform_async(status.id, redraft: false)
+      RemovalWorker.perform_async(status.id, immediate: true)
       Tombstone.find_or_create_by(uri: status.uri, account: status.account, by_moderator: true)
       log_action :destroy, status
     end
diff --git a/app/models/report.rb b/app/models/report.rb
index 1e707ff1c..fb2e040ee 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -59,6 +59,7 @@ class Report < ApplicationRecord
   end
 
   def resolve!(acting_account)
+    RemovalWorker.push_bulk(Status.with_discarded.discarded.where(id: status_ids).pluck(:id)) { |status_id| [status_id, { immediate: true }] }
     update!(action_taken: true, action_taken_by_account_id: acting_account.id)
   end
 
diff --git a/app/models/status.rb b/app/models/status.rb
index 9cfaddcec..471bb03b4 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -214,6 +214,10 @@ class Status < ApplicationRecord
     !sensitive? && with_media?
   end
 
+  def reported?
+    @reported ||= Report.where(target_account: account).unresolved.where('? = ANY(status_ids)', id).exists?
+  end
+
   def emojis
     return @emojis if defined?(@emojis)
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 95f1d8fc5..78b82a68f 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -171,6 +171,10 @@ class User < ApplicationRecord
     confirmed? && approved? && !disabled? && !account.suspended?
   end
 
+  def unconfirmed_or_pending?
+    !(confirmed? && approved?)
+  end
+
   def inactive_message
     !approved? ? :pending : super
   end
diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb
index 0ec6be503..ae461abf2 100644
--- a/app/services/block_domain_service.rb
+++ b/app/services/block_domain_service.rb
@@ -53,7 +53,7 @@ class BlockDomainService < BaseService
 
   def suspend_accounts!
     blocked_domain_accounts.without_suspended.reorder(nil).find_each do |account|
-      SuspendAccountService.new.call(account, suspended_at: @domain_block.created_at)
+      SuspendAccountService.new.call(account, reserve_username: true, suspended_at: @domain_block.created_at)
     end
   end
 
diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb
index 685c1d4bf..f9352ed3d 100644
--- a/app/services/remove_status_service.rb
+++ b/app/services/remove_status_service.rb
@@ -8,7 +8,8 @@ class RemoveStatusService < BaseService
   # @param   [Status] status
   # @param   [Hash] options
   # @option  [Boolean] :redraft
-  # @options [Boolean] :original_removed
+  # @option  [Boolean] :immediate
+  # @option [Boolean] :original_removed
   def call(status, **options)
     @payload  = Oj.dump(event: :delete, payload: status.id.to_s)
     @status   = status
@@ -31,7 +32,7 @@ class RemoveStatusService < BaseService
         remove_from_spam_check
         remove_media
 
-        @status.destroy!
+        @status.destroy! if @options[:immediate] || !@status.reported?
       else
         raise Mastodon::RaceConditionError
       end
@@ -150,7 +151,7 @@ class RemoveStatusService < BaseService
   end
 
   def remove_media
-    return if @options[:redraft]
+    return if @options[:redraft] || (!@options[:immediate] && @status.reported?)
 
     @status.media_attachments.destroy_all
   end
diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb
index 85da7e921..ecc893931 100644
--- a/app/services/suspend_account_service.rb
+++ b/app/services/suspend_account_service.rb
@@ -15,7 +15,6 @@ class SuspendAccountService < BaseService
     favourites
     follow_requests
     list_accounts
-    media_attachments
     mute_relationships
     muted_by_relationships
     notifications
@@ -32,14 +31,26 @@ class SuspendAccountService < BaseService
     targeted_reports
   ).freeze
 
-  # Suspend an account and remove as much of its data as possible
+  # Suspend or remove an account and remove as much of its data
+  # as possible. If it's a local account and it has not been confirmed
+  # or never been approved, then side effects are skipped and both
+  # the user and account records are removed fully. Otherwise,
+  # it is controlled by options.
   # @param [Account]
   # @param [Hash] options
-  # @option [Boolean] :including_user Remove the user record as well
-  # @option [Boolean] :destroy Remove the account record instead of suspending
+  # @option [Boolean] :reserve_email Keep user record. Only applicable for local accounts
+  # @option [Boolean] :reserve_username Keep account record
+  # @option [Boolean] :skip_side_effects Side effects are ActivityPub and streaming API payloads
+  # @option [Time]    :suspended_at Only applicable when :reserve_username is true
   def call(account, **options)
     @account = account
-    @options = options
+    @options = { reserve_username: true, reserve_email: true }.merge(options)
+
+    if @account.local? && @account.user_unconfirmed_or_pending?
+      @options[:reserve_email]     = false
+      @options[:reserve_username]  = false
+      @options[:skip_side_effects] = true
+    end
 
     reject_follows!
     purge_user!
@@ -60,27 +71,39 @@ class SuspendAccountService < BaseService
   def purge_user!
     return if !@account.local? || @account.user.nil?
 
-    if @options[:including_user]
-      @options[:destroy] = true if !@account.user_confirmed? || @account.user_pending?
-      @account.user.destroy
-    else
+    if @options[:reserve_email]
       @account.user.disable!
       @account.user.invites.where(uses: 0).destroy_all
+    else
+      @account.user.destroy
     end
   end
 
   def purge_content!
-    distribute_delete_actor! if @account.local? && !@options[:skip_distribution]
+    distribute_delete_actor! if @account.local? && !@options[:skip_side_effects]
 
     @account.statuses.reorder(nil).find_in_batches do |statuses|
-      BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:destroy])
+      statuses.reject! { |status| reported_status_ids.include?(status.id) } if @options[:reserve_username]
+      BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:skip_side_effects])
+    end
+
+    @account.media_attachments.reorder(nil).find_each do |media_attachment|
+      next if @options[:reserve_username] && reported_status_ids.include?(media_attachment.status_id)
+
+      media_attachment.destroy
+    end
+
+    @account.polls.reorder(nil).find_each do |poll|
+      next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id)
+
+      poll.destroy
     end
 
     associations_for_destruction.each do |association_name|
       destroy_all(@account.public_send(association_name))
     end
 
-    @account.destroy if @options[:destroy]
+    @account.destroy unless @options[:reserve_username]
   end
 
   def purge_profile!
@@ -88,11 +111,13 @@ class SuspendAccountService < BaseService
     # there is no point wasting time updating
     # its values first
 
-    return if @options[:destroy]
+    return unless @options[:reserve_username]
 
     @account.silenced_at      = nil
     @account.suspended_at     = @options[:suspended_at] || Time.now.utc
     @account.locked           = false
+    @account.memorial         = false
+    @account.discoverable     = false
     @account.display_name     = ''
     @account.note             = ''
     @account.fields           = []
@@ -100,6 +125,7 @@ class SuspendAccountService < BaseService
     @account.followers_count  = 0
     @account.following_count  = 0
     @account.moved_to_account = nil
+    @account.trust_level      = :untrusted
     @account.avatar.destroy
     @account.header.destroy
     @account.save!
@@ -135,11 +161,15 @@ class SuspendAccountService < BaseService
     Account.inboxes - delivery_inboxes
   end
 
+  def reported_status_ids
+    @reported_status_ids ||= Report.where(target_account: @account).unresolved.pluck(:status_ids).flatten.uniq
+  end
+
   def associations_for_destruction
-    if @options[:destroy]
-      ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY
-    else
+    if @options[:reserve_username]
       ASSOCIATIONS_ON_SUSPEND
+    else
+      ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY
     end
   end
 end
diff --git a/app/services/unallow_domain_service.rb b/app/services/unallow_domain_service.rb
index d4387c1a1..bd1ad328d 100644
--- a/app/services/unallow_domain_service.rb
+++ b/app/services/unallow_domain_service.rb
@@ -3,7 +3,7 @@
 class UnallowDomainService < BaseService
   def call(domain_allow)
     Account.where(domain: domain_allow.domain).find_each do |account|
-      SuspendAccountService.new.call(account, destroy: true)
+      SuspendAccountService.new.call(account, reserve_username: false)
     end
 
     domain_allow.destroy
diff --git a/app/workers/admin/suspension_worker.rb b/app/workers/admin/suspension_worker.rb
index ae8b24d8c..83c815efd 100644
--- a/app/workers/admin/suspension_worker.rb
+++ b/app/workers/admin/suspension_worker.rb
@@ -6,6 +6,6 @@ class Admin::SuspensionWorker
   sidekiq_options queue: 'pull'
 
   def perform(account_id, remove_user = false)
-    SuspendAccountService.new.call(Account.find(account_id), including_user: remove_user)
+    SuspendAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: !remove_user)
   end
 end
diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb
index b16bf2e38..a09a6ab04 100644
--- a/lib/mastodon/accounts_cli.rb
+++ b/lib/mastodon/accounts_cli.rb
@@ -185,7 +185,7 @@ module Mastodon
       end
 
       say("Deleting user with #{account.statuses_count} statuses, this might take a while...")
-      SuspendAccountService.new.call(account, including_user: true)
+      SuspendAccountService.new.call(account, reserve_email: false)
       say('OK', :green)
     end
 
@@ -239,7 +239,7 @@ module Mastodon
         end
 
         if [404, 410].include?(code)
-          SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run]
+          SuspendAccountService.new.call(account, reserve_username: false) unless options[:dry_run]
           1
         else
           # Touch account even during dry run to avoid getting the account into the window again
diff --git a/lib/mastodon/domains_cli.rb b/lib/mastodon/domains_cli.rb
index c612c2d72..8e52de1c3 100644
--- a/lib/mastodon/domains_cli.rb
+++ b/lib/mastodon/domains_cli.rb
@@ -42,7 +42,7 @@ module Mastodon
       end
 
       processed, = parallelize_with_progress(scope) do |account|
-        SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run]
+        SuspendAccountService.new.call(account, reserve_username: false, skip_side_effects: true) unless options[:dry_run]
       end
 
       DomainBlock.where(domain: domain).destroy_all unless options[:dry_run]
diff --git a/spec/controllers/admin/reported_statuses_controller_spec.rb b/spec/controllers/admin/reported_statuses_controller_spec.rb
index bd146b795..2a1598123 100644
--- a/spec/controllers/admin/reported_statuses_controller_spec.rb
+++ b/spec/controllers/admin/reported_statuses_controller_spec.rb
@@ -47,7 +47,7 @@ describe Admin::ReportedStatusesController do
       it 'removes a status' do
         allow(RemovalWorker).to receive(:perform_async)
         subject.call
-        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, redraft: false)
+        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, immediate: true)
       end
     end
 
diff --git a/spec/controllers/admin/statuses_controller_spec.rb b/spec/controllers/admin/statuses_controller_spec.rb
index 6b06343ef..d9690d83f 100644
--- a/spec/controllers/admin/statuses_controller_spec.rb
+++ b/spec/controllers/admin/statuses_controller_spec.rb
@@ -65,7 +65,7 @@ describe Admin::StatusesController do
       it 'removes a status' do
         allow(RemovalWorker).to receive(:perform_async)
         subject.call
-        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, redraft: false)
+        expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, immediate: true)
       end
     end
 
diff --git a/spec/models/form/status_batch_spec.rb b/spec/models/form/status_batch_spec.rb
index f9c58c90f..68d84a737 100644
--- a/spec/models/form/status_batch_spec.rb
+++ b/spec/models/form/status_batch_spec.rb
@@ -41,12 +41,12 @@ describe Form::StatusBatch do
 
     it 'call RemovalWorker' do
       form.save
-      expect(RemovalWorker).to have_received(:perform_async).with(status.id, redraft: false)
+      expect(RemovalWorker).to have_received(:perform_async).with(status.id, immediate: true)
     end
 
     it 'do not call RemovalWorker' do
       form.save
-      expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id, redraft: false)
+      expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id, immediate: true)
     end
   end
 end
-- 
cgit