summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEugen Rochko <eugen@zeonfederated.com>2021-05-07 14:33:43 +0200
committerGitHub <noreply@github.com>2021-05-07 14:33:43 +0200
commit74081433d0078784b7c2139f6caaa812740632b2 (patch)
tree02af62ad9e8dad4d2b9d7c504c7ffce27cdf26ac
parent2c77d97e0d59e045a9b04fccc83f0f24d190d8d8 (diff)
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
-rw-r--r--app/controllers/directories_controller.rb10
-rw-r--r--app/lib/activitypub/activity/announce.rb4
-rw-r--r--app/lib/activitypub/activity/create.rb2
-rw-r--r--app/models/account.rb11
-rw-r--r--app/models/account_tag_stat.rb24
-rw-r--r--app/models/tag.rb38
-rw-r--r--app/models/tag_filter.rb2
-rw-r--r--app/models/trending_tags.rb12
-rw-r--r--app/services/process_hashtags_service.rb3
-rw-r--r--app/services/reblog_service.rb11
-rw-r--r--app/views/admin/tags/_tag.html.haml2
-rw-r--r--app/views/admin/tags/index.html.haml9
-rw-r--r--app/views/admin/tags/show.html.haml13
-rw-r--r--config/locales/en.yml7
-rw-r--r--config/locales/simple_form.en.yml2
-rw-r--r--config/routes.rb2
-rw-r--r--db/post_migrate/20210502233513_drop_account_tag_stats.rb13
-rw-r--r--db/schema.rb10
-rw-r--r--spec/models/account_tag_stat_spec.rb38
-rw-r--r--spec/models/trending_tags_spec.rb6
20 files changed, 59 insertions, 160 deletions
diff --git a/app/controllers/directories_controller.rb b/app/controllers/directories_controller.rb
index f198ad5ba5b..f28c5b2afb5 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 a1081522eb1..9f778ffb985 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 c7a655d9db4..e46361c1473 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 a573365de21..994459338b0 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 3c36c155abe..00000000000
--- 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 efffc7eee4c..735c3060858 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 a9ff5b703c0..85bfcbea5a7 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 9c2aa0ee8b2..31890b08246 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 e8e139b058b..c42b79db801 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 5032397b34f..744bdf5673a 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 287d28e53b6..adf4ca7b2f8 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 d7719d45d62..d78f3c6d167 100644
--- a/app/views/admin/tags/index.html.haml
+++ b/app/views/admin/tags/index.html.haml
@@ -6,12 +6,6 @@
.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
%li= filter_link_to t('generic.all'), reviewed: nil, unreviewed: nil, pending_review: nil
@@ -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 c9a147587c6..c4caffda1e1 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 bfa4898174d..d8ad5bd84a5 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 8ff880ebc04..c4388ffc529 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 8ca7fccdd93..2373d8a51b9 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 00000000000..80adadcab07
--- /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 88e9060794b..19b1afe00d2 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 6d3057f3555..00000000000
--- 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 b6122c99481..dfbc7d6f805 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|