From 74081433d0078784b7c2139f6caaa812740632b2 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Fri, 7 May 2021 14:33:43 +0200
Subject: [PATCH] Change trending hashtags to be affected be reblogs (#16164)

If a status with a hashtag becomes very popular, it stands to
reason that the hashtag should have a chance at trending

Fix no stats being recorded for hashtags that are not allowed
to trend, and stop ignoring bots

Remove references to hashtags in profile directory from the code
and the admin UI
---
 app/controllers/directories_controller.rb     | 10 -----
 app/lib/activitypub/activity/announce.rb      |  4 ++
 app/lib/activitypub/activity/create.rb        |  2 +-
 app/models/account.rb                         | 11 +-----
 app/models/account_tag_stat.rb                | 24 ------------
 app/models/tag.rb                             | 38 +++++--------------
 app/models/tag_filter.rb                      |  2 -
 app/models/trending_tags.rb                   | 12 ++++--
 app/services/process_hashtags_service.rb      |  3 +-
 app/services/reblog_service.rb                | 11 ++++++
 app/views/admin/tags/_tag.html.haml           |  2 -
 app/views/admin/tags/index.html.haml          |  9 +----
 app/views/admin/tags/show.html.haml           | 13 +------
 config/locales/en.yml                         |  7 +---
 config/locales/simple_form.en.yml             |  2 +-
 config/routes.rb                              |  2 -
 .../20210502233513_drop_account_tag_stats.rb  | 13 +++++++
 db/schema.rb                                  | 10 -----
 spec/models/account_tag_stat_spec.rb          | 38 -------------------
 spec/models/trending_tags_spec.rb             |  6 +--
 20 files changed, 59 insertions(+), 160 deletions(-)
 delete mode 100644 app/models/account_tag_stat.rb
 create mode 100644 db/post_migrate/20210502233513_drop_account_tag_stats.rb
 delete mode 100644 spec/models/account_tag_stat_spec.rb

diff --git a/app/controllers/directories_controller.rb b/app/controllers/directories_controller.rb
index f198ad5ba5..f28c5b2afb 100644
--- a/app/controllers/directories_controller.rb
+++ b/app/controllers/directories_controller.rb
@@ -6,7 +6,6 @@ class DirectoriesController < ApplicationController
   before_action :authenticate_user!, if: :whitelist_mode?
   before_action :require_enabled!
   before_action :set_instance_presenter
-  before_action :set_tag, only: :show
   before_action :set_accounts
 
   skip_before_action :require_functional!, unless: :whitelist_mode?
@@ -15,23 +14,14 @@ class DirectoriesController < ApplicationController
     render :index
   end
 
-  def show
-    render :index
-  end
-
   private
 
   def require_enabled!
     return not_found unless Setting.profile_directory
   end
 
-  def set_tag
-    @tag = Tag.discoverable.find_normalized!(params[:id])
-  end
-
   def set_accounts
     @accounts = Account.local.discoverable.by_recent_status.page(params[:page]).per(20).tap do |query|
-      query.merge!(Account.tagged_with(@tag.id)) if @tag
       query.merge!(Account.not_excluded_by_account(current_account)) if current_account
     end
   end
diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb
index a1081522eb..9f778ffb98 100644
--- a/app/lib/activitypub/activity/announce.rb
+++ b/app/lib/activitypub/activity/announce.rb
@@ -22,6 +22,10 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity
         visibility: visibility_from_audience
       )
 
+      original_status.tags.each do |tag|
+        tag.use!(@account)
+      end
+
       distribute(@status)
     end
 
diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
index c7a655d9db..e46361c147 100644
--- a/app/lib/activitypub/activity/create.rb
+++ b/app/lib/activitypub/activity/create.rb
@@ -164,7 +164,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   def attach_tags(status)
     @tags.each do |tag|
       status.tags << tag
-      TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility?
+      tag.use!(@account, status: status, at_time: status.created_at) if status.public_visibility?
     end
 
     @mentions.each do |mention|
