From bbf74498f57513751f3506e3bbf7a04be4ad3b67 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 7 Nov 2022 22:35:53 +0100 Subject: [PATCH] Fix validation error in SynchronizeFeaturedTagsCollectionWorker (#20018) * Fix followers count not being updated when migrating follows Fixes #19900 * Fix validation error in SynchronizeFeaturedTagsCollectionWorker Also saves remote user's chosen case for hashtags * Limit remote featured tags before validation --- app/models/featured_tag.rb | 2 + .../fetch_featured_tags_collection_service.rb | 20 ++-- ...h_featured_tags_collection_service_spec.rb | 95 +++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 spec/services/activitypub/fetch_featured_tags_collection_service_spec.rb diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index 78185b2a99..debae22121 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -63,6 +63,8 @@ class FeaturedTag < ApplicationRecord end def validate_featured_tags_limit + return unless account.local? + errors.add(:base, I18n.t('featured_tags.errors.limit')) if account.featured_tags.count >= LIMIT end diff --git a/app/services/activitypub/fetch_featured_tags_collection_service.rb b/app/services/activitypub/fetch_featured_tags_collection_service.rb index 5559199381..ab047a0f8b 100644 --- a/app/services/activitypub/fetch_featured_tags_collection_service.rb +++ b/app/services/activitypub/fetch_featured_tags_collection_service.rb @@ -51,21 +51,17 @@ class ActivityPub::FetchFeaturedTagsCollectionService < BaseService end def process_items(items) - names = items.filter_map { |item| item['type'] == 'Hashtag' && item['name']&.delete_prefix('#') }.map { |name| HashtagNormalizer.new.normalize(name) } - to_remove = [] - to_add = names + names = items.filter_map { |item| item['type'] == 'Hashtag' && item['name']&.delete_prefix('#') }.take(FeaturedTag::LIMIT) + tags = names.index_by { |name| HashtagNormalizer.new.normalize(name) } + normalized_names = tags.keys - FeaturedTag.where(account: @account).map(&:name).each do |name| - if names.include?(name) - to_add.delete(name) - else - to_remove << name - end + FeaturedTag.includes(:tag).references(:tag).where(account: @account).where.not(tag: { name: normalized_names }).delete_all + + FeaturedTag.includes(:tag).references(:tag).where(account: @account, tag: { name: normalized_names }).each do |featured_tag| + featured_tag.update(name: tags.delete(featured_tag.tag.name)) end - FeaturedTag.includes(:tag).where(account: @account, tags: { name: to_remove }).delete_all unless to_remove.empty? - - to_add.each do |name| + tags.each_value do |name| FeaturedTag.create!(account: @account, name: name) end end diff --git a/spec/services/activitypub/fetch_featured_tags_collection_service_spec.rb b/spec/services/activitypub/fetch_featured_tags_collection_service_spec.rb new file mode 100644 index 0000000000..6ca22c9fc6 --- /dev/null +++ b/spec/services/activitypub/fetch_featured_tags_collection_service_spec.rb @@ -0,0 +1,95 @@ +require 'rails_helper' + +RSpec.describe ActivityPub::FetchFeaturedTagsCollectionService, type: :service do + let(:collection_url) { 'https://example.com/account/tags' } + let(:actor) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/account') } + + let(:items) do + [ + { type: 'Hashtag', href: 'https://example.com/account/tagged/foo', name: 'Foo' }, + { type: 'Hashtag', href: 'https://example.com/account/tagged/bar', name: 'bar' }, + { type: 'Hashtag', href: 'https://example.com/account/tagged/baz', name: 'baZ' }, + ] + end + + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_url, + items: items, + }.with_indifferent_access + end + + subject { described_class.new } + + shared_examples 'sets featured tags' do + before do + subject.call(actor, collection_url) + end + + it 'sets expected tags as pinned tags' do + expect(actor.featured_tags.map(&:display_name)).to match_array ['Foo', 'bar', 'baZ'] + end + end + + describe '#call' do + context 'when the endpoint is a Collection' do + before do + stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'sets featured tags' + end + + context 'when the account already has featured tags' do + before do + stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload)) + + actor.featured_tags.create!(name: 'FoO') + actor.featured_tags.create!(name: 'baz') + actor.featured_tags.create!(name: 'oh').update(name: nil) + end + + it_behaves_like 'sets featured tags' + end + + context 'when the endpoint is an OrderedCollection' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'OrderedCollection', + id: collection_url, + orderedItems: items, + }.with_indifferent_access + end + + before do + stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'sets featured tags' + end + + context 'when the endpoint is a paginated Collection' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_url, + first: { + type: 'CollectionPage', + partOf: collection_url, + items: items, + } + }.with_indifferent_access + end + + before do + stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'sets featured tags' + end + end +end