From 4a8281d04cf9549357efa55bf42fe9898e1684b9 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 15 Mar 2024 13:53:11 -0700 Subject: [PATCH 01/17] Add requires_study_review field to DataAffiliateAgreements --- ...greement_requires_study_review_and_more.py | 29 +++++++++++++++++++ primed/cdsa/models.py | 8 +++++ primed/cdsa/tests/test_models.py | 21 ++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 primed/cdsa/migrations/0018_dataaffiliateagreement_requires_study_review_and_more.py diff --git a/primed/cdsa/migrations/0018_dataaffiliateagreement_requires_study_review_and_more.py b/primed/cdsa/migrations/0018_dataaffiliateagreement_requires_study_review_and_more.py new file mode 100644 index 00000000..de611aa7 --- /dev/null +++ b/primed/cdsa/migrations/0018_dataaffiliateagreement_requires_study_review_and_more.py @@ -0,0 +1,29 @@ +# Generated by Django 4.2.10 on 2024-03-15 20:48 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cdsa", "0017_alter_cdsaworkspace_additional_limitations_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="dataaffiliateagreement", + name="requires_study_review", + field=models.BooleanField( + default=False, + help_text="Indicator of whether indicates investigators need to have an approved PRIMED paper proposal where this dataset was selected and approved in order to work with data brought under this CDSA.", + ), + ), + migrations.AddField( + model_name="historicaldataaffiliateagreement", + name="requires_study_review", + field=models.BooleanField( + default=False, + help_text="Indicator of whether indicates investigators need to have an approved PRIMED paper proposal where this dataset was selected and approved in order to work with data brought under this CDSA.", + ), + ), + ] diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index 0e223f55..b8f897bf 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -281,6 +281,14 @@ class DataAffiliateAgreement(TimeStampedModel, AgreementTypeModel, models.Model) blank=True, help_text="Additional limitations on data use as specified in the signed CDSA.", ) + requires_study_review = models.BooleanField( + default=False, + help_text=( + "Indicator of whether indicates investigators need to have an approved PRIMED paper proposal " + "where this dataset was selected and approved in order to work with data brought " + "under this CDSA." + ), + ) def get_absolute_url(self): return reverse( diff --git a/primed/cdsa/tests/test_models.py b/primed/cdsa/tests/test_models.py index 98b9858f..dfb2589b 100644 --- a/primed/cdsa/tests/test_models.py +++ b/primed/cdsa/tests/test_models.py @@ -471,6 +471,20 @@ def test_get_agreement_group(self): class DataAffiliateAgreementTest(TestCase): """Tests for the DataAffiliateAgreement model.""" + def test_defaults(self): + upload_group = ManagedGroupFactory.create() + signed_agreement = factories.SignedAgreementFactory.create( + type=models.SignedAgreement.DATA_AFFILIATE + ) + study = StudyFactory.create() + instance = models.DataAffiliateAgreement( + signed_agreement=signed_agreement, + study=study, + anvil_upload_group=upload_group, + ) + self.assertFalse(instance.requires_study_review) + self.assertEqual(instance.additional_limitations, "") + def test_model_saving(self): """Creation using the model constructor and .save() works.""" upload_group = ManagedGroupFactory.create() @@ -557,6 +571,13 @@ def test_get_agreement_group(self): instance = factories.DataAffiliateAgreementFactory.create() self.assertEqual(instance.get_agreement_group(), instance.study) + def test_requires_study_review(self): + """Can set requires_study_review""" + instance = factories.DataAffiliateAgreementFactory.create( + requires_study_review=True + ) + self.assertTrue(instance.requires_study_review) + class NonDataAffiliateAgreementTest(TestCase): """Tests for the NonDataAffiliateAgreement model.""" From cc40b38d3e5eebe475adf9143785459b40c3414f Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 15 Mar 2024 13:59:44 -0700 Subject: [PATCH 02/17] Only allow requires_study_review=True for primary agreements --- primed/cdsa/models.py | 18 ++++++++++-------- primed/cdsa/tests/test_models.py | 12 +++++++++++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index b8f897bf..8336da2f 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -298,14 +298,16 @@ def get_absolute_url(self): def clean(self): super().clean() - if ( - self.additional_limitations - and hasattr(self, "signed_agreement") - and not self.signed_agreement.is_primary - ): - raise ValidationError( - "Additional limitations are only allowed for primary agreements." - ) + # Checks for fields only allowed for primary agreements. + if hasattr(self, "signed_agreement") and not self.signed_agreement.is_primary: + if self.additional_limitations: + raise ValidationError( + "Additional limitations are only allowed for primary agreements." + ) + if self.requires_study_review: + raise ValidationError( + "requires_study_review can only be True for primary agreements." + ) def get_agreement_group(self): return self.study diff --git a/primed/cdsa/tests/test_models.py b/primed/cdsa/tests/test_models.py index dfb2589b..f772ca74 100644 --- a/primed/cdsa/tests/test_models.py +++ b/primed/cdsa/tests/test_models.py @@ -571,13 +571,23 @@ def test_get_agreement_group(self): instance = factories.DataAffiliateAgreementFactory.create() self.assertEqual(instance.get_agreement_group(), instance.study) - def test_requires_study_review(self): + def test_requires_study_review_primary(self): """Can set requires_study_review""" instance = factories.DataAffiliateAgreementFactory.create( requires_study_review=True ) self.assertTrue(instance.requires_study_review) + def test_requires_study_review_not_primary(self): + """ValidationError when trying to set requires_study_review=True for components.""" + instance = factories.DataAffiliateAgreementFactory.create( + signed_agreement__is_primary=False, + requires_study_review=True, + ) + with self.assertRaises(ValidationError) as e: + instance.clean() + self.assertIn("can only be True for primary", e.exception.message) + class NonDataAffiliateAgreementTest(TestCase): """Tests for the NonDataAffiliateAgreement model.""" From b26b237c985547482575d2da8dedea52700aee18 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 15 Mar 2024 14:03:13 -0700 Subject: [PATCH 03/17] Add requires_study_review to DataAffiliateAgreementForm --- primed/cdsa/forms.py | 1 + primed/cdsa/tests/test_forms.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/primed/cdsa/forms.py b/primed/cdsa/forms.py index 5fbf7dab..993e2250 100644 --- a/primed/cdsa/forms.py +++ b/primed/cdsa/forms.py @@ -78,6 +78,7 @@ class Meta: "signed_agreement", "study", "additional_limitations", + "requires_study_review", ) widgets = { "study": autocomplete.ModelSelect2( diff --git a/primed/cdsa/tests/test_forms.py b/primed/cdsa/tests/test_forms.py index 3ab4573f..d326207c 100644 --- a/primed/cdsa/tests/test_forms.py +++ b/primed/cdsa/tests/test_forms.py @@ -446,6 +446,35 @@ def test_invalid_component_with_additional_limitations(self): self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) self.assertIn("only allowed for primary", form.errors[NON_FIELD_ERRORS][0]) + def test_valid_primary_with_requires_study_review_true(self): + """Form is valid with necessary input.""" + signed_agreement = factories.SignedAgreementFactory.create( + type=models.SignedAgreement.DATA_AFFILIATE, is_primary=True + ) + form_data = { + "signed_agreement": signed_agreement, + "study": self.study, + "requires_study_review": True, + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + + def test_invalid_component_with_requires_study_review_true(self): + """Form is valid with necessary input.""" + signed_agreement = factories.SignedAgreementFactory.create( + type=models.SignedAgreement.DATA_AFFILIATE, is_primary=False + ) + form_data = { + "signed_agreement": signed_agreement, + "study": self.study, + "requires_study_review": True, + } + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertIn(NON_FIELD_ERRORS, form.errors) + self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) + self.assertIn("can only be True for primary", form.errors[NON_FIELD_ERRORS][0]) + class NonDataAffiliateAgreementFormTest(TestCase): """Tests for the NonDataAffiliateAgreementForm class.""" From ba7599fde3c84d49f625493f8a8306b17d9de903 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 15 Mar 2024 14:42:00 -0700 Subject: [PATCH 04/17] Add view tests for DataAffiliate primary-only fields Add tests in the DataAffiliateCreate view to verify that you can't create a DataAffiliate component with the additional_limitations or requires_study_review=True. These tests are failing - the clean method isn't working because it is essentially cleaning two models together, and the DataAffiliateAgreement form instance doesn't have the signed_agreement field set before the form/formset are saved. --- primed/cdsa/tests/test_views.py | 188 ++++++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 77116569..2b16442b 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -2862,6 +2862,194 @@ def test_success_message(self): views.DataAffiliateAgreementCreate.success_message, str(messages[0]) ) + def test_can_create_primary_with_requires_study_review(self): + """Can create an object.""" + self.client.force_login(self.user) + representative = UserFactory.create() + agreement_version = factories.AgreementVersionFactory.create() + study = StudyFactory.create() + # API response to create the associated anvil_access_group. + self.anvil_response_mock.add( + responses.POST, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_ACCESS_1234", + status=201, + json={"message": "mock message"}, + ) + self.anvil_response_mock.add( + responses.POST, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_UPLOAD_1234", + status=201, + json={"message": "mock message"}, + ) + # CC admins group membership. + self.anvil_response_mock.add( + responses.PUT, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_ACCESS_1234/admin/TEST_PRIMED_CC_ADMINS@firecloud.org", + status=204, + ) + self.anvil_response_mock.add( + responses.PUT, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_UPLOAD_1234/admin/TEST_PRIMED_CC_ADMINS@firecloud.org", + status=204, + ) + response = self.client.post( + self.get_url(), + { + "cc_id": 1234, + "representative": representative.pk, + "representative_role": "Test role", + "signing_institution": "Test institution", + "version": agreement_version.pk, + "date_signed": "2023-01-01", + "is_primary": True, + "agreementtype-TOTAL_FORMS": 1, + "agreementtype-INITIAL_FORMS": 0, + "agreementtype-MIN_NUM_FORMS": 1, + "agreementtype-MAX_NUM_FORMS": 1, + "agreementtype-0-study": study.pk, + "agreementtype-0-requires_study_review": True, + }, + ) + self.assertEqual(response.status_code, 302) + # Check the agreement type. + self.assertEqual(models.DataAffiliateAgreement.objects.count(), 1) + new_agreement_type = models.DataAffiliateAgreement.objects.latest("pk") + self.assertTrue(new_agreement_type.requires_study_review) + + def test_cannot_create_component_with_requires_study_review(self): + """Cannot create a component agreement with requires_study_review=True.""" + self.client.force_login(self.user) + representative = UserFactory.create() + agreement_version = factories.AgreementVersionFactory.create() + study = StudyFactory.create() + response = self.client.post( + self.get_url(), + { + "cc_id": 1234, + "representative": representative.pk, + "representative_role": "Test role", + "signing_institution": "Test institution", + "version": agreement_version.pk, + "date_signed": "2023-01-01", + "is_primary": False, + "agreementtype-TOTAL_FORMS": 1, + "agreementtype-INITIAL_FORMS": 0, + "agreementtype-MIN_NUM_FORMS": 1, + "agreementtype-MAX_NUM_FORMS": 1, + "agreementtype-0-study": study.pk, + "agreementtype-0-requires_study_review": True, + }, + ) + self.assertEqual(response.status_code, 200) + self.assertIn("form", response.context) + self.assertFalse(response.context["form"].is_valid()) + form = response.context["form"] + self.assertEqual(len(form.errors), 1) + self.assertIn(NON_FIELD_ERRORS, form.errors) + self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) + self.assertIn( + "can only be True for primary", + form.errors[NON_FIELD_ERRORS][0], + ) + + def test_can_create_primary_with_additional_limitations(self): + """Can create an object.""" + self.client.force_login(self.user) + representative = UserFactory.create() + agreement_version = factories.AgreementVersionFactory.create() + study = StudyFactory.create() + # API response to create the associated anvil_access_group. + self.anvil_response_mock.add( + responses.POST, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_ACCESS_1234", + status=201, + json={"message": "mock message"}, + ) + self.anvil_response_mock.add( + responses.POST, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_UPLOAD_1234", + status=201, + json={"message": "mock message"}, + ) + # CC admins group membership. + self.anvil_response_mock.add( + responses.PUT, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_ACCESS_1234/admin/TEST_PRIMED_CC_ADMINS@firecloud.org", + status=204, + ) + self.anvil_response_mock.add( + responses.PUT, + self.api_client.sam_entry_point + + "/api/groups/v1/TEST_PRIMED_CDSA_UPLOAD_1234/admin/TEST_PRIMED_CC_ADMINS@firecloud.org", + status=204, + ) + response = self.client.post( + self.get_url(), + { + "cc_id": 1234, + "representative": representative.pk, + "representative_role": "Test role", + "signing_institution": "Test institution", + "version": agreement_version.pk, + "date_signed": "2023-01-01", + "is_primary": True, + "agreementtype-TOTAL_FORMS": 1, + "agreementtype-INITIAL_FORMS": 0, + "agreementtype-MIN_NUM_FORMS": 1, + "agreementtype-MAX_NUM_FORMS": 1, + "agreementtype-0-study": study.pk, + "agreementtype-0-additional_limitations": "Test limitations", + }, + ) + self.assertEqual(response.status_code, 302) + # Check the agreement type. + self.assertEqual(models.DataAffiliateAgreement.objects.count(), 1) + new_agreement_type = models.DataAffiliateAgreement.objects.latest("pk") + self.assertEqual(new_agreement_type.additional_limitations, "Test limitations") + + def test_cannot_create_component_with_additional_limitations(self): + """Cannot create a component agreement with additional_limitations.""" + self.client.force_login(self.user) + representative = UserFactory.create() + agreement_version = factories.AgreementVersionFactory.create() + study = StudyFactory.create() + response = self.client.post( + self.get_url(), + { + "cc_id": 1234, + "representative": representative.pk, + "representative_role": "Test role", + "signing_institution": "Test institution", + "version": agreement_version.pk, + "date_signed": "2023-01-01", + "is_primary": False, + "agreementtype-TOTAL_FORMS": 1, + "agreementtype-INITIAL_FORMS": 0, + "agreementtype-MIN_NUM_FORMS": 1, + "agreementtype-MAX_NUM_FORMS": 1, + "agreementtype-0-study": study.pk, + "agreementtype-0-additional_limitations": "Test limitations", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertIn("form", response.context) + self.assertFalse(response.context["form"].is_valid()) + form = response.context["form"] + self.assertEqual(len(form.errors), 1) + self.assertIn(NON_FIELD_ERRORS, form.errors) + self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) + self.assertIn( + "only allowed for primary", + form.errors[NON_FIELD_ERRORS][0], + ) + def test_error_missing_cc_id(self): """Form shows an error when cc_id is missing.""" self.client.force_login(self.user) From 024ce552bfe04f69f680f60a5d89e5de6a198735 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Mon, 18 Mar 2024 10:50:48 -0700 Subject: [PATCH 05/17] Move the is_primary field to agreement type models This will make the forms easier to process, and also makes some sense because the NonDataAffiliate type does not differentiate between primary and components - they are all primary. --- primed/cdsa/admin.py | 10 +- .../0019_alter_signedagreement_is_primary.py | 29 +++ ...aaffiliateagreement_is_primary_and_more.py | 49 ++++ .../migrations/0021_populate_is_primary.py | 60 +++++ .../0022_remove_signedagreement_is_primary.py | 21 ++ primed/cdsa/models.py | 9 +- primed/cdsa/tests/test_migrations.py | 225 +++++++++++++++++- 7 files changed, 394 insertions(+), 9 deletions(-) create mode 100644 primed/cdsa/migrations/0019_alter_signedagreement_is_primary.py create mode 100644 primed/cdsa/migrations/0020_dataaffiliateagreement_is_primary_and_more.py create mode 100644 primed/cdsa/migrations/0021_populate_is_primary.py create mode 100644 primed/cdsa/migrations/0022_remove_signedagreement_is_primary.py diff --git a/primed/cdsa/admin.py b/primed/cdsa/admin.py index cdc71f4d..79abf632 100644 --- a/primed/cdsa/admin.py +++ b/primed/cdsa/admin.py @@ -46,13 +46,13 @@ class SignedAgreement(SimpleHistoryAdmin): "cc_id", "representative", "type", - "is_primary", + # "is_primary", "date_signed", "version", ) list_filter = ( "type", - "is_primary", + # "is_primary", "version", "status", ) @@ -79,7 +79,7 @@ class MemberAgreementAdmin(SimpleHistoryAdmin): ) list_filter = ( "study_site", - "signed_agreement__is_primary", + # "signed_agreement__is_primary", "signed_agreement__status", ) @@ -94,7 +94,7 @@ class DataAffiliateAgreementAdmin(SimpleHistoryAdmin): ) list_filter = ( "study", - "signed_agreement__is_primary", + # "signed_agreement__is_primary", "signed_agreement__status", ) @@ -108,7 +108,7 @@ class NonDataAffiliateAgreementAdmin(SimpleHistoryAdmin): "affiliation", ) list_filter = ( - "signed_agreement__is_primary", + # "signed_agreement__is_primary", "signed_agreement__status", ) diff --git a/primed/cdsa/migrations/0019_alter_signedagreement_is_primary.py b/primed/cdsa/migrations/0019_alter_signedagreement_is_primary.py new file mode 100644 index 00000000..cc8ab57f --- /dev/null +++ b/primed/cdsa/migrations/0019_alter_signedagreement_is_primary.py @@ -0,0 +1,29 @@ +# Generated by Django 4.2.10 on 2024-03-18 17:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cdsa", "0018_dataaffiliateagreement_requires_study_review_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="historicalsignedagreement", + name="is_primary", + field=models.BooleanField( + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + null=True, + ), + ), + migrations.AlterField( + model_name="signedagreement", + name="is_primary", + field=models.BooleanField( + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + null=True, + ), + ), + ] diff --git a/primed/cdsa/migrations/0020_dataaffiliateagreement_is_primary_and_more.py b/primed/cdsa/migrations/0020_dataaffiliateagreement_is_primary_and_more.py new file mode 100644 index 00000000..181a5bac --- /dev/null +++ b/primed/cdsa/migrations/0020_dataaffiliateagreement_is_primary_and_more.py @@ -0,0 +1,49 @@ +# Generated by Django 4.2.10 on 2024-03-15 22:43 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cdsa", "0019_alter_signedagreement_is_primary"), + ] + + operations = [ + migrations.AddField( + model_name="dataaffiliateagreement", + name="is_primary", + field=models.BooleanField( + default=False, + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + ), + preserve_default=False, + ), + migrations.AddField( + model_name="historicaldataaffiliateagreement", + name="is_primary", + field=models.BooleanField( + default=False, + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + ), + preserve_default=False, + ), + migrations.AddField( + model_name="historicalmemberagreement", + name="is_primary", + field=models.BooleanField( + default=False, + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + ), + preserve_default=False, + ), + migrations.AddField( + model_name="memberagreement", + name="is_primary", + field=models.BooleanField( + default=False, + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + ), + preserve_default=False, + ), + ] diff --git a/primed/cdsa/migrations/0021_populate_is_primary.py b/primed/cdsa/migrations/0021_populate_is_primary.py new file mode 100644 index 00000000..11cbd17a --- /dev/null +++ b/primed/cdsa/migrations/0021_populate_is_primary.py @@ -0,0 +1,60 @@ +# Generated by Django 4.2.10 on 2024-03-15 22:44 + +from django.db import migrations + +def forward_populate_memberagreement_is_primary(apps, schema_editor): + """Populate the MemberAgreement is_primary field using is_primary from the associated SignedAgreement.""" + MemberAgreement = apps.get_model("cdsa", "MemberAgreement") + for row in MemberAgreement.objects.all(): + row.is_primary = row.signed_agreement.is_primary + row.save(update_fields=["is_primary"]) + + +def forward_populate_dataaffiliateagreement_is_primary(apps, schema_editor): + """Populate the DataAffiliateAgreement is_primary field using is_primary from the associated SignedAgreement.""" + DataAffiliateAgreement = apps.get_model("cdsa", "DataAffiliateAgreement") + for row in DataAffiliateAgreement.objects.all(): + row.is_primary = row.signed_agreement.is_primary + row.save(update_fields=["is_primary"]) + +def backward_populate_memberagreement_is_primary(apps, schema_editor): + """Populate the MemberAgreement is_primary field using is_primary from the associated SignedAgreement.""" + MemberAgreement = apps.get_model("cdsa", "MemberAgreement") + for row in MemberAgreement.objects.all(): + row.signed_agreement.is_primary = row.is_primary + row.signed_agreement.save(update_fields=["is_primary"]) + +def backward_populate_dataaffiliateagreement_is_primary(apps, schema_editor): + """Populate the DataAffiliateAgreement is_primary field using is_primary from the associated SignedAgreement.""" + DataAffiliateAgreement = apps.get_model("cdsa", "DataAffiliateAgreement") + for row in DataAffiliateAgreement.objects.all(): + row.signed_agreement.is_primary = row.is_primary + row.signed_agreement.save(update_fields=["is_primary"]) + +def backward_populate_nondataaffiliateagreement_is_primary(apps, schema_editor): + """Set the NonDataAffiliateAgreement.signed_agreement.is_primary field to True.""" + NonDataAffiliateAgreement = apps.get_model("cdsa", "NonDataAffiliateAgreement") + for row in NonDataAffiliateAgreement.objects.all(): + row.signed_agreement.is_primary = True + row.signed_agreement.save(update_fields=["is_primary"]) + +class Migration(migrations.Migration): + + dependencies = [ + ("cdsa", "0020_dataaffiliateagreement_is_primary_and_more"), + ] + + operations = [ + migrations.RunPython( + forward_populate_memberagreement_is_primary, + reverse_code=backward_populate_memberagreement_is_primary, + ), + migrations.RunPython( + forward_populate_dataaffiliateagreement_is_primary, + reverse_code=backward_populate_dataaffiliateagreement_is_primary, + ), + migrations.RunPython( + migrations.RunPython.noop, + reverse_code=backward_populate_nondataaffiliateagreement_is_primary, + ), + ] diff --git a/primed/cdsa/migrations/0022_remove_signedagreement_is_primary.py b/primed/cdsa/migrations/0022_remove_signedagreement_is_primary.py new file mode 100644 index 00000000..c476b7a0 --- /dev/null +++ b/primed/cdsa/migrations/0022_remove_signedagreement_is_primary.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.10 on 2024-03-18 17:31 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("cdsa", "0021_populate_is_primary"), + ] + + operations = [ + migrations.RemoveField( + model_name="historicalsignedagreement", + name="is_primary", + ), + migrations.RemoveField( + model_name="signedagreement", + name="is_primary", + ), + ] diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index 8336da2f..0c0f82bd 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -155,9 +155,6 @@ class SignedAgreement( max_length=31, choices=TYPE_CHOICES, ) - is_primary = models.BooleanField( - help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", - ) version = models.ForeignKey( AgreementVersion, help_text="The version of the Agreement that was signed.", @@ -250,6 +247,9 @@ class MemberAgreement(TimeStampedModel, AgreementTypeModel, models.Model): AGREEMENT_TYPE = SignedAgreement.MEMBER + is_primary = models.BooleanField( + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + ) study_site = models.ForeignKey( StudySite, on_delete=models.CASCADE, @@ -271,6 +271,9 @@ class DataAffiliateAgreement(TimeStampedModel, AgreementTypeModel, models.Model) AGREEMENT_TYPE = SignedAgreement.DATA_AFFILIATE + is_primary = models.BooleanField( + help_text="Indicator of whether this is a primary Agreement (and not a component Agreement).", + ) study = models.ForeignKey( Study, on_delete=models.PROTECT, diff --git a/primed/cdsa/tests/test_migrations.py b/primed/cdsa/tests/test_migrations.py index 9eb16740..d355392c 100644 --- a/primed/cdsa/tests/test_migrations.py +++ b/primed/cdsa/tests/test_migrations.py @@ -5,7 +5,8 @@ from django_test_migrations.contrib.unittest_case import MigratorTestCase import factory -from . import factories +from primed.primed_anvil.tests.factories import StudySiteFactory + class AgreementMajorVersionMigrationsTest(MigratorTestCase): """Tests for the migrations associated with creating the new AgreementMajorVersion model.""" @@ -100,3 +101,225 @@ def test_agreement_version_major_version_correctly_populated(self): agreement_version.full_clean() self.assertEqual(agreement_version.major_version, major_version) self.assertEqual(agreement_version.minor_version, 6) + + +class PopulateIsPrimaryMigrationsForwardTest(MigratorTestCase): + """Tests for the migrations associated with creating the new AgreementMajorVersion model.""" + + migrate_from = ("cdsa", "0018_dataaffiliateagreement_requires_study_review_and_more") + migrate_to = ("cdsa", "0022_remove_signedagreement_is_primary") + + def prepare(self): + """Prepare some data before the migration.""" + # Get model definition for the old state. + User = self.old_state.apps.get_model("users", "User") + ManagedGroup = self.old_state.apps.get_model("anvil_consortium_manager", "ManagedGroup") + StudySite = self.old_state.apps.get_model("primed_anvil", "StudySite") + Study = self.old_state.apps.get_model("primed_anvil", "Study") + AgreementMajorVersion = self.old_state.apps.get_model("cdsa", "AgreementMajorVersion") + AgreementVersion = self.old_state.apps.get_model("cdsa", "AgreementVersion") + SignedAgreement = self.old_state.apps.get_model("cdsa", "SignedAgreement") + MemberAgreement = self.old_state.apps.get_model("cdsa", "MemberAgreement") + DataAffiliateAgreement = self.old_state.apps.get_model("cdsa", "DataAffiliateAgreement") + NonDataAffiliateAgreement = self.old_state.apps.get_model("cdsa", "NonDataAffiliateAgreement") + # Populate some signed agreements. + agreement_version = AgreementVersion.objects.create( + major_version=AgreementMajorVersion.objects.create(version=1, is_valid=True), + minor_version=0, + ) + tmp = SignedAgreement.objects.create( + cc_id=1, + representative=User.objects.create_user(username="test1", password="test1"), + representative_role="Test role 1", + signing_institution="Test institution 1", + type="member", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess1", email="testaccess1@example.com"), + is_primary=True + ) + self.member_agreement_1 = MemberAgreement.objects.create( + signed_agreement=tmp, + study_site=StudySite.objects.create(short_name="test1", full_name="Test Study 1"), + ) + tmp = SignedAgreement.objects.create( + cc_id=2, + representative=User.objects.create_user(username="test2", password="test2"), + representative_role="Test role 2", + signing_institution="Test institution 2", + type="member", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess2", email="testaccess2@example.com"), + is_primary=False + ) + self.member_agreement_2 = MemberAgreement.objects.create( + signed_agreement=tmp, + study_site=StudySite.objects.get(short_name="test1"), + ) + tmp = SignedAgreement.objects.create( + cc_id=3, + representative=User.objects.create_user(username="test3", password="test3"), + representative_role="Test role 3", + signing_institution="Test institution 3", + type="data_affiliate", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess3", email="testaccess3@example.com"), + is_primary=True + ) + self.data_affiliate_agreement_1 = DataAffiliateAgreement.objects.create( + signed_agreement=tmp, + study=Study.objects.create(short_name="test2", full_name="Test Study Site 2"), + anvil_upload_group=ManagedGroup.objects.create(name="testupload1", email="testupload1@example.com"), + ) + tmp = SignedAgreement.objects.create( + cc_id=4, + representative=User.objects.create_user(username="test4", password="test4"), + representative_role="Test role 4", + signing_institution="Test institution 4", + type="data_affiliate", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess4", email="testaccess4@example.com"), + is_primary=False + ) + self.data_affiliate_agreement_2 = DataAffiliateAgreement.objects.create( + signed_agreement=tmp, + study=Study.objects.get(short_name="test2"), + anvil_upload_group=ManagedGroup.objects.create(name="testupload2", email="testupload2@example.com"), + ) + tmp = SignedAgreement.objects.create( + cc_id=5, + representative=User.objects.create_user(username="test5", password="test5"), + representative_role="Test role 5", + signing_institution="Test institution 5", + type="non_data_affiliate", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess5", email="testaccess5@example.com"), + is_primary=False + ) + self.non_data_affiliate_agreement = NonDataAffiliateAgreement.objects.create( + signed_agreement=tmp, + ) + + def test_is_primary_correctly_populated(self): +# import ipdb; ipdb.set_trace() + MemberAgreement = self.new_state.apps.get_model("cdsa", "MemberAgreement") + DataAffiliateAgreement = self.new_state.apps.get_model("cdsa", "DataAffiliateAgreement") + NonDataAffiliateAgreement = self.new_state.apps.get_model("cdsa", "NonDataAffiliateAgreement") + instance = MemberAgreement.objects.get(pk=self.member_agreement_1.pk) + self.assertEqual(instance.is_primary, True) + instance = MemberAgreement.objects.get(pk=self.member_agreement_2.pk) + self.assertEqual(instance.is_primary, False) + instance = DataAffiliateAgreement.objects.get(pk=self.data_affiliate_agreement_1.pk) + self.assertEqual(instance.is_primary, True) + instance = DataAffiliateAgreement.objects.get(pk=self.data_affiliate_agreement_2.pk) + self.assertEqual(instance.is_primary, False) + instance = NonDataAffiliateAgreement.objects.get(pk=self.non_data_affiliate_agreement.pk) + self.assertFalse(hasattr(self.non_data_affiliate_agreement, "is_primary")) + + +class PopulateIsPrimaryMigrationsBackwardTest(MigratorTestCase): + """Tests for the migrations associated with creating the new AgreementMajorVersion model.""" + + migrate_from = ("cdsa", "0022_remove_signedagreement_is_primary") + migrate_to = ("cdsa", "0018_dataaffiliateagreement_requires_study_review_and_more") + + def prepare(self): + """Prepare some data before the migration.""" + # Get model definition for the old state. + User = self.old_state.apps.get_model("users", "User") + ManagedGroup = self.old_state.apps.get_model("anvil_consortium_manager", "ManagedGroup") + StudySite = self.old_state.apps.get_model("primed_anvil", "StudySite") + Study = self.old_state.apps.get_model("primed_anvil", "Study") + AgreementMajorVersion = self.old_state.apps.get_model("cdsa", "AgreementMajorVersion") + AgreementVersion = self.old_state.apps.get_model("cdsa", "AgreementVersion") + SignedAgreement = self.old_state.apps.get_model("cdsa", "SignedAgreement") + MemberAgreement = self.old_state.apps.get_model("cdsa", "MemberAgreement") + DataAffiliateAgreement = self.old_state.apps.get_model("cdsa", "DataAffiliateAgreement") + NonDataAffiliateAgreement = self.old_state.apps.get_model("cdsa", "NonDataAffiliateAgreement") + # Populate some signed agreements. + agreement_version = AgreementVersion.objects.create( + major_version=AgreementMajorVersion.objects.create(version=1, is_valid=True), + minor_version=0, + ) + tmp = SignedAgreement.objects.create( + cc_id=1, + representative=User.objects.create_user(username="test1", password="test1"), + representative_role="Test role 1", + signing_institution="Test institution 1", + type="member", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess1", email="testaccess1@example.com"), + ) + self.member_agreement_1 = MemberAgreement.objects.create( + signed_agreement=tmp, + study_site=StudySite.objects.create(short_name="test1", full_name="Test Study 1"), + is_primary=True, + ) + tmp = SignedAgreement.objects.create( + cc_id=2, + representative=User.objects.create_user(username="test2", password="test2"), + representative_role="Test role 2", + signing_institution="Test institution 2", + type="member", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess2", email="testaccess2@example.com"), + ) + self.member_agreement_2 = MemberAgreement.objects.create( + signed_agreement=tmp, + study_site=StudySite.objects.get(short_name="test1"), + is_primary=False, + ) + tmp = SignedAgreement.objects.create( + cc_id=3, + representative=User.objects.create_user(username="test3", password="test3"), + representative_role="Test role 3", + signing_institution="Test institution 3", + type="data_affiliate", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess3", email="testaccess3@example.com"), + ) + self.data_affiliate_agreement_1 = DataAffiliateAgreement.objects.create( + signed_agreement=tmp, + study=Study.objects.create(short_name="test2", full_name="Test Study Site 2"), + anvil_upload_group=ManagedGroup.objects.create(name="testupload1", email="testupload1@example.com"), + is_primary=True, + ) + tmp = SignedAgreement.objects.create( + cc_id=4, + representative=User.objects.create_user(username="test4", password="test4"), + representative_role="Test role 4", + signing_institution="Test institution 4", + type="data_affiliate", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess4", email="testaccess4@example.com"), + ) + self.data_affiliate_agreement_2 = DataAffiliateAgreement.objects.create( + signed_agreement=tmp, + study=Study.objects.get(short_name="test2"), + anvil_upload_group=ManagedGroup.objects.create(name="testupload2", email="testupload2@example.com"), + is_primary=False, + ) + tmp = SignedAgreement.objects.create( + cc_id=5, + representative=User.objects.create_user(username="test5", password="test5"), + representative_role="Test role 5", + signing_institution="Test institution 5", + type="non_data_affiliate", + version=agreement_version, + anvil_access_group=ManagedGroup.objects.create(name="testaccess5", email="testaccess5@example.com"), + ) + self.non_data_affiliate_agreement = NonDataAffiliateAgreement.objects.create( + signed_agreement=tmp, + ) + + def test_is_primary_correctly_populated(self): + SignedAgreement = self.new_state.apps.get_model("cdsa", "SignedAgreement") + instance = SignedAgreement.objects.get(pk=self.member_agreement_1.signed_agreement.pk) + self.assertEqual(instance.is_primary, True) + instance = SignedAgreement.objects.get(pk=self.member_agreement_2.signed_agreement.pk) + self.assertEqual(instance.is_primary, False) + instance = SignedAgreement.objects.get(pk=self.data_affiliate_agreement_1.signed_agreement.pk) + self.assertEqual(instance.is_primary, True) + instance = SignedAgreement.objects.get(pk=self.data_affiliate_agreement_2.signed_agreement.pk) + self.assertEqual(instance.is_primary, False) + instance = SignedAgreement.objects.get(pk=self.non_data_affiliate_agreement.signed_agreement.pk) + self.assertTrue(instance.is_primary) From 3b75d343dd9e1b0c7038f4aca5ad13f91672d4aa Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Mon, 18 Mar 2024 10:54:34 -0700 Subject: [PATCH 06/17] Move is_primary field in factories --- primed/cdsa/tests/factories.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primed/cdsa/tests/factories.py b/primed/cdsa/tests/factories.py index 855a3ed4..6d7473d9 100644 --- a/primed/cdsa/tests/factories.py +++ b/primed/cdsa/tests/factories.py @@ -46,8 +46,6 @@ class SignedAgreementFactory(DjangoModelFactory): models.SignedAgreement.TYPE_CHOICES, getter=lambda c: c[0], ) - # Assume is_primary=True for now. - is_primary = True version = SubFactory(AgreementVersionFactory) date_signed = Faker("date") anvil_access_group = SubFactory( @@ -69,6 +67,7 @@ class MemberAgreementFactory(DjangoModelFactory): SignedAgreementFactory, type=models.SignedAgreement.MEMBER ) study_site = SubFactory(StudySiteFactory) + is_primary = True class Meta: model = models.MemberAgreement @@ -80,6 +79,7 @@ class DataAffiliateAgreementFactory(DjangoModelFactory): SignedAgreementFactory, type=models.SignedAgreement.DATA_AFFILIATE ) study = SubFactory(StudyFactory) + is_primary = True anvil_upload_group = SubFactory( ManagedGroupFactory, name=LazyAttribute( From 2fdc1fee4aabb97459a7259e0db64d46ea38c76a Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 19 Mar 2024 09:10:42 -0700 Subject: [PATCH 07/17] Updating remaining code for moving is_primary to agreement types --- add_cdsa_example_data.py | 12 +- primed/cdsa/audit/signed_agreement_audit.py | 7 +- primed/cdsa/audit/workspace_audit.py | 2 +- primed/cdsa/forms.py | 23 +- primed/cdsa/helpers.py | 2 +- primed/cdsa/models.py | 17 +- primed/cdsa/tables.py | 16 +- primed/cdsa/tests/test_audit.py | 112 ++----- primed/cdsa/tests/test_commands.py | 16 +- primed/cdsa/tests/test_forms.py | 90 +++--- primed/cdsa/tests/test_models.py | 68 ++-- primed/cdsa/tests/test_tables.py | 8 +- primed/cdsa/tests/test_views.py | 333 +++++++------------- 13 files changed, 270 insertions(+), 436 deletions(-) diff --git a/add_cdsa_example_data.py b/add_cdsa_example_data.py index b091fe94..c8e5eb26 100644 --- a/add_cdsa_example_data.py +++ b/add_cdsa_example_data.py @@ -51,7 +51,7 @@ signed_agreement__representative=UserFactory.create(name="Ken Rice"), signed_agreement__signing_institution="UW", signed_agreement__representative_role="Contact PI", - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__version=v10, study_site=StudySite.objects.get(short_name="CC"), ) @@ -64,7 +64,7 @@ signed_agreement__representative=UserFactory.create(name="Sally Adebamowo"), signed_agreement__signing_institution="UM", signed_agreement__representative_role="Contact PI", - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__version=v10, study_site=StudySite.objects.get(short_name="CARDINAL"), ) @@ -77,7 +77,7 @@ signed_agreement__representative=UserFactory.create(name="Bamidele Tayo"), signed_agreement__signing_institution="Loyola", signed_agreement__representative_role="Co-PI", - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__version=v10, study_site=StudySite.objects.get(short_name="CARDINAL"), ) @@ -90,7 +90,7 @@ signed_agreement__representative=UserFactory.create(name="Brackie Mitchell"), signed_agreement__signing_institution="UM", signed_agreement__representative_role="Co-I", - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__version=v11, study_site=StudySite.objects.get(short_name="CARDINAL"), ) @@ -128,7 +128,7 @@ signed_agreement__representative=UserFactory.create(name="Wendy"), signed_agreement__signing_institution="JHU", signed_agreement__representative_role="Field Center PI", - signed_agreement__is_primary=False, + is_primary=False, study=Study.objects.get(short_name="MESA"), signed_agreement__version=v10, ) @@ -141,7 +141,7 @@ signed_agreement__representative=UserFactory.create(name="Jerry"), signed_agreement__signing_institution="Lundquist", signed_agreement__representative_role="Analysis Center PI", - signed_agreement__is_primary=False, + is_primary=False, study=Study.objects.get(short_name="MESA"), signed_agreement__version=v10, ) diff --git a/primed/cdsa/audit/signed_agreement_audit.py b/primed/cdsa/audit/signed_agreement_audit.py index 929783c4..0eeee1f4 100644 --- a/primed/cdsa/audit/signed_agreement_audit.py +++ b/primed/cdsa/audit/signed_agreement_audit.py @@ -218,13 +218,13 @@ def _audit_component_agreement(self, signed_agreement): if hasattr(signed_agreement, "memberagreement"): # Member primary_qs = models.SignedAgreement.objects.filter( - is_primary=True, + memberagreement__is_primary=True, memberagreement__study_site=signed_agreement.memberagreement.study_site, ) elif hasattr(signed_agreement, "dataaffiliateagreement"): # Data affiliate primary_qs = models.SignedAgreement.objects.filter( - is_primary=True, + dataaffiliateagreement__is_primary=True, dataaffiliateagreement__study=signed_agreement.dataaffiliateagreement.study, ) elif hasattr(signed_agreement, "nondataaffiliateagreement"): @@ -320,7 +320,8 @@ def _audit_component_agreement(self, signed_agreement): ) # pragma: no cover def _audit_signed_agreement(self, signed_agreement): - if signed_agreement.is_primary: + agreement_type = signed_agreement.get_agreement_type() + if not hasattr(agreement_type, "is_primary") or agreement_type.is_primary: self._audit_primary_agreement(signed_agreement) else: self._audit_component_agreement(signed_agreement) diff --git a/primed/cdsa/audit/workspace_audit.py b/primed/cdsa/audit/workspace_audit.py index 2e329859..e81d8287 100644 --- a/primed/cdsa/audit/workspace_audit.py +++ b/primed/cdsa/audit/workspace_audit.py @@ -153,7 +153,7 @@ def _audit_workspace(self, workspace): child_group=self.anvil_cdsa_group, ).exists() primary_qs = models.DataAffiliateAgreement.objects.filter( - study=workspace.study, signed_agreement__is_primary=True + study=workspace.study, is_primary=True ) primary_exists = primary_qs.exists() diff --git a/primed/cdsa/forms.py b/primed/cdsa/forms.py index 993e2250..5ec7d116 100644 --- a/primed/cdsa/forms.py +++ b/primed/cdsa/forms.py @@ -22,12 +22,6 @@ class Meta: class SignedAgreementForm(Bootstrap5MediaFormMixin, forms.ModelForm): """Form for a SignedAgreement object.""" - is_primary = forms.TypedChoiceField( - coerce=lambda x: x == "True", - choices=((True, "Primary"), (False, "Component")), - widget=forms.RadioSelect, - label="Agreement type", - ) version = forms.ModelChoiceField( queryset=models.AgreementVersion.objects.filter(major_version__is_valid=True) ) @@ -41,7 +35,6 @@ class Meta: "signing_institution", "version", "date_signed", - "is_primary", ) widgets = { "representative": autocomplete.ModelSelect2( @@ -63,20 +56,36 @@ class Meta: class MemberAgreementForm(forms.ModelForm): + is_primary = forms.TypedChoiceField( + coerce=lambda x: x == "True", + choices=((True, "Primary"), (False, "Component")), + widget=forms.RadioSelect, + label="Agreement type", + ) + class Meta: model = models.MemberAgreement fields = ( "signed_agreement", "study_site", + "is_primary", ) class DataAffiliateAgreementForm(Bootstrap5MediaFormMixin, forms.ModelForm): + is_primary = forms.TypedChoiceField( + coerce=lambda x: x == "True", + choices=((True, "Primary"), (False, "Component")), + widget=forms.RadioSelect, + label="Agreement type", + ) + class Meta: model = models.DataAffiliateAgreement fields = ( "signed_agreement", "study", + "is_primary", "additional_limitations", "requires_study_review", ) diff --git a/primed/cdsa/helpers.py b/primed/cdsa/helpers.py index 5f613969..ffc6a416 100644 --- a/primed/cdsa/helpers.py +++ b/primed/cdsa/helpers.py @@ -13,7 +13,7 @@ def get_study_records_table(): """Return the queryset for study records.""" qs = models.DataAffiliateAgreement.objects.filter( signed_agreement__status=models.SignedAgreement.StatusChoices.ACTIVE, - signed_agreement__is_primary=True, + is_primary=True, ) return tables.StudyRecordsTable(qs) diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index 0c0f82bd..72c8274b 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -175,16 +175,15 @@ class SignedAgreement( def __str__(self): return "{}".format(self.cc_id) - def clean(self): - if self.type == self.NON_DATA_AFFILIATE and self.is_primary is False: - raise ValidationError( - "Non-data affiliate agreements must be primary agreements." - ) - @property def combined_type(self): combined_type = self.get_type_display() - if not self.is_primary: + if self.type == self.MEMBER and not self.get_agreement_type().is_primary: + combined_type = combined_type + " component" + elif ( + self.type == self.DATA_AFFILIATE + and not self.get_agreement_type().is_primary + ): combined_type = combined_type + " component" return combined_type @@ -302,7 +301,7 @@ def get_absolute_url(self): def clean(self): super().clean() # Checks for fields only allowed for primary agreements. - if hasattr(self, "signed_agreement") and not self.signed_agreement.is_primary: + if not self.is_primary: if self.additional_limitations: raise ValidationError( "Additional limitations are only allowed for primary agreements." @@ -371,7 +370,7 @@ def get_primary_cdsa(self): """Return the primary, valid CDSA associated with this workspace.""" cdsa = DataAffiliateAgreement.objects.get( study=self.study, - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__status=SignedAgreement.StatusChoices.ACTIVE, ) return cdsa diff --git a/primed/cdsa/tables.py b/primed/cdsa/tables.py index c5f9ab6d..d7e3c1a6 100644 --- a/primed/cdsa/tables.py +++ b/primed/cdsa/tables.py @@ -43,9 +43,7 @@ class SignedAgreementTable(tables.Table): verbose_name="Representative", ) representative_role = tables.Column(verbose_name="Role") - agreement_type = tables.Column( - accessor="combined_type", order_by=("type", "-is_primary") - ) + agreement_type = tables.Column(accessor="combined_type", order_by=("type")) number_accessors = tables.Column( verbose_name="Number of accessors", accessor="anvil_access_group__groupaccountmembership_set__count", @@ -79,7 +77,7 @@ class MemberAgreementTable(tables.Table): signed_agreement__cc_id = tables.Column(linkify=True) study_site = tables.Column(linkify=True) - signed_agreement__is_primary = BooleanIconColumn(verbose_name="Primary?") + is_primary = BooleanIconColumn(verbose_name="Primary?") signed_agreement__representative__name = tables.Column( linkify=lambda record: record.signed_agreement.representative.get_absolute_url(), verbose_name="Representative", @@ -96,7 +94,7 @@ class Meta: fields = ( "signed_agreement__cc_id", "study_site", - "signed_agreement__is_primary", + "is_primary", "signed_agreement__representative__name", "signed_agreement__representative_role", "signed_agreement__signing_institution", @@ -113,7 +111,7 @@ class DataAffiliateAgreementTable(tables.Table): signed_agreement__cc_id = tables.Column(linkify=True) study = tables.Column(linkify=True) - signed_agreement__is_primary = BooleanIconColumn(verbose_name="Primary?") + is_primary = BooleanIconColumn(verbose_name="Primary?") signed_agreement__representative__name = tables.Column( linkify=lambda record: record.signed_agreement.representative.get_absolute_url(), verbose_name="Representative", @@ -130,7 +128,7 @@ class Meta: fields = ( "signed_agreement__cc_id", "study", - "signed_agreement__is_primary", + "is_primary", "signed_agreement__representative__name", "signed_agreement__representative_role", "signed_agreement__signing_institution", @@ -178,9 +176,7 @@ class RepresentativeRecordsTable(tables.Table): representative__name = tables.Column(verbose_name="Representative") signing_group = tables.Column(accessor="pk", orderable=False) - agreement_type = tables.Column( - accessor="combined_type", order_by=("type", "-is_primary") - ) + agreement_type = tables.Column(accessor="combined_type", order_by=("type")) class Meta: model = models.SignedAgreement diff --git a/primed/cdsa/tests/test_audit.py b/primed/cdsa/tests/test_audit.py index 467ff57f..ef5fc61c 100644 --- a/primed/cdsa/tests/test_audit.py +++ b/primed/cdsa/tests/test_audit.py @@ -322,7 +322,7 @@ def test_member_component_has_primary_in_group(self): study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -344,7 +344,7 @@ def test_member_component_has_primary_not_in_group(self): study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -366,7 +366,7 @@ def test_member_component_inactive_has_primary_in_group(self): study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study_site=study_site, signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) @@ -390,7 +390,7 @@ def test_member_component_inactive_has_primary_not_in_group(self): study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study_site=study_site, signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) @@ -417,7 +417,7 @@ def test_member_component_has_primary_with_invalid_version_in_group(self): signed_agreement__version__major_version__is_valid=False, ) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -442,7 +442,7 @@ def test_member_component_has_primary_with_invalid_version_not_in_group(self): signed_agreement__version__major_version__is_valid=False, ) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -467,7 +467,7 @@ def test_member_component_has_inactive_primary_in_group(self): signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -492,7 +492,7 @@ def test_member_component_has_inactive_primary_not_in_group(self): signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -513,7 +513,7 @@ def test_member_component_no_primary_in_group(self): """Member component agreement, with valid version, with no primary, in CDSA group.""" study_site = StudySiteFactory.create() this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -534,7 +534,7 @@ def test_member_component_no_primary_not_in_group(self): """Member component agreement, with valid version, with no primary, not in CDSA group.""" study_site = StudySiteFactory.create() this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, study_site=study_site + is_primary=False, study_site=study_site ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -556,7 +556,7 @@ def test_member_component_invalid_version_has_primary_in_group(self): study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study_site=study_site, signed_agreement__version__major_version__is_valid=False, ) @@ -580,7 +580,7 @@ def test_member_component_invalid_version_has_primary_not_in_group(self): study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study_site=study_site, signed_agreement__version__major_version__is_valid=False, ) @@ -606,7 +606,7 @@ def test_member_component_invalid_version_has_primary_with_invalid_version_in_gr study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study_site=study_site, signed_agreement__version__major_version__is_valid=False, ) @@ -632,7 +632,7 @@ def test_member_component_invalid_version_has_primary_with_invalid_version_not_i study_site = StudySiteFactory.create() factories.MemberAgreementFactory.create(study_site=study_site) this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study_site=study_site, signed_agreement__version__major_version__is_valid=False, ) @@ -654,7 +654,7 @@ def test_member_component_invalid_version_has_primary_with_invalid_version_not_i def test_member_component_invalid_version_no_primary_in_group(self): """Member component agreement, with invalid version, with no primary, in CDSA group.""" this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__version__major_version__is_valid=False, ) # Add the signed agreement access group to the CDSA group. @@ -675,7 +675,7 @@ def test_member_component_invalid_version_no_primary_in_group(self): def test_member_component_invalid_version_no_primary_not_in_group(self): """Member component agreement, with invalid version, with no primary, not in CDSA group.""" this_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__version__major_version__is_valid=False, ) # # Add the signed agreement access group to the CDSA group. @@ -814,7 +814,7 @@ def test_data_affiliate_component_has_primary_in_group(self): study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -836,7 +836,7 @@ def test_data_affiliate_component_has_primary_not_in_group(self): study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -858,7 +858,7 @@ def test_data_affiliate_component_inactive_has_primary_in_group(self): study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study=study, signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) @@ -882,7 +882,7 @@ def test_data_affiliate_component_inactive_has_primary_not_in_group(self): study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study=study, signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) @@ -909,7 +909,7 @@ def test_data_affiliate_component_has_primary_with_invalid_version_in_group(self signed_agreement__version__major_version__is_valid=False, ) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -936,7 +936,7 @@ def test_data_affiliate_component_has_primary_with_invalid_version_not_in_group( signed_agreement__version__major_version__is_valid=False, ) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -961,7 +961,7 @@ def test_data_affiliate_component_has_inactive_primary_in_group(self): signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -986,7 +986,7 @@ def test_data_affiliate_component_has_inactive_primary_not_in_group(self): signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -1007,7 +1007,7 @@ def test_data_affiliate_component_no_primary_in_group(self): """Member component agreement, with valid version, with no primary, in CDSA group.""" study = StudyFactory.create() this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # Add the signed agreement access group to the CDSA group. GroupGroupMembershipFactory.create( @@ -1028,7 +1028,7 @@ def test_data_affiliate_component_no_primary_not_in_group(self): """Member component agreement, with valid version, with no primary, not in CDSA group.""" study = StudyFactory.create() this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, study=study + is_primary=False, study=study ) # # Add the signed agreement access group to the CDSA group. # GroupGroupMembershipFactory.create( @@ -1050,7 +1050,7 @@ def test_data_affiliate_component_invalid_version_has_primary_in_group(self): study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study=study, signed_agreement__version__major_version__is_valid=False, ) @@ -1074,7 +1074,7 @@ def test_data_affiliate_component_invalid_version_has_primary_not_in_group(self) study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study=study, signed_agreement__version__major_version__is_valid=False, ) @@ -1100,7 +1100,7 @@ def test_data_affiliate_component_invalid_version_has_primary_with_invalid_versi study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study=study, signed_agreement__version__major_version__is_valid=False, ) @@ -1126,7 +1126,7 @@ def test_data_affiliate_component_invalid_version_has_primary_with_invalid_versi study = StudyFactory.create() factories.DataAffiliateAgreementFactory.create(study=study) this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, study=study, signed_agreement__version__major_version__is_valid=False, ) @@ -1148,7 +1148,7 @@ def test_data_affiliate_component_invalid_version_has_primary_with_invalid_versi def test_data_affiliate_component_invalid_version_no_primary_in_group(self): """Member component agreement, with invalid version, with no primary, in CDSA group.""" this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__version__major_version__is_valid=False, ) # Add the signed agreement access group to the CDSA group. @@ -1169,7 +1169,7 @@ def test_data_affiliate_component_invalid_version_no_primary_in_group(self): def test_data_affiliate_component_invalid_version_no_primary_not_in_group(self): """Member component agreement, with invalid version, with no primary, not in CDSA group.""" this_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__version__major_version__is_valid=False, ) # # Add the signed agreement access group to the CDSA group. @@ -1303,46 +1303,6 @@ def test_non_data_affiliate_primary_valid_not_active_not_in_group(self): self.assertEqual(record.signed_agreement, this_agreement.signed_agreement) self.assertEqual(record.note, cdsa_audit.INACTIVE_AGREEMENT) - def test_non_data_affiliate_component_in_cdsa_group(self): - """Non data affiliate component agreement.""" - this_agreement = factories.NonDataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False - ) - # Add the signed agreement access group to the CDSA group. - GroupGroupMembershipFactory.create( - parent_group=self.cdsa_group, - child_group=this_agreement.signed_agreement.anvil_access_group, - ) - cdsa_audit = signed_agreement_audit.SignedAgreementAccessAudit() - cdsa_audit._audit_signed_agreement(this_agreement.signed_agreement) - self.assertEqual(len(cdsa_audit.verified), 0) - self.assertEqual(len(cdsa_audit.needs_action), 0) - self.assertEqual(len(cdsa_audit.errors), 1) - record = cdsa_audit.errors[0] - self.assertIsInstance(record, signed_agreement_audit.OtherError) - self.assertEqual(record.signed_agreement, this_agreement.signed_agreement) - self.assertEqual(record.note, cdsa_audit.ERROR_NON_DATA_AFFILIATE_COMPONENT) - - def test_non_data_affiliate_component_not_in_cdsa_group(self): - """Non data affiliate component agreement.""" - this_agreement = factories.NonDataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False - ) - # Do not add the signed agreement access group to the CDSA group. - # GroupGroupMembershipFactory.create( - # parent_group=self.cdsa_group, - # child_group=this_agreement.signed_agreement.anvil_access_group, - # ) - cdsa_audit = signed_agreement_audit.SignedAgreementAccessAudit() - cdsa_audit._audit_signed_agreement(this_agreement.signed_agreement) - self.assertEqual(len(cdsa_audit.verified), 0) - self.assertEqual(len(cdsa_audit.needs_action), 0) - self.assertEqual(len(cdsa_audit.errors), 1) - record = cdsa_audit.errors[0] - self.assertIsInstance(record, signed_agreement_audit.OtherError) - self.assertEqual(record.signed_agreement, this_agreement.signed_agreement) - self.assertEqual(record.note, cdsa_audit.ERROR_NON_DATA_AFFILIATE_COMPONENT) - class SignedAgreementAccessAuditTableTest(TestCase): """Tests for the `SignedAgreementAccessAuditTable` table.""" @@ -1804,9 +1764,7 @@ def test_primary_inactive_in_auth_domain(self): def test_component_agreement_not_in_auth_domain(self): study = StudyFactory.create() workspace = factories.CDSAWorkspaceFactory.create(study=study) - factories.DataAffiliateAgreementFactory.create( - study=study, signed_agreement__is_primary=False - ) + factories.DataAffiliateAgreementFactory.create(study=study, is_primary=False) # Do not add the CDSA group to the auth domain. # GroupGroupMembershipFactory.create( # parent_group=workspace.workspace.authorization_domains.first(), @@ -1826,9 +1784,7 @@ def test_component_agreement_not_in_auth_domain(self): def test_component_agreement_in_auth_domain(self): study = StudyFactory.create() workspace = factories.CDSAWorkspaceFactory.create(study=study) - factories.DataAffiliateAgreementFactory.create( - study=study, signed_agreement__is_primary=False - ) + factories.DataAffiliateAgreementFactory.create(study=study, is_primary=False) # Add the CDSA group to the auth domain. GroupGroupMembershipFactory.create( parent_group=workspace.workspace.authorization_domains.first(), diff --git a/primed/cdsa/tests/test_commands.py b/primed/cdsa/tests/test_commands.py index 916bd62f..fbef817c 100644 --- a/primed/cdsa/tests/test_commands.py +++ b/primed/cdsa/tests/test_commands.py @@ -68,9 +68,7 @@ def test_study_records_zero(self): self.assertEqual(len(lines), 1) def test_study_records_one(self): - factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True - ) + factories.DataAffiliateAgreementFactory.create(is_primary=True) out = StringIO() call_command("cdsa_records", "--outdir", self.outdir, "--no-color", stdout=out) with open(os.path.join(self.outdir, "study_records.tsv")) as f: @@ -135,7 +133,7 @@ def test_command_output_no_records(self): def test_command_run_audit_one_agreement_verified(self): """Test command output with one verified instance.""" - factories.MemberAgreementFactory.create(signed_agreement__is_primary=False) + factories.MemberAgreementFactory.create(is_primary=False) out = StringIO() call_command("run_cdsa_audit", "--no-color", stdout=out) expected_output = ( @@ -174,9 +172,7 @@ def test_command_run_audit_one_agreement_needs_action(self): def test_command_run_audit_one_agreement_error(self): """Test command output with one error instance.""" - agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False - ) + agreement = factories.MemberAgreementFactory.create(is_primary=False) GroupGroupMembershipFactory.create( parent_group=self.cdsa_group, child_group=agreement.signed_agreement.anvil_access_group, @@ -197,7 +193,7 @@ def test_command_run_audit_one_agreement_error(self): def test_command_run_audit_one_agreement_verified_email(self): """No email is sent when there are no errors.""" - factories.MemberAgreementFactory.create(signed_agreement__is_primary=False) + factories.MemberAgreementFactory.create(is_primary=False) out = StringIO() call_command( "run_cdsa_audit", "--no-color", email="test@example.com", stdout=out @@ -233,9 +229,7 @@ def test_command_run_audit_one_agreement_needs_action_email(self): def test_command_run_audit_one_agreement_error_email(self): """Test command output with one error instance.""" - agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False - ) + agreement = factories.MemberAgreementFactory.create(is_primary=False) GroupGroupMembershipFactory.create( parent_group=self.cdsa_group, child_group=agreement.signed_agreement.anvil_access_group, diff --git a/primed/cdsa/tests/test_forms.py b/primed/cdsa/tests/test_forms.py index d326207c..3ea774f2 100644 --- a/primed/cdsa/tests/test_forms.py +++ b/primed/cdsa/tests/test_forms.py @@ -36,7 +36,6 @@ def test_valid(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertTrue(form.is_valid()) @@ -50,7 +49,6 @@ def test_missing_representative(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -68,7 +66,6 @@ def test_missing_cc_id(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -86,7 +83,6 @@ def test_missing_representative_role(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -104,7 +100,6 @@ def test_missing_signing_institution(self): # "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -122,7 +117,6 @@ def test_missing_version(self): "signing_institution": "Test insitution", # "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -140,7 +134,6 @@ def test_missing_date_signed(self): "signing_institution": "Test insitution", "version": self.agreement_version, # "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -149,24 +142,6 @@ def test_missing_date_signed(self): self.assertEqual(len(form.errors["date_signed"]), 1) self.assertIn("required", form.errors["date_signed"][0]) - def test_missing_is_primary(self): - """Form is invalid when missing representative_role.""" - form_data = { - "cc_id": 1234, - "representative": self.representative, - "representative_role": "Test role", - "signing_institution": "Test insitution", - "version": self.agreement_version, - "date_signed": "2023-01-01", - # "is_primary": True, - } - form = self.form_class(data=form_data) - self.assertFalse(form.is_valid()) - self.assertEqual(len(form.errors), 1) - self.assertIn("is_primary", form.errors) - self.assertEqual(len(form.errors["is_primary"]), 1) - self.assertIn("required", form.errors["is_primary"][0]) - def test_invalid_cc_id_zero(self): """Form is invalid when cc_id is zero.""" form_data = { @@ -176,7 +151,6 @@ def test_invalid_cc_id_zero(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -195,7 +169,6 @@ def test_invalid_duplicate_object(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -214,7 +187,6 @@ def test_invalid_version(self): "signing_institution": "Test insitution", "version": self.agreement_version, "date_signed": "2023-01-01", - "is_primary": True, } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) @@ -282,6 +254,7 @@ def test_valid(self): """Form is valid with necessary input.""" form_data = { "signed_agreement": self.signed_agreement, + "is_primary": True, "study_site": self.study_site, } form = self.form_class(data=form_data) @@ -291,6 +264,7 @@ def test_missing_signed_agreement(self): """Form is invalid when missing signed_agreement.""" form_data = { # "signed_agreement": self.signed_agreement, + "is_primary": True, "study_site": self.study_site, } form = self.form_class(data=form_data) @@ -300,10 +274,25 @@ def test_missing_signed_agreement(self): self.assertEqual(len(form.errors["signed_agreement"]), 1) self.assertIn("required", form.errors["signed_agreement"][0]) + def test_missing_is_primary(self): + """Form is invalid when missing study_site.""" + form_data = { + "signed_agreement": self.signed_agreement, + # "is_primary": True, + "study_site": self.study_site, + } + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertEqual(len(form.errors), 1) + self.assertIn("is_primary", form.errors) + self.assertEqual(len(form.errors["is_primary"]), 1) + self.assertIn("required", form.errors["is_primary"][0]) + def test_missing_study_site(self): """Form is invalid when missing study_site.""" form_data = { "signed_agreement": self.signed_agreement, + "is_primary": True, # "study_site": self.study_site, } form = self.form_class(data=form_data) @@ -318,6 +307,7 @@ def test_invalid_signed_agreement_already_has_member_agreement(self): obj = factories.MemberAgreementFactory.create() form_data = { "signed_agreement": obj.signed_agreement, + "is_primary": True, "study_site": self.study_site, } form = self.form_class(data=form_data) @@ -333,6 +323,7 @@ def test_invalid_signed_agreement_wrong_type(self): ) form_data = { "signed_agreement": obj, + "is_primary": True, "study_site": self.study_site, } form = self.form_class(data=form_data) @@ -358,6 +349,7 @@ def test_valid(self): """Form is valid with necessary input.""" form_data = { "signed_agreement": self.signed_agreement, + "is_primary": True, "study": self.study, } form = self.form_class(data=form_data) @@ -367,6 +359,7 @@ def test_missing_signed_agreement(self): """Form is invalid when missing signed_agreement.""" form_data = { # "signed_agreement": self.signed_agreement, + "is_primary": True, "study": self.study, } form = self.form_class(data=form_data) @@ -376,10 +369,25 @@ def test_missing_signed_agreement(self): self.assertEqual(len(form.errors["signed_agreement"]), 1) self.assertIn("required", form.errors["signed_agreement"][0]) + def test_missing_is_primary(self): + """Form is invalid when missing study_site.""" + form_data = { + "signed_agreement": self.signed_agreement, + # "is_primary": True, + "study": self.study, + } + form = self.form_class(data=form_data) + self.assertFalse(form.is_valid()) + self.assertEqual(len(form.errors), 1) + self.assertIn("is_primary", form.errors) + self.assertEqual(len(form.errors["is_primary"]), 1) + self.assertIn("required", form.errors["is_primary"][0]) + def test_missing_study(self): """Form is invalid when missing study.""" form_data = { "signed_agreement": self.signed_agreement, + "is_primary": True, # "study": self.study, } form = self.form_class(data=form_data) @@ -394,6 +402,7 @@ def test_invalid_signed_agreement_already_has_agreement_type(self): obj = factories.DataAffiliateAgreementFactory.create() form_data = { "signed_agreement": obj.signed_agreement, + "is_primary": True, "study": self.study, } form = self.form_class(data=form_data) @@ -409,6 +418,7 @@ def test_invalid_signed_agreement_wrong_type(self): ) form_data = { "signed_agreement": obj, + "is_primary": True, "study": self.study, } form = self.form_class(data=form_data) @@ -419,12 +429,10 @@ def test_invalid_signed_agreement_wrong_type(self): def test_valid_primary_with_additional_limitations(self): """Form is valid with necessary input.""" - signed_agreement = factories.SignedAgreementFactory.create( - type=models.SignedAgreement.DATA_AFFILIATE, is_primary=True - ) form_data = { - "signed_agreement": signed_agreement, + "signed_agreement": self.signed_agreement, "study": self.study, + "is_primary": True, "additional_limitations": "test limitations", } form = self.form_class(data=form_data) @@ -432,12 +440,10 @@ def test_valid_primary_with_additional_limitations(self): def test_invalid_component_with_additional_limitations(self): """Form is valid with necessary input.""" - signed_agreement = factories.SignedAgreementFactory.create( - type=models.SignedAgreement.DATA_AFFILIATE, is_primary=False - ) form_data = { - "signed_agreement": signed_agreement, + "signed_agreement": self.signed_agreement, "study": self.study, + "is_primary": False, "additional_limitations": "test limitations", } form = self.form_class(data=form_data) @@ -448,12 +454,10 @@ def test_invalid_component_with_additional_limitations(self): def test_valid_primary_with_requires_study_review_true(self): """Form is valid with necessary input.""" - signed_agreement = factories.SignedAgreementFactory.create( - type=models.SignedAgreement.DATA_AFFILIATE, is_primary=True - ) form_data = { - "signed_agreement": signed_agreement, + "signed_agreement": self.signed_agreement, "study": self.study, + "is_primary": True, "requires_study_review": True, } form = self.form_class(data=form_data) @@ -461,12 +465,10 @@ def test_valid_primary_with_requires_study_review_true(self): def test_invalid_component_with_requires_study_review_true(self): """Form is valid with necessary input.""" - signed_agreement = factories.SignedAgreementFactory.create( - type=models.SignedAgreement.DATA_AFFILIATE, is_primary=False - ) form_data = { - "signed_agreement": signed_agreement, + "signed_agreement": self.signed_agreement, "study": self.study, + "is_primary": False, "requires_study_review": True, } form = self.form_class(data=form_data) diff --git a/primed/cdsa/tests/test_models.py b/primed/cdsa/tests/test_models.py index f772ca74..6b7dbf78 100644 --- a/primed/cdsa/tests/test_models.py +++ b/primed/cdsa/tests/test_models.py @@ -8,7 +8,7 @@ ManagedGroupFactory, WorkspaceFactory, ) -from django.core.exceptions import NON_FIELD_ERRORS, ValidationError +from django.core.exceptions import ValidationError from django.db.models import ProtectedError from django.db.utils import IntegrityError from django.test import TestCase, override_settings @@ -179,7 +179,6 @@ def test_model_saving(self): representative_role="foo", signing_institution="bar", type=models.SignedAgreement.MEMBER, - is_primary=True, version=agreement_version, anvil_access_group=group, ) @@ -318,22 +317,14 @@ def test_status_field(self): def test_get_combined_type(self): obj = factories.MemberAgreementFactory() self.assertEqual(obj.signed_agreement.combined_type, "Member") - obj = factories.MemberAgreementFactory(signed_agreement__is_primary=False) + obj = factories.MemberAgreementFactory(is_primary=False) self.assertEqual(obj.signed_agreement.combined_type, "Member component") obj = factories.DataAffiliateAgreementFactory() self.assertEqual(obj.signed_agreement.combined_type, "Data affiliate") - obj = factories.DataAffiliateAgreementFactory( - signed_agreement__is_primary=False - ) + obj = factories.DataAffiliateAgreementFactory(is_primary=False) self.assertEqual(obj.signed_agreement.combined_type, "Data affiliate component") obj = factories.NonDataAffiliateAgreementFactory() self.assertEqual(obj.signed_agreement.combined_type, "Non-data affiliate") - obj = factories.NonDataAffiliateAgreementFactory( - signed_agreement__is_primary=False - ) - self.assertEqual( - obj.signed_agreement.combined_type, "Non-data affiliate component" - ) def test_get_agreement_type(self): obj = factories.MemberAgreementFactory() @@ -351,25 +342,6 @@ def test_get_agreement_group(self): obj = factories.NonDataAffiliateAgreementFactory() self.assertEqual(obj.signed_agreement.agreement_group, obj.affiliation) - def test_clean_non_data_affiliate_is_primary_false(self): - """ValidationError is raised when is_primary is False for a non-data affiliate.""" - user = UserFactory.create() - group = ManagedGroupFactory.create() - agreement_version = factories.AgreementVersionFactory.create() - instance = factories.SignedAgreementFactory.build( - representative=user, - anvil_access_group=group, - version=agreement_version, - type=models.SignedAgreement.NON_DATA_AFFILIATE, - is_primary=False, - ) - with self.assertRaises(ValidationError) as e: - instance.full_clean() - self.assertEqual(len(e.exception.message_dict), 1) - self.assertIn(NON_FIELD_ERRORS, e.exception.message_dict) - self.assertEqual(len(e.exception.message_dict[NON_FIELD_ERRORS]), 1) - self.assertIn("primary", e.exception.message_dict[NON_FIELD_ERRORS][0]) - def test_is_in_cdsa_group(self): """is_in_cdsa_group works as expected.""" obj = factories.SignedAgreementFactory.create() @@ -414,10 +386,18 @@ def test_model_saving(self): instance = models.MemberAgreement( signed_agreement=signed_agreement, study_site=study_site, + is_primary=True, ) instance.save() self.assertIsInstance(instance, models.MemberAgreement) + def test_is_primary(self): + """Creation using the model constructor and .save() works.""" + instance = factories.MemberAgreementFactory.create(is_primary=True) + self.assertEqual(instance.is_primary, True) + instance = factories.MemberAgreementFactory.create(is_primary=False) + self.assertEqual(instance.is_primary, False) + def test_clean_incorrect_type(self): signed_agreement = factories.SignedAgreementFactory.create( type=models.SignedAgreement.DATA_AFFILIATE @@ -496,10 +476,18 @@ def test_model_saving(self): signed_agreement=signed_agreement, study=study, anvil_upload_group=upload_group, + is_primary=True, ) instance.save() self.assertIsInstance(instance, models.DataAffiliateAgreement) + def test_is_primary(self): + """Creation using the model constructor and .save() works.""" + instance = factories.DataAffiliateAgreementFactory.create(is_primary=True) + self.assertEqual(instance.is_primary, True) + instance = factories.DataAffiliateAgreementFactory.create(is_primary=False) + self.assertEqual(instance.is_primary, False) + def test_clean_incorrect_type(self): signed_agreement = factories.SignedAgreementFactory.create( type=models.SignedAgreement.MEMBER @@ -522,14 +510,14 @@ def test_clean_incorrect_type(self): def test_clean_additional_limitations_primary(self): instance = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, additional_limitations="foo bar", ) instance.full_clean() def test_clean_additional_limitations_not_primary(self): instance = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, additional_limitations="foo bar", ) with self.assertRaises(ValidationError) as e: @@ -581,7 +569,7 @@ def test_requires_study_review_primary(self): def test_requires_study_review_not_primary(self): """ValidationError when trying to set requires_study_review=True for components.""" instance = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, requires_study_review=True, ) with self.assertRaises(ValidationError) as e: @@ -713,7 +701,7 @@ def test_get_primary_cdsa(self): """get_primary_cdsa returns the primary valid CDSA for the study.""" instance = factories.CDSAWorkspaceFactory.create() agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__status=models.SignedAgreement.StatusChoices.ACTIVE, study=instance.study, ) @@ -722,7 +710,7 @@ def test_get_primary_cdsa(self): def test_get_primary_cdsa_not_primary(self): instance = factories.CDSAWorkspaceFactory.create() factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False, + is_primary=False, signed_agreement__status=models.SignedAgreement.StatusChoices.ACTIVE, study=instance.study, ) @@ -732,7 +720,7 @@ def test_get_primary_cdsa_not_primary(self): def test_get_primary_cdsa_not_active(self): instance = factories.CDSAWorkspaceFactory.create() factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__status=models.SignedAgreement.StatusChoices.LAPSED, study=instance.study, ) @@ -742,7 +730,7 @@ def test_get_primary_cdsa_not_active(self): def test_get_primary_cdsa_different_study(self): instance = factories.CDSAWorkspaceFactory.create() factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__status=models.SignedAgreement.StatusChoices.ACTIVE, # study=instance.study, ) @@ -753,12 +741,12 @@ def test_get_primary_cdsa_multiple_agreements(self): """get_primary_cdsa returns the primary valid CDSA for the study.""" instance = factories.CDSAWorkspaceFactory.create() factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__status=models.SignedAgreement.StatusChoices.ACTIVE, study=instance.study, ) factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, signed_agreement__status=models.SignedAgreement.StatusChoices.ACTIVE, study=instance.study, ) diff --git a/primed/cdsa/tests/test_tables.py b/primed/cdsa/tests/test_tables.py index c7c8c9de..f239a383 100644 --- a/primed/cdsa/tests/test_tables.py +++ b/primed/cdsa/tests/test_tables.py @@ -339,15 +339,11 @@ def test_row_count_with_two_agreements_multiple_members(self): self.assertEqual(len(table.rows), 5) def test_includes_components(self): - agreement_1 = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement_1 = factories.MemberAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create( group__signedagreement=agreement_1.signed_agreement ) - agreement_2 = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False - ) + agreement_2 = factories.MemberAgreementFactory.create(is_primary=False) GroupAccountMembershipFactory.create( group__signedagreement=agreement_2.signed_agreement ) diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 2b16442b..2699ed64 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -1494,7 +1494,7 @@ def test_can_create_object(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1511,7 +1511,6 @@ def test_can_create_object(self): self.assertEqual(new_agreement.representative_role, "Test role") self.assertEqual(new_agreement.signing_institution, "Test institution") self.assertEqual(new_agreement.date_signed, date.fromisoformat("2023-01-01")) - self.assertEqual(new_agreement.is_primary, True) # Type was set correctly. self.assertEqual(new_agreement.type, new_agreement.MEMBER) # AnVIL group was set correctly. @@ -1527,6 +1526,7 @@ def test_can_create_object(self): new_agreement_type = models.MemberAgreement.objects.latest("pk") self.assertEqual(new_agreement.memberagreement, new_agreement_type) self.assertEqual(new_agreement_type.study_site, study_site) + self.assertEqual(new_agreement_type.is_primary, True) def test_redirect_url(self): """Redirects to successful url.""" @@ -1558,7 +1558,7 @@ def test_redirect_url(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1599,7 +1599,7 @@ def test_success_message(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1626,7 +1626,7 @@ def test_error_missing_cc_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1662,7 +1662,7 @@ def test_invalid_cc_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1696,7 +1696,7 @@ def test_error_missing_representative(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1730,7 +1730,7 @@ def test_error_invalid_representative(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1766,7 +1766,7 @@ def test_error_missing_representative_role(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1802,7 +1802,7 @@ def test_error_missing_signing_institution(self): # "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1837,7 +1837,7 @@ def test_error_missing_version(self): "signing_institution": "Test institution", # "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1872,7 +1872,7 @@ def test_error_invalid_version(self): "signing_institution": "Test institution", "version": 999, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1908,7 +1908,7 @@ def test_error_missing_date_signed(self): "signing_institution": "Test institution", "version": agreement_version.pk, # "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1944,7 +1944,7 @@ def test_error_missing_is_primary(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - # "is_primary": True, + # "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1959,11 +1959,14 @@ def test_error_missing_is_primary(self): # Form has errors in the correct field. self.assertIn("form", response.context_data) form = response.context_data["form"] - self.assertFalse(form.is_valid()) - self.assertEqual(len(form.errors), 1) - self.assertIn("is_primary", form.errors) - self.assertEqual(len(form.errors["is_primary"]), 1) - self.assertIn("required", form.errors["is_primary"][0]) + self.assertTrue(form.is_valid()) + formset = response.context_data["formset"] + self.assertFalse(formset.is_valid()) + self.assertFalse(formset.forms[0].is_valid()) + self.assertEqual(len(formset.forms[0].errors), 1) + self.assertIn("is_primary", formset.forms[0].errors) + self.assertEqual(len(formset.forms[0].errors["is_primary"]), 1) + self.assertIn("required", formset.forms[0].errors["is_primary"][0]) def test_error_missing_memberagreement_study_site(self): """Form shows an error when study_site is missing.""" @@ -1979,7 +1982,7 @@ def test_error_missing_memberagreement_study_site(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -1991,10 +1994,10 @@ def test_error_missing_memberagreement_study_site(self): # No new objects were created. self.assertEqual(models.SignedAgreement.objects.count(), 0) self.assertEqual(models.MemberAgreement.objects.count(), 0) - # Form has errors in the correct field. self.assertIn("form", response.context_data) form = response.context_data["form"] self.assertTrue(form.is_valid()) + # Formset has errors in the correct field. formset = response.context_data["formset"] self.assertFalse(formset.is_valid()) self.assertFalse(formset.forms[0].is_valid()) @@ -2017,7 +2020,7 @@ def test_error_invalid_memberagreement_study_site(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2055,7 +2058,7 @@ def test_error_duplicate_project_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2116,7 +2119,7 @@ def test_creates_anvil_access_group(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2167,7 +2170,7 @@ def test_creates_anvil_groups_different_setting_access_group_prefix(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2215,7 +2218,7 @@ def test_creates_anvil_groups_different_setting_cc_admins_group_name(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2253,7 +2256,7 @@ def test_manage_group_create_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2293,7 +2296,7 @@ def test_managed_group_already_exists_in_app(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2343,7 +2346,7 @@ def test_admin_group_membership_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2713,7 +2716,7 @@ def test_can_create_object(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2730,7 +2733,6 @@ def test_can_create_object(self): self.assertEqual(new_agreement.representative_role, "Test role") self.assertEqual(new_agreement.signing_institution, "Test institution") self.assertEqual(new_agreement.date_signed, date.fromisoformat("2023-01-01")) - self.assertEqual(new_agreement.is_primary, True) # Type was set correctly. self.assertEqual(new_agreement.type, new_agreement.DATA_AFFILIATE) # AnVIL group was set correctly. @@ -2745,6 +2747,7 @@ def test_can_create_object(self): self.assertEqual(models.DataAffiliateAgreement.objects.count(), 1) new_agreement_type = models.DataAffiliateAgreement.objects.latest("pk") self.assertEqual(new_agreement.dataaffiliateagreement, new_agreement_type) + self.assertEqual(new_agreement_type.is_primary, True) self.assertEqual(new_agreement_type.study, study) self.assertIsInstance(new_agreement_type.anvil_upload_group, ManagedGroup) self.assertEqual( @@ -2794,7 +2797,7 @@ def test_redirect_url(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2848,7 +2851,7 @@ def test_success_message(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2905,7 +2908,7 @@ def test_can_create_primary_with_requires_study_review(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2935,7 +2938,7 @@ def test_cannot_create_component_with_requires_study_review(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": False, + "agreementtype-0-is_primary": False, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -2946,14 +2949,18 @@ def test_cannot_create_component_with_requires_study_review(self): ) self.assertEqual(response.status_code, 200) self.assertIn("form", response.context) - self.assertFalse(response.context["form"].is_valid()) - form = response.context["form"] - self.assertEqual(len(form.errors), 1) - self.assertIn(NON_FIELD_ERRORS, form.errors) - self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) + form = response.context_data["form"] + self.assertTrue(form.is_valid()) + # Formset has errors in the correct field. + formset = response.context_data["formset"] + self.assertFalse(formset.is_valid()) + self.assertFalse(formset.forms[0].is_valid()) + self.assertEqual(len(formset.forms[0].errors), 1) + self.assertIn(NON_FIELD_ERRORS, formset.forms[0].errors) + self.assertEqual(len(formset.forms[0].errors[NON_FIELD_ERRORS]), 1) self.assertIn( "can only be True for primary", - form.errors[NON_FIELD_ERRORS][0], + formset.forms[0].errors[NON_FIELD_ERRORS][0], ) def test_can_create_primary_with_additional_limitations(self): @@ -2999,7 +3006,7 @@ def test_can_create_primary_with_additional_limitations(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3029,7 +3036,7 @@ def test_cannot_create_component_with_additional_limitations(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": False, + "agreementtype-0-is_primary": False, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3040,14 +3047,18 @@ def test_cannot_create_component_with_additional_limitations(self): ) self.assertEqual(response.status_code, 200) self.assertIn("form", response.context) - self.assertFalse(response.context["form"].is_valid()) - form = response.context["form"] - self.assertEqual(len(form.errors), 1) - self.assertIn(NON_FIELD_ERRORS, form.errors) - self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) + form = response.context_data["form"] + self.assertTrue(form.is_valid()) + # Formset has errors in the correct field. + formset = response.context_data["formset"] + self.assertFalse(formset.is_valid()) + self.assertFalse(formset.forms[0].is_valid()) + self.assertEqual(len(formset.forms[0].errors), 1) + self.assertIn(NON_FIELD_ERRORS, formset.forms[0].errors) + self.assertEqual(len(formset.forms[0].errors[NON_FIELD_ERRORS]), 1) self.assertIn( "only allowed for primary", - form.errors[NON_FIELD_ERRORS][0], + formset.forms[0].errors[NON_FIELD_ERRORS][0], ) def test_error_missing_cc_id(self): @@ -3065,7 +3076,7 @@ def test_error_missing_cc_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3101,7 +3112,7 @@ def test_invalid_cc_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3135,7 +3146,7 @@ def test_error_missing_representative(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3169,7 +3180,7 @@ def test_error_invalid_representative(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3205,7 +3216,7 @@ def test_error_missing_representative_role(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3241,7 +3252,7 @@ def test_error_missing_signing_institution(self): # "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3276,7 +3287,7 @@ def test_error_missing_version(self): "signing_institution": "Test institution", # "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3311,7 +3322,7 @@ def test_error_invalid_version(self): "signing_institution": "Test institution", "version": 9999, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3347,7 +3358,7 @@ def test_error_missing_date_signed(self): "signing_institution": "Test institution", "version": agreement_version.pk, # "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3383,7 +3394,7 @@ def test_error_missing_is_primary(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - # "is_primary": True, + # "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3398,11 +3409,14 @@ def test_error_missing_is_primary(self): # Form has errors in the correct field. self.assertIn("form", response.context_data) form = response.context_data["form"] - self.assertFalse(form.is_valid()) - self.assertEqual(len(form.errors), 1) - self.assertIn("is_primary", form.errors) - self.assertEqual(len(form.errors["is_primary"]), 1) - self.assertIn("required", form.errors["is_primary"][0]) + self.assertTrue(form.is_valid()) + formset = response.context_data["formset"] + self.assertFalse(formset.is_valid()) + self.assertFalse(formset.forms[0].is_valid()) + self.assertEqual(len(formset.forms[0].errors), 1) + self.assertIn("is_primary", formset.forms[0].errors) + self.assertEqual(len(formset.forms[0].errors["is_primary"]), 1) + self.assertIn("required", formset.forms[0].errors["is_primary"][0]) def test_error_missing_study(self): """Form shows an error when study is missing.""" @@ -3418,7 +3432,7 @@ def test_error_missing_study(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3456,7 +3470,7 @@ def test_error_invalid_memberagreement_study_site(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3494,7 +3508,7 @@ def test_error_duplicate_project_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3567,7 +3581,7 @@ def test_creates_anvil_groups(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3642,7 +3656,7 @@ def test_creates_anvil_access_group_different_setting(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3709,7 +3723,7 @@ def test_creates_anvil_groups_different_setting_cc_admins_group_name(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3753,7 +3767,7 @@ def test_access_group_create_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3813,7 +3827,7 @@ def test_upload_group_create_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3853,7 +3867,7 @@ def test_access_group_already_exists_in_app(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3890,7 +3904,7 @@ def test_upload_group_already_exists_in_app(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -3954,7 +3968,7 @@ def test_admin_group_membership_access_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4020,7 +4034,7 @@ def test_admin_group_membership_upload_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, + "agreementtype-0-is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4198,7 +4212,7 @@ def test_change_status_button_user_has_view_perm(self): def test_response_includes_additional_limitations(self): """Response includes a link to the study detail page.""" instance = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, additional_limitations="Test limitations for this data affiliate agreement", ) self.client.force_login(self.user) @@ -4211,7 +4225,7 @@ def test_response_includes_additional_limitations(self): def test_response_with_no_additional_limitations(self): """Response includes a link to the study detail page.""" instance = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, additional_limitations="", ) self.client.force_login(self.user) @@ -4406,7 +4420,6 @@ def test_can_create_object(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4423,7 +4436,6 @@ def test_can_create_object(self): self.assertEqual(new_agreement.representative_role, "Test role") self.assertEqual(new_agreement.signing_institution, "Test institution") self.assertEqual(new_agreement.date_signed, date.fromisoformat("2023-01-01")) - self.assertEqual(new_agreement.is_primary, True) # Type was set correctly. self.assertEqual(new_agreement.type, new_agreement.NON_DATA_AFFILIATE) # AnVIL group was set correctly. @@ -4469,7 +4481,6 @@ def test_redirect_url(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4509,7 +4520,6 @@ def test_success_message(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4537,7 +4547,6 @@ def test_error_missing_cc_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4572,7 +4581,6 @@ def test_invalid_cc_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4605,7 +4613,6 @@ def test_error_missing_representative(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4638,7 +4645,6 @@ def test_error_invalid_representative(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4673,7 +4679,6 @@ def test_error_missing_representative_role(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4708,7 +4713,6 @@ def test_error_missing_signing_institution(self): # "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4742,7 +4746,6 @@ def test_error_missing_version(self): "signing_institution": "Test institution", # "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4776,7 +4779,6 @@ def test_error_invalid_version(self): "signing_institution": "Test institution", "version": 999, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4811,7 +4813,6 @@ def test_error_missing_date_signed(self): "signing_institution": "Test institution", "version": agreement_version.pk, # "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4832,41 +4833,6 @@ def test_error_missing_date_signed(self): self.assertEqual(len(form.errors["date_signed"]), 1) self.assertIn("required", form.errors["date_signed"][0]) - def test_error_missing_is_primary(self): - """Form shows an error when representative is missing.""" - self.client.force_login(self.user) - representative = UserFactory.create() - agreement_version = factories.AgreementVersionFactory.create() - response = self.client.post( - self.get_url(), - { - "cc_id": 1, - "representative": representative.pk, - "representative_role": "Test role", - "signing_institution": "Test institution", - "version": agreement_version.pk, - "date_signed": "2023-01-01", - # "is_primary": True, - "agreementtype-TOTAL_FORMS": 1, - "agreementtype-INITIAL_FORMS": 0, - "agreementtype-MIN_NUM_FORMS": 1, - "agreementtype-MAX_NUM_FORMS": 1, - "agreementtype-0-affiliation": "Foo Bar", - }, - ) - self.assertEqual(response.status_code, 200) - # No new objects were created. - self.assertEqual(models.SignedAgreement.objects.count(), 0) - self.assertEqual(models.MemberAgreement.objects.count(), 0) - # Form has errors in the correct field. - self.assertIn("form", response.context_data) - form = response.context_data["form"] - self.assertFalse(form.is_valid()) - self.assertEqual(len(form.errors), 1) - self.assertIn("is_primary", form.errors) - self.assertEqual(len(form.errors["is_primary"]), 1) - self.assertIn("required", form.errors["is_primary"][0]) - def test_error_missing_nondataaffiliateagreement_affiliation(self): """Form shows an error when study_site is missing.""" self.client.force_login(self.user) @@ -4881,7 +4847,6 @@ def test_error_missing_nondataaffiliateagreement_affiliation(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4920,7 +4885,6 @@ def test_error_duplicate_project_id(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -4944,40 +4908,6 @@ def test_error_duplicate_project_id(self): self.assertEqual(len(form.errors["cc_id"]), 1) self.assertIn("already exists", form.errors["cc_id"][0]) - def test_error_is_primary_false(self): - """Form shows an error when trying to create a duplicate dbgap_phs.""" - self.client.force_login(self.user) - representative = UserFactory.create() - agreement_version = factories.AgreementVersionFactory.create() - response = self.client.post( - self.get_url(), - { - "cc_id": 1, - "representative": representative.pk, - "representative_role": "Test role", - "signing_institution": "Test institution", - "version": agreement_version.pk, - "date_signed": "2023-01-01", - "is_primary": False, - "agreementtype-TOTAL_FORMS": 1, - "agreementtype-INITIAL_FORMS": 0, - "agreementtype-MIN_NUM_FORMS": 1, - "agreementtype-MAX_NUM_FORMS": 1, - "agreementtype-0-affiliation": "Foo Bar", - }, - ) - self.assertEqual(response.status_code, 200) - # No new objects were created. - self.assertEqual(models.SignedAgreement.objects.count(), 0) - self.assertEqual(models.NonDataAffiliateAgreement.objects.count(), 0) - # Form has errors in the correct field. - form = response.context_data["form"] - 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("primary", form.errors[NON_FIELD_ERRORS][0]) - def test_post_blank_data(self): """Posting blank data does not create an object.""" self.client.force_login(self.user) @@ -5014,7 +4944,6 @@ def test_creates_anvil_access_group(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -5064,7 +4993,6 @@ def test_creates_anvil_groups_different_setting(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -5111,7 +5039,6 @@ def test_creates_anvil_groups_different_setting_cc_admins_group_name(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -5148,7 +5075,6 @@ def test_manage_group_create_api_error(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -5187,7 +5113,6 @@ def test_managed_group_already_exists_in_app(self): "signing_institution": "Test institution", "version": agreement_version.pk, "date_signed": "2023-01-01", - "is_primary": True, "agreementtype-TOTAL_FORMS": 1, "agreementtype-INITIAL_FORMS": 0, "agreementtype-MIN_NUM_FORMS": 1, @@ -5670,9 +5595,7 @@ def test_context_needs_action_table_grant(self): def test_context_error_table_has_access(self): """error shows a record when audit finds that access needs to be removed.""" - member_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False - ) + member_agreement = factories.MemberAgreementFactory.create(is_primary=False) GroupGroupMembershipFactory.create( parent_group=self.anvil_cdsa_group, child_group=member_agreement.signed_agreement.anvil_access_group, @@ -7069,25 +6992,19 @@ def test_table_no_rows(self): def test_table_three_rows(self): """Three rows are shown if there are three SignedAgreement objects.""" - factories.DataAffiliateAgreementFactory.create_batch( - 3, signed_agreement__is_primary=True - ) + factories.DataAffiliateAgreementFactory.create_batch(3, is_primary=True) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("table", response.context_data) self.assertEqual(len(response.context_data["table"].rows), 3) def test_only_shows_data_affiliate_records(self): - member_agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + member_agreement = factories.MemberAgreementFactory.create(is_primary=True) data_affiliate_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True + is_primary=True ) non_data_affiliate_agreement = ( - factories.NonDataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True - ) + factories.NonDataAffiliateAgreementFactory.create() ) self.client.force_login(self.user) response = self.client.get(self.get_url()) @@ -7099,10 +7016,10 @@ def test_only_shows_data_affiliate_records(self): def test_only_shows_primary_data_affiliate_records(self): primary_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True + is_primary=True ) component_agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False + is_primary=False ) self.client.force_login(self.user) response = self.client.get(self.get_url()) @@ -7176,7 +7093,7 @@ def test_table_no_rows(self): def test_table_one_agreement_no_members(self): """No row is shown if there is one agreement with no account group members.""" - factories.MemberAgreementFactory.create(signed_agreement__is_primary=True) + factories.MemberAgreementFactory.create(is_primary=True) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("table", response.context_data) @@ -7184,9 +7101,7 @@ def test_table_one_agreement_no_members(self): def test_table_one_agreement_one_member(self): """One row is shown if there is one agreement and one account group member.""" - agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement = factories.MemberAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create( group=agreement.signed_agreement.anvil_access_group ) @@ -7197,9 +7112,7 @@ def test_table_one_agreement_one_member(self): def test_table_one_agreements_two_members(self): """Two rows are shown if there is one agreement with two account group members.""" - agreement = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement = factories.MemberAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create_batch( 2, group=agreement.signed_agreement.anvil_access_group ) @@ -7210,15 +7123,11 @@ def test_table_one_agreements_two_members(self): def test_table_two_agreements(self): """Multiple rows is shown if there are two agreements and multiple account group members.""" - agreement_1 = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement_1 = factories.MemberAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create_batch( 2, group=agreement_1.signed_agreement.anvil_access_group ) - agreement_2 = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement_2 = factories.MemberAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create_batch( 3, group=agreement_2.signed_agreement.anvil_access_group ) @@ -7228,21 +7137,15 @@ def test_table_two_agreements(self): self.assertEqual(len(response.context_data["table"].rows), 5) def test_only_shows_records_for_all_agreement_types(self): - agreement_1 = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement_1 = factories.MemberAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create( group=agreement_1.signed_agreement.anvil_access_group ) - agreement_2 = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement_2 = factories.DataAffiliateAgreementFactory.create(is_primary=True) GroupAccountMembershipFactory.create( group=agreement_2.signed_agreement.anvil_access_group ) - agreement_3 = factories.NonDataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True - ) + agreement_3 = factories.NonDataAffiliateAgreementFactory.create() GroupAccountMembershipFactory.create( group=agreement_3.signed_agreement.anvil_access_group ) @@ -7252,28 +7155,18 @@ def test_only_shows_records_for_all_agreement_types(self): self.assertEqual(len(table.rows), 3) def test_shows_includes_component_agreements(self): - agreement_1 = factories.MemberAgreementFactory.create( - signed_agreement__is_primary=False - ) + agreement_1 = factories.MemberAgreementFactory.create(is_primary=False) GroupAccountMembershipFactory.create( group=agreement_1.signed_agreement.anvil_access_group ) - agreement_2 = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False - ) + agreement_2 = factories.DataAffiliateAgreementFactory.create(is_primary=False) GroupAccountMembershipFactory.create( group=agreement_2.signed_agreement.anvil_access_group ) - agreement_3 = factories.NonDataAffiliateAgreementFactory.create( - signed_agreement__is_primary=False - ) - GroupAccountMembershipFactory.create( - group=agreement_3.signed_agreement.anvil_access_group - ) self.client.force_login(self.user) response = self.client.get(self.get_url()) table = response.context_data["table"] - self.assertEqual(len(table.rows), 3) + self.assertEqual(len(table.rows), 2) def test_does_not_show_anvil_upload_group_members(self): agreement = factories.DataAffiliateAgreementFactory.create() @@ -7564,7 +7457,7 @@ def test_context_data_prep_active_with_one_active_one_inactive_prep_workspace(se def test_response_context_primary_cdsa(self): agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, ) instance = factories.CDSAWorkspaceFactory.create( study=agreement.study, @@ -7577,7 +7470,7 @@ def test_response_context_primary_cdsa(self): def test_response_includes_additional_limitations(self): """Response includes DataAffiliate additional limitations if they exist.""" agreement = factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, additional_limitations="Test limitations for this data affiliate agreement", ) instance = factories.CDSAWorkspaceFactory.create( @@ -7605,7 +7498,7 @@ def test_response_data_use_limitations(self): instance.data_use_modifiers.add(modifier_1, modifier_2) # Create an agreement with data use limitations. factories.DataAffiliateAgreementFactory.create( - signed_agreement__is_primary=True, + is_primary=True, study=instance.study, additional_limitations="Test limitations for this data affiliate agreement", ) From 483e15ccc7c390997ff18f2fed9b525c60cad8d8 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 19 Mar 2024 09:20:14 -0700 Subject: [PATCH 08/17] Create CC admins group in example script --- add_cdsa_example_data.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/add_cdsa_example_data.py b/add_cdsa_example_data.py index c8e5eb26..a2343eef 100644 --- a/add_cdsa_example_data.py +++ b/add_cdsa_example_data.py @@ -20,6 +20,11 @@ # Load duos call_command("load_duo") +# create the CDSA auth group +cdsa_group = ManagedGroupFactory.create(name=settings.ANVIL_CDSA_GROUP_NAME) +# Add PRIMED ADMINS group +cc_admins_group = ManagedGroupFactory.create(name=settings.ANVIL_CC_ADMINS_GROUP_NAME) + # Create major versions major_version = factories.AgreementMajorVersionFactory.create(version=1) @@ -35,9 +40,6 @@ dup = DataUsePermission.objects.get(abbreviation="GRU") dum = DataUseModifier.objects.get(abbreviation="NPU") -# create the CDSA auth group -cdsa_group = ManagedGroupFactory.create(name=settings.ANVIL_CDSA_GROUP_NAME) - # Create some study sites. StudySiteFactory.create(short_name="CC", full_name="Coordinating Center") StudySiteFactory.create(short_name="CARDINAL", full_name="CARDINAL") From 9a262acd9fcc8956bff07b49d3b7423df7d03f94 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 19 Mar 2024 09:41:40 -0700 Subject: [PATCH 09/17] Move custom clean errors to specific fields Move the additional_limitation and rqeuires_study_review Validation errors to the specific fields, not the NON_FIELD_ERRORS. This provides better behavior on the forms. --- primed/cdsa/models.py | 7 +++++-- primed/cdsa/tests/test_forms.py | 17 ++++++++++------- primed/cdsa/tests/test_models.py | 16 ++++++++++++++-- primed/cdsa/tests/test_views.py | 14 +++++++------- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index 72c8274b..4ba3cdec 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -301,15 +301,18 @@ def get_absolute_url(self): def clean(self): super().clean() # Checks for fields only allowed for primary agreements. + errors = {} if not self.is_primary: if self.additional_limitations: - raise ValidationError( + errors["additional_limitations"] = ValidationError( "Additional limitations are only allowed for primary agreements." ) if self.requires_study_review: - raise ValidationError( + errors["requires_study_review"] = ValidationError( "requires_study_review can only be True for primary agreements." ) + if errors: + raise ValidationError(errors) def get_agreement_group(self): return self.study diff --git a/primed/cdsa/tests/test_forms.py b/primed/cdsa/tests/test_forms.py index 3ea774f2..1dcea0af 100644 --- a/primed/cdsa/tests/test_forms.py +++ b/primed/cdsa/tests/test_forms.py @@ -1,7 +1,6 @@ """Tests for the `cdsa` app.""" from anvil_consortium_manager.tests.factories import WorkspaceFactory -from django.core.exceptions import NON_FIELD_ERRORS from django.test import TestCase from primed.duo.models import DataUseModifier @@ -448,9 +447,11 @@ def test_invalid_component_with_additional_limitations(self): } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) - self.assertIn(NON_FIELD_ERRORS, form.errors) - self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) - self.assertIn("only allowed for primary", form.errors[NON_FIELD_ERRORS][0]) + self.assertIn("additional_limitations", form.errors) + self.assertEqual(len(form.errors["additional_limitations"]), 1) + self.assertIn( + "only allowed for primary", form.errors["additional_limitations"][0] + ) def test_valid_primary_with_requires_study_review_true(self): """Form is valid with necessary input.""" @@ -473,9 +474,11 @@ def test_invalid_component_with_requires_study_review_true(self): } form = self.form_class(data=form_data) self.assertFalse(form.is_valid()) - self.assertIn(NON_FIELD_ERRORS, form.errors) - self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1) - self.assertIn("can only be True for primary", form.errors[NON_FIELD_ERRORS][0]) + self.assertIn("requires_study_review", form.errors) + self.assertEqual(len(form.errors["requires_study_review"]), 1) + self.assertIn( + "can only be True for primary", form.errors["requires_study_review"][0] + ) class NonDataAffiliateAgreementFormTest(TestCase): diff --git a/primed/cdsa/tests/test_models.py b/primed/cdsa/tests/test_models.py index 6b7dbf78..eb9fbe66 100644 --- a/primed/cdsa/tests/test_models.py +++ b/primed/cdsa/tests/test_models.py @@ -522,7 +522,13 @@ def test_clean_additional_limitations_not_primary(self): ) with self.assertRaises(ValidationError) as e: instance.clean() - self.assertIn("only allowed for primary agreements", e.exception.message) + self.assertEqual(len(e.exception.error_dict), 1) + self.assertIn("additional_limitations", e.exception.error_dict) + self.assertEqual(len(e.exception.error_dict["additional_limitations"]), 1) + self.assertIn( + "only allowed for primary agreements", + e.exception.error_dict["additional_limitations"][0].message, + ) def test_str_method(self): """The custom __str__ method returns the correct string.""" @@ -574,7 +580,13 @@ def test_requires_study_review_not_primary(self): ) with self.assertRaises(ValidationError) as e: instance.clean() - self.assertIn("can only be True for primary", e.exception.message) + self.assertEqual(len(e.exception.error_dict), 1) + self.assertIn("requires_study_review", e.exception.error_dict) + self.assertEqual(len(e.exception.error_dict["requires_study_review"]), 1) + self.assertIn( + "can only be True for primary", + e.exception.error_dict["requires_study_review"][0].message, + ) class NonDataAffiliateAgreementTest(TestCase): diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 2699ed64..82062681 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -21,7 +21,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.contrib.messages import get_messages -from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied +from django.core.exceptions import PermissionDenied from django.http import Http404 from django.shortcuts import resolve_url from django.test import RequestFactory, TestCase, override_settings @@ -2956,11 +2956,11 @@ def test_cannot_create_component_with_requires_study_review(self): self.assertFalse(formset.is_valid()) self.assertFalse(formset.forms[0].is_valid()) self.assertEqual(len(formset.forms[0].errors), 1) - self.assertIn(NON_FIELD_ERRORS, formset.forms[0].errors) - self.assertEqual(len(formset.forms[0].errors[NON_FIELD_ERRORS]), 1) + self.assertIn("requires_study_review", formset.forms[0].errors) + self.assertEqual(len(formset.forms[0].errors["requires_study_review"]), 1) self.assertIn( "can only be True for primary", - formset.forms[0].errors[NON_FIELD_ERRORS][0], + formset.forms[0].errors["requires_study_review"][0], ) def test_can_create_primary_with_additional_limitations(self): @@ -3054,11 +3054,11 @@ def test_cannot_create_component_with_additional_limitations(self): self.assertFalse(formset.is_valid()) self.assertFalse(formset.forms[0].is_valid()) self.assertEqual(len(formset.forms[0].errors), 1) - self.assertIn(NON_FIELD_ERRORS, formset.forms[0].errors) - self.assertEqual(len(formset.forms[0].errors[NON_FIELD_ERRORS]), 1) + self.assertIn("additional_limitations", formset.forms[0].errors) + self.assertEqual(len(formset.forms[0].errors["additional_limitations"]), 1) self.assertIn( "only allowed for primary", - formset.forms[0].errors[NON_FIELD_ERRORS][0], + formset.forms[0].errors["additional_limitations"][0], ) def test_error_missing_cc_id(self): From 87198cc3b88e85570b1faaccb4de7edafde183e3 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 19 Mar 2024 13:41:01 -0700 Subject: [PATCH 10/17] Show requires_study_review indicator on detail pages Include an indicator of whether requires_study_review is true on the CDSAWorkspace and DataAFfiliateAgreement detail pages. --- primed/cdsa/tests/test_views.py | 26 +++++++++++++++++++ .../templates/cdsa/cdsaworkspace_detail.html | 12 +++++++++ .../cdsa/dataaffiliateagreement_detail.html | 7 +++++ 3 files changed, 45 insertions(+) diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 82062681..9b6b546c 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -7524,6 +7524,32 @@ def test_response_data_use_limitations(self): "
  • Test additional limitations for workspace
    • ", ) + def test_response_requires_study_review_true(self): + """Response includes DataAffiliate info about study review required if true.""" + agreement = factories.DataAffiliateAgreementFactory.create( + is_primary=True, + requires_study_review=True, + ) + instance = factories.CDSAWorkspaceFactory.create( + study=agreement.study, + ) + self.client.force_login(self.user) + response = self.client.get(instance.get_absolute_url()) + self.assertContains(response, "Study review required") + + def test_response_requires_study_review_false(self): + """Response includes DataAffiliate info about study review required if true.""" + agreement = factories.DataAffiliateAgreementFactory.create( + is_primary=True, + requires_study_review=False, + ) + instance = factories.CDSAWorkspaceFactory.create( + study=agreement.study, + ) + self.client.force_login(self.user) + response = self.client.get(instance.get_absolute_url()) + self.assertNotContains(response, "Study review required") + class CDSAWorkspaceCreateTest(AnVILAPIMockTestMixin, TestCase): """Tests of the WorkspaceCreate view from ACM with this app's CDSAWorkspace model.""" diff --git a/primed/templates/cdsa/cdsaworkspace_detail.html b/primed/templates/cdsa/cdsaworkspace_detail.html index 9c2d8ee7..d8ea0341 100644 --- a/primed/templates/cdsa/cdsaworkspace_detail.html +++ b/primed/templates/cdsa/cdsaworkspace_detail.html @@ -6,6 +6,18 @@ {% include "snippets/gsr_restricted_badge.html" %} {% endif %} + {% if primary_cdsa.requires_study_review %} + + + Study review required + + + {% endif %} + {{ block.super }} {% endblock pills %} diff --git a/primed/templates/cdsa/dataaffiliateagreement_detail.html b/primed/templates/cdsa/dataaffiliateagreement_detail.html index 0d48c8a8..d13430ce 100644 --- a/primed/templates/cdsa/dataaffiliateagreement_detail.html +++ b/primed/templates/cdsa/dataaffiliateagreement_detail.html @@ -21,6 +21,13 @@
    Representative role
    {{ object.signed_agreement.representative_role }}
    Signing institution
    {{ object.signed_agreement.signing_institution }}
    Primary?
    {{ object.signed_agreement.is_primary }}
    +
    Study review required?
    + {% if object.requires_study_review %} + Yes + {% else %} + No + {% endif %} +
    Agreement version
    {{ object.signed_agreement.version }}
    From fe086506b627047d5c0ba6f3c08386522295c7f5 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 19 Mar 2024 13:50:25 -0700 Subject: [PATCH 11/17] Fix is_primary indicator on detail pages With is_primary being moved to the agreement type classes, the detail pages needed to be fixed to show it properly. --- primed/templates/cdsa/dataaffiliateagreement_detail.html | 8 +++++++- primed/templates/cdsa/memberagreement_detail.html | 8 +++++++- .../templates/cdsa/nondataaffiliateagreement_detail.html | 1 - 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/primed/templates/cdsa/dataaffiliateagreement_detail.html b/primed/templates/cdsa/dataaffiliateagreement_detail.html index d13430ce..ac650255 100644 --- a/primed/templates/cdsa/dataaffiliateagreement_detail.html +++ b/primed/templates/cdsa/dataaffiliateagreement_detail.html @@ -20,7 +20,13 @@
    Representative role
    {{ object.signed_agreement.representative_role }}
    Signing institution
    {{ object.signed_agreement.signing_institution }}
    -
    Primary?
    {{ object.signed_agreement.is_primary }}
    +
    Primary?
    + {% if object.is_primary %} + Yes + {% else %} + No + {% endif %} +
    Study review required?
    {% if object.requires_study_review %} Yes diff --git a/primed/templates/cdsa/memberagreement_detail.html b/primed/templates/cdsa/memberagreement_detail.html index 381b1f23..c4a4b1d9 100644 --- a/primed/templates/cdsa/memberagreement_detail.html +++ b/primed/templates/cdsa/memberagreement_detail.html @@ -20,7 +20,13 @@
    Representative role
    {{ object.signed_agreement.representative_role }}
    Signing institution
    {{ object.signed_agreement.signing_institution }}
    -
    Primary?
    {{ object.signed_agreement.is_primary }}
    +
    Primary?
    + {% if object.is_primary %} + Yes + {% else %} + No + {% endif %} +
    Agreement version
    {{ object.signed_agreement.version }}
    diff --git a/primed/templates/cdsa/nondataaffiliateagreement_detail.html b/primed/templates/cdsa/nondataaffiliateagreement_detail.html index bcd4b934..4d08830f 100644 --- a/primed/templates/cdsa/nondataaffiliateagreement_detail.html +++ b/primed/templates/cdsa/nondataaffiliateagreement_detail.html @@ -20,7 +20,6 @@
    Representative role
    {{ object.signed_agreement.representative_role }}
    Signing institution
    {{ object.signed_agreement.signing_institution }}
    -
    Primary?
    {{ object.signed_agreement.is_primary }}
    Agreement version
    {{ object.signed_agreement.version }}
    From 4670cc241e0bffe0286733d97eb99245a212db8d Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Tue, 19 Mar 2024 14:45:19 -0700 Subject: [PATCH 12/17] Add requires_study_review fields to CDSA tables Add a field to the DataAffiliateAgreementTable and the CDSAWorkspace Table to indicate whether study review is required. --- add_cdsa_example_data.py | 12 ++++++++ primed/cdsa/tables.py | 46 +++++++++++++++++++++++++++++ primed/cdsa/tests/test_tables.py | 50 ++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/add_cdsa_example_data.py b/add_cdsa_example_data.py index a2343eef..da5c0058 100644 --- a/add_cdsa_example_data.py +++ b/add_cdsa_example_data.py @@ -120,6 +120,7 @@ study=Study.objects.get(short_name="MESA"), signed_agreement__version=v10, additional_limitations="This data can only be used for testing the app.", + requires_study_review=True, ) GroupGroupMembershipFactory.create( parent_group=cdsa_group, child_group=cdsa_1006.signed_agreement.anvil_access_group @@ -229,3 +230,14 @@ additional_limitations="Additional limitations for workspace.", ) cdsa_workspace_2.data_use_modifiers.add(dum) + + +# Add a workspace with no primary cdsa. +cdsa_workspace_3 = factories.CDSAWorkspaceFactory.create( + workspace__billing_project__name="demo-primed-cdsa", + workspace__name="DEMO_PRIMED_CDSA_ARIC_1", + study=Study.objects.create( + short_name="ARIC", full_name="Atherosclerosis Risk in Communities" + ), + data_use_permission=dup, +) diff --git a/primed/cdsa/tables.py b/primed/cdsa/tables.py index d7e3c1a6..69512990 100644 --- a/primed/cdsa/tables.py +++ b/primed/cdsa/tables.py @@ -6,6 +6,7 @@ Workspace, WorkspaceGroupSharing, ) +from django.utils.safestring import mark_safe from primed.primed_anvil.tables import ( BooleanIconColumn, @@ -112,6 +113,12 @@ class DataAffiliateAgreementTable(tables.Table): signed_agreement__cc_id = tables.Column(linkify=True) study = tables.Column(linkify=True) is_primary = BooleanIconColumn(verbose_name="Primary?") + requires_study_review = BooleanIconColumn( + verbose_name="Study review required?", + orderable=False, + true_icon="dash-circle-fill", + true_color="#ffc107", + ) signed_agreement__representative__name = tables.Column( linkify=lambda record: record.signed_agreement.representative.get_absolute_url(), verbose_name="Representative", @@ -129,6 +136,7 @@ class Meta: "signed_agreement__cc_id", "study", "is_primary", + "requires_study_review", "signed_agreement__representative__name", "signed_agreement__representative_role", "signed_agreement__signing_institution", @@ -306,6 +314,12 @@ class CDSAWorkspaceStaffTable(tables.Table): verbose_name="DUO modifiers", linkify_item=True, ) + cdsaworkspace_requires_study_review = BooleanIconColumn( + verbose_name="Study review required?", + orderable=False, + true_icon="dash-circle-fill", + true_color="#ffc107", + ) cdsaworkspace__gsr_restricted = BooleanIconColumn( orderable=False, true_icon="dash-circle-fill", true_color="#ffc107" ) @@ -319,10 +333,23 @@ class Meta: "cdsaworkspace__study", "cdsaworkspace__data_use_permission__abbreviation", "cdsaworkspace__data_use_modifiers", + "cdsaworkspace_requires_study_review", "cdsaworkspace__gsr_restricted", ) order_by = ("name",) + def render_requires_study_review(self, record): + try: + if record.cdsaworkspace.get_primary_cdsa().requires_study_review: + icon = "dash-circle-fill" + color = "#ffc107" + else: + return "" + except models.DataAffiliateAgreement.DoesNotExist: + icon = "question-circle-fill" + color = "red" + return mark_safe(f'') + class CDSAWorkspaceUserTable(tables.Table): """A table for the CDSAWorkspace model.""" @@ -337,6 +364,12 @@ class CDSAWorkspaceUserTable(tables.Table): transform=lambda x: x.abbreviation, verbose_name="DUO modifiers", ) + cdsaworkspace_requires_study_review = BooleanIconColumn( + verbose_name="Study review required?", + orderable=False, + true_icon="dash-circle-fill", + true_color="#ffc107", + ) cdsaworkspace__gsr_restricted = BooleanIconColumn(orderable=False) is_shared = WorkspaceSharedWithConsortiumColumn() @@ -348,6 +381,19 @@ class Meta: "cdsaworkspace__study", "cdsaworkspace__data_use_permission__abbreviation", "cdsaworkspace__data_use_modifiers", + "cdsaworkspace_requires_study_review", "cdsaworkspace__gsr_restricted", ) order_by = ("name",) + + def render_requires_study_review(self, record): + try: + if record.cdsaworkspace.get_primary_cdsa().requires_study_review: + icon = "dash-circle-fill" + color = "#ffc107" + else: + return "" + except models.DataAffiliateAgreement.DoesNotExist: + icon = "question-circle-fill" + color = "red" + return mark_safe(f'') diff --git a/primed/cdsa/tests/test_tables.py b/primed/cdsa/tests/test_tables.py index f239a383..dbde4fd2 100644 --- a/primed/cdsa/tests/test_tables.py +++ b/primed/cdsa/tests/test_tables.py @@ -472,6 +472,31 @@ def test_ordering(self): self.assertEqual(table.data[0], instance_2.workspace) self.assertEqual(table.data[1], instance_1.workspace) + def test_render_requires_study_review(self): + table = self.table_class(self.model.objects.all()) + # CDSA workspace with no data_affiliate_agreement. + cdsa_workspace = factories.CDSAWorkspaceFactory.create() + self.assertIn( + "question-circle-fill", + table.render_requires_study_review(cdsa_workspace.workspace), + ) + # With a primary - no review required. + agreement = factories.DataAffiliateAgreementFactory.create( + is_primary=True, + requires_study_review=False, + study=cdsa_workspace.study, + ) + self.assertEqual( + "", table.render_requires_study_review(cdsa_workspace.workspace) + ) + # With a primary - review required. + agreement.requires_study_review = True + agreement.save() + self.assertIn( + "dash-circle-fill", + table.render_requires_study_review(cdsa_workspace.workspace), + ) + class CDSAWorkspaceUserTableTest(TestCase): """Tests for the CDSAWorkspaceUserTable class.""" @@ -501,3 +526,28 @@ def test_ordering(self): table = self.table_class(self.model.objects.all()) self.assertEqual(table.data[0], instance_2.workspace) self.assertEqual(table.data[1], instance_1.workspace) + + def test_render_requires_study_review(self): + table = self.table_class(self.model.objects.all()) + # CDSA workspace with no data_affiliate_agreement. + cdsa_workspace = factories.CDSAWorkspaceFactory.create() + self.assertIn( + "question-circle-fill", + table.render_requires_study_review(cdsa_workspace.workspace), + ) + # With a primary - no review required. + agreement = factories.DataAffiliateAgreementFactory.create( + is_primary=True, + requires_study_review=False, + study=cdsa_workspace.study, + ) + self.assertEqual( + "", table.render_requires_study_review(cdsa_workspace.workspace) + ) + # With a primary - review required. + agreement.requires_study_review = True + agreement.save() + self.assertIn( + "dash-circle-fill", + table.render_requires_study_review(cdsa_workspace.workspace), + ) From 959b3a64eedc7fa4e8451a1fee0820311d0abaaf Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 20 Mar 2024 16:13:52 -0700 Subject: [PATCH 13/17] Test detail page with and without requires_study_review --- primed/cdsa/tests/test_views.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 9b6b546c..4df993bb 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -4232,6 +4232,29 @@ def test_response_with_no_additional_limitations(self): response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) self.assertNotContains(response, "Additional limitations") + def test_response_requires_study_review(self): + """Response includes info about requires_study_review.""" + instance = factories.DataAffiliateAgreementFactory.create( + is_primary=True, requires_study_review=True + ) + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) + self.assertContains(response, "Study review required?") + self.assertContains( + response, + """
    Yes
    """, + html=True, + ) + instance.requires_study_review = False + instance.save() + response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) + self.assertContains(response, "Study review required?") + self.assertContains( + response, + """
    No
    """, + html=True, + ) + class DataAffiliateAgreementListTest(TestCase): """Tests for the DataAffiliateAgreement view.""" From a43b236863fa488272510b940240cff19d843e12 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 20 Mar 2024 16:24:35 -0700 Subject: [PATCH 14/17] Remove audit error for component NonDataAffiliateAgreements This concept no longer exists, since NonDataAffiliateAgreement does not have an is_primary field. They are all primary. --- primed/cdsa/audit/signed_agreement_audit.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/primed/cdsa/audit/signed_agreement_audit.py b/primed/cdsa/audit/signed_agreement_audit.py index 0eeee1f4..669df722 100644 --- a/primed/cdsa/audit/signed_agreement_audit.py +++ b/primed/cdsa/audit/signed_agreement_audit.py @@ -119,9 +119,6 @@ class SignedAgreementAccessAudit(PRIMEDAudit): PRIMARY_NOT_ACTIVE = "Primary agreement for this CDSA is not active." # Other errors - ERROR_NON_DATA_AFFILIATE_COMPONENT = ( - "Non-data affiliate agreements must be primary." - ) ERROR_OTHER_CASE = "Signed Agreement did not match any expected situations." results_table_class = SignedAgreementAccessAuditTable @@ -227,14 +224,6 @@ def _audit_component_agreement(self, signed_agreement): dataaffiliateagreement__is_primary=True, dataaffiliateagreement__study=signed_agreement.dataaffiliateagreement.study, ) - elif hasattr(signed_agreement, "nondataaffiliateagreement"): - self.errors.append( - OtherError( - signed_agreement=signed_agreement, - note=self.ERROR_NON_DATA_AFFILIATE_COMPONENT, - ) - ) - return primary_exists = primary_qs.exists() primary_active = primary_qs.filter( status=models.SignedAgreement.StatusChoices.ACTIVE, From 5498162d7264686e581fe332cb6b8cd8d97bed10 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 20 Mar 2024 17:06:37 -0700 Subject: [PATCH 15/17] Clean up test coverage --- primed/cdsa/tests/test_views.py | 51 +++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 4df993bb..1cdfa57a 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -2515,6 +2515,29 @@ def test_change_status_button_user_has_view_perm(self): ), ) + def test_response_is_primary(self): + """Response includes info about requires_study_review.""" + instance = factories.MemberAgreementFactory.create( + is_primary=True, + ) + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) + self.assertContains(response, "Primary?") + self.assertContains( + response, + """
    Primary?
    Yes
    """, # noqa: E501 + html=True, + ) + instance.is_primary = False + instance.save() + response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) + self.assertContains(response, "Primary?") + self.assertContains( + response, + """
    Primary?
    No
    """, # noqa: E501 + html=True, + ) + class MemberAgreementListTest(TestCase): """Tests for the MemberAgreementList view.""" @@ -4232,6 +4255,29 @@ def test_response_with_no_additional_limitations(self): response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) self.assertNotContains(response, "Additional limitations") + def test_response_is_primary(self): + """Response includes info about requires_study_review.""" + instance = factories.DataAffiliateAgreementFactory.create( + is_primary=True, + ) + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) + self.assertContains(response, "Primary?") + self.assertContains( + response, + """
    Primary?
    Yes
    """, # noqa: E501 + html=True, + ) + instance.is_primary = False + instance.save() + response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) + self.assertContains(response, "Primary?") + self.assertContains( + response, + """
    Primary?
    No
    """, # noqa: E501 + html=True, + ) + def test_response_requires_study_review(self): """Response includes info about requires_study_review.""" instance = factories.DataAffiliateAgreementFactory.create( @@ -4240,9 +4286,10 @@ def test_response_requires_study_review(self): self.client.force_login(self.user) response = self.client.get(self.get_url(instance.signed_agreement.cc_id)) self.assertContains(response, "Study review required?") + # import ipdb; ipdb.set_trace() self.assertContains( response, - """
    Yes
    """, + """
    Study review required?
    Yes
    """, # noqa: E501 html=True, ) instance.requires_study_review = False @@ -4251,7 +4298,7 @@ def test_response_requires_study_review(self): self.assertContains(response, "Study review required?") self.assertContains( response, - """
    No
    """, + """
    Study review required?
    No
    """, # noqa: E501 html=True, ) From 670b01ad7f7d5f6818b416a0a8ac856a1c7db8f0 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 21 Mar 2024 10:31:13 -0700 Subject: [PATCH 16/17] Fix requires_study_review display in CDSAWorkspace table --- primed/cdsa/tables.py | 12 ++++++------ primed/cdsa/tests/test_tables.py | 14 ++++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/primed/cdsa/tables.py b/primed/cdsa/tables.py index 69512990..4fd93ce1 100644 --- a/primed/cdsa/tables.py +++ b/primed/cdsa/tables.py @@ -314,7 +314,7 @@ class CDSAWorkspaceStaffTable(tables.Table): verbose_name="DUO modifiers", linkify_item=True, ) - cdsaworkspace_requires_study_review = BooleanIconColumn( + cdsaworkspace__requires_study_review = BooleanIconColumn( verbose_name="Study review required?", orderable=False, true_icon="dash-circle-fill", @@ -333,12 +333,12 @@ class Meta: "cdsaworkspace__study", "cdsaworkspace__data_use_permission__abbreviation", "cdsaworkspace__data_use_modifiers", - "cdsaworkspace_requires_study_review", + "cdsaworkspace__requires_study_review", "cdsaworkspace__gsr_restricted", ) order_by = ("name",) - def render_requires_study_review(self, record): + def render_cdsaworkspace__requires_study_review(self, record): try: if record.cdsaworkspace.get_primary_cdsa().requires_study_review: icon = "dash-circle-fill" @@ -364,7 +364,7 @@ class CDSAWorkspaceUserTable(tables.Table): transform=lambda x: x.abbreviation, verbose_name="DUO modifiers", ) - cdsaworkspace_requires_study_review = BooleanIconColumn( + cdsaworkspace__requires_study_review = BooleanIconColumn( verbose_name="Study review required?", orderable=False, true_icon="dash-circle-fill", @@ -381,12 +381,12 @@ class Meta: "cdsaworkspace__study", "cdsaworkspace__data_use_permission__abbreviation", "cdsaworkspace__data_use_modifiers", - "cdsaworkspace_requires_study_review", + "cdsaworkspace__requires_study_review", "cdsaworkspace__gsr_restricted", ) order_by = ("name",) - def render_requires_study_review(self, record): + def render_cdsaworkspace__requires_study_review(self, record): try: if record.cdsaworkspace.get_primary_cdsa().requires_study_review: icon = "dash-circle-fill" diff --git a/primed/cdsa/tests/test_tables.py b/primed/cdsa/tests/test_tables.py index dbde4fd2..83f6bf55 100644 --- a/primed/cdsa/tests/test_tables.py +++ b/primed/cdsa/tests/test_tables.py @@ -478,7 +478,7 @@ def test_render_requires_study_review(self): cdsa_workspace = factories.CDSAWorkspaceFactory.create() self.assertIn( "question-circle-fill", - table.render_requires_study_review(cdsa_workspace.workspace), + table.render_cdsaworkspace__requires_study_review(cdsa_workspace.workspace), ) # With a primary - no review required. agreement = factories.DataAffiliateAgreementFactory.create( @@ -487,14 +487,15 @@ def test_render_requires_study_review(self): study=cdsa_workspace.study, ) self.assertEqual( - "", table.render_requires_study_review(cdsa_workspace.workspace) + "", + table.render_cdsaworkspace__requires_study_review(cdsa_workspace.workspace), ) # With a primary - review required. agreement.requires_study_review = True agreement.save() self.assertIn( "dash-circle-fill", - table.render_requires_study_review(cdsa_workspace.workspace), + table.render_cdsaworkspace__requires_study_review(cdsa_workspace.workspace), ) @@ -533,7 +534,7 @@ def test_render_requires_study_review(self): cdsa_workspace = factories.CDSAWorkspaceFactory.create() self.assertIn( "question-circle-fill", - table.render_requires_study_review(cdsa_workspace.workspace), + table.render_cdsaworkspace__requires_study_review(cdsa_workspace.workspace), ) # With a primary - no review required. agreement = factories.DataAffiliateAgreementFactory.create( @@ -542,12 +543,13 @@ def test_render_requires_study_review(self): study=cdsa_workspace.study, ) self.assertEqual( - "", table.render_requires_study_review(cdsa_workspace.workspace) + "", + table.render_cdsaworkspace__requires_study_review(cdsa_workspace.workspace), ) # With a primary - review required. agreement.requires_study_review = True agreement.save() self.assertIn( "dash-circle-fill", - table.render_requires_study_review(cdsa_workspace.workspace), + table.render_cdsaworkspace__requires_study_review(cdsa_workspace.workspace), ) From 37d47e9259e862b45ebb0e8b1346fa8627c9b4d9 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 21 Mar 2024 10:31:40 -0700 Subject: [PATCH 17/17] Show primary cdsa link on CDSAWorkspace detail page --- primed/cdsa/tests/test_views.py | 24 +++++++++++++++++++ .../templates/cdsa/cdsaworkspace_detail.html | 19 ++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 1cdfa57a..beb8e4e8 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -7620,6 +7620,30 @@ def test_response_requires_study_review_false(self): response = self.client.get(instance.get_absolute_url()) self.assertNotContains(response, "Study review required") + def test_response_primary_cdsa(self): + """Response includes note about missing primary cdsa about study review required if true.""" + agreement = factories.DataAffiliateAgreementFactory.create( + is_primary=True, + ) + instance = factories.CDSAWorkspaceFactory.create( + study=agreement.study, + ) + self.client.force_login(self.user) + response = self.client.get(instance.get_absolute_url()) + self.assertContains(response, agreement.get_absolute_url()) + + def test_response_no_primary_cdsa(self): + """Response includes note about missing primary cdsa about study review required if true.""" + instance = factories.CDSAWorkspaceFactory.create() + self.client.force_login(self.user) + response = self.client.get(instance.get_absolute_url()) + self.assertContains( + response, + # """
    Associated CDSA
    mdash;
    """ + """No primary CDSA""" + # """
    Associated CDSA
    —
    """, # noqa: E501 + ) + class CDSAWorkspaceCreateTest(AnVILAPIMockTestMixin, TestCase): """Tests of the WorkspaceCreate view from ACM with this app's CDSAWorkspace model.""" diff --git a/primed/templates/cdsa/cdsaworkspace_detail.html b/primed/templates/cdsa/cdsaworkspace_detail.html index d8ea0341..3a192665 100644 --- a/primed/templates/cdsa/cdsaworkspace_detail.html +++ b/primed/templates/cdsa/cdsaworkspace_detail.html @@ -6,7 +6,17 @@ {% include "snippets/gsr_restricted_badge.html" %} {% endif %} - {% if primary_cdsa.requires_study_review %} + {% if not primary_cdsa %} + + + No primary CDSA + + + {% elif primary_cdsa.requires_study_review %} Study review required @@ -24,6 +34,13 @@ {% block workspace_data %}
    +
    Associated CDSA
    + {% if primary_cdsa %} + {{ primary_cdsa }} + {% else %} + — + {% endif %} +
    Study
    {{ object.cdsaworkspace.study }}