From af6f52fae7bf11ccb9f9bc389414b3eee7978d30 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Mon, 14 Mar 2022 17:52:51 +0100 Subject: [PATCH 1/5] allow to delete study domains --- .../templates/studies/domain_confirm_delete.html | 15 +++++++++++++++ .../templates/studies/study_domains_study.html | 3 +++ castellum/studies/urls/__init__.py | 6 ++++++ castellum/studies/views/studies.py | 12 ++++++++++++ .../studies/views/test_pseudonym_domains_view.py | 8 ++++++++ 5 files changed, 44 insertions(+) create mode 100644 castellum/studies/templates/studies/domain_confirm_delete.html diff --git a/castellum/studies/templates/studies/domain_confirm_delete.html b/castellum/studies/templates/studies/domain_confirm_delete.html new file mode 100644 index 000000000..c8a9b9705 --- /dev/null +++ b/castellum/studies/templates/studies/domain_confirm_delete.html @@ -0,0 +1,15 @@ +{% extends "studies/study_domains_base.html" %} +{% load i18n auth django_bootstrap5 utils %} + +{% block title %}{% translate "Delete" %} · {% translate "Study domains" %} · {{ block.super }}{% endblock %} + +{% block content %} +
+ {% csrf_token %} +

{% translate "Are you sure you want to permanently delete this domain and all related pseudonyms?" %}

