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

Refactor roleneed roleid #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alejandromumo
Copy link
Member

@alejandromumo alejandromumo commented Jul 27, 2023

closes inveniosoftware/invenio-access#205

ATTENTION

This PR must be merged first.

If you review this PR, review only the last commit.

def query_filter(self, identity=None, **kwargs):
"""Filters for current identity."""
for need in identity.provides:
if need.method == "role" and need.value == self.action.value:
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be

Suggested change
if need.method == "role" and need.value == self.action.value:
if need.method == "action" and need.value == self.action.value:

?

Copy link
Member

Choose a reason for hiding this comment

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

I see from the tests that you add a role with a name equal to the action name....I might miss some knowledge on how the action roles work, but isn't creating an action redundant if we only check a role?

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolving as it was explained IRL

invenio_users_resources/services/users/service.py Outdated Show resolved Hide resolved
"username": "user_moderator",
"email": "user_moderator@inveniosoftware.org",
"profile": {
"full_name": "Mr",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"full_name": "Mr",
"full_name": "Moderator",

tests/conftest.py Outdated Show resolved Hide resolved
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.

roles: modify RoleNeed creation by role name to role id
2 participants