From 2495108976b68b8e42d5bebfdd087a5aafd3c44b Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Mon, 8 Jul 2024 15:31:27 -0700 Subject: [PATCH] Check if authorization_domains is a field before altering it In the WorkspaceAuthDomainDisabledForm, check if authorization_domains is actually a field before attempting to set it to disabled. Before this fix, attempting to set it to disabled was causing an error in the ACM WorkspaceUpdate view, because that view removed some of the fields that can't be changed after creation, including the authorzations_domains field. Add tests to catch this case. --- .../tests/test_views.py | 53 ++++++++ primed/primed_anvil/forms.py | 9 +- primed/primed_anvil/tests/test_forms.py | 127 ++++++++++++++++++ 3 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 primed/primed_anvil/tests/test_forms.py diff --git a/primed/collaborative_analysis/tests/test_views.py b/primed/collaborative_analysis/tests/test_views.py index e057e984..8c316ca3 100644 --- a/primed/collaborative_analysis/tests/test_views.py +++ b/primed/collaborative_analysis/tests/test_views.py @@ -413,6 +413,59 @@ def test_creates_workspace(self): self.assertEqual(new_workspace_data.workspace, new_workspace) +class CollaborativeAnalysisWorkspaceUpdateTest(AnVILAPIMockTestMixin, TestCase): + """Tests of the WorkspaceUpdate view from ACM with this app's CollaborativeAnalysisWorkspace model.""" + + api_success_code = 201 + + def setUp(self): + """Set up test class.""" + # The superclass uses the responses package to mock API responses. + super().setUp() + # Create a user with both view and edit permissions. + self.user = User.objects.create_user(username="test", password="test") + self.user.user_permissions.add( + Permission.objects.get(codename=AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME) + ) + self.user.user_permissions.add( + Permission.objects.get(codename=AnVILProjectManagerAccess.STAFF_EDIT_PERMISSION_CODENAME) + ) + self.workspace = factories.CollaborativeAnalysisWorkspaceFactory.create() + # Add a source workspace. + self.workspace.source_workspaces.add(dbGaPWorkspaceFactory.create().workspace) + + def get_url(self, *args): + """Get the url for the view being tested.""" + return reverse("anvil_consortium_manager:workspaces:update", args=args) + + def test_updates_workspace_data(self): + """Posting valid data to the form creates a workspace data object when using a custom adapter.""" + # Make the post request + self.client.force_login(self.user) + response = self.client.post( + self.get_url( + self.workspace.workspace.billing_project.name, + self.workspace.workspace.name, + ), + { + # Workspace data form. + "workspacedata-TOTAL_FORMS": 1, + "workspacedata-INITIAL_FORMS": 1, + "workspacedata-MIN_NUM_FORMS": 1, + "workspacedata-MAX_NUM_FORMS": 1, + "workspacedata-0-id": self.workspace.pk, + "workspacedata-0-workspace": self.workspace.workspace.pk, + "workspacedata-0-custodian": self.workspace.custodian.pk, + "workspacedata-0-source_workspaces": self.workspace.source_workspaces.values_list("pk", flat=True), + "workspacedata-0-purpose": "updated purpose", + "workspacedata-0-analyst_group": self.workspace.analyst_group.pk, + }, + ) + self.assertEqual(response.status_code, 302) + self.workspace.refresh_from_db() + self.assertEqual(self.workspace.purpose, "updated purpose") + + class WorkspaceAuditTest(TestCase): """Tests for the CollaborativeAnalysisWorkspaceAuditTest view.""" diff --git a/primed/primed_anvil/forms.py b/primed/primed_anvil/forms.py index 91e1dfba..bacb8e1a 100644 --- a/primed/primed_anvil/forms.py +++ b/primed/primed_anvil/forms.py @@ -13,7 +13,8 @@ class WorkspaceAuthDomainDisabledForm(WorkspaceForm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields["authorization_domains"].disabled = True - self.fields["authorization_domains"].help_text = ( - "An authorization domain will be automatically created " "using the name of the workspace." - ) + if "authorization_domains" in self.fields: + self.fields["authorization_domains"].disabled = True + self.fields["authorization_domains"].help_text = ( + "An authorization domain will be automatically created " "using the name of the workspace." + ) diff --git a/primed/primed_anvil/tests/test_forms.py b/primed/primed_anvil/tests/test_forms.py new file mode 100644 index 00000000..faa0be16 --- /dev/null +++ b/primed/primed_anvil/tests/test_forms.py @@ -0,0 +1,127 @@ +"""Test forms for the `collaborative_analysis` app.""" + +from anvil_consortium_manager.models import ManagedGroup +from anvil_consortium_manager.tests.factories import BillingProjectFactory, ManagedGroupFactory, WorkspaceFactory +from django.core.exceptions import NON_FIELD_ERRORS +from django.test import TestCase +from faker import Faker + +from .. import forms + +fake = Faker() + + +class WorkspaceAuthDomainDisabledFormTest(TestCase): + """Tests for the WorkspaceAuthDomainDisabledForm class.""" + + form_class = forms.WorkspaceAuthDomainDisabledForm + + def test_valid(self): + """Form is valid with necessary input.""" + billing_project = BillingProjectFactory.create() + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + + def test_valid_with_note(self): + """Form is valid with necessary input and note is specified.""" + billing_project = BillingProjectFactory.create() + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + "note": "test note", + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + + def test_valid_with_is_requester_pays(self): + """Form is valid with necessary input and note is specified.""" + billing_project = BillingProjectFactory.create() + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + "note": "test note", + "is_requester_pays": True, + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + + def test_invalid_missing_billing_project(self): + """Form is invalid when missing billing_project_name.""" + form_data = {"name": "test-workspace"} + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertIn("billing_project", form.errors) + self.assertEqual(len(form.errors), 1) + + def test_invalid_missing_workspace(self): + """Form is invalid when missing billing_project_name.""" + billing_project = BillingProjectFactory.create() + form_data = {"billing_project": billing_project} + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertIn("name", form.errors) + self.assertEqual(len(form.errors), 1) + + def test_one_authorization_domain_ignored(self): + billing_project = BillingProjectFactory.create() + ManagedGroupFactory.create() + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + "authorization_domains": ManagedGroup.objects.all(), + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + self.assertEqual(len(form.cleaned_data["authorization_domains"]), 0) + + def test_two_authorization_domains_ignored(self): + billing_project = BillingProjectFactory.create() + ManagedGroupFactory.create_batch(2) + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + "authorization_domains": ManagedGroup.objects.all(), + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + self.assertEqual(len(form.cleaned_data["authorization_domains"]), 0) + + def test_invalid_not_user_of_billing_project(self): + billing_project = BillingProjectFactory.create(has_app_as_user=False) + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + } + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertEqual(len(form.errors), 1) + self.assertIn("billing_project", form.errors) + self.assertEqual(len(form.errors["billing_project"]), 1) + self.assertIn("has_app_as_user", form.errors["billing_project"][0]) + + def test_invalid_case_insensitive_duplicate(self): + """Cannot validate with the same case-insensitive name in the same billing project as an existing workspace.""" + billing_project = BillingProjectFactory.create() + name = "AbAbA" + WorkspaceFactory.create(billing_project=billing_project, name=name) + form_data = {"billing_project": billing_project, "name": name.lower()} + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertEqual(len(form.errors), 1) + self.assertIn(NON_FIELD_ERRORS, form.errors) + self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) + self.assertIn("already exists", form.errors[NON_FIELD_ERRORS][0]) + + def test_auth_domain_excluded(self): + "Form can be instantiated when auth domain is excluded from the form." + + class TestForm(forms.WorkspaceAuthDomainDisabledForm): + class Meta(forms.WorkspaceAuthDomainDisabledForm.Meta): + exclude = ["authorization_domains"] + + form = TestForm() + self.assertNotIn("authorization_domains", form.fields)