Change trending hashtags to be affected be reblogs (mastodon#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
hachyderm · May 7, 2021 · 7408143 · 7408143
1 parent 2c77d97
commit 7408143
Show file tree

Hide file tree

Showing 20 changed files with 59 additions and 160 deletions.
diff --git 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 @@ def index
     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
@@ -22,6 +22,10 @@ def perform
         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
@@ -164,7 +164,7 @@ def postprocess_audience_and_deliver
   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
@@ -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 @@ def tags_as_strings=(tag_names)
       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
diff --git 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 @@ def requested_review?
     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 @@ def find_or_create_by_names(name_or_names)
     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 @@ def normalize(str)
 
   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
@@ -33,8 +33,6 @@ def results
 
   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
@@ -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
@@ -8,8 +8,7 @@ def call(status, tags = [])
     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
@@ -35,6 +35,7 @@ def call(account, reblogged_status, options = {})
 
     create_notification(reblog)
     bump_potential_friendship(account, reblog)
+    record_use(account, reblog)
 
     reblog
   end
@@ -59,6 +60,16 @@ def bump_potential_friendship(account, reblog)
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -97,8 +97,6 @@
   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
@@ -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
@@ -115,15 +115,6 @@
     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 @@
   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