From 6e572b71240e055f682cce36e66a40bd07de14d1 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 31 Jan 2024 14:00:57 -0800 Subject: [PATCH 1/5] Add a new url pattern for DARs Move the current DAR list view into this pattern, and out of the dbgap_applications pattern. This is in preparation for adding the history of a given DAR. --- primed/dbgap/tests/test_views.py | 2 +- primed/dbgap/urls.py | 9 ++++++++- primed/templates/dbgap/dbgapdataaccessrequest_list.html | 2 +- primed/templates/dbgap/nav_items.html | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/primed/dbgap/tests/test_views.py b/primed/dbgap/tests/test_views.py index 442c237b..bf103bfa 100644 --- a/primed/dbgap/tests/test_views.py +++ b/primed/dbgap/tests/test_views.py @@ -3670,7 +3670,7 @@ def setUp(self): def get_url(self, *args): """Get the url for the view being tested.""" - return reverse("dbgap:dbgap_applications:dars", args=args) + return reverse("dbgap:dars:current", args=args) def get_view(self): """Return the view being tested.""" diff --git a/primed/dbgap/urls.py b/primed/dbgap/urls.py index e49c633a..b88a32ed 100644 --- a/primed/dbgap/urls.py +++ b/primed/dbgap/urls.py @@ -41,6 +41,12 @@ "dbgap_data_access_snapshots", ) +data_access_request_patterns = ( + [ + path("current/", views.dbGaPDataAccessRequestList.as_view(), name="current"), + ], + "dars", +) dbgap_application_patterns = ( [ path("", views.dbGaPApplicationList.as_view(), name="list"), @@ -50,7 +56,7 @@ views.dbGaPDataAccessSnapshotCreateMultiple.as_view(), name="update_dars", ), - path("dars/", views.dbGaPDataAccessRequestList.as_view(), name="dars"), + # path("dars/", views.dbGaPDataAccessRequestList.as_view(), name="dars"), path( "/", views.dbGaPApplicationDetail.as_view(), @@ -93,6 +99,7 @@ urlpatterns = [ path("studies/", include(dbgap_study_accession_patterns)), path("applications/", include(dbgap_application_patterns)), + path("dars/", include(data_access_request_patterns)), path("workspaces/", include(dbgap_workspace_patterns)), path("records/", include(records_patterns)), ] diff --git a/primed/templates/dbgap/dbgapdataaccessrequest_list.html b/primed/templates/dbgap/dbgapdataaccessrequest_list.html index c83a407a..b4fcb197 100644 --- a/primed/templates/dbgap/dbgapdataaccessrequest_list.html +++ b/primed/templates/dbgap/dbgapdataaccessrequest_list.html @@ -7,7 +7,7 @@

Current dbGaP data access requests

- Export + Export
{% render_table table %} diff --git a/primed/templates/dbgap/nav_items.html b/primed/templates/dbgap/nav_items.html index 1a2c8935..3b1c7f23 100644 --- a/primed/templates/dbgap/nav_items.html +++ b/primed/templates/dbgap/nav_items.html @@ -18,7 +18,7 @@ List dbGaP applications
  • - List all current DARs + List all current DARs
    • {% if perms.anvil_consortium_manager.anvil_consortium_manager_staff_edit %}
  • From cd3b6941af567e4cb7613edbdcd7a7f5fec13da2 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 31 Jan 2024 14:33:57 -0800 Subject: [PATCH 2/5] Add a view showing the history of a given DAR --- primed/dbgap/tests/test_views.py | 114 ++++++++++++++++++ primed/dbgap/urls.py | 5 + primed/dbgap/views.py | 35 ++++++ .../dbgap/dbgapdataaccessrequest_history.html | 11 ++ 4 files changed, 165 insertions(+) create mode 100644 primed/templates/dbgap/dbgapdataaccessrequest_history.html diff --git a/primed/dbgap/tests/test_views.py b/primed/dbgap/tests/test_views.py index bf103bfa..cd853578 100644 --- a/primed/dbgap/tests/test_views.py +++ b/primed/dbgap/tests/test_views.py @@ -3751,6 +3751,120 @@ def test_export(self): ) +class dbGaPDataAccessRequestHistoryTest(TestCase): + def setUp(self): + """Set up test class.""" + self.factory = RequestFactory() + # Create a user with both view and edit permission. + self.user = User.objects.create_user(username="test", password="test") + self.user.user_permissions.add( + Permission.objects.get( + codename=AnVILProjectManagerAccess.STAFF_VIEW_PERMISSION_CODENAME + ) + ) + + def get_url(self, *args): + """Get the url for the view being tested.""" + return reverse("dbgap:dars:history", args=args) + + def get_view(self): + """Return the view being tested.""" + return views.dbGaPDataAccessRequestHistory.as_view() + + def test_view_redirect_not_logged_in(self): + "View redirects to login view when user is not logged in." + # Need a client for redirects. + response = self.client.get(self.get_url(1)) + self.assertRedirects( + response, + resolve_url(settings.LOGIN_URL) + "?next=" + self.get_url(1), + ) + + def test_status_code_with_user_permission(self): + """Returns successful response code.""" + instance = factories.dbGaPDataAccessRequestFactory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.dbgap_dar_id)) + self.assertEqual(response.status_code, 200) + + def test_access_without_user_permission(self): + """Raises permission denied if user has no permissions.""" + user_no_perms = User.objects.create_user( + username="test-none", password="test-none" + ) + request = self.factory.get(self.get_url(1)) + request.user = user_no_perms + with self.assertRaises(PermissionDenied): + self.get_view()(request, 1) + + def test_dbgap_dar_id_does_not_exist(self): + """Raises permission denied if user has no permissions.""" + request = self.factory.get(self.get_url(1)) + request.user = self.user + with self.assertRaises(Http404): + self.get_view()(request, 1) + + def test_table_class(self): + """The table is the correct class.""" + instance = factories.dbGaPDataAccessRequestFactory.create() + self.client.force_login(self.user) + response = self.client.get(self.get_url(instance.dbgap_dar_id)) + self.assertIn("table", response.context_data) + self.assertIsInstance( + response.context_data["table"], tables.dbGaPDataAccessRequestTable + ) + + def test_one_dars(self): + """Table displays two dars.""" + dar = factories.dbGaPDataAccessRequestFactory.create(dbgap_dar_id=1) + self.client.force_login(self.user) + response = self.client.get(self.get_url(1)) + self.assertEqual(len(response.context_data["table"].rows), 1) + self.assertIn(dar, response.context_data["table"].data) + + def test_two_dars(self): + """Table displays two dars.""" + dar_1 = factories.dbGaPDataAccessRequestFactory.create(dbgap_dar_id=1) + dar_2 = factories.dbGaPDataAccessRequestFactory.create(dbgap_dar_id=1) + self.client.force_login(self.user) + response = self.client.get(self.get_url(1)) + self.assertEqual(len(response.context_data["table"].rows), 2) + self.assertIn(dar_1, response.context_data["table"].data) + self.assertIn(dar_2, response.context_data["table"].data) + + def test_matching_dbgap_dar_id(self): + """Only DARs with the same dbgap_dar_id are in the table.""" + dar_1 = factories.dbGaPDataAccessRequestFactory.create(dbgap_dar_id=1) + dar_2 = factories.dbGaPDataAccessRequestFactory.create(dbgap_dar_id=2) + self.client.force_login(self.user) + response = self.client.get(self.get_url(1)) + self.assertEqual(len(response.context_data["table"].rows), 1) + self.assertIn(dar_1, response.context_data["table"].data) + self.assertNotIn(dar_2, response.context_data["table"].data) + + def test_table_ordering(self): + """DARs from the most recent snapshot appear first.""" + old_snapshot = factories.dbGaPDataAccessSnapshotFactory.create( + created=timezone.now() - timedelta(weeks=5), + is_most_recent=False, + ) + dar_1 = factories.dbGaPDataAccessRequestFactory.create( + dbgap_dar_id=1, dbgap_data_access_snapshot=old_snapshot + ) + new_snapshot = factories.dbGaPDataAccessSnapshotFactory.create( + created=timezone.now() - timedelta(weeks=1), + is_most_recent=True, + ) + dar_2 = factories.dbGaPDataAccessRequestFactory.create( + dbgap_dar_id=1, dbgap_data_access_snapshot=new_snapshot + ) + self.client.force_login(self.user) + response = self.client.get(self.get_url(1)) + self.assertEqual(len(response.context_data["table"].rows), 2) + self.assertEqual(dar_2, response.context_data["table"].rows[0].record) + self.assertEqual(dar_1, response.context_data["table"].rows[1].record) + + class dbGaPApplicationAuditTest(TestCase): """Tests for the dbGaPApplicationAudit view.""" diff --git a/primed/dbgap/urls.py b/primed/dbgap/urls.py index b88a32ed..b79bdc07 100644 --- a/primed/dbgap/urls.py +++ b/primed/dbgap/urls.py @@ -44,6 +44,11 @@ data_access_request_patterns = ( [ path("current/", views.dbGaPDataAccessRequestList.as_view(), name="current"), + path( + "history/", + views.dbGaPDataAccessRequestHistory.as_view(), + name="history", + ), ], "dars", ) diff --git a/primed/dbgap/views.py b/primed/dbgap/views.py index be06d329..2f2eaf9f 100644 --- a/primed/dbgap/views.py +++ b/primed/dbgap/views.py @@ -517,6 +517,41 @@ def get_table_data(self): ) +class dbGaPDataAccessRequestHistory( + AnVILConsortiumManagerStaffViewRequired, ExportMixin, SingleTableView +): + """View to show the history of a given DAR.""" + + model = models.dbGaPDataAccessRequest + table_class = tables.dbGaPDataAccessRequestTable + template_name = "dbgap/dbgapdataaccessrequest_history.html" + + def get_dbgap_dar_id(self): + return self.kwargs.get("dbgap_dar_id") + + def get(self, request, *args, **kwargs): + self.dbgap_dar_id = self.get_dbgap_dar_id() + return super().get(request, *args, **kwargs) + + def get_table_data(self): + qs = self.get_queryset().filter( + dbgap_dar_id=self.dbgap_dar_id, + ) + if not qs.count(): + raise Http404("No DARs found matching the query.") + return qs + + def get_table_kwargs(self): + return { + "order_by": "-dbgap_data_access_snapshot__created", + } + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["dbgap_dar_id"] = self.dbgap_dar_id + return context + + class dbGaPApplicationAudit(AnVILConsortiumManagerStaffViewRequired, DetailView): """View to show audit results for a `dbGaPApplication`.""" diff --git a/primed/templates/dbgap/dbgapdataaccessrequest_history.html b/primed/templates/dbgap/dbgapdataaccessrequest_history.html new file mode 100644 index 00000000..45181e6a --- /dev/null +++ b/primed/templates/dbgap/dbgapdataaccessrequest_history.html @@ -0,0 +1,11 @@ +{% extends "anvil_consortium_manager/base.html" %} +{% load render_table from django_tables2 %} + +{% block title %}dbGaP DAR history{% endblock %} + +{% block content %} +

    dbGaP DAR history: {{ dbgap_dar_id }}

    + +{% render_table table %} + +{% endblock content %} From 388098c5960c3b7e1c38045c8d3b15231bc8759c Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 31 Jan 2024 15:04:37 -0800 Subject: [PATCH 3/5] Add a specific table for DAR history --- primed/dbgap/tables.py | 40 ++++++++++++++++++++++++++ primed/dbgap/tests/test_tables.py | 47 +++++++++++++++++++++++++++++++ primed/dbgap/tests/test_views.py | 2 +- primed/dbgap/views.py | 2 +- 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/primed/dbgap/tables.py b/primed/dbgap/tables.py index 34af7f27..fc89346d 100644 --- a/primed/dbgap/tables.py +++ b/primed/dbgap/tables.py @@ -339,6 +339,46 @@ class Meta: attrs = {"class": "table table-sm"} +class dbGaPDataAccessRequestHistoryTable(tables.Table): + """Class to render a table of dbGaPDataAccessRequest history by dbgap_dar_id.""" + + dbgap_data_access_snapshot__dbgap_application__dbgap_project_id = tables.columns.Column( + verbose_name=" dbGaP application", + linkify=lambda record: record.dbgap_data_access_snapshot.dbgap_application.get_absolute_url(), + ) + dbgap_dar_id = tables.columns.Column(verbose_name="DAR") + dbgap_dac = tables.columns.Column(verbose_name="DAC") + dbgap_accession = dbGaPAccessionColumn( + accessor="get_dbgap_accession", + verbose_name="Accession", + order_by=( + "dbgap_phs", + "original_version", + "original_participant_set", + ), + ) + dbgap_consent_abbreviation = tables.columns.Column(verbose_name="Consent") + dbgap_current_status = tables.columns.Column(verbose_name="Status") + dbgap_data_access_snapshot__created = tables.columns.DateTimeColumn( + verbose_name="Snapshot", + linkify=lambda record: record.dbgap_data_access_snapshot.get_absolute_url(), + ) + + class Meta: + model = models.dbGaPDataAccessRequest + fields = ( + "dbgap_dar_id", + "dbgap_data_access_snapshot__created", + "dbgap_current_status", + "dbgap_data_access_snapshot__dbgap_application__dbgap_project_id", + "dbgap_dac", + "dbgap_accession", + "dbgap_consent_abbreviation", + ) + order_by = ("-dbgap_data_access_snapshot__created",) + attrs = {"class": "table table-sm"} + + class dbGaPApplicationRecordsTable(tables.Table): """Class to render a publicly-viewable table of dbGaPApplication objects.""" diff --git a/primed/dbgap/tests/test_tables.py b/primed/dbgap/tests/test_tables.py index 514a27fd..4c901a44 100644 --- a/primed/dbgap/tests/test_tables.py +++ b/primed/dbgap/tests/test_tables.py @@ -611,6 +611,53 @@ def test_ordering(self): self.assertEqual(table.data[3], instance_1) +class dbGaPDataAccessRequestHistoryTest(TestCase): + model = models.dbGaPDataAccessRequest + model_factory = factories.dbGaPDataAccessRequestFactory + table_class = tables.dbGaPDataAccessRequestHistoryTable + + def test_row_count_with_no_objects(self): + table = self.table_class(self.model.objects.all()) + self.assertEqual(len(table.rows), 0) + + def test_row_count_with_one_object(self): + self.model_factory.create() + table = self.table_class(self.model.objects.all()) + self.assertEqual(len(table.rows), 1) + + def test_row_count_with_two_objects(self): + self.model_factory.create_batch(2) + table = self.table_class(self.model.objects.all()) + self.assertEqual(len(table.rows), 2) + + def test_ordering(self): + """Instances are ordered alphabetically by dbgap_application and dbgap_dar_id.""" + dbgap_snapshot_1 = factories.dbGaPDataAccessSnapshotFactory.create( + created=timezone.now() - timedelta(weeks=5), + ) + instance_1 = self.model_factory.create( + dbgap_data_access_snapshot=dbgap_snapshot_1, + ) + dbgap_snapshot_2 = factories.dbGaPDataAccessSnapshotFactory.create( + created=timezone.now() - timedelta(weeks=4) + ) + instance_2 = self.model_factory.create( + dbgap_dar_id=instance_1.dbgap_dar_id, + dbgap_data_access_snapshot=dbgap_snapshot_2, + ) + dbgap_snapshot_3 = factories.dbGaPDataAccessSnapshotFactory.create( + created=timezone.now() - timedelta(weeks=3) + ) + instance_3 = self.model_factory.create( + dbgap_dar_id=instance_1.dbgap_dar_id, + dbgap_data_access_snapshot=dbgap_snapshot_3, + ) + table = self.table_class(self.model.objects.all()) + self.assertEqual(table.data[0], instance_3) + self.assertEqual(table.data[1], instance_2) + self.assertEqual(table.data[2], instance_1) + + class dbGaPDataAccessRequestBySnapshotTableTest(TestCase): model = models.dbGaPDataAccessRequest model_factory = factories.dbGaPDataAccessRequestFactory diff --git a/primed/dbgap/tests/test_views.py b/primed/dbgap/tests/test_views.py index cd853578..24d6060a 100644 --- a/primed/dbgap/tests/test_views.py +++ b/primed/dbgap/tests/test_views.py @@ -3811,7 +3811,7 @@ def test_table_class(self): response = self.client.get(self.get_url(instance.dbgap_dar_id)) self.assertIn("table", response.context_data) self.assertIsInstance( - response.context_data["table"], tables.dbGaPDataAccessRequestTable + response.context_data["table"], tables.dbGaPDataAccessRequestHistoryTable ) def test_one_dars(self): diff --git a/primed/dbgap/views.py b/primed/dbgap/views.py index 2f2eaf9f..d52bc72b 100644 --- a/primed/dbgap/views.py +++ b/primed/dbgap/views.py @@ -523,7 +523,7 @@ class dbGaPDataAccessRequestHistory( """View to show the history of a given DAR.""" model = models.dbGaPDataAccessRequest - table_class = tables.dbGaPDataAccessRequestTable + table_class = tables.dbGaPDataAccessRequestHistoryTable template_name = "dbgap/dbgapdataaccessrequest_history.html" def get_dbgap_dar_id(self): From f5baffc43a2a32035513fc47c837ec7525a15517 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 31 Jan 2024 15:16:57 -0800 Subject: [PATCH 4/5] Rework dbGaPDataAccessRequest tables Instead of redefining the columns for DAR tables, instead define them once in the original dbGaPDataAccessRequestTable, then subclass that table for the other tables. The columns that need to change can be redfined (or set to None) in the child tables if necessary. Also add a link to the DAR history in some of the tables. --- primed/dbgap/tables.py | 73 ++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 49 deletions(-) diff --git a/primed/dbgap/tables.py b/primed/dbgap/tables.py index fc89346d..889fe934 100644 --- a/primed/dbgap/tables.py +++ b/primed/dbgap/tables.py @@ -237,7 +237,10 @@ class dbGaPDataAccessRequestTable(tables.Table): verbose_name=" dbGaP application", linkify=lambda record: record.dbgap_data_access_snapshot.dbgap_application.get_absolute_url(), ) - dbgap_dar_id = tables.columns.Column(verbose_name="DAR") + dbgap_dar_id = tables.columns.Column( + verbose_name="DAR", + linkify=("dbgap:dars:history", {"dbgap_dar_id": tables.A("dbgap_dar_id")}), + ) dbgap_dac = tables.columns.Column(verbose_name="DAC") dbgap_accession = dbGaPAccessionColumn( accessor="get_dbgap_accession", @@ -273,22 +276,11 @@ class Meta: attrs = {"class": "table table-sm"} -class dbGaPDataAccessRequestBySnapshotTable(tables.Table): +class dbGaPDataAccessRequestBySnapshotTable(dbGaPDataAccessRequestTable): """Class to render a table of dbGaPDataAccessRequest objects for a specific dbGaPDataAccessSnapshot.""" - dbgap_dar_id = tables.columns.Column(verbose_name="DAR") - dbgap_dac = tables.columns.Column(verbose_name="DAC") - dbgap_accession = dbGaPAccessionColumn( - accessor="get_dbgap_accession", - verbose_name="Accession", - order_by=( - "dbgap_phs", - "original_version", - "original_participant_set", - ), - ) - dbgap_consent_abbreviation = tables.columns.Column(verbose_name="Consent") - dbgap_current_status = tables.columns.Column(verbose_name="Current status") + dbgap_data_access_snapshot__dbgap_application__dbgap_project_id = None + dbgap_data_access_snapshot__created = None matching_workspaces = tables.columns.Column( accessor="get_dbgap_workspaces", orderable=False, default=" " ) @@ -326,42 +318,12 @@ def render_matching_workspaces(self, value, record): return html -class dbGaPDataAccessRequestSummaryTable(tables.Table): - """Table intended to show a summary of data access requests, grouped by DAC and current status.""" - - dbgap_dac = tables.columns.Column(attrs={"class": "col-auto"}) - dbgap_current_status = tables.columns.Column() - total = tables.columns.Column() - - class Meta: - model = models.dbGaPDataAccessRequest - fields = ("dbgap_dac", "dbgap_current_status", "total") - attrs = {"class": "table table-sm"} - - -class dbGaPDataAccessRequestHistoryTable(tables.Table): +class dbGaPDataAccessRequestHistoryTable(dbGaPDataAccessRequestTable): """Class to render a table of dbGaPDataAccessRequest history by dbgap_dar_id.""" - dbgap_data_access_snapshot__dbgap_application__dbgap_project_id = tables.columns.Column( - verbose_name=" dbGaP application", - linkify=lambda record: record.dbgap_data_access_snapshot.dbgap_application.get_absolute_url(), - ) - dbgap_dar_id = tables.columns.Column(verbose_name="DAR") - dbgap_dac = tables.columns.Column(verbose_name="DAC") - dbgap_accession = dbGaPAccessionColumn( - accessor="get_dbgap_accession", - verbose_name="Accession", - order_by=( - "dbgap_phs", - "original_version", - "original_participant_set", - ), - ) - dbgap_consent_abbreviation = tables.columns.Column(verbose_name="Consent") - dbgap_current_status = tables.columns.Column(verbose_name="Status") - dbgap_data_access_snapshot__created = tables.columns.DateTimeColumn( - verbose_name="Snapshot", - linkify=lambda record: record.dbgap_data_access_snapshot.get_absolute_url(), + dbgap_dar_id = tables.columns.Column( + verbose_name="DAR", + linkify=False, ) class Meta: @@ -379,6 +341,19 @@ class Meta: attrs = {"class": "table table-sm"} +class dbGaPDataAccessRequestSummaryTable(tables.Table): + """Table intended to show a summary of data access requests, grouped by DAC and current status.""" + + dbgap_dac = tables.columns.Column(attrs={"class": "col-auto"}) + dbgap_current_status = tables.columns.Column() + total = tables.columns.Column() + + class Meta: + model = models.dbGaPDataAccessRequest + fields = ("dbgap_dac", "dbgap_current_status", "total") + attrs = {"class": "table table-sm"} + + class dbGaPApplicationRecordsTable(tables.Table): """Class to render a publicly-viewable table of dbGaPApplication objects.""" From 51e21b996df76ccf6f9e9440380f7e00bb3efabc Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Wed, 31 Jan 2024 15:20:43 -0800 Subject: [PATCH 5/5] Add a small amount of explanatory text to template --- primed/templates/dbgap/dbgapdataaccessrequest_history.html | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/primed/templates/dbgap/dbgapdataaccessrequest_history.html b/primed/templates/dbgap/dbgapdataaccessrequest_history.html index 45181e6a..c0666335 100644 --- a/primed/templates/dbgap/dbgapdataaccessrequest_history.html +++ b/primed/templates/dbgap/dbgapdataaccessrequest_history.html @@ -6,6 +6,10 @@ {% block content %}

    dbGaP DAR history: {{ dbgap_dar_id }}

    +

    + The table below shows all snapshots that have a record of this DAR. +

    + {% render_table table %} {% endblock content %}