From 8e5ada93d183d643a362fb01bc7f3079664368c4 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 18 Apr 2023 11:53:20 -0700 Subject: [PATCH 01/11] Update version of ACM in requirements file --- requirements/base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/base.txt b/requirements/base.txt index 4aa6cad7..e304c089 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -21,7 +21,7 @@ django-dbbackup==4.0.1 # https://github.com/jazzband/django-dbbackup django-extensions==3.2.1 # https://github.com/django-extensions/django-extensions # anvil_consortium_manager -git+https://github.com/UW-GAC/django-anvil-consortium-manager.git@v0.14 +git+https://github.com/UW-GAC/django-anvil-consortium-manager.git@v015 # Simple history - model history tracking django-simple-history==3.1.1 # For tracking history From 80570ba73552d5886fe64eaa7bba849ca78743d3 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 18 Apr 2023 11:54:13 -0700 Subject: [PATCH 02/11] Use WorkspaceAutocompleteByType instead of UploadWorkspaceAutocomplete Remove the UploadWorkspaceAutocomplete view. Modify the tests to use the WorkspaceAutocompleteByType view from ACM. Override the get_autocomplete_queryset method in the UploadWorkspaceAdapter to provide the same functionality as UploadWorkspaceAutocomplete used to. --- gregor_django/gregor_anvil/adapters.py | 11 ++++++ gregor_django/gregor_anvil/forms.py | 6 ++- .../gregor_anvil/tests/test_views.py | 18 +++++---- gregor_django/gregor_anvil/urls.py | 11 ------ gregor_django/gregor_anvil/views.py | 37 +++++++++---------- 5 files changed, 44 insertions(+), 39 deletions(-) diff --git a/gregor_django/gregor_anvil/adapters.py b/gregor_django/gregor_anvil/adapters.py index 759d38e6..582ad0eb 100644 --- a/gregor_django/gregor_anvil/adapters.py +++ b/gregor_django/gregor_anvil/adapters.py @@ -36,6 +36,17 @@ class UploadWorkspaceAdapter(BaseWorkspaceAdapter): workspace_data_form_class = forms.UploadWorkspaceForm workspace_detail_template_name = "gregor_anvil/uploadworkspace_detail.html" + def get_autocomplete_queryset(self, queryset, q, forwarded={}): + """Filter to Accounts where the email or the associated user name matches the query `q`.""" + consent_group = forwarded.get("consent_group", None) + if consent_group: + queryset = queryset.filter(consent_group=consent_group) + + if q: + queryset = queryset.filter(workspace__name__icontains=q) + + return queryset + class ExampleWorkspaceAdapter(BaseWorkspaceAdapter): """Adapter for ExampleWorkspaces.""" diff --git a/gregor_django/gregor_anvil/forms.py b/gregor_django/gregor_anvil/forms.py index cd9926ff..d1aa3a97 100644 --- a/gregor_django/gregor_anvil/forms.py +++ b/gregor_django/gregor_anvil/forms.py @@ -4,6 +4,7 @@ from dal import autocomplete from django import forms from django.core.exceptions import ValidationError +from django.urls import reverse from . import models @@ -51,7 +52,10 @@ class Meta: } widgets = { "upload_workspaces": autocomplete.ModelSelect2Multiple( - url="gregor_anvil:upload_workspaces:autocomplete", + url=reverse( + "anvil_consortium_manager:workspaces:autocomplete_by_type", + args=["upload"], + ), attrs={"data-theme": "bootstrap-5"}, ), } diff --git a/gregor_django/gregor_anvil/tests/test_views.py b/gregor_django/gregor_anvil/tests/test_views.py index a3522eae..b61614c7 100644 --- a/gregor_django/gregor_anvil/tests/test_views.py +++ b/gregor_django/gregor_anvil/tests/test_views.py @@ -517,7 +517,7 @@ def test_creates_upload_workspace(self): self.assertEqual(new_workspace_data.version, 5) -class UploadWorkspaceAutocompleteTest(TestCase): +class UploadWorkspaceAutocompleteByTypeTest(TestCase): def setUp(self): """Set up test class.""" self.factory = RequestFactory() @@ -531,13 +531,15 @@ def setUp(self): def get_url(self, *args): """Get the url for the view being tested.""" - return reverse("gregor_anvil:upload_workspaces:autocomplete", args=args) + return reverse( + "anvil_consortium_manager:workspaces:autocomplete_by_type", args=args + ) def test_returns_all_objects(self): """Queryset returns all objects when there is no query.""" workspaces = factories.UploadWorkspaceFactory.create_batch(10) self.client.force_login(self.user) - response = self.client.get(self.get_url()) + response = self.client.get(self.get_url("upload")) returned_ids = [ int(x["id"]) for x in json.loads(response.content.decode("utf-8"))["results"] @@ -552,7 +554,7 @@ def test_returns_correct_object_match(self): workspace__name="test-workspace" ) self.client.force_login(self.user) - response = self.client.get(self.get_url(), {"q": "test-workspace"}) + response = self.client.get(self.get_url("upload"), {"q": "test-workspace"}) returned_ids = [ int(x["id"]) for x in json.loads(response.content.decode("utf-8"))["results"] @@ -567,7 +569,7 @@ def test_returns_correct_object_starting_with_query(self): workspace__name="test-workspace" ) self.client.force_login(self.user) - response = self.client.get(self.get_url(), {"q": "test"}) + response = self.client.get(self.get_url("upload"), {"q": "test"}) returned_ids = [ int(x["id"]) for x in json.loads(response.content.decode("utf-8"))["results"] @@ -582,7 +584,7 @@ def test_returns_correct_object_containing_query(self): workspace__name="test-workspace" ) self.client.force_login(self.user) - response = self.client.get(self.get_url(), {"q": "work"}) + response = self.client.get(self.get_url("upload"), {"q": "work"}) returned_ids = [ int(x["id"]) for x in json.loads(response.content.decode("utf-8"))["results"] @@ -597,7 +599,7 @@ def test_returns_correct_object_case_insensitive(self): workspace__name="test-workspace" ) self.client.force_login(self.user) - response = self.client.get(self.get_url(), {"q": "TEST-WORKSPACE"}) + response = self.client.get(self.get_url("upload"), {"q": "TEST-WORKSPACE"}) returned_ids = [ int(x["id"]) for x in json.loads(response.content.decode("utf-8"))["results"] @@ -617,7 +619,7 @@ def test_forwarded_consent_group(self): ) self.client.force_login(self.user) response = self.client.get( - self.get_url(), + self.get_url("upload"), {"q": "test", "forward": json.dumps({"consent_group": consent_group.pk})}, ) returned_ids = [ diff --git a/gregor_django/gregor_anvil/urls.py b/gregor_django/gregor_anvil/urls.py index f06d7660..3fc78c2c 100644 --- a/gregor_django/gregor_anvil/urls.py +++ b/gregor_django/gregor_anvil/urls.py @@ -21,16 +21,6 @@ "consent_groups", ) -upload_workspace_patterns = ( - [ - path( - "autocomplete/", - views.UploadWorkspaceAutocomplete.as_view(), - name="autocomplete", - ), - ], - "upload_workspaces", -) workspace_report_patterns = ( [ path("workspaces/", views.WorkspaceReport.as_view(), name="workspace"), @@ -41,6 +31,5 @@ # path("", views.Index.as_view(), name="index"), path("research_centers/", include(research_center_patterns)), path("consent_groups/", include(consent_group_patterns)), - path("upload_workspaces/", include(upload_workspace_patterns)), path("reports/", include(workspace_report_patterns)), ] diff --git a/gregor_django/gregor_anvil/views.py b/gregor_django/gregor_anvil/views.py index 3bc3e5c3..d7b53ccb 100644 --- a/gregor_django/gregor_anvil/views.py +++ b/gregor_django/gregor_anvil/views.py @@ -1,6 +1,5 @@ from anvil_consortium_manager.auth import AnVILConsortiumManagerViewRequired from anvil_consortium_manager.models import Account, Workspace -from dal import autocomplete from django.contrib.auth import get_user_model from django.db.models import Count, Q from django.views.generic import DetailView, TemplateView @@ -45,24 +44,24 @@ class ResearchCenterList(AnVILConsortiumManagerViewRequired, SingleTableView): table_class = tables.ResearchCenterTable -class UploadWorkspaceAutocomplete( - AnVILConsortiumManagerViewRequired, autocomplete.Select2QuerySetView -): - """View to provide autocompletion for UploadWorkspaces.""" - - def get_queryset(self): - qs = models.UploadWorkspace.objects.filter().order_by( - "workspace__billing_project__name", "workspace__name" - ) - - consent_group = self.forwarded.get("consent_group", None) - if consent_group: - qs = qs.filter(consent_group=consent_group) - - if self.q: - qs = qs.filter(workspace__name__icontains=self.q) - - return qs +# class UploadWorkspaceAutocomplete( +# AnVILConsortiumManagerViewRequired, autocomplete.Select2QuerySetView +# ): +# """View to provide autocompletion for UploadWorkspaces.""" +# +# def get_queryset(self): +# qs = models.UploadWorkspace.objects.filter().order_by( +# "workspace__billing_project__name", "workspace__name" +# ) +# +# consent_group = self.forwarded.get("consent_group", None) +# if consent_group: +# qs = qs.filter(consent_group=consent_group) +# +# if self.q: +# qs = qs.filter(workspace__name__icontains=self.q) +# +# return qs class WorkspaceReport(AnVILConsortiumManagerViewRequired, TemplateView): From 8bfc7d23587e11ac182251a27b8da1537fe219b1 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 18 Apr 2023 14:07:18 -0700 Subject: [PATCH 03/11] Update allauth adapter to add all users to a specific group ACM v0.15 adds the Account Link permission. Right now, we want all users to have this permission. In the future, we may add a specific Drupal role for this, but for now, add everyone to a group, and we can give that group permission. --- gregor_django/users/adapters.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gregor_django/users/adapters.py b/gregor_django/users/adapters.py index 9ce07ec7..32b20ddb 100644 --- a/gregor_django/users/adapters.py +++ b/gregor_django/users/adapters.py @@ -77,6 +77,13 @@ def update_user_groups(self, user, extra_data: Dict): "sociallogin.extra_data.managed_scope_status should be a dict" ) else: + # Is this the best place to implement this?? + # Right now, everyone should get the "account link" permission from ACM. + # Eventually, we may add a Drupal role indicating whether someone should get this permission. + # If a Drupal role is added, we will need to delete this group from the Django. + # Note that this breaks tests. + # managed_scope_status["ACM data access allowed"] = True + for group_name, user_has_group in managed_scope_status.items(): user_group, was_created = Group.objects.get_or_create( name=group_name From 0d86a31dd17aa5a050432a4e7fe74c97f58a7f8d Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 18 Apr 2023 14:57:11 -0700 Subject: [PATCH 04/11] Fix ACM tag in requirements file --- requirements/base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/base.txt b/requirements/base.txt index e304c089..cc22ca1e 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -21,7 +21,7 @@ django-dbbackup==4.0.1 # https://github.com/jazzband/django-dbbackup django-extensions==3.2.1 # https://github.com/django-extensions/django-extensions # anvil_consortium_manager -git+https://github.com/UW-GAC/django-anvil-consortium-manager.git@v015 +git+https://github.com/UW-GAC/django-anvil-consortium-manager.git@v0.15 # Simple history - model history tracking django-simple-history==3.1.1 # For tracking history From 5efc28dbdb616d48d7235a5c81d973193f91f666 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 18 Apr 2023 15:51:40 -0700 Subject: [PATCH 05/11] Add a default workspace table for workspace data adapters This table shows the name of the workspace, the billing project, the number of groups that it is shared with, and whether it is shared with the GREGoR consortium via GREGOR_ALL. Also add a table subclass that adds the "shared with GREGoR?" column. Use this new default table for all workspace data adapters that don't define their own class already. Add the "shared with GREGoR?" column to workspace data tables that define their own table calss, except for ReleaseWorkspaces (which should never be shared with the consortium). --- gregor_django/gregor_anvil/adapters.py | 5 +- gregor_django/gregor_anvil/tables.py | 54 ++++++++++++++++++- .../gregor_anvil/tests/test_views.py | 5 +- 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/gregor_django/gregor_anvil/adapters.py b/gregor_django/gregor_anvil/adapters.py index 759d38e6..e1568590 100644 --- a/gregor_django/gregor_anvil/adapters.py +++ b/gregor_django/gregor_anvil/adapters.py @@ -1,6 +1,5 @@ from anvil_consortium_manager.adapters.account import BaseAccountAdapter from anvil_consortium_manager.adapters.workspace import BaseWorkspaceAdapter -from anvil_consortium_manager.tables import WorkspaceTable from django.db.models import Q from . import forms, models, tables @@ -45,7 +44,7 @@ class ExampleWorkspaceAdapter(BaseWorkspaceAdapter): description = ( "Workspaces that contain examples of using AnVIL, working with data, etc." ) - list_table_class = WorkspaceTable + list_table_class = tables.DefaultWorkspaceTable workspace_data_model = models.ExampleWorkspace workspace_data_form_class = forms.ExampleWorkspaceForm workspace_detail_template_name = "anvil_consortium_manager/workspace_detail.html" @@ -69,7 +68,7 @@ class CombinedConsortiumDataWorkspaceAdapter(BaseWorkspaceAdapter): type = "combined_consortium" name = "Combined consortium data workspace" description = "Workspaces for internal consortium use that contain data tables combined across upload workspaces" - list_table_class = WorkspaceTable + list_table_class = tables.DefaultWorkspaceTable workspace_data_model = models.CombinedConsortiumDataWorkspace workspace_data_form_class = forms.CombinedConsortiumDataWorkspaceForm workspace_detail_template_name = ( diff --git a/gregor_django/gregor_anvil/tables.py b/gregor_django/gregor_anvil/tables.py index bf42ae76..5c982b7f 100644 --- a/gregor_django/gregor_anvil/tables.py +++ b/gregor_django/gregor_anvil/tables.py @@ -1,6 +1,7 @@ import django_tables2 as tables from anvil_consortium_manager.adapters.workspace import workspace_adapter_registry from anvil_consortium_manager.models import Account, Workspace +from django.utils.html import format_html from . import models @@ -46,7 +47,54 @@ class Meta: ) -class UploadWorkspaceTable(tables.Table): +class WorkspaceSharedWithConsortiumTable(tables.Table): + """Table including a column to indicate if a workspace is shared with PRIMED_ALL.""" + + is_shared = tables.columns.Column( + accessor="pk", + verbose_name="Shared with GREGoR?", + orderable=False, + ) + + def render_is_shared(self, record): + is_shared = record.workspacegroupsharing_set.filter( + group__name="GREGOR_ALL" + ).exists() + if is_shared: + icon = "check-circle-fill" + color = "green" + value = format_html( + """""".format(icon, color) + ) + else: + value = "" + return value + + +class DefaultWorkspaceTable(WorkspaceSharedWithConsortiumTable, tables.Table): + """Class to use for default workspace tables in GREGoR.""" + + name = tables.Column(linkify=True, verbose_name="Workspace") + billing_project = tables.Column(linkify=True) + number_groups = tables.Column( + verbose_name="Number of groups shared with", + empty_values=(), + orderable=False, + accessor="workspacegroupsharing_set__count", + ) + + class Meta: + model = Workspace + fields = ( + "name", + "billing_project", + "number_groups", + "is_shared", + ) + order_by = ("name",) + + +class UploadWorkspaceTable(WorkspaceSharedWithConsortiumTable, tables.Table): """A table for Workspaces that includes fields from UploadWorkspace.""" name = tables.columns.Column(linkify=True) @@ -58,10 +106,11 @@ class Meta: "uploadworkspace__research_center", "uploadworkspace__consent_group", "uploadworkspace__version", + "is_shared", ) -class TemplateWorkspaceTable(tables.Table): +class TemplateWorkspaceTable(WorkspaceSharedWithConsortiumTable, tables.Table): """A table for Workspaces that includes fields from TemplateWorkspace.""" name = tables.columns.Column(linkify=True) @@ -71,6 +120,7 @@ class Meta: fields = ( "name", "templateworkspace__intended_use", + "is_shared", ) diff --git a/gregor_django/gregor_anvil/tests/test_views.py b/gregor_django/gregor_anvil/tests/test_views.py index a3522eae..10e2f65a 100644 --- a/gregor_django/gregor_anvil/tests/test_views.py +++ b/gregor_django/gregor_anvil/tests/test_views.py @@ -2,7 +2,6 @@ import responses from anvil_consortium_manager import models as acm_models -from anvil_consortium_manager.tables import WorkspaceTable from anvil_consortium_manager.tests import factories as acm_factories from anvil_consortium_manager.tests.utils import AnVILAPIMockTestMixin from django.conf import settings @@ -653,7 +652,9 @@ def test_view_has_correct_table_class(self): self.client.force_login(self.user) response = self.client.get(self.get_url(self.workspace_type)) self.assertIn("table", response.context_data) - self.assertIsInstance(response.context_data["table"], WorkspaceTable) + self.assertIsInstance( + response.context_data["table"], tables.DefaultWorkspaceTable + ) class ExampleWorkspaceCreateTest(AnVILAPIMockTestMixin, TestCase): From 4d598cdc460ad2f755558bc8853eb1605a0e6271 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 20 Apr 2023 10:01:44 -0700 Subject: [PATCH 06/11] Pull in Drupal DSA completed role For now, we expect that this role will be used to assign the ACM account link permission. It's possible that this will change in the future. Note that if we want *everyone* with a Drupal account to be able to link their AnVIL accounts, we could use the drupal_machine_name="authorized" scope and map it to a django_group_name. --- config/settings/base.py | 8 ++++++++ gregor_django/users/adapters.py | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/config/settings/base.py b/config/settings/base.py index ceaf30bd..ddf36f02 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -321,16 +321,24 @@ "API_URL": DRUPAL_SITE_URL, "OVERRIDE_NAME": "Gregor Consortium Site Login", "SCOPES": [ + # This role will be used to grant ACM view permission. { "drupal_machine_name": "dcc_staff", "request_scope": True, "django_group_name": "DCC Staff", }, + # This role will be use dto grant ACM edit permission. { "drupal_machine_name": "dcc_acm_admin", "request_scope": True, "django_group_name": "DCC ACM Admin", }, + # For now we can use the "DSA completed" group to determine who can link their accounts. + { + "drupal_machine_name": "dsa_completed", + "request_scope": True, + "django_group_name": "ACM data access allowed", + }, ], } } diff --git a/gregor_django/users/adapters.py b/gregor_django/users/adapters.py index 32b20ddb..9ce07ec7 100644 --- a/gregor_django/users/adapters.py +++ b/gregor_django/users/adapters.py @@ -77,13 +77,6 @@ def update_user_groups(self, user, extra_data: Dict): "sociallogin.extra_data.managed_scope_status should be a dict" ) else: - # Is this the best place to implement this?? - # Right now, everyone should get the "account link" permission from ACM. - # Eventually, we may add a Drupal role indicating whether someone should get this permission. - # If a Drupal role is added, we will need to delete this group from the Django. - # Note that this breaks tests. - # managed_scope_status["ACM data access allowed"] = True - for group_name, user_has_group in managed_scope_status.items(): user_group, was_created = Group.objects.get_or_create( name=group_name From 337d4d73b92f81bbc75d4e7adcead41c2361ee46 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 20 Apr 2023 16:17:28 -0700 Subject: [PATCH 07/11] Change the dsa_completed mapping to just "DSA completed" This is because in the future, we may have a more complicated way to determine if someone should have AnVIL data access using two Drupal roles instead of just one. --- config/settings/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/settings/base.py b/config/settings/base.py index ddf36f02..179930af 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -337,7 +337,7 @@ { "drupal_machine_name": "dsa_completed", "request_scope": True, - "django_group_name": "ACM data access allowed", + "django_group_name": "DSA completed", }, ], } From fced1abf39102193a544e764a68eeee78a9b4e2e Mon Sep 17 00:00:00 2001 From: Jonas Carson Date: Thu, 27 Apr 2023 11:23:06 -0700 Subject: [PATCH 08/11] Add partner group model, link to user and add all supporting views and tests --- gregor_django/gregor_anvil/admin.py | 18 ++ ...008_historicalpartnergroup_partnergroup.py | 54 ++++++ gregor_django/gregor_anvil/models.py | 24 +++ gregor_django/gregor_anvil/tables.py | 10 ++ gregor_django/gregor_anvil/tests/factories.py | 11 ++ .../gregor_anvil/tests/test_views.py | 154 ++++++++++++++++++ gregor_django/gregor_anvil/urls.py | 9 + gregor_django/gregor_anvil/views.py | 19 +++ .../anvil_consortium_manager/navbar.html | 4 + .../gregor_anvil/partnergroup_detail.html | 17 ++ .../gregor_anvil/partnergroup_list.html | 11 ++ .../templates/users/user_detail.html | 14 +- gregor_django/users/adapters.py | 80 +++++++-- .../migrations/0004_user_partner_groups.py | 19 +++ gregor_django/users/models.py | 3 +- gregor_django/users/tests/test_adapters.py | 60 ++++++- 16 files changed, 487 insertions(+), 20 deletions(-) create mode 100644 gregor_django/gregor_anvil/migrations/0008_historicalpartnergroup_partnergroup.py create mode 100644 gregor_django/templates/gregor_anvil/partnergroup_detail.html create mode 100644 gregor_django/templates/gregor_anvil/partnergroup_list.html create mode 100644 gregor_django/users/migrations/0004_user_partner_groups.py diff --git a/gregor_django/gregor_anvil/admin.py b/gregor_django/gregor_anvil/admin.py index f7d0c274..5047f836 100644 --- a/gregor_django/gregor_anvil/admin.py +++ b/gregor_django/gregor_anvil/admin.py @@ -41,6 +41,24 @@ class ResearchCenterAdmin(SimpleHistoryAdmin): ) +@admin.register(models.PartnerGroup) +class PartnerGroupAdmin(SimpleHistoryAdmin): + """Admin class for the PartnerGroup model.""" + + list_display = ( + "short_name", + "full_name", + ) + search_fields = ( + "short_name", + "full_name", + ) + sortable_by = ( + "short_name", + "full_name", + ) + + @admin.register(models.UploadWorkspace) class UploadWorkspaceAdmin(SimpleHistoryAdmin): """Admin class for the UploadWorkspace model.""" diff --git a/gregor_django/gregor_anvil/migrations/0008_historicalpartnergroup_partnergroup.py b/gregor_django/gregor_anvil/migrations/0008_historicalpartnergroup_partnergroup.py new file mode 100644 index 00000000..d97d6473 --- /dev/null +++ b/gregor_django/gregor_anvil/migrations/0008_historicalpartnergroup_partnergroup.py @@ -0,0 +1,54 @@ +# Generated by Django 3.2.16 on 2023-04-27 17:10 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields +import simple_history.models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('gregor_anvil', '0007_releaseworkspace'), + ] + + operations = [ + migrations.CreateModel( + name='PartnerGroup', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('short_name', models.CharField(max_length=15, unique=True)), + ('full_name', models.CharField(max_length=255)), + ], + options={ + 'get_latest_by': 'modified', + 'abstract': False, + }, + ), + migrations.CreateModel( + name='HistoricalPartnerGroup', + fields=[ + ('id', models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('short_name', models.CharField(db_index=True, max_length=15)), + ('full_name', models.CharField(max_length=255)), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField(db_index=True)), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'historical partner group', + 'verbose_name_plural': 'historical partner groups', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + ] diff --git a/gregor_django/gregor_anvil/models.py b/gregor_django/gregor_anvil/models.py index d568729f..b7f88158 100644 --- a/gregor_django/gregor_anvil/models.py +++ b/gregor_django/gregor_anvil/models.py @@ -56,6 +56,30 @@ def get_absolute_url(self): return reverse("gregor_anvil:research_centers:detail", args=[self.pk]) +class PartnerGroup(TimeStampedModel, models.Model): + """A model to track Partner Groups""" + + short_name = models.CharField(max_length=15, unique=True) + """The short name of the Partner Group""" + + full_name = models.CharField(max_length=255) + """The full name of the Partner Group""" + + history = HistoricalRecords() + + def __str__(self): + """String method. + + Returns: + A string showing the short_name of the object. + """ + return self.short_name + + def get_absolute_url(self): + """Return the absolute url for this object.""" + return reverse("gregor_anvil:partner_groups:detail", args=[self.pk]) + + class UploadWorkspace(TimeStampedModel, BaseWorkspaceData): """A model to track additional data about an upload workspace.""" diff --git a/gregor_django/gregor_anvil/tables.py b/gregor_django/gregor_anvil/tables.py index 5c982b7f..0fa35a18 100644 --- a/gregor_django/gregor_anvil/tables.py +++ b/gregor_django/gregor_anvil/tables.py @@ -34,6 +34,16 @@ class Meta: fields = ("full_name", "short_name") +class PartnerGroupTable(tables.Table): + """A table for ResearchCenters.""" + + full_name = tables.Column(linkify=True) + + class Meta: + model = models.PartnerGroup + fields = ("full_name", "short_name") + + class ConsentGroupTable(tables.Table): """A table for `ConsentGroups`.""" diff --git a/gregor_django/gregor_anvil/tests/factories.py b/gregor_django/gregor_anvil/tests/factories.py index 9ef5d619..aa4b8d18 100644 --- a/gregor_django/gregor_anvil/tests/factories.py +++ b/gregor_django/gregor_anvil/tests/factories.py @@ -28,6 +28,17 @@ class Meta: django_get_or_create = ["short_name"] +class PartnerGroupFactory(DjangoModelFactory): + """A factory for the PartnerGroup model.""" + + short_name = Faker("word") + full_name = Faker("company") + + class Meta: + model = models.PartnerGroup + django_get_or_create = ["short_name"] + + class UploadWorkspaceFactory(DjangoModelFactory): """A factory for the UploadWorkspace model.""" diff --git a/gregor_django/gregor_anvil/tests/test_views.py b/gregor_django/gregor_anvil/tests/test_views.py index e1426989..2bc8f05a 100644 --- a/gregor_django/gregor_anvil/tests/test_views.py +++ b/gregor_django/gregor_anvil/tests/test_views.py @@ -383,6 +383,160 @@ def test_view_with_two_objects(self): self.assertEqual(len(response.context_data["table"].rows), 2) +class PartnerGroupDetailTest(TestCase): + """Tests for the PartnerGroupDetail view.""" + + def setUp(self): + """Set up test class.""" + self.factory = RequestFactory() + self.model_factory = factories.PartnerGroupFactory + # Create a user with both view and edit permission. + self.user = User.objects.create_user(username="test", password="test") + self.user.user_permissions.add( + Permission.objects.get( + codename=acm_models.AnVILProjectManagerAccess.VIEW_PERMISSION_CODENAME + ) + ) + + def get_url(self, *args): + """Get the url for the view being tested.""" + return reverse("gregor_anvil:partner_groups:detail", args=args) + + def get_view(self): + """Return the view being tested.""" + return views.PartnerGroupDetail.as_view() + + def test_view_redirect_not_logged_in(self): + "View redirects to login view when user is not logged in." + # Need a client for redirects. + response = self.client.get(self.get_url(1)) + self.assertRedirects( + response, resolve_url(settings.LOGIN_URL) + "?next=" + self.get_url(1) + ) + + def test_status_code_with_user_permission(self): + """Returns successful response code.""" + obj = self.model_factory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url(obj.pk)) + self.assertEqual(response.status_code, 200) + + def test_access_without_user_permission(self): + """Raises permission denied if user has no permissions.""" + user_no_perms = User.objects.create_user( + username="test-none", password="test-none" + ) + request = self.factory.get(self.get_url(1)) + request.user = user_no_perms + with self.assertRaises(PermissionDenied): + self.get_view()(request) + + def test_view_status_code_with_invalid_pk(self): + """Raises a 404 error with an invalid object pk.""" + obj = self.model_factory.create() + request = self.factory.get(self.get_url(obj.pk + 1)) + request.user = self.user + with self.assertRaises(Http404): + self.get_view()(request, pk=obj.pk + 1) + + def test_site_user_table(self): + """Contains a table of site users with the correct users.""" + obj = self.model_factory.create() + pg_user = UserFactory.create() + pg_user.partner_groups.set([obj]) + non_pg_user = UserFactory.create() + + self.client.force_login(self.user) + response = self.client.get(self.get_url(obj.pk)) + self.assertIn("partner_group_user_table", response.context_data) + table = response.context_data["partner_group_user_table"] + self.assertEqual(len(table.rows), 1) + + self.assertIn(pg_user, table.data) + self.assertNotIn(non_pg_user, table.data) + + +class PartnerGroupListTest(TestCase): + """Tests for the ResearchCenterList view.""" + + def setUp(self): + """Set up test class.""" + self.factory = RequestFactory() + self.model_factory = factories.PartnerGroupFactory + # Create a user with both view and edit permission. + self.user = User.objects.create_user(username="test", password="test") + self.user.user_permissions.add( + Permission.objects.get( + codename=acm_models.AnVILProjectManagerAccess.VIEW_PERMISSION_CODENAME + ) + ) + + def get_url(self): + """Get the url for the view being tested.""" + return reverse("gregor_anvil:partner_groups:list") + + def get_view(self): + """Return the view being tested.""" + return views.PartnerGroupList.as_view() + + def test_view_redirect_not_logged_in(self): + "View redirects to login view when user is not logged in." + # Need a client for redirects. + response = self.client.get(self.get_url()) + self.assertRedirects( + response, resolve_url(settings.LOGIN_URL) + "?next=" + self.get_url() + ) + + def test_status_code_with_user_permission(self): + """Returns successful response code.""" + self.client.force_login(self.user) + response = self.client.get(self.get_url()) + self.assertEqual(response.status_code, 200) + + def test_access_without_user_permission(self): + """Raises permission denied if user has no permissions.""" + user_no_perms = User.objects.create_user( + username="test-none", password="test-none" + ) + request = self.factory.get(self.get_url()) + request.user = user_no_perms + with self.assertRaises(PermissionDenied): + self.get_view()(request) + + def test_view_has_correct_table_class(self): + """View has the correct table class in the context.""" + self.client.force_login(self.user) + response = self.client.get(self.get_url()) + self.assertIn("table", response.context_data) + self.assertIsInstance(response.context_data["table"], tables.PartnerGroupTable) + + def test_view_with_no_objects(self): + """The table has no rows when there are no ResearchCenter objects.""" + self.client.force_login(self.user) + response = self.client.get(self.get_url()) + self.assertEqual(response.status_code, 200) + self.assertIn("table", response.context_data) + self.assertEqual(len(response.context_data["table"].rows), 0) + + def test_view_with_one_object(self): + """The table has one row when there is one ResearchCenter object.""" + self.model_factory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url()) + self.assertEqual(response.status_code, 200) + self.assertIn("table", response.context_data) + self.assertEqual(len(response.context_data["table"].rows), 1) + + def test_view_with_two_objects(self): + """The table has two rows when there are two ResearchCenter objects.""" + self.model_factory.create_batch(2) + self.client.force_login(self.user) + response = self.client.get(self.get_url()) + self.assertEqual(response.status_code, 200) + self.assertIn("table", response.context_data) + self.assertEqual(len(response.context_data["table"].rows), 2) + + class UploadWorkspaceDetailTest(TestCase): """Tests of the anvil_consortium_manager WorkspaceDetail view using the UploadWorkspace adapter.""" diff --git a/gregor_django/gregor_anvil/urls.py b/gregor_django/gregor_anvil/urls.py index 3fc78c2c..d43e6d49 100644 --- a/gregor_django/gregor_anvil/urls.py +++ b/gregor_django/gregor_anvil/urls.py @@ -13,6 +13,14 @@ "research_centers", ) +partner_group_patterns = ( + [ + path("", views.PartnerGroupDetail.as_view(), name="detail"), + path("", views.PartnerGroupList.as_view(), name="list"), + ], + "partner_groups", +) + consent_group_patterns = ( [ path("", views.ConsentGroupDetail.as_view(), name="detail"), @@ -30,6 +38,7 @@ urlpatterns = [ # path("", views.Index.as_view(), name="index"), path("research_centers/", include(research_center_patterns)), + path("partner_groups", include(partner_group_patterns)), path("consent_groups/", include(consent_group_patterns)), path("reports/", include(workspace_report_patterns)), ] diff --git a/gregor_django/gregor_anvil/views.py b/gregor_django/gregor_anvil/views.py index d7b53ccb..0d481a74 100644 --- a/gregor_django/gregor_anvil/views.py +++ b/gregor_django/gregor_anvil/views.py @@ -44,6 +44,25 @@ class ResearchCenterList(AnVILConsortiumManagerViewRequired, SingleTableView): table_class = tables.ResearchCenterTable +class PartnerGroupDetail( + AnVILConsortiumManagerViewRequired, SingleTableMixin, DetailView +): + """View to show details about a `PartnerGroup`.""" + + model = models.PartnerGroup + context_table_name = "partner_group_user_table" + + def get_table(self): + return UserTable(User.objects.filter(partner_groups=self.object)) + + +class PartnerGroupList(AnVILConsortiumManagerViewRequired, SingleTableView): + """View to show a list of `PartnerGroups`.""" + + model = models.PartnerGroup + table_class = tables.PartnerGroupTable + + # class UploadWorkspaceAutocomplete( # AnVILConsortiumManagerViewRequired, autocomplete.Select2QuerySetView # ): diff --git a/gregor_django/templates/anvil_consortium_manager/navbar.html b/gregor_django/templates/anvil_consortium_manager/navbar.html index 1bcd4b18..1d5180e1 100644 --- a/gregor_django/templates/anvil_consortium_manager/navbar.html +++ b/gregor_django/templates/anvil_consortium_manager/navbar.html @@ -7,6 +7,10 @@ Research centers +
  • + Partner Groups +
    • +
  • Consent Groups
    • diff --git a/gregor_django/templates/gregor_anvil/partnergroup_detail.html b/gregor_django/templates/gregor_anvil/partnergroup_detail.html new file mode 100644 index 00000000..ac82d75b --- /dev/null +++ b/gregor_django/templates/gregor_anvil/partnergroup_detail.html @@ -0,0 +1,17 @@ +{% extends "anvil_consortium_manager/__object_detail.html" %} +{% load render_table from django_tables2 %} + +{% block title %}Partner Group: {{ object.short_name }}{% endblock %} + +{% block panel %} +
      +
    • Full name: {{ object.full_name }}
      • +
    • Short name: {{ object.short_name }}
      • +
    +{% endblock panel %} + +{% block after_panel %} +

    Partner Group Users

    + {% render_table partner_group_user_table %} +

    Partner Group user list only contains those group users who have created an account on this website.

    +{% endblock after_panel %} diff --git a/gregor_django/templates/gregor_anvil/partnergroup_list.html b/gregor_django/templates/gregor_anvil/partnergroup_list.html new file mode 100644 index 00000000..477bc77e --- /dev/null +++ b/gregor_django/templates/gregor_anvil/partnergroup_list.html @@ -0,0 +1,11 @@ +{% extends "anvil_consortium_manager/base.html" %} +{% load render_table from django_tables2 %} + +{% block title %}Partner Group List{% endblock %} + +{% block content %} +

    Partner Groups

    + +{% render_table table %} + +{% endblock content %} diff --git a/gregor_django/templates/users/user_detail.html b/gregor_django/templates/users/user_detail.html index 480e549b..dbc327fe 100644 --- a/gregor_django/templates/users/user_detail.html +++ b/gregor_django/templates/users/user_detail.html @@ -36,13 +36,21 @@
    Email
  • Research Center{% if object.research_centers.all.count > 1 %}(s){% endif %}
    {% for rc in object.research_centers.all %} -

    • {{ rc.full_name }}

    +

    • {{ rc.full_name }}

    {% empty %}

    Not set

    {% endfor %} -
    • + +
  • +
    Partner Group{% if object.partner_groups.all.count > 1 %}(s){% endif %}
    + {% for pg in object.partner_groups.all %} +

    • {{ pg.full_name }}

    + {% empty %} +

    Not set

    + {% endfor %} +
    • {% if object == request.user %} -

    Update name and research center on the main GREGoR site

    +

    Update name, partner groups and research centers on the main GREGoR site

    {% endif %} diff --git a/gregor_django/users/adapters.py b/gregor_django/users/adapters.py index 9ce07ec7..c8145ecc 100644 --- a/gregor_django/users/adapters.py +++ b/gregor_django/users/adapters.py @@ -6,9 +6,11 @@ from django.conf import settings from django.contrib.auth.models import Group from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist +from django.core.mail import mail_admins +from django.db.models import Q from django.http import HttpRequest -from gregor_django.gregor_anvil.models import ResearchCenter +from gregor_django.gregor_anvil.models import PartnerGroup, ResearchCenter logger = logging.getLogger(__name__) @@ -33,34 +35,89 @@ def update_user_name(self, user, extra_data: Dict): user.name = full_name user.save() + def update_user_partner_groups(self, user, extra_data: Dict): + partner_groups = extra_data.get("partner_group", []) + logger.debug(f"partner groups: {partner_groups} for user {user}") + if partner_groups: + if not isinstance(partner_groups, list): + raise ImproperlyConfigured( + "sociallogin.extra_data.partner_groups should be None or a list" + ) + partner_group_object_list = [] + for pg_name in partner_groups: + try: + pg = PartnerGroup.objects.get( + Q(full_name=pg_name) | Q(short_name=pg_name) + ) + except ObjectDoesNotExist: + logger.debug( + f"[SocialAccountAdapter:update_user_partner_groups] Ignoring drupal " + f"partner_group {pg_name} - not in PartnerGroup domain" + ) + mail_admins( + subject="Missing PartnerGroup", + message=f"Missing partner group ({pg_name}) passed from drupal for user {user}", + ) + continue + else: + partner_group_object_list.append(pg) + + for pg in partner_group_object_list: + if not user.partner_groups.filter(pk=pg.pk): + user.partner_groups.add(pg) + logger.info( + f"[SocialAccountAdatpter:update_user_partner_groups] adding user " + f"partner_groups user: {user} rc: {pg}" + ) + + for existing_pg in user.partner_groups.all(): + if existing_pg not in partner_group_object_list: + user.partner_groups.remove(existing_pg) + logger.info( + "[SocialAccountAdapter:update_user_partner_groups] " + f"removing pg {existing_pg} for user {user}" + ) + def update_user_research_centers(self, user, extra_data: Dict): # Get list of research centers in domain table - research_center_or_site = extra_data.get("research_center_or_site") + research_center_or_site = extra_data.get("research_center_or_site", []) if research_center_or_site: if not isinstance(research_center_or_site, list): raise ImproperlyConfigured( "sociallogin.extra_data.research_center_or_site should be a list" ) + research_center_object_list = [] for rc_name in research_center_or_site: try: - rc = ResearchCenter.objects.get(full_name=rc_name) + # For transition from passed full name to short name + # support both + rc = ResearchCenter.objects.get( + Q(full_name=rc_name) | Q(short_name=rc_name) + ) except ObjectDoesNotExist: logger.debug( - f"[SocialAccountAdatpter:update_user_research_centers] Ignoring drupal " + f"[SocialAccountAdapter:update_user_research_centers] Ignoring drupal " f"research_center_or_site {rc_name} - not in ResearchCenter domain" ) + mail_admins( + subject="Missing ResearchCenter", + message=f"Missing research center {rc_name} passed from drupal for user {user}", + ) continue else: - if not user.research_centers.filter(pk=rc.pk): - user.research_centers.add(rc) - logger.info( - f"[SocialAccountAdatpter:update_user_research_centers] adding user " - f"research_centers user: {user} rc: {rc}" - ) + research_center_object_list.append(rc) + + for rc in research_center_object_list: + if not user.research_centers.filter(pk=rc.pk): + user.research_centers.add(rc) + logger.info( + f"[SocialAccountAdatpter:update_user_research_centers] adding user " + f"research_centers user: {user} rc: {rc}" + ) for existing_rc in user.research_centers.all(): - if existing_rc.full_name not in research_center_or_site: + if existing_rc not in research_center_object_list: user.research_centers.remove(existing_rc) logger.info( "[SocialAccountAdatpter:update_user_research_centers] " @@ -113,4 +170,5 @@ def update_user_data(self, sociallogin: Any): self.update_user_name(user, extra_data) self.update_user_research_centers(user, extra_data) + self.update_user_partner_groups(user, extra_data) self.update_user_groups(user, extra_data) diff --git a/gregor_django/users/migrations/0004_user_partner_groups.py b/gregor_django/users/migrations/0004_user_partner_groups.py new file mode 100644 index 00000000..8ca18f0d --- /dev/null +++ b/gregor_django/users/migrations/0004_user_partner_groups.py @@ -0,0 +1,19 @@ +# Generated by Django 3.2.16 on 2023-04-27 17:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('gregor_anvil', '0008_historicalpartnergroup_partnergroup'), + ('users', '0003_user_research_centers'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='partner_groups', + field=models.ManyToManyField(to='gregor_anvil.PartnerGroup'), + ), + ] diff --git a/gregor_django/users/models.py b/gregor_django/users/models.py index d1d0d17f..0e893179 100644 --- a/gregor_django/users/models.py +++ b/gregor_django/users/models.py @@ -3,7 +3,7 @@ from django.urls import reverse from django.utils.translation import gettext_lazy as _ -from gregor_django.gregor_anvil.models import ResearchCenter +from gregor_django.gregor_anvil.models import PartnerGroup, ResearchCenter class User(AbstractUser): @@ -14,6 +14,7 @@ class User(AbstractUser): first_name = None # type: ignore last_name = None # type: ignore research_centers = ManyToManyField(ResearchCenter) + partner_groups = ManyToManyField(PartnerGroup) def get_absolute_url(self): """Get url for user's detail view. diff --git a/gregor_django/users/tests/test_adapters.py b/gregor_django/users/tests/test_adapters.py index 72a63fec..582d44f6 100644 --- a/gregor_django/users/tests/test_adapters.py +++ b/gregor_django/users/tests/test_adapters.py @@ -10,7 +10,10 @@ from django.test.client import RequestFactory from django.test.utils import override_settings -from gregor_django.gregor_anvil.tests.factories import ResearchCenterFactory +from gregor_django.gregor_anvil.tests.factories import ( + PartnerGroupFactory, + ResearchCenterFactory, +) from gregor_django.users.adapters import AccountAdapter, SocialAccountAdapter from .factories import GroupFactory, UserFactory @@ -109,7 +112,7 @@ def test_update_research_centers_malformed(self): user, dict(research_center_or_site="FOO") ) - def test_update_user_research_centers_uknown(self): + def test_update_user_research_centers_unknown(self): adapter = SocialAccountAdapter() user = UserFactory() adapter.update_user_research_centers( @@ -119,7 +122,7 @@ def test_update_user_research_centers_uknown(self): def test_update_user_groups_add(self): adapter = SocialAccountAdapter() - rc1 = GroupFactory(name="g1") + g1 = GroupFactory(name="g1") User = get_user_model() user = User() @@ -129,11 +132,58 @@ def test_update_user_groups_add(self): user.save() adapter.update_user_groups( - user, extra_data=dict(managed_scope_status={rc1.name: True}) + user, extra_data=dict(managed_scope_status={g1.name: True}) ) - assert user.groups.filter(pk=rc1.pk).exists() + assert user.groups.filter(pk=g1.pk).exists() assert user.groups.all().count() == 1 + # Partner Groups + + def test_update_user_partner_groups_add(self): + adapter = SocialAccountAdapter() + pg1 = PartnerGroupFactory(short_name="pg1") + + User = get_user_model() + user = User() + setattr(user, account_settings.USER_MODEL_USERNAME_FIELD, "test") + setattr(user, account_settings.USER_MODEL_EMAIL_FIELD, "test@example.com") + + user.save() + + adapter.update_user_partner_groups(user, dict(partner_group=[pg1.full_name])) + assert user.partner_groups.filter(pk=pg1.pk).exists() + assert user.partner_groups.all().count() == 1 + + def test_update_user_partner_groups_remove(self): + adapter = SocialAccountAdapter() + pg1 = PartnerGroupFactory(short_name="pg1") + pg2 = PartnerGroupFactory(short_name="pg2") + + User = get_user_model() + user = User() + setattr(user, account_settings.USER_MODEL_USERNAME_FIELD, "test") + setattr(user, account_settings.USER_MODEL_EMAIL_FIELD, "test@example.com") + + user.save() + user.partner_groups.add(pg1, pg2) + assert user.partner_groups.all().count() == 2 + + adapter.update_user_partner_groups(user, dict(partner_group=[pg1.short_name])) + assert user.partner_groups.filter(pk=pg1.pk).exists() + assert user.partner_groups.all().count() == 1 + + def test_update_partner_groups_malformed(self): + adapter = SocialAccountAdapter() + user = UserFactory() + with pytest.raises(ImproperlyConfigured): + adapter.update_user_partner_groups(user, dict(partner_group="FOO")) + + def test_update_user_partner_groups_unknown(self): + adapter = SocialAccountAdapter() + user = UserFactory() + adapter.update_user_partner_groups(user, dict(partner_group=["UNKNOWN"])) + assert user.partner_groups.all().count() == 0 + def test_update_user_groups_create(self): adapter = SocialAccountAdapter() From c42d4c405afd7d030262c4a0d06f2f680317249e Mon Sep 17 00:00:00 2001 From: Jonas Carson Date: Thu, 27 Apr 2023 12:16:54 -0700 Subject: [PATCH 09/11] Add safer fallback to support both short name and full name for passed partner groups and research centers as drupal is updated --- gregor_django/users/adapters.py | 56 +++++++++++----------- gregor_django/users/tests/test_adapters.py | 8 +++- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/gregor_django/users/adapters.py b/gregor_django/users/adapters.py index c8145ecc..183e2f02 100644 --- a/gregor_django/users/adapters.py +++ b/gregor_django/users/adapters.py @@ -7,7 +7,6 @@ from django.contrib.auth.models import Group from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist from django.core.mail import mail_admins -from django.db.models import Q from django.http import HttpRequest from gregor_django.gregor_anvil.models import PartnerGroup, ResearchCenter @@ -46,19 +45,21 @@ def update_user_partner_groups(self, user, extra_data: Dict): partner_group_object_list = [] for pg_name in partner_groups: try: - pg = PartnerGroup.objects.get( - Q(full_name=pg_name) | Q(short_name=pg_name) - ) + pg = PartnerGroup.objects.get(short_name=pg_name) except ObjectDoesNotExist: - logger.debug( - f"[SocialAccountAdapter:update_user_partner_groups] Ignoring drupal " - f"partner_group {pg_name} - not in PartnerGroup domain" - ) - mail_admins( - subject="Missing PartnerGroup", - message=f"Missing partner group ({pg_name}) passed from drupal for user {user}", - ) - continue + try: + pg = PartnerGroup.objects.get(full_name=pg_name) + except ObjectDoesNotExist: + logger.debug( + f"[SocialAccountAdapter:update_user_partner_groups] Ignoring drupal " + f"partner_group {pg_name} - not in PartnerGroup domain" + ) + mail_admins( + subject="Missing PartnerGroup", + message=f"Missing partner group ({pg_name}) passed from drupal for user {user}", + ) + else: + partner_group_object_list.append(pg) else: partner_group_object_list.append(pg) @@ -90,21 +91,22 @@ def update_user_research_centers(self, user, extra_data: Dict): research_center_object_list = [] for rc_name in research_center_or_site: try: - # For transition from passed full name to short name - # support both - rc = ResearchCenter.objects.get( - Q(full_name=rc_name) | Q(short_name=rc_name) - ) + # For transition from passed full name to short name support both + rc = ResearchCenter.objects.get(short_name=rc_name) except ObjectDoesNotExist: - logger.debug( - f"[SocialAccountAdapter:update_user_research_centers] Ignoring drupal " - f"research_center_or_site {rc_name} - not in ResearchCenter domain" - ) - mail_admins( - subject="Missing ResearchCenter", - message=f"Missing research center {rc_name} passed from drupal for user {user}", - ) - continue + try: + rc = ResearchCenter.objects.get(full_name=rc_name) + except ObjectDoesNotExist: + logger.debug( + f"[SocialAccountAdapter:update_user_research_centers] Ignoring drupal " + f"research_center_or_site {rc_name} - not in ResearchCenter domain" + ) + mail_admins( + subject="Missing ResearchCenter", + message=f"Missing research center {rc_name} passed from drupal for user {user}", + ) + else: + research_center_object_list.append(rc) else: research_center_object_list.append(rc) diff --git a/gregor_django/users/tests/test_adapters.py b/gregor_django/users/tests/test_adapters.py index 582d44f6..8a7fb238 100644 --- a/gregor_django/users/tests/test_adapters.py +++ b/gregor_django/users/tests/test_adapters.py @@ -142,6 +142,7 @@ def test_update_user_groups_add(self): def test_update_user_partner_groups_add(self): adapter = SocialAccountAdapter() pg1 = PartnerGroupFactory(short_name="pg1") + pg2 = PartnerGroupFactory(short_name="pg2") User = get_user_model() user = User() @@ -150,10 +151,15 @@ def test_update_user_partner_groups_add(self): user.save() - adapter.update_user_partner_groups(user, dict(partner_group=[pg1.full_name])) + adapter.update_user_partner_groups(user, dict(partner_group=[pg1.short_name])) assert user.partner_groups.filter(pk=pg1.pk).exists() assert user.partner_groups.all().count() == 1 + # test full_name as well + adapter.update_user_partner_groups(user, dict(partner_group=[pg2.full_name])) + assert user.partner_groups.filter(pk=pg2.pk).exists() + assert user.partner_groups.all().count() == 1 + def test_update_user_partner_groups_remove(self): adapter = SocialAccountAdapter() pg1 = PartnerGroupFactory(short_name="pg1") From c2173c1516731082977048715c4850fda9e120fc Mon Sep 17 00:00:00 2001 From: Jonas Carson Date: Thu, 27 Apr 2023 12:44:10 -0700 Subject: [PATCH 10/11] Require unique full names for research centers and partner groups --- .../migrations/0009_unique_full_names.py | 33 +++++++++++++++++++ gregor_django/gregor_anvil/models.py | 4 +-- 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 gregor_django/gregor_anvil/migrations/0009_unique_full_names.py diff --git a/gregor_django/gregor_anvil/migrations/0009_unique_full_names.py b/gregor_django/gregor_anvil/migrations/0009_unique_full_names.py new file mode 100644 index 00000000..66b8c382 --- /dev/null +++ b/gregor_django/gregor_anvil/migrations/0009_unique_full_names.py @@ -0,0 +1,33 @@ +# Generated by Django 3.2.16 on 2023-04-27 19:41 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('gregor_anvil', '0008_historicalpartnergroup_partnergroup'), + ] + + operations = [ + migrations.AlterField( + model_name='historicalpartnergroup', + name='full_name', + field=models.CharField(db_index=True, max_length=255), + ), + migrations.AlterField( + model_name='historicalresearchcenter', + name='full_name', + field=models.CharField(db_index=True, max_length=255), + ), + migrations.AlterField( + model_name='partnergroup', + name='full_name', + field=models.CharField(max_length=255, unique=True), + ), + migrations.AlterField( + model_name='researchcenter', + name='full_name', + field=models.CharField(max_length=255, unique=True), + ), + ] diff --git a/gregor_django/gregor_anvil/models.py b/gregor_django/gregor_anvil/models.py index b7f88158..4425965f 100644 --- a/gregor_django/gregor_anvil/models.py +++ b/gregor_django/gregor_anvil/models.py @@ -38,7 +38,7 @@ class ResearchCenter(TimeStampedModel, models.Model): short_name = models.CharField(max_length=15, unique=True) """The short name of the Research Center.""" - full_name = models.CharField(max_length=255) + full_name = models.CharField(max_length=255, unique=True) """The full name of the Research Center.""" history = HistoricalRecords() @@ -62,7 +62,7 @@ class PartnerGroup(TimeStampedModel, models.Model): short_name = models.CharField(max_length=15, unique=True) """The short name of the Partner Group""" - full_name = models.CharField(max_length=255) + full_name = models.CharField(max_length=255, unique=True) """The full name of the Partner Group""" history = HistoricalRecords() From 34f2d9ec23b6d0dafc6f8cdff74641805256b192 Mon Sep 17 00:00:00 2001 From: Jonas Carson Date: Fri, 28 Apr 2023 06:51:22 -0700 Subject: [PATCH 11/11] combine partner groups and research center into one nav dropdown. Fix comment. --- gregor_django/gregor_anvil/tables.py | 2 +- .../anvil_consortium_manager/navbar.html | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/gregor_django/gregor_anvil/tables.py b/gregor_django/gregor_anvil/tables.py index 0fa35a18..b40c8e1e 100644 --- a/gregor_django/gregor_anvil/tables.py +++ b/gregor_django/gregor_anvil/tables.py @@ -35,7 +35,7 @@ class Meta: class PartnerGroupTable(tables.Table): - """A table for ResearchCenters.""" + """A table for PartnerGroups.""" full_name = tables.Column(linkify=True) diff --git a/gregor_django/templates/anvil_consortium_manager/navbar.html b/gregor_django/templates/anvil_consortium_manager/navbar.html index 1d5180e1..214db002 100644 --- a/gregor_django/templates/anvil_consortium_manager/navbar.html +++ b/gregor_django/templates/anvil_consortium_manager/navbar.html @@ -3,12 +3,18 @@ {% block anvil_consortium_manager_nav_items %} {{ block.super }} -
  • - Research centers -
    • - -
  • - Partner Groups +
  • + +