From 948b3fd5ab37d9d8b83e383b928a657e41e96f5c Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 18 Jan 2024 15:51:15 -0800 Subject: [PATCH 1/4] Add new REPLACED status for SignedAgreements This status indicates CDSAs that were replaced by newer agreements. We do not maintain a link to the CDSA that it is replacing in django; that is handled in outside pre-tracking. --- ...011_signedagreement_add_replaced_status.py | 46 +++++++++++++++++++ primed/cdsa/models.py | 3 ++ primed/cdsa/tests/test_models.py | 20 ++------ 3 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 primed/cdsa/migrations/0011_signedagreement_add_replaced_status.py diff --git a/primed/cdsa/migrations/0011_signedagreement_add_replaced_status.py b/primed/cdsa/migrations/0011_signedagreement_add_replaced_status.py new file mode 100644 index 00000000..f1c1bb06 --- /dev/null +++ b/primed/cdsa/migrations/0011_signedagreement_add_replaced_status.py @@ -0,0 +1,46 @@ +# Generated by Django 4.2.8 on 2024-01-18 23:49 + +from django.db import migrations +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ("cdsa", "0010_alter_historicalsignedagreement_status_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="historicalsignedagreement", + name="status", + field=model_utils.fields.StatusField( + choices=[ + ("active", "Active"), + ("withdrawn", "Withdrawn"), + ("lapsed", "Lapsed"), + ("replaced", "Replaced"), + ], + default="active", + max_length=100, + no_check_for_status=True, + verbose_name="status", + ), + ), + migrations.AlterField( + model_name="signedagreement", + name="status", + field=model_utils.fields.StatusField( + choices=[ + ("active", "Active"), + ("withdrawn", "Withdrawn"), + ("lapsed", "Lapsed"), + ("replaced", "Replaced"), + ], + default="active", + max_length=100, + no_check_for_status=True, + verbose_name="status", + ), + ), + ] diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index 5dd8a053..a055eef8 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -108,6 +108,9 @@ class StatusChoices(models.TextChoices): LAPSED = "lapsed", "Lapsed" """SignedAgreements from a AgreementMajorVersion that is no longer valid.""" + REPLACED = "replaced", "Replaced" + """SignedAgreements that have been replaced by a newer version.""" + STATUS = StatusChoices.choices diff --git a/primed/cdsa/tests/test_models.py b/primed/cdsa/tests/test_models.py index 1165c684..f39ba093 100644 --- a/primed/cdsa/tests/test_models.py +++ b/primed/cdsa/tests/test_models.py @@ -209,21 +209,6 @@ def test_get_absolute_url(self): instance.signed_agreement.get_absolute_url(), instance.get_absolute_url() ) - def test_statuses(self): - """All allowed statuses.""" - instance = factories.SignedAgreementFactory.create( - status=models.SignedAgreement.StatusChoices.ACTIVE - ) - instance.full_clean() - instance = factories.SignedAgreementFactory.create( - status=models.SignedAgreement.StatusChoices.WITHDRAWN - ) - instance.full_clean() - instance = factories.SignedAgreementFactory.create( - status=models.SignedAgreement.StatusChoices.LAPSED - ) - instance.full_clean() - def test_member_choices(self): """Can create instances with all of the member choices.""" instance = factories.SignedAgreementFactory.create( @@ -319,6 +304,11 @@ def test_status_field(self): ) self.assertEqual(instance.status, instance.StatusChoices.LAPSED) instance.full_clean() + instance = factories.SignedAgreementFactory.create( + status=models.SignedAgreement.StatusChoices.REPLACED + ) + self.assertEqual(instance.status, instance.StatusChoices.REPLACED) + instance.full_clean() # not allowed instance = factories.SignedAgreementFactory.create(status="foo") From f0e44844954087ccc7a360b2c830442008167aee Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 18 Jan 2024 16:14:01 -0800 Subject: [PATCH 2/4] Update view tests for new REPLACED status --- primed/cdsa/tests/test_views.py | 29 ++++++++++ primed/collaborative_analysis/urls.py | 21 ++++++++ .../collaborativeanalysisworkspace_audit.html | 53 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 primed/collaborative_analysis/urls.py create mode 100644 primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html diff --git a/primed/cdsa/tests/test_views.py b/primed/cdsa/tests/test_views.py index 3fb7b4fe..2302edb8 100644 --- a/primed/cdsa/tests/test_views.py +++ b/primed/cdsa/tests/test_views.py @@ -548,6 +548,10 @@ def test_only_sets_active_signed_agreements_to_lapsed(self): version__major_version=instance, status=models.SignedAgreement.StatusChoices.LAPSED, ) + replaced_agreement = factories.SignedAgreementFactory.create( + version__major_version=instance, + status=models.SignedAgreement.StatusChoices.REPLACED, + ) self.client.force_login(self.user) response = self.client.post(self.get_url(instance.version), {}) self.assertEqual(response.status_code, 302) @@ -559,6 +563,10 @@ def test_only_sets_active_signed_agreements_to_lapsed(self): self.assertEqual( withdrawn_agreement.status, models.SignedAgreement.StatusChoices.WITHDRAWN ) + replaced_agreement.refresh_from_db() + self.assertEqual( + replaced_agreement.status, models.SignedAgreement.StatusChoices.REPLACED + ) def test_only_sets_associated_signed_agreements_to_lapsed(self): """Does not set SignedAgreements associated with a different version to LAPSED.""" @@ -5293,6 +5301,9 @@ def test_only_includes_active_agreements(self): withdrawn_agreement = factories.MemberAgreementFactory.create( signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN ) + replaced_agreement = factories.MemberAgreementFactory.create( + signed_agreement__status=models.SignedAgreement.StatusChoices.REPLACED + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("table", response.context_data) @@ -5301,6 +5312,7 @@ def test_only_includes_active_agreements(self): self.assertIn(active_agreement.signed_agreement, table.data) self.assertNotIn(lapsed_agreement.signed_agreement, table.data) self.assertNotIn(withdrawn_agreement.signed_agreement, table.data) + self.assertNotIn(replaced_agreement.signed_agreement, table.data) class SignedAgreementAuditTest(TestCase): @@ -5758,6 +5770,9 @@ def test_only_includes_active_agreements(self): withdrawn_agreement = factories.DataAffiliateAgreementFactory.create( signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN ) + replaced_agreement = factories.DataAffiliateAgreementFactory.create( + signed_agreement__status=models.SignedAgreement.StatusChoices.REPLACED + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("table", response.context_data) @@ -5766,6 +5781,7 @@ def test_only_includes_active_agreements(self): self.assertIn(active_agreement, table.data) self.assertNotIn(lapsed_agreement, table.data) self.assertNotIn(withdrawn_agreement, table.data) + self.assertNotIn(replaced_agreement, table.data) class UserAccessRecordsList(TestCase): @@ -5943,6 +5959,12 @@ def test_only_includes_active_agreements(self): withdrawn_member = GroupAccountMembershipFactory.create( group=withdrawn_agreement.signed_agreement.anvil_access_group ) + replaced_agreement = factories.MemberAgreementFactory.create( + signed_agreement__status=models.SignedAgreement.StatusChoices.REPLACED + ) + replaced_member = GroupAccountMembershipFactory.create( + group=replaced_agreement.signed_agreement.anvil_access_group + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("table", response.context_data) @@ -5951,6 +5973,7 @@ def test_only_includes_active_agreements(self): self.assertIn(active_member, table.data) self.assertNotIn(lapsed_member, table.data) self.assertNotIn(withdrawn_member, table.data) + self.assertNotIn(replaced_member, table.data) class CDSAWorkspaceRecordsList(TestCase): @@ -6021,6 +6044,11 @@ def test_only_includes_workspaces_with_active_agreements(self): study=withdrawn_workspace.study, signed_agreement__status=models.SignedAgreement.StatusChoices.WITHDRAWN, ) + replaced_workspace = factories.CDSAWorkspaceFactory.create() + factories.DataAffiliateAgreementFactory.create( + study=replaced_workspace.study, + signed_agreement__status=models.SignedAgreement.StatusChoices.REPLACED, + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("table", response.context_data) @@ -6029,6 +6057,7 @@ def test_only_includes_workspaces_with_active_agreements(self): self.assertIn(active_workspace, table.data) self.assertNotIn(lapsed_workspace, table.data) self.assertNotIn(withdrawn_workspace, table.data) + self.assertNotIn(replaced_workspace, table.data) class CDSAWorkspaceDetailTest(TestCase): diff --git a/primed/collaborative_analysis/urls.py b/primed/collaborative_analysis/urls.py new file mode 100644 index 00000000..31735ff0 --- /dev/null +++ b/primed/collaborative_analysis/urls.py @@ -0,0 +1,21 @@ +from django.urls import include, path + +from . import views + +app_name = "collaborative_analysis" + + +collaborative_analysis_workspace_patterns = ( + [ + path( + "//audit/", + views.WorkspaceAudit.as_view(), + name="audit", + ), + ], + "workspaces", +) + +urlpatterns = [ + path("workspaces/", include(collaborative_analysis_workspace_patterns)), +] diff --git a/primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html b/primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html new file mode 100644 index 00000000..394609db --- /dev/null +++ b/primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html @@ -0,0 +1,53 @@ +{% extends "anvil_consortium_manager/base.html" %} + +{% block title %}dbGaP workspace audit{% endblock %} + + +{% block content %} + +

