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
This commit is contained in:
Eugen Rochko 2021-05-07 14:33:43 +02:00 committed by GitHub
parent 2c77d97e0d
commit 74081433d0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 59 additions and 160 deletions

View file

@ -6,7 +6,6 @@ class DirectoriesController < ApplicationController
before_action :authenticate_user!, if: :whitelist_mode? before_action :authenticate_user!, if: :whitelist_mode?
before_action :require_enabled! before_action :require_enabled!
before_action :set_instance_presenter before_action :set_instance_presenter
before_action :set_tag, only: :show
before_action :set_accounts before_action :set_accounts
skip_before_action :require_functional!, unless: :whitelist_mode? skip_before_action :require_functional!, unless: :whitelist_mode?
@ -15,23 +14,14 @@ class DirectoriesController < ApplicationController
render :index render :index
end end
def show
render :index
end
private private
def require_enabled! def require_enabled!
return not_found unless Setting.profile_directory return not_found unless Setting.profile_directory
end end
def set_tag
@tag = Tag.discoverable.find_normalized!(params[:id])
end
def set_accounts def set_accounts
@accounts = Account.local.discoverable.by_recent_status.page(params[:page]).per(20).tap do |query| @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 query.merge!(Account.not_excluded_by_account(current_account)) if current_account
end end
end end

View file

@ -22,6 +22,10 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity
visibility: visibility_from_audience visibility: visibility_from_audience
) )
original_status.tags.each do |tag|
tag.use!(@account)
end
distribute(@status) distribute(@status)
end end

View file

@ -164,7 +164,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
def attach_tags(status) def attach_tags(status)
@tags.each do |tag| @tags.each do |tag|
status.tags << 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 end
@mentions.each do |mention| @mentions.each do |mention|

View file

@ -111,7 +111,6 @@ class Account < ApplicationRecord
scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) }
scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } 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 :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_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 :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') } scope :popular, -> { order('account_stats.followers_count desc') }
@ -279,19 +278,13 @@ class Account < ApplicationRecord
if hashtags_map.key?(tag.name) if hashtags_map.key?(tag.name)
hashtags_map.delete(tag.name) hashtags_map.delete(tag.name)
else else
transaction do tags.delete(tag)
tags.delete(tag)
tag.decrement_count!(:accounts_count)
end
end end
end end
# Add hashtags that were so far missing # Add hashtags that were so far missing
hashtags_map.each_value do |tag| hashtags_map.each_value do |tag|
transaction do tags << tag
tags << tag
tag.increment_count!(:accounts_count)
end
end end
end end

View file

@ -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

View file

@ -20,10 +20,8 @@
class Tag < ApplicationRecord class Tag < ApplicationRecord
has_and_belongs_to_many :statuses has_and_belongs_to_many :statuses
has_and_belongs_to_many :accounts 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_many :featured_tags, dependent: :destroy, inverse_of: :tag
has_one :account_tag_stat, dependent: :destroy
HASHTAG_SEPARATORS = "_\u00B7\u200c" HASHTAG_SEPARATORS = "_\u00B7\u200c"
HASHTAG_NAME_RE = "([[:word:]_][[:word:]#{HASHTAG_SEPARATORS}]*[[:alpha:]#{HASHTAG_SEPARATORS}][[:word:]#{HASHTAG_SEPARATORS}]*[[:word:]_])|([[:word:]_]*[[:alpha:]][[:word:]_]*)" 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 :usable, -> { where(usable: [true, nil]) }
scope :listable, -> { where(listable: [true, nil]) } scope :listable, -> { where(listable: [true, nil]) }
scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } 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')) } 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)) } # 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
update_index('tags#tag', :self) 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 def to_param
name name
end end
@ -95,6 +75,10 @@ class Tag < ApplicationRecord
requested_review_at.present? requested_review_at.present?
end 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? def trending?
TrendingTags.trending?(self) TrendingTags.trending?(self)
end end
@ -127,9 +111,10 @@ class Tag < ApplicationRecord
end end
def search_for(term, limit = 5, offset = 0, options = {}) def search_for(term, limit = 5, offset = 0, options = {})
striped_term = term.strip stripped_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] 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')) query.order(Arel.sql('length(name) ASC, name ASC'))
.limit(limit) .limit(limit)
@ -161,11 +146,6 @@ class Tag < ApplicationRecord
private private
def save_account_tag_stat
return unless account_tag_stat&.changed?
account_tag_stat.save
end
def validate_name_change 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? errors.add(:name, I18n.t('tags.does_not_match_previous_name')) unless name_was.mb_chars.casecmp(name.mb_chars).zero?
end end

View file

@ -33,8 +33,6 @@ class TagFilter
def scope_for(key, value) def scope_for(key, value)
case key.to_s case key.to_s
when 'directory'
Tag.discoverable
when 'reviewed' when 'reviewed'
Tag.reviewed.order(reviewed_at: :desc) Tag.reviewed.order(reviewed_at: :desc)
when 'unreviewed' when 'unreviewed'

View file

@ -13,19 +13,23 @@ class TrendingTags
class << self class << self
include Redisable include Redisable
def record_use!(tag, account, at_time = Time.now.utc) def record_use!(tag, account, status: nil, at_time: Time.now.utc)
return if account.silenced? || account.bot? || !tag.usable? || !(tag.trendable? || tag.requires_review?) 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_historical_use!(tag.id, at_time)
increment_unique_use!(tag.id, account.id, at_time) increment_unique_use!(tag.id, account.id, at_time)
increment_use!(tag.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 end
def update!(at_time = Time.now.utc) 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) 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 # First pass to calculate scores and update the set

