Skip to content
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

Calculate profile status based on evaluation history tables #4149

Merged
merged 4 commits into from
Aug 15, 2024

Conversation

dmjb
Copy link
Contributor

@dmjb dmjb commented Aug 14, 2024

Remove the existing triggers on rule_evaluations and rule_details_eval.
Replace with triggers on latest_evaluation_statuses. Tweak the trigger
logic to work on the new tables.

Summary

Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.

Fixes #(related issue)

Change Type

Mark the type of change your PR introduces:

  • Bug fix (resolves an issue without affecting existing features)
  • Feature (adds new functionality without breaking changes)
  • Breaking change (may impact existing functionalities or require documentation updates)
  • Documentation (updates or additions to documentation)
  • Refactoring or test improvements (no bug fixes or new functionality)

Testing

Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.

Review Checklist:

  • Reviewed my own code for quality and clarity.
  • Added comments to complex or tricky code sections.
  • Updated any affected documentation.
  • Included tests that validate the fix or feature.
  • Checked that related changes are merged.

@dmjb dmjb force-pushed the profile-status-trigger-eval-history branch 2 times, most recently from c83df0c to 979eeae Compare August 14, 2024 17:05
@dmjb dmjb marked this pull request as ready for review August 14, 2024 17:07
@dmjb dmjb requested a review from a team as a code owner August 14, 2024 17:07
@dmjb dmjb changed the title [WIP] Calculate profile status based on evaluation history tables Calculate profile status based on evaluation history tables Aug 14, 2024
RAISE WARNING 'default case should not happen';
END CASE;

-- This turned out to be very useful during debugging
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is copy-pasted from the previous trigger function, but I can testify that it was in fact very useful during debugging.

@coveralls
Copy link

coveralls commented Aug 14, 2024

Coverage Status

Changes unknown
when pulling dd9c095 on profile-status-trigger-eval-history
into ** on main**.

AND les.rule_entity_id = NEW.rule_entity_id;

IF v_new_status IS NULL THEN
RAISE EXCEPTION 'oh no';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this gave me a chuckle, perhaps we should use a more descriptive exception message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a debugging message I forgot to remove

dmjb added 3 commits August 15, 2024 12:14
Remove the existing triggers on rule_evaluations and rule_details_eval.
Replace with triggers on latest_evaluation_statuses. Tweak the trigger
logic to work on the new tables.
@dmjb dmjb force-pushed the profile-status-trigger-eval-history branch from 6678a1d to ebd7f2b Compare August 15, 2024 11:14
@@ -180,6 +181,61 @@ func upsertEvalStatus(
require.NoError(t, err)
}

func createRuleEntity(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the changes in this test file are needed to use the evaluation history tables instead of the old rule evaluations tables when testing profile status transitions.

@dmjb dmjb merged commit e10dba1 into main Aug 15, 2024
22 checks passed
@dmjb dmjb deleted the profile-status-trigger-eval-history branch August 15, 2024 11:46
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.

3 participants