-
Notifications
You must be signed in to change notification settings - Fork 429
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
fix: subscription info cache race condition #4518
fix: subscription info cache race condition #4518
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Docker builds report
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4518 +/- ##
=======================================
Coverage 96.91% 96.91%
=======================================
Files 1178 1178
Lines 39515 39528 +13
=======================================
+ Hits 38296 38310 +14
+ Misses 1219 1218 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Fixes race condition(s) related to the cancellation of a subscription. Related sentry issue here.
As far as I can tell from doing some manual testing locally by running the API and pointing it to the staging database, there were likely 2 issues in the code here:
BEFORE_DELETE
hook on the organisation model which cancelled the subscription if it was paid. This involved deleting the relatedOrganisationSubscriptionInformationCache
object, but the execution path then continued to delete the organisation, and subsequently try and cascade delete the OSIC object. This would fail (seemingly only some of the time).How did you test this code?
I have updated a unit test to test the new behaviour, however, (perhaps more importantly) I have also tested this locally to confirm I can delete an organisation that I was previously unable to delete due to the related sentry issue.