Add a stored procedure to handle entity deletions #1618
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JAORMX
reviewed
Nov 13, 2023
| BEGIN; | ||
|
|
||
| -- Define the trigger function (if it doesn't already exist) | ||
| CREATE OR REPLACE FUNCTION update_profile_status_on_delete() RETURNS TRIGGER AS $$ |
|
|
||
| -- Update overall profile status if a rule evaluation status is updated | ||
| -- error takes precedence over failure, failure takes precedence over success | ||
| CREATE OR REPLACE FUNCTION update_profile_status() RETURNS TRIGGER AS $$ |
There was a problem hiding this comment.
Can you change the comment to mention what actually changed? It's tricky to figure out since we don't have a diff
There was a problem hiding this comment.
sure. I just realised I should have included the old function in the down migration for completeness, too
JAORMX
reviewed
Nov 13, 2023
| FROM rule_evaluations | ||
| WHERE id = NEW.rule_eval_id; | ||
|
|
||
| raise warning 'profile_id: %', v_profile_id; |
There was a problem hiding this comment.
Do we want to keep this raise warning?
There was a problem hiding this comment.
oops, no, sorry, this is a debug leftover
|
let me rebase atop the recent migration (I'll bump the file number) |
jhrozek
force-pushed
the
entity_delete
branch
from
57dd281 to
86d130a
Compare
In case an entity was deleted (typically a PR but a repo delete is also a valid use-case), we would not update the profile status until the next profile run. For that, we need to add a new database trigger in a migration. Sort-of-related is a bug in the original procedure that we had where if all the current rules were skipped and we added a failure the overall status would still be a failure. I added a new version of the old procedure to cover that plus some tests. Fixes: mindersec#1610
jhrozek
force-pushed
the
entity_delete
branch
from
86d130a to
50350c5
Compare
JAORMX
approved these changes
Nov 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In case an entity was deleted (typically a PR but a repo delete is also
a valid use-case), we would not update the profile status until the next
profile run.
For that, we need to add a new database trigger in a migration.
Sort-of-related is a bug in the original procedure that we had where if
all the current rules were skipped and we added a failure the overall
status would still be a failure. I added a new version of the old
procedure to cover that plus some tests.
Fixes: #1610