-
Notifications
You must be signed in to change notification settings - Fork 27
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
♻️ refactor project access rights 🗃️🚨 #6060
♻️ refactor project access rights 🗃️🚨 #6060
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6060 +/- ##
=========================================
+ Coverage 84.5% 88.1% +3.5%
=========================================
Files 10 1442 +1432
Lines 214 59383 +59169
Branches 25 1409 +1384
=========================================
+ Hits 181 52345 +52164
- Misses 23 6742 +6719
- Partials 10 296 +286
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
I guess still WIP, right? Please re-assign for review when ready. thx
yes, sorry it should be Draft |
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.
same here ;) see you!
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.
ask again when done (clearing notification)
…o is716/move-access-rights-from-projects-table
….com:matusdrobuliak66/osparc-simcore into is716/move-access-rights-from-projects-table
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.
Alles klar, merci!
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.
Very nice. Thanks a lot for the effort! Just a few suggestions
packages/postgres-database/src/simcore_postgres_database/models/projects.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/projects/_groups_api.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/projects/_groups_handlers.py
Show resolved
Hide resolved
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.
Thanks for the massive effort here.
I think we're almost there. Please consider my suggestions below.
Especially the one regarding the table name.
packages/postgres-database/src/simcore_postgres_database/models/project_to_groups.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/projects/_groups_api.py
Outdated
Show resolved
Hide resolved
services/web/server/tests/unit/with_dbs/02/test_projects_comments_handlers.py
Outdated
Show resolved
Hide resolved
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.
Left some suggestions. Please consider them before merging
Thx for the great effort.
packages/postgres-database/src/simcore_postgres_database/models/project_to_groups.py
Show resolved
Hide resolved
.../simcore_postgres_database/migration/versions/19f3d9085636_create_project_to_groups_table.py
Show resolved
Hide resolved
packages/postgres-database/src/simcore_postgres_database/models/projects.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/projects/_comments_handlers.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/projects/_db_utils.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/projects/exceptions.py
Show resolved
Hide resolved
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.
approved as requested
What do these changes do?
project_to_groups
was created 🗃️)/projects/<project-id>/groups/<group-id>
/projects/<project-id>/groups/<group-id>
/projects/<project-id>/groups/<group-id>
Breaking change !! @odeimaiz
/projects/<project-id>/groups
endpoints needs to be used!Related issue/s
access_rights
column inprojects
to a separate table osparc-issues#1547How to test
Dev-ops checklist