-
Notifications
You must be signed in to change notification settings - Fork 66
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
Remove notifications for deleted instances #4899
Conversation
We should probably do Devices as well, but want the approach verifying first |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4899 +/- ##
==========================================
- Coverage 78.74% 78.64% -0.10%
==========================================
Files 323 324 +1
Lines 15221 15254 +33
Branches 3496 3501 +5
==========================================
+ Hits 11986 11997 +11
- Misses 3235 3257 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Can we look at doing a bulk delete sql rather than lots of individual ones? https://sequelize.org/docs/v7/querying/delete/#deleting-in-bulk |
I'll have another look, but we only have raw SQL access at this point (none of the models are loaded when the migrations run). And the Notification table doesn't have a bare field with the project id that would could do a |
Good point; the reference field doesn't lend itself to using with |
This should speed up results when found I think
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.
Issue with the count query across both support db flavours.
forge/db/migrations/20241209-01-remove-dead-instance-notifications.js
Outdated
Show resolved
Hide resolved
…ions.js Co-authored-by: Nick O'Leary <[email protected]>
fixes #4845
Description
Adds afterDestroy hook to Projects table remove Notifications when Instances are deleted.
Also includes a migration to remove orphaned Notifications.
Related Issue(s)
#4845
Checklist
flowforge.yml
?FlowFuse/helm
to update ConfigMap TemplateFlowFuse/CloudProject
to update values for Staging/ProductionLabels
area:migration
label