From e18ca373ebaac162f379346c2fbbfe4d4733cd1a Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Thu, 21 May 2026 15:45:09 +0200 Subject: [PATCH] Revert "Add partial accounts to collections endpoint (#38919)" (#39128) --- .../api/v1_alpha/collections_controller.rb | 6 +-- app/models/collection.rb | 1 - app/models/collection_item.rb | 9 ---- app/presenters/collections_presenter.rb | 11 ----- ...ctions_with_account_previews_serializer.rb | 10 ---- spec/models/collection_spec.rb | 12 ----- spec/presenters/collections_presenter_spec.rb | 48 ------------------- ...s_with_account_previews_serializer_spec.rb | 45 ----------------- 8 files changed, 2 insertions(+), 140 deletions(-) delete mode 100644 app/presenters/collections_presenter.rb delete mode 100644 app/serializers/rest/collections_with_account_previews_serializer.rb delete mode 100644 spec/presenters/collections_presenter_spec.rb delete mode 100644 spec/serializers/rest/collections_with_account_previews_serializer_spec.rb diff --git a/app/controllers/api/v1_alpha/collections_controller.rb b/app/controllers/api/v1_alpha/collections_controller.rb index 4e15b65ff5..1ca1cd6923 100644 --- a/app/controllers/api/v1_alpha/collections_controller.rb +++ b/app/controllers/api/v1_alpha/collections_controller.rb @@ -28,10 +28,9 @@ class Api::V1Alpha::CollectionsController < Api::BaseController cache_if_unauthenticated! authorize @account, :index_collections? - presenter = CollectionsPresenter.new(collections: @collections) - render json: presenter, serializer: REST::CollectionsWithAccountPreviewsSerializer + render json: @collections, each_serializer: REST::CollectionSerializer, adapter: :json rescue Mastodon::NotPermittedError - render json: { collections: [], partial_accounts: [] } + render json: { collections: [] } end def show @@ -74,7 +73,6 @@ class Api::V1Alpha::CollectionsController < Api::BaseController def set_collections @collections = @account.collections .with_tag - .preload(top_items: :account) .order(created_at: :desc) .offset(offset_param) .limit(limit_param(DEFAULT_COLLECTIONS_LIMIT)) diff --git a/app/models/collection.rb b/app/models/collection.rb index be4a6ca6c2..c5082269e0 100644 --- a/app/models/collection.rb +++ b/app/models/collection.rb @@ -31,7 +31,6 @@ class Collection < ApplicationRecord has_many :collection_items, dependent: :delete_all has_many :accepted_collection_items, -> { accepted }, class_name: 'CollectionItem', inverse_of: :collection # rubocop:disable Rails/HasManyOrHasOneDependent - has_many :top_items, -> { top_items }, class_name: 'CollectionItem', inverse_of: :collection # rubocop:disable Rails/HasManyOrHasOneDependent has_many :collection_reports, dependent: :delete_all validates :name, presence: true diff --git a/app/models/collection_item.rb b/app/models/collection_item.rb index 7ecb28ddb8..8de27950f1 100644 --- a/app/models/collection_item.rb +++ b/app/models/collection_item.rb @@ -47,15 +47,6 @@ class CollectionItem < ApplicationRecord scope :local, -> { joins(:collection).merge(Collection.local) } scope :accepted_partial, ->(account) { joins(:account).merge(Account.local).accepted.where(uri: nil, account_id: account.id) } scope :pending_or_accepted, -> { where(state: [:pending, :accepted]) } - scope :top_items, lambda { |limit = 4| - subquery = where('collection_items.collection_id = collections.id') - .accepted.ordered.limit(limit) - .arel.lateral('top_items') - collection_query = Collection - .select('top_items.*') - .from([Collection.arel_table, subquery]) - from(collection_query, 'collection_items') - } def with_local_account? account&.local? diff --git a/app/presenters/collections_presenter.rb b/app/presenters/collections_presenter.rb deleted file mode 100644 index e97d872769..0000000000 --- a/app/presenters/collections_presenter.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -class CollectionsPresenter < ActiveModelSerializers::Model - attributes :collections - - def accounts - owners = collections.map(&:account) - top_accounts = collections.flat_map { |c| c.top_items.map(&:account) } - (owners + top_accounts).uniq - end -end diff --git a/app/serializers/rest/collections_with_account_previews_serializer.rb b/app/serializers/rest/collections_with_account_previews_serializer.rb deleted file mode 100644 index 95216ea649..0000000000 --- a/app/serializers/rest/collections_with_account_previews_serializer.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -class REST::CollectionsWithAccountPreviewsSerializer < ActiveModel::Serializer - has_many :collections, serializer: REST::CollectionSerializer - has_many :partial_accounts, serializer: REST::PartialAccountSerializer - - def partial_accounts - object.accounts - end -end diff --git a/spec/models/collection_spec.rb b/spec/models/collection_spec.rb index 0a04473d56..b9a5453642 100644 --- a/spec/models/collection_spec.rb +++ b/spec/models/collection_spec.rb @@ -199,16 +199,4 @@ RSpec.describe Collection do expect(subject.to_log_permalink).to eq ActivityPub::TagManager.instance.uri_for(subject) end end - - describe '#top_items' do - let(:collection) { Fabricate(:collection) } - - before do - 5.times { |i| Fabricate(:collection_item, collection:, position: i + 1) } - end - - it 'returns the topmost four items' do - expect(collection.top_items.map(&:position)).to contain_exactly(1, 2, 3, 4) - end - end end diff --git a/spec/presenters/collections_presenter_spec.rb b/spec/presenters/collections_presenter_spec.rb deleted file mode 100644 index 7207637481..0000000000 --- a/spec/presenters/collections_presenter_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe CollectionsPresenter do - subject { described_class.new(collections:) } - - let(:collection_owner_one) { Fabricate(:account) } - let(:collection_owner_two) { Fabricate(:account) } - let(:collection_one) do - Fabricate(:collection, - account: collection_owner_one, - name: 'Exquisite follows') - end - let(:collection_two) do - Fabricate(:collection, - account: collection_owner_two, - name: 'Excellent people') - end - let(:collections) { [collection_one, collection_two] } - - describe '#accounts' do - context 'when collections do not have any items' do - it 'includes only the collection owners' do - expect(subject.accounts).to contain_exactly(collection_owner_one, collection_owner_two) - end - end - - context 'when collections include accounts' do - let(:accounts) { Fabricate.times(3, :account) } - - before do - accounts[0..1].each { |a| Fabricate(:collection_item, collection: collection_one, account: a) } - accounts[1..2].each { |a| Fabricate(:collection_item, collection: collection_two, account: a) } - end - - it 'includes collection owners and unique preview accounts' do - expect(subject.accounts).to contain_exactly( - collection_owner_one, - collection_owner_two, - accounts[0], - accounts[1], - accounts[2] - ) - end - end - end -end diff --git a/spec/serializers/rest/collections_with_account_previews_serializer_spec.rb b/spec/serializers/rest/collections_with_account_previews_serializer_spec.rb deleted file mode 100644 index 3d65f6c99d..0000000000 --- a/spec/serializers/rest/collections_with_account_previews_serializer_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe REST::CollectionsWithAccountPreviewsSerializer do - subject do - serialized_record_json(presenter, described_class, options: { - scope_name: :current_user, scope: nil - }) - end - - let(:collection_owner_one) { Fabricate(:account) } - let(:collection_owner_two) { Fabricate(:account) } - let(:featured_account) { Fabricate(:account) } - let(:collection_one) do - Fabricate(:collection, - account: collection_owner_one, - name: 'Exquisite follows') - end - let(:collection_two) do - Fabricate(:collection, - account: collection_owner_two, - name: 'Excellent people') - end - let(:collections) { [collection_one, collection_two] } - let(:presenter) { CollectionsPresenter.new(collections:) } - - before do - Fabricate(:collection_item, collection: collection_one, account: featured_account) - end - - it 'includes collections and partial accounts with the expected attributes' do - expect(subject).to include({ - 'collections' => [ - a_hash_including({ 'name' => 'Exquisite follows' }), - a_hash_including({ 'name' => 'Excellent people' }), - ], - 'partial_accounts' => [ - a_hash_including({ 'id' => collection_owner_one.id.to_s }), - a_hash_including({ 'id' => collection_owner_two.id.to_s }), - a_hash_including({ 'id' => featured_account.id.to_s }), - ], - }) - end -end