Collaborative analysis workspace audit

+ +
+ +
+ +

Audit results

+ +
+

+ To have access to a Collaborative Analysis Workspace, an account must meet both of the following criteria: + +

    +
  • Be in the analyst group associated with the workspace
    • +
  • Be in the auth domain for all source workspaces
    • +
+

+

The audit result categories are explained below. +

    + +
  • Verified includes the following:
    • +
      +
    • An account in the analyst group is in the auth domain for this workspace and is in all auth domains for all source workspaces.
      • +
    • An account in the analyst group is not in the auth domain for this workspace and is not in all auth domains for all source workspaces.
      • +
    + +
  • Needs action includes the following:
    • +
      +
    • An account in the analyst group is not in the auth domain for this workspace and is in all auth domains for all source workspaces.
      • +
    • An account in the analyst group is in the auth domain for this workspace and is not in all auth domains for all source workspaces.
      • +
    + +
  • Errors
    • +
      +
    • A group is unexpectedly in the auth domain.
      • +
    +
+

+

Any errors should be reported!

+
+ +{% include "__audit_tables.html" with verified_table=verified_table needs_action_table=needs_action_table errors_table=errors_table %} + +{% endblock content %} From a938b2691995057d803fea64368b28a22c1ed1df Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 19 Jan 2024 15:51:04 -0800 Subject: [PATCH 3/4] Add no cover to specific docstring lines These lines are not code, just documentation of a variable, so they shouldn't count against our coverage metric. --- primed/cdsa/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/primed/cdsa/models.py b/primed/cdsa/models.py index a055eef8..a2b1e0dc 100644 --- a/primed/cdsa/models.py +++ b/primed/cdsa/models.py @@ -99,17 +99,17 @@ class SignedAgreementStatusMixin: class StatusChoices(models.TextChoices): ACTIVE = "active", "Active" - """SignedAgreements that are currently active.""" + """SignedAgreements that are currently active.""" # pragma: no cover WITHDRAWN = "withdrawn", "Withdrawn" """SignedAgreements that have been withdrawn for some reason (e.g., PI changed institution, study no longer wanted to participate.)""" LAPSED = "lapsed", "Lapsed" - """SignedAgreements from a AgreementMajorVersion that is no longer valid.""" + """SignedAgreements from a AgreementMajorVersion that is no longer valid.""" # pragma: no cover REPLACED = "replaced", "Replaced" - """SignedAgreements that have been replaced by a newer version.""" + """SignedAgreements that have been replaced by a newer version.""" # pragma: no cover STATUS = StatusChoices.choices From 6b8d3efde0daf87109cda030e491e8a830e60871 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 19 Jan 2024 16:15:22 -0800 Subject: [PATCH 4/4] Remove accidentally committed files --- primed/collaborative_analysis/urls.py | 21 -------- .../collaborativeanalysisworkspace_audit.html | 53 ------------------- 2 files changed, 74 deletions(-) delete mode 100644 primed/collaborative_analysis/urls.py delete mode 100644 primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html diff --git a/primed/collaborative_analysis/urls.py b/primed/collaborative_analysis/urls.py deleted file mode 100644 index 31735ff0..00000000 --- a/primed/collaborative_analysis/urls.py +++ /dev/null @@ -1,21 +0,0 @@ -from django.urls import include, path - -from . import views - -app_name = "collaborative_analysis" - - -collaborative_analysis_workspace_patterns = ( - [ - path( - "//audit/", - views.WorkspaceAudit.as_view(), - name="audit", - ), - ], - "workspaces", -) - -urlpatterns = [ - path("workspaces/", include(collaborative_analysis_workspace_patterns)), -] diff --git a/primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html b/primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html deleted file mode 100644 index 394609db..00000000 --- a/primed/templates/collaborative_analysis/collaborativeanalysisworkspace_audit.html +++ /dev/null @@ -1,53 +0,0 @@ -{% extends "anvil_consortium_manager/base.html" %} - -{% block title %}dbGaP workspace audit{% endblock %} - - -{% block content %} - -

Collaborative analysis workspace audit

- -
- -
- -

Audit results

- -
-

- To have access to a Collaborative Analysis Workspace, an account must meet both of the following criteria: - -

    -
  • Be in the analyst group associated with the workspace
    • -
  • Be in the auth domain for all source workspaces
    • -
-

-

The audit result categories are explained below. -

    - -
  • Verified includes the following:
    • -
      -
    • An account in the analyst group is in the auth domain for this workspace and is in all auth domains for all source workspaces.
      • -
    • An account in the analyst group is not in the auth domain for this workspace and is not in all auth domains for all source workspaces.
      • -
    - -
  • Needs action includes the following:
    • -
      -
    • An account in the analyst group is not in the auth domain for this workspace and is in all auth domains for all source workspaces.
      • -
    • An account in the analyst group is in the auth domain for this workspace and is not in all auth domains for all source workspaces.
      • -
    - -
  • Errors
    • -
      -
    • A group is unexpectedly in the auth domain.
      • -
    -
-

-

Any errors should be reported!

-
- -{% include "__audit_tables.html" with verified_table=verified_table needs_action_table=needs_action_table errors_table=errors_table %} - -{% endblock content %}