HJ-86 system endpoint new lifecycle table #5484
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
refs/pull/5484/merge
|
| Run status |
|
| Run duration | 00m 38s |
| Commit |
|
| Committer | Kirk Hardy |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
erosselli
reviewed
Nov 12, 2024
Comment on lines
424
to
425
| (System.connection_configs != None) | ||
| | (System.dataset_references.any()) # noqa: E712 |
There was a problem hiding this comment.
why do we use != None for one but .any() for the other condition?
There was a problem hiding this comment.
dataset_references is a column on ctl_systems, so we just check for anything in that column. System.connection_configs is a derived relationship, Systems have a property connection_configs but it's attached via the connectionconfig table having a system_id, so the SQL that gets generated has a join and works better with != None
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5484 +/- ##
=======================================
Coverage 85.20% 85.20%
=======================================
Files 386 386
Lines 24242 24247 +5
Branches 2642 2644 +2
=======================================
+ Hits 20655 20660 +5
Misses 3033 3033
Partials 554 554 ☔ View full report in Codecov by Sentry. |
erosselli
approved these changes
Nov 12, 2024
thingscouldbeworse
deleted the
HJ-86_system-endpoint-new-lifecycle-table
branch
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 00m 37s |
| Commit |
|
| Committer | Kirk Hardy |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
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.
Closes #HJ-86
Description Of Changes
Necessary changes for lifecycle tables. Option to retrieve only DnD relevant systems when paginating/filtering, logic to filter out "hidden" systems, and corresponding migration to add
hiddenbool columnCode Changes
GETendpoint also allow fordnd_relevantsystems, decided asctl_systemshidden, defaults toFalseGETendpoint filtering, but provide a param to optionally include themSteps to Confirm
ConnectionConfigobject (viaconnectionconfig.system_id)ctl_systems.dataset_references)hiddencolumn set toTrue/systemendpoint withGETto retrieve systems with pagination parameters and withdnd_relevantset totrue, and alsofalse.trueshould exclude the system without a connection config or dataset referenceshow_hiddenproperty on/off. When on, the hidden system should be returned, otherwise notPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works