diff --git a/app/models/account.rb b/app/models/account.rb
index a573365de2..994459338b 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -111,7 +111,6 @@ class Account < ApplicationRecord
   scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) }
   scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) }
   scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) }
-  scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) }
   scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) }
   scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) }
   scope :popular, -> { order('account_stats.followers_count desc') }
@@ -279,19 +278,13 @@ class Account < ApplicationRecord
       if hashtags_map.key?(tag.name)
         hashtags_map.delete(tag.name)
       else
-        transaction do
-          tags.delete(tag)
-          tag.decrement_count!(:accounts_count)
-        end
+        tags.delete(tag)
       end
     end
 
     # Add hashtags that were so far missing
     hashtags_map.each_value do |tag|
-      transaction do
-        tags << tag
-        tag.increment_count!(:accounts_count)
-      end
+      tags << tag
     end
   end
 
diff --git a/app/models/account_tag_stat.rb b/app/models/account_tag_stat.rb
deleted file mode 100644
index 3c36c155ab..0000000000
--- a/app/models/account_tag_stat.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-# frozen_string_literal: true
-# == Schema Information
-#
-# Table name: account_tag_stats
-#
-#  id             :bigint(8)        not null, primary key
-#  tag_id         :bigint(8)        not null
-#  accounts_count :bigint(8)        default(0), not null
-#  hidden         :boolean          default(FALSE), not null
-#  created_at     :datetime         not null
-#  updated_at     :datetime         not null
-#
-
-class AccountTagStat < ApplicationRecord
-  belongs_to :tag, inverse_of: :account_tag_stat
-
-  def increment_count!(key)
-    update(key => public_send(key) + 1)
-  end
-
-  def decrement_count!(key)
-    update(key => [public_send(key) - 1, 0].max)
-  end
-end
diff --git a/app/models/tag.rb b/app/models/tag.rb
index efffc7eee4..735c306085 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -20,10 +20,8 @@
 class Tag < ApplicationRecord
   has_and_belongs_to_many :statuses
   has_and_belongs_to_many :accounts
-  has_and_belongs_to_many :sample_accounts, -> { local.discoverable.popular.limit(3) }, class_name: 'Account'
 
   has_many :featured_tags, dependent: :destroy, inverse_of: :tag
-  has_one :account_tag_stat, dependent: :destroy
 
   HASHTAG_SEPARATORS = "_\u00B7\u200c"
   HASHTAG_NAME_RE    = "([[:word:]_][[:word:]#{HASHTAG_SEPARATORS}]*[[:alpha:]#{HASHTAG_SEPARATORS}][[:word:]#{HASHTAG_SEPARATORS}]*[[:word:]_])|([[:word:]_]*[[:alpha:]][[:word:]_]*)"
@@ -38,29 +36,11 @@ class Tag < ApplicationRecord
   scope :usable, -> { where(usable: [true, nil]) }
   scope :listable, -> { where(listable: [true, nil]) }
   scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) }
-  scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) }
   scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) }
-  # Search with case-sensitive to use B-tree index.
-  scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) }
-
-  delegate :accounts_count,
-           :accounts_count=,
-           :increment_count!,
-           :decrement_count!,
-           to: :account_tag_stat
-
-  after_save :save_account_tag_stat
+  scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } # Search with case-sensitive to use B-tree index
 
   update_index('tags#tag', :self)
 
-  def account_tag_stat
-    super || build_account_tag_stat
-  end
-
-  def cached_sample_accounts
-    Rails.cache.fetch("#{cache_key}/sample_accounts", expires_in: 12.hours) { sample_accounts }
-  end
-
   def to_param
     name
   end
@@ -95,6 +75,10 @@ class Tag < ApplicationRecord
     requested_review_at.present?
   end
 
