From f8f0ac6c21ffdd82b726c932773a7d65c3354d9b Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Tue, 16 Apr 2024 09:53:21 -0700 Subject: [PATCH 01/10] added initial field to pi changing form --- coldfront/core/project/forms.py | 15 +++++++++++++++ .../views_/new_project_views/approval_views.py | 6 ++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index 0d4afb0f50..f4a3ef27e0 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -7,6 +7,8 @@ from coldfront.core.project.models import (Project, ProjectReview, ProjectUserRoleChoice) +from coldfront.core.user.models import UserProfile +from django.contrib.auth.models import User from coldfront.core.user.utils_.host_user_utils import eligible_host_project_users from coldfront.core.utils.common import import_from_settings from coldfront.core.resource.utils import get_compute_resource_names @@ -211,6 +213,11 @@ class ReviewDenyForm(forms.Form): class ReviewStatusForm(forms.Form): + pi = forms.ModelChoiceField( + queryset=User.objects.filter(userprofile__is_pi=True).order_by('first_name', 'last_name'), + label='Principal Investigator', + required=False) + status = forms.ChoiceField( choices=( ('', 'Select one.'), @@ -231,6 +238,14 @@ class ReviewStatusForm(forms.Form): required=False, widget=forms.Textarea(attrs={'rows': 3})) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['pi'].label_from_instance = self.label_from_instance + + @staticmethod + def label_from_instance(obj): + return f'{obj.first_name} {obj.last_name} ({obj.email})' + def clean(self): cleaned_data = super().clean() status = cleaned_data.get('status', 'Pending') diff --git a/coldfront/core/project/views_/new_project_views/approval_views.py b/coldfront/core/project/views_/new_project_views/approval_views.py index d4dcb0899f..967869cb26 100644 --- a/coldfront/core/project/views_/new_project_views/approval_views.py +++ b/coldfront/core/project/views_/new_project_views/approval_views.py @@ -615,7 +615,7 @@ def dispatch(self, request, *args, **kwargs): if redirect is not None: return redirect return super().dispatch(request, *args, **kwargs) - + def form_valid(self, form): form_data = form.cleaned_data status = form_data['status'] @@ -631,7 +631,8 @@ def form_valid(self, form): if status == 'Denied': runner = ProjectDenialRunner(self.request_obj) runner.run() - + + self.request_obj.pi = form_data['pi'] self.request_obj.save() message = ( @@ -651,6 +652,7 @@ def get_context_data(self, **kwargs): def get_initial(self): initial = super().get_initial() eligibility = self.request_obj.state['eligibility'] + initial['pi'] = self.request_obj.pi initial['status'] = eligibility['status'] initial['justification'] = eligibility['justification'] return initial From b6f32ca277116e2624175c581faf85411ec31c41 Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Tue, 23 Apr 2024 10:37:20 -0700 Subject: [PATCH 02/10] added optional new pi functionality to approval view --- coldfront/core/project/forms.py | 24 ++++++++++---- .../forms_/new_project_forms/request_forms.py | 22 +++++++++++-- .../new_project_views/approval_views.py | 32 ++++++++++++++++--- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index f4a3ef27e0..59eadbb2a2 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -5,6 +5,7 @@ from django.shortcuts import get_object_or_404 from django.db.models import Q +import coldfront.core.project.forms_.new_project_forms.request_forms as request_forms from coldfront.core.project.models import (Project, ProjectReview, ProjectUserRoleChoice) from coldfront.core.user.models import UserProfile @@ -211,13 +212,15 @@ class ReviewDenyForm(forms.Form): widget=forms.Textarea(attrs={'rows': 3})) -class ReviewStatusForm(forms.Form): - - pi = forms.ModelChoiceField( - queryset=User.objects.filter(userprofile__is_pi=True).order_by('first_name', 'last_name'), - label='Principal Investigator', - required=False) +class ReviewStatusForm(request_forms.SavioProjectNewPIForm, + request_forms.SavioProjectExistingPIForm): + # PI = SavioProjectExistingPIForm.PI + # first_name = forms.CharField(max_length=30, required=True) + # middle_name = forms.CharField(max_length=30, required=False) + # last_name = forms.CharField(max_length=150, required=True) + # email = forms.EmailField(max_length=100, required=True) + status = forms.ChoiceField( choices=( ('', 'Select one.'), @@ -240,7 +243,14 @@ class ReviewStatusForm(forms.Form): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields['pi'].label_from_instance = self.label_from_instance + self.fields['PI'].label_from_instance = self.label_from_instance + self.fields['PI'].empty_label = 'New PI' + self.fields['PI'].help_text = ( + 'Please confirm the PI for this project. If the PI is not listed, ' + 'select "New PI" and provide the PI\'s information below.') + self.fields['first_name'].required = False + self.fields['last_name'].required = False + self.fields['email'].required = False @staticmethod def label_from_instance(obj): diff --git a/coldfront/core/project/forms_/new_project_forms/request_forms.py b/coldfront/core/project/forms_/new_project_forms/request_forms.py index 7029299ddf..42a33ecb28 100644 --- a/coldfront/core/project/forms_/new_project_forms/request_forms.py +++ b/coldfront/core/project/forms_/new_project_forms/request_forms.py @@ -1,6 +1,5 @@ from coldfront.core.allocation.forms import AllocationPeriodChoiceField from coldfront.core.allocation.models import AllocationPeriod -from coldfront.core.project.forms import DisabledChoicesSelectWidget from coldfront.core.project.models import Project from coldfront.core.project.utils_.new_project_utils import non_denied_new_project_request_statuses from coldfront.core.project.utils_.new_project_utils import pis_with_new_project_requests_pks @@ -144,6 +143,24 @@ def label_from_instance(self, obj): return f'{obj.first_name} {obj.last_name} ({obj.email})' +class DisabledChoicesSelectWidget(forms.Select): + + def __init__(self, *args, **kwargs): + self.disabled_choices = kwargs.pop('disabled_choices', set()) + super().__init__(*args, **kwargs) + + def create_option(self, name, value, label, selected, index, subindex=None, + attrs=None): + option = super().create_option( + name, value, label, selected, index, subindex=subindex, + attrs=attrs) + try: + if int(str(value)) in self.disabled_choices: + option['attrs']['disabled'] = True + except Exception: + pass + return option + class SavioProjectExistingPIForm(forms.Form): PI = PIChoiceField( @@ -231,7 +248,8 @@ class SavioProjectNewPIForm(forms.Form): def clean_email(self): cleaned_data = super().clean() email = cleaned_data['email'].lower() - if (User.objects.filter(username=email).exists() or + # "email and" for project.forms.ReviewStatusForm + if email and (User.objects.filter(username=email).exists() or User.objects.filter(email=email).exists()): raise forms.ValidationError( 'A user with that email address already exists.') diff --git a/coldfront/core/project/views_/new_project_views/approval_views.py b/coldfront/core/project/views_/new_project_views/approval_views.py index 967869cb26..74f9726ad5 100644 --- a/coldfront/core/project/views_/new_project_views/approval_views.py +++ b/coldfront/core/project/views_/new_project_views/approval_views.py @@ -19,6 +19,7 @@ from coldfront.core.project.utils_.new_project_utils import send_project_request_pooling_email from coldfront.core.project.utils_.new_project_utils import VectorProjectProcessingRunner from coldfront.core.project.utils_.new_project_utils import vector_request_state_status +from django.contrib.auth.models import User from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.utils.common import display_time_zone_current_date @@ -33,7 +34,7 @@ from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.auth.mixins import UserPassesTestMixin -from django.db import transaction +from django.db import IntegrityError, transaction from django.db.models import Q from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 @@ -600,6 +601,8 @@ class SavioProjectReviewEligibilityView(LoginRequiredMixin, template_name = ( 'project/project_request/savio/project_review_eligibility.html') + logger = logging.getLogger(__name__) + def test_func(self): """UserPassesTestMixin tests.""" if self.request.user.is_superuser: @@ -632,8 +635,29 @@ def form_valid(self, form): runner = ProjectDenialRunner(self.request_obj) runner.run() - self.request_obj.pi = form_data['pi'] - self.request_obj.save() + if form_data['PI'] != self.request_obj.pi: + if form_data['PI'] is not None: + self.request_obj.pi = form_data['PI'] + self.request_obj.save() + elif all([form_data['first_name'], + form_data['last_name'], + form_data['email']]): + try: + self.request_obj.pi = User.objects.create( + username=form_data['email'], + first_name=form_data['first_name'], + last_name=form_data['last_name'], + email=form_data['email'], + is_active=True) + self.request_obj.pi.save() + self.request_obj.save() + except IntegrityError as e: + self.logger.error(f'User {form_data["email"]} unexpectedly exists.') + raise e + else: + message = 'PI information is incomplete.' + messages.error(self.request, message) + return self.form_invalid(form) message = ( f'Eligibility status for request {self.request_obj.pk} has been ' @@ -652,7 +676,7 @@ def get_context_data(self, **kwargs): def get_initial(self): initial = super().get_initial() eligibility = self.request_obj.state['eligibility'] - initial['pi'] = self.request_obj.pi + initial['PI'] = self.request_obj.pi initial['status'] = eligibility['status'] initial['justification'] = eligibility['justification'] return initial From 8d4bb2bfb9521249b4f93fef8eabd5f9d6343075 Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Tue, 30 Apr 2024 10:12:23 -0700 Subject: [PATCH 03/10] fixed testing --- coldfront/core/utils/tests/test_mou_notify_upload_download.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coldfront/core/utils/tests/test_mou_notify_upload_download.py b/coldfront/core/utils/tests/test_mou_notify_upload_download.py index 73ed9656a8..1ce6b6afd4 100644 --- a/coldfront/core/utils/tests/test_mou_notify_upload_download.py +++ b/coldfront/core/utils/tests/test_mou_notify_upload_download.py @@ -171,7 +171,7 @@ def edit_extra_fields_url(pk): def test_new_project(self): """Test that the MOU notification task, MOU upload, and MOU download features work as expected.""" - eligibility = { 'status': 'Approved' } + eligibility = { 'PI': self.request.pi, 'status': 'Approved' } readiness = { 'status': 'Approved' } extra_fields = { 'course_name': 'TEST 101', From 68cb5e6585a06184175386b2d9e7b4380f1f119b Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Mon, 7 Oct 2024 12:58:25 -0700 Subject: [PATCH 04/10] fixed request_obj saving --- .../core/project/views_/new_project_views/approval_views.py | 1 + coldfront/core/utils/tests/test_mou_notify_upload_download.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/coldfront/core/project/views_/new_project_views/approval_views.py b/coldfront/core/project/views_/new_project_views/approval_views.py index 914e9c3824..a730f65ae7 100644 --- a/coldfront/core/project/views_/new_project_views/approval_views.py +++ b/coldfront/core/project/views_/new_project_views/approval_views.py @@ -650,6 +650,7 @@ def form_valid(self, form): messages.error(self.request, message) return self.form_invalid(form) + self.request_obj.save() message = ( f'Eligibility status for request {self.request_obj.pk} has been ' f'set to {status}.') diff --git a/coldfront/core/utils/tests/test_mou_notify_upload_download.py b/coldfront/core/utils/tests/test_mou_notify_upload_download.py index 1ce6b6afd4..5a26caca33 100644 --- a/coldfront/core/utils/tests/test_mou_notify_upload_download.py +++ b/coldfront/core/utils/tests/test_mou_notify_upload_download.py @@ -171,7 +171,7 @@ def edit_extra_fields_url(pk): def test_new_project(self): """Test that the MOU notification task, MOU upload, and MOU download features work as expected.""" - eligibility = { 'PI': self.request.pi, 'status': 'Approved' } + eligibility = { 'PI': self.request.pi.pk, 'status': 'Approved' } readiness = { 'status': 'Approved' } extra_fields = { 'course_name': 'TEST 101', From 11665b40a46dbc9972ae0d0fe48e965344a4b024 Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Mon, 28 Oct 2024 10:40:01 -0700 Subject: [PATCH 05/10] refactored forms and help text --- coldfront/core/project/forms.py | 141 +++++++++++++++--- .../forms_/new_project_forms/request_forms.py | 25 +--- .../savio/project_existing_pi.html | 12 +- .../savio/project_review_eligibility.html | 14 ++ .../new_project_views/approval_views.py | 27 +++- 5 files changed, 164 insertions(+), 55 deletions(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index 59eadbb2a2..f657e2ee6c 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -1,19 +1,22 @@ -import datetime - from django import forms from django.core.validators import MinLengthValidator from django.shortcuts import get_object_or_404 from django.db.models import Q -import coldfront.core.project.forms_.new_project_forms.request_forms as request_forms from coldfront.core.project.models import (Project, ProjectReview, ProjectUserRoleChoice) +from coldfront.core.project.utils_.new_project_utils import non_denied_new_project_request_statuses, pis_with_new_project_requests_pks, project_pi_pks +from coldfront.core.project.utils_.renewal_utils import non_denied_renewal_request_statuses, pis_with_renewal_requests_pks +from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.user.models import UserProfile from django.contrib.auth.models import User from coldfront.core.user.utils_.host_user_utils import eligible_host_project_users from coldfront.core.utils.common import import_from_settings from coldfront.core.resource.utils import get_compute_resource_names +import datetime +from flags.state import flag_enabled + EMAIL_DIRECTOR_PENDING_PROJECT_REVIEW_EMAIL = import_from_settings( 'EMAIL_DIRECTOR_PENDING_PROJECT_REVIEW_EMAIL') @@ -211,16 +214,20 @@ class ReviewDenyForm(forms.Form): required=True, widget=forms.Textarea(attrs={'rows': 3})) +class ReviewDenyForm(forms.Form): -class ReviewStatusForm(request_forms.SavioProjectNewPIForm, - request_forms.SavioProjectExistingPIForm): + justification = forms.CharField( + help_text=( + 'Provide reasoning for your decision. It will be included in the ' + 'notification email.'), + label='Justification', + validators=[MinLengthValidator(10)], + required=True, + widget=forms.Textarea(attrs={'rows': 3})) + + +class ReviewStatusForm(forms.Form): - # PI = SavioProjectExistingPIForm.PI - # first_name = forms.CharField(max_length=30, required=True) - # middle_name = forms.CharField(max_length=30, required=False) - # last_name = forms.CharField(max_length=150, required=True) - # email = forms.EmailField(max_length=100, required=True) - status = forms.ChoiceField( choices=( ('', 'Select one.'), @@ -241,16 +248,112 @@ class ReviewStatusForm(request_forms.SavioProjectNewPIForm, required=False, widget=forms.Textarea(attrs={'rows': 3})) + def clean(self): + cleaned_data = super().clean() + status = cleaned_data.get('status', 'Pending') + # Require justification for denials. + if status == 'Denied': + justification = cleaned_data.get('justification', '') + if not justification.strip(): + raise forms.ValidationError( + 'Please provide a justification for your decision.') + return cleaned_data + + +# note: from coldfront\core\project\forms_\new_project_forms\request_forms.py +class PIChoiceField(forms.ModelChoiceField): + + def label_from_instance(self, obj): + return f'{obj.first_name} {obj.last_name} ({obj.email})' + +class ReviewEligibilityForm(ReviewStatusForm): + + PI = PIChoiceField( + label='Principal Investigator', + queryset=User.objects.none(), + required=False, + widget=DisabledChoicesSelectWidget(), + empty_label='New PI', + help_text= 'Please confirm the PI for this project. If the PI is ' \ + 'listed, select them from the dropdown and do not fill ' \ + 'out the PI information fields. If the PI is not ' \ + 'listed, select "New PI" and provide the PI\'s ' \ + 'information below.') + first_name = forms.CharField(max_length=30, required=False) + middle_name = forms.CharField(max_length=30, required=False) + last_name = forms.CharField(max_length=150, required=False) + email = forms.EmailField(max_length=100, required=False) + + field_order=['PI', 'first_name', 'middle_name', 'last_name', 'email', + 'status', 'justification'] + def __init__(self, *args, **kwargs): + self.computing_allowance = kwargs.pop('computing_allowance', None) + self.allocation_period = kwargs.pop('allocation_period', None) super().__init__(*args, **kwargs) - self.fields['PI'].label_from_instance = self.label_from_instance - self.fields['PI'].empty_label = 'New PI' - self.fields['PI'].help_text = ( - 'Please confirm the PI for this project. If the PI is not listed, ' - 'select "New PI" and provide the PI\'s information below.') - self.fields['first_name'].required = False - self.fields['last_name'].required = False - self.fields['email'].required = False + if self.computing_allowance is not None: + self.computing_allowance = ComputingAllowance( + self.computing_allowance) + self.disable_pi_choices() + self.exclude_pi_choices() + + def clean(self): + cleaned_data = super().clean() + pi = self.cleaned_data['PI'] + if pi is not None and pi not in self.fields['PI'].queryset: + raise forms.ValidationError(f'Invalid selection {pi.username}.') + return cleaned_data + + def disable_pi_choices(self): + """Prevent certain Users, who should be displayed, from being + selected as PIs.""" + disable_user_pks = set() + + if self.computing_allowance.is_one_per_pi() and self.allocation_period: + # Disable any PI who has: + # (a) an existing Project with the allowance*, + # (b) a new project request for a Project with the allowance + # during the AllocationPeriod*, or + # (c) an allowance renewal request for a Project with the + # allowance during the AllocationPeriod*. + # * Projects/requests must have ineligible statuses. + resource = self.computing_allowance.get_resource() + project_status_names = ['New', 'Active', 'Inactive'] + disable_user_pks.update( + project_pi_pks( + computing_allowance=resource, + project_status_names=project_status_names)) + new_project_request_status_names = list( + non_denied_new_project_request_statuses().values_list( + 'name', flat=True)) + disable_user_pks.update( + pis_with_new_project_requests_pks( + self.allocation_period, + computing_allowance=resource, + request_status_names=new_project_request_status_names)) + renewal_request_status_names = list( + non_denied_renewal_request_statuses().values_list( + 'name', flat=True)) + disable_user_pks.update( + pis_with_renewal_requests_pks( + self.allocation_period, + computing_allowance=resource, + request_status_names=renewal_request_status_names)) + + if flag_enabled('LRC_ONLY'): + # On LRC, PIs must be LBL employees. + non_lbl_employees = set( + [user.pk for user in User.objects.all() + if not is_lbl_employee(user)]) + disable_user_pks.update(non_lbl_employees) + + self.fields['PI'].widget.disabled_choices = disable_user_pks + + def exclude_pi_choices(self): + """Exclude certain Users from being displayed as PI options.""" + # Exclude any user that does not have an email address or is inactive. + self.fields['PI'].queryset = User.objects.exclude( + Q(email__isnull=True) | Q(email__exact='') | Q(is_active=False)) @staticmethod def label_from_instance(obj): diff --git a/coldfront/core/project/forms_/new_project_forms/request_forms.py b/coldfront/core/project/forms_/new_project_forms/request_forms.py index 816978136d..53e9253d76 100644 --- a/coldfront/core/project/forms_/new_project_forms/request_forms.py +++ b/coldfront/core/project/forms_/new_project_forms/request_forms.py @@ -1,5 +1,6 @@ from coldfront.core.allocation.forms import AllocationPeriodChoiceField from coldfront.core.allocation.models import AllocationPeriod +from coldfront.core.project.forms import DisabledChoicesSelectWidget from coldfront.core.project.models import Project from coldfront.core.project.utils_.new_project_utils import non_denied_new_project_request_statuses from coldfront.core.project.utils_.new_project_utils import pis_with_new_project_requests_pks @@ -145,25 +146,6 @@ class PIChoiceField(forms.ModelChoiceField): def label_from_instance(self, obj): return f'{obj.first_name} {obj.last_name} ({obj.email})' - -class DisabledChoicesSelectWidget(forms.Select): - - def __init__(self, *args, **kwargs): - self.disabled_choices = kwargs.pop('disabled_choices', set()) - super().__init__(*args, **kwargs) - - def create_option(self, name, value, label, selected, index, subindex=None, - attrs=None): - option = super().create_option( - name, value, label, selected, index, subindex=subindex, - attrs=attrs) - try: - if int(str(value)) in self.disabled_choices: - option['attrs']['disabled'] = True - except Exception: - pass - return option - class SavioProjectExistingPIForm(forms.Form): PI = PIChoiceField( @@ -251,9 +233,8 @@ class SavioProjectNewPIForm(forms.Form): def clean_email(self): cleaned_data = super().clean() email = cleaned_data['email'].lower() - # "email and" for project.forms.ReviewStatusForm - if email and (User.objects.filter(username=email).exists() or - User.objects.filter(email=email).exists()): + if User.objects.filter(username=email).exists() or \ + User.objects.filter(email=email).exists(): raise forms.ValidationError( 'A user with that email address already exists.') diff --git a/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html b/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html index 2c68eb4456..88aa9050f5 100644 --- a/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html +++ b/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html @@ -64,11 +64,12 @@

{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator

{% endif %} - @@ -96,11 +97,4 @@

{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator

Step {{ wizard.steps.step1 }} of {{ wizard.steps.count }}

- - {% endblock %} diff --git a/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html b/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html index d76e7e6159..ab1dff2fdd 100644 --- a/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html +++ b/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html @@ -39,6 +39,13 @@

Cancel + @@ -46,4 +53,11 @@

{% include 'project/project_request/savio/project_request_extra_fields_modal.html' with extra_fields_form=extra_fields_form %} {% include 'project/project_request/savio/project_request_survey_modal.html' with survey_form=survey_form %} + + {% endblock %} diff --git a/coldfront/core/project/views_/new_project_views/approval_views.py b/coldfront/core/project/views_/new_project_views/approval_views.py index a730f65ae7..f4df2918ae 100644 --- a/coldfront/core/project/views_/new_project_views/approval_views.py +++ b/coldfront/core/project/views_/new_project_views/approval_views.py @@ -3,7 +3,7 @@ from coldfront.core.allocation.utils import calculate_service_units_to_allocate from coldfront.core.project.forms import MemorandumSignedForm from coldfront.core.project.forms import ReviewDenyForm -from coldfront.core.project.forms import ReviewStatusForm +from coldfront.core.project.forms import ReviewStatusForm, ReviewEligibilityForm from coldfront.core.project.forms_.new_project_forms.request_forms import NewProjectExtraFieldsFormFactory from coldfront.core.project.forms_.new_project_forms.request_forms import SavioProjectSurveyForm from coldfront.core.project.forms_.new_project_forms.approval_forms import SavioProjectReviewSetupForm @@ -21,6 +21,7 @@ from django.contrib.auth.models import User from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface +from coldfront.core.user.models import UserProfile from coldfront.core.utils.common import display_time_zone_current_date from coldfront.core.utils.common import format_date_month_name_day_year from coldfront.core.utils.common import utc_now_offset_aware @@ -588,7 +589,7 @@ def is_checklist_complete(self): class SavioProjectReviewEligibilityView(LoginRequiredMixin, UserPassesTestMixin, SavioProjectRequestMixin, FormView): - form_class = ReviewStatusForm + form_class = ReviewEligibilityForm template_name = ( 'project/project_request/savio/project_review_eligibility.html') @@ -643,10 +644,26 @@ def form_valid(self, form): self.request_obj.pi.save() self.request_obj.save() except IntegrityError as e: - self.logger.error(f'User {form_data["email"]} unexpectedly exists.') + self.logger.error(f'User {form_data["email"]} ' + 'unexpectedly exists.') raise e + try: + pi_profile = self.request_obj.pi.userprofile + except UserProfile.DoesNotExist as e: + self.logger.error( + f'User {form_data["email"]} unexpectedly has no ' + 'UserProfile.') + raise e + pi_profile.middle_name = form_data['middle_name'] or None + pi_profile.upgrade_request = utc_now_offset_aware() + pi_profile.save() else: - message = 'PI information is incomplete.' + incomplete_fields = [field for field in ['email', 'first_name', + 'last_name'] if not form_data[field]] + message = \ + f'Incomplete field(s): {", ".join(incomplete_fields)}. ' \ + 'Please specify a PI or provide all required fields ' \ + 'for a new PI.' messages.error(self.request, message) return self.form_invalid(form) @@ -1247,7 +1264,7 @@ def is_checklist_complete(self): class VectorProjectReviewEligibilityView(LoginRequiredMixin, UserPassesTestMixin, VectorProjectRequestMixin, FormView): - form_class = ReviewStatusForm + form_class = ReviewEligibilityForm template_name = ( 'project/project_request/vector/project_review_eligibility.html') From 79942c2d736d5ef1bd1571c2e91eb61b0fdf5116 Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Mon, 28 Oct 2024 10:53:52 -0700 Subject: [PATCH 06/10] fixed selectize --- .../project_request/savio/project_existing_pi.html | 10 ++++++++-- .../savio/project_review_eligibility.html | 14 ++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html b/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html index 88aa9050f5..66f16762be 100644 --- a/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html +++ b/coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html @@ -65,11 +65,10 @@

{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator

@@ -97,4 +96,11 @@

{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator

Step {{ wizard.steps.step1 }} of {{ wizard.steps.count }}

+ + {% endblock %} diff --git a/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html b/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html index ab1dff2fdd..7bb387a805 100644 --- a/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html +++ b/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html @@ -9,6 +9,8 @@ {% block content %} + +

Review Eligibility

@@ -40,10 +42,10 @@

Cancel @@ -52,12 +54,4 @@

{% include 'project/project_request/savio/project_request_extra_fields_modal.html' with extra_fields_form=extra_fields_form %} {% include 'project/project_request/savio/project_request_survey_modal.html' with survey_form=survey_form %} - - - {% endblock %} From 2ac3640db8ef3c5aac43d50808277a60f180544d Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Mon, 28 Oct 2024 10:58:05 -0700 Subject: [PATCH 07/10] fixed selectize wording --- coldfront/core/project/forms.py | 3 +-- .../project_request/savio/project_review_eligibility.html | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index f657e2ee6c..3fee69d24e 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -273,11 +273,10 @@ class ReviewEligibilityForm(ReviewStatusForm): queryset=User.objects.none(), required=False, widget=DisabledChoicesSelectWidget(), - empty_label='New PI', help_text= 'Please confirm the PI for this project. If the PI is ' \ 'listed, select them from the dropdown and do not fill ' \ 'out the PI information fields. If the PI is not ' \ - 'listed, select "New PI" and provide the PI\'s ' \ + 'listed, empty the field and provide the PI\'s ' \ 'information below.') first_name = forms.CharField(max_length=30, required=False) middle_name = forms.CharField(max_length=30, required=False) diff --git a/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html b/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html index 7bb387a805..0bae8bbaf0 100644 --- a/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html +++ b/coldfront/core/project/templates/project/project_request/savio/project_review_eligibility.html @@ -11,6 +11,7 @@ {% block content %} +

Review Eligibility

@@ -45,7 +46,7 @@

$('#id_PI').selectize({ create: false, sortField: 'text', - placeholder: 'Type to search.' + placeholder: 'Type to search or fill out the PI information below.' }) From 7674a3c6ec1097449a68854ada6f4a442c13cca5 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 3 Mar 2025 10:47:49 -0800 Subject: [PATCH 08/10] Refactor allowance eligibility into class --- .../load_allocation_renewal_requests.py | 23 +- coldfront/core/project/forms.py | 74 ++----- .../forms_/new_project_forms/request_forms.py | 55 +---- .../forms_/renewal_forms/request_forms.py | 46 ++-- ...computing_allowance_eligibility_manager.py | 204 ++++++++++++++++++ .../core/project/utils_/new_project_utils.py | 75 ------- .../core/project/utils_/renewal_utils.py | 69 +----- .../views_/renewal_views/request_views.py | 40 ++-- coldfront/core/user/utils_/host_user_utils.py | 15 +- 9 files changed, 309 insertions(+), 292 deletions(-) create mode 100644 coldfront/core/project/utils_/computing_allowance_eligibility_manager.py diff --git a/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py b/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py index ecc7598d8f..33d020ff49 100644 --- a/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py +++ b/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py @@ -17,8 +17,9 @@ from coldfront.core.allocation.models import AllocationRenewalRequestStatusChoice from coldfront.core.project.models import Project from coldfront.core.project.models import ProjectUser +from coldfront.core.project.utils_.computing_allowance_eligibility_manager import ComputingAllowanceEligibilityManager from coldfront.core.project.utils_.renewal_utils import AllocationRenewalProcessingRunner -from coldfront.core.project.utils_.renewal_utils import has_non_denied_renewal_request +from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.utils.common import add_argparse_dry_run_argument from coldfront.core.utils.common import utc_now_offset_aware @@ -110,6 +111,8 @@ def parse_input_file(self, file_path, allocation_period): seen_pi_emails = set() + interface = ComputingAllowanceInterface() + for input_dict in input_dicts: for key in input_dict: input_dict[key] = input_dict[key].strip() @@ -128,13 +131,21 @@ def parse_input_file(self, file_path, allocation_period): continue seen_pi_emails.add(pi_email) - # Validate that the PI does not already have a non-'Denied' + # Validate that the PI is eligible to make a # AllocationRenewalRequest for this period. pi = self._get_user_with_email(pi_email) - if (isinstance(pi, User) and - has_non_denied_renewal_request(pi, allocation_period)): - already_renewed.append(pi) - continue + if isinstance(pi, User): + computing_allowance = ComputingAllowance( + interface.allowance_from_code(post_project_name[:3])) + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + computing_allowance, + allocation_period=allocation_period) + is_user_eligible = \ + computing_allowance_eligibility_manager.is_user_eligible(pi) + if not is_user_eligible: + already_renewed.append(pi) + continue # Validate that the pre-Project exists, if given. The PI may have # previously not had a Project. diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index 3fee69d24e..c3279a1ced 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -5,17 +5,14 @@ from coldfront.core.project.models import (Project, ProjectReview, ProjectUserRoleChoice) -from coldfront.core.project.utils_.new_project_utils import non_denied_new_project_request_statuses, pis_with_new_project_requests_pks, project_pi_pks -from coldfront.core.project.utils_.renewal_utils import non_denied_renewal_request_statuses, pis_with_renewal_requests_pks +from coldfront.core.project.utils_.computing_allowance_eligibility_manager import ComputingAllowanceEligibilityManager from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance -from coldfront.core.user.models import UserProfile -from django.contrib.auth.models import User +from django.contrib.auth.models import User from coldfront.core.user.utils_.host_user_utils import eligible_host_project_users from coldfront.core.utils.common import import_from_settings from coldfront.core.resource.utils import get_compute_resource_names import datetime -from flags.state import flag_enabled EMAIL_DIRECTOR_PENDING_PROJECT_REVIEW_EMAIL = import_from_settings( @@ -203,17 +200,6 @@ class MemorandumSignedForm(forms.Form): required=True) -class ReviewDenyForm(forms.Form): - - justification = forms.CharField( - help_text=( - 'Provide reasoning for your decision. It will be included in the ' - 'notification email.'), - label='Justification', - validators=[MinLengthValidator(10)], - required=True, - widget=forms.Textarea(attrs={'rows': 3})) - class ReviewDenyForm(forms.Form): justification = forms.CharField( @@ -266,6 +252,7 @@ class PIChoiceField(forms.ModelChoiceField): def label_from_instance(self, obj): return f'{obj.first_name} {obj.last_name} ({obj.email})' + class ReviewEligibilityForm(ReviewStatusForm): PI = PIChoiceField( @@ -301,50 +288,23 @@ def clean(self): pi = self.cleaned_data['PI'] if pi is not None and pi not in self.fields['PI'].queryset: raise forms.ValidationError(f'Invalid selection {pi.username}.') + + # TODO: This doesn't include the clean_email method from + # SavioProjectNewPIForm. + return cleaned_data def disable_pi_choices(self): """Prevent certain Users, who should be displayed, from being selected as PIs.""" - disable_user_pks = set() - - if self.computing_allowance.is_one_per_pi() and self.allocation_period: - # Disable any PI who has: - # (a) an existing Project with the allowance*, - # (b) a new project request for a Project with the allowance - # during the AllocationPeriod*, or - # (c) an allowance renewal request for a Project with the - # allowance during the AllocationPeriod*. - # * Projects/requests must have ineligible statuses. - resource = self.computing_allowance.get_resource() - project_status_names = ['New', 'Active', 'Inactive'] - disable_user_pks.update( - project_pi_pks( - computing_allowance=resource, - project_status_names=project_status_names)) - new_project_request_status_names = list( - non_denied_new_project_request_statuses().values_list( - 'name', flat=True)) - disable_user_pks.update( - pis_with_new_project_requests_pks( - self.allocation_period, - computing_allowance=resource, - request_status_names=new_project_request_status_names)) - renewal_request_status_names = list( - non_denied_renewal_request_statuses().values_list( - 'name', flat=True)) - disable_user_pks.update( - pis_with_renewal_requests_pks( - self.allocation_period, - computing_allowance=resource, - request_status_names=renewal_request_status_names)) - - if flag_enabled('LRC_ONLY'): - # On LRC, PIs must be LBL employees. - non_lbl_employees = set( - [user.pk for user in User.objects.all() - if not is_lbl_employee(user)]) - disable_user_pks.update(non_lbl_employees) + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + self.computing_allowance, + allocation_period=self.allocation_period) + + disable_user_pks = \ + computing_allowance_eligibility_manager.get_ineligible_users( + pks_only=True) self.fields['PI'].widget.disabled_choices = disable_user_pks @@ -354,10 +314,6 @@ def exclude_pi_choices(self): self.fields['PI'].queryset = User.objects.exclude( Q(email__isnull=True) | Q(email__exact='') | Q(is_active=False)) - @staticmethod - def label_from_instance(obj): - return f'{obj.first_name} {obj.last_name} ({obj.email})' - def clean(self): cleaned_data = super().clean() status = cleaned_data.get('status', 'Pending') diff --git a/coldfront/core/project/forms_/new_project_forms/request_forms.py b/coldfront/core/project/forms_/new_project_forms/request_forms.py index 53e9253d76..144f8f28f3 100644 --- a/coldfront/core/project/forms_/new_project_forms/request_forms.py +++ b/coldfront/core/project/forms_/new_project_forms/request_forms.py @@ -2,23 +2,17 @@ from coldfront.core.allocation.models import AllocationPeriod from coldfront.core.project.forms import DisabledChoicesSelectWidget from coldfront.core.project.models import Project -from coldfront.core.project.utils_.new_project_utils import non_denied_new_project_request_statuses -from coldfront.core.project.utils_.new_project_utils import pis_with_new_project_requests_pks -from coldfront.core.project.utils_.new_project_utils import project_pi_pks -from coldfront.core.project.utils_.renewal_utils import non_denied_renewal_request_statuses -from coldfront.core.project.utils_.renewal_utils import pis_with_renewal_requests_pks +from coldfront.core.project.utils_.computing_allowance_eligibility_manager import ComputingAllowanceEligibilityManager from coldfront.core.resource.models import Resource from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.constants import BRCAllowances from coldfront.core.resource.utils_.allowance_utils.constants import LRCAllowances from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface -from coldfront.core.user.utils_.host_user_utils import is_lbl_employee from coldfront.core.utils.common import utc_now_offset_aware from django import forms from django.conf import settings from django.contrib.auth.models import User -from django.core.exceptions import ImproperlyConfigured from django.core.validators import MaxValueValidator from django.core.validators import MinLengthValidator from django.core.validators import MinValueValidator @@ -174,45 +168,14 @@ def clean(self): def disable_pi_choices(self): """Prevent certain Users, who should be displayed, from being selected as PIs.""" - disable_user_pks = set() - - if self.computing_allowance.is_one_per_pi() and self.allocation_period: - # Disable any PI who has: - # (a) an existing Project with the allowance*, - # (b) a new project request for a Project with the allowance - # during the AllocationPeriod*, or - # (c) an allowance renewal request for a Project with the - # allowance during the AllocationPeriod*. - # * Projects/requests must have ineligible statuses. - resource = self.computing_allowance.get_resource() - project_status_names = ['New', 'Active', 'Inactive'] - disable_user_pks.update( - project_pi_pks( - computing_allowance=resource, - project_status_names=project_status_names)) - new_project_request_status_names = list( - non_denied_new_project_request_statuses().values_list( - 'name', flat=True)) - disable_user_pks.update( - pis_with_new_project_requests_pks( - self.allocation_period, - computing_allowance=resource, - request_status_names=new_project_request_status_names)) - renewal_request_status_names = list( - non_denied_renewal_request_statuses().values_list( - 'name', flat=True)) - disable_user_pks.update( - pis_with_renewal_requests_pks( - self.allocation_period, - computing_allowance=resource, - request_status_names=renewal_request_status_names)) - - if flag_enabled('LRC_ONLY'): - # On LRC, PIs must be LBL employees. - non_lbl_employees = set( - [user.pk for user in User.objects.all() - if not is_lbl_employee(user)]) - disable_user_pks.update(non_lbl_employees) + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + self.computing_allowance, + allocation_period=self.allocation_period) + + disable_user_pks = \ + computing_allowance_eligibility_manager.get_ineligible_users( + pks_only=True) self.fields['PI'].widget.disabled_choices = disable_user_pks diff --git a/coldfront/core/project/forms_/renewal_forms/request_forms.py b/coldfront/core/project/forms_/renewal_forms/request_forms.py index 3f656fbfab..30c0080886 100644 --- a/coldfront/core/project/forms_/renewal_forms/request_forms.py +++ b/coldfront/core/project/forms_/renewal_forms/request_forms.py @@ -8,10 +8,7 @@ from coldfront.core.project.models import ProjectUser from coldfront.core.project.models import ProjectUserRoleChoice from coldfront.core.project.models import ProjectUserStatusChoice -from coldfront.core.project.utils_.new_project_utils import non_denied_new_project_request_statuses -from coldfront.core.project.utils_.new_project_utils import pis_with_new_project_requests_pks -from coldfront.core.project.utils_.renewal_utils import non_denied_renewal_request_statuses -from coldfront.core.project.utils_.renewal_utils import pis_with_renewal_requests_pks +from coldfront.core.project.utils_.computing_allowance_eligibility_manager import ComputingAllowanceEligibilityManager from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.project.utils_.renewal_survey import is_renewal_survey_completed @@ -19,7 +16,6 @@ from flags.state import flag_enabled from django import forms from django.utils.safestring import mark_safe -from django.core.validators import MinLengthValidator from django.core.exceptions import ValidationError @@ -68,34 +64,20 @@ def __init__(self, *args, **kwargs): def disable_pi_choices(self, pi_project_users): """Prevent certain of the given ProjectUsers, who should be displayed, from being selected for renewal.""" + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + self.computing_allowance, + allocation_period=self.allocation_period) + + disable_user_pks = \ + computing_allowance_eligibility_manager.get_ineligible_users( + is_renewal=True, pks_only=True) + disable_project_user_pks = set() - if self.computing_allowance.is_one_per_pi(): - # Disable any PI who has: - # (a) a new project request for a Project during the - # AllocationPeriod*, or - # (b) an AllocationRenewalRequest during the AllocationPeriod*. - # * Requests must have ineligible statuses. - resource = self.computing_allowance.get_resource() - disable_user_pks = set() - new_project_request_status_names = list( - non_denied_new_project_request_statuses().values_list( - 'name', flat=True)) - disable_user_pks.update( - pis_with_new_project_requests_pks( - self.allocation_period, - computing_allowance=resource, - request_status_names=new_project_request_status_names)) - renewal_request_status_names = list( - non_denied_renewal_request_statuses().values_list( - 'name', flat=True)) - disable_user_pks.update( - pis_with_renewal_requests_pks( - self.allocation_period, - computing_allowance=resource, - request_status_names=renewal_request_status_names)) - for project_user in pi_project_users: - if project_user.user.pk in disable_user_pks: - disable_project_user_pks.add(project_user.pk) + for project_user in pi_project_users: + if project_user.user.pk in disable_user_pks: + disable_project_user_pks.add(project_user.pk) + self.fields['PI'].widget.disabled_choices = disable_project_user_pks diff --git a/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py b/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py new file mode 100644 index 0000000000..660b441044 --- /dev/null +++ b/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py @@ -0,0 +1,204 @@ +from django.contrib.auth.models import User + +from flags.state import flag_enabled + +from coldfront.core.allocation.models import AllocationPeriod +from coldfront.core.allocation.models import AllocationRenewalRequest +from coldfront.core.project.models import ProjectUser +from coldfront.core.allocation.models import AllocationRenewalRequestStatusChoice +from coldfront.core.project.models import ProjectAllocationRequestStatusChoice +from coldfront.core.project.models import ProjectStatusChoice +from coldfront.core.project.models import SavioProjectAllocationRequest +from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance +from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface +from coldfront.core.user.utils_.host_user_utils import is_lbl_employee + + +class ComputingAllowanceEligibilityManager(object): + """A class for managing whether a User is eligible for a computing + allowance of a given type, optionally under a particular + AllocationPeriod. The following constraints apply: + - On LRC, computing allowances may only be allocated to PIs who + are LBL employees. + - If the computing allowance is limited to one per PI and is + periodic: + - A PI may not have an existing renewal request with the + same computing allowance type in the period. + - A PI may not have an existing new project request with + the same computing allowance type in the period. + - A PI may not already have an active Project with the + same computing allowance type, except in the case of + renewal. + """ + + def __init__(self, computing_allowance, allocation_period=None): + """Take a ComputingAllowance and an optional AllocationPeriod. + The period is required if the allowance is periodic.""" + assert isinstance(computing_allowance, ComputingAllowance) + self._computing_allowance = computing_allowance + + if allocation_period is not None: + assert isinstance(allocation_period, AllocationPeriod) + self._allocation_period = allocation_period + + if self._computing_allowance.is_periodic(): + assert self._allocation_period is not None + + def get_ineligible_users(self, is_renewal=False, pks_only=False): + """Return a QuerySet of Users who are ineligible for the + computing allowance. Optionally return a set of User primary + keys instead.""" + ineligible_user_pks = set() + + if flag_enabled('LRC_ONLY'): + non_lbl_employee_user_pks = { + user.pk + for user in User.objects.all() + if not is_lbl_employee(user)} + ineligible_user_pks.update(non_lbl_employee_user_pks) + + if self._computing_allowance.is_one_per_pi(): + + if self._computing_allowance.is_periodic(): + + existing_renewal_requests = self._existing_renewal_requests( + self._allocation_period) + renewal_request_pi_pks = existing_renewal_requests.values_list( + 'pi', flat=True) + ineligible_user_pks.update(set(renewal_request_pi_pks)) + + existing_new_project_requests = \ + self._existing_new_project_requests(self._allocation_period) + new_project_request_pi_pks = \ + existing_new_project_requests.values_list('pi', flat=True) + ineligible_user_pks.update(set(new_project_request_pi_pks)) + + if not is_renewal: + existing_pi_project_users = \ + self._existing_pi_project_users() + existing_pk_pks = existing_pi_project_users.values_list( + 'user', flat=True) + ineligible_user_pks.update(set(existing_pk_pks)) + + if pks_only: + return ineligible_user_pks + return User.objects.filter(pk__in=ineligible_user_pks) + + def is_user_eligible(self, user, is_renewal=False): + """Return whether the given User is eligible for the computing + allowance.""" + if flag_enabled('LRC_ONLY'): + if not is_lbl_employee(user): + return False + + if self._computing_allowance.is_one_per_pi(): + + if self._computing_allowance.is_periodic(): + + existing_renewal_requests = self._existing_renewal_requests( + user, self._allocation_period) + if existing_renewal_requests.exists(): + return False + + existing_new_project_requests = \ + self._existing_new_project_requests( + user, self._allocation_period) + if existing_new_project_requests.exists(): + return False + + if not is_renewal: + existing_pi_project_users = \ + self._existing_pi_project_users(user) + if existing_pi_project_users.exists(): + return False + + return True + + def _existing_new_project_requests(self, allocation_period, pi=None): + """Return a QuerySet of new project request objects: + - Under the given AllocationPeriod + - With the given computing allowance + - With a status that would render the associated PI + ineligible to make another one + - Optionally belonging to the given PI + """ + ineligible_statuses = self._ineligible_new_project_request_statuses() + kwargs = { + 'allocation_period': allocation_period, + 'computing_allowance': self._computing_allowance.get_resource(), + 'status__in': ineligible_statuses, + } + if pi is not None: + kwargs['pi'] = pi + return SavioProjectAllocationRequest.objects.filter(**kwargs) + + def _existing_pi_project_users(self, pi=None): + """Return a QuerySet of ProjectUser objects: + - With the given computing allowance + - With the "Principal Investigator" ProjectUser role + - With the "Active" ProjectUser status + - With a project status that would render the associated PIs + ineligible to be the PI of another project with the same + computing allowance + - Optionally belonging to the given PI + """ + computing_allowance_interface = ComputingAllowanceInterface() + project_prefix = computing_allowance_interface.code_from_name( + self._computing_allowance.get_name()) + + ineligible_project_statuses = self._ineligible_project_statuses() + kwargs = { + 'project__name__startswith': project_prefix, + 'role__name': 'Principal Investigator', + 'status__name': 'Active', + 'project__status__in': ineligible_project_statuses, + } + if pi is not None: + kwargs['user'] = pi + return ProjectUser.objects.filter(**kwargs) + + def _existing_renewal_requests(self, allocation_period, pi=None): + """Return a QuerySet of AllocationRenewalRequest objects: + - Under the given AllocationPeriod + - With the given computing allowance + - With a status that would render the associated PI + ineligible to make another one + - Optionally belonging to the given PI + """ + ineligible_statuses = self._ineligible_renewal_request_statuses() + kwargs = { + 'allocation_period': allocation_period, + 'computing_allowance': self._computing_allowance.get_resource(), + 'status__in': ineligible_statuses, + } + if pi is not None: + kwargs['pi'] = pi + return AllocationRenewalRequest.objects.filter(**kwargs) + + @staticmethod + def _ineligible_new_project_request_statuses(): + """Return a QuerySet of ProjectAllocationRequestStatusChoice + objects. If a PI has a relevant request with one of these + statuses, they are considered ineligible for a computing + allowance.""" + return ProjectAllocationRequestStatusChoice.objects.exclude( + name='Denied') + + @staticmethod + def _ineligible_project_statuses(): + """Return a QuerySet of ProjectStatusChoice objects. If the + computing allowance is one-per-PI, and a user is the PI of a + project with one of these statuses, they are considered + ineligible to receive another computing allowance of the same + type.""" + return ProjectStatusChoice.objects.exclude( + name__in=['Archived', 'Denied']) + + @staticmethod + def _ineligible_renewal_request_statuses(): + """Return a QuerySet of AllocationRenewalRequestStatusChoice + objects. If a PI has a relevant request with one of these + statuses, they are considered ineligible for a computing + allowance.""" + return AllocationRenewalRequestStatusChoice.objects.exclude( + name='Denied') diff --git a/coldfront/core/project/utils_/new_project_utils.py b/coldfront/core/project/utils_/new_project_utils.py index ffcf8fdd14..c4be12210a 100644 --- a/coldfront/core/project/utils_/new_project_utils.py +++ b/coldfront/core/project/utils_/new_project_utils.py @@ -1,19 +1,15 @@ from coldfront.api.statistics.utils import set_project_user_allocation_value from coldfront.core.allocation.models import AllocationAttribute from coldfront.core.allocation.models import AllocationAttributeType -from coldfront.core.allocation.models import AllocationPeriod from coldfront.core.allocation.models import AllocationStatusChoice from coldfront.core.allocation.utils import get_project_compute_allocation from coldfront.core.project.models import ProjectAllocationRequestStatusChoice -from coldfront.core.project.models import ProjectUser from coldfront.core.project.models import ProjectStatusChoice from coldfront.core.project.models import SavioProjectAllocationRequest from coldfront.core.project.models import VectorProjectAllocationRequest from coldfront.core.project.signals import new_project_request_denied from coldfront.core.project.utils_.request_processing_utils import create_project_users -from coldfront.core.resource.models import Resource from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance -from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.statistics.models import ProjectTransaction from coldfront.core.statistics.models import ProjectUserTransaction from coldfront.core.user.utils import account_activation_url @@ -30,7 +26,6 @@ from django.conf import settings from django.db import transaction -from django.db.models import Q from django.urls import reverse from flags.state import flag_enabled @@ -43,76 +38,6 @@ logger = logging.getLogger(__name__) -def non_denied_new_project_request_statuses(): - """Return a queryset of ProjectAllocationRequestStatusChoices that - do not have the name 'Denied'.""" - return ProjectAllocationRequestStatusChoice.objects.filter( - ~Q(name='Denied')) - - -def pis_with_new_project_requests_pks(allocation_period, - computing_allowance=None, - request_status_names=[]): - """Return a list of primary keys of PIs of new project requests for - the given AllocationPeriod that match the given filters. - - Parameters: - - allocation_period (AllocationPeriod): The AllocationPeriod to - filter with - - computing_allowance (Resource): An optional computing - allowance to filter with - - request_status_names (list[str]): A list of names of request - statuses to filter with - - Returns: - - A list of integers representing primary keys of matching PIs. - - Raises: - - AssertionError, if an input has an unexpected type. - """ - assert isinstance(allocation_period, AllocationPeriod) - f = Q(allocation_period=allocation_period) - if computing_allowance is not None: - assert isinstance(computing_allowance, Resource) - f = f & Q(computing_allowance=computing_allowance) - if request_status_names: - f = f & Q(status__name__in=request_status_names) - return set( - SavioProjectAllocationRequest.objects.filter( - f).values_list('pi__pk', flat=True)) - - -def project_pi_pks(computing_allowance=None, project_status_names=[]): - """Return a list of primary keys of PI Users of Projects that match - the given filters. - - Parameters: - - computing_allowance (Resource): An optional computing - allowance to filter with - - project_status_names (list[str]): A list of names of Project - statuses to filter with - - Returns: - - A list of integers representing primary keys of matching PIs. - - Raises: - - AssertionError, if an input has an unexpected type. - - ComputingAllowanceInterfaceError, if allowance-related values - cannot be retrieved. - """ - project_prefix = '' - if computing_allowance is not None: - assert isinstance(computing_allowance, Resource) - interface = ComputingAllowanceInterface() - project_prefix = interface.code_from_name(computing_allowance.name) - return set( - ProjectUser.objects.filter( - role__name='Principal Investigator', - project__name__startswith=project_prefix, - project__status__name__in=project_status_names - ).values_list('user__pk', flat=True)) - - class ProjectDenialRunner(object): """An object that performs necessary database changes when a new project request is denied.""" diff --git a/coldfront/core/project/utils_/renewal_utils.py b/coldfront/core/project/utils_/renewal_utils.py index 2a4a044ef7..572c67adfa 100644 --- a/coldfront/core/project/utils_/renewal_utils.py +++ b/coldfront/core/project/utils_/renewal_utils.py @@ -6,15 +6,14 @@ from coldfront.core.allocation.models import AllocationRenewalRequestStatusChoice from coldfront.core.allocation.models import AllocationStatusChoice from coldfront.core.allocation.utils import get_project_compute_allocation -from coldfront.core.allocation.utils import prorated_allocation_amount from coldfront.core.project.models import Project from coldfront.core.project.models import ProjectAllocationRequestStatusChoice from coldfront.core.project.models import ProjectStatusChoice from coldfront.core.project.models import ProjectUser from coldfront.core.project.models import ProjectUserRoleChoice from coldfront.core.project.models import SavioProjectAllocationRequest +from coldfront.core.project.utils_.computing_allowance_eligibility_manager import ComputingAllowanceEligibilityManager from coldfront.core.project.utils_.request_processing_utils import create_project_users -from coldfront.core.resource.models import Resource from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.statistics.models import ProjectTransaction @@ -199,73 +198,23 @@ def get_pi_active_unique_project(pi_user, computing_allowance, return project -def has_non_denied_renewal_request(pi, allocation_period): - """Return whether the given PI User has a non-"Denied" - AllocationRenewalRequest for the given AllocationPeriod.""" - if not isinstance(pi, User): - raise TypeError(f'{pi} is not a User object.') - if not isinstance(allocation_period, AllocationPeriod): - raise TypeError( - f'{allocation_period} is not an AllocationPeriod object.') - status_names = ['Under Review', 'Approved', 'Complete'] - return AllocationRenewalRequest.objects.filter( - pi=pi, - allocation_period=allocation_period, - status__name__in=status_names).exists() - - def is_any_project_pi_renewable(project, allocation_period): """Return whether the Project has at least one PI who is eligible to make an AllocationRenewalRequest during the given AllocationPeriod.""" + interface = ComputingAllowanceInterface() + computing_allowance = ComputingAllowance( + interface.allowance_from_project(project)) + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + computing_allowance, allocation_period=allocation_period) for pi in project.pis(): - if not has_non_denied_renewal_request(pi, allocation_period): + if computing_allowance_eligibility_manager.is_pi_eligible( + pi, is_renewal=True): return True return False -def non_denied_renewal_request_statuses(): - """Return a queryset of AllocationRenewalRequestStatusChoices that - do not have the name 'Denied'.""" - return AllocationRenewalRequestStatusChoice.objects.filter( - ~Q(name='Denied')) - - -def pis_with_renewal_requests_pks(allocation_period, computing_allowance=None, - request_status_names=[]): - """Return a list of primary keys of PIs of allocation renewal - requests for the given AllocationPeriod that match the given filters. - - Parameters: - - allocation_period (AllocationPeriod): The AllocationPeriod to - filter with - - computing_allowance (Resource): An optional computing - allowance to filter with - - request_status_names (list[str]): A list of names of request - statuses to filter with - - Returns: - - A list of integers representing primary keys of matching PIs. - - Raises: - - AssertionError, if an input has an unexpected type. - - ComputingAllowanceInterfaceError, if allowance-related values - cannot be retrieved. - """ - assert isinstance(allocation_period, AllocationPeriod) - f = Q(allocation_period=allocation_period) - if computing_allowance is not None: - assert isinstance(computing_allowance, Resource) - interface = ComputingAllowanceInterface() - project_prefix = interface.code_from_name(computing_allowance.name) - f = f & Q(post_project__name__startswith=project_prefix) - if request_status_names: - f = f & Q(status__name__in=request_status_names) - return set( - AllocationRenewalRequest.objects.filter( - f).values_list('pi__pk', flat=True)) - - def send_allocation_renewal_request_approval_email(request, num_service_units): """Send a notification email to the requester and PI associated with the given AllocationRenewalRequest stating that the request has been diff --git a/coldfront/core/project/views_/renewal_views/request_views.py b/coldfront/core/project/views_/renewal_views/request_views.py index a8a901c8bd..0ce110046d 100644 --- a/coldfront/core/project/views_/renewal_views/request_views.py +++ b/coldfront/core/project/views_/renewal_views/request_views.py @@ -17,10 +17,10 @@ from coldfront.core.project.models import ProjectUser from coldfront.core.project.models import ProjectUserStatusChoice from coldfront.core.project.models import SavioProjectAllocationRequest +from coldfront.core.project.utils_.computing_allowance_eligibility_manager import ComputingAllowanceEligibilityManager from coldfront.core.project.utils_.permissions_utils import is_user_manager_or_pi_of_project from coldfront.core.project.utils_.renewal_utils import get_current_allowance_year_period from coldfront.core.project.utils_.renewal_utils import get_pi_active_unique_project -from coldfront.core.project.utils_.renewal_utils import has_non_denied_renewal_request from coldfront.core.project.utils_.renewal_utils import send_new_allocation_renewal_request_admin_notification_email from coldfront.core.project.utils_.renewal_utils import send_new_allocation_renewal_request_pi_notification_email from coldfront.core.project.utils_.renewal_utils import send_new_allocation_renewal_request_pooling_notification_email @@ -349,13 +349,20 @@ def done(self, form_list, **kwargs): pi = tmp['PI'].user allocation_period = tmp['allocation_period'] - # If the PI already has a non-denied request for the period, raise - # an exception. Such PIs are not selectable in the 'pi_selection' - # step, but a request could have been created between selection and - # final submission. - if has_non_denied_renewal_request(pi, allocation_period): + # If the PI is ineligible for a renewal request for the computing + # allowance for the period, raise an exception. Such PIs are not + # selectable in the 'pi_selection' step, but a request could have + # been created beteween selection and final submission. + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + self.computing_allowance, + allocation_period=allocation_period) + is_pi_eligible = \ + computing_allowance_eligibility_manager.is_user_eligible( + pi, is_renewal=True) + if not is_pi_eligible: raise Exception( - f'PI {pi.username} already has a non-denied ' + f'PI {pi.username} is ineligible to make an ' f'AllocationRenewalRequest for AllocationPeriod ' f'{allocation_period.name}.') @@ -627,13 +634,20 @@ def done(self, form_list, **kwargs): pi = tmp['PI'].user allocation_period = tmp['allocation_period'] - # If the PI already has a non-denied request for the period, raise - # an exception. Such PIs are not selectable in the 'pi_selection' - # step, but a request could have been created between selection and - # final submission. - if has_non_denied_renewal_request(pi, allocation_period): + # If the PI is ineligible for a renewal request for the computing + # allowance for the period, raise an exception. Such PIs are not + # selectable in the 'pi_selection' step, but a request could have + # been created beteween selection and final submission. + computing_allowance_eligibility_manager = \ + ComputingAllowanceEligibilityManager( + self.computing_allowance, + allocation_period=allocation_period) + is_pi_eligible = \ + computing_allowance_eligibility_manager.is_user_eligible( + pi, is_renewal=True) + if not is_pi_eligible: raise Exception( - f'PI {pi.username} already has a non-denied ' + f'PI {pi.username} is ineligible to make an ' f'AllocationRenewalRequest for AllocationPeriod ' f'{allocation_period.name}.') diff --git a/coldfront/core/user/utils_/host_user_utils.py b/coldfront/core/user/utils_/host_user_utils.py index ba0ae51acc..e6afe42ab7 100644 --- a/coldfront/core/user/utils_/host_user_utils.py +++ b/coldfront/core/user/utils_/host_user_utils.py @@ -22,6 +22,14 @@ def host_user_lbl_email(user): return lbl_email_address(host_user) +def is_lbl_email_address(email): + """Return whether the given email address (str) is an LBL email + address.""" + email = email.lower() + email_domain = lbl_email_domain() + return email.endswith(email_domain) + + def is_lbl_employee(user): """Return whether the given User is an LBL employee.""" return bool(lbl_email_address(user)) @@ -31,7 +39,7 @@ def lbl_email_address(user): """Return the LBL email address (str) of the given User if they have one, else None.""" assert isinstance(user, User) - email_domain = '@lbl.gov' + email_domain = lbl_email_domain() if user.email.endswith(email_domain): return user.email email_addresses = EmailAddress.objects.filter( @@ -42,6 +50,11 @@ def lbl_email_address(user): return email_addresses.first().email +def lbl_email_domain(): + """Return the LBL email domain, including the "@" symbol.""" + return '@lbl.gov' + + def needs_host(user): """Return whether the given User needs a host user.""" assert isinstance(user, User) From 865703c9140605c37179d6a17fc13798eb4516a0 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 15 Apr 2025 08:50:27 -0700 Subject: [PATCH 09/10] Correct function call --- coldfront/core/project/utils_/renewal_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coldfront/core/project/utils_/renewal_utils.py b/coldfront/core/project/utils_/renewal_utils.py index 572c67adfa..a180b21f0f 100644 --- a/coldfront/core/project/utils_/renewal_utils.py +++ b/coldfront/core/project/utils_/renewal_utils.py @@ -209,7 +209,7 @@ def is_any_project_pi_renewable(project, allocation_period): ComputingAllowanceEligibilityManager( computing_allowance, allocation_period=allocation_period) for pi in project.pis(): - if computing_allowance_eligibility_manager.is_pi_eligible( + if computing_allowance_eligibility_manager.is_user_eligible( pi, is_renewal=True): return True return False From f31214232ff3a4399703b7f6e7e6b13388508940 Mon Sep 17 00:00:00 2001 From: Hamza Kundi Date: Mon, 7 Jul 2025 13:59:15 -0500 Subject: [PATCH 10/10] fixed bug with argument ordering --- .../project/utils_/computing_allowance_eligibility_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py b/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py index 660b441044..37a37faf6e 100644 --- a/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py +++ b/coldfront/core/project/utils_/computing_allowance_eligibility_manager.py @@ -96,13 +96,13 @@ def is_user_eligible(self, user, is_renewal=False): if self._computing_allowance.is_periodic(): existing_renewal_requests = self._existing_renewal_requests( - user, self._allocation_period) + self._allocation_period, user) if existing_renewal_requests.exists(): return False existing_new_project_requests = \ self._existing_new_project_requests( - user, self._allocation_period) + self._allocation_period, user) if existing_new_project_requests.exists(): return False