Change Collections API shape (#37580)

app/controllers/api/v1_alpha/collection_items_controller.rb

@@ -21,7 +21,7 @@ class Api::V1Alpha::CollectionItemsController < Api::BaseController
 
     @item = AddAccountToCollectionService.new.call(@collection, @account)
 
-    render json: @item, serializer: REST::CollectionItemSerializer
+    render json: @item, serializer: REST::CollectionItemSerializer, adapter: :json
   end
 
   def destroy

app/controllers/api/v1_alpha/collections_controller.rb

@@ -28,14 +28,14 @@ class Api::V1Alpha::CollectionsController < Api::BaseController
     cache_if_unauthenticated!
     authorize Collection, :index?
 
-    render json: @collections, each_serializer: REST::BaseCollectionSerializer
+    render json: @collections, each_serializer: REST::CollectionSerializer, adapter: :json
   end
 
   def show
     cache_if_unauthenticated!
     authorize @collection, :show?
 
-    render json: @collection, serializer: REST::CollectionSerializer
+    render json: @collection, serializer: REST::CollectionWithAccountsSerializer
   end
 
   def create
@@ -43,7 +43,7 @@ class Api::V1Alpha::CollectionsController < Api::BaseController
 
     @collection = CreateCollectionService.new.call(collection_creation_params, current_user.account)
 
-    render json: @collection, serializer: REST::CollectionSerializer
+    render json: @collection, serializer: REST::CollectionSerializer, adapter: :json
   end
 
   def update
@@ -51,7 +51,7 @@ class Api::V1Alpha::CollectionsController < Api::BaseController
 
     @collection.update!(collection_update_params) # TODO: Create a service for this to federate changes
 
-    render json: @collection, serializer: REST::CollectionSerializer
+    render json: @collection, serializer: REST::CollectionSerializer, adapter: :json
   end
 
   def destroy

app/serializers/rest/base_collection_serializer.rb

@@ -1,12 +0,0 @@
-# frozen_string_literal: true
-
-class REST::BaseCollectionSerializer < ActiveModel::Serializer
-  attributes :id, :uri, :name, :description, :language, :local, :sensitive,
-             :discoverable, :item_count, :created_at, :updated_at
-
-  belongs_to :tag, serializer: REST::StatusSerializer::TagSerializer
-
-  def id
-    object.id.to_s
-  end
-end

app/serializers/rest/collection_item_serializer.rb

@@ -3,11 +3,15 @@
 class REST::CollectionItemSerializer < ActiveModel::Serializer
   delegate :accepted?, to: :object
 
-  attributes :id, :position, :state
+  attributes :id, :state
 
-  belongs_to :account, serializer: REST::AccountSerializer, if: :accepted?
+  attribute :account_id, if: :accepted?
 
   def id
     object.id.to_s
   end
+
+  def account_id
+    object.account_id.to_s
+  end
 end

app/serializers/rest/collection_serializer.rb

@@ -1,11 +1,23 @@
 # frozen_string_literal: true
 
-class REST::CollectionSerializer < REST::BaseCollectionSerializer
+class REST::CollectionSerializer < ActiveModel::Serializer
-  belongs_to :account, serializer: REST::AccountSerializer
+  attributes :id, :uri, :name, :description, :language, :account_id,
+             :local, :sensitive, :discoverable, :item_count,
+             :created_at, :updated_at
+
+  belongs_to :tag, serializer: REST::StatusSerializer::TagSerializer
 
   has_many :items, serializer: REST::CollectionItemSerializer
 
+  def id
+    object.id.to_s
+  end
+
   def items
     object.items_for(current_user&.account)
   end
+
+  def account_id
+    object.account_id.to_s
+  end
 end

app/serializers/rest/collection_with_accounts_serializer.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class REST::CollectionWithAccountsSerializer < ActiveModel::Serializer
+  belongs_to :collection, serializer: REST::CollectionSerializer
+
+  has_many :accounts, serializer: REST::AccountSerializer
+
+  def collection
+    object
+  end
+
+  def accounts
+    [object.account] + object.collection_items.map(&:account)
+  end
+end

spec/requests/api/v1_alpha/collection_items_spec.rb

@@ -26,6 +26,7 @@ RSpec.describe 'Api::V1Alpha::CollectionItems', feature: :collections do
           end.to change(collection.collection_items, :count).by(1)
 
           expect(response).to have_http_status(200)
+          expect(response.parsed_body).to have_key('collection_item')
         end
       end
 

spec/requests/api/v1_alpha/collections_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
       subject
 
       expect(response).to have_http_status(200)
-      expect(response.parsed_body.size).to eq 3
+      expect(response.parsed_body[:collections].size).to eq 3
     end
 
     context 'with limit param' do
@@ -30,7 +30,7 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
         subject
 
         expect(response).to have_http_status(200)
-        expect(response.parsed_body.size).to eq 1
+        expect(response.parsed_body[:collections].size).to eq 1
 
         expect(response)
           .to include_pagination_headers(
@@ -46,7 +46,7 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
         subject
 
         expect(response).to have_http_status(200)
-        expect(response.parsed_body.size).to eq 1
+        expect(response.parsed_body[:collections].size).to eq 1
 
         expect(response)
           .to include_pagination_headers(
@@ -66,7 +66,7 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
           subject
 
           expect(response).to have_http_status(200)
-          expect(response.parsed_body.size).to eq 3
+          expect(response.parsed_body[:collections].size).to eq 3
         end
       end
 
@@ -77,7 +77,7 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
           subject
 
           expect(response).to have_http_status(200)
-          expect(response.parsed_body.size).to eq 4
+          expect(response.parsed_body[:collections].size).to eq 4
         end
       end
     end
@@ -96,7 +96,7 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
         subject
 
         expect(response).to have_http_status(200)
-        expect(response.parsed_body[:items].size).to eq 2
+        expect(response.parsed_body[:collection][:items].size).to eq 2
       end
     end
 
@@ -120,8 +120,8 @@ RSpec.describe 'Api::V1Alpha::Collections', feature: :collections do
           subject
 
           expect(response).to have_http_status(200)
-          expect(response.parsed_body[:items].size).to eq 1
+          expect(response.parsed_body[:collection][:items].size).to eq 1
-          expect(response.parsed_body[:items][0]['position']).to eq items.last.position
+          expect(response.parsed_body[:collection][:items][0]['id']).to eq items.last.id.to_s
         end
       end
     end

spec/serializers/rest/base_collection_serializer_spec.rb

@@ -1,41 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe REST::BaseCollectionSerializer do
-  subject do
-    serialized_record_json(collection, described_class, options: {
-      scope: current_user,
-      scope_name: :current_user,
-    })
-  end
-
-  let(:current_user) { nil }
-
-  let(:tag) { Fabricate(:tag, name: 'discovery') }
-  let(:collection) do
-    Fabricate(:collection,
-              id: 2342,
-              name: 'Exquisite follows',
-              description: 'Always worth a follow',
-              local: true,
-              sensitive: true,
-              discoverable: false,
-              tag:)
-  end
-
-  it 'includes the relevant attributes' do
-    expect(subject)
-      .to include(
-        'id' => '2342',
-        'name' => 'Exquisite follows',
-        'description' => 'Always worth a follow',
-        'local' => true,
-        'sensitive' => true,
-        'discoverable' => false,
-        'tag' => a_hash_including('name' => 'discovery'),
-        'created_at' => match_api_datetime_format,
-        'updated_at' => match_api_datetime_format
-      )
-  end
-end

spec/serializers/rest/collection_item_serializer_spec.rb

@@ -8,8 +8,7 @@ RSpec.describe REST::CollectionItemSerializer do
   let(:collection_item) do
     Fabricate(:collection_item,
               id: 2342,
-              state:,
+              state:)
-              position: 4)
   end
 
   context 'when state is `accepted`' do
@@ -19,9 +18,8 @@ RSpec.describe REST::CollectionItemSerializer do
       expect(subject)
         .to include(
           'id' => '2342',
-          'account' => an_instance_of(Hash),
+          'account_id' => collection_item.account_id.to_s,
-          'state' => 'accepted',
+          'state' => 'accepted'
-          'position' => 4
         )
     end
   end
@@ -31,7 +29,7 @@ RSpec.describe REST::CollectionItemSerializer do
       let(:state) { unaccepted_state }
 
       it 'does not include an account' do
-        expect(subject.keys).to_not include('account')
+        expect(subject.keys).to_not include('account_id')
       end
     end
   end

spec/serializers/rest/collection_serializer_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe REST::CollectionSerializer do
   it 'includes the relevant attributes' do
     expect(subject)
       .to include(
-        'account' => an_instance_of(Hash),
+        'account_id' => collection.account_id.to_s,
         'id' => '2342',
         'name' => 'Exquisite follows',
         'description' => 'Always worth a follow',
@@ -38,7 +38,9 @@ RSpec.describe REST::CollectionSerializer do
         'discoverable' => false,
         'tag' => a_hash_including('name' => 'discovery'),
         'created_at' => match_api_datetime_format,
-        'updated_at' => match_api_datetime_format
+        'updated_at' => match_api_datetime_format,
+        'item_count' => 0,
+        'items' => []
       )
   end
 end

spec/serializers/rest/collection_with_accounts_serializer_spec.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe REST::CollectionWithAccountsSerializer do
+  subject do
+    serialized_record_json(collection, described_class, options: {
+      scope: current_user,
+      scope_name: :current_user,
+    })
+  end
+
+  let(:current_user) { nil }
+
+  let(:tag) { Fabricate(:tag, name: 'discovery') }
+  let(:accounts) { Fabricate.times(3, :account) }
+  let(:collection) do
+    Fabricate(:collection,
+              account: accounts.first,
+              id: 2342,
+              name: 'Exquisite follows',
+              description: 'Always worth a follow',
+              language: 'en',
+              local: true,
+              sensitive: true,
+              discoverable: false,
+              tag:)
+  end
+
+  before do
+    accounts[1..2].each do |account|
+      Fabricate(:collection_item, collection:, account:)
+    end
+    collection.reload
+  end
+
+  it 'includes the relevant attributes' do
+    expect(subject)
+      .to include(
+        'accounts' => an_instance_of(Array),
+        'collection' => a_hash_including({
+          'account_id' => accounts.first.id.to_s,
+          'id' => '2342',
+          'name' => 'Exquisite follows',
+          'description' => 'Always worth a follow',
+          'language' => 'en',
+          'local' => true,
+          'sensitive' => true,
+          'discoverable' => false,
+          'tag' => a_hash_including('name' => 'discovery'),
+          'created_at' => match_api_datetime_format,
+          'updated_at' => match_api_datetime_format,
+          'item_count' => 2,
+          'items' => an_instance_of(Array),
+        })
+      )
+    expect(subject['accounts'].size).to eq 3
+  end
+end