View file

@ -8,8 +8,7 @@ class ProcessHashtagsService < BaseService
Tag.find_or_create_by_names(tags) do |tag| Tag.find_or_create_by_names(tags) do |tag|
status.tags << tag status.tags << tag
records << tag records << tag
tag.use!(status.account, status: status, at_time: status.created_at) if status.public_visibility?
TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility?
end end
return unless status.distributable? return unless status.distributable?

View file

@ -35,6 +35,7 @@ class ReblogService < BaseService
create_notification(reblog) create_notification(reblog)
bump_potential_friendship(account, reblog) bump_potential_friendship(account, reblog)
record_use(account, reblog)
reblog reblog
end end
@ -59,6 +60,16 @@ class ReblogService < BaseService
PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog) PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog)
end 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) def build_json(reblog)
Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(reblog), ActivityPub::ActivitySerializer, signer: reblog.account)) Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(reblog), ActivityPub::ActivitySerializer, signer: reblog.account))
end end

View file

@ -10,8 +10,6 @@
= tag.name = tag.name
%small %small
= t('admin.tags.in_directory', count: tag.accounts_count)
&bull;
= t('admin.tags.unique_uses_today', count: tag.history.first[:accounts]) = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts])
- if tag.trending? - if tag.trending?

View file

@ -5,12 +5,6 @@
= javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous' = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous'
.filters .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 .filter-subset
%strong= t('admin.tags.review') %strong= t('admin.tags.review')
%ul %ul
@ -23,8 +17,9 @@
%strong= t('generic.order_by') %strong= t('generic.order_by')
%ul %ul
%li= filter_link_to t('admin.tags.most_recent'), popular: nil, active: nil %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.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 = form_tag admin_tags_url, method: 'GET', class: 'simple_form' do
.fields-group .fields-group

View file

@ -10,15 +10,6 @@
%div %div
.dashboard__counters__num= number_with_delimiter @accounts_week .dashboard__counters__num= number_with_delimiter @accounts_week
.dashboard__counters__label= t 'admin.tags.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/ %hr.spacer/
@ -30,8 +21,8 @@
.fields-group .fields-group
= f.input :usable, as: :boolean, wrapper: :with_label = f.input :usable, as: :boolean, wrapper: :with_label
= f.input :trendable, as: :boolean, wrapper: :with_label, disabled: !Setting.trends = f.input :trendable, as: :boolean, wrapper: :with_label
= f.input :listable, as: :boolean, wrapper: :with_label, disabled: !Setting.profile_directory = f.input :listable, as: :boolean, wrapper: :with_label
.actions .actions
= f.button :button, t('generic.save_changes'), type: :submit = f.button :button, t('generic.save_changes'), type: :submit

View file

@ -698,12 +698,9 @@ en:
accounts_today: Unique uses today accounts_today: Unique uses today
accounts_week: Unique uses this week accounts_week: Unique uses this week
breakdown: Breakdown of today's usage by source breakdown: Breakdown of today's usage by source
context: Context last_active: Recently used
directory: In directory
in_directory: "%{count} in directory"
last_active: Last active
most_popular: Most popular most_popular: Most popular
most_recent: Most recent most_recent: Recently created
name: Hashtag name: Hashtag
review: Review status review: Review status
reviewed: Reviewed reviewed: Reviewed

View file

@ -208,7 +208,7 @@ en:
rule: rule:
text: Rule text: Rule
tag: 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 name: Hashtag
trendable: Allow this hashtag to appear under trends trendable: Allow this hashtag to appear under trends
usable: Allow posts to use this hashtag usable: Allow posts to use this hashtag

View file

@ -97,8 +97,6 @@ Rails.application.routes.draw do
post '/interact/:id', to: 'remote_interaction#create' post '/interact/:id', to: 'remote_interaction#create'
get '/explore', to: 'directories#index', as: :explore get '/explore', to: 'directories#index', as: :explore
get '/explore/:id', to: 'directories#show', as: :explore_hashtag
get '/settings', to: redirect('/settings/profile') get '/settings', to: redirect('/settings/profile')
namespace :settings do namespace :settings do

View file

@ -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

View file

@ -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 t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true
end 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| create_table "account_warning_presets", force: :cascade do |t|
t.text "text", default: "", null: false t.text "text", default: "", null: false
t.datetime "created_at", 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", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_pins", "accounts", 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_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", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_warnings", "accounts", on_delete: :nullify add_foreign_key "account_warnings", "accounts", on_delete: :nullify
add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify

View file

@ -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

View file

@ -7,9 +7,9 @@ RSpec.describe TrendingTags do
describe '.update!' do describe '.update!' do
let!(:at_time) { Time.now.utc } let!(:at_time) { Time.now.utc }
let!(:tag1) { Fabricate(:tag, name: 'Catstodon') } let!(:tag1) { Fabricate(:tag, name: 'Catstodon', trendable: true) }
let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon') } let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon', trendable: true) }
let!(:tag3) { Fabricate(:tag, name: 'OCs') } let!(:tag3) { Fabricate(:tag, name: 'OCs', trendable: true) }
before do before do
allow(Redis.current).to receive(:pfcount) do |key| allow(Redis.current).to receive(:pfcount) do |key|