Merge pull request #627 from UW-GAC/feature/harmonize-dbgap-cdsa-updates

Harmonize dbgap and cdsa updates

add_cdsa_example_data.py

@@ -37,11 +37,28 @@
 dum = DataUseModifier.objects.get(abbreviation="NPU")
 
 # Create some study sites.
-StudySiteFactory.create(short_name="CC", full_name="Coordinating Center")
-StudySiteFactory.create(short_name="CARDINAL", full_name="CARDINAL")
+try:
+    cc = StudySite.objects.get(short_name="CC")
+except StudySite.DoesNotExist:
+    cc = StudySiteFactory.create(short_name="CC", full_name="Coordinating Center")
+try:
+    cardinal = StudySite.objects.get(short_name="CARDINAL")
+except StudySite.DoesNotExist:
+    cardinal = StudySiteFactory.create(short_name="CARDINAL", full_name="CARDINAL")
+
 # Create some studies.
-StudyFactory.create(short_name="Amish", full_name="Amish")
-StudyFactory.create(short_name="MESA", full_name="MESA")
+try:
+    amish = Study.objects.get(short_name="Amish")
+except Study.DoesNotExist:
+    amish = StudyFactory.create(short_name="Amish", full_name="Amish")
+try:
+    mesa = Study.objects.get(short_name="MESA")
+except Study.DoesNotExist:
+    mesa = StudyFactory.create(short_name="MESA", full_name="MESA")
+try:
+    aric = Study.objects.get(short_name="ARIC")
+except Study.DoesNotExist:
+    aric = Study.objects.create(short_name="ARIC", full_name="Atherosclerosis Risk in Communities")
 
 # Create some CDSAs
 cdsa_1001 = factories.MemberAgreementFactory.create(
@@ -51,7 +68,7 @@
     signed_agreement__representative_role="Contact PI",
     is_primary=True,
     signed_agreement__version=v10,
-    study_site=StudySite.objects.get(short_name="CC"),
+    study_site=cc,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1001.signed_agreement.anvil_access_group)
 
@@ -62,7 +79,7 @@
     signed_agreement__representative_role="Contact PI",
     is_primary=True,
     signed_agreement__version=v10,
-    study_site=StudySite.objects.get(short_name="CARDINAL"),
+    study_site=cardinal,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1002.signed_agreement.anvil_access_group)
 
@@ -73,7 +90,7 @@
     signed_agreement__representative_role="Co-PI",
     is_primary=False,
     signed_agreement__version=v10,
-    study_site=StudySite.objects.get(short_name="CARDINAL"),
+    study_site=cardinal,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1003.signed_agreement.anvil_access_group)
 
@@ -84,7 +101,7 @@
     signed_agreement__representative_role="Co-I",
     is_primary=False,
     signed_agreement__version=v11,
-    study_site=StudySite.objects.get(short_name="CARDINAL"),
+    study_site=cardinal,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1004.signed_agreement.anvil_access_group)
 
@@ -93,7 +110,7 @@
     signed_agreement__representative=User.objects.get(name="Brackie Mitchell"),
     signed_agreement__signing_institution="UMaryland",
     signed_agreement__representative_role="Study PI",
-    study=Study.objects.get(short_name="Amish"),
+    study=amish,
     signed_agreement__version=v10,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1005.signed_agreement.anvil_access_group)
@@ -103,7 +120,7 @@
     signed_agreement__representative=UserFactory.create(name="Robyn"),
     signed_agreement__representative_role="DCC PI",
     signed_agreement__signing_institution="UW",
-    study=Study.objects.get(short_name="MESA"),
+    study=mesa,
     signed_agreement__version=v10,
     additional_limitations="This data can only be used for testing the app.",
     requires_study_review=True,
@@ -116,7 +133,7 @@
     signed_agreement__signing_institution="JHU",
     signed_agreement__representative_role="Field Center PI",
     is_primary=False,
-    study=Study.objects.get(short_name="MESA"),
+    study=mesa,
     signed_agreement__version=v10,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1007.signed_agreement.anvil_access_group)
