From 7788281759e9bbed23c1ecd4f76ccaa7734eb617 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Tue, 24 Mar 2026 15:43:50 +0100 Subject: [PATCH] Fix adding items without a position (#38368) --- app/models/collection_item.rb | 2 +- spec/models/collection_item_spec.rb | 6 +++++ .../process_featured_item_service_spec.rb | 25 +++++++++++++------ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/app/models/collection_item.rb b/app/models/collection_item.rb index 018fd3cb17..f7067fb2fc 100644 --- a/app/models/collection_item.rb +++ b/app/models/collection_item.rb @@ -62,7 +62,7 @@ class CollectionItem < ApplicationRecord private def set_position - return if position_changed? + return if position.present? && position_changed? self.position = self.class.where(collection_id:).maximum(:position).to_i + 1 end diff --git a/spec/models/collection_item_spec.rb b/spec/models/collection_item_spec.rb index 8960d43439..89ef2bc534 100644 --- a/spec/models/collection_item_spec.rb +++ b/spec/models/collection_item_spec.rb @@ -62,6 +62,12 @@ RSpec.describe CollectionItem do expect(custom_item.position).to eq 7 end + it 'automatically sets the position if excplicitly set to `nil`' do + item = collection.collection_items.create!(account:, position: nil) + + expect(item.position).to eq 1 + end + it 'automatically sets `activity_uri` when account is remote' do item = collection.collection_items.create(account: Fabricate(:remote_account)) diff --git a/spec/services/activitypub/process_featured_item_service_spec.rb b/spec/services/activitypub/process_featured_item_service_spec.rb index 6a91f6dba8..bfcea2a5da 100644 --- a/spec/services/activitypub/process_featured_item_service_spec.rb +++ b/spec/services/activitypub/process_featured_item_service_spec.rb @@ -47,15 +47,26 @@ RSpec.describe ActivityPub::ProcessFeaturedItemService do it_behaves_like 'non-matching URIs' context 'when item does not yet exist' do - it 'creates and verifies the item' do - expect { subject.call(collection, object, position:) }.to change(collection.collection_items, :count).by(1) + context 'when a position is given' do + it 'creates and verifies the item' do + expect { subject.call(collection, object, position:) }.to change(collection.collection_items, :count).by(1) - expect(stubbed_service).to have_received(:call) + expect(stubbed_service).to have_received(:call) - new_item = collection.collection_items.last - expect(new_item.object_uri).to eq 'https://example.com/actor/1' - expect(new_item.approval_uri).to be_nil - expect(new_item.position).to eq 3 + new_item = collection.collection_items.last + expect(new_item.object_uri).to eq 'https://example.com/actor/1' + expect(new_item.approval_uri).to be_nil + expect(new_item.position).to eq 3 + end + end + + context 'when no position is given' do + it 'creates the item' do + expect { subject.call(collection, object) }.to change(collection.collection_items, :count).by(1) + new_item = collection.collection_items.last + + expect(new_item.position).to eq 1 + end end end