Skip to content

Commit

Permalink
Change trending hashtags to be affected be reblogs (#16164)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Gargron committed May 7, 2021
1 parent 2c77d97 commit 7408143
Show file tree
Hide file tree
Showing 20 changed files with 59 additions and 160 deletions.
10 changes: 0 additions & 10 deletions app/controllers/directories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions app/lib/activitypub/activity/announce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def perform
visibility: visibility_from_audience
)

original_status.tags.each do |tag|
tag.use!(@account)
end

distribute(@status)
end

Expand Down
2 changes: 1 addition & 1 deletion app/lib/activitypub/activity/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
11 changes: 2 additions & 9 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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') }
Expand Down Expand Up @@ -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

Expand Down
24 changes: 0 additions & 24 deletions app/models/account_tag_stat.rb

This file was deleted.

38 changes: 9 additions & 29 deletions app/models/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:]_]*)"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions app/models/tag_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
12 changes: 8 additions & 4 deletions app/models/trending_tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions app/services/process_hashtags_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
11 changes: 11 additions & 0 deletions app/services/reblog_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def call(account, reblogged_status, options = {})

create_notification(reblog)
bump_potential_friendship(account, reblog)
record_use(account, reblog)

reblog
end
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions app/views/admin/tags/_tag.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
9 changes: 2 additions & 7 deletions app/views/admin/tags/index.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
13 changes: 2 additions & 11 deletions app/views/admin/tags/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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/

Expand All @@ -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
Expand Down
7 changes: 2 additions & 5 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion config/locales/simple_form.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions db/post_migrate/20210502233513_drop_account_tag_stats.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 0 additions & 10 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7408143

Please sign in to comment.