From f57167c61a1a35c7035337472bd075bbf8071d43 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Thu, 12 Feb 2026 16:31:45 +0100 Subject: [PATCH] Add ID/URI to collection items (#37842) --- .../collection_items_controller.rb | 42 ++++++ app/lib/activitypub/tag_manager.rb | 2 + app/models/collection_item.rb | 6 + app/models/concerns/account/associations.rb | 1 + .../activitypub/featured_item_serializer.rb | 6 +- .../remove_featured_item_serializer.rb | 6 +- config/routes.rb | 1 + ...60212113020_add_uri_to_collection_items.rb | 7 + db/schema.rb | 3 +- .../fabricators/collection_item_fabricator.rb | 1 + spec/models/collection_item_spec.rb | 2 + spec/requests/collection_items_spec.rb | 127 ++++++++++++++++++ .../featured_collection_serializer_spec.rb | 2 + .../featured_item_serializer_spec.rb | 18 +++ .../remove_featured_item_serializer_spec.rb | 4 +- 15 files changed, 222 insertions(+), 6 deletions(-) create mode 100644 app/controllers/collection_items_controller.rb create mode 100644 db/migrate/20260212113020_add_uri_to_collection_items.rb create mode 100644 spec/requests/collection_items_spec.rb create mode 100644 spec/serializers/activitypub/featured_item_serializer_spec.rb diff --git a/app/controllers/collection_items_controller.rb b/app/controllers/collection_items_controller.rb new file mode 100644 index 0000000000..09c1e0e192 --- /dev/null +++ b/app/controllers/collection_items_controller.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +class CollectionItemsController < ApplicationController + include SignatureAuthentication + include Authorization + include AccountOwnedConcern + + vary_by -> { public_fetch_mode? ? 'Accept, Accept-Language, Cookie' : 'Accept, Accept-Language, Cookie, Signature' } + + before_action :check_feature_enabled + before_action :require_account_signature!, if: -> { authorized_fetch_mode? } + before_action :set_collection_item + + skip_around_action :set_locale + skip_before_action :require_functional!, unless: :limited_federation_mode? + + def show + respond_to do |format| + format.json do + expires_in(3.minutes, public: public_fetch_mode?) + + render json: @collection_item, + serializer: ActivityPub::FeaturedItemSerializer, + adapter: ActivityPub::Adapter, + content_type: 'application/activity+json' + end + end + end + + private + + def set_collection_item + @collection_item = @account.curated_collection_items.find(params[:id]) + authorize @collection_item.collection, :show? + rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError + not_found + end + + def check_feature_enabled + raise ActionController::RoutingError unless Mastodon::Feature.collections_enabled? + end +end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index f9cb90f548..be9003082d 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -64,6 +64,8 @@ class ActivityPub::TagManager target.uri when :featured_collection ap_account_collection_url(target.account.id, target) + when :featured_item + ap_account_collection_item_url(target.collection.account_id, target) end end diff --git a/app/models/collection_item.rb b/app/models/collection_item.rb index 5c624165e6..d05d12df60 100644 --- a/app/models/collection_item.rb +++ b/app/models/collection_item.rb @@ -11,6 +11,7 @@ # object_uri :string # position :integer default(1), not null # state :integer default("pending"), not null +# uri :string # created_at :datetime not null # updated_at :datetime not null # account_id :bigint(8) @@ -31,6 +32,7 @@ class CollectionItem < ApplicationRecord validates :approval_uri, absence: true, unless: :local? validates :account, presence: true, if: :accepted? validates :object_uri, presence: true, if: -> { account.nil? } + validates :uri, presence: true, if: :remote? before_validation :set_position, on: :create @@ -42,6 +44,10 @@ class CollectionItem < ApplicationRecord local? && account&.remote? end + def object_type + :featured_item + end + private def set_position diff --git a/app/models/concerns/account/associations.rb b/app/models/concerns/account/associations.rb index 03ec713941..4b4dfc8879 100644 --- a/app/models/concerns/account/associations.rb +++ b/app/models/concerns/account/associations.rb @@ -17,6 +17,7 @@ module Account::Associations has_many :bookmarks has_many :collections has_many :collection_items + has_many :curated_collection_items, through: :collections, class_name: 'CollectionItem', source: :collection_items has_many :conversations, class_name: 'AccountConversation' has_many :custom_filters has_many :favourites diff --git a/app/serializers/activitypub/featured_item_serializer.rb b/app/serializers/activitypub/featured_item_serializer.rb index 87db67d95b..a524d6c25f 100644 --- a/app/serializers/activitypub/featured_item_serializer.rb +++ b/app/serializers/activitypub/featured_item_serializer.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true class ActivityPub::FeaturedItemSerializer < ActivityPub::Serializer - attributes :type, :featured_object, :featured_object_type + attributes :id, :type, :featured_object, :featured_object_type + + def id + ActivityPub::TagManager.instance.uri_for(object) + end def type 'FeaturedItem' diff --git a/app/serializers/activitypub/remove_featured_item_serializer.rb b/app/serializers/activitypub/remove_featured_item_serializer.rb index cc1783c7e7..22ea07cd30 100644 --- a/app/serializers/activitypub/remove_featured_item_serializer.rb +++ b/app/serializers/activitypub/remove_featured_item_serializer.rb @@ -4,7 +4,7 @@ class ActivityPub::RemoveFeaturedItemSerializer < ActivityPub::Serializer include RoutingHelper attributes :type, :actor, :target - has_one :object, serializer: ActivityPub::FeaturedItemSerializer + has_one :virtual_object, key: :object def type 'Remove' @@ -18,6 +18,10 @@ class ActivityPub::RemoveFeaturedItemSerializer < ActivityPub::Serializer ActivityPub::TagManager.instance.uri_for(collection) end + def virtual_object + ActivityPub::TagManager.instance.uri_for(object) + end + private def collection diff --git a/config/routes.rb b/config/routes.rb index bf50b67fe1..b516a48866 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -124,6 +124,7 @@ Rails.application.routes.draw do scope path: 'ap', as: 'ap' do resources :accounts, path: 'users', only: [:show], param: :id, concerns: :account_resources do + resources :collection_items, only: [:show] resources :featured_collections, only: [:index], module: :activitypub resources :statuses, only: [:show] do diff --git a/db/migrate/20260212113020_add_uri_to_collection_items.rb b/db/migrate/20260212113020_add_uri_to_collection_items.rb new file mode 100644 index 0000000000..eea574a4e3 --- /dev/null +++ b/db/migrate/20260212113020_add_uri_to_collection_items.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddUriToCollectionItems < ActiveRecord::Migration[8.0] + def change + add_column :collection_items, :uri, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index a3ee1f23b0..9311dbac5c 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: 2026_02_11_132603) do +ActiveRecord::Schema[8.0].define(version: 2026_02_12_113020) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -365,6 +365,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_02_11_132603) do t.integer "state", default: 0, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "uri" t.index ["account_id"], name: "index_collection_items_on_account_id" t.index ["approval_uri"], name: "index_collection_items_on_approval_uri", unique: true, where: "(approval_uri IS NOT NULL)" t.index ["collection_id"], name: "index_collection_items_on_collection_id" diff --git a/spec/fabricators/collection_item_fabricator.rb b/spec/fabricators/collection_item_fabricator.rb index db55f7d237..7c4bcb3d85 100644 --- a/spec/fabricators/collection_item_fabricator.rb +++ b/spec/fabricators/collection_item_fabricator.rb @@ -12,4 +12,5 @@ Fabricator(:unverified_remote_collection_item, from: :collection_item) do state :pending object_uri { Fabricate.build(:remote_account).uri } approval_uri { sequence(:uri) { |i| "https://example.com/authorizations/#{i}" } } + uri { sequence(:uri) { |i| "https://example.com/collection_items/#{i}" } } end diff --git a/spec/models/collection_item_spec.rb b/spec/models/collection_item_spec.rb index 3592c75e66..f5497d9041 100644 --- a/spec/models/collection_item_spec.rb +++ b/spec/models/collection_item_spec.rb @@ -30,6 +30,8 @@ RSpec.describe CollectionItem do let(:remote_collection) { Fabricate.build(:collection, local: false) } it { is_expected.to validate_absence_of(:approval_uri) } + + it { is_expected.to validate_presence_of(:uri) } end context 'when account is not present' do diff --git a/spec/requests/collection_items_spec.rb b/spec/requests/collection_items_spec.rb new file mode 100644 index 0000000000..f0802fc3f3 --- /dev/null +++ b/spec/requests/collection_items_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'CollectionItems' do + describe 'GET /ap/users/@:account_id/collection_items/:id', feature: :collections do + subject { get ap_account_collection_item_path(account.id, collection_item, format: :json) } + + let(:collection_item) { Fabricate(:collection_item) } + let(:collection) { collection_item.collection } + let(:account) { collection.account } + + context 'when signed out' do + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + subject + + expect(response) + .to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before { account.suspend! } + + it 'returns http forbidden' do + subject + + expect(response) + .to have_http_status(403) + end + end + + context 'when account is accessible' do + it 'renders ActivityPub representation successfully', :aggregate_failures do + subject + + expect(response) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + + expect(response.headers).to include( + 'Content-Type' => include('application/activity+json') + ) + expect(response.parsed_body) + .to include({ + 'type' => 'FeaturedItem', + }) + end + end + end + + context 'when signed in' do + let(:user) { Fabricate(:user) } + + before do + post user_session_path, params: { user: { email: user.email, password: user.password } } + end + + context 'when account blocks user' do + before { account.block!(user.account) } + + it 'returns http not found' do + subject + + expect(response) + .to have_http_status(404) + end + end + end + + context 'with "HTTP Signature" access signed by a remote account' do + subject do + get ap_account_collection_item_path(account.id, collection_item, format: :json), + headers: nil, + sign_with: remote_account + end + + let(:remote_account) { Fabricate(:account, domain: 'host.example') } + + context 'when account blocks the remote account' do + before { account.block!(remote_account) } + + it 'returns http not found' do + subject + + expect(response) + .to have_http_status(404) + end + end + + context 'when account domain blocks the domain of the remote account' do + before { account.block_domain!(remote_account.domain) } + + it 'returns http not found' do + subject + + expect(response) + .to have_http_status(404) + end + end + + context 'with JSON' do + it 'renders ActivityPub FeaturedItem object successfully', :aggregate_failures do + subject + + expect(response) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + + expect(response.headers).to include( + 'Content-Type' => include('application/activity+json') + ) + expect(response.parsed_body) + .to include({ + 'type' => 'FeaturedItem', + }) + end + end + end + end +end diff --git a/spec/serializers/activitypub/featured_collection_serializer_spec.rb b/spec/serializers/activitypub/featured_collection_serializer_spec.rb index e6bb4ea4b0..24dd065480 100644 --- a/spec/serializers/activitypub/featured_collection_serializer_spec.rb +++ b/spec/serializers/activitypub/featured_collection_serializer_spec.rb @@ -31,11 +31,13 @@ RSpec.describe ActivityPub::FeaturedCollectionSerializer do 'totalItems' => 2, 'orderedItems' => [ { + 'id' => ActivityPub::TagManager.instance.uri_for(collection_items.first), 'type' => 'FeaturedItem', 'featuredObject' => ActivityPub::TagManager.instance.uri_for(collection_items.first.account), 'featuredObjectType' => 'Person', }, { + 'id' => ActivityPub::TagManager.instance.uri_for(collection_items.last), 'type' => 'FeaturedItem', 'featuredObject' => ActivityPub::TagManager.instance.uri_for(collection_items.last.account), 'featuredObjectType' => 'Person', diff --git a/spec/serializers/activitypub/featured_item_serializer_spec.rb b/spec/serializers/activitypub/featured_item_serializer_spec.rb new file mode 100644 index 0000000000..f17faf2410 --- /dev/null +++ b/spec/serializers/activitypub/featured_item_serializer_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ActivityPub::FeaturedItemSerializer do + subject { serialized_record_json(collection_item, described_class, adapter: ActivityPub::Adapter) } + + let(:collection_item) { Fabricate(:collection_item) } + + it 'serializes to the expected structure' do + expect(subject).to include({ + 'type' => 'FeaturedItem', + 'id' => ActivityPub::TagManager.instance.uri_for(collection_item), + 'featuredObject' => ActivityPub::TagManager.instance.uri_for(collection_item.account), + 'featuredObjectType' => 'Person', + }) + end +end diff --git a/spec/serializers/activitypub/remove_featured_item_serializer_spec.rb b/spec/serializers/activitypub/remove_featured_item_serializer_spec.rb index bce81c2552..6ffe77fec9 100644 --- a/spec/serializers/activitypub/remove_featured_item_serializer_spec.rb +++ b/spec/serializers/activitypub/remove_featured_item_serializer_spec.rb @@ -14,9 +14,7 @@ RSpec.describe ActivityPub::RemoveFeaturedItemSerializer do 'type' => 'Remove', 'actor' => tag_manager.uri_for(collection.account), 'target' => tag_manager.uri_for(collection), - 'object' => a_hash_including({ - 'type' => 'FeaturedItem', - }), + 'object' => tag_manager.uri_for(object), }) expect(subject).to_not have_key('id')