Skip to content

Refactor roleneed roleid#94

Open
alejandromumo wants to merge 1 commit intoinveniosoftware:masterfrom
alejandromumo:refactor_roleneed_roleid
Open

Refactor roleneed roleid#94
alejandromumo wants to merge 1 commit intoinveniosoftware:masterfrom
alejandromumo:refactor_roleneed_roleid

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

"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",

@alejandromumo alejandromumo force-pushed the refactor_roleneed_roleid branch from 22325a2 to c0ae04e Compare August 9, 2023 13:52
@alejandromumo alejandromumo force-pushed the refactor_roleneed_roleid branch from c0ae04e to 5b5aade Compare August 9, 2023 13:53
@utnapischtim utnapischtim added this to v14 Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

roles: modify RoleNeed creation by role name to role id

2 participants