@@ -127,7 +144,7 @@
     signed_agreement__signing_institution="Lundquist",
     signed_agreement__representative_role="Analysis Center PI",
     is_primary=False,
-    study=Study.objects.get(short_name="MESA"),
+    study=mesa,
     signed_agreement__version=v10,
 )
 GroupGroupMembershipFactory.create(parent_group=cdsa_group, child_group=cdsa_1008.signed_agreement.anvil_access_group)
@@ -209,7 +226,7 @@
 cdsa_workspace_1 = factories.CDSAWorkspaceFactory.create(
     workspace__billing_project__name="demo-primed-cdsa",
     workspace__name="DEMO_PRIMED_CDSA_MESA_1",
-    study=Study.objects.get(short_name="MESA"),
+    study=mesa,
     data_use_permission=dup,
 )
 GroupGroupMembershipFactory.create(
@@ -222,7 +239,7 @@
 cdsa_workspace_2 = factories.CDSAWorkspaceFactory.create(
     workspace__billing_project__name="demo-primed-cdsa",
     workspace__name="DEMO_PRIMED_CDSA_MESA_2",
-    study=Study.objects.get(short_name="MESA"),
+    study=mesa,
     data_use_permission=dup,
     additional_limitations="Additional limitations for workspace.",
 )
@@ -233,6 +250,6 @@
 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"),
+    study=aric,
     data_use_permission=dup,
 )

add_dbgap_example_data.py

@@ -1,17 +1,30 @@
 # Temporary script to create some test data.
 # Run with: python manage.py shell < add_cdsa_example_data.py
 
-from anvil_consortium_manager.tests.factories import GroupGroupMembershipFactory
+from anvil_consortium_manager.tests.factories import (
+    AccountFactory,
+    GroupAccountMembershipFactory,
+    GroupGroupMembershipFactory,
+)
 
 from primed.dbgap import models
 from primed.dbgap.tests import factories
 from primed.primed_anvil.tests.factories import StudyFactory
 from primed.users.tests.factories import UserFactory
 
 # Studies
-fhs = StudyFactory.create(short_name="FHS", full_name="Framingham Heart Study")
-mesa = StudyFactory.create(short_name="MESA", full_name="Multi-Ethnic Study of Atherosclerosis")
-aric = StudyFactory.create(short_name="ARIC", full_name="Atherosclerosis Risk in Communities")
+try:
+    fhs = models.Study.objects.get(short_name="FHS")
+except models.Study.DoesNotExist:
+    fhs = StudyFactory.create(short_name="FHS", full_name="Framingham Heart Study")
+try:
+    mesa = models.Study.objects.get(short_name="MESA")
+except models.Study.DoesNotExist:
+    mesa = StudyFactory.create(short_name="MESA", full_name="Multi-Ethnic Study of Atherosclerosis")
+try:
+    aric = models.Study.objects.get(short_name="ARIC")
+except models.Study.DoesNotExist:
+    aric = StudyFactory.create(short_name="ARIC", full_name="Atherosclerosis Risk in Communities")
 
 # dbGaP study accessions
 dbgap_study_accession_fhs = factories.dbGaPStudyAccessionFactory.create(dbgap_phs=7, studies=[fhs])
@@ -124,3 +137,24 @@
     parent_group=workspace_fhs_1.workspace.authorization_domains.first(),
     child_group=dbgap_application_1.anvil_access_group,
 )
+
+# Add the PI to the access group for the first application.
+GroupAccountMembershipFactory.create(
+    group=dbgap_application_1.anvil_access_group,
+    account__user=dbgap_application_1.principal_investigator,
+)
+# Add some collaborators for the first application.
+users = UserFactory.create_batch(3)
+dbgap_application_1.collaborators.add(users[0])
+GroupAccountMembershipFactory.create(
+    group=dbgap_application_1.anvil_access_group,
+    account__user=users[0],
+)
+# # Do not add as an collaborator so we can check auditing.
+# dbgap_application_1.collaborators.add(users[1])
+GroupAccountMembershipFactory.create(
+    group=dbgap_application_1.anvil_access_group,
+    account__user=users[1],
+)
+dbgap_application_1.collaborators.add(users[2])
+AccountFactory.create(user=users[2], verified=True)

