diff --git a/bootstrap/ansible/main.copyme b/bootstrap/ansible/main.copyme index d30e65214..a8236dc02 100644 --- a/bootstrap/ansible/main.copyme +++ b/bootstrap/ansible/main.copyme @@ -5,6 +5,7 @@ # Types of Ansible tasks to run by default. provisioning_tasks: true common_tasks: true +chmod_tasks: true # Can be false when a Windows FS is mounted ############################################################################### # General Settings @@ -36,7 +37,7 @@ db_host: localhost # The credentials for the database admin user. # TODO: Replace the username and password. db_admin_user: admin -db_admin_passwd: '' +db_admin_passwd: root #------------------------------------------------------------------------------ # Redis settings @@ -44,8 +45,7 @@ db_admin_passwd: '' # The password for Redis. # TODO: Replace the password. -redis_passwd: '' - +redis_passwd: root redis_host: localhost #------------------------------------------------------------------------------ @@ -123,7 +123,7 @@ cilogon_app_secret: "" # Django Flags settings #------------------------------------------------------------------------------ -# # Note: Use uppercase True/False so that Python interprets these as booleans. +# Note: Use uppercase True/False so that Python interprets these as booleans. # TODO: For LRC, disable link login. flag_basic_auth_enabled: False @@ -149,6 +149,15 @@ flag_mou_generation_enabled: False # Whether to include a survey as part of the allowance renewal request process. flag_renewal_survey_enabled: True +#------------------------------------------------------------------------------ +# Plugin: departments +#------------------------------------------------------------------------------ + +# TODO: Enable for BRC, disable for LRC. +plugin_departments_enabled: true +plugin_departments_department_display_name: "Department" +plugin_departments_department_data_source: "coldfront.plugins.departments.utils.data_sources.backends.calnet_ldap.CalNetLdapDataSourceBackend" + #------------------------------------------------------------------------------ # User-facing strings #------------------------------------------------------------------------------ @@ -167,6 +176,8 @@ center_user_guide: "https://docs-research-it.berkeley.edu/services/high-performa center_login_guide: "https://docs-research-it.berkeley.edu/services/high-performance-computing/user-guide/logging-brc-clusters/#Logging-in" # TODO: For MyLRC, use "hpcshelp@lbl.gov". center_help_email: "brc-hpc-help@berkeley.edu" +# TODO: For MyLRC, use "Division". +department_display_name: "Department" #------------------------------------------------------------------------------ # BRC Vector settings @@ -421,16 +432,13 @@ sentry_dsn: "" # # Email settings. # email_host: localhost # email_port: 1025 -# # TODO: Set these addresses to yours. -# from_email: you@email.com -# admin_email: you@email.com +# from_email: placeholder@dev.dev +# admin_email: placeholder@dev.dev # # TODO: For LRC, use the substring 'MyLRC'. # email_subject_prefix: '[MyBRC-User-Portal]' # # A list of admin email addresses to be notified about new requests and other # # events. -# # TODO: Set these addresses to yours. -# email_admin_list: ['you@email.com'] +# email_admin_list: ['placeholder@dev.dev'] # # A list of email addresses to CC when certain requests are processed. -# # TODO: Set these addresses to yours. -# request_approval_cc_list: ['you@email.com'] +# request_approval_cc_list: ['placeholder@dev.dev'] diff --git a/bootstrap/ansible/playbook.yml b/bootstrap/ansible/playbook.yml index d10c23368..c693c9583 100644 --- a/bootstrap/ansible/playbook.yml +++ b/bootstrap/ansible/playbook.yml @@ -13,7 +13,6 @@ cwd: "{{ lookup('env', 'PWD') }} " provisioning_tasks: true common_tasks: true - python_version: 3.6 postgres_version: 15 python_version_dotless: "{{ python_version | regex_replace('\\.','') }}" @@ -61,6 +60,9 @@ name: https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6-1/wkhtmltox-0.12.6-1.centos7.x86_64.rpm state: present + # The SCL role runs many tasks when it's included, so having it do + # everything in one run would be ideal to not have redundant tasks. + - name: Install SCL include_role: role: smbambling.scl @@ -648,6 +650,8 @@ requirements: "{{ git_prefix }}/{{ reponame }}/requirements.txt" executable: "{{ git_prefix }}/venv/bin/pip3" become_user: "{{ djangooperator }}" + environment: # needed as pg_config isn't in PATH by default + PATH: "{{ ansible_env.PATH }}:/usr/pgsql-{{ postgres_version }}/bin" # Run Django management commands. @@ -731,6 +735,7 @@ state: directory mode: 0755 group: apache + when: chmod_tasks - name: Grant Apache recursive read access to the application directory file: @@ -739,6 +744,7 @@ recurse: true mode: "u=rwX,g=rX,o=rX" group: apache + when: chmod_tasks # Gracefully restart Apache so that processes handle current requests # before being replaced by a new process. diff --git a/bootstrap/ansible/settings_template.tmpl b/bootstrap/ansible/settings_template.tmpl index 4fc068fba..c1bf6cffa 100644 --- a/bootstrap/ansible/settings_template.tmpl +++ b/bootstrap/ansible/settings_template.tmpl @@ -21,7 +21,7 @@ CENTER_BASE_URL = '{{ full_host_path }}' CENTER_HELP_URL = CENTER_BASE_URL + '/help' CENTER_PROJECT_RENEWAL_HELP_URL = CENTER_BASE_URL + '/help' -EMAIL_HOST = '{{ email_host}}' +EMAIL_HOST = '{{ email_host }}' EMAIL_PORT = {{ email_port }} EMAIL_SUBJECT_PREFIX = '{{ email_subject_prefix }}' # A list of admin email addresses to be notified about new requests and other @@ -298,6 +298,7 @@ FLAGS = { 'SECURE_DIRS_REQUESTABLE': [{'condition': 'boolean', 'value': {{ flag_brc_enabled }}}], 'SERVICE_UNITS_PURCHASABLE': [{'condition': 'boolean', 'value': {{ flag_brc_enabled }}}], 'SSO_ENABLED': [{'condition': 'boolean', 'value': {{ flag_sso_enabled }}}], + 'USER_DEPARTMENTS_ENABLED': [{'condition': 'app installed', 'value': 'coldfront.plugins.departments'}], 'MOU_GENERATION_ENABLED': [{'condition': 'boolean', 'value': {{ flag_mou_generation_enabled }}}], 'RENEWAL_SURVEY_ENABLED': [{'condition': 'boolean', 'value': {{ flag_renewal_survey_enabled }}}], } @@ -318,6 +319,19 @@ if (not FLAGS['SSO_ENABLED'][0]['value'] and 'LINK_LOGIN_ENABLED should only be enabled when SSO_ENABLED is ' 'enabled.') +{% if plugin_departments_enabled is defined and plugin_departments_enabled | bool %} +#------------------------------------------------------------------------------ +# Plugin: departments +#------------------------------------------------------------------------------ + +EXTRA_EXTRA_APPS += [ + 'coldfront.plugins.departments' +] + +DEPARTMENTS_DEPARTMENT_DISPLAY_NAME = '{{ plugin_departments_department_display_name }}' +DEPARTMENTS_DEPARTMENT_DATA_SOURCE = '{{ plugin_departments_department_data_source }}' +{% endif %} + {% if file_storage_backend == 'google_drive' %} #------------------------------------------------------------------------------ # django-googledrive-storage settings diff --git a/bootstrap/development/docker-legacy/docker-compose.yml b/bootstrap/development/docker-legacy/docker-compose.yml index 1b7306963..a72fbd888 100644 --- a/bootstrap/development/docker-legacy/docker-compose.yml +++ b/bootstrap/development/docker-legacy/docker-compose.yml @@ -26,7 +26,7 @@ services: - 6379:6379 volumes: - redis_data:/data - command: --requirepass ${REDIS_PASSWORD} + command: --requirepass "${REDIS_PASSWORD}" healthcheck: test: ["CMD", "redis-cli", "ping"] interval: 10s diff --git a/bootstrap/development/docker-legacy/docker_load_database_backup.sh b/bootstrap/development/docker-legacy/docker_load_database_backup.sh index e27a08d42..f89ff7c2a 100644 --- a/bootstrap/development/docker-legacy/docker_load_database_backup.sh +++ b/bootstrap/development/docker-legacy/docker_load_database_backup.sh @@ -32,4 +32,4 @@ echo MODIFYING PERMISSIONS... docker exec -i coldfront-db-1 \ psql -U $DB_OWNER -c "ALTER SCHEMA public OWNER TO $DB_OWNER;" $DB_NAME -echo DONE. \ No newline at end of file +echo DONE. diff --git a/bootstrap/development/docker/config/brc_defaults.yml b/bootstrap/development/docker/config/brc_defaults.yml index 5bf4473cb..933ed1248 100644 --- a/bootstrap/development/docker/config/brc_defaults.yml +++ b/bootstrap/development/docker/config/brc_defaults.yml @@ -16,6 +16,10 @@ flag_lrc_enabled: False flag_next_period_renewal_requestable_month: 5 flag_multiple_email_addresses_allowed: False +plugin_departments_enabled: true +plugin_departments_department_display_name: "Department" +plugin_departments_department_data_source: "coldfront.plugins.departments.utils.data_sources.backends.calnet_ldap.CalNetLdapDataSourceBackend" + portal_name: MyBRC program_name_long: Berkeley Research Computing program_name_short: BRC diff --git a/bootstrap/development/docker/config/lrc_defaults.yml b/bootstrap/development/docker/config/lrc_defaults.yml index 969e27acb..657eae09c 100644 --- a/bootstrap/development/docker/config/lrc_defaults.yml +++ b/bootstrap/development/docker/config/lrc_defaults.yml @@ -16,6 +16,10 @@ flag_lrc_enabled: True flag_next_period_renewal_requestable_month: 9 flag_multiple_email_addresses_allowed: False +plugin_departments_enabled: false +plugin_departments_department_display_name: "Division" +plugin_departments_department_data_source: "coldfront.plugins.departments.utils.data_sources.backends.dummy.DummyDataSourceBackend" + portal_name: MyLRC program_name_long: Laboratory Research Computing program_name_short: LRC diff --git a/bootstrap/development/docker/docker-compose.yml b/bootstrap/development/docker/docker-compose.yml index 05ac369bc..60db810ad 100644 --- a/bootstrap/development/docker/docker-compose.yml +++ b/bootstrap/development/docker/docker-compose.yml @@ -53,6 +53,11 @@ services: volumes: - ../../..:/var/www/coldfront_app/coldfront + qcluster: + image: coldfront-qcluster + volumes: + - ../../..:/var/www/coldfront_app/coldfront + volumes: db-postgres: external: false diff --git a/bootstrap/development/docker/images/qcluster.Dockerfile b/bootstrap/development/docker/images/qcluster.Dockerfile new file mode 100644 index 000000000..980d9cf5e --- /dev/null +++ b/bootstrap/development/docker/images/qcluster.Dockerfile @@ -0,0 +1,3 @@ +FROM coldfront-app-base + +CMD ["python3", "manage.py", "qcluster"] diff --git a/bootstrap/development/docker/scripts/build_images.sh b/bootstrap/development/docker/scripts/build_images.sh index 6aca0e44a..2dbbce365 100755 --- a/bootstrap/development/docker/scripts/build_images.sh +++ b/bootstrap/development/docker/scripts/build_images.sh @@ -7,3 +7,4 @@ docker build -f bootstrap/development/docker/images/app-shell.Dockerfile -t cold docker build -f bootstrap/development/docker/images/web.Dockerfile -t coldfront-web . docker build -f bootstrap/development/docker/images/email-server.Dockerfile -t coldfront-email-server . docker build -f bootstrap/development/docker/images/db-postgres-shell.Dockerfile -t coldfront-db-postgres-shell . +docker build -f bootstrap/development/docker/images/qcluster.Dockerfile -t coldfront-qcluster . diff --git a/bootstrap/development/docs/vagrant-vm-README.md b/bootstrap/development/docs/vagrant-vm-README.md index c41d51bf3..22d9ba1bb 100644 --- a/bootstrap/development/docs/vagrant-vm-README.md +++ b/bootstrap/development/docs/vagrant-vm-README.md @@ -30,16 +30,14 @@ The application may be installed within a Vagrant VM that is running on Scientif # This produces two lines: condense them into one. openssl rand -base64 64 ``` -8. Customize `main.yml`. In particular, uncomment everything under the `dev_settings` section, and fill in the below variables. Note that quotes need not be provided, except in the list variable. - ``` - django_secret_key: secret_key_from_previous_step - db_admin_passwd: password_here - redis_passwd: password_here - from_email: you@email.com - admin_email: you@email.com - email_admin_list: ["you@email.com"] - request_approval_cc_list: ["you@email.com"] - ``` +8. In `main.yml`, uncomment everything under the dev_settings section, +and customize the following variables with your own values. + ``` + django_secret_key: secret_key_from_previous_step + chmod_tasks: true # Can be false when a Windows FS is mounted + db_admin_passwd: root + redis_passwd: root + ``` 9. Provision the VM. This should run the Ansible playbook. Expect this to take a few minutes on the first run. ``` vagrant up diff --git a/coldfront/config/settings.py b/coldfront/config/settings.py index 5b871b1ea..2b5afd517 100644 --- a/coldfront/config/settings.py +++ b/coldfront/config/settings.py @@ -53,6 +53,7 @@ INSTALLED_APPS += [ 'coldfront.core.user', 'coldfront.core.field_of_science', + # 'coldfront.core.department', 'coldfront.core.utils', 'coldfront.core.portal', 'coldfront.core.project', diff --git a/coldfront/config/test_settings.py.sample b/coldfront/config/test_settings.py.sample index 44f99a412..741c11914 100644 --- a/coldfront/config/test_settings.py.sample +++ b/coldfront/config/test_settings.py.sample @@ -157,6 +157,26 @@ FLAGS = { 'SECURE_DIRS_REQUESTABLE': [{'condition': 'boolean', 'value': True}], 'SERVICE_UNITS_PURCHASABLE': [{'condition': 'boolean', 'value': True}], 'SSO_ENABLED': [{'condition': 'boolean', 'value': False}], + 'USER_DEPARTMENTS_ENABLED': [{'condition': 'app installed', 'value': 'coldfront.plugins.departments'}], 'MOU_GENERATION_ENABLED': [{'condition': 'boolean', 'value': False}], 'RENEWAL_SURVEY_ENABLED': [{'condition': 'boolean', 'value': True}], } + +#------------------------------------------------------------------------------ +# Plugin: departments +#------------------------------------------------------------------------------ + +EXTRA_EXTRA_APPS += [ + 'coldfront.plugins.departments' +] + +DEPARTMENTS_DEPARTMENT_DISPLAY_NAME = 'Department' +DEPARTMENTS_DEPARTMENT_DATA_SOURCE = 'coldfront.plugins.departments.utils.data_sources.backends.dummy.DummyDataSourceBackend' + +#------------------------------------------------------------------------------ +# django-q settings +#------------------------------------------------------------------------------ + +Q_CLUSTER = { + 'sync': True, +} 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 ddccb697b..d3fa1d3e2 100644 --- a/coldfront/core/project/forms_/new_project_forms/request_forms.py +++ b/coldfront/core/project/forms_/new_project_forms/request_forms.py @@ -18,7 +18,6 @@ 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 diff --git a/coldfront/core/project/templates/project/project_request/savio/project_pi_department.html b/coldfront/core/project/templates/project/project_request/savio/project_pi_department.html new file mode 100644 index 000000000..b72656b7d --- /dev/null +++ b/coldfront/core/project/templates/project/project_request/savio/project_pi_department.html @@ -0,0 +1,84 @@ +{% extends "common/base.html" %} +{% load feature_flags %} +{% load crispy_forms_tags %} +{% load common_tags %} +{% load static %} + + +{% block title %} +{{ PRIMARY_CLUSTER_NAME }} Project Request - PI {% settings_value 'DEPARTMENTS_DEPARTMENT_DISPLAY_NAME' %} +{% endblock %} + + +{% block head %} +{{ wizard.form.media }} +{% endblock %} + + +{% block content %} + + + + +