+
+ {% translate 'Cancel' %} + +
+
+{% endblock %} diff --git a/castellum/studies/templates/studies/study_domains_study.html b/castellum/studies/templates/studies/study_domains_study.html index ad0e58211..6b1afc7ac 100644 --- a/castellum/studies/templates/studies/study_domains_study.html +++ b/castellum/studies/templates/studies/study_domains_study.html @@ -32,6 +32,9 @@
+
+ {% translate 'Delete' %} +
{% endfor %} diff --git a/castellum/studies/urls/__init__.py b/castellum/studies/urls/__init__.py index 243691e71..d0f066919 100644 --- a/castellum/studies/urls/__init__.py +++ b/castellum/studies/urls/__init__.py @@ -30,6 +30,7 @@ from ..views.studies import StudyCreateView from ..views.studies import StudyDeleteView from ..views.studies import StudyDetailView from ..views.studies import StudyDiffView +from ..views.studies import StudyDomainDeleteView from ..views.studies import StudyDomainsGeneralView from ..views.studies import StudyDomainsView from ..views.studies import StudyExportView @@ -52,6 +53,11 @@ urlpatterns = [ path('/delete/', StudyDeleteView.as_view(), name='delete'), path('/domains/', StudyDomainsView.as_view(), name='domains'), path('/domains/general', StudyDomainsGeneralView.as_view(), name='domains-general'), + path( + '/domains//', + StudyDomainDeleteView.as_view(), + name='domain-delete', + ), path('/start/', StudyStartRecruitmentView.as_view(), name='start'), path('/finish/', StudyFinishRecruitmentView.as_view(), name='finish'), path('/mail/', OneTimeInvitationMailRecruitmentView.as_view(), name='mail'), diff --git a/castellum/studies/views/studies.py b/castellum/studies/views/studies.py index 329533c91..237d21ec1 100644 --- a/castellum/studies/views/studies.py +++ b/castellum/studies/views/studies.py @@ -457,6 +457,18 @@ class StudyDomainsGeneralView(StudyDomainsMixin, UpdateView): return super().form_valid(form) +class StudyDomainDeleteView(StudyDomainsMixin, DeleteView): + model = Domain + template_name = 'studies/domain_confirm_delete.html' + subtab = 'domains-study' + + def get_object(self): + return get_object_or_404(self.study.domains.all(), key=self.kwargs['key']) + + def get_success_url(self): + return reverse('studies:domains', args=[self.study.pk]) + + class StudyImportView(PermissionRequiredMixin, FormView): permission_required = 'studies.change_study' template_name = 'studies/study_import.html' diff --git a/tests/studies/views/test_pseudonym_domains_view.py b/tests/studies/views/test_pseudonym_domains_view.py index 69485f2d7..95bee6ab1 100644 --- a/tests/studies/views/test_pseudonym_domains_view.py +++ b/tests/studies/views/test_pseudonym_domains_view.py @@ -28,3 +28,11 @@ def test_name(client, study, member): assert response.status_code == 302 domain.refresh_from_db() assert domain.name == 'foo' + + +def test_delete(client, study, member): + client.force_login(member) + domain = study.domains.create() + response = client.post('/studies/{}/domains/{}/'.format(study.pk, domain.key)) + assert response.status_code == 302 + assert study.domains.count() == 1 -- GitLab From 743d567ba7ba4567e5a4a849399c04c8b6faee15 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Mon, 14 Mar 2022 17:55:54 +0100 Subject: [PATCH 2/5] create study domain in CreateView instead of on every save --- castellum/studies/apps.py | 1 - castellum/studies/signals.py | 11 ----------- castellum/studies/views/studies.py | 5 +++++ tests/execution/views/test_api_domains_view.py | 4 ++-- tests/execution/views/test_pseudonyms_view.py | 2 +- tests/recruitment/models/test_recruitment.py | 3 ++- tests/studies/models/test_study.py | 3 ++- tests/studies/views/test_pseudonym_domains_view.py | 9 ++++++--- 8 files changed, 18 insertions(+), 20 deletions(-) diff --git a/castellum/studies/apps.py b/castellum/studies/apps.py index a55922c3a..4d9c19b52 100644 --- a/castellum/studies/apps.py +++ b/castellum/studies/apps.py @@ -29,7 +29,6 @@ class StudiesConfig(AppConfig): name = 'castellum.studies' def ready(self): - post_save.connect(signals.create_study_domain) pre_delete.connect(signals.delete_study_domain) post_save.connect(signals.create_session_domain) diff --git a/castellum/studies/signals.py b/castellum/studies/signals.py index 97e1d7486..108685d29 100644 --- a/castellum/studies/signals.py +++ b/castellum/studies/signals.py @@ -21,17 +21,6 @@ from django.conf import settings -def create_study_domain(sender, instance, using, **kwargs): - from castellum.pseudonyms.models import Domain - from castellum.studies.models import Study - - if sender is Study and not instance.domains.exists(): - Domain.objects.create( - bits=settings.CASTELLUM_STUDY_DOMAIN_BITS, - context=instance, - ) - - def delete_study_domain(sender, instance, using, **kwargs): from castellum.studies.models import Study diff --git a/castellum/studies/views/studies.py b/castellum/studies/views/studies.py index 237d21ec1..771aefd8f 100644 --- a/castellum/studies/views/studies.py +++ b/castellum/studies/views/studies.py @@ -260,6 +260,11 @@ class StudyCreateView(PermissionRequiredMixin, CreateView): for tag in self.duplicate.executiontag_set.all(): self.object.executiontag_set.create(name=tag.name) else: + Domain.objects.create( + bits=settings.CASTELLUM_STUDY_DOMAIN_BITS, + context=self.object, + ) + for name, color in settings.CASTELLUM_DEFAULT_EXECUTION_TAGS: self.object.executiontag_set.create(name=name, color=color) diff --git a/tests/execution/views/test_api_domains_view.py b/tests/execution/views/test_api_domains_view.py index 7b082fef7..bfe499ed1 100644 --- a/tests/execution/views/test_api_domains_view.py +++ b/tests/execution/views/test_api_domains_view.py @@ -12,7 +12,7 @@ def test_apidomainsview(client, conductor, study_in_execution_status): assert response.status_code == 200 data = response.json() - assert len(data['domains']) == 3 + assert len(data['domains']) == 2 def test_no_general_domains(client, conductor, study_in_execution_status): @@ -25,7 +25,7 @@ def test_no_general_domains(client, conductor, study_in_execution_status): assert response.status_code == 200 data = response.json() - assert len(data['domains']) == 1 + assert len(data['domains']) == 0 def test_permission(client, recruiter, study_in_execution_status): diff --git a/tests/execution/views/test_pseudonyms_view.py b/tests/execution/views/test_pseudonyms_view.py index 01da3e21e..0981f0a41 100644 --- a/tests/execution/views/test_pseudonyms_view.py +++ b/tests/execution/views/test_pseudonyms_view.py @@ -28,7 +28,7 @@ def test_lists_all_pseudonyms(client, conductor, study): study.pk, participation.pk )) assert response.status_code == 200 - assert response.content.count(b' Date: Mon, 14 Mar 2022 17:53:04 +0100 Subject: [PATCH 3/5] make studies work without domains --- .../templates/execution/participation_detail.html | 2 +- .../templates/execution/study_resolve.html | 2 +- castellum/pseudonyms/forms.py | 5 ++++- tests/execution/views/test_export_view.py | 14 +++++++++----- tests/execution/views/test_views.py | 2 +- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/castellum/execution/templates/execution/participation_detail.html b/castellum/execution/templates/execution/participation_detail.html index 93b3c7f1c..a48b3ae97 100644 --- a/castellum/execution/templates/execution/participation_detail.html +++ b/castellum/execution/templates/execution/participation_detail.html @@ -14,7 +14,7 @@ {% endfor %} - {% else %} + {% elif domains|length > 0 %}
{% translate 'Pseudonym' %}
{% for domain in domains %} diff --git a/castellum/execution/templates/execution/study_resolve.html b/castellum/execution/templates/execution/study_resolve.html index d9ba259f6..71b27716d 100644 --- a/castellum/execution/templates/execution/study_resolve.html +++ b/castellum/execution/templates/execution/study_resolve.html @@ -6,7 +6,7 @@ {% include 'utils/form_errors.html' with form=form %} {% csrf_token %} - {% if form.domain.field.choices|length > 1 %} + {% if form.domain.field.choices|length != 1 %} {% bootstrap_field form.domain %} {% else %} {{ form.domain.as_hidden }} diff --git a/castellum/pseudonyms/forms.py b/castellum/pseudonyms/forms.py index 4f4ceaf74..5f579b2c6 100644 --- a/castellum/pseudonyms/forms.py +++ b/castellum/pseudonyms/forms.py @@ -40,7 +40,10 @@ class DomainForm(forms.Form): super().__init__(*args, **kwargs) self.fields['domain'].choices = [(d.key, str(d)) for d in context.domains.all()] - self.fields['domain'].initial = self.fields['domain'].choices[0][0] + if self.fields['domain'].choices: + self.fields['domain'].initial = self.fields['domain'].choices[0][0] + else: + self.fields['domain'].disabled = True class PseudonymForm(DomainForm): diff --git a/tests/execution/views/test_export_view.py b/tests/execution/views/test_export_view.py index cace1e11c..52efdcfdf 100644 --- a/tests/execution/views/test_export_view.py +++ b/tests/execution/views/test_export_view.py @@ -1,4 +1,3 @@ -import pytest from model_bakery import baker from castellum.pseudonyms.models import Domain @@ -7,8 +6,9 @@ from castellum.recruitment.models import Participation from castellum.studies.models import Study -@pytest.mark.smoketest -def test_200(client, conductor, study_in_execution_status): +def test_single_domain(client, conductor, study_in_execution_status): + baker.make(Domain, context=study_in_execution_status) + client.force_login(conductor) url = '/execution/{}/export/'.format(study_in_execution_status.pk) response = client.get(url) @@ -17,17 +17,18 @@ def test_200(client, conductor, study_in_execution_status): assert response['Content-Type'] == 'application/zip' -def test_single_domain(client, conductor, study_in_execution_status): +def test_no_domain(client, conductor, study_in_execution_status): client.force_login(conductor) url = '/execution/{}/export/'.format(study_in_execution_status.pk) response = client.get(url) assert response.status_code == 200 - assert response['Content-Type'] == 'application/zip' + assert response['Content-Type'] == 'text/html; charset=utf-8' def test_multi_domain(client, conductor, study_in_execution_status): baker.make(Domain, context=study_in_execution_status) + baker.make(Domain, context=study_in_execution_status) client.force_login(conductor) url = '/execution/{}/export/'.format(study_in_execution_status.pk) @@ -39,6 +40,7 @@ def test_multi_domain(client, conductor, study_in_execution_status): def test_multi_domain_given(client, conductor, study_in_execution_status): baker.make(Participation, study=study_in_execution_status, status=Participation.INVITED) + baker.make(Domain, context=study_in_execution_status) domain2 = baker.make(Domain, context=study_in_execution_status) client.force_login(conductor) @@ -55,6 +57,7 @@ def test_multi_domain_given(client, conductor, study_in_execution_status): def test_multi_domain_given_invalid(client, conductor, study_in_execution_status): baker.make(Domain, context=study_in_execution_status) + baker.make(Domain, context=study_in_execution_status) client.force_login(conductor) url = '/execution/{}/export/?domain={}'.format(study_in_execution_status.pk, 'invalid') @@ -65,6 +68,7 @@ def test_multi_domain_given_invalid(client, conductor, study_in_execution_status def test_multi_domain_given_wrong_study(client, conductor, study_in_execution_status): study2 = baker.make(Study) + baker.make(Domain, context=study_in_execution_status) domain2 = baker.make(Domain, context=study_in_execution_status) client.force_login(conductor) diff --git a/tests/execution/views/test_views.py b/tests/execution/views/test_views.py index 34ddb885b..00d4c30ff 100644 --- a/tests/execution/views/test_views.py +++ b/tests/execution/views/test_views.py @@ -105,6 +105,7 @@ def test_participation_detail_view_tags(client, subject, conductor, study_in_exe def test_resolve_view(client, member, study_in_execution_status, subject): client.force_login(member) + domain = baker.make(Domain, context=study_in_execution_status) participation = baker.make( Participation, study=study_in_execution_status, @@ -117,7 +118,6 @@ def test_resolve_view(client, member, study_in_execution_status, subject): response = client.get(url) assert b' Date: Tue, 15 Mar 2022 10:16:12 +0100 Subject: [PATCH 4/5] show warning on no domain --- castellum/execution/templates/execution/study_export.html | 1 + castellum/pseudonyms/forms.py | 2 ++ tests/execution/views/test_export_view.py | 1 + 3 files changed, 4 insertions(+) diff --git a/castellum/execution/templates/execution/study_export.html b/castellum/execution/templates/execution/study_export.html index b78758e78..d1ff65109 100644 --- a/castellum/execution/templates/execution/study_export.html +++ b/castellum/execution/templates/execution/study_export.html @@ -5,6 +5,7 @@ {% block content %}
+ {% include 'utils/form_errors.html' with form=form %} {% bootstrap_field form.domain %}
diff --git a/castellum/pseudonyms/forms.py b/castellum/pseudonyms/forms.py index 5f579b2c6..800e24067 100644 --- a/castellum/pseudonyms/forms.py +++ b/castellum/pseudonyms/forms.py @@ -44,6 +44,8 @@ class DomainForm(forms.Form): self.fields['domain'].initial = self.fields['domain'].choices[0][0] else: self.fields['domain'].disabled = True + self.cleaned_data = {} + self.add_error(None, _('All pseudonym domains have been deleted.')) class PseudonymForm(DomainForm): diff --git a/tests/execution/views/test_export_view.py b/tests/execution/views/test_export_view.py index 52efdcfdf..32a26d55c 100644 --- a/tests/execution/views/test_export_view.py +++ b/tests/execution/views/test_export_view.py @@ -24,6 +24,7 @@ def test_no_domain(client, conductor, study_in_execution_status): assert response.status_code == 200 assert response['Content-Type'] == 'text/html; charset=utf-8' + assert b'All pseudonym domains have been deleted.' in response.content def test_multi_domain(client, conductor, study_in_execution_status): -- GitLab From 8406a80d201f5f10877b3f207574acdadaebfea0 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Tue, 15 Mar 2022 11:27:32 +0100 Subject: [PATCH 5/5] extend confirm delete view --- .../templates/studies/domain_confirm_delete.html | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/castellum/studies/templates/studies/domain_confirm_delete.html b/castellum/studies/templates/studies/domain_confirm_delete.html index c8a9b9705..a800809b8 100644 --- a/castellum/studies/templates/studies/domain_confirm_delete.html +++ b/castellum/studies/templates/studies/domain_confirm_delete.html @@ -6,10 +6,17 @@ {% block content %}
{% csrf_token %} -

{% translate "Are you sure you want to permanently delete this domain and all related pseudonyms?" %}

+

{% blocktranslate %}Are you sure you want to permanently delete the domain "{{ object }}" and all related pseudonyms?{% endblocktranslate %}

+

{% translate "Once a pseudonym is deleted, it is no longer possible to find the corresponding contact information. Note, however, that additional steps might be necessary for full anonymization (e.g. for image data)." %}

+

{% translate "The date when a domain should be deleted is usually defined in the ethics application and the study consent form." %}

+ +
{% translate 'Cancel' %} - +
{% endblock %} -- GitLab