+  def use!(account, status: nil, at_time: Time.now.utc)
+    TrendingTags.record_use!(self, account, status: status, at_time: at_time)
+  end
+
   def trending?
     TrendingTags.trending?(self)
   end
@@ -127,9 +111,10 @@ class Tag < ApplicationRecord
     end
 
     def search_for(term, limit = 5, offset = 0, options = {})
-      striped_term = term.strip
-      query = Tag.listable.matches_name(striped_term)
-      query = query.merge(matching_name(striped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed]
+      stripped_term = term.strip
+
+      query = Tag.listable.matches_name(stripped_term)
+      query = query.merge(matching_name(stripped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed]
 
       query.order(Arel.sql('length(name) ASC, name ASC'))
            .limit(limit)
@@ -161,11 +146,6 @@ class Tag < ApplicationRecord
 
   private
 
-  def save_account_tag_stat
-    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
diff --git a/app/models/tag_filter.rb b/app/models/tag_filter.rb
index a9ff5b703c..85bfcbea5a 100644
--- a/app/models/tag_filter.rb
+++ b/app/models/tag_filter.rb
@@ -33,8 +33,6 @@ class TagFilter
 
   def scope_for(key, value)
     case key.to_s
-    when 'directory'
-      Tag.discoverable
     when 'reviewed'
       Tag.reviewed.order(reviewed_at: :desc)
     when 'unreviewed'
diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb
index 9c2aa0ee8b..31890b0824 100644
--- a/app/models/trending_tags.rb
+++ b/app/models/trending_tags.rb
@@ -13,19 +13,23 @@ class TrendingTags
   class << self
     include Redisable
 
-    def record_use!(tag, account, at_time = Time.now.utc)
-      return if account.silenced? || account.bot? || !tag.usable? || !(tag.trendable? || tag.requires_review?)
+    def record_use!(tag, account, status: nil, at_time: Time.now.utc)
+      return unless tag.usable? && !account.silenced?
 
+      # Even if a tag is not allowed to trend, we still need to
+      # record the stats since they can be displayed in other places
       increment_historical_use!(tag.id, at_time)
       increment_unique_use!(tag.id, account.id, at_time)
       increment_use!(tag.id, at_time)
 
-      tag.update(last_status_at: Time.now.utc) if tag.last_status_at.nil? || tag.last_status_at < 12.hours.ago
+      # Only update when the tag was last used once every 12 hours
+      # and only if a status is given (lets use ignore reblogs)
+      tag.update(last_status_at: at_time) if status.present? && (tag.last_status_at.nil? || (tag.last_status_at < at_time && tag.last_status_at < 12.hours.ago))
     end
 
     def update!(at_time = Time.now.utc)
       tag_ids = redis.smembers("#{KEY}:used:#{at_time.beginning_of_day.to_i}") + redis.zrange(KEY, 0, -1)
-      tags    = Tag.where(id: tag_ids.uniq)
+      tags    = Tag.trendable.where(id: tag_ids.uniq)
 
       # First pass to calculate scores and update the set
 
diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb
index e8e139b058..c42b79db80 100644
--- a/app/services/process_hashtags_service.rb
+++ b/app/services/process_hashtags_service.rb
@@ -8,8 +8,7 @@ class ProcessHashtagsService < BaseService
     Tag.find_or_create_by_names(tags) do |tag|
       status.tags << tag
       records << tag
-
-      TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility?
+      tag.use!(status.account, status: status, at_time: status.created_at) if status.public_visibility?
     end
 
     return unless status.distributable?
diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb
index 5032397b34..744bdf5673 100644
--- a/app/services/reblog_service.rb
+++ b/app/services/reblog_service.rb
@@ -35,6 +35,7 @@ class ReblogService < BaseService
 
     create_notification(reblog)
     bump_potential_friendship(account, reblog)
+    record_use(account, reblog)
 
     reblog
   end
@@ -59,6 +60,16 @@ class ReblogService < BaseService
     PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog)
   end
 
+  def record_use(account, reblog)
+    return unless reblog.public_visibility?
+
+    original_status = reblog.reblog
+
+    original_status.tags.each do |tag|
+      tag.use!(account)
+    end
+  end
+
   def build_json(reblog)
     Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(reblog), ActivityPub::ActivitySerializer, signer: reblog.account))
   end
diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml
index 287d28e53b..adf4ca7b2f 100644
--- a/app/views/admin/tags/_tag.html.haml
+++ b/app/views/admin/tags/_tag.html.haml
@@ -10,8 +10,6 @@
         = tag.name
 
         %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?
diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml
index d7719d45d6..d78f3c6d16 100644
--- a/app/views/admin/tags/index.html.haml
+++ b/app/views/admin/tags/index.html.haml
@@ -5,12 +5,6 @@
   = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous'
 
 .filters
-  .filter-subset
-    %strong= t('admin.tags.context')
-    %ul
-      %li= filter_link_to t('generic.all'), directory: nil
-      %li= filter_link_to t('admin.tags.directory'), directory: '1'
-
   .filter-subset
     %strong= t('admin.tags.review')
     %ul
@@ -23,8 +17,9 @@
     %strong= t('generic.order_by')
     %ul
       %li= filter_link_to t('admin.tags.most_recent'), popular: nil, active: nil
-      %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil
       %li= filter_link_to t('admin.tags.last_active'), active: '1', popular: nil
+      %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil
+
 
 = form_tag admin_tags_url, method: 'GET', class: 'simple_form' do
   .fields-group
diff --git a/app/views/admin/tags/show.html.haml b/app/views/admin/tags/show.html.haml
index c9a147587c..c4caffda1e 100644
--- a/app/views/admin/tags/show.html.haml
+++ b/app/views/admin/tags/show.html.haml
@@ -10,15 +10,6 @@
     %div
       .dashboard__counters__num= number_with_delimiter @accounts_week
       .dashboard__counters__label= t 'admin.tags.accounts_week'
-  %div
-    - if @tag.accounts_count > 0
-      = link_to explore_hashtag_path(@tag) do
-        .dashboard__counters__num= number_with_delimiter @tag.accounts_count
-        .dashboard__counters__label= t 'admin.tags.directory'
-    - else
-      %div
-        .dashboard__counters__num= number_with_delimiter @tag.accounts_count
-        .dashboard__counters__label= t 'admin.tags.directory'
 
 %hr.spacer/
 
@@ -30,8 +21,8 @@
 
   .fields-group
     = f.input :usable, as: :boolean, wrapper: :with_label
-    = f.input :trendable, as: :boolean, wrapper: :with_label, disabled: !Setting.trends
-    = f.input :listable, as: :boolean, wrapper: :with_label, disabled: !Setting.profile_directory
+    = 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/config/locales/en.yml b/config/locales/en.yml
index bfa4898174..d8ad5bd84a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -698,12 +698,9 @@ en:
       accounts_today: Unique uses today
       accounts_week: Unique uses this week
       breakdown: Breakdown of today's usage by source
-      context: Context
-      directory: In directory
-      in_directory: "%{count} in directory"
-      last_active: Last active
+      last_active: Recently used
       most_popular: Most popular
-      most_recent: Most recent
+      most_recent: Recently created
       name: Hashtag
       review: Review status
       reviewed: Reviewed
diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml
index 8ff880ebc0..c4388ffc52 100644
--- a/config/locales/simple_form.en.yml
+++ b/config/locales/simple_form.en.yml
@@ -208,7 +208,7 @@ en:
       rule:
         text: Rule
       tag:
-        listable: Allow this hashtag to appear in searches and on the profile directory
+        listable: Allow this hashtag to appear in searches and suggestions
         name: Hashtag
         trendable: Allow this hashtag to appear under trends
         usable: Allow posts to use this hashtag
