Skip to content

fix(invoice): Fix race-condition in invoice update jobs #3851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

groyoh
Copy link
Contributor

@groyoh groyoh commented Jun 19, 2025

Context

When doing a manual payment, it will update the invoice payment status. When doing so, we're supposed to do a few things asynchronously like sending a webhook.

Unfortunately, the way it is working right now, our server will sometimes process those asynchronous jobs before the database transaction ends, and therefore before we actually update the invoice. We end up with not up-to-date data within those different actions.

For instance, the webhook might be triggered with payment_status: 'pending' in the payload instead of payment_status: 'succeeded'.

Description

This commit fixes this issue by ensuring the jobs run after the transaction is committed. This is done by using AfterCommitEverywhere.after_commit callbacks.

To reduce the risk of mistakes, two new class methods were added:

  • ApplicationJob.perform_after_commit
  • Utils::ActivityLog.produce_after_commit

Doing this, we make the intention clear that the job should run after the transaction is committed.

Test were added for the new methods, and existing tests were updated to properly test that jobs are enqueued after the transaction is committed. To do so, a expect {}.to enqueue_after_commit matcher was added.

When doing a manual payment, it will update the invoice payment status. When doing so, we're supposed to do a few things asynchronously like sending a webhook.

Unfortunately, the way it is working right now, our server will sometimes process those asynchronous jobs before the database transaction ends, and therefore before we actually update the invoice. We end up with not up-to-date data within those different actions.

For instance, the webhook might be triggered with `payment_status: 'pending'` in the payload instead of `payment_status: 'succeeded'`.

This commit fixes this issue by ensuring the jobs run after the transaction is committed. This is done by using `AfterCommitEverywhere.after_commit` callbacks.

To reduce the risk of mistakes, two new class methods were added:

- `ApplicationJob.perform_after_commit`
- `Utils::ActivityLog.produce_after_commit`

Doing this, we make the intention clear that the job should run after the transaction is committed.

Test were added for the new methods, and existing tests were updated to properly test that jobs are enqueued after the transaction is committed. To do so, a `expect {}.to enqueue_after_commit` matcher was added.
@groyoh groyoh force-pushed the fix/after-commit-job branch from f628117 to 5636a6c Compare June 23, 2025 08:00
@groyoh groyoh merged commit 0279f64 into main Jun 23, 2025
14 checks passed
@groyoh groyoh deleted the fix/after-commit-job branch June 23, 2025 08:21
diegocharles pushed a commit that referenced this pull request Jul 11, 2025
## Context

When doing a manual payment, it will update the invoice payment status. When doing so, we're supposed to do a few things asynchronously like sending a webhook.

Unfortunately, the way it is working right now, our server will sometimes process those asynchronous jobs before the database transaction ends, and therefore before we actually update the invoice. We end up with not up-to-date data within those different actions.

For instance, the webhook might be triggered with `payment_status: 'pending'` in the payload instead of `payment_status: 'succeeded'`.

## Description

This commit fixes this issue by ensuring the jobs run after the transaction is committed. This is done by using `AfterCommitEverywhere.after_commit` callbacks.

To reduce the risk of mistakes, two new class methods were added:

- `ApplicationJob.perform_after_commit`
- `Utils::ActivityLog.produce_after_commit`

Doing this, we make the intention clear that the job should run after the transaction is committed.

Test were added for the new methods, and existing tests were updated to properly test that jobs are enqueued after the transaction is committed. To do so, a `expect {}.to enqueue_after_commit` matcher was added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants