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
[beta] v1 legacy roles MVP #1390
Conversation
✅ Deploy Preview for galaxyng ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
/retest |
3 similar comments
/retest |
/retest |
/retest |
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.
Provided comments. After all conversations are resolved, LGTM
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
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.
General notes:
- There's a lot of misc
print
statements that need to be removed.
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
No-Issue Signed-off-by: James Tanner <tanner.jc@gmail.com>
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.
A couple of minor nitpicks, no deal breakers.
Before this goes into production I do want to see:
- A proper RBAC implementation for legacy namespace
- Replacement for the
username==legacy_namespace.name
logic to determine if a user owns a namespace - Some kind of system to migrate the existing users from galaxy into galaxy_ng so we don't have to rely on the user's github username matching one of the namespaces
permission_classes = [LegacyAccessPolicy] | ||
authentication_classes = GALAXY_AUTHENTICATION_CLASSES | ||
|
||
def destroy(self, request, pk=None): |
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 think the default DRF method should work for this
"""A list of legacy roles.""" | ||
|
||
queryset = LegacyRole.objects.all().order_by('full_metadata__created') | ||
ordering_fields = ('full_metadata__created') |
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.
do you need to order this in 3 places?
What is this PR doing:
Issue: AAH-1812
Reviewers must know:
Notes:
PR Author: Add a QE reviewer (exceptions);
Reviewers: look for sound code, no code smells, docs & test coverage
Merger: When merging, include the Jira issue link in the squashed commit