Skip to content

Commit

Permalink
Require first & last name on project requests
Browse files Browse the repository at this point in the history
 * 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
  • Loading branch information
helbashandy committed Oct 8, 2024
1 parent 1f76b06 commit ea63ec3
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 20 deletions.
4 changes: 2 additions & 2 deletions bootstrap/development/docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ Note that these steps must be run from the root directory of the repo.
6. Start the application stack. Specify a unique Docker [project name](https://docs.docker.com/compose/project-name/) so that resources are placed within a Docker namespace. Examples: "brc-dev", "lrc-dev".

```bash
export DOCKER_PROJECT_NAME=brc-dev
` export DOCKER_PROJECT_NAME=brc-dev
docker-compose \
-f bootstrap/development/docker/docker-compose.yml \
-p $DOCKER_PROJECT_NAME \
up
up`
```

Notes:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
class TestSavioProjectRequestWizard(TestBase):
"""A class for testing SavioProjectRequestWizard."""


@enable_deployment('BRC')
def setUp(self):
"""Set up test data."""
Expand All @@ -27,18 +28,20 @@ def request_url():
project."""
return reverse('new-project-request')

@enable_deployment('BRC')
def test_post_creates_request(self):
"""Test that a POST request creates a
SavioProjectAllocationRequest."""
self.assertEqual(SavioProjectAllocationRequest.objects.count(), 0)
self.assertEqual(Project.objects.count(), 0)

computing_allowance = self.get_predominant_computing_allowance()
allocation_period = get_current_allowance_year_period()
def get_form_preparation_data(self, computing_allowance=None, allocation_period=None, pi=None, name='name',
title='title', description='a' * 20, scope_and_intent='b' * 20,
computational_aspects='c' * 20):
"""Prepare the form data for the wizard."""
if computing_allowance is None:
computing_allowance = self.get_predominant_computing_allowance()
if allocation_period is None:
allocation_period = get_current_allowance_year_period()
if pi is None:
pi = self.user

view_name = 'savio_project_request_wizard'
current_step_key = f'{view_name}-current_step'

computing_allowance_form_data = {
'0-computing_allowance': computing_allowance.pk,
current_step_key: '0',
Expand All @@ -48,22 +51,22 @@ def test_post_creates_request(self):
current_step_key: '1',
}
existing_pi_form_data = {
'2-PI': self.user.pk,
'2-PI': pi.pk,
current_step_key: '2',
}
pool_allocations_data = {
'6-pool': False,
current_step_key: '6',
}
details_data = {
'8-name': 'name',
'8-title': 'title',
'8-description': 'a' * 20,
'8-name': name,
'8-title': title,
'8-description': description,
current_step_key: '8',
}
survey_data = {
'10-scope_and_intent': 'b' * 20,
'10-computational_aspects': 'c' * 20,
'10-scope_and_intent': scope_and_intent,
'10-computational_aspects': computational_aspects,
current_step_key: '10',
}
form_data = [
Expand All @@ -74,6 +77,19 @@ def test_post_creates_request(self):
details_data,
survey_data,
]
return form_data, details_data, survey_data

@enable_deployment('BRC')
def test_post_creates_request(self):
"""Test that a POST request creates a
SavioProjectAllocationRequest."""
self.assertEqual(SavioProjectAllocationRequest.objects.count(), 0)
self.assertEqual(Project.objects.count(), 0)

computing_allowance = self.get_predominant_computing_allowance()
allocation_period = get_current_allowance_year_period()

form_data, details_data, survey_data = self.get_form_preparation_data()

url = self.request_url()
for i, data in enumerate(form_data):
Expand Down Expand Up @@ -110,4 +126,29 @@ def test_post_creates_request(self):
survey_data['10-computational_aspects'])
self.assertEqual(request.status.name, 'Under Review')

# TODO
@enable_deployment('BRC')
def test_post_fails_without_user_last_name(self):
"""Test that a user without a last name cannot create a request."""

self.user.last_name = ''
self.user.save()

self.assertEqual(SavioProjectAllocationRequest.objects.count(), 0)
self.assertEqual(Project.objects.count(), 0)

form_data, _, _ = self.get_form_preparation_data()

url = self.request_url()
for i, data in enumerate(form_data):
response = self.client.post(url, data, follow=True)
print(response.content)
self.assertEqual(response.status_code, HTTPStatus.OK)
# Once the error message is displayed, we break to check if the request was created
if b'You must set your first and last name on your account' in response.content:
break

# Assert that the request was not created (1 from the test above)
requests = SavioProjectAllocationRequest.objects.all()
self.assertEqual(requests.count(), 1)
projects = Project.objects.all()
self.assertEqual(projects.count(), 1)
2 changes: 2 additions & 0 deletions coldfront/core/project/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from coldfront.core.utils.common import (get_domain_url, import_from_settings)
from coldfront.core.utils.email.email_strategy import EnqueueEmailStrategy
from coldfront.core.utils.mail import send_email, send_email_template
from coldfront.core.utils.permissions import permissions_required, check_first_last_name

from flags.state import flag_enabled

Expand Down Expand Up @@ -630,6 +631,7 @@ class ProjectCreateView(LoginRequiredMixin, UserPassesTestMixin, CreateView):
template_name_suffix = '_create_form'
fields = ['title', 'description', 'field_of_science', ]

@permissions_required(check_first_last_name)
def test_func(self):
""" UserPassesTestMixin Tests"""
if self.request.user.is_superuser:
Expand Down
2 changes: 2 additions & 0 deletions coldfront/core/project/views_/join_views/request_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from coldfront.core.project.views import ProjectListView
from coldfront.core.user.utils_.host_user_utils import is_lbl_employee
from coldfront.core.user.utils_.host_user_utils import needs_host
from coldfront.core.utils.permissions import permissions_required, check_first_last_name


logger = logging.getLogger(__name__)
Expand All @@ -38,6 +39,7 @@ class ProjectJoinView(LoginRequiredMixin, UserPassesTestMixin, TemplateView):

logger = logging.getLogger(__name__)

@permissions_required(check_first_last_name)
def test_func(self):
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
user_obj = self.request.user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
from coldfront.core.user.utils import access_agreement_signed
from coldfront.core.utils.common import session_wizard_all_form_data
from coldfront.core.utils.common import utc_now_offset_aware
from coldfront.core.utils.permissions import permissions_required, check_first_last_name


from django.conf import settings
from django.contrib import messages
Expand All @@ -58,6 +60,7 @@ class ProjectRequestView(LoginRequiredMixin, UserPassesTestMixin,
TemplateView):
template_name = 'project/project_request/project_request.html'

@permissions_required(check_first_last_name)
def test_func(self):
if self.request.user.is_superuser:
return True
Expand Down
3 changes: 3 additions & 0 deletions coldfront/core/project/views_/renewal_views/request_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
from coldfront.core.user.utils import access_agreement_signed
from coldfront.core.utils.common import session_wizard_all_form_data
from coldfront.core.utils.common import utc_now_offset_aware
from coldfront.core.utils.permissions import permissions_required, check_first_last_name


from decimal import Decimal

Expand All @@ -60,6 +62,7 @@ class AllocationRenewalLandingView(LoginRequiredMixin, UserPassesTestMixin,
TemplateView):
template_name = 'project/project_renewal/request_landing.html'

@permissions_required(check_first_last_name)
def test_func(self):
if self.request.user.is_superuser:
return True
Expand Down
38 changes: 38 additions & 0 deletions coldfront/core/utils/permissions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from functools import wraps
from django.contrib import messages
from django.urls import reverse
from django.utils.html import format_html



def permissions_required(*permissions):
"""
Decorator to check if a user has all specified permissions before allowing them to access a view.
The decorator is used to wrap a test_func from UserPassesTestMixin, which allows granular refactoring of
the permission check logic on test_func without having to change the permission logic.
:param permissions: variable number of functions where each returns True if the user has the permission, False otherwise
:return:
"""
def decorator(test_func):
@wraps(test_func)
def wrapper(view_instance, *args, **kwargs):
# Check if all provided permissions return True
if not all(permission(view_instance.request) for permission in permissions):
return False
return test_func(view_instance, *args, **kwargs)
return wrapper
return decorator


def check_first_last_name(request):
"""
Check if the user has set their first and last name on their account before allowing them to make requests.
:param request:
:return:
"""
if request.user.first_name == '' or request.user.last_name == '':
profile_url = request.build_absolute_uri(reverse('user-profile'))
messages.error(request, format_html(f'You must set your first and last name on your account before you can make requests. Update your profile <a href="{profile_url}">here</a>.'))
return False
return True
2 changes: 1 addition & 1 deletion coldfront/templates/common/messages.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<button type="button" class="close" data-dismiss="alert" aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
{{ message }}
{{ message|safe }}
</div>
{% endfor %}

Expand Down
2 changes: 1 addition & 1 deletion coldfront/templates/error_with_message.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<div class="row justify-content-center">
<div class="col-lg-10 col-xs-12">
<div class="alert alert-danger" role="alert">
{{ message }}
{{ message|safe }}
</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ tqdm==4.62.3
urllib3==1.24.2
user-agents==2.2.0
wcwidth==0.1.7
pytest-django

0 comments on commit ea63ec3

Please sign in to comment.