Skip to content

Comments

Chore: Fix hasRole warning#23914

Merged
ggazzo merged 7 commits intodevelopfrom
fix-hasrole-warning
Dec 10, 2021
Merged

Chore: Fix hasRole warning#23914
ggazzo merged 7 commits intodevelopfrom
fix-hasrole-warning

Conversation

@sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented Dec 9, 2021

Proposed changes (including videos or screenshots)

Fix hasRole warnings and a few imports pointing to /app/module/index .

W20211209-15:50:53.771(-3)? (STDERR) [WARN] RolesRaw.isUserInRoles: roles should be an array
W20211209-15:50:53.788(-3)? (STDERR) [WARN] RolesRaw.isUserInRoles: roles should be an array
W20211209-15:50:53.801(-3)? (STDERR) [WARN] RolesRaw.isUserInRoles: roles should be an array

A few other changes were made:

  • hasRoleAsync was renamed to hasAnyRoleAsync .. to make it more clear that it returns true if the user has any of the roles provided
  • hasRole now accepts only a single role. to check multiple roles at once hasAnyRole or hasAnyRoleAsync should be used instead
  • Created its sync version called hasAnyRole
  • Changed places using hasRole that were already async to use the async version of the function

Issue(s)

Steps to test or reproduce

Further comments

@sampaiodiego sampaiodiego requested review from a team December 9, 2021 18:57
@lgtm-com
Copy link

lgtm-com bot commented Dec 9, 2021

This pull request introduces 1 alert when merging 7967781 into 8c0a6c5 - view on LGTM.com

new alerts:

  • 1 for Invocation of non-function

@lgtm-com
Copy link

lgtm-com bot commented Dec 9, 2021

This pull request introduces 1 alert when merging 8cd3baa into 8c0a6c5 - view on LGTM.com

new alerts:

  • 1 for Invocation of non-function

@lgtm-com
Copy link

lgtm-com bot commented Dec 10, 2021

This pull request introduces 1 alert when merging cd5a404 into 2bcb2ed - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@ggazzo ggazzo merged commit 8b58e45 into develop Dec 10, 2021
@ggazzo ggazzo deleted the fix-hasrole-warning branch December 10, 2021 13:05
@sampaiodiego sampaiodiego mentioned this pull request Dec 29, 2021
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.

2 participants