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

Require first & last name on project requests #637

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

helbashandy
Copy link
Collaborator

Description

  • Adds a permissions.py file as an initial step to centeralize
    permissions across the app.

  • Adds first and last name check to the test_func on requests to create or join a project.

  • Creates a generic wrapper decorator to be used around test_func to
    allow gradual progressive refactor of test_func towards a more
    centralized and modular permissions management solution.

Fixes #605

Type of change

**** Please delete options that are not relevant. ****

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

**** Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration ****

  1. Last name was removed from a user account and saved.
  2. Clicked on project and attempted to create project - Saw the error message.
  3. Clicked on project and attempted to join a project - Saw the error message.

PR Self Evaluation

strikethrough things that don’t make sense for your PR

  • My code follows the agreed upon best practices
  • I have commented my code, particularly in hard-to-understand areas
    - [ ] I have made corresponding changes to the documentation (if needed)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in the appropriate modules
  • I have performed a self-review of my own code

@helbashandy helbashandy requested a review from matthew-li August 19, 2024 15:01
@helbashandy
Copy link
Collaborator Author

Hi @matthew-li - Please note that I've updated the implementation by replacing the use of permission_classes that I had initially in mind with the permission_required decorator.

This change was made for two key reasons:

  • permission_classes are specific to DRF and are not compatible with regular Django views.
  • I saw this as an opportunity to explore the centralized approach to permissions management. By using the permission_required decorator, we can seamlessly integrate it with our existing test_func methods while also passing any common permissions from utils/permissions.py. This approach will facilitate a gradual refactor of our test_func logic across the app progressively.

:return:
"""
if request.user.first_name == '' or request.user.last_name == '':
messages.error(request, 'You must set your first and last name on your account before you can make requests.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Link directly to the update form within the message?

adeng27 and others added 18 commits September 26, 2024 15:06
 * Adds a permissions.py file as an initial step to centeralize
 permissions across the app.

 * Adds first and last name check to the test_func on requests to create or join a project.

 * Creates a generic wrapper decorator to be used around test_func to
 allow gradual progressive refactor of test_func towards a more
 centralized and modular permissions management solution.

 * Adds test for request creation

 Fixes #605
@helbashandy helbashandy force-pushed the issue-605-require-first-last-name branch from 743ad96 to ea63ec3 Compare October 8, 2024 20:02
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.

3 participants