Skip to content

Commit

Permalink
Expire requestable synchronously
Browse files Browse the repository at this point in the history
This fixes an issue where if more than one requestable happened to
expire at the same time, there is a race condition between when the
status and action required at fields are updated, leading to incorrect
application statuses.
  • Loading branch information
thomasleese committed Jun 13, 2024
1 parent 7720aad commit acf8145
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 190 deletions.
11 changes: 0 additions & 11 deletions app/jobs/expire_requestable_job.rb

This file was deleted.

6 changes: 4 additions & 2 deletions app/jobs/expire_requestables_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ def perform(requestable_class_name)
.requested
.not_received
.not_expired
.find_each do |requestable|
ExpireRequestableJob.perform_later(requestable)
.each do |requestable|
if requestable.expires? && Time.zone.now > requestable.expires_at
ExpireRequestable.call(requestable:, user: "Expirer")
end
end
end
end
167 changes: 0 additions & 167 deletions spec/jobs/expire_requestable_job_spec.rb

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,36 @@
describe "#perform" do
subject(:perform) { described_class.new.perform(class_name) }

let!(:requested_requestable) { create(:"requested_#{factory_name}") }
let!(:requested_requestable) do
create(:"requested_#{factory_name}", requested_at: 1.year.ago)
end
let!(:received_requestable) { create(:"received_#{factory_name}") }
let!(:expired_requestable) do
create(:"requested_#{factory_name}", :expired)
end

it "enqueues a job for each 'requested' #{class_name}s" do
expect { perform }.to have_enqueued_job(ExpireRequestableJob).with(
requested_requestable,
it "enqueues a job for each requested #{class_name}s" do
expect(ExpireRequestable).to receive(:call).with(
requestable: requested_requestable,
user: "Expirer",
)
perform
end

it "doesn't enqueue a job for 'received' #{class_name}s" do
expect { perform }.to_not have_enqueued_job(ExpireRequestableJob).with(
received_requestable,
it "doesn't enqueue a job for received #{class_name}s" do
expect(ExpireRequestable).to_not receive(:call).with(
requestable: received_requestable,
user: "Expirer",
)
perform
end

it "doesn't enqueue a job for 'expired' #{class_name}s" do
expect { perform }.to_not have_enqueued_job(ExpireRequestableJob).with(
expired_requestable,
it "doesn't enqueue a job for expired #{class_name}s" do
expect(ExpireRequestable).to_not receive(:call).with(
requestable: expired_requestable,
user: "Expirer",
)
perform
end
end
end
Expand Down

0 comments on commit acf8145

Please sign in to comment.