Improve collection item behavior in REST API (#38732)

app/models/collection.rb

@@ -65,6 +65,7 @@ class Collection < ApplicationRecord
 
   def items_for(account = nil)
     result = collection_items.with_accounts
+    result = account == self.account ? result.pending_or_accepted : result.accepted
     result = result.not_blocked_by(account) unless account.nil?
     result
   end

app/models/collection_item.rb

@@ -43,6 +43,7 @@ class CollectionItem < ApplicationRecord
   scope :not_blocked_by, ->(account) { where.not(accounts: { id: account.blocking }) }
   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]) }
 
   def revoke!
     update!(state: :revoked)

app/serializers/rest/collection_item_serializer.rb

@@ -1,11 +1,9 @@
 # frozen_string_literal: true
 
 class REST::CollectionItemSerializer < ActiveModel::Serializer
-  delegate :accepted?, to: :object
-
   attributes :id, :state, :created_at
 
-  attribute :account_id, if: :accepted?
+  attribute :account_id, if: :accepted_or_pending?
 
   def id
     object.id.to_s
@@ -14,4 +12,8 @@ class REST::CollectionItemSerializer < ActiveModel::Serializer
   def account_id
     object.account_id.to_s
   end
+
+  def accepted_or_pending?
+    object.pending? || object.accepted?
+  end
 end

app/serializers/rest/collection_serializer.rb

@@ -29,7 +29,11 @@ class REST::CollectionSerializer < ActiveModel::Serializer
   end
 
   def items
-    object.items_for(current_user&.account)
+    @items ||= object.items_for(current_user&.account)
+  end
+
+  def item_count
+    items.size
   end
 
   def account_id

spec/models/collection_spec.rb

@@ -64,11 +64,18 @@ RSpec.describe Collection do
   describe '#item_for' do
     subject { Fabricate(:collection) }
 
-    let!(:items) { Fabricate.times(2, :collection_item, collection: subject) }
+    let!(:accepted_items) { Fabricate.times(2, :collection_item, collection: subject, state: :accepted) }
+    let!(:pending_item) { Fabricate(:collection_item, collection: subject, state: :pending) }
+
+    before do
+      %i(rejected revoked).each do |state|
+        Fabricate(:collection_item, collection: subject, state:)
+      end
+    end
 
     context 'when given no account' do
-      it 'returns all items' do
-        expect(subject.items_for).to match_array(items)
+      it 'returns all accepted items' do
+        expect(subject.items_for).to match_array(accepted_items)
       end
     end
 
@@ -76,11 +83,19 @@ RSpec.describe Collection do
       let(:account) { Fabricate(:account) }
 
       before do
-        account.block!(items.first.account)
+        account.block!(accepted_items.first.account)
       end
 
       it 'does not return items blocked by this account' do
-        expect(subject.items_for(account)).to contain_exactly(items.last)
+        expect(subject.items_for(account)).to contain_exactly(accepted_items.last)
+      end
+    end
+
+    context 'when given the owner of the collection' do
+      let(:account) { subject.account }
+
+      it 'returns accepted and pending items' do
+        expect(subject.items_for(account)).to match_array(accepted_items + [pending_item])
       end
     end
   end

spec/serializers/rest/collection_item_serializer_spec.rb

@@ -11,23 +11,33 @@ RSpec.describe REST::CollectionItemSerializer do
               state:)
   end
 
-  context 'when state is `accepted`' do
-    let(:state) { :accepted }
-
+  shared_examples 'full result' do
     it 'includes the relevant attributes including the account' do
       expect(subject)
         .to include(
           'id' => '2342',
           'account_id' => collection_item.account_id.to_s,
-          'state' => 'accepted',
+          'state' => state.to_s,
           'created_at' => match_api_datetime_format
         )
     end
   end
 
-  %i(pending rejected revoked).each do |unaccepted_state|
-    context "when state is `#{unaccepted_state}`" do
-      let(:state) { unaccepted_state }
+  context 'when state is `accepted`' do
+    let(:state) { :accepted }
+
+    it_behaves_like 'full result'
+  end
+
+  context 'when state is `pending`' do
+    let(:state) { :pending }
+
+    it_behaves_like 'full result'
+  end
+
+  %i(rejected revoked).each do |rejected_state|
+    context "when state is `#{rejected_state}`" do
+      let(:state) { rejected_state }
 
       it 'does not include an account' do
         expect(subject.keys).to_not include('account_id')

spec/serializers/rest/collection_serializer_spec.rb

@@ -67,4 +67,30 @@ RSpec.describe REST::CollectionSerializer do
       end
     end
   end
+
+  context 'when the collection has items in different states' do
+    before do
+      %i(accepted pending rejected revoked).each do |state|
+        Fabricate(:collection_item, collection:, state:)
+      end
+    end
+
+    context 'when `current_user` is the owner of the collection' do
+      let(:current_user) { collection.account.user }
+
+      it 'includes accepted and pending items' do
+        expect(subject['item_count']).to eq 2
+        expect(subject['items'].size).to eq 2
+        expect(subject['items'].pluck('state')).to contain_exactly('accepted', 'pending')
+      end
+    end
+
+    context 'when `current_user` is not the owner of the collection' do
+      it 'only includes the accepted item' do
+        expect(subject['item_count']).to eq 1
+        expect(subject['items'].size).to eq 1
+        expect(subject['items'].first['state']).to eq 'accepted'
+      end
+    end
+  end
 end