From ec0db580e59360d186da8785a7b9ae5b22af0cd6 Mon Sep 17 00:00:00 2001 From: Connor Brock Date: Fri, 21 Mar 2025 16:12:51 +0000 Subject: [PATCH] Implemented institution feature. Associated models, view, utils, and templates changed Signed-off-by: Connor Brock moving view logic into utils functions altering utils function logic to avoid issues with HTTP response. adding some comments too adding back accidentally deleted import statement adding institution field onto project detail view starting of test and model/migration changes managing errors if no email is associated with accounts ammending utils method as a preventative removing whitespace, and signals fixing templating bug --- coldfront/config/core.py | 7 +++ coldfront/core/project/forms.py | 12 ++++ .../migrations/0005_institution_feature.py | 23 +++++++ coldfront/core/project/models.py | 1 + .../templates/project/project_detail.html | 3 + coldfront/core/project/tests.py | 62 ++++++++++++++++++- coldfront/core/project/utils.py | 35 +++++++++++ coldfront/core/project/views.py | 20 +++++- 8 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 coldfront/core/project/migrations/0005_institution_feature.py diff --git a/coldfront/config/core.py b/coldfront/config/core.py index 10f2b8a6ea..5eb0e78dee 100644 --- a/coldfront/config/core.py +++ b/coldfront/config/core.py @@ -92,3 +92,10 @@ Please see instructions on our website. Staff, students, and external collaborators must request an account through a university faculty member. ''' + +#------------------------------------------------------------------------------ +# Enable project institution code feature. +#------------------------------------------------------------------------------ + +PROJECT_INSTITUTION_LIST = ENV.list('PROJECT_INSTITUTION_LIST', default=None) +PROJECT_INSTITUTION_EMAIL_MAP = ENV.dict('PROJECT_INSTITUTION_EMAIL_MAP', default={}) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index beeea24e9c..91d462d236 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -17,6 +17,7 @@ EMAIL_DIRECTOR_EMAIL_ADDRESS = import_from_settings( 'EMAIL_DIRECTOR_EMAIL_ADDRESS', '') +PROJECT_INSTITUTION_LIST = import_from_settings('PROJECT_INSTITUTION_LIST', []) class ProjectSearchForm(forms.Form): """ Search form for the Project list page. @@ -190,3 +191,14 @@ def clean(self): proj_attr = ProjectAttribute.objects.get(pk=cleaned_data.get('pk')) proj_attr.value = cleaned_data.get('new_value') proj_attr.clean() + +class ProjectCreationForm(forms.ModelForm): + class Meta: + model = Project + fields = ['title', 'description', 'field_of_science'] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if PROJECT_INSTITUTION_LIST: + institution_code = [(item, item) for item in PROJECT_INSTITUTION_LIST] + self.fields['institution'] = forms.ChoiceField(choices=institution_code) \ No newline at end of file diff --git a/coldfront/core/project/migrations/0005_institution_feature.py b/coldfront/core/project/migrations/0005_institution_feature.py new file mode 100644 index 0000000000..344b7cf357 --- /dev/null +++ b/coldfront/core/project/migrations/0005_institution_feature.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.11 on 2025-04-02 12:26 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('project', '0004_auto_20230406_1133'), + ] + + operations = [ + migrations.AddField( + model_name='historicalproject', + name='institution', + field=models.CharField(blank=True, max_length=10, default='None'), + ), + migrations.AddField( + model_name='project', + name='institution', + field=models.CharField(blank=True, max_length=10, default='None'), + ), + ] diff --git a/coldfront/core/project/models.py b/coldfront/core/project/models.py index 45f9470ac4..e9213ac58d 100644 --- a/coldfront/core/project/models.py +++ b/coldfront/core/project/models.py @@ -94,6 +94,7 @@ def get_by_natural_key(self, title, pi_username): requires_review = models.BooleanField(default=True) history = HistoricalRecords() objects = ProjectManager() + institution = models.CharField(max_length=10, blank=True, default='None') def clean(self): """ Validates the project and raises errors if the project is invalid. """ diff --git a/coldfront/core/project/templates/project/project_detail.html b/coldfront/core/project/templates/project/project_detail.html index 9f0f39657c..ea1f93c593 100644 --- a/coldfront/core/project/templates/project/project_detail.html +++ b/coldfront/core/project/templates/project/project_detail.html @@ -75,6 +75,9 @@

project review pending {% endif %}

+ {% if PROJECT_INSTITUTION_LIST or PROJECT_INSTITUTION_EMAIL_MAP %} +

Institution: {{ project.institution }}

+ {% endif %}

Created: {{ project.created|date:"M. d, Y" }}

diff --git a/coldfront/core/project/tests.py b/coldfront/core/project/tests.py index afc85f9af8..cdb2e0267e 100644 --- a/coldfront/core/project/tests.py +++ b/coldfront/core/project/tests.py @@ -1,8 +1,10 @@ import logging +from unittest.mock import patch from django.core.exceptions import ValidationError -from django.test import TestCase +from django.test import TestCase, TransactionTestCase +from coldfront.core.project.utils import add_automated_institution_choice, add_manual_institution_choice from coldfront.core.test_helpers.factories import ( UserFactory, ProjectFactory, @@ -205,3 +207,61 @@ def test_attribute_must_match_datatype(self): ) with self.assertRaises(ValidationError): new_attr.clean() + +class TestInstitution(TransactionTestCase): + """Tear down database after each run to prevent conflicts across cases """ + reset_sequences = True + + def setUp(self): + self.user = UserFactory(username='capeo') + self.field_of_science = FieldOfScienceFactory(description='Physics') + self.status = ProjectStatusChoiceFactory(name='Active') + + + def create_project_with_institution(self, title, institution_dict = None, institution_list = None): + """Helper method to create a project and assign a institution value based on the argument passed""" + # Project Creation + project = Project.objects.create( + title=title, + pi=self.user, + status=self.status, + field_of_science=self.field_of_science, + ) + + if institution_dict and institution_list: + pass + elif institution_dict: + add_automated_institution_choice(project, institution_dict) + elif institution_list: + add_manual_institution_choice(project, institution_list) + + project.save() + + return project.institution + + @patch('coldfront.config.core.PROJECT_INSTITUTION_EMAIL_MAP', {'inst.ac.com': 'AC', 'inst.edu.com': 'EDU', 'bfo.ac.uk': 'BFO'}) + @patch('coldfront.config.core.PROJECT_INSTITUTION_LIST', ['test_one', 'test_two', 'test_three'] ) + def test_institution_is_none(self): + from coldfront.config.core import PROJECT_INSTITUTION_EMAIL_MAP + from coldfront.config.core import PROJECT_INSTITUTION_LIST + """Test to check if institution is none after both env vars are enabled. """ + + # Create project with both institution + project_institution = self.create_project_with_institution('Project 1', PROJECT_INSTITUTION_LIST, PROJECT_INSTITUTION_EMAIL_MAP) + + # Create the first project + self.assertEqual(project_institution, 'None') + + + @patch('coldfront.config.core.PROJECT_INSTITUTION_EMAIL_MAP', {'inst.ac.com': 'AC', 'inst.edu.com': 'EDU', 'bfo.ac.uk': 'BFO'}) + @patch('coldfront.config.core.PROJECT_INSTITUTION_LIST', ['test_one', 'test_two', 'test_three'] ) + def test_institution_is_none(self): + from coldfront.config.core import PROJECT_INSTITUTION_EMAIL_MAP + from coldfront.config.core import PROJECT_INSTITUTION_LIST + """Test to check if institution is none after both env vars are enabled. """ + + # Create project with both institution + project_institution = self.create_project_with_institution('Project 1', PROJECT_INSTITUTION_LIST, PROJECT_INSTITUTION_EMAIL_MAP) + + # Create the first project + self.assertEqual(project_institution, 'None') diff --git a/coldfront/core/project/utils.py b/coldfront/core/project/utils.py index be5b611c52..ee98c51a84 100644 --- a/coldfront/core/project/utils.py +++ b/coldfront/core/project/utils.py @@ -18,3 +18,38 @@ def add_project_user_status_choices(apps, schema_editor): for choice in ['Active', 'Pending Remove', 'Denied', 'Removed', ]: ProjectUserStatusChoice.objects.get_or_create(name=choice) + + +def add_manual_institution_choice(project, form): + project.institution = form.cleaned_data['institution'] + + +def add_automated_institution_choice(project, institution_map: dict): + + """ + Adding automated institution choices to a project. Taking PI email of current project + and comparing to domain key from institution map. + :param project: Project to add automated institution choices to. + :param institution_map: Dictionary of institution keys, values. + + """ + + email = project.pi.email + + try: + split_domain = email.split('@') + except IndexError: + split_domain = None + + try: + direct_dict_match = institution_map.get(split_domain[1]) + except IndexError: + direct_dict_match = None + + + if direct_dict_match: + project.institution = direct_dict_match + else: + for key, value in institution_map.items(): + if key in split_domain[1]: + project.institution = value diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 39d144c5e0..c11b95bc65 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -11,6 +11,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.contrib.auth.decorators import user_passes_test, login_required from django.contrib.auth.models import User +from coldfront.core.project.utils import add_automated_institution_choice, add_manual_institution_choice from coldfront.core.utils.common import import_from_settings from django.contrib.messages.views import SuccessMessageMixin from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator @@ -43,7 +44,8 @@ ProjectReviewForm, ProjectSearchForm, ProjectUserUpdateForm, - ProjectAttributeUpdateForm) + ProjectAttributeUpdateForm, + ProjectCreationForm) from coldfront.core.project.models import (Project, ProjectAttribute, ProjectReview, @@ -72,6 +74,8 @@ EMAIL_SENDER = import_from_settings('EMAIL_SENDER') logger = logging.getLogger(__name__) +PROJECT_INSTITUTION_LIST = import_from_settings('PROJECT_INSTITUTION_LIST', False) +PROJECT_INSTITUTION_EMAIL_MAP = import_from_settings('PROJECT_INSTITUTION_EMAIL_MAP', False) class ProjectDetailView(LoginRequiredMixin, UserPassesTestMixin, DetailView): model = Project @@ -176,6 +180,8 @@ def get_context_data(self, **kwargs): context['attributes_with_usage'] = attributes_with_usage context['project_users'] = project_users context['ALLOCATION_ENABLE_ALLOCATION_RENEWAL'] = ALLOCATION_ENABLE_ALLOCATION_RENEWAL + context['PROJECT_INSTITUTION_LIST'] = PROJECT_INSTITUTION_LIST + context['PROJECT_INSTITUTION_EMAIL_MAP'] = PROJECT_INSTITUTION_EMAIL_MAP try: context['ondemand_url'] = settings.ONDEMAND_URL @@ -451,7 +457,7 @@ def post(self, request, *args, **kwargs): class ProjectCreateView(LoginRequiredMixin, UserPassesTestMixin, CreateView): model = Project template_name_suffix = '_create_form' - fields = ['title', 'description', 'field_of_science', ] + form_class = ProjectCreationForm def test_func(self): """ UserPassesTestMixin Tests""" @@ -475,6 +481,16 @@ def form_valid(self, form): status=ProjectUserStatusChoice.objects.get(name='Active') ) + if PROJECT_INSTITUTION_LIST and PROJECT_INSTITUTION_EMAIL_MAP: + """ + If both features are enabled, pass, to avoid duplicate data being stored. + """ + pass + elif PROJECT_INSTITUTION_LIST: + add_manual_institution_choice(project_obj, form) + elif PROJECT_INSTITUTION_EMAIL_MAP: + add_automated_institution_choice(project_obj, PROJECT_INSTITUTION_EMAIL_MAP) + return super().form_valid(form) def get_success_url(self):