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

Related field query param. #1182

Merged
merged 7 commits into from
Mar 24, 2022

Conversation

newswangerd
Copy link
Member

@newswangerd newswangerd commented Mar 22, 2022

Description 🛠

Adds the related_fields query parameter and my_permissions to collection Namespaces.

Example:

GET http://localhost:5001/api/automation-hub/v3/namespaces/?include_related=my_permissions
    "data": [
        {
            "id": 1,
            "name": "test",
            "company": "",
            "email": "",
            "avatar_url": "",
            "description": "",
            "groups": [
                {
                    "id": 1,
                    "name": "system:partner-engineers",
                    "object_permissions": [
                        "change_namespace",
                        "upload_to_namespace"
                    ]
                }
            ],
            "related_fields": {
                "my_permissions": [
                    "galaxy.change_namespace",
                    "galaxy.upload_to_namespace"
                ]
            }
        },
        {
            "id": 2,
            "name": "test2",
            "company": "",
            "email": "",
            "avatar_url": "",
            "description": "",
            "groups": [],
            "related_fields": {
                "my_permissions": []
            }
        },
    ]

Reviewer Checklists 👀

Developer reviewer:

  • Code looks sound, good architectural decisions, no code smells
  • There is a Jira issue associated (note that "No-Issue" should be rarely used)
  • Tests are included in galaxy_ng/tests/integration or galaxy_ng/tests/functional, and they fully cover necessary test scenarios… or tests not needed

QE reviewer (exceptions):

  • Tests are included in galaxy_ng/tests/integration or galaxy_ng/tests/functional, and they fully cover necessary test scenarios… or tests not needed
  • PR meets applicable Acceptance Criteria for associated Jira issue

Note: when merging, include the Jira issue link in the squashed commit

@newswangerd newswangerd marked this pull request as draft March 22, 2022 17:09
@newswangerd newswangerd marked this pull request as ready for review March 22, 2022 20:48
@newswangerd
Copy link
Member Author

/retest

@newswangerd newswangerd reopened this Mar 23, 2022
@newswangerd
Copy link
Member Author

/retest

@@ -86,7 +86,7 @@ def _create_group(scope, name, users=None, perms=[]):
return group

@staticmethod
def _create_namespace(name, groups=None):
def _create_namespace(name, groups=[]):
Copy link
Contributor

Choose a reason for hiding this comment

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

default parameter values are globally shared, even between calls. I think it looks safe in the use here, but its generally a good idea to avoid mutable default values. It would be really easy for a future change to mutate this and that mutation leak into all other calls to the method.

Maybe revert it and add if groups is None: groups = []?

Copy link
Member

Choose a reason for hiding this comment

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

it can be

def _create_namespace(name, groups=None):
    groups = groups or []

@netlify
Copy link

netlify bot commented Mar 24, 2022

Deploy Preview for galaxyng ready!

Name Link
🔨 Latest commit f535355
🔍 Latest deploy log https://app.netlify.com/sites/galaxyng/deploys/623c8d518801140009e523c4
😎 Deploy Preview https://deploy-preview-1182--galaxyng.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@brumik
Copy link
Contributor

brumik commented Mar 28, 2022

@newswangerd Is it possible to get my_permissions when asking for the collection like this?

http://localhost:8002/api/automation-hub/_ui/v1/repo/published/footest/fooh/?include_related=my_permissions

This query gives the collection with the namespace field in it, but the my_permissions are empty.

@newswangerd
Copy link
Member Author

newswangerd commented Mar 28, 2022

@newswangerd Is it possible to get my_permissions when asking for the collection like this?

http://localhost:8002/api/automation-hub/_ui/v1/repo/published/footest/fooh/?include_related=my_permissions

This query gives the collection with the namespace field in it, but the my_permissions are empty.

@brumik #1188.

This doesn't currently work on the collection list view fyi. There's an issue with pulp ansible that's preventing it from parsing ?include_related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants