-
Notifications
You must be signed in to change notification settings - Fork 4
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
Updating user management flyway migrations #367
Updating user management flyway migrations #367
Conversation
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🔴 | Statements | 53.72% | 938/1746 |
🔴 | Branches | 53.66% | 293/546 |
🔴 | Functions | 52.6% | 192/365 |
🔴 | Lines | 53.79% | 881/1638 |
Test suite run success
72 tests passing in 9 suites.
Report generated by 🧪jest coverage report action from 0c10c5e
not entirely sure what is happening with these errors...I originally tried just editing the 5th migration, and thought to instead use a new 7th migration, but still running into the same errors... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for documenting the schema 🥇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
PULL REQUEST
Summary
This updates the user management flyway migrations to be in-line with the new ERD for user migrations: https://app.diagrams.net/#G1E5F9_cakvji46qRMMa0lGOZiaAXnJa5M#%7B%22pageId%22%3A%22WHVhyQuvTKeJfM7__wGX%22%7D
This is necessary to ensure that the user migration work is able to update/manager usergroups data.
Other minor note:
user
is a reserved keyword, which is why the table is calledusers
. We have a mix of tables in the singular, and some in the plural; not sure if we have we want to have a rule around that, or if it matters materially. Just a nit I noticed when reviewing the ERD.Related Issue
Fixes #361, #362
Additional Information
Anything else the review team should know?
Checklist