From 2340f4df81fccad0a0a53c71abb61c387eafcef9 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 29 Jan 2025 11:15:32 +0100 Subject: [PATCH 01/17] =?UTF-8?q?Fix=20=E2=80=9Cx=E2=80=9D=20hotkey=20not?= =?UTF-8?q?=20working=20on=20boosted=20filtered=20posts=20(#33758)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/javascript/mastodon/components/status.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/mastodon/components/status.jsx b/app/javascript/mastodon/components/status.jsx index 4e85285a08..cafa57a099 100644 --- a/app/javascript/mastodon/components/status.jsx +++ b/app/javascript/mastodon/components/status.jsx @@ -330,7 +330,7 @@ class Status extends ImmutablePureComponent { const { onToggleHidden } = this.props; const status = this._properStatus(); - if (status.get('matched_filters')) { + if (this.props.status.get('matched_filters')) { const expandedBecauseOfCW = !status.get('hidden') || status.get('spoiler_text').length === 0; const expandedBecauseOfFilter = this.state.showDespiteFilter; From 9be391514bae4ddc144a455f578cff9f1ce74087 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 24 Mar 2025 15:07:27 +0100 Subject: [PATCH 02/17] Fix streaming server refusing unix socket path in `DATABASE_URL` (#34091) --- streaming/database.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/streaming/database.js b/streaming/database.js index 9f1d742143..d5caec4fca 100644 --- a/streaming/database.js +++ b/streaming/database.js @@ -49,7 +49,7 @@ export function configFromEnv(env, environment) { if (typeof parsedUrl.password === 'string') baseConfig.password = parsedUrl.password; if (typeof parsedUrl.host === 'string') baseConfig.host = parsedUrl.host; if (typeof parsedUrl.user === 'string') baseConfig.user = parsedUrl.user; - if (typeof parsedUrl.port === 'string') { + if (typeof parsedUrl.port === 'string' && parsedUrl.port) { const parsedPort = parseInt(parsedUrl.port, 10); if (isNaN(parsedPort)) { throw new Error('Invalid port specified in DATABASE_URL environment variable'); From c48413ad4c5ede89c51723fc4a0ca42e488e3c14 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 17 Mar 2025 17:40:28 +0100 Subject: [PATCH 03/17] Fix incorrect URL being used when cache busting (#34189) --- app/models/media_attachment.rb | 2 +- app/services/suspend_account_service.rb | 2 +- app/services/unsuspend_account_service.rb | 2 +- spec/models/media_attachment_spec.rb | 8 ++--- spec/services/suspend_account_service_spec.rb | 30 ++++++++----------- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 6708cd7793..6dc8e4a3d1 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -421,7 +421,7 @@ class MediaAttachment < ApplicationRecord @paths_to_cache_bust = MediaAttachment.attachment_definitions.keys.flat_map do |attachment_name| attachment = public_send(attachment_name) styles = DEFAULT_STYLES | attachment.styles.keys - styles.map { |style| attachment.path(style) } + styles.map { |style| attachment.url(style) } end.compact rescue => e # We really don't want any error here preventing media deletion diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 8d5446f1a8..561ac665cb 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -95,7 +95,7 @@ class SuspendAccountService < BaseService end end - CacheBusterWorker.perform_async(attachment.path(style)) if Rails.configuration.x.cache_buster_enabled + CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster_enabled end end end diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb index 652dd6a845..7d3bb806a6 100644 --- a/app/services/unsuspend_account_service.rb +++ b/app/services/unsuspend_account_service.rb @@ -91,7 +91,7 @@ class UnsuspendAccountService < BaseService end end - CacheBusterWorker.perform_async(attachment.path(style)) if Rails.configuration.x.cache_buster_enabled + CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster_enabled end end end diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 5f91ae0967..bf818c1e1e 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -295,12 +295,12 @@ RSpec.describe MediaAttachment, :attachment_processing do end it 'queues CacheBusterWorker jobs' do - original_path = media.file.path(:original) - small_path = media.file.path(:small) + original_url = media.file.url(:original) + small_url = media.file.url(:small) expect { media.destroy } - .to enqueue_sidekiq_job(CacheBusterWorker).with(original_path) - .and enqueue_sidekiq_job(CacheBusterWorker).with(small_path) + .to enqueue_sidekiq_job(CacheBusterWorker).with(original_url) + .and enqueue_sidekiq_job(CacheBusterWorker).with(small_url) end end diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb index 4a2f494e0c..c15c23ca30 100644 --- a/spec/services/suspend_account_service_spec.rb +++ b/spec/services/suspend_account_service_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe SuspendAccountService, :inline_jobs do +RSpec.describe SuspendAccountService do shared_examples 'common behavior' do subject { described_class.new.call(account) } @@ -11,6 +11,7 @@ RSpec.describe SuspendAccountService, :inline_jobs do before do allow(FeedManager.instance).to receive_messages(unmerge_from_home: nil, unmerge_from_list: nil) + allow(Rails.configuration.x).to receive(:cache_buster_enabled).and_return(true) local_follower.follow!(account) list.accounts << account @@ -23,6 +24,7 @@ RSpec.describe SuspendAccountService, :inline_jobs do it 'unmerges from feeds of local followers and changes file mode and preserves suspended flag' do expect { subject } .to change_file_mode + .and enqueue_sidekiq_job(CacheBusterWorker).with(account.media_attachments.first.file.url(:original)) .and not_change_suspended_flag expect(FeedManager.instance).to have_received(:unmerge_from_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) @@ -38,17 +40,12 @@ RSpec.describe SuspendAccountService, :inline_jobs do end describe 'suspending a local account' do - def match_update_actor_request(req, account) - json = JSON.parse(req.body) + def match_update_actor_request(json, account) + json = JSON.parse(json) actor_id = ActivityPub::TagManager.instance.uri_for(account) json['type'] == 'Update' && json['actor'] == actor_id && json['object']['id'] == actor_id && json['object']['suspended'] end - before do - stub_request(:post, 'https://alice.com/inbox').to_return(status: 201) - stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) - end - include_examples 'common behavior' do let!(:account) { Fabricate(:account) } let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub, domain: 'alice.com') } @@ -61,22 +58,20 @@ RSpec.describe SuspendAccountService, :inline_jobs do it 'sends an Update actor activity to followers and reporters' do subject - expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once - expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once + + expect(ActivityPub::DeliveryWorker) + .to have_enqueued_sidekiq_job(satisfying { |json| match_update_actor_request(json, account) }, account.id, remote_follower.inbox_url).once + .and have_enqueued_sidekiq_job(satisfying { |json| match_update_actor_request(json, account) }, account.id, remote_reporter.inbox_url).once end end end describe 'suspending a remote account' do - def match_reject_follow_request(req, account, followee) - json = JSON.parse(req.body) + def match_reject_follow_request(json, account, followee) + json = JSON.parse(json) json['type'] == 'Reject' && json['actor'] == ActivityPub::TagManager.instance.uri_for(followee) && json['object']['actor'] == account.uri end - before do - stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) - end - include_examples 'common behavior' do let!(:account) { Fabricate(:account, domain: 'bob.com', uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } let!(:local_followee) { Fabricate(:account) } @@ -88,7 +83,8 @@ RSpec.describe SuspendAccountService, :inline_jobs do it 'sends a Reject Follow activity', :aggregate_failures do subject - expect(a_request(:post, account.inbox_url).with { |req| match_reject_follow_request(req, account, local_followee) }).to have_been_made.once + expect(ActivityPub::DeliveryWorker) + .to have_enqueued_sidekiq_job(satisfying { |json| match_reject_follow_request(json, account, local_followee) }, local_followee.id, account.inbox_url).once end end end From 8197e65cb3615c8d3903bd3160bbb9b8cee70cc9 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 24 Mar 2025 09:58:15 +0100 Subject: [PATCH 04/17] Fix `CacheBuster` being queued for missing media attachments (#34253) --- app/models/media_attachment.rb | 2 ++ spec/models/media_attachment_spec.rb | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 6dc8e4a3d1..ccf685b353 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -420,6 +420,8 @@ class MediaAttachment < ApplicationRecord @paths_to_cache_bust = MediaAttachment.attachment_definitions.keys.flat_map do |attachment_name| attachment = public_send(attachment_name) + next if attachment.blank? + styles = DEFAULT_STYLES | attachment.styles.keys styles.map { |style| attachment.url(style) } end.compact diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index bf818c1e1e..43e9ed087b 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -302,6 +302,15 @@ RSpec.describe MediaAttachment, :attachment_processing do .to enqueue_sidekiq_job(CacheBusterWorker).with(original_url) .and enqueue_sidekiq_job(CacheBusterWorker).with(small_url) end + + context 'with a missing remote attachment' do + let(:media) { Fabricate(:media_attachment, remote_url: 'https://example.com/foo.png', file: nil) } + + it 'does not queue CacheBusterWorker jobs' do + expect { media.destroy } + .to_not enqueue_sidekiq_job(CacheBusterWorker) + end + end end private From 4cb3fe35beb5f85186d68f174179e9d67c704113 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 18 Mar 2025 15:50:41 +0100 Subject: [PATCH 05/17] Fix handling of malformed/unusual HTML (#34201) --- app/helpers/admin/trends/statuses_helper.rb | 17 ++++++++++++----- app/lib/emoji_formatter.rb | 10 +++++++++- app/lib/plain_text_formatter.rb | 10 +++++++++- app/models/account/field.rb | 9 ++++++++- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/app/helpers/admin/trends/statuses_helper.rb b/app/helpers/admin/trends/statuses_helper.rb index c7a59660cf..33da1f7216 100644 --- a/app/helpers/admin/trends/statuses_helper.rb +++ b/app/helpers/admin/trends/statuses_helper.rb @@ -2,11 +2,18 @@ module Admin::Trends::StatusesHelper def one_line_preview(status) - text = if status.local? - status.text.split("\n").first - else - Nokogiri::HTML5(status.text).css('html > body > *').first&.text - end + text = begin + if status.local? + status.text.split("\n").first + else + Nokogiri::HTML5(status.text).css('html > body > *').first&.text + end + rescue ArgumentError + # This can happen if one of the Nokogumbo limits is encountered + # Unfortunately, it does not use a more precise error class + # nor allows more graceful handling + '' + end return '' if text.blank? diff --git a/app/lib/emoji_formatter.rb b/app/lib/emoji_formatter.rb index 5f1a4651f7..a31353616d 100644 --- a/app/lib/emoji_formatter.rb +++ b/app/lib/emoji_formatter.rb @@ -24,7 +24,15 @@ class EmojiFormatter def to_s return html if custom_emojis.empty? || html.blank? - tree = Nokogiri::HTML5.fragment(html) + begin + tree = Nokogiri::HTML5.fragment(html) + rescue ArgumentError + # This can happen if one of the Nokogumbo limits is encountered + # Unfortunately, it does not use a more precise error class + # nor allows more graceful handling + return '' + end + tree.xpath('./text()|.//text()[not(ancestor[@class="invisible"])]').to_a.each do |node| i = -1 inside_shortname = false diff --git a/app/lib/plain_text_formatter.rb b/app/lib/plain_text_formatter.rb index f960ba7acc..e8ff79806f 100644 --- a/app/lib/plain_text_formatter.rb +++ b/app/lib/plain_text_formatter.rb @@ -16,7 +16,15 @@ class PlainTextFormatter if local? text else - node = Nokogiri::HTML5.fragment(insert_newlines) + begin + node = Nokogiri::HTML5.fragment(insert_newlines) + rescue ArgumentError + # This can happen if one of the Nokogumbo limits is encountered + # Unfortunately, it does not use a more precise error class + # nor allows more graceful handling + return '' + end + # Elements that are entirely removed with our Sanitize config node.xpath('.//iframe|.//math|.//noembed|.//noframes|.//noscript|.//plaintext|.//script|.//style|.//svg|.//xmp').remove node.text.chomp diff --git a/app/models/account/field.rb b/app/models/account/field.rb index bcd89015de..4b3ccea9c4 100644 --- a/app/models/account/field.rb +++ b/app/models/account/field.rb @@ -73,7 +73,14 @@ class Account::Field < ActiveModelSerializers::Model end def extract_url_from_html - doc = Nokogiri::HTML5.fragment(value) + begin + doc = Nokogiri::HTML5.fragment(value) + rescue ArgumentError + # This can happen if one of the Nokogumbo limits is encountered + # Unfortunately, it does not use a more precise error class + # nor allows more graceful handling + return + end return if doc.nil? return if doc.children.size != 1 From 653868bb0c381044522690a6c4f1f8727c621c95 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 24 Mar 2025 10:59:05 +0100 Subject: [PATCH 06/17] Change user archive signed URL TTL from 10 seconds to 1 hour (#34254) --- app/controllers/backups_controller.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/backups_controller.rb b/app/controllers/backups_controller.rb index 5df1af5f2f..076d19874b 100644 --- a/app/controllers/backups_controller.rb +++ b/app/controllers/backups_controller.rb @@ -9,13 +9,15 @@ class BackupsController < ApplicationController before_action :authenticate_user! before_action :set_backup + BACKUP_LINK_TIMEOUT = 1.hour.freeze + def download case Paperclip::Attachment.default_options[:storage] when :s3, :azure - redirect_to @backup.dump.expiring_url(10), allow_other_host: true + redirect_to @backup.dump.expiring_url(BACKUP_LINK_TIMEOUT.to_i), allow_other_host: true when :fog if Paperclip::Attachment.default_options.dig(:fog_credentials, :openstack_temp_url_key).present? - redirect_to @backup.dump.expiring_url(Time.now.utc + 10), allow_other_host: true + redirect_to @backup.dump.expiring_url(BACKUP_LINK_TIMEOUT.from_now), allow_other_host: true else redirect_to full_asset_url(@backup.dump.url), allow_other_host: true end From d6442b5455de734e84ee5ecfec4ea007f19d788a Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Mar 2025 14:11:49 +0100 Subject: [PATCH 07/17] Fix filters not applying in detailed view (#34259) --- .../status/components/detailed_status.tsx | 37 ++++++++++++++----- .../mastodon/features/status/index.jsx | 2 +- app/javascript/mastodon/selectors/index.js | 5 ++- app/javascript/mastodon/utils/filters.ts | 2 + 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/app/javascript/mastodon/features/status/components/detailed_status.tsx b/app/javascript/mastodon/features/status/components/detailed_status.tsx index 7ad82edb31..5d1df89b88 100644 --- a/app/javascript/mastodon/features/status/components/detailed_status.tsx +++ b/app/javascript/mastodon/features/status/components/detailed_status.tsx @@ -15,6 +15,7 @@ import AlternateEmailIcon from '@/material-icons/400-24px/alternate_email.svg?re import { AnimatedNumber } from 'mastodon/components/animated_number'; import { ContentWarning } from 'mastodon/components/content_warning'; import EditedTimestamp from 'mastodon/components/edited_timestamp'; +import { FilterWarning } from 'mastodon/components/filter_warning'; import type { StatusLike } from 'mastodon/components/hashtag_bar'; import { getHashtagBarForStatus } from 'mastodon/components/hashtag_bar'; import { Icon } from 'mastodon/components/icon'; @@ -68,6 +69,7 @@ export const DetailedStatus: React.FC<{ }) => { const properStatus = status?.get('reblog') ?? status; const [height, setHeight] = useState(0); + const [showDespiteFilter, setShowDespiteFilter] = useState(false); const nodeRef = useRef(); const handleOpenVideo = useCallback( @@ -80,6 +82,10 @@ export const DetailedStatus: React.FC<{ [onOpenVideo, status], ); + const handleFilterToggle = useCallback(() => { + setShowDespiteFilter(!showDespiteFilter); + }, [showDespiteFilter, setShowDespiteFilter]); + const handleExpandedToggle = useCallback(() => { if (onToggleHidden) onToggleHidden(status); }, [onToggleHidden, status]); @@ -290,8 +296,12 @@ export const DetailedStatus: React.FC<{ const { statusContentProps, hashtagBar } = getHashtagBarForStatus( status as StatusLike, ); + + const matchedFilters = status.get('matched_filters'); + const expanded = - !status.get('hidden') || status.get('spoiler_text').length === 0; + (!matchedFilters || showDespiteFilter) && + (!status.get('hidden') || status.get('spoiler_text').length === 0); return (
@@ -328,17 +338,26 @@ export const DetailedStatus: React.FC<{ )} - {status.get('spoiler_text').length > 0 && ( - )} + {status.get('spoiler_text').length > 0 && + (!matchedFilters || showDespiteFilter) && ( + + )} + {expanded && ( <> { }); const mapStateToProps = (state, props) => { - const status = getStatus(state, { id: props.params.statusId }); + const status = getStatus(state, { id: props.params.statusId, contextType: 'detailed' }); let ancestorsIds = Immutable.List(); let descendantsIds = Immutable.List(); diff --git a/app/javascript/mastodon/selectors/index.js b/app/javascript/mastodon/selectors/index.js index 345ceac49a..b35e4b7a84 100644 --- a/app/javascript/mastodon/selectors/index.js +++ b/app/javascript/mastodon/selectors/index.js @@ -15,9 +15,10 @@ export const makeGetStatus = () => { (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', id, 'account'])]), (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', state.getIn(['statuses', id, 'reblog']), 'account'])]), getFilters, + (_, { contextType }) => contextType === 'detailed', ], - (statusBase, statusReblog, accountBase, accountReblog, filters) => { + (statusBase, statusReblog, accountBase, accountReblog, filters, warnInsteadOfHide) => { if (!statusBase || statusBase.get('isLoading')) { return null; } @@ -31,7 +32,7 @@ export const makeGetStatus = () => { let filtered = false; if ((accountReblog || accountBase).get('id') !== me && filters) { let filterResults = statusReblog?.get('filtered') || statusBase.get('filtered') || ImmutableList(); - if (filterResults.some((result) => filters.getIn([result.get('filter'), 'filter_action']) === 'hide')) { + if (!warnInsteadOfHide && filterResults.some((result) => filters.getIn([result.get('filter'), 'filter_action']) === 'hide')) { return null; } filterResults = filterResults.filter(result => filters.has(result.get('filter'))); diff --git a/app/javascript/mastodon/utils/filters.ts b/app/javascript/mastodon/utils/filters.ts index d299e80c40..c944599bb9 100644 --- a/app/javascript/mastodon/utils/filters.ts +++ b/app/javascript/mastodon/utils/filters.ts @@ -6,6 +6,8 @@ export const toServerSideType = (columnType: string) => { case 'thread': case 'account': return columnType; + case 'detailed': + return 'thread'; default: if (columnType.includes('list:')) { return 'home'; From 105e5b1d76a56f9fd9b3165f71be7d44fa706a52 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Mar 2025 16:20:36 +0100 Subject: [PATCH 08/17] Fix bookmarks and favourites not being filtered (#34260) --- app/javascript/mastodon/features/bookmarked_statuses/index.jsx | 1 + app/javascript/mastodon/features/favourited_statuses/index.jsx | 1 + app/javascript/mastodon/selectors/index.js | 2 +- app/javascript/mastodon/utils/filters.ts | 3 +++ 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/javascript/mastodon/features/bookmarked_statuses/index.jsx b/app/javascript/mastodon/features/bookmarked_statuses/index.jsx index 5494ad44b0..122baafd6c 100644 --- a/app/javascript/mastodon/features/bookmarked_statuses/index.jsx +++ b/app/javascript/mastodon/features/bookmarked_statuses/index.jsx @@ -99,6 +99,7 @@ class Bookmarks extends ImmutablePureComponent { onLoadMore={this.handleLoadMore} emptyMessage={emptyMessage} bindToDocument={!multiColumn} + timelineId='bookmarks' /> diff --git a/app/javascript/mastodon/features/favourited_statuses/index.jsx b/app/javascript/mastodon/features/favourited_statuses/index.jsx index 8e65ff5b68..9e0b982239 100644 --- a/app/javascript/mastodon/features/favourited_statuses/index.jsx +++ b/app/javascript/mastodon/features/favourited_statuses/index.jsx @@ -99,6 +99,7 @@ class Favourites extends ImmutablePureComponent { onLoadMore={this.handleLoadMore} emptyMessage={emptyMessage} bindToDocument={!multiColumn} + timelineId='favourites' /> diff --git a/app/javascript/mastodon/selectors/index.js b/app/javascript/mastodon/selectors/index.js index b35e4b7a84..7589d5d821 100644 --- a/app/javascript/mastodon/selectors/index.js +++ b/app/javascript/mastodon/selectors/index.js @@ -15,7 +15,7 @@ export const makeGetStatus = () => { (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', id, 'account'])]), (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', state.getIn(['statuses', id, 'reblog']), 'account'])]), getFilters, - (_, { contextType }) => contextType === 'detailed', + (_, { contextType }) => ['detailed', 'bookmarks', 'favourites'].includes(contextType), ], (statusBase, statusReblog, accountBase, accountReblog, filters, warnInsteadOfHide) => { diff --git a/app/javascript/mastodon/utils/filters.ts b/app/javascript/mastodon/utils/filters.ts index c944599bb9..a4a1c38294 100644 --- a/app/javascript/mastodon/utils/filters.ts +++ b/app/javascript/mastodon/utils/filters.ts @@ -8,6 +8,9 @@ export const toServerSideType = (columnType: string) => { return columnType; case 'detailed': return 'thread'; + case 'bookmarks': + case 'favourites': + return 'home'; default: if (columnType.includes('list:')) { return 'home'; From 86d8df0c03d73463293ef35b2a70f20d21ab5da4 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Mar 2025 16:50:05 +0100 Subject: [PATCH 09/17] Fix follower synchronization mechanism erroneously removing followers from multi-page collections (#34272) --- .../synchronize_followers_service.rb | 3 ++ .../synchronize_followers_service_spec.rb | 41 +++++++++++++++---- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/app/services/activitypub/synchronize_followers_service.rb b/app/services/activitypub/synchronize_followers_service.rb index f51d671a00..9d721dfaa1 100644 --- a/app/services/activitypub/synchronize_followers_service.rb +++ b/app/services/activitypub/synchronize_followers_service.rb @@ -57,6 +57,9 @@ class ActivityPub::SynchronizeFollowersService < BaseService collection = fetch_collection(collection['first']) if collection['first'].present? return unless collection.is_a?(Hash) + # Abort if we'd have to paginate through more than one page of followers + return if collection['next'].present? + case collection['type'] when 'Collection', 'CollectionPage' as_array(collection['items']) diff --git a/spec/services/activitypub/synchronize_followers_service_spec.rb b/spec/services/activitypub/synchronize_followers_service_spec.rb index 974368b7d7..3399e4fdf0 100644 --- a/spec/services/activitypub/synchronize_followers_service_spec.rb +++ b/spec/services/activitypub/synchronize_followers_service_spec.rb @@ -27,14 +27,14 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do }.with_indifferent_access end + before do + alice.follow!(actor) + bob.follow!(actor) + mallory.request_follow!(actor) + end + shared_examples 'synchronizes followers' do before do - alice.follow!(actor) - bob.follow!(actor) - mallory.request_follow!(actor) - - allow(ActivityPub::DeliveryWorker).to receive(:perform_async) - subject.call(actor, collection_uri) end @@ -46,7 +46,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do expect(mallory) .to be_following(actor) # Convert follow request to follow when accepted expect(ActivityPub::DeliveryWorker) - .to have_received(:perform_async).with(anything, eve.id, actor.inbox_url) # Send Undo Follow to actor + .to have_enqueued_sidekiq_job(anything, eve.id, actor.inbox_url) # Send Undo Follow to actor end end @@ -76,7 +76,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do it_behaves_like 'synchronizes followers' end - context 'when the endpoint is a paginated Collection of actor URIs' do + context 'when the endpoint is a single-page paginated Collection of actor URIs' do let(:payload) do { '@context': 'https://www.w3.org/ns/activitystreams', @@ -96,5 +96,30 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do it_behaves_like 'synchronizes followers' end + + context 'when the endpoint is a paginated Collection of actor URIs with a next page' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_uri, + first: { + type: 'CollectionPage', + partOf: collection_uri, + items: items, + next: "#{collection_uri}/page2", + }, + }.with_indifferent_access + end + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' }) + end + + it 'does not change followers' do + expect { subject.call(actor, collection_uri) } + .to_not(change { actor.followers.reload.reorder(id: :asc).pluck(:id) }) + end + end end end From 0615febd84755174ca5dfe2e442cfe0cc64fed54 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 26 Mar 2025 12:33:59 +0100 Subject: [PATCH 10/17] Add support for paginating partial collections in `SynchronizeFollowersService` (#34277) --- app/models/account.rb | 2 +- .../synchronize_followers_service.rb | 66 +++++++++----- .../synchronize_followers_service_spec.rb | 86 +++++++++++++++++-- 3 files changed, 128 insertions(+), 26 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index 708415b6ee..a3f724dc5f 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -149,7 +149,7 @@ class Account < ApplicationRecord scope :not_excluded_by_account, ->(account) { where.not(id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { where(arel_table[:domain].eq(nil).or(arel_table[:domain].not_in(account.excluded_from_timeline_domains))) } scope :dormant, -> { joins(:account_stat).merge(AccountStat.without_recent_activity) } - scope :with_username, ->(value) { where arel_table[:username].lower.eq(value.to_s.downcase) } + scope :with_username, ->(value) { value.is_a?(Array) ? where(arel_table[:username].lower.in(value.map { |x| x.to_s.downcase })) : where(arel_table[:username].lower.eq(value.to_s.downcase)) } scope :with_domain, ->(value) { where arel_table[:domain].lower.eq(value&.to_s&.downcase) } scope :without_memorial, -> { where(memorial: false) } scope :duplicate_uris, -> { select(:uri, Arel.star.count).group(:uri).having(Arel.star.count.gt(1)) } diff --git a/app/services/activitypub/synchronize_followers_service.rb b/app/services/activitypub/synchronize_followers_service.rb index 9d721dfaa1..82d84a2f21 100644 --- a/app/services/activitypub/synchronize_followers_service.rb +++ b/app/services/activitypub/synchronize_followers_service.rb @@ -4,32 +4,46 @@ class ActivityPub::SynchronizeFollowersService < BaseService include JsonLdHelper include Payloadable + MAX_COLLECTION_PAGES = 10 + def call(account, partial_collection_url) @account = account + @expected_followers_ids = [] - items = collection_items(partial_collection_url) - return if items.nil? - - # There could be unresolved accounts (hence the call to .compact) but this - # should never happen in practice, since in almost all cases we keep an - # Account record, and should we not do that, we should have sent a Delete. - # In any case there is not much we can do if that occurs. - @expected_followers = items.filter_map { |uri| ActivityPub::TagManager.instance.uri_to_resource(uri, Account) } + return unless process_collection!(partial_collection_url) remove_unexpected_local_followers! - handle_unexpected_outgoing_follows! end private + def process_page!(items) + page_expected_followers = extract_local_followers(items) + @expected_followers_ids.concat(page_expected_followers.pluck(:id)) + + handle_unexpected_outgoing_follows!(page_expected_followers) + end + + def extract_local_followers(items) + # There could be unresolved accounts (hence the call to .filter_map) but this + # should never happen in practice, since in almost all cases we keep an + # Account record, and should we not do that, we should have sent a Delete. + # In any case there is not much we can do if that occurs. + + # TODO: this will need changes when switching to numeric IDs + + usernames = items.filter_map { |uri| ActivityPub::TagManager.instance.uri_to_local_id(uri, :username)&.downcase } + Account.local.with_username(usernames) + end + def remove_unexpected_local_followers! - @account.followers.local.where.not(id: @expected_followers.map(&:id)).reorder(nil).find_each do |unexpected_follower| + @account.followers.local.where.not(id: @expected_followers_ids).reorder(nil).find_each do |unexpected_follower| UnfollowService.new.call(unexpected_follower, @account) end end - def handle_unexpected_outgoing_follows! - @expected_followers.each do |expected_follower| + def handle_unexpected_outgoing_follows!(expected_followers) + expected_followers.each do |expected_follower| next if expected_follower.following?(@account) if expected_follower.requested?(@account) @@ -50,21 +64,33 @@ class ActivityPub::SynchronizeFollowersService < BaseService Oj.dump(serialize_payload(follow, ActivityPub::UndoFollowSerializer)) end - def collection_items(collection_or_uri) - collection = fetch_collection(collection_or_uri) - return unless collection.is_a?(Hash) + # Only returns true if the whole collection has been processed + def process_collection!(collection_uri, max_pages: MAX_COLLECTION_PAGES) + collection = fetch_collection(collection_uri) + return false unless collection.is_a?(Hash) collection = fetch_collection(collection['first']) if collection['first'].present? - return unless collection.is_a?(Hash) - # Abort if we'd have to paginate through more than one page of followers - return if collection['next'].present? + while collection.is_a?(Hash) + process_page!(as_array(collection_page_items(collection))) + max_pages -= 1 + + return true if collection['next'].blank? # We reached the end of the collection + return false if max_pages <= 0 # We reached our pages limit + + collection = fetch_collection(collection['next']) + end + + false + end + + def collection_page_items(collection) case collection['type'] when 'Collection', 'CollectionPage' - as_array(collection['items']) + collection['items'] when 'OrderedCollection', 'OrderedCollectionPage' - as_array(collection['orderedItems']) + collection['orderedItems'] end end diff --git a/spec/services/activitypub/synchronize_followers_service_spec.rb b/spec/services/activitypub/synchronize_followers_service_spec.rb index 3399e4fdf0..70f27627e1 100644 --- a/spec/services/activitypub/synchronize_followers_service_spec.rb +++ b/spec/services/activitypub/synchronize_followers_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do let(:bob) { Fabricate(:account, username: 'bob') } let(:eve) { Fabricate(:account, username: 'eve') } let(:mallory) { Fabricate(:account, username: 'mallory') } - let(:collection_uri) { 'http://example.com/partial-followers' } + let(:collection_uri) { 'https://example.com/partial-followers' } let(:items) do [alice, eve, mallory].map do |account| @@ -97,7 +97,76 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do it_behaves_like 'synchronizes followers' end - context 'when the endpoint is a paginated Collection of actor URIs with a next page' do + context 'when the endpoint is a paginated Collection of actor URIs split across multiple pages' do + before do + stub_request(:get, 'https://example.com/partial-followers') + .to_return(status: 200, headers: { 'Content-Type': 'application/activity+json' }, body: Oj.dump({ + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: 'https://example.com/partial-followers', + first: 'https://example.com/partial-followers/1', + })) + + stub_request(:get, 'https://example.com/partial-followers/1') + .to_return(status: 200, headers: { 'Content-Type': 'application/activity+json' }, body: Oj.dump({ + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'CollectionPage', + id: 'https://example.com/partial-followers/1', + partOf: 'https://example.com/partial-followers', + next: 'https://example.com/partial-followers/2', + items: [alice, eve].map { |account| ActivityPub::TagManager.instance.uri_for(account) }, + })) + + stub_request(:get, 'https://example.com/partial-followers/2') + .to_return(status: 200, headers: { 'Content-Type': 'application/activity+json' }, body: Oj.dump({ + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'CollectionPage', + id: 'https://example.com/partial-followers/2', + partOf: 'https://example.com/partial-followers', + items: ActivityPub::TagManager.instance.uri_for(mallory), + })) + end + + it_behaves_like 'synchronizes followers' + end + + context 'when the endpoint is a paginated Collection of actor URIs split across, but one page errors out' do + before do + stub_request(:get, 'https://example.com/partial-followers') + .to_return(status: 200, headers: { 'Content-Type': 'application/activity+json' }, body: Oj.dump({ + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: 'https://example.com/partial-followers', + first: 'https://example.com/partial-followers/1', + })) + + stub_request(:get, 'https://example.com/partial-followers/1') + .to_return(status: 200, headers: { 'Content-Type': 'application/activity+json' }, body: Oj.dump({ + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'CollectionPage', + id: 'https://example.com/partial-followers/1', + partOf: 'https://example.com/partial-followers', + next: 'https://example.com/partial-followers/2', + items: [mallory].map { |account| ActivityPub::TagManager.instance.uri_for(account) }, + })) + + stub_request(:get, 'https://example.com/partial-followers/2') + .to_return(status: 404) + end + + it 'confirms pending follow request but does not remove extra followers' do + previous_follower_ids = actor.followers.pluck(:id) + + subject.call(actor, collection_uri) + + expect(previous_follower_ids - actor.followers.reload.pluck(:id)) + .to be_empty + expect(mallory) + .to be_following(actor) + end + end + + context 'when the endpoint is a paginated Collection of actor URIs with more pages than we allow' do let(:payload) do { '@context': 'https://www.w3.org/ns/activitystreams', @@ -113,12 +182,19 @@ RSpec.describe ActivityPub::SynchronizeFollowersService do end before do + stub_const('ActivityPub::SynchronizeFollowersService::MAX_COLLECTION_PAGES', 1) stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' }) end - it 'does not change followers' do - expect { subject.call(actor, collection_uri) } - .to_not(change { actor.followers.reload.reorder(id: :asc).pluck(:id) }) + it 'confirms pending follow request but does not remove extra followers' do + previous_follower_ids = actor.followers.pluck(:id) + + subject.call(actor, collection_uri) + + expect(previous_follower_ids - actor.followers.reload.pluck(:id)) + .to be_empty + expect(mallory) + .to be_following(actor) end end end From 8a3f25a4fa1019acd41e30c7c08590a4977c5d6b Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Wed, 26 Mar 2025 14:26:24 +0100 Subject: [PATCH 11/17] Use fixed order in flaky spec (#34279) --- spec/requests/api/v2/notifications_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/api/v2/notifications_spec.rb b/spec/requests/api/v2/notifications_spec.rb index a7608e1419..72df063d49 100644 --- a/spec/requests/api/v2/notifications_spec.rb +++ b/spec/requests/api/v2/notifications_spec.rb @@ -170,7 +170,7 @@ RSpec.describe 'Notifications' do end context 'with min_id param' do - let(:params) { { min_id: user.account.notifications.reload.first.id - 1 } } + let(:params) { { min_id: user.account.notifications.order(id: :asc).first.id - 1 } } it 'returns a notification group covering all notifications' do subject From 29eae75ca0005c2f29130a9c1250b872270fe103 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 12 Nov 2024 03:38:08 -0500 Subject: [PATCH 12/17] Define constants for sampling sizes in `AccountReachFinder` (#32805) --- app/lib/account_reach_finder.rb | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/app/lib/account_reach_finder.rb b/app/lib/account_reach_finder.rb index 481e254396..19464024a6 100644 --- a/app/lib/account_reach_finder.rb +++ b/app/lib/account_reach_finder.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class AccountReachFinder + RECENT_LIMIT = 2_000 + STATUS_LIMIT = 200 + STATUS_SINCE = 2.days + def initialize(account) @account = account end @@ -20,13 +24,27 @@ class AccountReachFinder end def recently_mentioned_inboxes - cutoff_id = Mastodon::Snowflake.id_at(2.days.ago, with_random: false) - recent_statuses = @account.statuses.recent.where(id: cutoff_id...).limit(200) - - Account.joins(:mentions).where(mentions: { status: recent_statuses }).inboxes.take(2000) + Account + .joins(:mentions) + .where(mentions: { status: recent_statuses }) + .inboxes + .take(RECENT_LIMIT) end def relay_inboxes Relay.enabled.pluck(:inbox_url) end + + def oldest_status_id + Mastodon::Snowflake + .id_at(STATUS_SINCE.ago, with_random: false) + end + + def recent_statuses + @account + .statuses + .recent + .where(id: oldest_status_id...) + .limit(STATUS_LIMIT) + end end From 6af733d1d8ba1ae2b7c14d3b15d10955d62e5226 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 27 Mar 2025 14:41:13 +0100 Subject: [PATCH 13/17] Change `AccountReachFinder` to consider statuses based on suspension date (#34291) --- app/lib/account_reach_finder.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/lib/account_reach_finder.rb b/app/lib/account_reach_finder.rb index 19464024a6..f5e229c6a3 100644 --- a/app/lib/account_reach_finder.rb +++ b/app/lib/account_reach_finder.rb @@ -37,7 +37,11 @@ class AccountReachFinder def oldest_status_id Mastodon::Snowflake - .id_at(STATUS_SINCE.ago, with_random: false) + .id_at(oldest_status_date, with_random: false) + end + + def oldest_status_date + @account.suspended? && @account.suspension_origin_local? ? @account.suspended_at - STATUS_SINCE : STATUS_SINCE.ago end def recent_statuses From d9fb61f3053b9e8e765e067b8317b27d48597321 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 28 Mar 2025 10:20:32 +0100 Subject: [PATCH 14/17] Change account suspensions to be federated to recently-followed accounts as well (#34294) --- app/lib/account_reach_finder.rb | 21 +++++++++++++++++--- spec/lib/account_reach_finder_spec.rb | 28 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/app/lib/account_reach_finder.rb b/app/lib/account_reach_finder.rb index f5e229c6a3..4bf5c229a5 100644 --- a/app/lib/account_reach_finder.rb +++ b/app/lib/account_reach_finder.rb @@ -10,7 +10,7 @@ class AccountReachFinder end def inboxes - (followers_inboxes + reporters_inboxes + recently_mentioned_inboxes + relay_inboxes).uniq + (followers_inboxes + reporters_inboxes + recently_mentioned_inboxes + recently_followed_inboxes + recently_requested_inboxes + relay_inboxes).uniq end private @@ -31,16 +31,31 @@ class AccountReachFinder .take(RECENT_LIMIT) end + def recently_followed_inboxes + @account + .following + .where(follows: { created_at: recent_date_cutoff... }) + .inboxes + .take(RECENT_LIMIT) + end + + def recently_requested_inboxes + Account + .where(id: @account.follow_requests.where({ created_at: recent_date_cutoff... }).select(:target_account_id)) + .inboxes + .take(RECENT_LIMIT) + end + def relay_inboxes Relay.enabled.pluck(:inbox_url) end def oldest_status_id Mastodon::Snowflake - .id_at(oldest_status_date, with_random: false) + .id_at(recent_date_cutoff, with_random: false) end - def oldest_status_date + def recent_date_cutoff @account.suspended? && @account.suspension_origin_local? ? @account.suspended_at - STATUS_SINCE : STATUS_SINCE.ago end diff --git a/spec/lib/account_reach_finder_spec.rb b/spec/lib/account_reach_finder_spec.rb index 0c1d92b2da..ed16c07c22 100644 --- a/spec/lib/account_reach_finder_spec.rb +++ b/spec/lib/account_reach_finder_spec.rb @@ -13,13 +13,28 @@ RSpec.describe AccountReachFinder do let(:ap_mentioned_example_com) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-3', domain: 'example.com') } let(:ap_mentioned_example_org) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.org/inbox-4', domain: 'example.org') } + let(:ap_followed_example_com) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-5', domain: 'example.com') } + let(:ap_followed_example_org) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-6', domain: 'example.org') } + + let(:ap_requested_example_com) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-7', domain: 'example.com') } + let(:ap_requested_example_org) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-8', domain: 'example.org') } + let(:unrelated_account) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/unrelated-inbox', domain: 'example.com') } + let(:old_followed_account) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/old-followed-inbox', domain: 'example.com') } before do + travel_to(2.months.ago) { account.follow!(old_followed_account) } + ap_follower_example_com.follow!(account) ap_follower_example_org.follow!(account) ap_follower_with_shared.follow!(account) + account.follow!(ap_followed_example_com) + account.follow!(ap_followed_example_org) + + account.request_follow!(ap_requested_example_com) + account.request_follow!(ap_requested_example_org) + Fabricate(:status, account: account).tap do |status| status.mentions << Mention.new(account: ap_follower_example_com) status.mentions << Mention.new(account: ap_mentioned_with_shared) @@ -44,7 +59,10 @@ RSpec.describe AccountReachFinder do expect(subject) .to include(*follower_inbox_urls) .and include(*mentioned_account_inbox_urls) + .and include(*recently_followed_inbox_urls) + .and include(*recently_requested_inbox_urls) .and not_include(unrelated_account.preferred_inbox_url) + .and not_include(old_followed_account.preferred_inbox_url) end def follower_inbox_urls @@ -56,5 +74,15 @@ RSpec.describe AccountReachFinder do [ap_mentioned_with_shared, ap_mentioned_example_com, ap_mentioned_example_org] .map(&:preferred_inbox_url) end + + def recently_followed_inbox_urls + [ap_followed_example_com, ap_followed_example_org] + .map(&:preferred_inbox_url) + end + + def recently_requested_inbox_urls + [ap_requested_example_com, ap_requested_example_org] + .map(&:preferred_inbox_url) + end end end From 6d53e8c6c5273bc8405ed8cf10ec6455ad7cc677 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 28 Mar 2025 16:30:28 +0100 Subject: [PATCH 15/17] Add delay to profile updates to debounce them (#34137) --- .../api/v1/accounts/credentials_controller.rb | 2 +- app/controllers/api/v1/profile/avatars_controller.rb | 2 +- app/controllers/api/v1/profile/headers_controller.rb | 2 +- app/controllers/settings/pictures_controller.rb | 2 +- app/controllers/settings/privacy_controller.rb | 2 +- app/controllers/settings/profiles_controller.rb | 2 +- app/controllers/settings/verifications_controller.rb | 2 +- app/workers/activitypub/update_distribution_worker.rb | 2 ++ spec/controllers/settings/privacy_controller_spec.rb | 8 ++++---- spec/controllers/settings/profiles_controller_spec.rb | 8 ++++---- spec/requests/api/v1/accounts/credentials_spec.rb | 4 +--- spec/requests/api/v1/profiles_spec.rb | 10 ++++------ 12 files changed, 22 insertions(+), 24 deletions(-) diff --git a/app/controllers/api/v1/accounts/credentials_controller.rb b/app/controllers/api/v1/accounts/credentials_controller.rb index a378425183..22a51f197d 100644 --- a/app/controllers/api/v1/accounts/credentials_controller.rb +++ b/app/controllers/api/v1/accounts/credentials_controller.rb @@ -14,7 +14,7 @@ class Api::V1::Accounts::CredentialsController < Api::BaseController @account = current_account UpdateAccountService.new.call(@account, account_params, raise_error: true) current_user.update(user_params) if user_params - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) render json: @account, serializer: REST::CredentialAccountSerializer rescue ActiveRecord::RecordInvalid => e render json: ValidationErrorFormatter.new(e).as_json, status: 422 diff --git a/app/controllers/api/v1/profile/avatars_controller.rb b/app/controllers/api/v1/profile/avatars_controller.rb index bc4d01a597..e6c954ed63 100644 --- a/app/controllers/api/v1/profile/avatars_controller.rb +++ b/app/controllers/api/v1/profile/avatars_controller.rb @@ -7,7 +7,7 @@ class Api::V1::Profile::AvatarsController < Api::BaseController def destroy @account = current_account UpdateAccountService.new.call(@account, { avatar: nil }, raise_error: true) - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) render json: @account, serializer: REST::CredentialAccountSerializer end end diff --git a/app/controllers/api/v1/profile/headers_controller.rb b/app/controllers/api/v1/profile/headers_controller.rb index 9f4daa2f77..4472a01b05 100644 --- a/app/controllers/api/v1/profile/headers_controller.rb +++ b/app/controllers/api/v1/profile/headers_controller.rb @@ -7,7 +7,7 @@ class Api::V1::Profile::HeadersController < Api::BaseController def destroy @account = current_account UpdateAccountService.new.call(@account, { header: nil }, raise_error: true) - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) render json: @account, serializer: REST::CredentialAccountSerializer end end diff --git a/app/controllers/settings/pictures_controller.rb b/app/controllers/settings/pictures_controller.rb index 58a4325307..7e61e6d580 100644 --- a/app/controllers/settings/pictures_controller.rb +++ b/app/controllers/settings/pictures_controller.rb @@ -8,7 +8,7 @@ module Settings def destroy if valid_picture? if UpdateAccountService.new.call(@account, { @picture => nil, "#{@picture}_remote_url" => '' }) - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) redirect_to settings_profile_path, notice: I18n.t('generic.changes_saved_msg'), status: 303 else redirect_to settings_profile_path diff --git a/app/controllers/settings/privacy_controller.rb b/app/controllers/settings/privacy_controller.rb index 1102c89fad..6c4836ec56 100644 --- a/app/controllers/settings/privacy_controller.rb +++ b/app/controllers/settings/privacy_controller.rb @@ -8,7 +8,7 @@ class Settings::PrivacyController < Settings::BaseController def update if UpdateAccountService.new.call(@account, account_params.except(:settings)) current_user.update!(settings_attributes: account_params[:settings]) - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) redirect_to settings_privacy_path, notice: I18n.t('generic.changes_saved_msg') else render :show diff --git a/app/controllers/settings/profiles_controller.rb b/app/controllers/settings/profiles_controller.rb index 8ae69b7fe0..2d64132751 100644 --- a/app/controllers/settings/profiles_controller.rb +++ b/app/controllers/settings/profiles_controller.rb @@ -9,7 +9,7 @@ class Settings::ProfilesController < Settings::BaseController def update if UpdateAccountService.new.call(@account, account_params) - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) redirect_to settings_profile_path, notice: I18n.t('generic.changes_saved_msg') else @account.build_fields diff --git a/app/controllers/settings/verifications_controller.rb b/app/controllers/settings/verifications_controller.rb index 4e0663253c..7575be3a78 100644 --- a/app/controllers/settings/verifications_controller.rb +++ b/app/controllers/settings/verifications_controller.rb @@ -8,7 +8,7 @@ class Settings::VerificationsController < Settings::BaseController def update if UpdateAccountService.new.call(@account, account_params) - ActivityPub::UpdateDistributionWorker.perform_async(@account.id) + ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) redirect_to settings_verification_path, notice: I18n.t('generic.changes_saved_msg') else render :show diff --git a/app/workers/activitypub/update_distribution_worker.rb b/app/workers/activitypub/update_distribution_worker.rb index a04ac621f3..9a418f0f3d 100644 --- a/app/workers/activitypub/update_distribution_worker.rb +++ b/app/workers/activitypub/update_distribution_worker.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker + DEBOUNCE_DELAY = 5.seconds + sidekiq_options queue: 'push', lock: :until_executed, lock_ttl: 1.day.to_i # Distribute an profile update to servers that might have a copy diff --git a/spec/controllers/settings/privacy_controller_spec.rb b/spec/controllers/settings/privacy_controller_spec.rb index 59fd342199..9dcabd1d54 100644 --- a/spec/controllers/settings/privacy_controller_spec.rb +++ b/spec/controllers/settings/privacy_controller_spec.rb @@ -31,7 +31,7 @@ RSpec.describe Settings::PrivacyController do describe 'PUT #update' do context 'when update succeeds' do before do - allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) + allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in) end it 'updates the user profile' do @@ -44,14 +44,14 @@ RSpec.describe Settings::PrivacyController do .to redirect_to(settings_privacy_path) expect(ActivityPub::UpdateDistributionWorker) - .to have_received(:perform_async).with(account.id) + .to have_received(:perform_in).with(anything, account.id) end end context 'when update fails' do before do allow(UpdateAccountService).to receive(:new).and_return(failing_update_service) - allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) + allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in) end it 'updates the user profile' do @@ -61,7 +61,7 @@ RSpec.describe Settings::PrivacyController do .to render_template(:show) expect(ActivityPub::UpdateDistributionWorker) - .to_not have_received(:perform_async) + .to_not have_received(:perform_in) end private diff --git a/spec/controllers/settings/profiles_controller_spec.rb b/spec/controllers/settings/profiles_controller_spec.rb index e3197f0a6d..7447049751 100644 --- a/spec/controllers/settings/profiles_controller_spec.rb +++ b/spec/controllers/settings/profiles_controller_spec.rb @@ -29,23 +29,23 @@ RSpec.describe Settings::ProfilesController do end it 'updates the user profile' do - allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) + allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in) put :update, params: { account: { display_name: 'New name' } } expect(account.reload.display_name).to eq 'New name' expect(response).to redirect_to(settings_profile_path) - expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id) + expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id) end end describe 'PUT #update with new profile image' do it 'updates profile image' do - allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) + allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in) expect(account.avatar.instance.avatar_file_name).to be_nil put :update, params: { account: { avatar: fixture_file_upload('avatar.gif', 'image/gif') } } expect(response).to redirect_to(settings_profile_path) expect(account.reload.avatar.instance.avatar_file_name).to_not be_nil - expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id) + expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id) end end end diff --git a/spec/requests/api/v1/accounts/credentials_spec.rb b/spec/requests/api/v1/accounts/credentials_spec.rb index 0bd3ace132..91a4721eca 100644 --- a/spec/requests/api/v1/accounts/credentials_spec.rb +++ b/spec/requests/api/v1/accounts/credentials_spec.rb @@ -53,8 +53,6 @@ RSpec.describe 'credentials API' do patch '/api/v1/accounts/update_credentials', headers: headers, params: params end - before { allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) } - let(:params) do { avatar: fixture_file_upload('avatar.gif', 'image/gif'), @@ -112,7 +110,7 @@ RSpec.describe 'credentials API' do }) expect(ActivityPub::UpdateDistributionWorker) - .to have_received(:perform_async).with(user.account_id) + .to have_enqueued_sidekiq_job(user.account_id) end def expect_account_updates diff --git a/spec/requests/api/v1/profiles_spec.rb b/spec/requests/api/v1/profiles_spec.rb index fd3ab4bf58..de7a20b133 100644 --- a/spec/requests/api/v1/profiles_spec.rb +++ b/spec/requests/api/v1/profiles_spec.rb @@ -15,10 +15,6 @@ RSpec.describe 'Deleting profile images' do let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } describe 'DELETE /api/v1/profile' do - before do - allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) - end - context 'when deleting an avatar' do context 'with wrong scope' do before do @@ -38,7 +34,8 @@ RSpec.describe 'Deleting profile images' do account.reload expect(account.avatar).to_not exist expect(account.header).to exist - expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id) + expect(ActivityPub::UpdateDistributionWorker) + .to have_enqueued_sidekiq_job(account.id) end end @@ -61,7 +58,8 @@ RSpec.describe 'Deleting profile images' do account.reload expect(account.avatar).to exist expect(account.header).to_not exist - expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id) + expect(ActivityPub::UpdateDistributionWorker) + .to have_enqueued_sidekiq_job(account.id) end end end From 51fcb9ca99e1c2a7b31e859dc03d97a9e5a71b12 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Mar 2025 14:11:49 +0100 Subject: [PATCH 16/17] [Glitch] Fix filters not applying in detailed view Port 8c3eeb4d29dd5ed492de3518eea51454d26c6e49 to glitch-soc Signed-off-by: Claire --- .../status/components/detailed_status.tsx | 50 +++++++++++++------ .../flavours/glitch/features/status/index.jsx | 2 +- .../flavours/glitch/selectors/index.js | 5 +- .../flavours/glitch/utils/filters.ts | 2 + 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/app/javascript/flavours/glitch/features/status/components/detailed_status.tsx b/app/javascript/flavours/glitch/features/status/components/detailed_status.tsx index 3fd998992d..7c41449dc7 100644 --- a/app/javascript/flavours/glitch/features/status/components/detailed_status.tsx +++ b/app/javascript/flavours/glitch/features/status/components/detailed_status.tsx @@ -14,6 +14,7 @@ import { Link } from 'react-router-dom'; import { AnimatedNumber } from 'flavours/glitch/components/animated_number'; import AttachmentList from 'flavours/glitch/components/attachment_list'; import EditedTimestamp from 'flavours/glitch/components/edited_timestamp'; +import { FilterWarning } from 'flavours/glitch/components/filter_warning'; import type { StatusLike } from 'flavours/glitch/components/hashtag_bar'; import { getHashtagBarForStatus } from 'flavours/glitch/components/hashtag_bar'; import { IconLogo } from 'flavours/glitch/components/logo'; @@ -72,6 +73,7 @@ export const DetailedStatus: React.FC<{ }) => { const properStatus = status?.get('reblog') ?? status; const [height, setHeight] = useState(0); + const [showDespiteFilter, setShowDespiteFilter] = useState(false); const nodeRef = useRef(); const history = useAppHistory(); @@ -108,6 +110,10 @@ export const DetailedStatus: React.FC<{ [onOpenVideo, status], ); + const handleFilterToggle = useCallback(() => { + setShowDespiteFilter(!showDespiteFilter); + }, [showDespiteFilter, setShowDespiteFilter]); + const _measureHeight = useCallback( (heightJustChanged?: boolean) => { if (measureHeight && nodeRef.current) { @@ -358,6 +364,8 @@ export const DetailedStatus: React.FC<{ ); contentMedia.push(hashtagBar); + const matchedFilters = status.get('matched_filters'); + return (
- + {matchedFilters && ( + + )} + + {(!matchedFilters || showDespiteFilter) && ( + + )}
diff --git a/app/javascript/flavours/glitch/features/status/index.jsx b/app/javascript/flavours/glitch/features/status/index.jsx index 521db781b0..86d809088f 100644 --- a/app/javascript/flavours/glitch/features/status/index.jsx +++ b/app/javascript/flavours/glitch/features/status/index.jsx @@ -133,7 +133,7 @@ const makeMapStateToProps = () => { }); const mapStateToProps = (state, props) => { - const status = getStatus(state, { id: props.params.statusId }); + const status = getStatus(state, { id: props.params.statusId, contextType: 'detailed' }); let ancestorsIds = Immutable.List(); let descendantsIds = Immutable.List(); diff --git a/app/javascript/flavours/glitch/selectors/index.js b/app/javascript/flavours/glitch/selectors/index.js index 7c9a68cba4..dcfba7427d 100644 --- a/app/javascript/flavours/glitch/selectors/index.js +++ b/app/javascript/flavours/glitch/selectors/index.js @@ -15,9 +15,10 @@ export const makeGetStatus = () => { (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', id, 'account'])]), (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', state.getIn(['statuses', id, 'reblog']), 'account'])]), getFilters, + (_, { contextType }) => contextType === 'detailed', ], - (statusBase, statusReblog, accountBase, accountReblog, filters) => { + (statusBase, statusReblog, accountBase, accountReblog, filters, warnInsteadOfHide) => { if (!statusBase || statusBase.get('isLoading')) { return null; } @@ -25,7 +26,7 @@ export const makeGetStatus = () => { let filtered = false; if ((accountReblog || accountBase).get('id') !== me && filters) { let filterResults = statusReblog?.get('filtered') || statusBase.get('filtered') || ImmutableList(); - if (filterResults.some((result) => filters.getIn([result.get('filter'), 'filter_action']) === 'hide')) { + if (!warnInsteadOfHide && filterResults.some((result) => filters.getIn([result.get('filter'), 'filter_action']) === 'hide')) { return null; } filterResults = filterResults.filter(result => filters.has(result.get('filter'))); diff --git a/app/javascript/flavours/glitch/utils/filters.ts b/app/javascript/flavours/glitch/utils/filters.ts index d299e80c40..c944599bb9 100644 --- a/app/javascript/flavours/glitch/utils/filters.ts +++ b/app/javascript/flavours/glitch/utils/filters.ts @@ -6,6 +6,8 @@ export const toServerSideType = (columnType: string) => { case 'thread': case 'account': return columnType; + case 'detailed': + return 'thread'; default: if (columnType.includes('list:')) { return 'home'; From cbb9b8316029778177396010d6a297f418a3c29e Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Mar 2025 16:20:36 +0100 Subject: [PATCH 17/17] [Glitch] Fix bookmarks and favourites not being filtered Port 2eb6d815d63b91d4d3b6e7eb1a717350a7093e2b to glitch-soc Signed-off-by: Claire --- .../flavours/glitch/features/bookmarked_statuses/index.jsx | 1 + .../flavours/glitch/features/favourited_statuses/index.jsx | 1 + app/javascript/flavours/glitch/selectors/index.js | 2 +- app/javascript/flavours/glitch/utils/filters.ts | 3 +++ 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/javascript/flavours/glitch/features/bookmarked_statuses/index.jsx b/app/javascript/flavours/glitch/features/bookmarked_statuses/index.jsx index 81d2e79a12..49a5a73e1a 100644 --- a/app/javascript/flavours/glitch/features/bookmarked_statuses/index.jsx +++ b/app/javascript/flavours/glitch/features/bookmarked_statuses/index.jsx @@ -100,6 +100,7 @@ class Bookmarks extends ImmutablePureComponent { onLoadMore={this.handleLoadMore} emptyMessage={emptyMessage} bindToDocument={!multiColumn} + timelineId='bookmarks' /> diff --git a/app/javascript/flavours/glitch/features/favourited_statuses/index.jsx b/app/javascript/flavours/glitch/features/favourited_statuses/index.jsx index 119fd247b6..ce3624a54b 100644 --- a/app/javascript/flavours/glitch/features/favourited_statuses/index.jsx +++ b/app/javascript/flavours/glitch/features/favourited_statuses/index.jsx @@ -100,6 +100,7 @@ class Favourites extends ImmutablePureComponent { onLoadMore={this.handleLoadMore} emptyMessage={emptyMessage} bindToDocument={!multiColumn} + timelineId='favourites' /> diff --git a/app/javascript/flavours/glitch/selectors/index.js b/app/javascript/flavours/glitch/selectors/index.js index dcfba7427d..ae978b50c1 100644 --- a/app/javascript/flavours/glitch/selectors/index.js +++ b/app/javascript/flavours/glitch/selectors/index.js @@ -15,7 +15,7 @@ export const makeGetStatus = () => { (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', id, 'account'])]), (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', state.getIn(['statuses', id, 'reblog']), 'account'])]), getFilters, - (_, { contextType }) => contextType === 'detailed', + (_, { contextType }) => ['detailed', 'bookmarks', 'favourites'].includes(contextType), ], (statusBase, statusReblog, accountBase, accountReblog, filters, warnInsteadOfHide) => { diff --git a/app/javascript/flavours/glitch/utils/filters.ts b/app/javascript/flavours/glitch/utils/filters.ts index c944599bb9..a4a1c38294 100644 --- a/app/javascript/flavours/glitch/utils/filters.ts +++ b/app/javascript/flavours/glitch/utils/filters.ts @@ -8,6 +8,9 @@ export const toServerSideType = (columnType: string) => { return columnType; case 'detailed': return 'thread'; + case 'bookmarks': + case 'favourites': + return 'home'; default: if (columnType.includes('list:')) { return 'home';