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

Fix dudoc_has_perm #622

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix dudoc_has_perm #622

wants to merge 2 commits into from

Conversation

moesoha
Copy link
Member

@moesoha moesoha commented Jan 1, 2022

No description provided.

vj4/handler/base.py Outdated Show resolved Hide resolved
@undefined-moe
Copy link
Member

I think we should use ROLE_DEFAULT if udoc has PRIV_USER_PROFILE.

@twd2
Copy link
Member

twd2 commented Jan 16, 2022

From original comment it seems like the intention was to fix the caller. From code reference all 3 callers (problem.py:242, problem.py:613, problem.py:633) will not pass in null dudoc. In this case, we can remove the comment and null check. The modified code should look like this:

def dudoc_has_perm(self, ...):
  if not udoc:
    role = builtin.ROLE_GUEST
  else:
    role = dudoc.get('role', builtin.ROLE_DEFAULT)
  ...

There are also a lot of uses of dudoc_has_perm from templates, e.g., ui/templates/user_detail.html:69 and ui/templates/components/user.html:21 (the render_inline macro, which is used everywhere). I'm not sure whether they will pass in null dudoc. So I prefer to keep the comment and the null check.

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.

None yet

4 participants