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

added conditon to allow only add admin to access add club #21

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

Conversation

shanky2003g
Copy link

Initially club member and convenor both were able to access user add page by simply modifying the url to http://127.0.0.1:8000/user_add/. Added an if condition to check whether user has an action to perform user_add else will redirect to error page.

Copy link
Owner

@mittal-parth mittal-parth left a comment

Choose a reason for hiding this comment

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

Thanks!

  1. Create a different branch from your main and submit a new PR. The convention is to do that and not from a fork's main to my main.
  2. Rename the title to be more concise. Also, tense should be 'add' and not added

messages.success(request, 'Club added successfully!')
return redirect('/')
# Adding a new club
if can_user_access(request.user.id, 'club_add'): # preventing convenor and member to access add clubs
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if can_user_access(request.user.id, 'club_add'): # preventing convenor and member to access add clubs
if can_user_access(request.user.id, 'club_add'):

We need not hardcode who has access in the comments.

messages.success(request, 'Club added successfully!')
return redirect('/')
else:
return redirect('error_page')
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
return redirect('error_page')
return render(request, 'error_page.html')

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