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

Migrate system > meta > vendor > id if it exists to system > vendor_id. #4088

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Sep 13, 2023

Closes #4084

Description Of Changes

Migrate system > meta > vendor > id if it exists and vendor_id doesn't already exist to system > vendor id. Remove the "vendor" key from meta dict if applicable and the migration took place on that row.

Before

fides=# select id, meta, vendor_id from ctl_systems order by id
;
                    id                    |                 meta                  | vendor_id 
------------------------------------------+---------------------------------------+-----------
 ctl_0118fbbd-282e-4d31-bc8b-5777d713cf3a | {"health": 2, "vendor": {"id": "87"}} | 
 ctl_25efee31-7fb7-41cb-a660-5141cc79c23e | {"health": 1}                         | 
 ctl_40f88ca0-810d-4af0-a26e-64aff9181e59 | {"health": 3, "vendor": {"id": "29"}} | 21
 ctl_483e730c-3dba-4fe9-b59f-dc2ac562bc95 | {}                                    | 
 ctl_a179bd58-8d6e-40ce-9c88-17e18f14e3b4 | {"vendor": {"id": "1111"}}            | 
(5 rows)

After

Note: ctl_40f88ca0-810d-4af0-a26e-64aff9181e59 didn't migrate because there was also a vendor_id. I also left the old record there. Logging in this case.

fides=# select id, meta, vendor_id from ctl_systems order by id
;
                    id                    |                 meta                  | vendor_id 
------------------------------------------+---------------------------------------+-----------
 ctl_0118fbbd-282e-4d31-bc8b-5777d713cf3a | {"health": 2}                         | 87
 ctl_25efee31-7fb7-41cb-a660-5141cc79c23e | {"health": 1}                         | 
 ctl_40f88ca0-810d-4af0-a26e-64aff9181e59 | {"health": 3, "vendor": {"id": "29"}} | 21
 ctl_483e730c-3dba-4fe9-b59f-dc2ac562bc95 | {}                                    | 
 ctl_a179bd58-8d6e-40ce-9c88-17e18f14e3b4 | {}                                    | 1111

Code Changes

  • list your code changes here

Steps to Confirm

  • list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Sep 13, 2023

Passing run #4130 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge c471d3e into 40addea...
Project: fides Commit: 771b5dffe8 ℹ️
Status: Passed Duration: 01:29 💡
Started: Sep 13, 2023 10:19 PM Ended: Sep 13, 2023 10:21 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@pattisdr pattisdr marked this pull request as ready for review September 13, 2023 22:08
Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

looks great! was even able to test e2e by checkout out fidesplus 2.19.3 and running the BE while running the FE off of fides 2.19.1.

then i tested switching over to this fides branch and running the FE while running a fidesplus BE that was built with dev_prerelease referencing the 2.19.2a1 tag that's pinned to the HEAD of this branch. the migration worked, confirmed by looking at the DB and also by looking at my existing system in the datamap UI!

@pattisdr
Copy link
Contributor Author

That's some thorough testing 🔎

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (25f8fc7) 87.39% compared to head (c471d3e) 87.39%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4088   +/-   ##
=======================================
  Coverage   87.39%   87.39%           
=======================================
  Files         320      320           
  Lines       19693    19693           
  Branches     2528     2528           
=======================================
  Hits        17211    17211           
  Misses       2041     2041           
  Partials      441      441           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pattisdr pattisdr merged commit 8fac42b into main Sep 13, 2023
43 checks passed
@pattisdr pattisdr deleted the fides_4084_vendor_id branch September 13, 2023 23:11
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.

Migrate System > meta > vendor > id to System > vendor id
2 participants