From 344e2903b3d0df7b6c07a47d1e52548bf923e50a Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 20 Dec 2024 12:50:31 +0100 Subject: [PATCH] Fix featured tags for remote accounts not being kept up to date (#33372) --- app/lib/activitypub/activity/create.rb | 10 ++ .../process_status_update_service.rb | 21 ++- spec/lib/activitypub/activity/create_spec.rb | 128 +++++++++++++++--- .../process_status_update_service_spec.rb | 12 +- 4 files changed, 147 insertions(+), 24 deletions(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 85a66c6852..a756912592 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -53,6 +53,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity ApplicationRecord.transaction do @status = Status.create!(@params) attach_tags(@status) + attach_mentions(@status) attach_counts(@status) end @@ -161,6 +162,15 @@ class ActivityPub::Activity::Create < ActivityPub::Activity # not a big deal Trends.tags.register(status) + # Update featured tags + return if @tags.empty? || !status.distributable? + + @account.featured_tags.where(tag_id: @tags.pluck(:id)).find_each do |featured_tag| + featured_tag.increment(status.created_at) + end + end + + def attach_mentions(status) @mentions.each do |mention| mention.status = status mention.save diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index dba4ea7aef..b545b094ff 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -186,7 +186,26 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end def update_tags! - @status.tags = Tag.find_or_create_by_names(@raw_tags) + previous_tags = @status.tags.to_a + current_tags = @status.tags = Tag.find_or_create_by_names(@raw_tags) + + return unless @status.distributable? + + added_tags = current_tags - previous_tags + + unless added_tags.empty? + @account.featured_tags.where(tag_id: added_tags.pluck(:id)).find_each do |featured_tag| + featured_tag.increment(@status.created_at) + end + end + + removed_tags = previous_tags - current_tags + + unless removed_tags.empty? + @account.featured_tags.where(tag_id: removed_tags.pluck(:id)).find_each do |featured_tag| + featured_tag.decrement(@status) + end + end end def update_mentions! diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 9482a30959..34b6fb2978 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -160,10 +160,6 @@ RSpec.describe ActivityPub::Activity::Create do context 'when fetching' do subject { described_class.new(json, sender) } - before do - subject.perform - end - context 'when object publication date is below ISO8601 range' do let(:object_json) do { @@ -175,6 +171,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status with a valid creation date', :aggregate_failures do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -195,6 +193,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status with a valid creation date', :aggregate_failures do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -216,6 +216,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status with appropriate creation and edition dates', :aggregate_failures do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -239,17 +241,13 @@ RSpec.describe ActivityPub::Activity::Create do } end - it 'creates status' do + it 'creates status and does not mark it as edited' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil expect(status.text).to eq 'Lorem ipsum' - end - - it 'does not mark status as edited' do - status = sender.statuses.first - - expect(status).to_not be_nil expect(status.edited?).to be false end end @@ -264,7 +262,7 @@ RSpec.describe ActivityPub::Activity::Create do end it 'does not create a status' do - expect(sender.statuses.count).to be_zero + expect { subject.perform }.to_not change(sender.statuses, :count) end end @@ -278,6 +276,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -285,6 +285,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'missing to/cc defaults to direct privacy' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -303,6 +305,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -321,6 +325,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -339,6 +345,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -357,6 +365,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -375,6 +385,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -393,6 +405,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -411,6 +425,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -433,6 +449,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -452,15 +470,13 @@ RSpec.describe ActivityPub::Activity::Create do } end - it 'creates status' do + it 'creates status with a silent mention' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil expect(status.visibility).to eq 'limited' - end - - it 'creates silent mention' do - status = sender.statuses.first expect(status.mentions.first).to be_silent end end @@ -482,6 +498,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -502,6 +520,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -530,6 +550,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -552,6 +574,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil end @@ -579,6 +603,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status with correctly-ordered media attachments' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -605,6 +631,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -630,6 +658,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -655,6 +685,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -678,6 +710,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil end @@ -700,6 +734,42 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.tags.map(&:name)).to include('test') + end + end + + context 'with featured hashtags' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + to: 'https://www.w3.org/ns/activitystreams#Public', + tag: [ + { + type: 'Hashtag', + href: 'http://example.com/blah', + name: '#test', + }, + ], + } + end + + before do + sender.featured_tags.create!(name: 'test') + end + + it 'creates status and updates featured tag' do + expect { subject.perform } + .to change(sender.statuses, :count).by(1) + .and change { sender.featured_tags.first.reload.statuses_count }.by(1) + .and change { sender.featured_tags.first.reload.last_status_at }.from(nil).to(be_within(0.1).of(Time.now.utc)) + status = sender.statuses.first expect(status).to_not be_nil @@ -723,6 +793,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil end @@ -745,6 +817,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil end @@ -769,6 +843,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -795,6 +871,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil @@ -820,6 +898,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil end @@ -841,6 +921,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'creates status' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil end @@ -871,13 +953,13 @@ RSpec.describe ActivityPub::Activity::Create do } end - it 'creates status' do + it 'creates status with a poll' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status).to_not be_nil expect(status.poll).to_not be_nil - end - it 'creates a poll' do poll = sender.polls.first expect(poll).to_not be_nil expect(poll.status).to_not be_nil @@ -900,6 +982,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'adds a vote to the poll with correct uri' do + expect { subject.perform }.to change(poll.votes, :count).by(1) + vote = poll.votes.first expect(vote).to_not be_nil expect(vote.uri).to eq object_json[:id] @@ -925,6 +1009,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'does not add a vote to the poll' do + expect { subject.perform }.to_not change(poll.votes, :count) + expect(poll.votes.first).to be_nil end end @@ -949,6 +1035,8 @@ RSpec.describe ActivityPub::Activity::Create do end it 'uses the counts from the created object' do + expect { subject.perform }.to change(sender.statuses, :count).by(1) + status = sender.statuses.first expect(status.untrusted_favourites_count).to eq 50 expect(status.untrusted_reblogs_count).to eq 100 diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index ed6e8e3167..176af7d432 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -256,16 +256,22 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do updated: '2021-09-08T22:39:25Z', tag: [ { type: 'Hashtag', name: 'foo' }, + { type: 'Hashtag', name: 'bar' }, ], } end before do - subject.call(status, json, json) + status.account.featured_tags.create!(name: 'bar') + status.account.featured_tags.create!(name: 'test') end - it 'updates tags' do - expect(status.tags.reload.map(&:name)).to eq %w(foo) + it 'updates tags and featured tags' do + expect { subject.call(status, json, json) } + .to change { status.tags.reload.pluck(:name) }.from(%w(test foo)).to(%w(foo bar)) + .and change { status.account.featured_tags.find_by(name: 'test').statuses_count }.by(-1) + .and change { status.account.featured_tags.find_by(name: 'bar').statuses_count }.by(1) + .and change { status.account.featured_tags.find_by(name: 'bar').last_status_at }.from(nil).to(be_within(0.1).of(Time.now.utc)) end end