primed/cdsa/tests/test_views.py

@@ -2879,7 +2879,7 @@ def test_response_is_primary(self):
         self.assertContains(response, "Primary?")
         self.assertContains(
             response,
-            """<dt class="col-sm-2">Primary?</dt><dd class="col-sm-9">Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i></dd>""",  # noqa: E501
+            """<dt class="col-sm-3">Primary?</dt><dd class="col-sm-9">Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i></dd>""",  # noqa: E501
             html=True,
         )
         instance.is_primary = False
@@ -2888,7 +2888,7 @@ def test_response_is_primary(self):
         self.assertContains(response, "Primary?")
         self.assertContains(
             response,
-            """<dt class="col-sm-2">Primary?</dt><dd class="col-sm-9">No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i></dd>""",  # noqa: E501
+            """<dt class="col-sm-3">Primary?</dt><dd class="col-sm-9">No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i></dd>""",  # noqa: E501
             html=True,
         )
 
@@ -4698,7 +4698,7 @@ def test_response_is_primary(self):
         self.assertContains(response, "Primary?")
         self.assertContains(
             response,
-            """<dt class="col-sm-2">Primary?</dt><dd class="col-sm-9">Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i></dd>""",  # noqa: E501
+            """<dt class="col-sm-3">Primary?</dt><dd class="col-sm-9">Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i></dd>""",  # noqa: E501
             html=True,
         )
         instance.is_primary = False
@@ -4707,7 +4707,7 @@ def test_response_is_primary(self):
         self.assertContains(response, "Primary?")
         self.assertContains(
             response,
-            """<dt class="col-sm-2">Primary?</dt><dd class="col-sm-9">No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i></dd>""",  # noqa: E501
+            """<dt class="col-sm-3">Primary?</dt><dd class="col-sm-9">No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i></dd>""",  # noqa: E501
             html=True,
         )
 
@@ -4720,7 +4720,7 @@ def test_response_requires_study_review(self):
         # import ipdb; ipdb.set_trace()
         self.assertContains(
             response,
-            """<dt class="col-sm-2">Study review required?</dt> <dd class="col-sm-9">Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i></dd>""",  # noqa: E501
+            """<dt class="col-sm-3">Study review required?</dt> <dd class="col-sm-9">Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i></dd>""",  # noqa: E501
             html=True,
         )
         instance.requires_study_review = False
@@ -4729,7 +4729,7 @@ def test_response_requires_study_review(self):
         self.assertContains(response, "Study review required?")
         self.assertContains(
             response,
-            """<dt class="col-sm-2">Study review required?</dt> <dd class="col-sm-9">No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i></dd>""",  # noqa: E501
+            """<dt class="col-sm-3">Study review required?</dt> <dd class="col-sm-9">No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i></dd>""",  # noqa: E501
             html=True,
         )
 

primed/dbgap/tests/test_views.py

@@ -1452,6 +1452,7 @@ def test_staff_edit_links(self):
         self.assertContains(
             response, reverse("anvil_consortium_manager:managed_groups:detail", args=[self.obj.anvil_access_group.name])
         )
+        self.assertContains(response, reverse("dbgap:dbgap_applications:update", args=[self.obj.dbgap_project_id]))
 
     def test_staff_view_links(self):
         """No edit links if staff user only has view permission."""
@@ -1475,6 +1476,7 @@ def test_staff_view_links(self):
         self.assertContains(
             response, reverse("anvil_consortium_manager:managed_groups:detail", args=[self.obj.anvil_access_group.name])
         )
+        self.assertNotContains(response, reverse("dbgap:dbgap_applications:update", args=[self.obj.dbgap_project_id]))
 
     def test_links_pi(self):
         """Links seen by PI are correct."""
@@ -1500,6 +1502,7 @@ def test_links_pi(self):
         self.assertNotContains(
             response, reverse("anvil_consortium_manager:managed_groups:detail", args=[self.obj.anvil_access_group.name])
         )
