From ddbe906c25fc370c92fa557d3637d50b7af97754 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 12 Mar 2022 09:11:36 +0100 Subject: Fix not updating a status when newer version is fetched manually (#17745) --- .../fetch_remote_status_service_spec.rb | 48 +++++++++++++++++++--- 1 file changed, 42 insertions(+), 6 deletions(-) (limited to 'spec/services') diff --git a/spec/services/activitypub/fetch_remote_status_service_spec.rb b/spec/services/activitypub/fetch_remote_status_service_spec.rb index 94574aa7f..68816e554 100644 --- a/spec/services/activitypub/fetch_remote_status_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_status_service_spec.rb @@ -3,9 +3,11 @@ require 'rails_helper' RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do include ActionView::Helpers::TextHelper - let(:sender) { Fabricate(:account) } - let(:recipient) { Fabricate(:account) } - let(:valid_domain) { Rails.configuration.x.local_domain } + let!(:sender) { Fabricate(:account).tap { |account| account.update(uri: ActivityPub::TagManager.instance.uri_for(account)) } } + let!(:recipient) { Fabricate(:account) } + let!(:valid_domain) { Rails.configuration.x.local_domain } + + let(:existing_status) { nil } let(:note) do { @@ -19,11 +21,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do subject { described_class.new } + before do + stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '') + end + describe '#call' do before do - sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender)) - - stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '') + existing_status subject.call(object[:id], prefetched_body: Oj.dump(object)) end @@ -186,5 +190,37 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(sender.statuses.first).to be_nil end end + + context 'when status already exists' do + let(:existing_status) { Fabricate(:status, account: sender, text: 'Foo', uri: note[:id]) } + + context 'with a Note object' do + let(:object) { note } + + it 'updates status' do + existing_status.reload + expect(existing_status.text).to eq 'Lorem ipsum' + expect(existing_status.edits).to_not be_empty + end + end + + context 'with a Create activity' do + let(:object) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: "https://#{valid_domain}/@foo/1234/create", + type: 'Create', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: note, + } + end + + it 'updates status' do + existing_status.reload + expect(existing_status.text).to eq 'Lorem ipsum' + expect(existing_status.edits).to_not be_empty + end + end + end end end -- cgit From 92a86b958ee7b96dbcbc17237dfcb1af0f15740c Mon Sep 17 00:00:00 2001 From: Claire Date: Sat, 12 Mar 2022 19:33:10 +0100 Subject: Fix issues with processing toot edits (#17748) * Fix searching for an already-known status by URL not working * Fix Update processing from statuses prior to 20220302232632 `ordered_media_attachment_ids_changed?` would return `true` when going from `nil` to anything (including `[]`). * Add tests --- .../activitypub/process_status_update_service.rb | 11 +++++++---- .../process_status_update_service_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) (limited to 'spec/services') diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 70e9cb7d9..47a788c30 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -13,7 +13,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @poll_changed = false # Only native types can be updated at the moment - return if !expected_type? || already_updated_more_recently? + return @status if !expected_type? || already_updated_more_recently? last_edit_date = status.edited_at.presence || status.created_at @@ -41,13 +41,16 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end forward_activity! if significant_changes? && @status_parser.edited_at.present? && @status_parser.edited_at > last_edit_date + + @status end private def update_media_attachments! - previous_media_attachments = @status.media_attachments.to_a - next_media_attachments = [] + previous_media_attachments = @status.media_attachments.to_a + previous_media_attachments_ids = @status.ordered_media_attachment_ids || previous_media_attachments.map(&:id) + next_media_attachments = [] as_array(@json['attachment']).each do |attachment| media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) @@ -87,7 +90,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @status.ordered_media_attachment_ids = next_media_attachments.map(&:id) @status.media_attachments.reload - @media_attachments_changed = true if @status.ordered_media_attachment_ids_changed? + @media_attachments_changed = true if @status.ordered_media_attachment_ids != previous_media_attachments_ids end def update_poll! diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 40b405217..788c7c9d9 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -46,6 +46,26 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do expect(status.reload.spoiler_text).to eq 'Show more' end + context 'with no changes and originally with no ordered_media_attachment_ids' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Note', + content: 'Hello world', + } + end + + before do + status.update(ordered_media_attachment_ids: nil) + subject.call(status, json) + end + + it 'does not record an update' do + expect(status.reload.edited?).to be false + end + end + context 'originally without tags' do before do subject.call(status, json) -- cgit