{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator

+ +
    +
  1. Allowance: {{ computing_allowance.name }}
    2. +
  2.   
    3. +
  3. Allocation Period: {{ allocation_period.name }}
    4. +
  4.   
    5. +
  5. {{ breadcrumb_pi }}
    6. +
+ +{% settings_value 'DEPARTMENTS_DEPARTMENT_DISPLAY_NAME' as department_display_name %} +

Select one or more {{ department_display_name | lower }}s for the Principal Investigator of the project. You may search for the {{ department_display_name | lower }}(s) in the selection field.

+

+ {% flag_enabled 'BRC_ONLY' as brc_only %} + {% if brc_only %} Non-Berkeley {% else %} Non-Lab {% endif %} + users should select "Other".

+ +
+ {% csrf_token %} + + {{ wizard.management_form }} + {% if wizard.form.forms %} + {{ wizard.form.management_form }} + {% for form in wizard.form.forms %} + {{ form|crispy }} + {% endfor %} + {% else %} + {{ wizard.form|crispy }} + {% endif %} +
+ {% if wizard.steps.prev %} + + + {% endif %} + +
+
+ +

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

+ + + +{% endblock %} diff --git a/coldfront/core/project/tests/test_views/test_new_project_views/test_savio_project_request_wizard.py b/coldfront/core/project/tests/test_views/test_new_project_views/test_savio_project_request_wizard.py index 5b2667d05..2e9c6cf10 100644 --- a/coldfront/core/project/tests/test_views/test_new_project_views/test_savio_project_request_wizard.py +++ b/coldfront/core/project/tests/test_views/test_new_project_views/test_savio_project_request_wizard.py @@ -1,14 +1,27 @@ +from copy import deepcopy +from http import HTTPStatus + +from django.conf import settings +from django.test import override_settings +from django.urls import reverse + from coldfront.core.project.models import Project from coldfront.core.project.models import SavioProjectAllocationRequest from coldfront.core.project.utils_.renewal_utils import get_current_allowance_year_period from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.utils.tests.test_base import enable_deployment -from coldfront.core.utils.tests.test_base import TestBase -from django.urls import reverse -from http import HTTPStatus +from coldfront.core.utils.tests.test_base import TransactionTestBase + +from coldfront.plugins.departments.models import Department +from coldfront.plugins.departments.models import UserDepartment + +Q_CLUSTER_COPY = deepcopy(settings.Q_CLUSTER) +Q_CLUSTER_COPY['sync'] = True -class TestSavioProjectRequestWizard(TestBase): + +@override_settings(Q_CLUSTER=Q_CLUSTER_COPY) +class TestSavioProjectRequestWizard(TransactionTestBase): """A class for testing SavioProjectRequestWizard.""" @enable_deployment('BRC') @@ -21,61 +34,80 @@ def setUp(self): self.interface = ComputingAllowanceInterface() + self._department_1 = Department.objects.create( + name='Department 1', code='DEPT1') + self._department_2 = Department.objects.create( + name='Department 2', code='DEPT2') + @staticmethod - def request_url(): + def _request_url(): """Return the URL for requesting to create a new Savio 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) + def _send_post_data(self, computing_allowance, allocation_period, + details_data, survey_data, new_pi_details=None, + existing_pi=None, pi_departments=None): + """Send a POST request to the view with the given parameters. - computing_allowance = self.get_predominant_computing_allowance() - allocation_period = get_current_allowance_year_period() + TODO: Some POST data should be made configurable (e.g., new PI + details, pooling, etc.). + """ + form_data = [] 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', } + form_data.append(computing_allowance_form_data) + allocation_period_form_data = { '1-allocation_period': allocation_period.pk, current_step_key: '1', } - existing_pi_form_data = { - '2-PI': self.user.pk, - current_step_key: '2', - } + form_data.append(allocation_period_form_data) + + if existing_pi is not None: + existing_pi_form_data = { + '2-PI': existing_pi.pk, + current_step_key: '2', + } + form_data.append(existing_pi_form_data) + + # TODO: Account for new_pi_details. + + if pi_departments is not None: + pi_department_form_data = { + '4-departments': [ + department.pk for department in pi_departments], + current_step_key: '4' + } + form_data.append(pi_department_form_data) + pool_allocations_data = { - '6-pool': False, - current_step_key: '6', + '7-pool': False, + current_step_key: '7', } - details_data = { - '8-name': 'name', - '8-title': 'title', - '8-description': 'a' * 20, - current_step_key: '8', + form_data.append(pool_allocations_data) + + details_data_copy = { + current_step_key: '9', } - survey_data = { - '10-scope_and_intent': 'b' * 20, - '10-computational_aspects': 'c' * 20, - current_step_key: '10', + for key in details_data: + details_data_copy[f'9-{key}'] = details_data[key] + form_data.append(details_data_copy) + + survey_data_copy = { + current_step_key: '11', } - form_data = [ - computing_allowance_form_data, - allocation_period_form_data, - existing_pi_form_data, - pool_allocations_data, - details_data, - survey_data, - ] - - url = self.request_url() + for key in survey_data: + survey_data_copy[f'11-{key}'] = survey_data[key] + form_data.append(survey_data_copy) + + url = self._request_url() for i, data in enumerate(form_data): response = self.client.post(url, data) if i == len(form_data) - 1: @@ -83,11 +115,33 @@ def test_post_creates_request(self): else: self.assertEqual(response.status_code, HTTPStatus.OK) + @enable_deployment('BRC') + def test_post_creates_request_and_project(self): + """Test that a POST request creates a + SavioProjectAllocationRequest and a Project.""" + 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() + details_data = { + 'name': 'name', + 'title': 'title', + 'description': 'a' * 20, + } + survey_data = { + 'scope_and_intent': 'b' * 20, + 'computational_aspects': 'c' * 20, + } + + self._send_post_data( + computing_allowance, allocation_period, details_data, survey_data, + existing_pi=self.user, pi_departments=[self._department_1]) + requests = SavioProjectAllocationRequest.objects.all() self.assertEqual(requests.count(), 1) projects = Project.objects.all() self.assertEqual(projects.count(), 1) - request = requests.first() project = projects.first() self.assertEqual(request.requester, self.user) @@ -98,16 +152,61 @@ def test_post_creates_request(self): self.assertEqual(request.allocation_period, allocation_period) self.assertEqual(request.pi, self.user) self.assertEqual(request.project, project) - self.assertEqual(project.name, f'fc_{details_data["8-name"]}') - self.assertEqual(project.title, details_data['8-title']) - self.assertEqual(project.description, details_data['8-description']) + + details_data['name'] = f'fc_{details_data["name"]}' + for key in details_data: + self.assertEqual(getattr(project, key), details_data[key]) + self.assertFalse(request.pool) - self.assertEqual( - request.survey_answers['scope_and_intent'], - survey_data['10-scope_and_intent']) - self.assertEqual( - request.survey_answers['computational_aspects'], - survey_data['10-computational_aspects']) + + for key in survey_data: + self.assertEqual(request.survey_answers[key], survey_data[key]) + self.assertEqual(request.status.name, 'Under Review') + @override_settings( + DEPARTMENT_DATA_SOURCE=( + 'coldfront.plugins.departments.utils.data_sources.backends.dummy.' + 'DummyDataSourceBackend')) + def test_post_sets_user_departments(self): + """Test that a POST request sets authoritative and + non-authoritative UserDepartments for the PI.""" + self.user.first_name = 'First' + self.user.last_name = 'Last' + self.user.save() + + self.assertFalse(UserDepartment.objects.filter(user=self.user).exists()) + + computing_allowance = self.get_predominant_computing_allowance() + allocation_period = get_current_allowance_year_period() + details_data = { + 'name': 'name', + 'title': 'title', + 'description': 'a' * 20, + } + survey_data = { + 'scope_and_intent': 'b' * 20, + 'computational_aspects': 'c' * 20, + } + self._send_post_data( + computing_allowance, allocation_period, details_data, survey_data, + existing_pi=self.user, pi_departments=[self._department_1]) + + self.assertEqual(UserDepartment.objects.count(), 3) + self.assertTrue( + UserDepartment.objects.filter( + user=self.user, + department=self._department_1, + is_authoritative=False).exists()) + self.assertTrue( + UserDepartment.objects.filter( + user=self.user, + department=Department.objects.get(name='Department F'), + is_authoritative=True).exists()) + self.assertTrue( + UserDepartment.objects.filter( + user=self.user, + department=Department.objects.get(name='Department L'), + is_authoritative=True).exists()) + # TODO diff --git a/coldfront/core/project/views_/new_project_views/request_views.py b/coldfront/core/project/views_/new_project_views/request_views.py index 2a8d41d12..2c94bcc9c 100644 --- a/coldfront/core/project/views_/new_project_views/request_views.py +++ b/coldfront/core/project/views_/new_project_views/request_views.py @@ -4,6 +4,7 @@ from coldfront.core.allocation.models import AllocationStatusChoice from coldfront.core.billing.forms import BillingIDValidationForm from coldfront.core.billing.utils.queries import get_or_create_billing_activity_from_full_id + from coldfront.core.project.forms_.new_project_forms.request_forms import ComputingAllowanceForm from coldfront.core.project.forms_.new_project_forms.request_forms import SavioProjectAllocationPeriodForm from coldfront.core.project.forms_.new_project_forms.request_forms import SavioProjectDetailsForm @@ -32,9 +33,14 @@ from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.user.models import UserProfile from coldfront.core.user.utils import access_agreement_signed +from coldfront.core.utils.common import import_from_settings from coldfront.core.utils.common import session_wizard_all_form_data from coldfront.core.utils.common import utc_now_offset_aware +from coldfront.plugins.departments.forms import NonAuthoritativeDepartmentSelectionForm +from coldfront.plugins.departments.utils.queries import get_departments_for_user +from coldfront.plugins.departments.utils.queries import UserDepartmentUpdater + from django.conf import settings from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin @@ -48,9 +54,11 @@ from django.views.generic.base import TemplateView from django.views.generic.edit import FormView +from django_q.tasks import async_task from flags.state import flag_enabled from formtools.wizard.views import SessionWizardView +import hashlib import logging @@ -125,6 +133,7 @@ class SavioProjectRequestWizard(LoginRequiredMixin, UserPassesTestMixin, ('allocation_period', SavioProjectAllocationPeriodForm), ('existing_pi', SavioProjectExistingPIForm), ('new_pi', SavioProjectNewPIForm), + ('pi_department', NonAuthoritativeDepartmentSelectionForm), ('ica_extra_fields', SavioProjectICAExtraFieldsForm), ('recharge_extra_fields', SavioProjectRechargeExtraFieldsForm), ('pool_allocations', SavioProjectPoolAllocationsForm), @@ -143,6 +152,8 @@ class SavioProjectRequestWizard(LoginRequiredMixin, UserPassesTestMixin, 'project/project_request/savio/project_existing_pi.html', 'new_pi': 'project/project_request/savio/project_new_pi.html', + 'pi_department': + 'project/project_request/savio/project_pi_department.html', 'ica_extra_fields': 'project/project_request/savio/project_ica_extra_fields.html', 'recharge_extra_fields': @@ -162,6 +173,7 @@ class SavioProjectRequestWizard(LoginRequiredMixin, UserPassesTestMixin, SavioProjectAllocationPeriodForm, SavioProjectExistingPIForm, SavioProjectNewPIForm, + NonAuthoritativeDepartmentSelectionForm, SavioProjectICAExtraFieldsForm, SavioProjectRechargeExtraFieldsForm, SavioProjectPoolAllocationsForm, @@ -247,6 +259,9 @@ def done(self, form_list, **kwargs): allocation_period = self.__get_allocation_period(form_data) pi = self.__handle_pi_data(form_data) + if self.__departments_enabled(): + self.__handle_pi_departments(form_data, pi) + if computing_allowance_wrapper.is_instructional(): self.__handle_ica_allowance( form_data, computing_allowance_wrapper, request_kwargs) @@ -285,6 +300,20 @@ def done(self, form_list, **kwargs): request = SavioProjectAllocationRequest.objects.create( **request_kwargs) + try: + if self.__departments_enabled(): + # Enqueue a task to update the PI's authoritative + # departments. This is purposefully placed outside the + # transaction to prevent it from closing during processing + # (when processing is synchronous). + func = ( + 'coldfront.plugins.departments.tasks.' + 'fetch_and_set_user_authoritative_departments') + async_task( + func, pi.pk, sync=settings.Q_CLUSTER.get('sync', False)) + except Exception as e: + self.logger.exception(e) + # Send a notification email to admins. try: send_new_project_request_admin_notification_email(request) @@ -321,12 +350,13 @@ def condition_dict(): return { '1': view.show_allocation_period_form_condition, '3': view.show_new_pi_form_condition, - '4': view.show_ica_extra_fields_form_condition, - '5': view.show_recharge_extra_fields_form_condition, - '6': view.show_pool_allocations_form_condition, - '7': view.show_pooled_project_selection_form_condition, - '8': view.show_details_form_condition, - '9': view.show_billing_id_form_condition, + '4': view.show_pi_department_form_condition, + '5': view.show_ica_extra_fields_form_condition, + '6': view.show_recharge_extra_fields_form_condition, + '7': view.show_pool_allocations_form_condition, + '8': view.show_pooled_project_selection_form_condition, + '9': view.show_details_form_condition, + '10': view.show_billing_id_form_condition, } def show_allocation_period_form_condition(self): @@ -369,11 +399,34 @@ def show_ica_extra_fields_form_condition(self): computing_allowance.requires_extra_information()) def show_new_pi_form_condition(self): + """Only show the form for providing details about a new PI if + the user did not select an existing PI.""" step_name = 'existing_pi' step = str(self.step_numbers_by_form_name[step_name]) cleaned_data = self.get_cleaned_data_for_step(step) or {} return cleaned_data.get('PI', None) is None + def show_pi_department_form_condition(self): + """Only show the form for providing department(s) for the PI + when the following are true: + - Department information is required. + - The PI does not already have any departments set. + """ + if not self.__departments_enabled(): + return False + + existing_pi_step = str(self.step_numbers_by_form_name['existing_pi']) + existing_pi_cleaned_data = self.get_cleaned_data_for_step( + existing_pi_step) or {} + if existing_pi_cleaned_data: + pi = existing_pi_cleaned_data['PI'] + authoritative_departments, non_authoritative_departments = \ + get_departments_for_user(pi) + if authoritative_departments or non_authoritative_departments: + return False + + return True + def show_pool_allocations_form_condition(self): step_name = 'computing_allowance' step = str(self.step_numbers_by_form_name[step_name]) @@ -404,10 +457,15 @@ def show_recharge_extra_fields_form_condition(self): @staticmethod def __billing_id_required(): """Return whether a billing ID should be requested from the - user. Ultimately, the form will only be included if pooling is - not requested.""" + user. The form for requesting it will be included based on + additional factors.""" return flag_enabled('LRC_ONLY') + @staticmethod + def __departments_enabled(): + """Return whether department functionality is enabled.""" + return flag_enabled('USER_DEPARTMENTS_ENABLED') + def __get_allocation_period(self, form_data): """Return the AllocationPeriod the user selected.""" step_number = self.step_numbers_by_form_name['allocation_period'] @@ -465,48 +523,66 @@ def __handle_pi_data(self, form_data): step_number = self.step_numbers_by_form_name['existing_pi'] data = form_data[step_number] if data['PI']: - return data['PI'] - - # Create a new User object intended to be a new PI. - step_number = self.step_numbers_by_form_name['new_pi'] - data = form_data[step_number] - email = data['email'] - try: - pi = User.objects.create( - username=email, - first_name=data['first_name'], - last_name=data['last_name'], - email=email, - is_active=True) - except IntegrityError as e: - self.logger.error(f'User {email} unexpectedly exists.') - raise e - - # Set the user's middle name in the UserProfile; generate a PI request. - try: + pi = data['PI'] pi_profile = pi.userprofile - except UserProfile.DoesNotExist as e: - self.logger.error( - f'User {email} unexpectedly has no UserProfile.') - raise e - pi_profile.middle_name = data['middle_name'] - pi_profile.upgrade_request = utc_now_offset_aware() - pi_profile.save() + else: + # Create a new User object intended to be a new PI. + step_number = self.step_numbers_by_form_name['new_pi'] + data = form_data[step_number] - # Create an unverified, primary EmailAddress for the new User object. - try: - EmailAddress.objects.create( - user=pi, - email=email, - verified=False, - primary=True) - except IntegrityError as e: - self.logger.error( - f'EmailAddress {email} unexpectedly already exists.') - raise e + try: + email = data['email'] + pi = User.objects.create( + username=email, + first_name=data['first_name'], + last_name=data['last_name'], + email=email, + is_active=False) + except IntegrityError as e: + self.logger.error(f'User {email} unexpectedly exists.') + raise e + + # Set user's middle name in the UserProfile; generate a PI request. + try: + pi_profile = pi.userprofile + except UserProfile.DoesNotExist as e: + self.logger.error( + f'User {email} unexpectedly has no UserProfile.') + raise e + pi_profile.middle_name = data['middle_name'] + pi_profile.upgrade_request = utc_now_offset_aware() + pi_profile.save() + + # Create an unverified, primary EmailAddress for the new User object. + try: + EmailAddress.objects.create( + user=pi, + email=email, + verified=False, + primary=True) + except IntegrityError as e: + self.logger.error( + f'EmailAddress {email} unexpectedly already exists.') + raise e return pi + def __handle_pi_departments(self, form_data, pi): + """Update the PI's non-authoritative departments to the + specified ones, if the requester was prompted to provide them. + Do not update the PI's authoritative departments.""" + step_number = self.step_numbers_by_form_name['pi_department'] + data = form_data[step_number] + non_authoritative_departments = data.get('departments', None) + + if non_authoritative_departments is None: + # The requester was not prompted to provide departments. + return + + user_department_updater = UserDepartmentUpdater( + pi, non_authoritative_departments) + user_department_updater.run(authoritative=False, non_authoritative=True) + def __handle_recharge_allowance(self, form_data, computing_allowance_wrapper, request_kwargs): @@ -611,7 +687,7 @@ def __set_data_from_previous_steps(self, step, dictionary): dictionary.update({ 'breadcrumb_pi': ( f'Existing PI: {pi.first_name} {pi.last_name} ' - f'({pi.email})') + f'({pi.email})'), }) else: first_name = new_pi_form_data['first_name'] diff --git a/coldfront/core/user/models.py b/coldfront/core/user/models.py index 6810cb02b..4f24f4a4c 100644 --- a/coldfront/core/user/models.py +++ b/coldfront/core/user/models.py @@ -12,7 +12,6 @@ from rest_framework.authtoken.models import Token from simple_history.models import HistoricalRecords - class UserProfile(models.Model): user = models.OneToOneField(User, on_delete=models.CASCADE) is_pi = models.BooleanField(default=False) @@ -43,7 +42,6 @@ class UserProfile(models.Model): history = HistoricalRecords() - class EmailAddress(models.Model): user = models.ForeignKey( User, on_delete=models.CASCADE, related_name='emailaddress_user') diff --git a/coldfront/core/user/templates/user/user_profile.html b/coldfront/core/user/templates/user/user_profile.html index 6904e6961..855130505 100644 --- a/coldfront/core/user/templates/user/user_profile.html +++ b/coldfront/core/user/templates/user/user_profile.html @@ -76,6 +76,43 @@

You do not have a cluster account. {% endif %} + + {% flag_enabled 'USER_DEPARTMENTS_ENABLED' as user_departments_enabled %} + {% if user_departments_enabled %} + + + {{ department_display_name }}s: + + + {% if auth_department_list|length == 0 and non_auth_department_list|length == 0 %} + - + {% else %} + + {% endif %} + {% if requester_is_viewed_user %} + + + Update + + {% endif %} + + + {% endif %} + @@ -97,6 +134,7 @@

+ Phone Number: diff --git a/coldfront/core/user/urls.py b/coldfront/core/user/urls.py index 81edea4f7..3dfe6ce39 100644 --- a/coldfront/core/user/urls.py +++ b/coldfront/core/user/urls.py @@ -9,6 +9,7 @@ from flags.urls import flagged_paths import coldfront.core.user.views as user_views +import coldfront.plugins.departments.views as department_views import coldfront.core.user.views_.link_login_views as link_login_views import coldfront.core.user.views_.request_hub_views as request_hub_views from coldfront.core.user.forms import VerifiedEmailAddressPasswordResetForm @@ -80,7 +81,6 @@ name='password-reset-complete'), ] - with flagged_paths('LINK_LOGIN_ENABLED') as f_path: urlpatterns += [ f_path('request-login-link/', @@ -91,6 +91,12 @@ name='link-login'), ] +with flagged_paths('USER_DEPARTMENTS_ENABLED') as f_path: + urlpatterns += [ + f_path('update-departments', + department_views.UpdateDepartmentsView.as_view(), + name='update-departments'), + ] with flagged_paths('SSO_ENABLED') as f_path: urlpatterns += [ diff --git a/coldfront/core/user/views.py b/coldfront/core/user/views.py index caf52d8bf..628a41591 100644 --- a/coldfront/core/user/views.py +++ b/coldfront/core/user/views.py @@ -42,6 +42,8 @@ from coldfront.core.utils.common import (import_from_settings, utc_now_offset_aware) +from coldfront.plugins.departments.utils.queries import get_departments_for_user + from flags.state import flag_enabled logger = logging.getLogger(__name__) @@ -89,6 +91,7 @@ def get_context_data(self, viewed_username=None, **kwargs): group_list = ', '.join( [group.name for group in viewed_user.groups.all()]) context['group_list'] = group_list + context['viewed_user'] = viewed_user context['has_cluster_access'] = has_cluster_access(viewed_user) @@ -96,18 +99,19 @@ def get_context_data(self, viewed_username=None, **kwargs): requester_is_viewed_user = viewed_user == self.request.user context['requester_is_viewed_user'] = requester_is_viewed_user - if requester_is_viewed_user: - self._update_context_with_identity_linking_request_data(context) - context['help_email'] = import_from_settings('CENTER_HELP_EMAIL') - context['requester_is_viewed_user'] = requester_is_viewed_user + if requester_is_viewed_user: + self._update_context_with_identity_linking_request_data(context) self._update_context_with_email_and_account_data(context, viewed_user) if self._flag_lrc_only: self._update_context_with_billing_data(context, viewed_user) + if self._flag_user_departments_enabled: + self._update_context_with_department_data(context, viewed_user) + context['is_lbl_employee'] = is_lbl_employee(viewed_user) return context @@ -119,12 +123,13 @@ def _get_flag_states(self): self._flag_multiple_email_addresses_allowed = flag_enabled( 'MULTIPLE_EMAIL_ADDRESSES_ALLOWED') self._flag_sso_enabled = flag_enabled('SSO_ENABLED') + self._flag_user_departments_enabled = flag_enabled( + 'USER_DEPARTMENTS_ENABLED') @staticmethod def _update_context_with_billing_data(context, viewed_user): """Update the given context dictionary with fields relating to - billing IDs. Take the currently-viewed User object as an input - to make determinations.""" + billing IDs.""" billing_id = 'N/A' try: user_profile = viewed_user.userprofile @@ -139,12 +144,21 @@ def _update_context_with_billing_data(context, viewed_user): billing_id = billing_activity.full_id() context['monthly_user_account_fee_billing_id'] = billing_id + @staticmethod + def _update_context_with_department_data(context, viewed_user): + """Update the given context dictionary with fields relating to + departments.""" + authoritative_department_strs, non_authoritative_department_strs = \ + get_departments_for_user(viewed_user, strs_only=True) + context['auth_department_list'] = authoritative_department_strs + context['non_auth_department_list'] = non_authoritative_department_strs + context['department_display_name'] = import_from_settings( + 'DEPARTMENTS_DEPARTMENT_DISPLAY_NAME') + def _update_context_with_email_and_account_data(self, context, viewed_user): """Update the given context directory with fields relating to - user emails, passwords, and third-party accounts. Take the - currently-viewed User object as an input to make - determinations.""" + user emails, passwords, and third-party accounts.""" requester_is_viewed_user = viewed_user == self.request.user context['change_password_enabled'] = ( diff --git a/coldfront/core/utils/flag_conditions.py b/coldfront/core/utils/flag_conditions.py index 43051db35..338c43728 100644 --- a/coldfront/core/utils/flag_conditions.py +++ b/coldfront/core/utils/flag_conditions.py @@ -1,7 +1,10 @@ -from coldfront.core.utils.common import display_time_zone_current_date +from django.apps import apps from django.core.exceptions import ValidationError + from flags import conditions +from coldfront.core.utils.common import display_time_zone_current_date + """Conditions that dynamically enable flags in django-flags.""" @@ -14,3 +17,8 @@ def validate_during_month(month_number_str): @conditions.register('during month', validator=validate_during_month) def during_month_condition(month_number_str, request=None, **kwargs): return int(month_number_str) == display_time_zone_current_date().month + + +@conditions.register('app installed') +def app_installed(app_label, request=None, **kwargs): + return apps.is_installed(app_label) diff --git a/coldfront/core/utils/management/commands/export_data.py b/coldfront/core/utils/management/commands/export_data.py index 7b545a5d3..7f494606f 100644 --- a/coldfront/core/utils/management/commands/export_data.py +++ b/coldfront/core/utils/management/commands/export_data.py @@ -6,18 +6,21 @@ from django.core.management.base import BaseCommand, CommandError from django.db.models import Value, F, CharField, Func, \ DurationField, ExpressionWrapper -from django.contrib.auth.models import User from coldfront.core.allocation.models import AllocationAttributeType, \ AllocationUserAttribute, AllocationRenewalRequest, AllocationPeriod from coldfront.core.statistics.models import Job from coldfront.core.project.models import Project, ProjectStatusChoice, \ SavioProjectAllocationRequest, VectorProjectAllocationRequest +from django.contrib.auth.models import User from coldfront.core.project.forms_.renewal_forms.request_forms import DeprecatedProjectRenewalSurveyForm from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.utils.common import display_time_zone_date_to_utc_datetime +from coldfront.plugins.departments.utils.queries import get_departments_for_user + from django import forms +from flags.state import flag_enabled """An admin command that exports the results of useful database queries in user-friendly formats.""" @@ -105,6 +108,19 @@ def add_subparsers(self, subparsers): help='Filter jobs by the partition they requested.', type=str) + user_subparser = subparsers.add_parser( + 'users', help='Export user data.') + user_subparser.add_argument( + '--format', + choices=['csv', 'json'], + required=True, + help='Export results in the given format.', + type=str) + user_subparser.add_argument( + '--pi_only', + help='Only return PI users.', + action='store_true') + project_subparser = subparsers.add_parser('projects', help='Export projects data') project_subparser.add_argument('--allowance_type', @@ -305,6 +321,60 @@ def handle_job_avg_queue_time(self, *args, **options): self.stdout.write(self.style.SUCCESS(time_str)) + def handle_users(self, *args, **kwargs): + + def _get_department_fields_for_user(_user): + """Return a list of two strs representing the given User's + authoritative and non-authoritative departments. Each str is + a semicolon-separated list of str representations of + departments.""" + authoritative_department_strs, non_authoritative_department_strs = \ + get_departments_for_user(_user) + authoritative_str = ';'.join( + [s for s in authoritative_department_strs]) + non_authoritative_str = ';'.join( + [s for s in non_authoritative_department_strs]) + return [authoritative_str, non_authoritative_str] + + _format = kwargs['format'] + pi_only = kwargs['pi_only'] + + users = User.objects.order_by('pk') + if pi_only: + users = users.filter(userprofile__is_pi=True) + + include_departments = flag_enabled('USER_DEPARTMENTS_ENABLED') + + fields = [ + 'id', 'username', 'first_name', 'last_name', 'email', 'is_active'] + + all_user_data = [] + for user in users: + user_data = [getattr(user, field) for field in fields] + if include_departments: + user_data.extend(_get_department_fields_for_user(user)) + all_user_data.append(user_data) + + if include_departments: + department_fields = [ + 'authoritative_departments', 'non_authoritative_departments'] + fields.extend(department_fields) + + if _format == 'csv': + self.to_csv( + all_user_data, + header=fields, + output=kwargs.get('stdout', stdout), + error=kwargs.get('stderr', stderr)) + elif _format == 'json': + json_objects = [] + for user_data in all_user_data: + json_objects.append(dict(zip(fields, user_data))) + self.to_json( + json_objects, + output=kwargs.get('stdout', stdout), + error=kwargs.get('stderr', stderr)) + def handle_projects(self, *args, **kwargs): format = kwargs['format'] active_only = kwargs['active_only'] @@ -576,8 +646,6 @@ def _swap_form_answer_id_for_text(_survey, _multiple_choice_fields): self.to_json(surveys, output=kwargs.get('stdout', stdout), error=kwargs.get('stderr', stderr)) - - @staticmethod def to_csv(query_set, header=None, output=stdout, error=stderr): diff --git a/coldfront/core/utils/templatetags/common_tags.py b/coldfront/core/utils/templatetags/common_tags.py index e0bb8218b..5d8bdc3c5 100644 --- a/coldfront/core/utils/templatetags/common_tags.py +++ b/coldfront/core/utils/templatetags/common_tags.py @@ -15,6 +15,7 @@ def settings_value(name): 'CENTER_HELP_EMAIL', 'CENTER_HELP_URL', 'CENTER_USER_GUIDE', + 'DEPARTMENTS_DEPARTMENT_DISPLAY_NAME', 'DISPLAY_TIME_ZONE', 'EMAIL_PROJECT_REVIEW_CONTACT', 'PORTAL_NAME', diff --git a/coldfront/core/utils/tests/test_base.py b/coldfront/core/utils/tests/test_base.py index 71272a342..569c2cc99 100644 --- a/coldfront/core/utils/tests/test_base.py +++ b/coldfront/core/utils/tests/test_base.py @@ -13,6 +13,7 @@ from django.test import Client from django.test import override_settings from django.test import TestCase +from django.test import TransactionTestCase from django.test.utils import TestContextDecorator from django.urls import reverse @@ -31,8 +32,9 @@ from coldfront.core.utils.common import utc_now_offset_aware -class TestBase(TestCase): - """A base class for testing the application.""" +class BaseTestMixin(object): + """A mixin with useful methods for testing the application, to be + incorporated into a class inheriting from e.g., TestCase.""" # A password for convenient reference. password = 'password' @@ -167,6 +169,17 @@ def sign_user_access_agreement(user): user_profile.save() +class TestBase(BaseTestMixin, TestCase): + """A base class for testing the application.""" + pass + + +class TransactionTestBase(BaseTestMixin, TransactionTestCase): + """A base class for testing the application, with real database + transactions.""" + pass + + class enable_deployment(TestContextDecorator): """A class that enables the deployment with the given name and disables the other, and then reverts to the previous states when diff --git a/coldfront/core/utils/tests/test_export_data.py b/coldfront/core/utils/tests/test_export_data.py index 237774cb9..e1eeffcfe 100644 --- a/coldfront/core/utils/tests/test_export_data.py +++ b/coldfront/core/utils/tests/test_export_data.py @@ -281,7 +281,8 @@ def test_json_no_date(self): '--format=json') output = self.convert_output(output, 'json') - post_time = utc_now_offset_aware().replace(tzinfo=None, microsecond=0) + post_time = utc_now_offset_aware().replace(tzinfo=None, + microsecond=999999) for index, item in enumerate(output): self.assertEqual(item['username'], f'user{index}') date_created = \ @@ -320,7 +321,8 @@ def test_json_with_date(self): output = self.convert_output(output, 'json') # this should only output the cluster account creation for user1 - post_time = utc_now_offset_aware().replace(tzinfo=None, microsecond=0) + post_time = utc_now_offset_aware().replace(tzinfo=None, + microsecond=999999) self.assertEqual(len(output), 1) self.assertEqual(output[0]['username'], 'user1') date_created = \ @@ -340,7 +342,8 @@ def test_csv_no_date(self): '--format=csv') output = self.convert_output(output, 'csv') - post_time = utc_now_offset_aware().replace(tzinfo=None, microsecond=999999) + post_time = utc_now_offset_aware().replace(tzinfo=None, + microsecond=999999) for index, item in enumerate(output): if index == 0: self.assertEqual(item, ['username', 'date_created']) @@ -381,7 +384,8 @@ def test_csv_with_date(self): f'--start_date={start_date}') output = self.convert_output(output, 'csv') - post_time = utc_now_offset_aware().replace(tzinfo=None, microsecond=0) + post_time = utc_now_offset_aware().replace(tzinfo=None, + microsecond=999999) for index, item in enumerate(output): if index == 0: self.assertEqual(item, ['username', 'date_created']) diff --git a/coldfront/plugins/departments/__init__.py b/coldfront/plugins/departments/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/admin.py b/coldfront/plugins/departments/admin.py new file mode 100644 index 000000000..321ab3c91 --- /dev/null +++ b/coldfront/plugins/departments/admin.py @@ -0,0 +1,64 @@ +from django.contrib import admin +from coldfront.plugins.departments.models import Department, UserDepartment + +# Register your models here. + + +class DepartmentHasUsersFilter(admin.SimpleListFilter): + title = 'Has Users' + parameter_name = 'has_users' + + def lookups(self, request, model_admin): + """ + Returns a list of tuples. The first element in each + tuple is the coded value for the option that will + appear in the URL query. The second element is the + human-readable name for the option that will appear + in the right sidebar. + """ + return ( + ('yes', 'Yes'), + ('no', 'No'), + ) + + def queryset(self, request, queryset): + """ + Returns the filtered queryset based on the value + provided in the query string and retrievable via + `self.value()`. + """ + if self.value() == 'yes': + return queryset.exclude(userdepartment__isnull=True) + if self.value() == 'no': + return queryset.filter(userdepartment__isnull=True) + + +@admin.register(Department) +class DepartmentAdmin(admin.ModelAdmin): + list_display = ('name', 'code') + ordering = ('name', 'code') + search_fields = ['name', 'code'] + list_filter = (DepartmentHasUsersFilter,) + + +@admin.register(UserDepartment) +class UserDepartmentAdmin(admin.ModelAdmin): + list_display = ('department', 'username', 'first_name', 'last_name', 'is_authoritative') + ordering = ('department', 'is_authoritative', 'user__username') + list_filter = ('user__userprofile__is_pi', 'is_authoritative') + search_fields = ['department__name', 'department__code', + 'user__username', + 'user__first_name', + 'user__last_name', + 'user__email'] + fields = ('user', 'department', 'is_authoritative') + readonly_fields = ('user', 'username') + + def username(self, obj): + return obj.user.username + + def first_name(self, obj): + return obj.user.first_name + + def last_name(self, obj): + return obj.user.last_name diff --git a/coldfront/plugins/departments/apps.py b/coldfront/plugins/departments/apps.py new file mode 100644 index 000000000..e45071be7 --- /dev/null +++ b/coldfront/plugins/departments/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class DepartmentsConfig(AppConfig): + default_auto_field = 'django.db.models.BigAutoField' + name = 'coldfront.plugins.departments' diff --git a/coldfront/plugins/departments/conf/__init__.py b/coldfront/plugins/departments/conf/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/conf/settings.py b/coldfront/plugins/departments/conf/settings.py new file mode 100644 index 000000000..f0e6f7a7e --- /dev/null +++ b/coldfront/plugins/departments/conf/settings.py @@ -0,0 +1,9 @@ +from django.conf import settings as django_settings + + +DEPARTMENT_DISPLAY_NAME = getattr( + django_settings, 'DEPARTMENTS_DEPARTMENT_DISPLAY_NAME', 'Departments') + +DEPARTMENT_DATA_SOURCE = getattr( + django_settings, 'DEPARTMENTS_DEPARTMENT_DATA_SOURCE', + 'coldfront.plugins.departments.utils.data_sources.backends.dummy.DummyDataSourceBackend') diff --git a/coldfront/plugins/departments/forms.py b/coldfront/plugins/departments/forms.py new file mode 100644 index 000000000..b1cf36a1c --- /dev/null +++ b/coldfront/plugins/departments/forms.py @@ -0,0 +1,60 @@ +from django import forms +from django.contrib.auth.models import User + +from coldfront.plugins.departments.conf import settings +from coldfront.plugins.departments.models import Department +from coldfront.plugins.departments.models import UserDepartment + + +class NonAuthoritativeDepartmentSelectionForm(forms.Form): + """A form that allows the user to select the departments that they + are non-authoritatively associated with.""" + + departments = forms.ModelMultipleChoiceField( + label='Departments', + queryset=Department.objects.order_by('name').all(), + required=True) + + def __init__(self, *args, **kwargs): + """If a User object is provided, tailor choices to the user.""" + user = kwargs.pop('user', None) + self.user = user + + super().__init__(*args, **kwargs) + + if isinstance(self.user, User): + self._disable_department_choices() + self._set_initial_departments() + + self.fields['departments'].label = ( + f'{settings.DEPARTMENT_DISPLAY_NAME}s') + + def _disable_department_choices(self): + """Prevent certain Departments, which should be displayed, from + being selected.""" + disable_department_pks = set() + + # Disable any Department that the User is authoritatively associated + # with. + authoritative_user_departments = UserDepartment.objects.filter( + user=self.user, + is_authoritative=True) + authoritative_department_pks = set( + authoritative_user_departments.values_list( + 'department__pk', flat=True)) + disable_department_pks.update(authoritative_department_pks) + + self.fields['departments'].widget.disabled_choices = \ + disable_department_pks + + def _set_initial_departments(self): + """Pre-select the Departments that the user is non- + authoritatively associated with.""" + non_authoritative_user_departments = UserDepartment.objects.filter( + user=self.user, + is_authoritative=False) + non_authoritative_department_pks = list( + non_authoritative_user_departments.values_list( + 'department__pk', flat=True)) + self.fields['departments'].initial = Department.objects.filter( + pk__in=non_authoritative_department_pks) diff --git a/coldfront/plugins/departments/management/__init__.py b/coldfront/plugins/departments/management/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/management/commands/__init__.py b/coldfront/plugins/departments/management/commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/management/commands/load_departments.py b/coldfront/plugins/departments/management/commands/load_departments.py new file mode 100644 index 000000000..8c29406fe --- /dev/null +++ b/coldfront/plugins/departments/management/commands/load_departments.py @@ -0,0 +1,52 @@ +import logging + +from django.core.management import BaseCommand + +from coldfront.core.utils.common import add_argparse_dry_run_argument + +from coldfront.plugins.departments.models import Department +from coldfront.plugins.departments.utils.data_sources import fetch_departments + + +class Command(BaseCommand): + + help = ( + 'Fetch department data from the configured data source, and ' + 'create Departments in the database.') + logger = logging.getLogger(__name__) + + def add_arguments(self, parser): + add_argparse_dry_run_argument(parser) + + def log(self, message, dry_run): + if not dry_run: + self.logger.info(message) + self.stdout.write(self.style.SUCCESS(message)) + + def handle(self, *args, **options): + dry_run = options['dry_run'] + + if dry_run: + department = Department() + department.pk = '{placeholder_pk}' + created = not Department.objects.filter(code='OTH').exists() + else: + department, created = Department.objects.get_or_create( + code='OTH', name='Other') + if created: + message = f'Created Department {department}.' + self.log(message, dry_run) + + department_entries = fetch_departments() + for code, name in department_entries: + department = Department() + if dry_run: + created = not Department.objects.filter(code=code).exists() + department.name = name + department.code = code + else: + department, created = Department.objects.get_or_create( + name=name, code=code) + if created: + message = f'Created Department {department}.' + self.log(message, dry_run) diff --git a/coldfront/plugins/departments/management/commands/load_user_departments.py b/coldfront/plugins/departments/management/commands/load_user_departments.py new file mode 100644 index 000000000..581967413 --- /dev/null +++ b/coldfront/plugins/departments/management/commands/load_user_departments.py @@ -0,0 +1,62 @@ +from django.contrib.auth.models import User +from django.core.management import BaseCommand + +from allauth.account.models import EmailAddress + +from coldfront.core.utils.common import add_argparse_dry_run_argument + +from coldfront.plugins.departments.models import UserDepartment +from coldfront.plugins.departments.utils import UserInfoDict +from coldfront.plugins.departments.utils.data_sources import fetch_departments_for_user +from coldfront.plugins.departments.utils.data_sources import get_data_source +from coldfront.plugins.departments.utils.queries import create_or_update_department + + +class Command(BaseCommand): + + help = ( + 'Fetch data about departments associated with users from the ' + 'configured data source, and create UserDepartments in the database.') + + def add_arguments(self, parser): + parser.add_argument( + '--only_pis', + action='store_true', + help='Only populate the departments of users who are PIs.') + add_argparse_dry_run_argument(parser) + + def handle(self, *args, **options): + only_pis = options['only_pis'] + dry_run = options['dry_run'] + users = User.objects.select_related('userprofile') + if only_pis: + users = users.filter(userprofile__is_pi=True) + + # TODO: Account for dry_run. + + data_source = get_data_source() + + # A mapping from Department code to the corresponding object. + department_by_code = {} + + for user in users: + # TODO: This is going to get duplicated. Make a function. + user_data = UserInfoDict.from_user(user) + user_department_data = fetch_departments_for_user( + user_data, data_source=data_source) + + for code, name in user_department_data: + + if code not in department_by_code: + department, department_created = \ + create_or_update_department(code, name) + department_by_code[code] = department + department = department_by_code[code] + + user_department, user_department_created = \ + UserDepartment.objects.update_or_create( + user=user, + department=department, + defaults={ + 'is_authoritative': True, + }) diff --git a/coldfront/plugins/departments/migrations/0001_initial.py b/coldfront/plugins/departments/migrations/0001_initial.py new file mode 100644 index 000000000..12120b3b5 --- /dev/null +++ b/coldfront/plugins/departments/migrations/0001_initial.py @@ -0,0 +1,58 @@ +# Generated by Django 3.2.5 on 2024-11-11 17:15 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import simple_history.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='Department', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=128)), + ('code', models.CharField(max_length=5, unique=True)), + ], + ), + migrations.CreateModel( + name='HistoricalUserDepartment', + fields=[ + ('id', models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('is_authoritative', models.BooleanField(default=False)), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField()), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('department', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='departments.department')), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'historical user department', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': 'history_date', + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + migrations.CreateModel( + name='UserDepartment', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('is_authoritative', models.BooleanField(default=False)), + ('department', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='departments.department')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'unique_together': {('user', 'department')}, + }, + ), + ] diff --git a/coldfront/plugins/departments/migrations/__init__.py b/coldfront/plugins/departments/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/models.py b/coldfront/plugins/departments/models.py new file mode 100644 index 000000000..120ea327d --- /dev/null +++ b/coldfront/plugins/departments/models.py @@ -0,0 +1,27 @@ +from django.contrib.auth.models import User +from django.db import models + +from simple_history.models import HistoricalRecords + + +class Department(models.Model): + + name = models.CharField(max_length=128) + code = models.CharField(max_length=5, unique=True) + + def __str__(self): + return f'{self.name} ({self.code})' + + +class UserDepartment(models.Model): + + user = models.ForeignKey(User, on_delete=models.CASCADE) + department = models.ForeignKey(Department, on_delete=models.CASCADE) + is_authoritative = models.BooleanField(default=False) + history = HistoricalRecords() + + def __str__(self): + return f'{self.user.username}-{self.department.code}' + + class Meta: + unique_together = ('user', 'department') diff --git a/coldfront/plugins/departments/tasks.py b/coldfront/plugins/departments/tasks.py new file mode 100644 index 000000000..3770af612 --- /dev/null +++ b/coldfront/plugins/departments/tasks.py @@ -0,0 +1,16 @@ +from django.contrib.auth.models import User + +from coldfront.plugins.departments.utils.queries import UserDepartmentUpdater + + +"""Functions designed to be run as Django-Q tasks.""" + + +def fetch_and_set_user_authoritative_departments(user_pk): + """Attempt to fetch departments for the User with the given primary + key from the configured data source, and create authoritative + UserDepartments.""" + user = User.objects.get(pk=user_pk) + + user_department_updater = UserDepartmentUpdater(user, []) + user_department_updater.run(authoritative=True, non_authoritative=False) diff --git a/coldfront/plugins/departments/templates/user_update_departments.html b/coldfront/plugins/departments/templates/user_update_departments.html new file mode 100644 index 000000000..27b658da1 --- /dev/null +++ b/coldfront/plugins/departments/templates/user_update_departments.html @@ -0,0 +1,65 @@ +{% extends "common/base.html" %} +{% load feature_flags %} +{% load crispy_forms_tags %} +{% load static %} + + +{% block title %} +Update {{ department_display_name }}s +{% endblock %} + +{% block head %} +{{ wizard.form.media }} +{% endblock %} + +{% block content %} + +

Update Departments

+
+ + + + + +

Select the {{ department_display_name | lower }}s you are associated with. +Deselect a {{ department_display_name | lower }} to remove it from your associations.

+ +

External collaborators should select "Other".

+ +{% if auth_department_list %} +

The following {{ department_display_name | lower }}s are verified by the institution, and may not be updated.

+ +

+{% endif %} + +
+
+
+ {% csrf_token %} + {{ form|crispy }} + + + Cancel + +
+
+
+ + + + +{% endblock %} diff --git a/coldfront/plugins/departments/tests/__init__.py b/coldfront/plugins/departments/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/tests/test_views/__init__.py b/coldfront/plugins/departments/tests/test_views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/tests/test_views/test_update_departments_view.py b/coldfront/plugins/departments/tests/test_views/test_update_departments_view.py new file mode 100644 index 000000000..41bd09e01 --- /dev/null +++ b/coldfront/plugins/departments/tests/test_views/test_update_departments_view.py @@ -0,0 +1,345 @@ +from copy import deepcopy +from http import HTTPStatus +from importlib import reload +from unittest.mock import patch + +from django.conf import settings as django_settings +from django.test import override_settings +from django.urls import reverse + +from coldfront.core.utils.tests.test_base import TransactionTestBase +from coldfront.plugins.departments.conf import settings +from coldfront.plugins.departments.models import Department +from coldfront.plugins.departments.models import HistoricalUserDepartment +from coldfront.plugins.departments.models import UserDepartment + + +Q_CLUSTER_COPY = deepcopy(django_settings.Q_CLUSTER) +Q_CLUSTER_COPY['sync'] = True + + +@override_settings( + Q_CLUSTER=Q_CLUSTER_COPY, + DEPARTMENTS_DEPARTMENT_DATA_SOURCE=( + 'coldfront.plugins.departments.utils.data_sources.backends.dummy.' + 'DummyDataSourceBackend') +) +class TestUpdateDepartmentsView(TransactionTestBase): + """A class for testing UpdateDepartmentsView.""" + + def setUp(self): + """Set up test data.""" + super().setUp() + + # conf.settings needs to be reloaded after the override. + reload(settings) + + self.create_test_user() + self.client.login(username=self.user.username, password=self.password) + + self.department1, _ = Department.objects.get_or_create( + name='Department 1', code='DEPT1') + self.department2, _ = Department.objects.get_or_create( + name='Department 2', code='DEPT2') + self.department3, _ = Department.objects.get_or_create( + name='Department 3', code='DEPT3') + + def _assert_historical_user_departments(self, user, department, + expected_history_types): + """Assert that there are HistoricalUserDepartments between the + given User and Department, with history types matching the ones + in the given ordered list.""" + historical_user_departments = HistoricalUserDepartment.objects.filter( + user=user, department=department).order_by('history_id') + self.assertEqual( + historical_user_departments.count(), len(expected_history_types)) + actual_history_types = list( + historical_user_departments.values_list('history_type', flat=True)) + self.assertEqual(actual_history_types, expected_history_types) + + def _assert_user_departments(self, user, + expected_departments_name_and_is_authoritative): + """Given a User and a dict mapping the names of departments the + user is expected to be in to whether the association is expected + to be authoritative, return whether the user is in those + departments with the expected association.""" + user_departments = UserDepartment.objects.filter(user=user) + for user_department in user_departments: + department_name = user_department.department.name + self.assertIn( + department_name, expected_departments_name_and_is_authoritative) + expected_is_authoritative = \ + expected_departments_name_and_is_authoritative.pop( + department_name) + self.assertEqual( + user_department.is_authoritative, expected_is_authoritative) + self.assertFalse(expected_departments_name_and_is_authoritative) + + def _assert_redirects(self, response): + """Assert that the given response involves being redirected to + the expected success URL (the user profile).""" + success_url = reverse('user-profile') + self.assertEqual(response.status_code, HTTPStatus.FOUND) + self.assertRedirects(response, success_url) + + @staticmethod + def _request_url(): + """Return the URL for requesting to update the user's + departments.""" + return reverse('update-departments') + + def test_departments_required(self): + """Test that an error is raised if no departments are provided.""" + UserDepartment.objects.create( + user=self.user, + department=self.department1) + + form_data = { + 'departments': [], + } + response = self.client.post(self._request_url(), form_data) + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertContains(response, 'This field is required.') + + self.assertEqual(UserDepartment.objects.count(), 1) + + try: + UserDepartment.objects.get( + user=self.user, department=self.department1) + except UserDepartment.DoesNotExist: + self.fail('The user\'s UserDepartment should not have been unset.') + + def test_sets_selected_departments_non_authoritatively(self): + """Test that a POST request updates the user's non-authoritative + departments, retaining history.""" + self.user.first_name = '' + self.user.last_name = '' + self.user.save() + + self.assertEqual(UserDepartment.objects.count(), 0) + self.assertEqual(HistoricalUserDepartment.objects.count(), 0) + + # Set department1. + form_data = { + 'departments': [self.department1.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + user_departments = UserDepartment.objects.filter(user=self.user) + self.assertEqual(user_departments.count(), 1) + + user_department_1 = user_departments.first() + self.assertEqual(user_department_1.department, self.department1) + self.assertFalse(user_department_1.is_authoritative) + + self._assert_historical_user_departments( + self.user, self.department1, ['+']) + + # Set department2 and department3, unsetting department 1. + form_data = { + 'departments': [self.department2.pk, self.department3.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + user_departments = UserDepartment.objects.filter(user=self.user) + self.assertEqual(user_departments.count(), 2) + + user_department_2 = user_departments.get(department=self.department2) + user_department_3 = user_departments.get(department=self.department3) + self.assertFalse(user_department_2.is_authoritative) + self.assertFalse(user_department_3.is_authoritative) + + self._assert_historical_user_departments( + self.user, self.department1, ['+', '-']) + self._assert_historical_user_departments( + self.user, self.department2, ['+']) + self._assert_historical_user_departments( + self.user, self.department3, ['+']) + + # Set department2, unsetting department3. + form_data = { + 'departments': [self.department2.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + user_departments = UserDepartment.objects.filter(user=self.user) + self.assertEqual(user_departments.count(), 1) + + user_department_2 = user_departments.get(department=self.department2) + self.assertFalse(user_department_2.is_authoritative) + + self._assert_historical_user_departments( + self.user, self.department1, ['+', '-']) + self._assert_historical_user_departments( + self.user, self.department2, ['+', '~']) + self._assert_historical_user_departments( + self.user, self.department3, ['+', '-']) + + @override_settings( + DEPARTMENTS_DEPARTMENT_DATA_SOURCE=( + 'coldfront.plugins.departments.utils.data_sources.backends.dummy.' + 'DummyDataSourceBackend')) + def test_creates_data_source_departments_and_sets_authoritatively(self): + """Test that a POST request attempts to fetch departments from a + data source backend, creates Departments, and updates the user's + authoritative departments.""" + self.user.first_name = 'First' + self.user.last_name = 'Last' + self.user.save() + + self.assertFalse(UserDepartment.objects.filter(user=self.user).exists()) + self.assertFalse( + Department.objects.filter(name='Department F').exists()) + self.assertFalse( + Department.objects.filter(name='Department L').exists()) + + form_data = { + 'departments': [self.department1.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + self.assertTrue(Department.objects.filter(name='Department F').exists()) + self.assertTrue(Department.objects.filter(name='Department L').exists()) + + expected_departments_name_and_is_authoritative = { + 'Department 1': False, + 'Department F': True, + 'Department L': True, + } + self._assert_user_departments( + self.user, expected_departments_name_and_is_authoritative) + + def test_no_departments_found_in_data_source(self): + """Test that, if no departments for the user are found in the + data source backend, none are set authoritatively.""" + self.user.first_name = '1' + self.user.last_name = '2' + self.user.save() + + self.assertFalse(UserDepartment.objects.filter(user=self.user).exists()) + + form_data = { + 'departments': [self.department1.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + expected_departments_name_and_is_authoritative = { + 'Department 1': False, + } + self._assert_user_departments( + self.user, expected_departments_name_and_is_authoritative) + + def test_authoritative_not_overridden_as_non_authoritative(self): + """Test that, if a department is set authoritatively for a user, + the user cannot set it as non-authoritative.""" + self.user.first_name = 'First' + self.user.last_name = 'Last' + self.user.save() + + # The user is not already associated with Department F. + department_f = Department.objects.create( + name='Department F', code='DEPTF') + + # The user attempts to set F as non-authoritative. + form_data = { + 'departments': [self.department1.pk, department_f.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + # F is still set authoritatively. + expected_departments_name_and_is_authoritative = { + 'Department 1': False, + 'Department F': True, + 'Department L': True, + } + self._assert_user_departments( + self.user, expected_departments_name_and_is_authoritative) + + # The user is already associated with Department F. The user attempts + # to set F as non-authoritative. + form_data = { + 'departments': [self.department1.pk, department_f.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + # F is still set authoritatively. + expected_departments_name_and_is_authoritative = { + 'Department 1': False, + 'Department F': True, + 'Department L': True, + } + self._assert_user_departments( + self.user, expected_departments_name_and_is_authoritative) + + def test_outdated_authoritative_departments_unset(self): + """Test that the view removes any authoritatively-associated + departments that are no longer returned by the data source.""" + self.user.first_name = 'First' + self.user.last_name = 'Last' + self.user.save() + + # The user is already authoritatively associated with Department M. + department_m = Department.objects.create( + name='Department M', code='DEPTM') + UserDepartment.objects.create( + user=self.user, department=department_m, is_authoritative=True) + + form_data = { + 'departments': [self.department1.pk], + } + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + # M is no longer set. + expected_departments_name_and_is_authoritative = { + 'Department 1': False, + 'Department F': True, + 'Department L': True, + } + self._assert_user_departments( + self.user, expected_departments_name_and_is_authoritative) + + def test_data_source_error_authoritative_unaffected(self): + """Test that, if an error occurs when fetching department data + from the data source, authoritative department associations are + unaffected.""" + self.user.first_name = 'First' + self.user.last_name = 'Last' + self.user.save() + + # The user is already authoritatively associated with Department M. + department_m = Department.objects.create( + name='Department M', code='DEPTM') + UserDepartment.objects.create( + user=self.user, department=department_m, is_authoritative=True) + + form_data = { + 'departments': [self.department1.pk], + } + + def raise_exception(*args, **kwargs): + raise Exception('Test exception.') + + method_to_patch = ( + 'coldfront.plugins.departments.utils.queries.' + 'fetch_departments_for_user') + with patch(method_to_patch) as patched_method: + patched_method.side_effect = raise_exception + response = self.client.post(self._request_url(), form_data) + self._assert_redirects(response) + + # M is still set authoritatively. + expected_departments_name_and_is_authoritative = { + 'Department 1': False, + 'Department M': True, + } + self._assert_user_departments( + self.user, expected_departments_name_and_is_authoritative) diff --git a/coldfront/plugins/departments/utils/__init__.py b/coldfront/plugins/departments/utils/__init__.py new file mode 100644 index 000000000..1ffd67934 --- /dev/null +++ b/coldfront/plugins/departments/utils/__init__.py @@ -0,0 +1,31 @@ +from allauth.account.models import EmailAddress + + +__all__ = [ + 'UserInfoDict', +] + + +class UserInfoDict(dict): + """A dict of identifying information about a user, for the purpose + of fetching departments.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.__dict__ = self + + assert 'emails' in self + assert 'first_name' in self + assert 'last_name' in self + + @classmethod + def from_user(cls, user): + """Instantiate from the given User.""" + user_data = { + 'emails': list( + EmailAddress.objects.filter(user=user).values_list( + 'email', flat=True)), + 'first_name': user.first_name, + 'last_name': user.last_name, + } + return cls(**user_data) diff --git a/coldfront/plugins/departments/utils/data_sources/__init__.py b/coldfront/plugins/departments/utils/data_sources/__init__.py new file mode 100644 index 000000000..198b1dc48 --- /dev/null +++ b/coldfront/plugins/departments/utils/data_sources/__init__.py @@ -0,0 +1,28 @@ +from django.utils.module_loading import import_string + +from coldfront.plugins.departments.conf import settings + + +"""Methods relating to fetching of department data.""" + + +__all__ = [ + 'get_data_source', + 'fetch_departments', + 'fetch_departments_for_user', +] + + +def get_data_source(data_source=None, **kwds): + klass = import_string(data_source or settings.DEPARTMENT_DATA_SOURCE) + return klass(**kwds) + + +def fetch_departments(data_source=None): + data_source = data_source or get_data_source() + return data_source.fetch_departments() + + +def fetch_departments_for_user(user_data, data_source=None): + data_source = data_source or get_data_source() + return data_source.fetch_departments_for_user(user_data) diff --git a/coldfront/plugins/departments/utils/data_sources/backends/__init__.py b/coldfront/plugins/departments/utils/data_sources/backends/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/plugins/departments/utils/data_sources/backends/base.py b/coldfront/plugins/departments/utils/data_sources/backends/base.py new file mode 100644 index 000000000..2a30ae266 --- /dev/null +++ b/coldfront/plugins/departments/utils/data_sources/backends/base.py @@ -0,0 +1,41 @@ +from abc import ABC +from abc import abstractmethod + + +class BaseDataSourceBackend(ABC): + """An interface for fetching department data using any of a number + of backends.""" + + @abstractmethod + def fetch_departments(self): + """Return a generator of all departments as tuples of the form + (department identifier (str), department description (str)). + + Returns: + - Generator of tuples of the form (department identifier + (str), department description (str)) + """ + pass + + @abstractmethod + def fetch_departments_for_user(self, user_data): + """Return a generator of departments associated with the given + dict, which represents a user, as tuples of the form (department + identifier (str), department description (str)). + + Parameters: + - user_data (dict): A dict with the following format: + { + 'first_name': 'user first name', + 'last_name': 'user last name', + 'emails': [ + 'email1@berkeley.edu', + 'email2@berkeley.edu', + ] + } + + Returns: + - Generator of tuples of the form (department identifier + (str), department description (str)) + """ + pass diff --git a/coldfront/plugins/departments/utils/data_sources/backends/calnet_ldap.py b/coldfront/plugins/departments/utils/data_sources/backends/calnet_ldap.py new file mode 100644 index 000000000..e461f314d --- /dev/null +++ b/coldfront/plugins/departments/utils/data_sources/backends/calnet_ldap.py @@ -0,0 +1,245 @@ +from ldap3 import Connection + +from .base import BaseDataSourceBackend + + +class CalNetLdapDataSourceBackend(BaseDataSourceBackend): + """A backend that fetches department data from UC Berkeley's CalNet + LDAP directory service. + + https://calnet.berkeley.edu/calnet-technologists/ldap-directory-service + + TODO: Discuss structure, which org units are considered departments + in our case, etc. Give examples, etc. + + The departments of interest are "L4" org units. + """ + + DIRECTORY_URL = 'ldap.berkeley.edu' + # The distinguished name (DN) of the "people" organizational unit. + PEOPLE_DN = 'ou=people,dc=berkeley,dc=edu' + # The distinguished name (DN) of the "org units" organizational unit. + ORG_UNITS_OU = 'ou=org units,dc=berkeley,dc=edu' + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._connection = Connection( + self.DIRECTORY_URL, auto_bind=True, auto_range=True) + # A mapping from the name of an "org units" OU to a tuple of the + # identifier and description for the OU's department. + self._cache_department_data_by_ou = {} + + def fetch_departments(self): + """Return a generator of UC Berkeley departments, represented as + tuples. + + Departments are org units at level 4 (L4) of the org tree. + """ + identifier_attr = 'berkeleyEduOrgUnitHierarchyString' + # This search filter returns L4 org units. + # The first portion includes org units with at least three hyphens (L4 + # and above). + # The second portion excludes org units with at least four hyphens (L5 + # and above). + search_filter = ( + f'(&({identifier_attr}=*-*-*-*)(!({identifier_attr}=*-*-*-*-*)))') + for identifier, description in self._lookup_org_units(search_filter): + yield identifier.split('-')[3], description + + def fetch_departments_for_user(self, user_data): + """Return a generator of UC Berkeley departments, represented as + tuples, associated with the given person, represented as a dict. + + Search for the person in the "people" OU, as follows: + 1. Attempt to find entries for the person from associated + email addresses. + 2. If and only if none are found, attempt to find an entry + for the person from first and last name. Only use an + entry if it is the only one (i.e., not if there are + multiple people with the same name). + + The resulting entries list department numbers associated with + the person. Look up and return the corresponding L4 org units + from the "org units" OU. + + A mapping from department number to the corresponding L4 org + units is cached within the instance to reduce redundant LDAP + lookups. + """ + # Due to short-circuiting, if the email lookup produces results, the + # name lookup is skipped. + results = ( + self._lookup_person_department_numbers_from_emails( + user_data['emails']) + or + self._lookup_person_department_numbers_from_name( + user_data['first_name'], user_data['last_name'])) + + for department_number in results: + if department_number not in self._cache_department_data_by_ou: + identifier, description = \ + self._lookup_department_info_for_org_unit(department_number) + self._cache_department_data_by_ou[department_number] = ( + identifier, description) + identifier, description = self._cache_department_data_by_ou[ + department_number] + if identifier is not None and description is not None: + yield identifier, description + + def _lookup_department_info_for_org_unit(self, ou): + """Given the identifier for an org unit at the department (L4) + level or above (higher levels are deeper in the org tree), + return the identifier and description for its department. Return + None for both if no matching OU is found. + + Parameters: + - ou (str): The identifier for an org unit (e.g., "JICCS") + + Returns: + - Two strs denoting the identifier and description for the + department that the org unit is (a) under or (b) identical + to + - None, None if no matching OU is found + + Raises: + - ValueError, if the provided OU is not at least as deep as + the department (L4) level. + """ + search_filter = f'(&(objectClass=organizationalUnit)(ou={ou}))' + for identifier, description in self._lookup_org_units(search_filter): + if identifier.count('-') < 3: + raise ValueError( + f'Org unit "{ou}" is broader than the department level.') + # There should only be one result. + return identifier.split('-')[3], description + return None, None + + def _lookup_org_units(self, search_filter): + """Given an LDAP search filter for the "org units" OU, return a + generator of tuples containing the hierarchy string and + description for each matching org unit. Return an empty list if + no matching org units are found. + + Parameters: + - search_filter (str): An LDAP search filter to be used to + search the "org units" OU + + Returns: + - Generator of tuples of the form (org unit hierarchy + string (str), org unit description (str)) if matching + results are found. E.g., + ("UCBKL-AVCIS-VRIST-JICCS", "Sample Description 1"), + ("UCBKL-AVCIS-VRIST-JJCNS", "Sample Description 2"), + - An empty list otherwise + """ + search_base = self.ORG_UNITS_OU + hierarchy_string_attr = 'berkeleyEduOrgUnitHierarchyString' + description_attr = 'description' + attributes = [hierarchy_string_attr, description_attr] + + results_found = self._connection.search( + search_base, search_filter, attributes=attributes) + if not results_found: + return [] + + for entry in self._connection.entries: + hierarchy_string = getattr(entry, hierarchy_string_attr).value + description = getattr(entry, description_attr).value + yield hierarchy_string, description + + def _lookup_person_department_numbers(self, search_filter, + assert_one_person=False): + """Given an LDAP search filter for the "people" OU, return a set + of department numbers associated with all matching entries. + + Optionally require that there be at most one matching person. + + Parameters: + - search_filter (str): An LDAP search filter to be used to + search the "people" OU + - assert_one_person (bool): An optional flag that requires + that there must be at most one matching person in the + search results + + Returns: + - Set of strs representing department numbers that are + identifiers for L4 org units (e.g., {"JICCS", "JJCNS"}) + - An empty set of no people entries are found + + Raises: + - AssertionError, if assert_one_person is specified, but + more than one person is found + """ + search_base = self.PEOPLE_DN + department_number_attr = 'departmentNumber' + attributes = [department_number_attr] + + results_found = self._connection.search( + search_base, search_filter, attributes=attributes) + if not results_found: + return set() + + if assert_one_person: + message = 'More than one matching person found.' + assert len(self._connection.entries) == 1, message + # The for loop below will run once. + + results = set() + for entry in self._connection.entries: + department_numbers = getattr(entry, department_number_attr).values + for department_number in department_numbers: + results.add(department_number) + return results + + def _lookup_person_department_numbers_from_emails(self, emails): + """Given a list of emails (strs) associated with a single + person, return a set of department numbers associated with all + entries in the "people" OU matching those* emails. + + *Only consider email addresses that end with "berkeley.edu" + (e.g., "email@berkeley.edu", "email@cs.berkeley.edu"). + + Parameters: + - emails (list): Emails associated with a single person + + Returns: + - Set of strs representing department numbers that are + identifiers for L4 org units (e.g., {"JICCS", "JJCNS"}) + """ + berkeley_email_suffix = 'berkeley.edu' + results = set() + for email in emails: + if not email.endswith(berkeley_email_suffix): + continue + search_filter = f'(&(objectClass=person)(mail={email}))' + for department_number in self._lookup_person_department_numbers( + search_filter, assert_one_person=False): + results.add(department_number) + return results + + def _lookup_person_department_numbers_from_name(self, first_name, + last_name): + """Given a person's first and last name (strs), return a set of + department numbers associated with at most a single entry in the + "people" OU matching that name. If there are multiple, return an + empty set. + + Parameters: + - first_name (str): A person's first name + - last_name (str): A person's last name + + Returns: + - Set of strs representing department numbers that are + identifiers for L4 org units (e.g., {"JICCS", "JJCNS"}) + """ + results = set() + search_filter = f'(&(givenName={first_name})(sn={last_name}))' + try: + department_numbers = self._lookup_person_department_numbers( + search_filter, assert_one_person=True) + except AssertionError: + pass + else: + for department_number in department_numbers: + results.add(department_number) + return results diff --git a/coldfront/plugins/departments/utils/data_sources/backends/dummy.py b/coldfront/plugins/departments/utils/data_sources/backends/dummy.py new file mode 100644 index 000000000..7a293fa10 --- /dev/null +++ b/coldfront/plugins/departments/utils/data_sources/backends/dummy.py @@ -0,0 +1,65 @@ +from .base import BaseDataSourceBackend + + +class DummyDataSourceBackend(BaseDataSourceBackend): + """A backend for testing purposes.""" + + def fetch_departments(self): + """Return a generator of 26 tuples, each representing a + department associated with an uppercase alphabetic letter: + ("DEPTA", "Department A"), + ("DEPTB", "Department B"), + ... + ("DEPTZ", "Department Z"). + """ + for ascii_code in range(ord('A'), ord('Z') + 1): + letter = chr(ascii_code) + yield self._get_department_for_letter(letter) + + def fetch_departments_for_user(self, user_data): + """Return a generator of up to two departments matching the + first and last initials of the given user dict. If a particular + initial is non-alphabetic, do not include a result for that + initial. Yield the departments in alphabetic order. + + Examples: + - "John Smith" --> + ("DEPTJ", "Department J"), + ("DEPTS", "Department S") + - "Jane Doe" --> + ("DEPTD", "Department D"), + ("DEPTJ", "Department J") + - "First 0" --> + ("DEPTF, "Department F") + - "Last" (no first name) --> + ("DEPTL, "Department L") + - (no first or last name) --> no results + """ + + def _get_department_for_name_part(_name_part): + if not _name_part: + return None + initial = _name_part[0] + if not initial.isalpha(): + return None + initial = initial.upper() + return self._get_department_for_letter(initial) + + departments = [] + for name_part in (user_data['first_name'], user_data['last_name']): + department = _get_department_for_name_part(name_part) + if not department: + continue + departments.append(department) + departments.sort(key=lambda d: d[0]) + + yield from departments + + @staticmethod + def _get_department_for_letter(letter): + """Return the identifier and description for the department + corresponding to the given letter, which is assumed to be an + uppercase alphabetic letter.""" + identifier = f'DEPT{letter}' + description = f'Department {letter}' + return identifier, description diff --git a/coldfront/plugins/departments/utils/queries.py b/coldfront/plugins/departments/utils/queries.py new file mode 100644 index 000000000..9b1338f54 --- /dev/null +++ b/coldfront/plugins/departments/utils/queries.py @@ -0,0 +1,160 @@ +import logging + +from django.db import transaction + +from coldfront.plugins.departments.models import Department +from coldfront.plugins.departments.models import UserDepartment +from coldfront.plugins.departments.utils import UserInfoDict +from coldfront.plugins.departments.utils.data_sources import fetch_departments_for_user + + +logger = logging.getLogger(__name__) + + +def create_or_update_department(code, name): + department, created = Department.objects.update_or_create( + code=code, + defaults={ + 'name': name, + }) + return department, created + + +def get_departments_for_user(user, strs_only=False): + """Return two lists: Departments the given User is (a) + authoritatively and (b) non-authoritatively associated with. Each + list is sorted by ascending name. + + Optionally return the str representation of each Department instead + of the Department itself. + """ + user_departments = ( + UserDepartment.objects + .filter(user=user) + .select_related('department') + .order_by('department__name')) + + authoritative, non_authoritative = [], [] + for user_department in user_departments: + department = user_department.department + entry = str(department) if strs_only else department + if user_department.is_authoritative: + authoritative.append(entry) + else: + non_authoritative.append(entry) + + return authoritative, non_authoritative + + +class UserDepartmentUpdater(object): + """A class that updates a User's authoritative and/or + non-authoritative Department associations.""" + + def __init__(self, user, non_authoritative_departments): + self._user = user + self._non_authoritative_departments = set(non_authoritative_departments) + self._authoritative_departments = set() + + def run(self, authoritative=True, non_authoritative=True): + """Update associations. Fetch and update authoritative ones, + unless indicated. Update non-authoritative ones to those + provided during instantiation, unless indicated. If any error + occurs during fetching, skip updating authoritative ones.""" + if authoritative: + try: + authoritative_user_department_data = \ + self._fetch_authoritative_user_departments() + except Exception as e: + authoritative = False + + with transaction.atomic(): + if authoritative: + self._process_authoritative_user_departments( + authoritative_user_department_data) + if authoritative and non_authoritative: + # The order of updates should not matter since the two sets of + # Departments should be mutually exclusive. + self._update_non_authoritative_user_departments() + self._update_authoritative_user_departments() + elif authoritative: + self._update_authoritative_user_departments() + elif non_authoritative: + self._update_non_authoritative_user_departments() + + def _fetch_authoritative_user_departments(self): + """Fetch department data for the User from the data source.""" + try: + user_data = UserInfoDict.from_user(self._user) + return fetch_departments_for_user(user_data) + except Exception as e: + logger.error( + f'Failed to fetch department data for User {self._user.pk}. ' + f'Details:') + logger.exception(e) + raise e + + def _process_authoritative_user_departments(self, + authoritative_user_department_data): + """Given department data for the User, fetched from the data + source: + 1. Create Department objects as needed. + 2. Mark the Department as being authoritative, and NOT + non-authoritative. + """ + for code, name in authoritative_user_department_data: + department, department_created = create_or_update_department( + code, name) + if department_created: + logger.info(f'Created Department {department}.') + self._authoritative_departments.add(department) + self._non_authoritative_departments.discard(department) + + def _update_authoritative_user_departments(self): + """Update authoritative associations to those set in this + instance. Delete any other existing authoritative associations. + This may involve updating non-authoritative associations to be + authoritative.""" + for department in self._authoritative_departments: + UserDepartment.objects.update_or_create( + user=self._user, + department=department, + defaults={ + 'is_authoritative': True, + }) + + to_delete = ( + UserDepartment.objects + .filter( + user=self._user, + is_authoritative=True) + .exclude(department__in=self._authoritative_departments) + ) + to_delete.delete() + + def _update_non_authoritative_user_departments(self): + """Update non-authoritative associations to those set in this + instance. Delete any other existing non-authoritative + associations. Do not override any associations that are set + authoritatively.""" + for department in self._non_authoritative_departments: + existing_authoritative = UserDepartment.objects.filter( + user=self._user, + department=department, + is_authoritative=True).exists() + if existing_authoritative: + continue + UserDepartment.objects.update_or_create( + user=self._user, + department=department, + defaults={ + 'is_authoritative': False, + }) + + to_delete = ( + UserDepartment.objects + .filter( + user=self._user, + is_authoritative=False) + .exclude(department__in=self._non_authoritative_departments) + ) + to_delete.delete() diff --git a/coldfront/plugins/departments/views.py b/coldfront/plugins/departments/views.py new file mode 100644 index 000000000..6d81b80fc --- /dev/null +++ b/coldfront/plugins/departments/views.py @@ -0,0 +1,62 @@ +import logging + +from django.conf import settings as django_settings +from django.contrib.auth.mixins import LoginRequiredMixin +from django.views.generic.edit import FormView +from django.urls import reverse + +from django_q.tasks import async_task + +from coldfront.plugins.departments.conf import settings +from coldfront.plugins.departments.forms import NonAuthoritativeDepartmentSelectionForm +from coldfront.plugins.departments.utils.queries import get_departments_for_user +from coldfront.plugins.departments.utils.queries import UserDepartmentUpdater + + +logger = logging.getLogger(__name__) + + +class UpdateDepartmentsView(LoginRequiredMixin, FormView): + + form_class = NonAuthoritativeDepartmentSelectionForm + template_name = 'user_update_departments.html' + login_url = '/' + + error_message = 'Unexpected failure. Please contact an administrator.' + + def form_valid(self, form): + user = self.request.user + non_authoritative_departments = form.cleaned_data['departments'] + + user_department_updater = UserDepartmentUpdater( + user, non_authoritative_departments) + user_department_updater.run(authoritative=False, non_authoritative=True) + + func = ( + 'coldfront.plugins.departments.tasks.' + 'fetch_and_set_user_authoritative_departments') + async_task( + func, user.pk, sync=django_settings.Q_CLUSTER.get('sync', False)) + + return super().form_valid(form) + + def get_context_data(self, viewed_username=None, **kwargs): + context = super().get_context_data(**kwargs) + + context['department_display_name'] = settings.DEPARTMENT_DISPLAY_NAME + + # TODO: Account for viewed_username or not? + + authoritative_department_strs, _ = get_departments_for_user( + self.request.user, strs_only=True) + context['auth_department_list'] = authoritative_department_strs + + return context + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs['user'] = self.request.user + return kwargs + + def get_success_url(self): + return reverse('user-profile') diff --git a/requirements.txt b/requirements.txt index 038956950..63a5e0d6a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -35,10 +35,10 @@ gspread==3.6.0 humanize==0.5.1 idna==2.8 iso8601==0.1.16 +ldap3==2.9.1 mod-wsgi oauth2client==4.1.3 openpyxl==3.0.9 -#pandas==1.1.5 phonenumbers==8.12.23 psycopg2-binary pylint==2.12.2