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

Add profile ID to latest_evaluation_statuses #3993

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Conversation

dmjb
Copy link
Contributor

@dmjb dmjb commented Jul 25, 2024

Adding this information to the latest_evaluation_statuses table simplifies the process of changing the profile status queries to use the evaluation history tables instead of the old tables I am trying to replace.

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.

Adding this information to the `latest_evaluation_statuses` table
simplifies the process of changing the profile status queries to use the
evaluation history tables instead of the old tables I am trying to
replace.
@dmjb dmjb requested a review from a team as a code owner July 25, 2024 09:16
@coveralls
Copy link

Coverage Status

coverage: 54.316% (+0.005%) from 54.311%
when pulling 655f21b on profile-id-latest-eval
into 211f5f3 on main.

Copy link
Contributor

@blkt blkt left a comment

Choose a reason for hiding this comment

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

Minor comment, looks good to me.

-- namely: retrieving the latest rule statuses for a specific profile
-- A future PR will backfill this column and make it NOT NULL
ALTER TABLE latest_evaluation_statuses ADD COLUMN profile_id UUID REFERENCES profiles(id);
CREATE INDEX idx_profile_id ON latest_evaluation_statuses(profile_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: index names are global to the schema, and this might be ambiguous. Besides, if we don't use the index name anywhere, it's better off left unspecified

Suggested change
CREATE INDEX idx_profile_id ON latest_evaluation_statuses(profile_id);
CREATE INDEX ON latest_evaluation_statuses(profile_id);

@dmjb dmjb merged commit 1cff724 into main Jul 25, 2024
22 checks passed
@dmjb dmjb deleted the profile-id-latest-eval branch July 25, 2024 10:51
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