+        self.assertNotContains(response, reverse("dbgap:dbgap_applications:update", args=[self.obj.dbgap_project_id]))
 
     def test_links_collaborators(self):
         """Links seen by collaborators are correct."""
@@ -1526,6 +1529,7 @@ def test_links_collaborators(self):
         self.assertNotContains(
             response, reverse("anvil_consortium_manager:managed_groups:detail", args=[self.obj.anvil_access_group.name])
         )
+        self.assertNotContains(response, reverse("dbgap:dbgap_applications:update", args=[self.obj.dbgap_project_id]))
 
     def test_table_classes(self):
         """The table classes are correct."""

primed/dbgap/views.py

@@ -246,7 +246,8 @@ class dbGaPApplicationUpdate(AnVILConsortiumManagerStaffEditRequired, SuccessMes
 
     model = models.dbGaPApplication
     form_class = forms.dbGaPApplicationUpdateForm
-    success_message = "dbGaP application successfully updated."
+    success_message = "dbGaP collaborators successfully updated."
+    template_name = "dbgap/dbgapapplication_update_collaborators.html"
 
     def get_object(self, queryset=None):
         queryset = self.get_queryset()

primed/templates/cdsa/dataaffiliateagreement_detail.html

@@ -14,51 +14,51 @@
 
 {% block panel %}
 <dl class="row">
-  <dt class="col-sm-2">Coordinating Center ID</dt> <dd class="col-sm-9">{{ object.signed_agreement.cc_id }}</dd>
-  <dt class="col-sm-2">Representative</dt> <dd class="col-sm-9">
+  <dt class="col-sm-3">Coordinating Center ID</dt> <dd class="col-sm-9">{{ object.signed_agreement.cc_id }}</dd>
+  <dt class="col-sm-3">Representative</dt> <dd class="col-sm-9">
     {% if perms.anvil_consortium_manager.anvil_consortium_manager_staff_view %}
       <a href="{{ object.signed_agreement.representative.get_absolute_url }}">{{ object.signed_agreement.representative.name }}</a>
     {% else %}
       {{ object.signed_agreement.representative.name }}
     {% endif %}
   </dd>
-  <dt class="col-sm-2">Representative role</dt> <dd class="col-sm-9">{{ object.signed_agreement.representative_role }}</dd>
-  <dt class="col-sm-2">Signing institution</dt> <dd class="col-sm-9">{{ object.signed_agreement.signing_institution }}</dd>
-  <dt class="col-sm-2">Primary?</dt> <dd class="col-sm-9">
+  <dt class="col-sm-3">Representative role</dt> <dd class="col-sm-9">{{ object.signed_agreement.representative_role }}</dd>
+  <dt class="col-sm-3">Signing institution</dt> <dd class="col-sm-9">{{ object.signed_agreement.signing_institution }}</dd>
+  <dt class="col-sm-3">Primary?</dt> <dd class="col-sm-9">
     {% if object.is_primary %}
       Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i>
     {% else %}
       No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i>
     {% endif %}
   </dd>
-  <dt class="col-sm-2">Study review required?</dt> <dd class="col-sm-9">
+  <dt class="col-sm-3">Study review required?</dt> <dd class="col-sm-9">
     {% if object.requires_study_review %}
       Yes <i class="bi bi-check-circle-fill px-2" style="color: green;"></i>
     {% else %}
       No <i class="bi bi-x-circle-fill px-2" style="color: red;"></i>
     {% endif %}
   </dd>
-  <dt class="col-sm-2">Agreement version</dt> <dd class="col-sm-9">
+  <dt class="col-sm-3">Agreement version</dt> <dd class="col-sm-9">
     {% if perms.anvil_consortium_manager.anvil_consortium_manager_staff_view %}
       <a href="{{ object.signed_agreement.version.get_absolute_url }}">{{ object.signed_agreement.version }}</a>
     {% else %}
       {{ object.signed_agreement.version }}
     {% endif %}
   </dd>
-  <dt class="col-sm-2">Status</dt> <dd class="col-sm-9">{{ object.signed_agreement.get_status_display }}</dd>
-  <dt class="col-sm-2">Date signed</dt> <dd class="col-sm-9">{{ object.signed_agreement.date_signed }}</dd>
+  <dt class="col-sm-3">Status</dt> <dd class="col-sm-9">{{ object.signed_agreement.get_status_display }}</dd>
+  <dt class="col-sm-3">Date signed</dt> <dd class="col-sm-9">{{ object.signed_agreement.date_signed }}</dd>
 
-  <dt class="col-sm-2">Study</dt> <dd class="col-sm-9">
+  <dt class="col-sm-3">Study</dt> <dd class="col-sm-9">
     <a href="{{ object.study.get_absolute_url }}">{{ object.study }}</a>
   </dd>
-  <dt class="col-sm-2">AnVIL access group</dt><dd class="col-sm-9">
+  <dt class="col-sm-3">AnVIL access group</dt><dd class="col-sm-9">
     {% if perms.anvil_consortium_manager.anvil_consortium_manager_staff_view %}
       <a href="{{ object.signed_agreement.anvil_access_group.get_absolute_url }}">{{ object.signed_agreement.anvil_access_group }}</a>
     {% else %}
       {{ object.signed_agreement.anvil_access_group }}
     {% endif %}
   </dd>
-  <dt class="col-sm-2">AnVIL upload group</dt><dd class="col-sm-9">
+  <dt class="col-sm-3">AnVIL upload group</dt><dd class="col-sm-9">
     {% if perms.anvil_consortium_manager.anvil_consortium_manager_staff_view %}
       <a href="{{ object.anvil_upload_group.get_absolute_url }}">{{ object.anvil_upload_group }}</a>
     {% else %}
@@ -68,8 +68,8 @@
 
   {% if perms.anvil_consortium_manager.anvil_consortium_manager_staff_view %}
   <hr>
-  <dt class="col-sm-2">Date created</dt> <dd class="col-sm-9">{{ object.created }}</dd>
-  <dt class="col-sm-2">Date modified</dt> <dd class="col-sm-9">{{ object.modified }}</dd>
+  <dt class="col-sm-3">Date created</dt> <dd class="col-sm-9">{{ object.created }}</dd>
+  <dt class="col-sm-3">Date modified</dt> <dd class="col-sm-9">{{ object.modified }}</dd>
   {% endif %}
 
 </dl>
@@ -112,11 +112,12 @@ <h2 class="accordion-header" id="headingAccessorsOne">
             This table shows accessors named for this signed agreement, their AnVIL account (if linked), and whether or not they are a member of the access group on AnVIL.
             Only accessors who are PRIMED members are included.
             An accessor must have linked their AnVIL account to be granted access to data on AnVIL.
-            If changes need to be made, please contact the CC.
           </p>
 
           {% render_table tables.0 %}
 
+          <p class='alert alert-secondary'><i class="bi bi-question-circle-fill"></i> If accessors need to be added or removed, please contact the CC at <a href="mailto:{{ DCC_CONTACT_EMAIL }}">{{ DCC_CONTACT_EMAIL }}</a></p>
+
         </div>
       </div>
     </div>
@@ -140,11 +141,12 @@ <h2 class="accordion-header" id="headingUploadersOne">
             This table shows uploaders named for this signed agreement, their AnVIL account (if linked), and whether or not they are a member of the upload group on AnVIL.
             Only uploaders who are PRIMED members are included.
             An uploaders must have linked their AnVIL account to be granted access to data on AnVIL.
-            If changes need to be made, please contact the CC.
           </p>
 
           {% render_table tables.1 %}
 
+          <p class='alert alert-secondary'><i class="bi bi-question-circle-fill"></i> If uploaders need to be added or removed, please contact the CC at <a href="mailto:{{ DCC_CONTACT_EMAIL }}">{{ DCC_CONTACT_EMAIL }}</a></p>
+
         </div>
       </div>
     </div>