From 5c8dd91bc2f3f3c93e300da1967ca60eec373492 Mon Sep 17 00:00:00 2001 From: Sergio Mira Date: Mon, 19 Jan 2026 13:15:52 +0100 Subject: [PATCH] Fix flaky ItemizableUpdateService. Logic was order dependent and have removed the global Timecop freeze to be able to order Event entries. --- .../itemizable_update_service_spec.rb | 79 +++++++++---------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/spec/services/itemizable_update_service_spec.rb b/spec/services/itemizable_update_service_spec.rb index 4345e23805..0a0031b71d 100644 --- a/spec/services/itemizable_update_service_spec.rb +++ b/spec/services/itemizable_update_service_spec.rb @@ -5,6 +5,7 @@ let(:item1) { create(:item, organization: organization, name: "My Item 1") } let(:item2) { create(:item, organization: organization, name: "My Item 2") } let(:item3) { create(:item, organization: organization, name: "My Item 3") } + let(:two_days_ago) { 2.days.ago } # Store exact timestamp to be able to assert later before(:each) do TestInventory.create_inventory(storage_location.organization, { storage_location.id => { @@ -18,12 +19,6 @@ }) end - around(:each) do |ex| - freeze_time do - ex.run - end - end - describe "increases" do let(:itemizable) do line_items = [ @@ -39,12 +34,12 @@ let(:attributes) do { - issued_at: 2.days.ago, + issued_at: two_days_ago, line_items_attributes: {"0": {item_id: item1.id, quantity: 2}, "1": {item_id: item2.id, quantity: 2}} } end - subject do + def call_update_service described_class.call(itemizable: itemizable, params: attributes, event_class: DonationEvent) @@ -53,19 +48,19 @@ it "should update quantity in same storage location" do expect(storage_location.size).to eq(20) expect(new_storage_location.size).to eq(20) - subject + call_update_service expect(itemizable.reload.line_items.count).to eq(2) expect(itemizable.line_items.sum(&:quantity)).to eq(4) expect(storage_location.size).to eq(14) expect(new_storage_location.size).to eq(20) - expect(itemizable.issued_at).to eq(2.days.ago) + expect(itemizable.issued_at.to_fs(:db)).to eq(two_days_ago.to_fs(:db)) # Use to_fs to ignore nanosecond differences expect(UpdateExistingEvent.count).to eq(1) end it "fails with a validation message for donation events with invalid issued_at" do attributes[:issued_at] = "" - expect { subject }.to raise_error do |e| + expect { call_update_service }.to raise_error do |e| expect(e).to be_a(ActiveRecord::RecordInvalid) expect(e.message).to eq("Validation failed: Issue date can't be blank") end @@ -75,7 +70,7 @@ context "when there is no intervening audit" do it "should update quantity in different locations" do attributes[:storage_location_id] = new_storage_location.id - subject + call_update_service expect(itemizable.reload.line_items.count).to eq(2) expect(itemizable.line_items.sum(&:quantity)).to eq(4) expect(storage_location.size).to eq(10) @@ -89,7 +84,7 @@ "If you need to change the storage location, please delete this donation and create a new donation with the new storage location." create(:audit, :with_items, item: itemizable.items.first, organization: organization, storage_location: storage_location, status: "finalized") attributes[:storage_location_id] = new_storage_location.id - expect { subject }.to raise_error(msg) + expect { call_update_service }.to raise_error(msg) end end end @@ -97,7 +92,7 @@ it "should raise an error if any item is inactive" do item1.update!(active: false) msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing." - expect { subject }.to raise_error(msg) + expect { call_update_service }.to raise_error(msg) end end @@ -116,30 +111,30 @@ let(:attributes) do { - issued_at: 2.days.ago, + issued_at: two_days_ago, line_items_attributes: {"0": {item_id: item1.id, quantity: 2}, "1": {item_id: item2.id, quantity: 2}} } end - subject do + def call_update_service described_class.call(itemizable: itemizable, params: attributes, event_class: DistributionEvent) end it "should update quantity in same storage location" do expect(storage_location.size).to eq(20) expect(new_storage_location.size).to eq(20) - subject + call_update_service expect(itemizable.reload.line_items.count).to eq(2) expect(itemizable.line_items.sum(&:quantity)).to eq(4) expect(storage_location.size).to eq(26) expect(new_storage_location.size).to eq(20) - expect(itemizable.issued_at).to eq(2.days.ago) + expect(itemizable.issued_at.to_fs(:db)).to eq(two_days_ago.to_fs(:db)) # Use to_fs to ignore nanosecond differences end it "fails with a validation message for distribution events with invalid issued_at" do attributes[:issued_at] = "" - expect { subject }.to raise_error do |e| + expect { call_update_service }.to raise_error do |e| expect(e).to be_a(ActiveRecord::RecordInvalid) expect(e.message).to eq("Validation failed: Distribution date and time can't be blank") end @@ -149,7 +144,7 @@ context "when there is no intervening audit" do it "should update quantity in different locations" do attributes[:storage_location_id] = new_storage_location.id - subject + call_update_service expect(itemizable.reload.line_items.count).to eq(2) expect(itemizable.line_items.sum(&:quantity)).to eq(4) expect(storage_location.size).to eq(30) @@ -163,7 +158,7 @@ "If you need to change the storage location, please reclaim this distribution and create a new distribution from the new storage location." create(:audit, :with_items, item: itemizable.items.first, organization: organization, storage_location: storage_location, status: "finalized") attributes[:storage_location_id] = new_storage_location.id - expect { subject }.to raise_error(msg) + expect { call_update_service }.to raise_error(msg) end end end @@ -171,7 +166,7 @@ it "should raise an error if any item is inactive" do item1.update!(active: false) msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing." - expect { subject }.to raise_error(msg) + expect { call_update_service }.to raise_error(msg) end end @@ -188,12 +183,14 @@ line_items: line_items, issued_at: 1.day.ago) end + let(:attributes) do { - issued_at: 2.days.ago, + issued_at: two_days_ago, line_items_attributes: {"0": {item_id: item1.id, quantity: 5}, "1": {item_id: item3.id, quantity: 50}} } end + it "should send an itemizable event if it already exists" do DonationEvent.publish(itemizable) expect(DonationEvent.count).to eq(1) @@ -216,14 +213,8 @@ expect(View::Inventory.total_inventory(organization.id)).to eq(75) # 40 - 5 (item1) - 10 (item2) + 50 (item3) end end + describe "with distributions" do - before(:each) do - TestInventory.create_inventory(storage_location.organization, { - storage_location.id => { - item3.id => 10 - } - }) - end let(:itemizable) do line_items = [ create(:line_item, item_id: item1.id, quantity: 5), @@ -235,12 +226,22 @@ line_items: line_items, issued_at: 1.day.ago) end + let(:attributes) do { - issued_at: 2.days.ago, + issued_at: two_days_ago, line_items_attributes: {"0": {item_id: item1.id, quantity: 2}, "1": {item_id: item3.id, quantity: 6}} } end + + before(:each) do + TestInventory.create_inventory(storage_location.organization, { + storage_location.id => { + item3.id => 10 + } + }) + end + it "should send an itemizable event if it already exists" do DistributionEvent.publish(itemizable) expect(DistributionEvent.count).to eq(1) @@ -264,10 +265,9 @@ end it "should raise an error if there is an intervening snapshot" do - itemizable.save! - travel(-1.week) - SnapshotEvent.publish(organization) - travel 1.week + travel(-1.week) do + SnapshotEvent.publish(organization) + end itemizable.update!(created_at: 2.weeks.ago) expect do described_class.call(itemizable: itemizable, params: attributes, event_class: DistributionEvent) @@ -276,16 +276,15 @@ it "should not raise an error if no inventory was changed" do no_change_attrs = { - issued_at: 2.days.ago, + issued_at: two_days_ago, line_items_attributes: { "0": {item_id: item1.id, quantity: 10}, "1": {item_id: item2.id, quantity: 10} } } - itemizable.save! - travel(-1.week) - SnapshotEvent.publish(organization) - travel 1.week + travel(-1.week) do + SnapshotEvent.publish(organization) + end itemizable.update!(created_at: 2.weeks.ago) expect do described_class.call(itemizable: itemizable, params: no_change_attrs, event_class: DistributionEvent)