From bdebc4dc4666adf187d14c6dffb40d867c84d139 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 8 Sep 2025 18:11:51 +0200 Subject: [PATCH] Revert "Implement FEP 7888: Part 1 - publish conversation context (#35959)" This reverts commit 65b4a0a6f1c19adde71df567dd7a9f0f6af5dc14. --- .../activitypub/contexts_controller.rb | 81 ----------- app/lib/activitypub/tag_manager.rb | 8 -- app/models/conversation.rb | 27 +--- app/models/status.rb | 5 +- .../activitypub/context_presenter.rb | 14 -- .../activitypub/context_serializer.rb | 11 -- .../activitypub/note_serializer.rb | 8 +- config/routes.rb | 5 - ...tus_and_parent_account_to_conversations.rb | 8 -- ...0_add_index_on_conversation_to_statuses.rb | 9 -- db/schema.rb | 5 +- spec/fabricators/conversation_fabricator.rb | 4 +- spec/models/account_conversation_spec.rb | 8 +- spec/requests/activitypub/contexts_spec.rb | 127 ------------------ .../activitypub/note_serializer_spec.rb | 1 - 15 files changed, 12 insertions(+), 309 deletions(-) delete mode 100644 app/controllers/activitypub/contexts_controller.rb delete mode 100644 app/presenters/activitypub/context_presenter.rb delete mode 100644 app/serializers/activitypub/context_serializer.rb delete mode 100644 db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb delete mode 100644 db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb delete mode 100644 spec/requests/activitypub/contexts_spec.rb diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb deleted file mode 100644 index 2a7e69cff9b..00000000000 --- a/app/controllers/activitypub/contexts_controller.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -class ActivityPub::ContextsController < ActivityPub::BaseController - vary_by -> { 'Signature' if authorized_fetch_mode? } - - before_action :require_account_signature!, if: :authorized_fetch_mode? - before_action :set_conversation - before_action :set_items - - DESCENDANTS_LIMIT = 60 - - def show - expires_in 3.minutes, public: public_fetch_mode? - render_with_cache json: context_presenter, serializer: ActivityPub::ContextSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' - end - - def items - expires_in 3.minutes, public: public_fetch_mode? - render_with_cache json: items_collection_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' - end - - private - - def account_required? - false - end - - def set_conversation - @conversation = Conversation.local.find(params[:id]) - end - - def set_items - @items = @conversation.statuses.distributable_visibility.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id]) - end - - def context_presenter - first_page = ActivityPub::CollectionPresenter.new( - id: items_context_url(@conversation, page_params), - type: :unordered, - part_of: items_context_url(@conversation), - next: next_page, - items: @items.map { |status| status.local? ? ActivityPub::TagManager.instance.uri_for(status) : status.uri } - ) - - ActivityPub::ContextPresenter.from_conversation(@conversation).tap do |presenter| - presenter.first = first_page - end - end - - def items_collection_presenter - page = ActivityPub::CollectionPresenter.new( - id: items_context_url(@conversation, page_params), - type: :unordered, - part_of: items_context_url(@conversation), - next: next_page, - items: @items.map { |status| status.local? ? ActivityPub::TagManager.instance.uri_for(status) : status.uri } - ) - - return page if page_requested? - - ActivityPub::CollectionPresenter.new( - id: items_context_url(@conversation), - type: :unordered, - first: page - ) - end - - def page_requested? - truthy_param?(:page) - end - - def next_page - return nil if @items.size < DESCENDANTS_LIMIT - - items_context_url(@conversation, page: true, min_id: @items.last.id) - end - - def page_params - params.permit(:page, :min_id) - end -end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 9f72a150d35..975763e82fe 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -40,8 +40,6 @@ class ActivityPub::TagManager case target.object_type when :person target.instance_actor? ? instance_actor_url : account_url(target) - when :conversation - context_url(target) when :note, :comment, :activity return activity_account_status_url(target.account, target) if target.reblog? @@ -78,12 +76,6 @@ class ActivityPub::TagManager activity_account_status_url(target.account, target) end - def context_uri_for(target, page_params = nil) - raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local? - - items_context_url(target.conversation, page_params) - end - def replies_uri_for(target, page_params = nil) raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local? diff --git a/app/models/conversation.rb b/app/models/conversation.rb index c86b90ad7dc..a7fe1488407 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -4,12 +4,10 @@ # # Table name: conversations # -# id :bigint(8) not null, primary key -# uri :string -# created_at :datetime not null -# updated_at :datetime not null -# parent_account_id :bigint(8) -# parent_status_id :bigint(8) +# id :bigint(8) not null, primary key +# uri :string +# created_at :datetime not null +# updated_at :datetime not null # class Conversation < ApplicationRecord @@ -17,24 +15,7 @@ class Conversation < ApplicationRecord has_many :statuses, dependent: nil - belongs_to :parent_status, class_name: 'Status', optional: true, inverse_of: :conversation - belongs_to :parent_account, class_name: 'Account', optional: true - - scope :local, -> { where(uri: nil) } - - before_validation :set_parent_account, on: :create - def local? uri.nil? end - - def object_type - :conversation - end - - private - - def set_parent_account - self.parent_account = parent_status.account if parent_status.present? - end end diff --git a/app/models/status.rb b/app/models/status.rb index f72a92078b3..e933c92cae0 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -70,8 +70,6 @@ class Status < ApplicationRecord belongs_to :reblog, foreign_key: 'reblog_of_id', inverse_of: :reblogs end - has_one :owned_conversation, class_name: 'Conversation', foreign_key: 'parent_status_id', inverse_of: :parent_status, dependent: nil - has_many :favourites, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy @@ -444,8 +442,7 @@ class Status < ApplicationRecord 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? - conversation = build_owned_conversation - self.conversation = conversation + self.conversation = Conversation.new end end diff --git a/app/presenters/activitypub/context_presenter.rb b/app/presenters/activitypub/context_presenter.rb deleted file mode 100644 index c13900ffdab..00000000000 --- a/app/presenters/activitypub/context_presenter.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -class ActivityPub::ContextPresenter < ActiveModelSerializers::Model - attributes :id, :type, :attributed_to, :first, :object_type - - class << self - def from_conversation(conversation) - new.tap do |presenter| - presenter.id = ActivityPub::TagManager.instance.uri_for(conversation) - presenter.attributed_to = ActivityPub::TagManager.instance.uri_for(conversation.parent_account) - end - end - end -end diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb deleted file mode 100644 index 33c70d8e9de..00000000000 --- a/app/serializers/activitypub/context_serializer.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -class ActivityPub::ContextSerializer < ActivityPub::Serializer - include RoutingHelper - - attributes :id, :type, :attributed_to, :first - - def type - 'Collection' - end -end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 20f23c0e8a1..8c44baf95f6 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -9,7 +9,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer :in_reply_to, :published, :url, :attributed_to, :to, :cc, :sensitive, :atom_uri, :in_reply_to_atom_uri, - :conversation, :context + :conversation attribute :content attribute :content_map, if: :language? @@ -163,12 +163,6 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer end end - def context - return if object.conversation.nil? - - ActivityPub::TagManager.instance.uri_for(object.conversation) - end - def local? object.account.local? end diff --git a/config/routes.rb b/config/routes.rb index 26c5b3151f3..49fcf3de792 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -120,11 +120,6 @@ Rails.application.routes.draw do end resource :inbox, only: [:create], module: :activitypub - resources :contexts, only: [:show], module: :activitypub do - member do - get :items - end - end constraints(encoded_path: /%40.*/) do get '/:encoded_path', to: redirect { |params| diff --git a/db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb b/db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb deleted file mode 100644 index e9e6a84b725..00000000000 --- a/db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -class AddParentStatusAndParentAccountToConversations < ActiveRecord::Migration[8.0] - def change - add_column :conversations, :parent_status_id, :bigint, null: true, default: nil - add_column :conversations, :parent_account_id, :bigint, null: true, default: nil - end -end diff --git a/db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb b/db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb deleted file mode 100644 index 9be338e8f67..00000000000 --- a/db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AddIndexOnConversationToStatuses < ActiveRecord::Migration[8.0] - disable_ddl_transaction! - - def change - add_index :statuses, :conversation_id, algorithm: :concurrently - end -end diff --git a/db/schema.rb b/db/schema.rb index 124eb0f3580..81b0fe7ccc1 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[8.0].define(version: 2025_09_02_221600) do +ActiveRecord::Schema[8.0].define(version: 2025_08_20_084312) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -359,8 +359,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_02_221600) do t.string "uri" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.bigint "parent_status_id" - t.bigint "parent_account_id" t.index ["uri"], name: "index_conversations_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)" end @@ -1147,7 +1145,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_02_221600) do t.integer "quote_approval_policy", default: 0, null: false t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20190820", order: { id: :desc }, where: "(deleted_at IS NULL)" t.index ["account_id"], name: "index_statuses_on_account_id" - t.index ["conversation_id"], name: "index_statuses_on_conversation_id" t.index ["deleted_at"], name: "index_statuses_on_deleted_at", where: "(deleted_at IS NOT NULL)" t.index ["id", "account_id"], name: "index_statuses_local_20190824", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" t.index ["id", "language", "account_id"], name: "index_statuses_public_20250129", order: { id: :desc }, where: "((deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" diff --git a/spec/fabricators/conversation_fabricator.rb b/spec/fabricators/conversation_fabricator.rb index 2d3f98c037b..5440e4380c7 100644 --- a/spec/fabricators/conversation_fabricator.rb +++ b/spec/fabricators/conversation_fabricator.rb @@ -1,5 +1,3 @@ # frozen_string_literal: true -Fabricator(:conversation) do - parent_account { Fabricate(:account) } -end +Fabricator(:conversation) diff --git a/spec/models/account_conversation_spec.rb b/spec/models/account_conversation_spec.rb index 4ccbed05165..4e8727ca395 100644 --- a/spec/models/account_conversation_spec.rb +++ b/spec/models/account_conversation_spec.rb @@ -21,7 +21,7 @@ RSpec.describe AccountConversation do it 'appends to old record when there is a match' do last_status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [last_status.id]) + conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [last_status.id]) status = Fabricate(:status, account: bob, visibility: :direct, thread: last_status) status.mentions.create(account: alice) @@ -36,7 +36,7 @@ RSpec.describe AccountConversation do it 'creates new record when new participants are added' do last_status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [last_status.id]) + conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [last_status.id]) status = Fabricate(:status, account: bob, visibility: :direct, thread: last_status) status.mentions.create(account: alice) @@ -55,7 +55,7 @@ RSpec.describe AccountConversation do it 'updates last status to a previous value' do last_status = Fabricate(:status, account: alice, visibility: :direct) status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [status.id, last_status.id]) + conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [status.id, last_status.id]) last_status.mentions.create(account: bob) last_status.destroy! conversation.reload @@ -65,7 +65,7 @@ RSpec.describe AccountConversation do it 'removes the record if no other statuses are referenced' do last_status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [last_status.id]) + conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [last_status.id]) last_status.mentions.create(account: bob) last_status.destroy! expect(described_class.where(id: conversation.id).count).to eq 0 diff --git a/spec/requests/activitypub/contexts_spec.rb b/spec/requests/activitypub/contexts_spec.rb deleted file mode 100644 index 22ecea04ebd..00000000000 --- a/spec/requests/activitypub/contexts_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'ActivityPub Contexts' do - let(:conversation) { Fabricate(:conversation) } - - describe 'GET #show' do - subject { get context_path(id: conversation.id), headers: nil } - - let!(:status) { Fabricate(:status, conversation: conversation) } - let!(:unrelated_status) { Fabricate(:status) } - - it 'returns http success and correct media type and correct items' do - subject - - expect(response) - .to have_http_status(200) - .and have_cacheable_headers - - expect(response.media_type) - .to eq 'application/activity+json' - - expect(response.parsed_body[:type]) - .to eq 'Collection' - - expect(response.parsed_body[:first][:items]) - .to be_an(Array) - .and have_attributes(size: 1) - .and include(ActivityPub::TagManager.instance.uri_for(status)) - .and not_include(ActivityPub::TagManager.instance.uri_for(unrelated_status)) - end - - context 'with pagination' do - context 'with few statuses' do - before do - 3.times do - Fabricate(:status, conversation: conversation) - end - end - - it 'does not include a next page link' do - subject - - expect(response.parsed_body[:first][:next]).to be_nil - end - end - - context 'with many statuses' do - before do - (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do - Fabricate(:status, conversation: conversation) - end - end - - it 'includes a next page link' do - subject - - expect(response.parsed_body['first']['next']).to_not be_nil - end - end - end - end - - describe 'GET #items' do - subject { get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil } - - context 'with few statuses' do - before do - 3.times do - Fabricate(:status, conversation: conversation) - end - end - - it 'returns http success and correct media type and correct items' do - subject - - expect(response) - .to have_http_status(200) - - expect(response.media_type) - .to eq 'application/activity+json' - - expect(response.parsed_body[:type]) - .to eq 'Collection' - - expect(response.parsed_body[:first][:items]) - .to be_an(Array) - .and have_attributes(size: 3) - - expect(response.parsed_body[:first][:next]).to be_nil - end - end - - context 'with many statuses' do - before do - (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do - Fabricate(:status, conversation: conversation) - end - end - - it 'includes a next page link' do - subject - - expect(response.parsed_body['first']['next']).to_not be_nil - end - end - - context 'with page requested' do - before do - (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do |_i| - Fabricate(:status, conversation: conversation) - end - end - - it 'returns the correct items' do - get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil - next_page = response.parsed_body['first']['next'] - get next_page, headers: nil - - expect(response.parsed_body['items']) - .to be_an(Array) - .and have_attributes(size: 1) - end - end - end -end diff --git a/spec/serializers/activitypub/note_serializer_spec.rb b/spec/serializers/activitypub/note_serializer_spec.rb index 336f394337e..d5d02a0d495 100644 --- a/spec/serializers/activitypub/note_serializer_spec.rb +++ b/spec/serializers/activitypub/note_serializer_spec.rb @@ -23,7 +23,6 @@ RSpec.describe ActivityPub::NoteSerializer do 'zh-TW' => a_kind_of(String), }), 'replies' => replies_collection_values, - 'context' => ActivityPub::TagManager.instance.uri_for(parent.conversation), }) end