From ef01e91f405cb3a4237ffbfd861274e08adf6c86 Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 15 Nov 2023 12:16:59 +0900 Subject: [PATCH 01/24] =?UTF-8?q?Add:=20`conversations`=E3=83=86=E3=83=BC?= =?UTF-8?q?=E3=83=96=E3=83=AB=E3=81=AB`ancestor=5Fstatus`=E3=83=97?= =?UTF-8?q?=E3=83=AD=E3=83=91=E3=83=86=E3=82=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/conversation.rb | 15 +++++++++++---- app/models/status.rb | 8 +++++++- ...31115001356_add_inbox_url_to_conversations.rb | 16 ++++++++++++++++ db/schema.rb | 5 ++++- spec/models/status_spec.rb | 10 ++++++++++ 5 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20231115001356_add_inbox_url_to_conversations.rb diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 5de2599627842d..01eb4acc4e7557 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -4,18 +4,25 @@ # # Table name: conversations # -# id :bigint(8) not null, primary key -# uri :string -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# uri :string +# created_at :datetime not null +# updated_at :datetime not null +# inbox_url :string +# ancestor_status_id :bigint(8) # class Conversation < ApplicationRecord validates :uri, uniqueness: true, if: :uri? has_many :statuses + has_one :ancestor_status, class_name: 'Status', inverse_of: false def local? uri.nil? end + + def object_type + :conversation + end end diff --git a/app/models/status.rb b/app/models/status.rb index 0cb4c03a517a05..44a33de9c5b039 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -66,6 +66,7 @@ class Status < ApplicationRecord belongs_to :account, inverse_of: :statuses belongs_to :in_reply_to_account, class_name: 'Account', optional: true belongs_to :conversation, optional: true + has_one :owned_conversation, class_name: 'Conversation', foreign_key: 'ancestor_status_id', inverse_of: false belongs_to :preloadable_poll, class_name: 'Poll', foreign_key: 'poll_id', optional: true, inverse_of: false belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true @@ -655,7 +656,12 @@ def set_conversation self.in_reply_to_account_id = carried_over_reply_to_account_id self.conversation_id = thread.conversation_id if conversation_id.nil? elsif conversation_id.nil? - self.conversation = Conversation.new + if local? + self.owned_conversation = Conversation.new + self.conversation = owned_conversation + else + self.conversation = Conversation.new + end end end diff --git a/db/migrate/20231115001356_add_inbox_url_to_conversations.rb b/db/migrate/20231115001356_add_inbox_url_to_conversations.rb new file mode 100644 index 00000000000000..793c81fa8d3397 --- /dev/null +++ b/db/migrate/20231115001356_add_inbox_url_to_conversations.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddInboxURLToConversations < ActiveRecord::Migration[7.1] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def change + safety_assured do + add_column_with_default :conversations, :inbox_url, :string, default: nil, allow_null: true + add_reference :conversations, :ancestor_status, foreign_key: { to_table: :statuses, on_delete: :nullify }, index: false, null: true, default: nil + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4850882e11557a..2f2d50a4eb90ba 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2023_11_05_225839) do +ActiveRecord::Schema[7.1].define(version: 2023_11_15_001356) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -475,6 +475,8 @@ t.string "uri" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false + t.string "inbox_url" + t.bigint "ancestor_status_id" t.index ["uri"], name: "index_conversations_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)" end @@ -1463,6 +1465,7 @@ add_foreign_key "circles", "accounts", on_delete: :cascade add_foreign_key "conversation_mutes", "accounts", name: "fk_225b4212bb", on_delete: :cascade add_foreign_key "conversation_mutes", "conversations", on_delete: :cascade + add_foreign_key "conversations", "statuses", column: "ancestor_status_id", on_delete: :nullify add_foreign_key "custom_filter_keywords", "custom_filters", on_delete: :cascade add_foreign_key "custom_filter_statuses", "custom_filters", on_delete: :cascade add_foreign_key "custom_filter_statuses", "statuses", on_delete: :cascade diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 4e10f3d316e425..5036777c8249b6 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -593,6 +593,16 @@ expect(described_class.create(account: alice, text: 'First').conversation_id).to_not be_nil end + it 'creates new owned-conversation for stand-alone status' do + expect(described_class.create(account: alice, text: 'First').owned_conversation.id).to_not be_nil + end + + it 'creates new owned-conversation and linked for stand-alone status' do + status = described_class.create(account: alice, text: 'First') + expect(status.owned_conversation.ancestor_status_id).to eq status.id + expect(status.conversation.ancestor_status_id).to eq status.id + end + it 'keeps conversation of parent node' do parent = Fabricate(:status, text: 'First') expect(described_class.create(account: alice, thread: parent, text: 'Response').conversation_id).to eq parent.conversation_id From 9105d88fdcae44d4b6dcb3994c83ab077ef94a4b Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 15 Nov 2023 12:56:04 +0900 Subject: [PATCH 02/24] Fix test --- app/models/conversation.rb | 2 +- app/models/status.rb | 10 +++------- .../20231115001356_add_inbox_url_to_conversations.rb | 2 +- db/schema.rb | 1 - 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 01eb4acc4e7557..90fdd8143bcf8c 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -16,7 +16,7 @@ class Conversation < ApplicationRecord validates :uri, uniqueness: true, if: :uri? has_many :statuses - has_one :ancestor_status, class_name: 'Status', inverse_of: false + belongs_to :ancestor_status, class_name: 'Status', inverse_of: :owned_conversation, optional: true def local? uri.nil? diff --git a/app/models/status.rb b/app/models/status.rb index 44a33de9c5b039..4fa290cc1cea3c 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -66,7 +66,7 @@ class Status < ApplicationRecord belongs_to :account, inverse_of: :statuses belongs_to :in_reply_to_account, class_name: 'Account', optional: true belongs_to :conversation, optional: true - has_one :owned_conversation, class_name: 'Conversation', foreign_key: 'ancestor_status_id', inverse_of: false + has_one :owned_conversation, class_name: 'Conversation', foreign_key: 'ancestor_status_id', dependent: :nullify, inverse_of: false belongs_to :preloadable_poll, class_name: 'Poll', foreign_key: 'poll_id', optional: true, inverse_of: false belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true @@ -656,12 +656,8 @@ def set_conversation self.in_reply_to_account_id = carried_over_reply_to_account_id self.conversation_id = thread.conversation_id if conversation_id.nil? elsif conversation_id.nil? - if local? - self.owned_conversation = Conversation.new - self.conversation = owned_conversation - else - self.conversation = Conversation.new - end + self.owned_conversation = Conversation.new + self.conversation = owned_conversation end end diff --git a/db/migrate/20231115001356_add_inbox_url_to_conversations.rb b/db/migrate/20231115001356_add_inbox_url_to_conversations.rb index 793c81fa8d3397..460d61fc45b31e 100644 --- a/db/migrate/20231115001356_add_inbox_url_to_conversations.rb +++ b/db/migrate/20231115001356_add_inbox_url_to_conversations.rb @@ -10,7 +10,7 @@ class AddInboxURLToConversations < ActiveRecord::Migration[7.1] def change safety_assured do add_column_with_default :conversations, :inbox_url, :string, default: nil, allow_null: true - add_reference :conversations, :ancestor_status, foreign_key: { to_table: :statuses, on_delete: :nullify }, index: false, null: true, default: nil + add_column_with_default :conversations, :ancestor_status_id, :bigint, default: nil, allow_null: true end end end diff --git a/db/schema.rb b/db/schema.rb index 2f2d50a4eb90ba..3a4536ea43d2d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1465,7 +1465,6 @@ add_foreign_key "circles", "accounts", on_delete: :cascade add_foreign_key "conversation_mutes", "accounts", name: "fk_225b4212bb", on_delete: :cascade add_foreign_key "conversation_mutes", "conversations", on_delete: :cascade - add_foreign_key "conversations", "statuses", column: "ancestor_status_id", on_delete: :nullify add_foreign_key "custom_filter_keywords", "custom_filters", on_delete: :cascade add_foreign_key "custom_filter_statuses", "custom_filters", on_delete: :cascade add_foreign_key "custom_filter_statuses", "statuses", on_delete: :cascade From be1871281f267dbe28588e15c65aa32a3a286fc3 Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 15 Nov 2023 13:23:15 +0900 Subject: [PATCH 03/24] Fix test more --- app/models/status.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 4fa290cc1cea3c..c65f99269d581b 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -656,8 +656,12 @@ def set_conversation self.in_reply_to_account_id = carried_over_reply_to_account_id self.conversation_id = thread.conversation_id if conversation_id.nil? elsif conversation_id.nil? - self.owned_conversation = Conversation.new - self.conversation = owned_conversation + if local? + self.owned_conversation = Conversation.new + self.conversation = owned_conversation + else + self.conversation = Conversation.new + end end end From 84a587057e10004802b543f638bf30cd5edd5bb8 Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 16 Nov 2023 09:16:56 +0900 Subject: [PATCH 04/24] =?UTF-8?q?Add:=20`limited=5Fvisibility`=E3=81=AB`Re?= =?UTF-8?q?ply`=E3=82=92=E8=BF=BD=E5=8A=A0=E3=80=81`context`=E3=81=AEURI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../activitypub/contexts_controller.rb | 23 +++++++++++++++++++ app/lib/activitypub/parser/status_parser.rb | 2 ++ app/lib/activitypub/tag_manager.rb | 16 ++++++++----- app/models/status.rb | 2 +- .../activitypub/context_serializer.rb | 21 +++++++++++++++++ .../activitypub/note_serializer.rb | 9 +++++++- config/routes.rb | 1 + 7 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 app/controllers/activitypub/contexts_controller.rb create mode 100644 app/serializers/activitypub/context_serializer.rb diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb new file mode 100644 index 00000000000000..a3263ed82ec01f --- /dev/null +++ b/app/controllers/activitypub/contexts_controller.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class ActivityPub::ContextsController < ActivityPub::BaseController + include SignatureVerification + + vary_by -> { 'Signature' if authorized_fetch_mode? } + + before_action :set_context + + def show + expires_in 3.minutes, public: true + render json: @context, + serializer: ActivityPub::ContextSerializer, + adapter: ActivityPub::Adapter, + content_type: 'application/activity+json' + end + + private + + def set_context + @context = Conversation.find(params[:id]) + end +end diff --git a/app/lib/activitypub/parser/status_parser.rb b/app/lib/activitypub/parser/status_parser.rb index 4cd48d03f6009d..059860979c5d3a 100644 --- a/app/lib/activitypub/parser/status_parser.rb +++ b/app/lib/activitypub/parser/status_parser.rb @@ -105,6 +105,8 @@ def limited_scope :mutual when 'Circle' :circle + when 'Reply' + :reply else :none end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index ef3bb8310d010a..d6821d69370822 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -49,6 +49,8 @@ def uri_for(target) emoji_url(target) when :emoji_reaction emoji_reaction_url(target) + when :conversation + context_url(target) when :flag target.uri end @@ -119,10 +121,7 @@ def to(status) end.compact end when 'limited' - status.mentions.each_with_object([]) do |mention, result| - result << uri_for(mention.account) - result << followers_uri_for(mention.account) if mention.account.group? - end.compact + status.conversation.nil? ? ['kmyblue:Limited'] : [context_url(status.conversation)] end end @@ -228,10 +227,15 @@ def uri_to_resource(uri, klass) end def limited_scope(status) - if status.mutual_limited? + case status.limited_scope + when 'mutual' 'Mutual' + when 'circle' + 'Circle' + when 'reply' + 'Reply' else - status.circle_limited? ? 'Circle' : '' + '' end end diff --git a/app/models/status.rb b/app/models/status.rb index c65f99269d581b..f1098bdcda9afe 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -59,7 +59,7 @@ class Status < ApplicationRecord enum visibility: { public: 0, unlisted: 1, private: 2, direct: 3, limited: 4, public_unlisted: 10, login: 11 }, _suffix: :visibility enum searchability: { public: 0, private: 1, direct: 2, limited: 3, unsupported: 4, public_unlisted: 10 }, _suffix: :searchability - enum limited_scope: { none: 0, mutual: 1, circle: 2, personal: 3 }, _suffix: :limited + enum limited_scope: { none: 0, mutual: 1, circle: 2, personal: 3, reply: 4 }, _suffix: :limited belongs_to :application, class_name: 'Doorkeeper::Application', optional: true diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb new file mode 100644 index 00000000000000..a6a0bbb07fbed6 --- /dev/null +++ b/app/serializers/activitypub/context_serializer.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class ActivityPub::ContextSerializer < ActivityPub::Serializer + include RoutingHelper + + attributes :id, :type, :inbox + + def id + ActivityPub::TagManager.instance.uri_for(object) + end + + def type + 'Group' + end + + def inbox + return '' if object.ancestor_status.nil? + + account_inbox_url(object.ancestor_status.account) + end +end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 3c89c7b632f9b4..6cfbc97f2fc616 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -9,11 +9,12 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer :in_reply_to, :published, :url, :attributed_to, :to, :cc, :sensitive, :atom_uri, :in_reply_to_atom_uri, - :conversation, :searchable_by, :limited_scope + :conversation, :searchable_by, :context attribute :content attribute :content_map, if: :language? attribute :updated, if: :edited? + attribute :limited_scope, if: :limited_visibility? attribute :quote_uri, if: :quote? attribute :misskey_quote, key: :_misskey_quote, if: :quote? @@ -52,6 +53,10 @@ def content_map { object.language => content } end + def context + ActivityPub::TagManager.instance.uri_for(object.conversation) + end + def replies replies = object.self_replies(5).pluck(:id, :uri) last_id = replies.last&.first @@ -88,6 +93,8 @@ def language? object.language.present? end + delegate :limited_visibility?, to: :object + delegate :edited?, to: :object def in_reply_to diff --git a/config/routes.rb b/config/routes.rb index 13a115d37bef65..48c392038da064 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -140,6 +140,7 @@ def redirect_with_vary(path) end resource :inbox, only: [:create], module: :activitypub + resources :contexts, only: [:show], module: :activitypub get '/:encoded_at(*path)', to: redirect("/@%{path}"), constraints: { encoded_at: /%40/ } From 7edb06c1a0184048a1bac48fa7b783c587a8fa36 Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 16 Nov 2023 10:21:13 +0900 Subject: [PATCH 05/24] =?UTF-8?q?Add:=20=E5=A4=96=E9=83=A8=E3=81=8B?= =?UTF-8?q?=E3=82=89=E3=81=AE`context`=E5=8F=97=E4=BF=A1=E5=87=A6=E7=90=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/activitypub/activity/create.rb | 30 ++++++- spec/lib/activitypub/activity/create_spec.rb | 90 ++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 204899fa398f19..4925cbc19d08b6 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -132,7 +132,7 @@ def process_status_params limited_scope: @status_parser.limited_scope, searchability: @status_parser.searchability, thread: replied_to_status, - conversation: conversation_from_uri(@object['conversation']), + conversation: conversation_from_activity, media_attachment_ids: process_attachments.take(MediaAttachment::ACTIVITYPUB_STATUS_ATTACHMENT_MAX).map(&:id), poll: process_poll, } @@ -184,6 +184,10 @@ def process_audience @silenced_account_ids = @mentions.map(&:account_id) - accounts_in_audience.map(&:id) end + def account_representative + accounts_in_audience.detect(&:local?) || Account.representative + end + def postprocess_audience_and_deliver return if @status.mentions.find_by(account_id: @options[:delivered_to_account_id]) @@ -373,6 +377,10 @@ def fetch_replies(status) ActivityPub::FetchRepliesWorker.perform_async(status.id, uri, { 'request_id' => @options[:request_id] }) unless uri.nil? end + def conversation_from_activity + conversation_from_context(@object['context']) || conversation_from_uri(@object['conversation']) + end + def conversation_from_uri(uri) return nil if uri.nil? return Conversation.find_by(id: OStatus::TagManager.instance.unique_tag_to_local_id(uri, 'Conversation')) if OStatus::TagManager.instance.local_id?(uri) @@ -384,6 +392,26 @@ def conversation_from_uri(uri) end end + def conversation_from_context(uri) + return nil if uri.nil? + return Conversation.find_by(id: ActivityPub::TagManager.instance.uri_to_local_id(uri)) if ActivityPub::TagManager.instance.local_uri?(uri) + + begin + conversation = Conversation.find_or_create_by!(uri: uri) + + json = fetch_resource_without_id_validation(uri, account_representative) + return conversation if json.nil? || json['type'] != 'Group' + return conversation if json['inbox'].blank? || json['inbox'] == conversation.inbox_url + + conversation.update!(inbox_url: json['inbox']) + conversation + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique + retry + rescue Mastodon::UnexpectedResponseError + Conversation.find_or_create_by!(uri: uri) + end + end + def replied_to_status return @replied_to_status if defined?(@replied_to_status) diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 62e9831893ae5e..4603bf0a4d3864 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -16,12 +16,22 @@ }.with_indifferent_access end + let(:conversation) do + { + id: 'http://example.com/conversation', + type: 'Group', + inbox: 'http://example.com/actor/inbox', + }.with_indifferent_access + end + before do sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender)) stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt')) stub_request(:get, 'http://example.com/emoji.png').to_return(body: attachment_fixture('emojo.png')) stub_request(:get, 'http://example.com/emojib.png').to_return(body: attachment_fixture('emojo.png'), headers: { 'Content-Type' => 'application/octet-stream' }) + stub_request(:get, 'http://example.com/conversation').to_return(body: Oj.dump(conversation)) + stub_request(:get, 'http://example.com/invalid-conversation').to_return(status: 404) end describe 'processing posts received out of order' do @@ -939,6 +949,86 @@ def activity_for_object(json) end end + context 'with a context' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + context: 'http://example.com/conversation', + } + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.conversation).to_not be_nil + expect(status.conversation.uri).to eq 'http://example.com/conversation' + expect(status.conversation.inbox_url).to eq 'http://example.com/actor/inbox' + end + + context 'when existing' do + let(:custom_before) { true } + let!(:existing) { Fabricate(:conversation, uri: 'http://example.com/conversation', inbox_url: 'http://example.com/actor/invalid') } + + before do + subject.perform + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.conversation).to_not be_nil + expect(status.conversation.id).to eq existing.id + expect(status.conversation.inbox_url).to eq 'http://example.com/actor/inbox' + end + end + end + + context 'with an invalid context' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + context: 'http://example.com/invalid-conversation', + } + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.text).to eq 'Lorem ipsum' + expect(status.conversation).to_not be_nil + expect(status.conversation.uri).to eq 'http://example.com/invalid-conversation' + end + end + + context 'with a local context' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + context: "https://cb6e6126.ngrok.io/contexts/#{existing.id}", + } + end + + let(:existing) { Fabricate(:conversation, id: 3500) } + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.conversation).to_not be_nil + expect(status.conversation.uri).to be_nil + expect(status.conversation.id).to eq existing.id + end + end + context 'with media attachments' do let(:object_json) do { From 2ca4d3df079b927c773f3103dd4456caa771b7c9 Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 16 Nov 2023 10:28:13 +0900 Subject: [PATCH 06/24] Fix test --- app/lib/activitypub/tag_manager.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index d6821d69370822..64ea27d783891e 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -257,8 +257,6 @@ def searchable_by(status) [COLLECTIONS[:public]] when 'private' [account_followers_url(status.account)] - when 'direct' - status.conversation_id.present? ? [uri_for(status.conversation)] : [] when 'limited' ['as:Limited', 'kmyblue:Limited'] else @@ -278,7 +276,7 @@ def account_searchable_by(account) case account.compute_searchability_activitypub when 'public' [COLLECTIONS[:public]] - when 'private', 'direct' + when 'private' [account_followers_url(account)] when 'limited' ['as:Limited', 'kmyblue:Limited'] From 52a69beef89ed566e50859706d96173cfb712836 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 17 Nov 2023 11:51:42 +0900 Subject: [PATCH 07/24] =?UTF-8?q?Add:=20=E5=85=AC=E9=96=8B=E7=AF=84?= =?UTF-8?q?=E5=9B=B2=E3=80=8C=E8=BF=94=E4=BF=A1=E3=80=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mastodon/components/compacted_status.jsx | 10 ------- app/javascript/mastodon/components/status.jsx | 10 ------- .../mastodon/components/visibility_icon.tsx | 11 ++++++++ .../compose/components/privacy_dropdown.jsx | 20 ++++++++++++-- .../containers/privacy_dropdown_container.js | 1 + app/javascript/mastodon/reducers/compose.js | 21 +++++++++++++-- app/services/post_status_service.rb | 19 ++++++++----- spec/services/post_status_service_spec.rb | 27 ++++++++++++++++++- 8 files changed, 87 insertions(+), 32 deletions(-) diff --git a/app/javascript/mastodon/components/compacted_status.jsx b/app/javascript/mastodon/components/compacted_status.jsx index 01e1105127552f..263e08809c1229 100644 --- a/app/javascript/mastodon/components/compacted_status.jsx +++ b/app/javascript/mastodon/components/compacted_status.jsx @@ -61,16 +61,6 @@ export const defaultMediaVisibility = (status) => { }; const messages = defineMessages({ - public_short: { id: 'privacy.public.short', defaultMessage: 'Public' }, - unlisted_short: { id: 'privacy.unlisted.short', defaultMessage: 'Unlisted' }, - public_unlisted_short: { id: 'privacy.public_unlisted.short', defaultMessage: 'Public unlisted' }, - login_short: { id: 'privacy.login.short', defaultMessage: 'Login only' }, - private_short: { id: 'privacy.private.short', defaultMessage: 'Followers only' }, - limited_short: { id: 'privacy.limited.short', defaultMessage: 'Limited menbers only' }, - mutual_short: { id: 'privacy.mutual.short', defaultMessage: 'Mutual followers only' }, - circle_short: { id: 'privacy.circle.short', defaultMessage: 'Circle members only' }, - personal_short: { id: 'privacy.personal.short', defaultMessage: 'Yourself only' }, - direct_short: { id: 'privacy.direct.short', defaultMessage: 'Mentioned people only' }, edited: { id: 'status.edited', defaultMessage: 'Edited {date}' }, }); diff --git a/app/javascript/mastodon/components/status.jsx b/app/javascript/mastodon/components/status.jsx index cf6a924d27fe47..69919c7490529a 100644 --- a/app/javascript/mastodon/components/status.jsx +++ b/app/javascript/mastodon/components/status.jsx @@ -76,16 +76,6 @@ export const defaultMediaVisibility = (status) => { }; const messages = defineMessages({ - public_short: { id: 'privacy.public.short', defaultMessage: 'Public' }, - unlisted_short: { id: 'privacy.unlisted.short', defaultMessage: 'Unlisted' }, - public_unlisted_short: { id: 'privacy.public_unlisted.short', defaultMessage: 'Public unlisted' }, - login_short: { id: 'privacy.login.short', defaultMessage: 'Login only' }, - private_short: { id: 'privacy.private.short', defaultMessage: 'Followers only' }, - limited_short: { id: 'privacy.limited.short', defaultMessage: 'Limited menbers only' }, - mutual_short: { id: 'privacy.mutual.short', defaultMessage: 'Mutual followers only' }, - circle_short: { id: 'privacy.circle.short', defaultMessage: 'Circle members only' }, - personal_short: { id: 'privacy.personal.short', defaultMessage: 'Yourself only' }, - direct_short: { id: 'privacy.direct.short', defaultMessage: 'Mentioned people only' }, edited: { id: 'status.edited', defaultMessage: 'Edited {date}' }, }); diff --git a/app/javascript/mastodon/components/visibility_icon.tsx b/app/javascript/mastodon/components/visibility_icon.tsx index acb405c29f498d..c7e2707c91e480 100644 --- a/app/javascript/mastodon/components/visibility_icon.tsx +++ b/app/javascript/mastodon/components/visibility_icon.tsx @@ -8,6 +8,7 @@ import { ReactComponent as LoginIcon } from '@material-symbols/svg-600/outlined/ import { ReactComponent as LockIcon } from '@material-symbols/svg-600/outlined/lock.svg'; import { ReactComponent as LockOpenIcon } from '@material-symbols/svg-600/outlined/no_encryption.svg'; import { ReactComponent as PublicIcon } from '@material-symbols/svg-600/outlined/public.svg'; +import { ReactComponent as ReplyIcon } from '@material-symbols/svg-600/outlined/reply.svg'; import { ReactComponent as LimitedIcon } from '@material-symbols/svg-600/outlined/shield.svg'; import { ReactComponent as PersonalIcon } from '@material-symbols/svg-600/outlined/sticky_note.svg'; @@ -23,6 +24,7 @@ type Visibility = | 'mutual' | 'circle' | 'personal' + | 'reply' | 'limited'; const messages = defineMessages({ @@ -49,6 +51,10 @@ const messages = defineMessages({ id: 'privacy.circle.short', defaultMessage: 'Circle members only', }, + reply_short: { + id: 'privacy.reply.short', + defaultMessage: 'Reply', + }, personal_short: { id: 'privacy.personal.short', defaultMessage: 'Yourself only', @@ -105,6 +111,11 @@ export const VisibilityIcon: React.FC<{ visibility: Visibility }> = ({ iconComponent: CircleIcon, text: intl.formatMessage(messages.circle_short), }, + reply: { + icon: 'reply', + iconComponent: ReplyIcon, + text: intl.formatMessage(messages.reply_short), + }, personal: { icon: 'sticky-note-o', iconComponent: PersonalIcon, diff --git a/app/javascript/mastodon/features/compose/components/privacy_dropdown.jsx b/app/javascript/mastodon/features/compose/components/privacy_dropdown.jsx index 239dec3f5b460c..db4dcd5f512a6e 100644 --- a/app/javascript/mastodon/features/compose/components/privacy_dropdown.jsx +++ b/app/javascript/mastodon/features/compose/components/privacy_dropdown.jsx @@ -14,6 +14,7 @@ import { ReactComponent as LoginIcon } from '@material-symbols/svg-600/outlined/ import { ReactComponent as LockIcon } from '@material-symbols/svg-600/outlined/lock.svg'; import { ReactComponent as LockOpenIcon } from '@material-symbols/svg-600/outlined/no_encryption.svg'; import { ReactComponent as PublicIcon } from '@material-symbols/svg-600/outlined/public.svg'; +import { ReactComponent as ReplyIcon } from '@material-symbols/svg-600/outlined/reply.svg'; import { supportsPassiveEvents } from 'detect-passive-events'; import Overlay from 'react-overlays/Overlay'; @@ -38,6 +39,8 @@ const messages = defineMessages({ mutual_long: { id: 'privacy.mutual.long', defaultMessage: 'Mutual follows only' }, circle_short: { id: 'privacy.circle.short', defaultMessage: 'Circle' }, circle_long: { id: 'privacy.circle.long', defaultMessage: 'Circle members only' }, + reply_short: { id: 'privacy.reply.short', defaultMessage: 'Reply' }, + reply_long: { id: 'privacy.reply.long', defaultMessage: 'Reply to limited post' }, direct_short: { id: 'privacy.direct.short', defaultMessage: 'Mentioned people only' }, direct_long: { id: 'privacy.direct.long', defaultMessage: 'Visible for mentioned users only' }, change_privacy: { id: 'privacy.change', defaultMessage: 'Adjust status privacy' }, @@ -166,6 +169,7 @@ class PrivacyDropdown extends PureComponent { value: PropTypes.string.isRequired, onChange: PropTypes.func.isRequired, noDirect: PropTypes.bool, + replyToLimited: PropTypes.bool, container: PropTypes.func, disabled: PropTypes.bool, intl: PropTypes.object.isRequired, @@ -280,10 +284,22 @@ class PrivacyDropdown extends PureComponent { }; render () { - const { value, container, disabled, intl } = this.props; + const { value, container, disabled, intl, replyToLimited } = this.props; const { open, placement } = this.state; - const valueOption = this.options.find(item => item.value === value) || this.options[0]; + if (replyToLimited) { + if (!this.selectableOptions.some((op) => op.value === 'reply')) { + this.selectableOptions.unshift( + { icon: 'reply', iconComponent: ReplyIcon, value: 'reply', text: intl.formatMessage(messages.reply_short), meta: intl.formatMessage(messages.reply_long) }, + ); + } + } else { + if (this.selectableOptions.some((op) => op.value === 'reply')) { + this.selectableOptions = this.selectableOptions.filter((op) => op.value !== 'reply'); + } + } + + const valueOption = this.selectableOptions.find(item => item.value === value) || this.selectableOptions[0]; return (
diff --git a/app/javascript/mastodon/features/compose/containers/privacy_dropdown_container.js b/app/javascript/mastodon/features/compose/containers/privacy_dropdown_container.js index 6d26abf4f6623b..89a001a222b58e 100644 --- a/app/javascript/mastodon/features/compose/containers/privacy_dropdown_container.js +++ b/app/javascript/mastodon/features/compose/containers/privacy_dropdown_container.js @@ -7,6 +7,7 @@ import PrivacyDropdown from '../components/privacy_dropdown'; const mapStateToProps = state => ({ value: state.getIn(['compose', 'privacy']), + replyToLimited: state.getIn(['compose', 'reply_to_limited']), }); const mapDispatchToProps = dispatch => ({ diff --git a/app/javascript/mastodon/reducers/compose.js b/app/javascript/mastodon/reducers/compose.js index 0b52bbd0a6b108..c6b30c16ec8dbd 100644 --- a/app/javascript/mastodon/reducers/compose.js +++ b/app/javascript/mastodon/reducers/compose.js @@ -77,6 +77,7 @@ const initialState = ImmutableMap({ caretPosition: null, preselectDate: null, in_reply_to: null, + reply_to_limited: false, is_composing: false, is_submitting: false, is_changing_upload: false, @@ -144,6 +145,8 @@ function clearAll(state) { if (!state.get('in_reply_to')) { map.set('posted_on_this_session', true); } + map.set('reply_to_limited', false); + console.dir(map.get('reply_to_limited')); map.set('limited_scope', null); map.set('id', null); map.set('in_reply_to', null); @@ -293,6 +296,10 @@ const insertReference = (state, url, attributeType) => { }; const privacyPreference = (a, b) => { + if (a === 'limited') { + return 'reply'; + } + const order = ['public', 'public_unlisted', 'unlisted', 'login', 'private', 'direct']; return order[Math.max(order.indexOf(a), order.indexOf(b), 0)]; }; @@ -411,6 +418,8 @@ export default function compose(state = initialState, action) { map.set('id', null); map.set('in_reply_to', action.status.get('id')); map.set('text', statusToTextMentions(state, action.status)); + map.set('reply_to_limited', action.status.get('visibility_ex') === 'limited'); + console.dir(map.get('reply_to_limited')); map.set('privacy', privacyPreference(action.status.get('visibility_ex'), state.get('default_privacy'))); map.set('limited_scope', null); map.set('searchability', privacyPreference(action.status.get('searchability'), state.get('default_searchability'))); @@ -521,7 +530,11 @@ export default function compose(state = initialState, action) { return state.set('tagHistory', fromJS(action.tags)); case TIMELINE_DELETE: if (action.id === state.get('in_reply_to')) { - return state.set('in_reply_to', null); + if (state.get('privacy') === 'reply') { + return state.set('in_reply_to', null).set('privacy', 'circle'); + } else { + return state.set('in_reply_to', null); + } } else if (action.id === state.get('id')) { return state.set('id', null); } else { @@ -549,6 +562,8 @@ export default function compose(state = initialState, action) { map.set('text', action.raw_text || unescapeHTML(expandMentions(action.status))); map.set('in_reply_to', action.status.get('in_reply_to_id')); map.set('privacy', action.status.get('visibility_ex')); + map.set('reply_to_limited', action.status.get('limited_scope') === 'reply'); + console.dir(map.get('reply_to_limited')); map.set('limited_scope', null); map.set('media_attachments', action.status.get('media_attachments').map((media) => media.set('unattached', true))); map.set('focusDate', new Date()); @@ -583,8 +598,10 @@ export default function compose(state = initialState, action) { if (action.status.get('visibility_ex') !== 'limited') { map.set('privacy', action.status.get('visibility_ex')); } else { - map.set('privacy', action.status.get('limited_scope') === 'mutual' ? 'mutual' : 'circle'); + map.set('privacy', action.status.get('limited_scope') || 'circle'); } + map.set('reply_to_limited', action.status.get('limited_scope') === 'reply'); + console.dir(map.get('reply_to_limited')); map.set('limited_scope', action.status.get('limited_scope')); map.set('media_attachments', action.status.get('media_attachments')); map.set('focusDate', new Date()); diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index cec0737273e184..d9c9344908e7d8 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -75,10 +75,9 @@ def preprocess_attributes! end) || @options[:spoiler_text].present? @text = @options.delete(:spoiler_text) if @text.blank? && @options[:spoiler_text].present? @visibility = @options[:visibility]&.to_sym || @account.user&.setting_default_privacy&.to_sym - @visibility = :direct if @in_reply_to&.limited_visibility? - @visibility = :limited if %w(mutual circle).include?(@options[:visibility]) - @visibility = :unlisted if (@visibility&.to_sym == :public || @visibility&.to_sym == :public_unlisted || @visibility&.to_sym == :login) && @account.silenced? - @visibility = :public_unlisted if @visibility&.to_sym == :public && !@options[:force_visibility] && !@options[:application]&.superapp && @account.user&.setting_public_post_to_unlisted && Setting.enable_public_unlisted_visibility + @visibility = :limited if %w(mutual circle reply).include?(@options[:visibility]) + @visibility = :unlisted if (@visibility == :public || @visibility == :public_unlisted || @visibility == :login) && @account.silenced? + @visibility = :public_unlisted if @visibility == :public && !@options[:force_visibility] && !@options[:application]&.superapp && @account.user&.setting_public_post_to_unlisted && Setting.enable_public_unlisted_visibility @limited_scope = @options[:visibility]&.to_sym if @visibility == :limited @searchability = searchability @searchability = :private if @account.silenced? && %i(public public_unlisted).include?(@searchability&.to_sym) @@ -88,6 +87,11 @@ def preprocess_attributes! @reference_ids = (@options[:status_reference_ids] || []).map(&:to_i).filter(&:positive?) raise ArgumentError if !Setting.enable_public_unlisted_visibility && @visibility == :public_unlisted + if @in_reply_to&.limited_visibility? && @visibility != :direct + @visibility = :limited + @limited_scope = :reply + end + load_circle overwrite_dtl_post process_sensitive_words @@ -96,8 +100,9 @@ def preprocess_attributes! end def load_circle - raise ArgumentError if @options[:visibility] == 'limited' && @options[:circle_id].nil? - return unless @options[:visibility] == 'circle' || (@options[:visibility] == 'limited' && @options[:circle_id].present?) + return if @visibility == :limited && @limited_scope == :reply + return unless %w(circle limited).include?(@options[:visibility]) + raise ArgumentError if @options[:circle_id].nil? @circle = @options[:circle_id].present? && Circle.find(@options[:circle_id]) @limited_scope = :circle @@ -148,7 +153,7 @@ def process_status! safeguard_mentions!(@status) validate_status_mentions! - @status.limited_scope = :personal if @status.limited_visibility? && !process_mentions_service.mentions? + @status.limited_scope = :personal if @status.limited_visibility? && !@status.reply_limited? && !process_mentions_service.mentions? UpdateStatusExpirationService.new.call(@status) diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index e27baf13a91673..32f27634642e15 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -263,7 +263,7 @@ expect(status.limited_scope).to eq 'circle' end - it 'limited visibility and empty circle' do + it 'limited visibility without circle' do account = Fabricate(:account) text = 'This is an English text.' @@ -295,6 +295,31 @@ expect(status.mentioned_accounts.count).to eq 1 end + it 'creates a new response status to limited post' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' + + status = subject.call(account, text: text, thread: in_reply_to_status) + + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'limited' + expect(status.limited_scope).to eq 'reply' + end + + it 'creates a new direct message to limited post' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' + + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: :direct) + + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'direct' + end + it 'safeguards mentions' do account = Fabricate(:account) mentioned_account = Fabricate(:account, username: 'alice') From fee76c95a256690489ad068c8e4e0437c6e1150e Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 17 Nov 2023 11:56:05 +0900 Subject: [PATCH 08/24] Fix test --- app/javascript/mastodon/reducers/compose.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/javascript/mastodon/reducers/compose.js b/app/javascript/mastodon/reducers/compose.js index c6b30c16ec8dbd..6ea511ed45db7f 100644 --- a/app/javascript/mastodon/reducers/compose.js +++ b/app/javascript/mastodon/reducers/compose.js @@ -146,7 +146,6 @@ function clearAll(state) { map.set('posted_on_this_session', true); } map.set('reply_to_limited', false); - console.dir(map.get('reply_to_limited')); map.set('limited_scope', null); map.set('id', null); map.set('in_reply_to', null); @@ -419,7 +418,6 @@ export default function compose(state = initialState, action) { map.set('in_reply_to', action.status.get('id')); map.set('text', statusToTextMentions(state, action.status)); map.set('reply_to_limited', action.status.get('visibility_ex') === 'limited'); - console.dir(map.get('reply_to_limited')); map.set('privacy', privacyPreference(action.status.get('visibility_ex'), state.get('default_privacy'))); map.set('limited_scope', null); map.set('searchability', privacyPreference(action.status.get('searchability'), state.get('default_searchability'))); @@ -563,7 +561,6 @@ export default function compose(state = initialState, action) { map.set('in_reply_to', action.status.get('in_reply_to_id')); map.set('privacy', action.status.get('visibility_ex')); map.set('reply_to_limited', action.status.get('limited_scope') === 'reply'); - console.dir(map.get('reply_to_limited')); map.set('limited_scope', null); map.set('media_attachments', action.status.get('media_attachments').map((media) => media.set('unattached', true))); map.set('focusDate', new Date()); @@ -601,7 +598,6 @@ export default function compose(state = initialState, action) { map.set('privacy', action.status.get('limited_scope') || 'circle'); } map.set('reply_to_limited', action.status.get('limited_scope') === 'reply'); - console.dir(map.get('reply_to_limited')); map.set('limited_scope', action.status.get('limited_scope')); map.set('media_attachments', action.status.get('media_attachments')); map.set('focusDate', new Date()); From e38267b8088fe2e0e9c00d7f96d6e51b14e26c35 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 17 Nov 2023 13:41:27 +0900 Subject: [PATCH 09/24] =?UTF-8?q?Fix:=20=E8=BF=94=E4=BF=A1=E3=81=AB?= =?UTF-8?q?=E8=BF=94=E4=BF=A1=E4=BB=A5=E5=A4=96=E3=81=AE=E5=85=AC=E9=96=8B?= =?UTF-8?q?=E7=AF=84=E5=9B=B2=E3=82=92=E8=A8=AD=E5=AE=9A=E3=81=A7=E3=81=8D?= =?UTF-8?q?=E3=81=AA=E3=81=84=E5=95=8F=E9=A1=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/services/post_status_service.rb | 6 +- spec/services/post_status_service_spec.rb | 83 ++++++++++++++++++----- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index d9c9344908e7d8..cba204eaf548e3 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -87,7 +87,7 @@ def preprocess_attributes! @reference_ids = (@options[:status_reference_ids] || []).map(&:to_i).filter(&:positive?) raise ArgumentError if !Setting.enable_public_unlisted_visibility && @visibility == :public_unlisted - if @in_reply_to&.limited_visibility? && @visibility != :direct + if @in_reply_to.present? && ((@visibility == :limited && @options[:circle_id].nil?) || @limited_scope == :reply) @visibility = :limited @limited_scope = :reply end @@ -100,8 +100,8 @@ def preprocess_attributes! end def load_circle - return if @visibility == :limited && @limited_scope == :reply - return unless %w(circle limited).include?(@options[:visibility]) + return if @visibility == :limited && @limited_scope == :reply && @in_reply_to.present? + return unless %w(circle limited reply).include?(@options[:visibility]) raise ArgumentError if @options[:circle_id].nil? @circle = @options[:circle_id].present? && Circle.find(@options[:circle_id]) diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 32f27634642e15..2956bd44c60cfc 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -295,29 +295,76 @@ expect(status.mentioned_accounts.count).to eq 1 end - it 'creates a new response status to limited post' do - in_reply_to_status = Fabricate(:status, visibility: :limited) - account = Fabricate(:account) - text = 'test status update' + describe 'create a new response status to limited post' do + it 'when reply visibility' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' - status = subject.call(account, text: text, thread: in_reply_to_status) + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: 'reply') - expect(status).to be_persisted - expect(status.thread).to eq in_reply_to_status - expect(status.visibility).to eq 'limited' - expect(status.limited_scope).to eq 'reply' - end + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'limited' + expect(status.limited_scope).to eq 'reply' + end - it 'creates a new direct message to limited post' do - in_reply_to_status = Fabricate(:status, visibility: :limited) - account = Fabricate(:account) - text = 'test status update' + it 'when limited visibility' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' - status = subject.call(account, text: text, thread: in_reply_to_status, visibility: :direct) + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: :limited) - expect(status).to be_persisted - expect(status.thread).to eq in_reply_to_status - expect(status.visibility).to eq 'direct' + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'limited' + expect(status.limited_scope).to eq 'reply' + end + + it 'when circle visibility' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' + + circle = Fabricate(:circle, account: account) + circle_account = Fabricate(:account) + circle_account.follow!(account) + circle.accounts << circle_account + circle.save! + + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: 'circle', circle_id: circle.id) + + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'limited' + expect(status.limited_scope).to eq 'circle' + expect(status.mentioned_accounts.pluck(:id)).to eq [circle_account.id] + end + + it 'when public visibility' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' + + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: :public) + + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'public' + end + + it 'when direct visibility' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + account = Fabricate(:account) + text = 'test status update' + + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: :direct) + + expect(status).to be_persisted + expect(status.thread).to eq in_reply_to_status + expect(status.visibility).to eq 'direct' + end end it 'safeguards mentions' do From d3182a74e220036ad54048d3748155491bd0e43c Mon Sep 17 00:00:00 2001 From: KMY Date: Mon, 20 Nov 2023 12:42:41 +0900 Subject: [PATCH 10/24] =?UTF-8?q?Add:=20=E3=83=AD=E3=83=BC=E3=82=AB?= =?UTF-8?q?=E3=83=AB=E6=8A=95=E7=A8=BF=E6=99=82=E3=81=AB=E3=83=A1=E3=83=B3?= =?UTF-8?q?=E3=82=B7=E3=83=A7=E3=83=B3=E8=BF=BD=E5=8A=A0=E3=83=BB=E4=BB=96?= =?UTF-8?q?=E3=82=B5=E3=83=BC=E3=83=90=E3=83=BC=E3=81=B8=E3=81=AE=E8=BB=A2?= =?UTF-8?q?=E9=80=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/activitypub/activity/create.rb | 11 +++ app/models/status.rb | 2 +- app/services/post_status_service.rb | 5 +- app/services/process_conversion_service.rb | 33 ++++++++ .../activitypub/distribution_worker.rb | 15 ++++ .../activitypub/forward_conversion_worker.rb | 37 +++++++++ spec/services/post_status_service_spec.rb | 82 ++++++++++++++++++- .../activitypub/distribution_worker_spec.rb | 16 ++++ 8 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 app/services/process_conversion_service.rb create mode 100644 app/workers/activitypub/forward_conversion_worker.rb diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 4925cbc19d08b6..cd9fee8b158a10 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -96,6 +96,7 @@ def process_status process_references! distribute forward_for_reply + forward_for_conversation if @status.limited_visibility? join_group! end @@ -511,6 +512,16 @@ def forward_for_reply ActivityPub::RawDistributionWorker.perform_async(Oj.dump(@json), replied_to_status.account_id, [@account.preferred_inbox_url]) end + def forward_for_conversation + return unless @status.conversation.present? && @status.conversation.local? + + ProcessConversionService.new.call(@status) + + return if @json['signature'].blank? + + ActivityPub::ForwardConversationWorker.perform_async(Oj.dump(@json), @status.id) + end + def increment_voters_count! poll = replied_to_status.preloadable_poll diff --git a/app/models/status.rb b/app/models/status.rb index 938f196948cdcd..631ffc4aaaf394 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -657,7 +657,7 @@ def set_conversation self.reply = !(in_reply_to_id.nil? && thread.nil?) unless reply - if reply? && !thread.nil? + if reply? && !thread.nil? && (!limited_visibility? || none_limited? || reply_limited?) self.in_reply_to_account_id = carried_over_reply_to_account_id self.conversation_id = thread.conversation_id if conversation_id.nil? elsif conversation_id.nil? diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index cba204eaf548e3..0ae6d0a9a75fdc 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -78,7 +78,7 @@ def preprocess_attributes! @visibility = :limited if %w(mutual circle reply).include?(@options[:visibility]) @visibility = :unlisted if (@visibility == :public || @visibility == :public_unlisted || @visibility == :login) && @account.silenced? @visibility = :public_unlisted if @visibility == :public && !@options[:force_visibility] && !@options[:application]&.superapp && @account.user&.setting_public_post_to_unlisted && Setting.enable_public_unlisted_visibility - @limited_scope = @options[:visibility]&.to_sym if @visibility == :limited + @limited_scope = @options[:visibility]&.to_sym if @visibility == :limited && @options[:visibility] != 'limited' @searchability = searchability @searchability = :private if @account.silenced? && %i(public public_unlisted).include?(@searchability&.to_sym) @markdown = @options[:markdown] || false @@ -87,7 +87,7 @@ def preprocess_attributes! @reference_ids = (@options[:status_reference_ids] || []).map(&:to_i).filter(&:positive?) raise ArgumentError if !Setting.enable_public_unlisted_visibility && @visibility == :public_unlisted - if @in_reply_to.present? && ((@visibility == :limited && @options[:circle_id].nil?) || @limited_scope == :reply) + if @in_reply_to.present? && ((@options[:visibility] == 'limited' && @options[:circle_id].nil?) || @limited_scope == :reply) @visibility = :limited @limited_scope = :reply end @@ -201,6 +201,7 @@ def postprocess_status! process_hashtags_service.call(@status) Trends.tags.register(@status) + ProcessConversionService.new.call(@status) if @status.limited_visibility? && @status.reply_limited? ProcessReferencesService.call_service(@status, @reference_ids, []) LinkCrawlWorker.perform_async(@status.id) DistributionWorker.perform_async(@status.id) diff --git a/app/services/process_conversion_service.rb b/app/services/process_conversion_service.rb new file mode 100644 index 00000000000000..ccae2881936629 --- /dev/null +++ b/app/services/process_conversion_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class ProcessConversionService < BaseService + def call(status) + @status = status + + return if !@status.limited_visibility? || @status.conversation.nil? + + duplicate_reply! + end + + private + + def thread + @thread ||= @status.thread || @status.conversation.ancestor_status + end + + def duplicate_reply! + return unless @status.conversation.local? + return if !@status.reply? || thread.nil? + return if thread.conversation_id != @status.conversation_id + + mentioned_account_ids = @status.mentions.pluck(:account_id) + + thread.mentioned_accounts.find_each do |account| + @status.mentions << @status.mentions.new(silent: true, account: account) unless mentioned_account_ids.include?(account.id) + mentioned_account_ids << account.id + end + @status.mentions << @status.mentions.new(silent: true, account: thread.account) unless mentioned_account_ids.include?(thread.account.id) + + @status.save! + end +end diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index f8f80864a4afc4..4ea25a58e14938 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -19,11 +19,26 @@ def perform(status_id) protected def distribute_limited! + if @status.reply? && @status.conversation.present? && !@status.conversation.local? + distribute_conversation! + else + distribute_limited_mentions! + end + end + + def distribute_limited_mentions! ActivityPub::DeliveryWorker.push_bulk(inboxes_for_limited, limit: 1_000) do |inbox_url| [payload, @account.id, inbox_url, options] end end + def distribute_conversation! + inbox_url = @status.conversation.inbox_url + return if inbox_url.blank? + + ActivityPub::DeliveryWorker.perform_async(payload, @account.id, inbox_url, options) + end + def inboxes @inboxes ||= status_reach_finder.inboxes end diff --git a/app/workers/activitypub/forward_conversion_worker.rb b/app/workers/activitypub/forward_conversion_worker.rb new file mode 100644 index 00000000000000..3bc846b4df2196 --- /dev/null +++ b/app/workers/activitypub/forward_conversion_worker.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class ActivityPub::ForwardConversationWorker + def perform(payload, status_id) + @status = Status.find(status_id) + @payload = payload + + return unless @status.conversation.present? && @status.conversation.local? && @status.conversation.ancestor_status.present? + return unless @status.limited_visibility? + + @account = @status.conversation.ancestor_status.account + + distribute_limited_mentions! + rescue ActiveRecord::RecordNotFound + true + end + + protected + + def distribute_limited_mentions! + ActivityPub::DeliveryWorker.push_bulk(inboxes_for_limited, limit: 1_000) do |inbox_url| + [payload, @account.id, inbox_url, options] + end + end + + def inboxes_for_limited + DeliveryFailureTracker.without_unavailable( + Account.remote.merge(@status.mentioned_accounts).pluck(:inbox_url).compact_blank.uniq + ) + end + + def options + { 'synchronize_followers' => @status.private_visibility? } + end + + attr_reader :payload +end diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 2956bd44c60cfc..455dac4e7e9c00 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -314,7 +314,7 @@ account = Fabricate(:account) text = 'test status update' - status = subject.call(account, text: text, thread: in_reply_to_status, visibility: :limited) + status = subject.call(account, text: text, thread: in_reply_to_status, visibility: 'limited') expect(status).to be_persisted expect(status.thread).to eq in_reply_to_status @@ -365,6 +365,86 @@ expect(status.thread).to eq in_reply_to_status expect(status.visibility).to eq 'direct' end + + it 'duplicate replies' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + in_reply_to_status.mentions.create!(account: Fabricate(:account)) + + status = subject.call(Fabricate(:user).account, text: 'Ohagi is good', thread: in_reply_to_status, visibility: 'reply') + + thread_account_ids = [in_reply_to_status.account, in_reply_to_status.mentions.first.account].map(&:id) + + expect(status).to be_persisted + expect(status.conversation_id).to eq in_reply_to_status.conversation_id + expect(status.conversation.ancestor_status_id).to eq in_reply_to_status.id + expect(status.mentions.pluck(:account_id)).to match_array thread_account_ids + end + + it 'duplicate reply-to-reply' do + ancestor_account = Fabricate(:account, username: 'ancestor', domain: nil) + reply_account = Fabricate(:account) + + first_status = Fabricate(:status, account: ancestor_account, visibility: :limited) + in_reply_to_status = subject.call(reply_account, text: 'Ohagi is good, @ancestor', thread: first_status, visibility: 'reply') + status = subject.call(ancestor_account, text: 'Ohagi is good', thread: in_reply_to_status, visibility: 'reply') + + thread_account_ids = [ancestor_account, reply_account].map(&:id) + + expect(status).to be_persisted + expect(status.conversation_id).to eq in_reply_to_status.conversation_id + expect(status.conversation_id).to eq first_status.conversation_id + expect(status.conversation.ancestor_status_id).to eq first_status.id + expect(status.mentions.pluck(:account_id)).to match_array thread_account_ids + end + + it 'duplicate reply-to-third_reply' do + first_status = Fabricate(:status, visibility: :limited) + first_status.mentions.create!(account: Fabricate(:account)) + + mentioned_account = Fabricate(:account, username: 'ohagi', domain: nil) + mentioned_account2 = Fabricate(:account, username: 'bob', domain: nil) + in_reply_to_status = subject.call(Fabricate(:user).account, text: 'Ohagi is good, @ohagi', thread: first_status, visibility: 'reply') + status = subject.call(Fabricate(:user).account, text: 'Ohagi is good, @bob', thread: in_reply_to_status, visibility: 'reply') + + thread_account_ids = [first_status.account, first_status.mentions.first.account, mentioned_account, mentioned_account2, in_reply_to_status.account].map(&:id) + + expect(status).to be_persisted + expect(status.conversation_id).to eq in_reply_to_status.conversation_id + expect(status.conversation_id).to eq first_status.conversation_id + expect(status.conversation.ancestor_status_id).to eq first_status.id + expect(status.mentions.pluck(:account_id)).to match_array thread_account_ids + end + + it 'do not duplicate replies when limited post' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + in_reply_to_status.mentions.create!(account: Fabricate(:account)) + + status = subject.call(Fabricate(:user).account, text: 'Ohagi is good', thread: in_reply_to_status, visibility: 'mutual') + + [in_reply_to_status.account, in_reply_to_status.mentions.first.account].map(&:id) + + expect(status).to be_persisted + expect(status.limited_scope).to eq 'personal' + + mentions = status.mentions.pluck(:account_id) + expect(mentions).to_not include in_reply_to_status.account_id + expect(mentions).to_not include in_reply_to_status.mentions.first.account_id + end + + it 'do not duplicate replies when not limited post' do + in_reply_to_status = Fabricate(:status, visibility: :limited) + in_reply_to_status.mentions.create!(account: Fabricate(:account)) + + status = subject.call(Fabricate(:user).account, text: 'Ohagi is good', thread: in_reply_to_status, visibility: 'public') + + [in_reply_to_status.account, in_reply_to_status.mentions.first.account].map(&:id) + + expect(status).to be_persisted + + mentions = status.mentions.pluck(:account_id) + expect(mentions).to_not include in_reply_to_status.account_id + expect(mentions).to_not include in_reply_to_status.mentions.first.account_id + end end it 'safeguards mentions' do diff --git a/spec/workers/activitypub/distribution_worker_spec.rb b/spec/workers/activitypub/distribution_worker_spec.rb index 3b11207d8eb817..635eac5042b359 100644 --- a/spec/workers/activitypub/distribution_worker_spec.rb +++ b/spec/workers/activitypub/distribution_worker_spec.rb @@ -63,6 +63,22 @@ end end + context 'with limited response status' do + before do + allow(ActivityPub::DeliveryWorker).to receive(:perform_async).with(kind_of(String), status.account.id, 'http://example.com/conversation/inbox', anything) + status.update(visibility: :limited, thread: Fabricate(:status)) + status.conversation.update(uri: 'https://example.com/conversation', inbox_url: 'http://example.com/conversation/inbox') + status.capability_tokens.create! + status.mentions.create!(account: follower, silent: true) + stub_request(:post, 'http://example.com/conversation/inbox') + end + + it 'delivers to followers' do + subject.perform(status.id) + expect(ActivityPub::DeliveryWorker).to have_received(:perform_async) + end + end + context 'with direct status' do let(:mentioned_account) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/inbox', domain: 'foo.bar') } From 1d9ff2a78068a52ebc66083832388b335fbc93ba Mon Sep 17 00:00:00 2001 From: KMY Date: Mon, 20 Nov 2023 13:37:33 +0900 Subject: [PATCH 11/24] Fix test --- app/lib/activitypub/activity/create.rb | 2 +- ...ss_conversion_service.rb => process_conversation_service.rb} | 2 +- ...ward_conversion_worker.rb => forward_conversation_worker.rb} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename app/services/{process_conversion_service.rb => process_conversation_service.rb} (95%) rename app/workers/activitypub/{forward_conversion_worker.rb => forward_conversation_worker.rb} (100%) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index cd9fee8b158a10..db5fa8d93b5204 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -515,7 +515,7 @@ def forward_for_reply def forward_for_conversation return unless @status.conversation.present? && @status.conversation.local? - ProcessConversionService.new.call(@status) + ProcessConversationService.new.call(@status) return if @json['signature'].blank? diff --git a/app/services/process_conversion_service.rb b/app/services/process_conversation_service.rb similarity index 95% rename from app/services/process_conversion_service.rb rename to app/services/process_conversation_service.rb index ccae2881936629..7facbb5a42cd5e 100644 --- a/app/services/process_conversion_service.rb +++ b/app/services/process_conversation_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class ProcessConversionService < BaseService +class ProcessConversationService < BaseService def call(status) @status = status diff --git a/app/workers/activitypub/forward_conversion_worker.rb b/app/workers/activitypub/forward_conversation_worker.rb similarity index 100% rename from app/workers/activitypub/forward_conversion_worker.rb rename to app/workers/activitypub/forward_conversation_worker.rb From ba02bbee36828945fdcf7f2464360d24bfc2fd35 Mon Sep 17 00:00:00 2001 From: KMY Date: Mon, 20 Nov 2023 14:02:27 +0900 Subject: [PATCH 12/24] Fix test --- app/services/post_status_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 0ae6d0a9a75fdc..b8c5d2778290f8 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -201,7 +201,7 @@ def postprocess_status! process_hashtags_service.call(@status) Trends.tags.register(@status) - ProcessConversionService.new.call(@status) if @status.limited_visibility? && @status.reply_limited? + ProcessConversationService.new.call(@status) if @status.limited_visibility? && @status.reply_limited? ProcessReferencesService.call_service(@status, @reference_ids, []) LinkCrawlWorker.perform_async(@status.id) DistributionWorker.perform_async(@status.id) From 6b1452b44a3713dd007617345d11509fe5155b8d Mon Sep 17 00:00:00 2001 From: KMY Date: Tue, 21 Nov 2023 08:42:13 +0900 Subject: [PATCH 13/24] =?UTF-8?q?Test:=20=E3=83=AD=E3=83=BC=E3=82=AB?= =?UTF-8?q?=E3=83=AB=E3=82=B9=E3=83=AC=E3=83=83=E3=83=89=E3=81=B8=E3=81=AE?= =?UTF-8?q?=E8=BF=94=E4=BF=A1=E6=8A=95=E7=A8=BF=E3=81=AE=E8=BB=A2=E9=80=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../forward_conversation_worker.rb | 2 + spec/lib/activitypub/activity/create_spec.rb | 172 +++++++++++++++++- 2 files changed, 173 insertions(+), 1 deletion(-) diff --git a/app/workers/activitypub/forward_conversation_worker.rb b/app/workers/activitypub/forward_conversation_worker.rb index 3bc846b4df2196..2037a216236f00 100644 --- a/app/workers/activitypub/forward_conversation_worker.rb +++ b/app/workers/activitypub/forward_conversation_worker.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ActivityPub::ForwardConversationWorker + include Sidekiq::Worker + def perform(payload, status_id) @status = Status.find(status_id) @payload = payload diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 4603bf0a4d3864..21bafe69c985c5 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -139,11 +139,12 @@ def activity_for_object(json) describe '#perform' do context 'when fetching' do - subject { described_class.new(json, sender) } + subject { delivered_to_account_id ? described_class.new(json, sender, delivered_to_account_id: delivered_to_account_id) : described_class.new(json, sender) } let(:sender_software) { 'mastodon' } let(:custom_before) { false } let(:active_friend) { false } + let(:delivered_to_account_id) { nil } before do Fabricate(:instance_info, domain: 'example.com', software: sender_software) @@ -1029,6 +1030,175 @@ def activity_for_object(json) end end + context 'with a context as a reply' do + let(:custom_before) { true } + let(:custom_before_sub) { false } + let(:ancestor_account) { Fabricate(:account, domain: 'or.example.com', inbox_url: 'http://or.example.com/actor/inbox') } + let(:mentioned_account) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/bob', inbox_url: 'http://example.com/bob/inbox', shared_inbox_url: 'http://exmaple.com/inbox') } + let(:local_mentioned_account) { Fabricate(:account, domain: nil) } + let(:original_status) { Fabricate(:status, conversation: conversation, account: ancestor_account) } + let!(:conversation) { Fabricate(:conversation) } + let(:recipient) { Fabricate(:account) } + let(:delivered_to_account_id) { recipient.id } + + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: [ActivityPub::TagManager.instance.uri_for(sender), '#foo'].join, + type: 'Create', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: object_json, + signature: 'dummy', + }.with_indifferent_access + end + + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + context: ActivityPub::TagManager.instance.uri_for(conversation), + inReplyTo: ActivityPub::TagManager.instance.uri_for(original_status), + } + end + + before do + original_status.mentions << Fabricate(:mention, account: mentioned_account, silent: true) + original_status.mentions << Fabricate(:mention, account: local_mentioned_account, silent: true) + original_status.save! + conversation.update(ancestor_status: original_status) + + stub_request(:post, 'http://or.example.com/actor/inbox').to_return(status: 200) + stub_request(:post, 'http://example.com/bob/inbox').to_return(status: 200) + + subject.perform unless custom_before_sub + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.conversation_id).to eq conversation.id + expect(status.thread.id).to eq original_status.id + expect(status.mentions.map(&:account_id)).to contain_exactly(recipient.id, ancestor_account.id, mentioned_account.id, local_mentioned_account.id) + end + + it 'forwards to observers' do + expect(a_request(:post, 'http://or.example.com/actor/inbox')).to have_been_made.once + expect(a_request(:post, 'http://example.com/bob/inbox')).to have_been_made.once + end + + context 'when new mention is added' do + let(:custom_before_sub) { true } + + let(:new_mentioned_account) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/alice', inbox_url: 'http://example.com/alice/inbox', shared_inbox_url: 'http://exmaple.com/inbox') } + let(:new_local_mentioned_account) { Fabricate(:account, domain: nil) } + + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + context: ActivityPub::TagManager.instance.uri_for(conversation), + inReplyTo: ActivityPub::TagManager.instance.uri_for(original_status), + tag: [ + { + type: 'Mention', + href: ActivityPub::TagManager.instance.uri_for(new_mentioned_account), + }, + { + type: 'Mention', + href: ActivityPub::TagManager.instance.uri_for(new_local_mentioned_account), + }, + ], + } + end + + before do + stub_request(:post, 'http://example.com/alice/inbox').to_return(status: 200) + subject.perform + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.mentions.map(&:account_id)).to contain_exactly(recipient.id, ancestor_account.id, mentioned_account.id, local_mentioned_account.id, new_mentioned_account.id, new_local_mentioned_account.id) + end + + it 'forwards to observers' do + expect(a_request(:post, 'http://or.example.com/actor/inbox')).to have_been_made.once + expect(a_request(:post, 'http://example.com/bob/inbox')).to have_been_made.once + expect(a_request(:post, 'http://example.com/alice/inbox')).to have_been_made.once + end + end + + context 'when unknown mentioned account' do + let(:custom_before_sub) { true } + + let(:actor_json) do + { + id: 'https://foo.test', + type: 'Actor', + inbox: 'https://foo.test/inbox', + followers: 'https://example.com/followers', + actor_type: 'Person', + }.with_indifferent_access + end + + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + context: ActivityPub::TagManager.instance.uri_for(conversation), + inReplyTo: ActivityPub::TagManager.instance.uri_for(original_status), + tag: [ + { + type: 'Mention', + href: 'https://foo.test', + }, + ], + } + end + + before do + stub_request(:get, 'https://foo.test').to_return(status: 200, body: Oj.dump(actor_json)) + stub_request(:post, 'https://foo.test/inbox').to_return(status: 200) + subject.perform + end + + it 'creates status', pending: 'in development' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.mentioned_accounts.map(&:uri)).to include 'https://foo.test' + end + + it 'forwards to observers', pending: 'in development' do + expect(a_request(:post, 'https://foo.test/inbox')).to have_been_made.once + end + end + + context 'when remote conversation' do + let(:conversation) { Fabricate(:conversation, uri: 'http://example.com/conversation', inbox_url: 'http://example.com/actor/inbox') } + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.conversation_id).to eq conversation.id + expect(status.thread.id).to eq original_status.id + expect(status.mentions.map(&:account_id)).to contain_exactly(recipient.id) + end + + it 'do not forward to observers' do + expect(a_request(:post, 'http://or.example.com/actor/inbox')).to_not have_been_made + expect(a_request(:post, 'http://example.com/bob/inbox')).to_not have_been_made + end + end + end + context 'with media attachments' do let(:object_json) do { From 71ead7e922f06ffd678c9aa50ce42250b4f2a8d6 Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 22 Nov 2023 08:32:33 +0900 Subject: [PATCH 14/24] =?UTF-8?q?Test:=20=E6=9C=AA=E7=9F=A5=E3=81=AE?= =?UTF-8?q?=E3=82=A2=E3=82=AB=E3=82=A6=E3=83=B3=E3=83=88=E3=81=8B=E3=82=89?= =?UTF-8?q?=E3=81=AE=E3=83=A1=E3=83=B3=E3=82=B7=E3=83=A7=E3=83=B3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/lib/activitypub/activity/create_spec.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 21bafe69c985c5..75ed9cbffb8b4c 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -1138,13 +1138,15 @@ def activity_for_object(json) let(:actor_json) do { + '@context': 'https://www.w3.org/ns/activitystreams', id: 'https://foo.test', - type: 'Actor', + type: 'Person', + preferredUsername: 'actor', + name: 'Tomas Cat', inbox: 'https://foo.test/inbox', - followers: 'https://example.com/followers', - actor_type: 'Person', }.with_indifferent_access end + let!(:webfinger) { { subject: 'acct:actor@foo.test', links: [{ rel: 'self', href: 'https://foo.test' }] } } let(:object_json) do { @@ -1164,18 +1166,20 @@ def activity_for_object(json) before do stub_request(:get, 'https://foo.test').to_return(status: 200, body: Oj.dump(actor_json)) + stub_request(:get, 'https://foo.test/.well-known/webfinger?resource=acct:actor@foo.test').to_return(status: 200, body: Oj.dump(webfinger)) stub_request(:post, 'https://foo.test/inbox').to_return(status: 200) + stub_request(:get, 'https://foo.test/.well-known/nodeinfo').to_return(status: 200) subject.perform end - it 'creates status', pending: 'in development' do + it 'creates status' do status = sender.statuses.first expect(status).to_not be_nil expect(status.mentioned_accounts.map(&:uri)).to include 'https://foo.test' end - it 'forwards to observers', pending: 'in development' do + it 'forwards to observers' do expect(a_request(:post, 'https://foo.test/inbox')).to have_been_made.once end end From e588a148383d75397a3972dd7b5e8b913a59255e Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 24 Nov 2023 08:56:41 +0900 Subject: [PATCH 15/24] =?UTF-8?q?Add:=20=E7=B7=A8=E9=9B=86=E3=83=BB?= =?UTF-8?q?=E5=89=8A=E9=99=A4=E3=81=AE=E9=80=A3=E5=90=88=E3=81=AB=E5=AF=BE?= =?UTF-8?q?=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/status_reach_finder.rb | 6 +- app/services/remove_status_service.rb | 8 +++ .../status_update_distribution_worker.rb | 10 +++- spec/lib/activitypub/activity/update_spec.rb | 41 +++++++++++++ spec/services/remove_status_service_spec.rb | 57 ++++++++++++++++++- .../status_update_distribution_worker_spec.rb | 44 +++++++++++++- 6 files changed, 159 insertions(+), 7 deletions(-) diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 62634f36c1b3d7..2efe9d38143ab2 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -54,9 +54,9 @@ def reached_account_inboxes_for_misskey if @status.reblog? [] elsif @status.limited_visibility? - Account.where(id: mentioned_account_ids).where(domain: banned_domains_for_misskey).inboxes + Account.where(id: mentioned_account_ids, domain: banned_domains_for_misskey).inboxes else - Account.where(id: reached_account_ids).where(domain: banned_domains_for_misskey - friend_domains).inboxes + Account.where(id: reached_account_ids, domain: banned_domains_for_misskey - friend_domains).inboxes end end @@ -64,7 +64,7 @@ def reached_account_inboxes_for_friend if @status.reblog? [] elsif @status.limited_visibility? - Account.where(id: mentioned_account_ids).where.not(domain: banned_domains).inboxes + Account.where(id: mentioned_account_ids, domain: friend_domains).where.not(domain: banned_domains).inboxes else Account.where(id: reached_account_ids, domain: friend_domains).where.not(domain: banned_domains - friend_domains).inboxes end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index ada2fde1be14ee..7bdf5a84017284 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -106,6 +106,8 @@ def remove_from_remote_reach # the author and wouldn't normally receive the delete # notification - so here, we explicitly send it to them + return remove_from_conversation if @status.limited_visibility? && @status.conversation.present? && !@status.conversation.local? + status_reach_finder = StatusReachFinder.new(@status, unsafe: true) ActivityPub::DeliveryWorker.push_bulk(status_reach_finder.all_inboxes, limit: 1_000) do |inbox_url| @@ -113,6 +115,12 @@ def remove_from_remote_reach end end + def remove_from_conversation + return if @status.conversation.nil? || @status.conversation.inbox_url.blank? + + ActivityPub::DeliveryWorker.perform_async(signed_activity_json, @account.id, @status.conversation.inbox_url) + end + def signed_activity_json @signed_activity_json ||= Oj.dump(serialize_payload(@status, @status.reblog? ? ActivityPub::UndoAnnounceSerializer : ActivityPub::DeleteSerializer, signer: @account, always_sign: true)) end diff --git a/app/workers/activitypub/status_update_distribution_worker.rb b/app/workers/activitypub/status_update_distribution_worker.rb index 49ac509761ab66..3379f7e6e83382 100644 --- a/app/workers/activitypub/status_update_distribution_worker.rb +++ b/app/workers/activitypub/status_update_distribution_worker.rb @@ -8,13 +8,21 @@ def perform(status_id, options = {}) @status = Status.find(status_id) @account = @status.account - distribute! + if @status.limited_visibility? + distribute_limited! + else + distribute! + end rescue ActiveRecord::RecordNotFound true end protected + def inboxes_for_limited + @inboxes_for_limited ||= @status.mentioned_accounts.inboxes + end + def build_activity(for_misskey: false, for_friend: false) ActivityPub::ActivityPresenter.new( id: [ActivityPub::TagManager.instance.uri_for(@status), '#updates/', @status.edited_at.to_i].join, diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 87e96d2d1b12cb..5bb5993ba370ef 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -64,6 +64,47 @@ end end + context 'with a Status object' do + let!(:at_time) { Time.now.utc } + let!(:status) { Fabricate(:status, uri: 'https://example.com/status', account: sender) } + + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Update', + actor: sender.uri, + object: { + type: 'Note', + id: status.uri, + content: 'Foo', + updated: at_time.iso8601, + }, + }.with_indifferent_access + end + + before do + subject.perform + end + + it 'content is updated' do + expect(status.reload.text).to eq 'Foo' + end + + it 'set status as edited' do + expect(status.reload.edited_at).to_not be_nil + end + + it 'history is set' do + expect(status.reload.edits.count).to eq 2 + end + + it 'duplication activity' do + subject.perform # again + expect(status.reload.edits.count).to eq 2 + end + end + context 'with a Question object' do let!(:at_time) { Time.now.utc } let!(:status) { Fabricate(:status, uri: 'https://example.com/statuses/poll', account: sender, poll: Poll.new(account: sender, options: %w(Bar Baz), cached_tallies: [0, 0], expires_at: at_time + 5.days)) } diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index 7754ae80047892..bbba222f752acb 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -8,12 +8,14 @@ let!(:alice) { Fabricate(:account) } let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com') } let!(:jeff) { Fabricate(:account) } - let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } - let!(:bill) { Fabricate(:account, username: 'bill', protocol: :activitypub, domain: 'example2.com', inbox_url: 'http://example2.com/inbox') } + let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', shared_inbox_url: 'http://example.com/inbox', inbox_url: 'http://example.com/hank/inbox') } + let!(:bill) { Fabricate(:account, username: 'bill', protocol: :activitypub, domain: 'example2.com', shared_inbox_url: 'http://example2.com/inbox', inbox_url: 'http://example2.com/bill/inbox') } before do stub_request(:post, 'http://example.com/inbox').to_return(status: 200) + stub_request(:post, 'http://example.com/hank/inbox').to_return(status: 200) stub_request(:post, 'http://example2.com/inbox').to_return(status: 200) + stub_request(:post, 'http://example2.com/bill/inbox').to_return(status: 200) jeff.follow!(alice) hank.follow!(alice) @@ -72,6 +74,57 @@ end end + context 'when removed status is limited' do + let(:status) { PostStatusService.new.call(alice, visibility: 'mutual', text: 'limited post') } + + before do + status.mentions << Fabricate(:mention, account: hank, silent: true) + end + + it 'sends Delete activity to followers' do + subject.call(status) + expect(a_request(:post, 'http://example.com/inbox').with( + body: hash_including({ + 'type' => 'Delete', + 'object' => { + 'type' => 'Tombstone', + 'id' => ActivityPub::TagManager.instance.uri_for(status), + 'atomUri' => OStatus::TagManager.instance.uri_for(status), + }, + }) + )).to have_been_made.once + end + end + + context 'when removed status is limited and remote conversation' do + let(:status) { PostStatusService.new.call(alice, visibility: 'mutual', text: 'limited post') } + + before do + status.conversation.update(uri: 'http://example2.com/conversation', inbox_url: 'http://example2.com/bill/inbox') + status.mentions << Fabricate(:mention, account: hank, silent: true) + end + + it 'sends Delete activity to conversation' do + subject.call(status) + expect(a_request(:post, 'http://example2.com/bill/inbox').with( + body: hash_including({ + 'type' => 'Delete', + 'object' => { + 'type' => 'Tombstone', + 'id' => ActivityPub::TagManager.instance.uri_for(status), + 'atomUri' => OStatus::TagManager.instance.uri_for(status), + }, + }) + )).to have_been_made.once + end + + it 'do not send Delete activity to followers' do + subject.call(status) + expect(a_request(:post, 'http://example.com/hank/inbox')).to_not have_been_made + expect(a_request(:post, 'http://example.com/inbox')).to_not have_been_made + end + end + context 'when removed status is a private self-reblog' do let!(:original_status) { Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :private) } let!(:status) { ReblogService.new.call(alice, original_status) } diff --git a/spec/workers/activitypub/status_update_distribution_worker_spec.rb b/spec/workers/activitypub/status_update_distribution_worker_spec.rb index a4fd246e534eb9..8f2cefcc32694b 100644 --- a/spec/workers/activitypub/status_update_distribution_worker_spec.rb +++ b/spec/workers/activitypub/status_update_distribution_worker_spec.rb @@ -6,7 +6,7 @@ subject { described_class.new } let(:status) { Fabricate(:status, text: 'foo') } - let(:follower) { Fabricate(:account, protocol: :activitypub, inbox_url: 'http://example.com', domain: 'example.com') } + let(:follower) { Fabricate(:account, protocol: :activitypub, shared_inbox_url: 'http://example.com', inbox_url: 'http://example.com/follower/inbox', domain: 'example.com') } describe '#perform' do before do @@ -31,6 +31,18 @@ end end + context 'with unlisted status' do + before do + status.update(visibility: :unlisted) + end + + it 'delivers to followers' do + expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'http://example.com', anything]]) do + subject.perform(status.id) + end + end + end + context 'with private status' do before do status.update(visibility: :private) @@ -42,5 +54,35 @@ end end end + + context 'with limited status' do + before do + status.update(visibility: :limited) + status.capability_tokens.create! + status.mentions.create!(account: follower, silent: true) + end + + it 'delivers to followers' do + expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'http://example.com', anything]]) do + subject.perform(status.id) + end + end + end + + context 'with limited response status' do + before do + allow(ActivityPub::DeliveryWorker).to receive(:perform_async).with(kind_of(String), status.account.id, 'http://example.com/conversation/inbox', anything) + status.update(visibility: :limited, thread: Fabricate(:status)) + status.conversation.update(uri: 'https://example.com/conversation', inbox_url: 'http://example.com/conversation/inbox') + status.capability_tokens.create! + status.mentions.create!(account: follower, silent: true) + stub_request(:post, 'http://example.com/conversation/inbox') + end + + it 'delivers to followers' do + subject.perform(status.id) + expect(ActivityPub::DeliveryWorker).to have_received(:perform_async) + end + end end end From aec726bc71fa696e04218bf799d96d62141b8621 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 24 Nov 2023 08:59:02 +0900 Subject: [PATCH 16/24] =?UTF-8?q?Remove:=20=E9=87=8D=E8=A4=87=E3=83=86?= =?UTF-8?q?=E3=82=B9=E3=83=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/lib/activitypub/activity/update_spec.rb | 41 -------------------- 1 file changed, 41 deletions(-) diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 5bb5993ba370ef..87e96d2d1b12cb 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -64,47 +64,6 @@ end end - context 'with a Status object' do - let!(:at_time) { Time.now.utc } - let!(:status) { Fabricate(:status, uri: 'https://example.com/status', account: sender) } - - let(:json) do - { - '@context': 'https://www.w3.org/ns/activitystreams', - id: 'foo', - type: 'Update', - actor: sender.uri, - object: { - type: 'Note', - id: status.uri, - content: 'Foo', - updated: at_time.iso8601, - }, - }.with_indifferent_access - end - - before do - subject.perform - end - - it 'content is updated' do - expect(status.reload.text).to eq 'Foo' - end - - it 'set status as edited' do - expect(status.reload.edited_at).to_not be_nil - end - - it 'history is set' do - expect(status.reload.edits.count).to eq 2 - end - - it 'duplication activity' do - subject.perform # again - expect(status.reload.edits.count).to eq 2 - end - end - context 'with a Question object' do let!(:at_time) { Time.now.utc } let!(:status) { Fabricate(:status, uri: 'https://example.com/statuses/poll', account: sender, poll: Poll.new(account: sender, options: %w(Bar Baz), cached_tallies: [0, 0], expires_at: at_time + 5.days)) } From 320f0800acfee493522f6c8a3e20ec34cd4da594 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 24 Nov 2023 09:12:55 +0900 Subject: [PATCH 17/24] =?UTF-8?q?Fix:=20=E6=94=B9=E5=96=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/status_reach_finder.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 2efe9d38143ab2..f588e4374ddc6d 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -51,22 +51,18 @@ def reached_account_inboxes end def reached_account_inboxes_for_misskey - if @status.reblog? + if @status.reblog? || @status.limited_visibility? [] - elsif @status.limited_visibility? - Account.where(id: mentioned_account_ids, domain: banned_domains_for_misskey).inboxes else Account.where(id: reached_account_ids, domain: banned_domains_for_misskey - friend_domains).inboxes end end def reached_account_inboxes_for_friend - if @status.reblog? + if @status.reblog? || @status.limited_visibility? [] - elsif @status.limited_visibility? - Account.where(id: mentioned_account_ids, domain: friend_domains).where.not(domain: banned_domains).inboxes else - Account.where(id: reached_account_ids, domain: friend_domains).where.not(domain: banned_domains - friend_domains).inboxes + Account.where(id: reached_account_ids, domain: friend_domains).inboxes end end From 104da54a87c2e58272d829c69c6fc3add006385a Mon Sep 17 00:00:00 2001 From: KMY Date: Mon, 27 Nov 2023 10:49:09 +0900 Subject: [PATCH 18/24] =?UTF-8?q?Add:=20=E7=B7=A8=E9=9B=86=E5=89=8A?= =?UTF-8?q?=E9=99=A4=E3=81=AE=E8=BB=A2=E9=80=81=E5=87=A6=E7=90=86=E3=83=BB?= =?UTF-8?q?=E8=BF=94=E4=BF=A1=E3=81=AA=E3=81=AE=E3=81=ABsilent=E3=81=AA?= =?UTF-8?q?=E3=83=A1=E3=83=B3=E3=82=B7=E3=83=A7=E3=83=B3=E3=81=A7=E3=81=AE?= =?UTF-8?q?=E9=80=9A=E7=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/javascript/mastodon/reducers/compose.js | 4 ++ app/lib/activitypub/activity/create.rb | 4 +- app/lib/activitypub/activity/delete.rb | 7 +++ app/lib/activitypub/activity/update.rb | 8 +++ app/models/status.rb | 1 + app/services/fan_out_on_write_service.rb | 12 +++++ .../activitypub/distribution_worker.rb | 10 ++-- .../forward_conversation_worker.rb | 15 ++++-- .../status_update_distribution_worker.rb | 4 ++ spec/lib/activitypub/activity/delete_spec.rb | 21 ++++++++ spec/lib/activitypub/activity/update_spec.rb | 34 +++++++++++++ .../services/fan_out_on_write_service_spec.rb | 50 +++++++++++++++++++ 12 files changed, 161 insertions(+), 9 deletions(-) diff --git a/app/javascript/mastodon/reducers/compose.js b/app/javascript/mastodon/reducers/compose.js index 6ea511ed45db7f..d8704088381f47 100644 --- a/app/javascript/mastodon/reducers/compose.js +++ b/app/javascript/mastodon/reducers/compose.js @@ -115,6 +115,10 @@ const initialPoll = ImmutableMap({ }); function statusToTextMentions(state, status) { + if (status.get('visibility_ex') === 'limited') { + return ''; + } + let set = ImmutableOrderedSet([]); if (status.getIn(['account', 'id']) !== me) { diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index db5fa8d93b5204..d9cd9b727206d7 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -93,10 +93,10 @@ def process_status resolve_thread(@status) fetch_replies(@status) + process_conversation! if @status.limited_visibility? process_references! distribute forward_for_reply - forward_for_conversation if @status.limited_visibility? join_group! end @@ -512,7 +512,7 @@ def forward_for_reply ActivityPub::RawDistributionWorker.perform_async(Oj.dump(@json), replied_to_status.account_id, [@account.preferred_inbox_url]) end - def forward_for_conversation + def process_conversation! return unless @status.conversation.present? && @status.conversation.local? ProcessConversationService.new.call(@status) diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index f401714430a419..4f02d834273dbe 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -40,10 +40,17 @@ def delete_note return if @status.nil? forwarder.forward! if forwarder.forwardable? + forward_for_conversation delete_now! end end + def forward_for_conversation + return unless @status.conversation.present? && @status.conversation.local? && @json['signature'].present? + + ActivityPub::ForwardConversationWorker.perform_async(Oj.dump(@json), @status.id, true) + end + def delete_friend friend = FriendDomain.find_by(domain: @account.domain) friend&.destroy diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 973706f5957904..1e98eecd583ece 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -31,5 +31,13 @@ def update_status return if @status.nil? ActivityPub::ProcessStatusUpdateService.new.call(@status, @json, @object, request_id: @options[:request_id]) + + forward_for_conversation + end + + def forward_for_conversation + return unless @status.conversation.present? && @status.conversation.local? && @json['signature'].present? + + ActivityPub::ForwardConversationWorker.perform_async(Oj.dump(@json), @status.id, true) end end diff --git a/app/models/status.rb b/app/models/status.rb index 631ffc4aaaf394..e1d42c0e6f1a11 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -84,6 +84,7 @@ class Status < ApplicationRecord has_many :mentions, dependent: :destroy, inverse_of: :status has_many :mentioned_accounts, through: :mentions, source: :account, class_name: 'Account' has_many :active_mentions, -> { active }, class_name: 'Mention', inverse_of: :status + has_many :silent_mentions, -> { silent }, class_name: 'Mention', inverse_of: :status has_many :media_attachments, dependent: :nullify has_many :reference_objects, class_name: 'StatusReference', inverse_of: :status, dependent: :destroy has_many :references, through: :reference_objects, class_name: 'Status', source: :target_status diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index 1455eed2ff8251..41515fa59b4998 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -46,6 +46,7 @@ def fan_out_to_local_recipients! unless @options[:skip_notifications] notify_mentioned_accounts! + notify_for_conversation! if @status.limited_visibility? notify_about_update! if update? end @@ -93,6 +94,17 @@ def notify_mentioned_accounts! end end + def notify_for_conversation! + return if @status.conversation.nil? + + account_ids = @status.conversation.statuses.pluck(:account_id).uniq.reject { |account_id| account_id == @status.account_id } + @status.silent_mentions.where(account_id: account_ids).joins(:account).merge(Account.local).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| + LocalNotificationWorker.push_bulk(mentions) do |mention| + [mention.account_id, mention.id, 'Mention', 'mention'] + end + end + end + def notify_about_update! @status.reblogged_by_accounts.or(@status.quoted_by_accounts).merge(Account.local).select(:id).reorder(nil).find_in_batches do |accounts| LocalNotificationWorker.push_bulk(accounts) do |account| diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 4ea25a58e14938..2e99396c25d680 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -60,15 +60,15 @@ def status_reach_finder end def payload - @payload ||= Oj.dump(serialize_payload(activity, ActivityPub::ActivitySerializer, signer: @account)) + @payload ||= Oj.dump(serialize_payload(activity, ActivityPub::ActivitySerializer, signer: @account, always_sign: always_sign)) end def payload_for_misskey - @payload_for_misskey ||= Oj.dump(serialize_payload(activity_for_misskey, ActivityPub::ActivityForMisskeySerializer, signer: @account)) + @payload_for_misskey ||= Oj.dump(serialize_payload(activity_for_misskey, ActivityPub::ActivityForMisskeySerializer, signer: @account, always_sign: always_sign)) end def payload_for_friend - @payload_for_friend ||= Oj.dump(serialize_payload(activity_for_friend, ActivityPub::ActivityForFriendSerializer, signer: @account)) + @payload_for_friend ||= Oj.dump(serialize_payload(activity_for_friend, ActivityPub::ActivityForFriendSerializer, signer: @account, always_sign: always_sign)) end def activity @@ -83,6 +83,10 @@ def activity_for_friend ActivityPub::ActivityPresenter.from_status(@status, for_friend: true) end + def always_sign + false + end + def options { 'synchronize_followers' => @status.private_visibility? } end diff --git a/app/workers/activitypub/forward_conversation_worker.rb b/app/workers/activitypub/forward_conversation_worker.rb index 2037a216236f00..770e94c3c39251 100644 --- a/app/workers/activitypub/forward_conversation_worker.rb +++ b/app/workers/activitypub/forward_conversation_worker.rb @@ -3,9 +3,10 @@ class ActivityPub::ForwardConversationWorker include Sidekiq::Worker - def perform(payload, status_id) + def perform(payload, status_id, shared_inbox = false) # rubocop:disable Style/OptionalBooleanParameter @status = Status.find(status_id) @payload = payload + @shared_inbox = shared_inbox return unless @status.conversation.present? && @status.conversation.local? && @status.conversation.ancestor_status.present? return unless @status.limited_visibility? @@ -26,9 +27,15 @@ def distribute_limited_mentions! end def inboxes_for_limited - DeliveryFailureTracker.without_unavailable( - Account.remote.merge(@status.mentioned_accounts).pluck(:inbox_url).compact_blank.uniq - ) + if @shared_inbox + inbox_accounts.inboxes + else + DeliveryFailureTracker.without_unavailable(inbox_accounts.pluck(:inbox_url).compact_blank.uniq) + end + end + + def inbox_accounts + Account.remote.merge(@status.mentioned_accounts) end def options diff --git a/app/workers/activitypub/status_update_distribution_worker.rb b/app/workers/activitypub/status_update_distribution_worker.rb index 3379f7e6e83382..26460905737c2b 100644 --- a/app/workers/activitypub/status_update_distribution_worker.rb +++ b/app/workers/activitypub/status_update_distribution_worker.rb @@ -46,4 +46,8 @@ def activity_for_misskey def activity_for_friend build_activity(for_friend: true) end + + def always_sign + true + end end diff --git a/spec/lib/activitypub/activity/delete_spec.rb b/spec/lib/activitypub/activity/delete_spec.rb index f0c957c8a162c0..5fd0c557ff30fb 100644 --- a/spec/lib/activitypub/activity/delete_spec.rb +++ b/spec/lib/activitypub/activity/delete_spec.rb @@ -74,6 +74,27 @@ end end + context 'when the status is limited post and has conversation' do + subject { described_class.new(json, sender) } + + let(:conversation) { Fabricate(:conversation, ancestor_status: status) } + + before do + status.update(conversation: conversation, visibility: :limited) + status.mentions << Fabricate(:mention, silent: true, account: Fabricate(:account, protocol: :activitypub, domain: 'example.com', inbox_url: 'https://example.com/actor/inbox', shared_inbox_url: 'https://example.com/inbox')) + status.save + stub_request(:post, 'https://example.com/inbox').to_return(status: 200) + subject.perform + end + + it 'forwards to parent status holder' do + expect(a_request(:post, 'https://example.com/inbox').with(body: hash_including({ + type: 'Delete', + signature: 'foo', + }))).to have_been_made.once + end + end + context 'when given a friend server' do subject { described_class.new(json, sender) } diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 87e96d2d1b12cb..c300768ff28545 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -115,5 +115,39 @@ expect(status.edited_at).to be_nil end end + + context 'when the status is limited post and has conversation' do + let(:status) { Fabricate(:status, visibility: :limited, account: sender, uri: 'https://example.com/note', text: 'Ohagi is koshian') } + let(:conversation) { Fabricate(:conversation, ancestor_status: status) } + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Update', + actor: sender.uri, + signature: 'foo', + object: { + type: 'Note', + id: status.uri, + content: 'Ohagi is tsubuan', + }, + }.with_indifferent_access + end + + before do + status.update(conversation: conversation, visibility: :limited) + status.mentions << Fabricate(:mention, silent: true, account: Fabricate(:account, protocol: :activitypub, domain: 'example.com', inbox_url: 'https://example.com/actor/inbox', shared_inbox_url: 'https://example.com/inbox')) + status.save + stub_request(:post, 'https://example.com/inbox').to_return(status: 200) + subject.perform + end + + it 'forwards to parent status holder' do + expect(a_request(:post, 'https://example.com/inbox').with(body: hash_including({ + type: 'Update', + signature: 'foo', + }))).to have_been_made.once + end + end end end diff --git a/spec/services/fan_out_on_write_service_spec.rb b/spec/services/fan_out_on_write_service_spec.rb index d7393a7e5e7b69..2f9973bcd9a58b 100644 --- a/spec/services/fan_out_on_write_service_spec.rb +++ b/spec/services/fan_out_on_write_service_spec.rb @@ -683,6 +683,56 @@ def antenna_with_options(owner, **options) end end + context 'when status has a conversation' do + let(:conversation) { Fabricate(:conversation) } + let(:status) { Fabricate(:status, account: alice, visibility: visibility, thread: parent_status, conversation: conversation) } + let(:parent_status) { Fabricate(:status, account: bob, visibility: visibility, conversation: conversation) } + let(:custom_before) { true } + + before do + Fabricate(:status, account: tom, visibility: visibility, conversation: conversation) + Fabricate(:status, account: ohagi, visibility: visibility, conversation: conversation) + status.mentions << Fabricate(:mention, account: bob, silent: true) + status.mentions << Fabricate(:mention, account: ohagi, silent: true) + status.mentions << Fabricate(:mention, account: tom, silent: false) + status.save + subject.call(status) + end + + context 'when public visibility' do + it 'does not create notification' do + notification = Notification.find_by(account: bob, type: 'mention') + + expect(notification).to be_nil + end + + it 'creates notification for active mention' do + notification = Notification.find_by(account: tom, type: 'mention') + + expect(notification).to_not be_nil + expect(notification.mention.status_id).to eq status.id + end + end + + context 'when limited visibility' do + let(:visibility) { :limited } + + it 'creates notification' do + notification = Notification.find_by(account: bob, type: 'mention') + + expect(notification).to_not be_nil + expect(notification.mention.status_id).to eq status.id + end + + it 'creates notification for other conversation account' do + notification = Notification.find_by(account: ohagi, type: 'mention') + + expect(notification).to_not be_nil + expect(notification.mention.status_id).to eq status.id + end + end + end + context 'when updated status is already boosted or quoted' do let(:custom_before) { true } From 00ee8d156899817aa99c7c9d6b8f70e2ad2748ac Mon Sep 17 00:00:00 2001 From: KMY Date: Tue, 28 Nov 2023 09:13:24 +0900 Subject: [PATCH 19/24] =?UTF-8?q?Fix:=20=E3=83=AA=E3=83=97=E3=83=A9?= =?UTF-8?q?=E3=82=A4=E3=81=8C=E7=AC=AC=E4=B8=89=E8=80=85=E3=81=AB=E5=B1=8A?= =?UTF-8?q?=E3=81=8B=E3=81=AA=E3=81=84=E5=95=8F=E9=A1=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/feed_manager.rb | 18 +++++++----- .../services/fan_out_on_write_service_spec.rb | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index f993ee5207927c..54371e8b731aa4 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -409,9 +409,9 @@ def blocks_or_mutes?(receiver_id, account_ids, context) # @param [Integer] receiver_id # @param [Hash] crutches # @return [Boolean] - def filter_from_home?(status, receiver_id, crutches, timeline_type = :home, stl_home = false) # rubocop:disable Style/OptionalBooleanParameter + def filter_from_home?(status, receiver_id, crutches, timeline_type = :home, stl_home = false) # rubocop:disable Style/OptionalBooleanParameter, Metrics/PerceivedComplexity return false if receiver_id == status.account_id - return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) + return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) && !(timeline_type == :home && status.limited_visibility?) return true if (timeline_type != :list || stl_home) && (crutches[:exclusive_list_users][status.account_id].present? || crutches[:exclusive_antenna_users][status.account_id].present?) return true if crutches[:languages][status.account_id].present? && status.language.present? && !crutches[:languages][status.account_id].include?(status.language) @@ -426,10 +426,11 @@ def filter_from_home?(status, receiver_id, crutches, timeline_type = :home, stl_ return true if check_for_blocks.any? { |target_account_id| crutches[:blocking][target_account_id] || crutches[:muting][target_account_id] } return true if crutches[:blocked_by][status.account_id] - if status.reply? && !status.in_reply_to_account_id.nil? # Filter out if it's a reply - should_filter = !crutches[:following][status.in_reply_to_account_id] # and I'm not following the person it's a reply to - should_filter &&= receiver_id != status.in_reply_to_account_id # and it's not a reply to me - should_filter &&= status.account_id != status.in_reply_to_account_id # and it's not a self-reply + if status.reply? && (!status.in_reply_to_account_id.nil? || (status.thread.present? && status.limited_visibility?)) # Filter out if it's a reply + account_id = status.in_reply_to_account_id || status.thread.account_id + should_filter = !crutches[:following][account_id] # and I'm not following the person it's a reply to + should_filter &&= receiver_id != account_id # and it's not a reply to me + should_filter &&= status.account_id != account_id # and it's not a self-reply return !!should_filter elsif status.reblog? # Filter out a reblog @@ -607,7 +608,10 @@ def build_crutches(receiver_id, statuses) lists = List.where(account_id: receiver_id, exclusive: true) antennas = Antenna.where(list: lists, insert_feeds: true) - crutches[:following] = Follow.where(account_id: receiver_id, target_account_id: statuses.filter_map(&:in_reply_to_account_id)).pluck(:target_account_id).index_with(true) + replied_accounts = statuses.filter_map(&:in_reply_to_account_id) + replied_accounts += statuses.filter { |status| status.limited_visibility? && status.thread.present? }.map { |status| status.thread.account_id } + + crutches[:following] = Follow.where(account_id: receiver_id, target_account_id: replied_accounts).pluck(:target_account_id).index_with(true) crutches[:languages] = Follow.where(account_id: receiver_id, target_account_id: statuses.map(&:account_id)).pluck(:target_account_id, :languages).to_h crutches[:hiding_reblogs] = Follow.where(account_id: receiver_id, target_account_id: statuses.filter_map { |s| s.account_id if s.reblog? }, show_reblogs: false).pluck(:target_account_id).index_with(true) crutches[:blocking] = Block.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).index_with(true) diff --git a/spec/services/fan_out_on_write_service_spec.rb b/spec/services/fan_out_on_write_service_spec.rb index 2f9973bcd9a58b..aa6c64624e8f38 100644 --- a/spec/services/fan_out_on_write_service_spec.rb +++ b/spec/services/fan_out_on_write_service_spec.rb @@ -687,13 +687,17 @@ def antenna_with_options(owner, **options) let(:conversation) { Fabricate(:conversation) } let(:status) { Fabricate(:status, account: alice, visibility: visibility, thread: parent_status, conversation: conversation) } let(:parent_status) { Fabricate(:status, account: bob, visibility: visibility, conversation: conversation) } + let(:zilu) { Fabricate(:user, current_sign_in_at: last_active_at).account } let(:custom_before) { true } before do + zilu.follow!(alice) + zilu.follow!(bob) Fabricate(:status, account: tom, visibility: visibility, conversation: conversation) Fabricate(:status, account: ohagi, visibility: visibility, conversation: conversation) status.mentions << Fabricate(:mention, account: bob, silent: true) status.mentions << Fabricate(:mention, account: ohagi, silent: true) + status.mentions << Fabricate(:mention, account: zilu, silent: true) status.mentions << Fabricate(:mention, account: tom, silent: false) status.save subject.call(status) @@ -712,6 +716,18 @@ def antenna_with_options(owner, **options) expect(notification).to_not be_nil expect(notification.mention.status_id).to eq status.id end + + it 'inserts home feed for reply' do + expect(home_feed_of(bob)).to include status.id + end + + it 'inserts home feed for non-replied but mentioned and following replied account' do + expect(home_feed_of(zilu)).to include status.id + end + + it 'does not insert home feed for non-replied, non-following replied account but mentioned' do + expect(home_feed_of(tom)).to_not include status.id + end end context 'when limited visibility' do @@ -730,6 +746,18 @@ def antenna_with_options(owner, **options) expect(notification).to_not be_nil expect(notification.mention.status_id).to eq status.id end + + it 'inserts home feed for reply' do + expect(home_feed_of(bob)).to include status.id + end + + it 'inserts home feed for non-replied but mentioned and following replied account' do + expect(home_feed_of(zilu)).to include status.id + end + + it 'does not insert home feed for non-replied, non-following replied account but mentioned' do + expect(home_feed_of(tom)).to_not include status.id + end end end From d6c211b113995c92665e4b22cf942711de4c7342 Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 29 Nov 2023 09:17:23 +0900 Subject: [PATCH 20/24] Add: `always_sign_unsafe` --- app/lib/activitypub/activity/create.rb | 2 +- app/services/concerns/payloadable.rb | 3 ++- app/services/remove_status_service.rb | 2 +- app/workers/activitypub/distribution_worker.rb | 6 +++--- app/workers/activitypub/forward_conversation_worker.rb | 2 +- .../activitypub/status_update_distribution_worker.rb | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index d9cd9b727206d7..57212acb407cd6 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -519,7 +519,7 @@ def process_conversation! return if @json['signature'].blank? - ActivityPub::ForwardConversationWorker.perform_async(Oj.dump(@json), @status.id) + ActivityPub::ForwardConversationWorker.perform_async(Oj.dump(@json), @status.id, false) end def increment_voters_count! diff --git a/app/services/concerns/payloadable.rb b/app/services/concerns/payloadable.rb index b0bab9a081ecb0..42896ea26cb493 100644 --- a/app/services/concerns/payloadable.rb +++ b/app/services/concerns/payloadable.rb @@ -14,11 +14,12 @@ def serialize_payload(record, serializer, options = {}) signer = options.delete(:signer) sign_with = options.delete(:sign_with) always_sign = options.delete(:always_sign) + always_sign_unsafe = options.delete(:always_sign_unsafe) payload = ActiveModelSerializers::SerializableResource.new(record, options.merge(serializer: serializer, adapter: ActivityPub::Adapter)).as_json object = record.respond_to?(:virtual_object) ? record.virtual_object : record bearcap = object.is_a?(String) && record.respond_to?(:type) && (record.type == 'Create' || record.type == 'Update') - if ((object.respond_to?(:sign?) && object.sign?) && signer && (always_sign || signing_enabled?)) || bearcap + if ((object.respond_to?(:sign?) && object.sign?) && signer && (always_sign || signing_enabled?)) || bearcap || always_sign_unsafe ActivityPub::LinkedDataSignature.new(payload).sign!(signer, sign_with: sign_with) else payload diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 7bdf5a84017284..448ae614f5ec68 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -122,7 +122,7 @@ def remove_from_conversation end def signed_activity_json - @signed_activity_json ||= Oj.dump(serialize_payload(@status, @status.reblog? ? ActivityPub::UndoAnnounceSerializer : ActivityPub::DeleteSerializer, signer: @account, always_sign: true)) + @signed_activity_json ||= Oj.dump(serialize_payload(@status, @status.reblog? ? ActivityPub::UndoAnnounceSerializer : ActivityPub::DeleteSerializer, signer: @account, always_sign_unsafe: @status.limited_visibility?)) end def remove_reblogs diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 2e99396c25d680..260334760edb82 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -60,15 +60,15 @@ def status_reach_finder end def payload - @payload ||= Oj.dump(serialize_payload(activity, ActivityPub::ActivitySerializer, signer: @account, always_sign: always_sign)) + @payload ||= Oj.dump(serialize_payload(activity, ActivityPub::ActivitySerializer, signer: @account, always_sign_unsafe: always_sign)) end def payload_for_misskey - @payload_for_misskey ||= Oj.dump(serialize_payload(activity_for_misskey, ActivityPub::ActivityForMisskeySerializer, signer: @account, always_sign: always_sign)) + @payload_for_misskey ||= Oj.dump(serialize_payload(activity_for_misskey, ActivityPub::ActivityForMisskeySerializer, signer: @account)) end def payload_for_friend - @payload_for_friend ||= Oj.dump(serialize_payload(activity_for_friend, ActivityPub::ActivityForFriendSerializer, signer: @account, always_sign: always_sign)) + @payload_for_friend ||= Oj.dump(serialize_payload(activity_for_friend, ActivityPub::ActivityForFriendSerializer, signer: @account, always_sign_unsafe: always_sign)) end def activity diff --git a/app/workers/activitypub/forward_conversation_worker.rb b/app/workers/activitypub/forward_conversation_worker.rb index 770e94c3c39251..8e0bd59767fb69 100644 --- a/app/workers/activitypub/forward_conversation_worker.rb +++ b/app/workers/activitypub/forward_conversation_worker.rb @@ -3,7 +3,7 @@ class ActivityPub::ForwardConversationWorker include Sidekiq::Worker - def perform(payload, status_id, shared_inbox = false) # rubocop:disable Style/OptionalBooleanParameter + def perform(payload, status_id, shared_inbox) @status = Status.find(status_id) @payload = payload @shared_inbox = shared_inbox diff --git a/app/workers/activitypub/status_update_distribution_worker.rb b/app/workers/activitypub/status_update_distribution_worker.rb index 26460905737c2b..2c7922e4194082 100644 --- a/app/workers/activitypub/status_update_distribution_worker.rb +++ b/app/workers/activitypub/status_update_distribution_worker.rb @@ -48,6 +48,6 @@ def activity_for_friend end def always_sign - true + @status.limited_visibility? end end From f70673faf0afd6d3beb8d90d0026245787de8ae5 Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 29 Nov 2023 09:21:33 +0900 Subject: [PATCH 21/24] Add: Subject --- app/services/concerns/payloadable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/concerns/payloadable.rb b/app/services/concerns/payloadable.rb index 42896ea26cb493..113b389fac2e53 100644 --- a/app/services/concerns/payloadable.rb +++ b/app/services/concerns/payloadable.rb @@ -19,7 +19,7 @@ def serialize_payload(record, serializer, options = {}) object = record.respond_to?(:virtual_object) ? record.virtual_object : record bearcap = object.is_a?(String) && record.respond_to?(:type) && (record.type == 'Create' || record.type == 'Update') - if ((object.respond_to?(:sign?) && object.sign?) && signer && (always_sign || signing_enabled?)) || bearcap || always_sign_unsafe + if ((object.respond_to?(:sign?) && object.sign?) && signer && (always_sign || signing_enabled?)) || bearcap || (signer && always_sign_unsafe) ActivityPub::LinkedDataSignature.new(payload).sign!(signer, sign_with: sign_with) else payload From d2d8e2f26a1c8d234fc4de5c340f2fc6d8b8d7be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Wed, 29 Nov 2023 11:24:55 +0900 Subject: [PATCH 22/24] Remove space --- spec/workers/activitypub/distribution_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/workers/activitypub/distribution_worker_spec.rb b/spec/workers/activitypub/distribution_worker_spec.rb index abf734b730c0aa..523cab85bfff20 100644 --- a/spec/workers/activitypub/distribution_worker_spec.rb +++ b/spec/workers/activitypub/distribution_worker_spec.rb @@ -78,7 +78,7 @@ expect(ActivityPub::DeliveryWorker).to have_received(:perform_async) end end - + context 'with limited status for no-follower but non-mentioned follower' do let(:no_follower) { Fabricate(:account, domain: 'example.com', inbox_url: 'http://example.com/no_follower/inbox', shared_inbox_url: 'http://example.com') } From 300ab3f8a404c65fd09a54d2dce7a03d1474ae21 Mon Sep 17 00:00:00 2001 From: KMY Date: Wed, 29 Nov 2023 14:16:06 +0900 Subject: [PATCH 23/24] =?UTF-8?q?Fix:=20=E4=BB=96=E4=BA=BA=E3=81=AE?= =?UTF-8?q?=E3=82=B9=E3=83=AC=E3=83=83=E3=83=89=E3=81=AE=E9=80=81=E4=BF=A1?= =?UTF-8?q?=E5=85=88=E4=B8=80=E8=A6=A7=E3=82=92=E9=9D=9E=E8=A1=A8=E7=A4=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mastodon/components/status_action_bar.jsx | 2 +- .../features/status/components/action_bar.jsx | 5 +- app/policies/status_policy.rb | 7 ++- spec/policies/status_policy_spec.rb | 52 +++++++++++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/app/javascript/mastodon/components/status_action_bar.jsx b/app/javascript/mastodon/components/status_action_bar.jsx index 583c5fdedd0750..6e355de6e97661 100644 --- a/app/javascript/mastodon/components/status_action_bar.jsx +++ b/app/javascript/mastodon/components/status_action_bar.jsx @@ -336,7 +336,7 @@ class StatusActionBar extends ImmutablePureComponent { } if (signedIn) { - if (writtenByMe) { + if (writtenByMe && status.get('limited_scope') !== 'reply') { menu.push({ text: intl.formatMessage(messages.mentions), action: this.handleOpenMentions }); } diff --git a/app/javascript/mastodon/features/status/components/action_bar.jsx b/app/javascript/mastodon/features/status/components/action_bar.jsx index ac7d6a82010b2a..aefb889c411045 100644 --- a/app/javascript/mastodon/features/status/components/action_bar.jsx +++ b/app/javascript/mastodon/features/status/components/action_bar.jsx @@ -287,7 +287,10 @@ class ActionBar extends PureComponent { menu.push(null); } - menu.push({ text: intl.formatMessage(messages.mentions), action: this.handleOpenMentions }); + if (status.get('limited_scope') !== 'reply') { + menu.push({ text: intl.formatMessage(messages.mentions), action: this.handleOpenMentions }); + } + menu.push({ text: intl.formatMessage(mutingConversation ? messages.unmuteConversation : messages.muteConversation), action: this.handleConversationMuteClick }); menu.push(null); menu.push({ text: intl.formatMessage(messages.edit), action: this.handleEditClick }); diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index 41d242e0888b70..e57c68546471e2 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -25,7 +25,7 @@ def show? end def show_mentioned_users? - owned? + record.limited_visibility? ? owned_conversation? : owned? end def reblog? @@ -64,6 +64,11 @@ def owned? author.id == current_account&.id end + def owned_conversation? + record.conversation&.local? && + (record.conversation.ancestor_status.nil? ? owned? : record.conversation.ancestor_status.account_id == current_account&.id) + end + def private? record.private_visibility? end diff --git a/spec/policies/status_policy_spec.rb b/spec/policies/status_policy_spec.rb index 3bdc2084d8a7be..6bfc02c58289a1 100644 --- a/spec/policies/status_policy_spec.rb +++ b/spec/policies/status_policy_spec.rb @@ -115,6 +115,58 @@ end end + context 'with the permission of show_mentioned_users?' do + permissions :show_mentioned_users? do + it 'grants access when public and account is viewer' do + status.visibility = :public + + expect(subject).to permit(status.account, status) + end + + it 'grants access when public and account is not viewer' do + status.visibility = :public + + expect(subject).to_not permit(bob, status) + end + + it 'grants access when limited and no conversation ancestor_status and account is viewer' do + status.visibility = :limited + status.conversation = Fabricate(:conversation) + + expect(subject).to permit(status.account, status) + end + + it 'grants access when limited and my conversation and account is viewer' do + status.visibility = :limited + status.conversation = Fabricate(:conversation, ancestor_status: status) + + expect(subject).to permit(status.account, status) + end + + it 'grants access when limited and another conversation and account is viewer' do + status.visibility = :limited + status.conversation = Fabricate(:conversation, ancestor_status: Fabricate(:status, account: bob)) + + expect(subject).to_not permit(status.account, status) + end + + it 'grants access when limited and viewer is mentioned' do + status.visibility = :limited + status.mentions = [Fabricate(:mention, account: bob)] + + expect(subject).to_not permit(bob, status) + end + + it 'grants access when limited and non-owner viewer is mentioned and mentions are loaded' do + status.visibility = :limited + status.mentions = [Fabricate(:mention, account: bob)] + status.mentions.load + + expect(subject).to_not permit(bob, status) + end + end + end + context 'with the permission of reblog?' do permissions :reblog? do it 'denies access when private' do From 91988aa02e638026250c367e34a4bf0d9031b0c2 Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 30 Nov 2023 08:50:07 +0900 Subject: [PATCH 24/24] =?UTF-8?q?Fix:=20=E3=81=8A=E3=81=8B=E3=81=97?= =?UTF-8?q?=E3=81=84=E3=82=B3=E3=83=BC=E3=83=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/javascript/mastodon/reducers/compose.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/javascript/mastodon/reducers/compose.js b/app/javascript/mastodon/reducers/compose.js index d8704088381f47..b4707e3321a42f 100644 --- a/app/javascript/mastodon/reducers/compose.js +++ b/app/javascript/mastodon/reducers/compose.js @@ -299,10 +299,6 @@ const insertReference = (state, url, attributeType) => { }; const privacyPreference = (a, b) => { - if (a === 'limited') { - return 'reply'; - } - const order = ['public', 'public_unlisted', 'unlisted', 'login', 'private', 'direct']; return order[Math.max(order.indexOf(a), order.indexOf(b), 0)]; }; @@ -422,7 +418,11 @@ export default function compose(state = initialState, action) { map.set('in_reply_to', action.status.get('id')); map.set('text', statusToTextMentions(state, action.status)); map.set('reply_to_limited', action.status.get('visibility_ex') === 'limited'); - map.set('privacy', privacyPreference(action.status.get('visibility_ex'), state.get('default_privacy'))); + if (action.status.get('visibility_ex') === 'limited') { + map.set('privacy', 'reply'); + } else { + map.set('privacy', privacyPreference(action.status.get('visibility_ex'), state.get('default_privacy'))); + } map.set('limited_scope', null); map.set('searchability', privacyPreference(action.status.get('searchability'), state.get('default_searchability'))); map.set('focusDate', new Date());