diff --git a/config/routes.rb b/config/routes.rb
index 8ca7fccdd9..2373d8a51b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -97,8 +97,6 @@ Rails.application.routes.draw do
   post '/interact/:id', to: 'remote_interaction#create'
 
   get '/explore', to: 'directories#index', as: :explore
-  get '/explore/:id', to: 'directories#show', as: :explore_hashtag
-
   get '/settings', to: redirect('/settings/profile')
 
   namespace :settings do
diff --git a/db/post_migrate/20210502233513_drop_account_tag_stats.rb b/db/post_migrate/20210502233513_drop_account_tag_stats.rb
new file mode 100644
index 0000000000..80adadcab0
--- /dev/null
+++ b/db/post_migrate/20210502233513_drop_account_tag_stats.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class DropAccountTagStats < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def up
+    drop_table :account_tag_stats
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 88e9060794..19b1afe00d 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -115,15 +115,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do
     t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true
   end
 
-  create_table "account_tag_stats", force: :cascade do |t|
-    t.bigint "tag_id", null: false
-    t.bigint "accounts_count", default: 0, null: false
-    t.boolean "hidden", default: false, null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-    t.index ["tag_id"], name: "index_account_tag_stats_on_tag_id", unique: true
-  end
-
   create_table "account_warning_presets", force: :cascade do |t|
     t.text "text", default: "", null: false
     t.datetime "created_at", null: false
@@ -985,7 +976,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do
   add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade
   add_foreign_key "account_pins", "accounts", on_delete: :cascade
   add_foreign_key "account_stats", "accounts", on_delete: :cascade
-  add_foreign_key "account_tag_stats", "tags", on_delete: :cascade
   add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade
   add_foreign_key "account_warnings", "accounts", on_delete: :nullify
   add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify
diff --git a/spec/models/account_tag_stat_spec.rb b/spec/models/account_tag_stat_spec.rb
deleted file mode 100644
index 6d3057f355..0000000000
--- a/spec/models/account_tag_stat_spec.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe AccountTagStat, type: :model do
-  key = 'accounts_count'
-  let(:account_tag_stat) { Fabricate(:tag).account_tag_stat }
-
-  describe '#increment_count!' do
-    it 'calls #update' do
-      args = { key => account_tag_stat.public_send(key) + 1 }
-      expect(account_tag_stat).to receive(:update).with(args)
-      account_tag_stat.increment_count!(key)
-    end
-
-    it 'increments value by 1' do
-      expect do
-        account_tag_stat.increment_count!(key)
-      end.to change { account_tag_stat.accounts_count }.by(1)
-    end
-  end
-
-  describe '#decrement_count!' do
-    it 'calls #update' do
-      args = { key => [account_tag_stat.public_send(key) - 1, 0].max }
-      expect(account_tag_stat).to receive(:update).with(args)
-      account_tag_stat.decrement_count!(key)
-    end
-
-    it 'decrements value by 1' do
-      account_tag_stat.update(key => 1)
-
-      expect do
-        account_tag_stat.decrement_count!(key)
-      end.to change { account_tag_stat.accounts_count }.by(-1)
-    end
-  end
-end
diff --git a/spec/models/trending_tags_spec.rb b/spec/models/trending_tags_spec.rb
index b6122c9948..dfbc7d6f80 100644
--- a/spec/models/trending_tags_spec.rb
+++ b/spec/models/trending_tags_spec.rb
@@ -7,9 +7,9 @@ RSpec.describe TrendingTags do
 
   describe '.update!' do
     let!(:at_time) { Time.now.utc }
-    let!(:tag1) { Fabricate(:tag, name: 'Catstodon') }
-    let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon') }
-    let!(:tag3) { Fabricate(:tag, name: 'OCs') }
+    let!(:tag1) { Fabricate(:tag, name: 'Catstodon', trendable: true) }
+    let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon', trendable: true) }
+    let!(:tag3) { Fabricate(:tag, name: 'OCs', trendable: true) }
 
     before do
       allow(Redis.current).to receive(:pfcount) do |key|