Change has_access and require_access signatures#3611
Merged
Conversation
…cts that require access, instead of their groups
ghost
assigned
arikfr
requested changes
Mar 26, 2019
| require_access(dict(groups), user, view_only) | ||
| object = lambda: None | ||
| object.groups = dict(groups) | ||
| require_access(object, user, view_only) |
Member
There was a problem hiding this comment.
This feels wrong. Let's add require_access_to_groups that takes groups?
Btw, object is a reserved word.
Contributor
Author
There was a problem hiding this comment.
Btw, object is a reserved word.
You sure? I would expect things to not work if it were a reserved keyword.
Member
There was a problem hiding this comment.
>>> object
<type 'object'>
Python is funny in this way. id is reserved too and often we override it locally.
Member
There was a problem hiding this comment.
I guess that reserved word is not the correct term here. Just a built-in object.
Contributor
Author
There was a problem hiding this comment.
Yeah, while not reserved, it might cause confusion. I'll rename.
rauchy
force-pushed
the
change-has-access-signature
branch
from
March 27, 2019 07:10
53269ce to
1f16998
Compare
arikfr
approved these changes
Mar 28, 2019
Member
|
|
harveyrendell
pushed a commit
to pushpay/redash
that referenced
this pull request
Nov 14, 2019
* change has_access and require_access signatures to work with the objects that require access, instead of their groups * rename `object` to `obj` * support both objects and group dicts in `has_access` and `require_access` * simplify permission tests once `has_access` accepts groups
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was actually part of #3495, but it's spread across many files and is a simple refactor so I just wanted to take it out to it's own PR in order to clean up #3495.
What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
#3495
Mobile & Desktop Screenshots/Recordings (if there are UI changes)