From b4c48e53649c364aa7545d16675623776e04b6d7 Mon Sep 17 00:00:00 2001 From: Todd Dembrey <todd.dembrey@torchbox.com> Date: Tue, 5 Nov 2019 10:21:37 +0000 Subject: [PATCH] Feature/bugfix allow private and public messages (#1658) * Remove privacy toggle and use explicit private public fields * Add in content help text --- opentech/apply/projects/forms.py | 29 ++++-- .../0031_add_public_private_content.py | 28 ++++++ opentech/apply/projects/models.py | 4 +- .../application_projects/report_detail.html | 12 +-- opentech/apply/projects/tests/factories.py | 3 +- opentech/apply/projects/tests/test_views.py | 90 ++++++++++--------- opentech/apply/projects/views/report.py | 4 +- 7 files changed, 114 insertions(+), 56 deletions(-) create mode 100644 opentech/apply/projects/migrations/0031_add_public_private_content.py diff --git a/opentech/apply/projects/forms.py b/opentech/apply/projects/forms.py index 44d1ae4fa..218b2df53 100644 --- a/opentech/apply/projects/forms.py +++ b/opentech/apply/projects/forms.py @@ -380,7 +380,12 @@ class UpdateProjectLeadForm(forms.ModelForm): class ReportEditForm(forms.ModelForm): - content = RichTextField(required=True) + public_content = RichTextField( + help_text="This section will be shared with the wider community." + ) + private_content = RichTextField( + help_text="This section will be shared with staff members only." + ) file_list = forms.ModelMultipleChoiceField( widget=forms.CheckboxSelectMultiple(attrs={'class': 'delete'}), queryset=ReportPrivateFiles.objects.all(), @@ -391,7 +396,12 @@ class ReportEditForm(forms.ModelForm): class Meta: model = Report - fields = ['public'] + fields = ( + 'public_content', + 'private_content', + 'file_list', + 'files', + ) def __init__(self, *args, user=None, initial={}, **kwargs): self.report_files = initial.pop( @@ -402,9 +412,15 @@ class ReportEditForm(forms.ModelForm): self.fields['file_list'].queryset = self.report_files self.user = user - # Cant change the privacy of a submitted report - if self.instance.current: - del self.fields['public'] + def clean(self): + cleaned_data = super().clean() + public = cleaned_data['public_content'] + private = cleaned_data['private_content'] + if not private and not public: + missing_content = 'Must include either public or private content when submitting a report.' + self.add_error('public_content', missing_content) + self.add_error('private_content', missing_content) + return cleaned_data @transaction.atomic def save(self, commit=True): @@ -412,7 +428,8 @@ class ReportEditForm(forms.ModelForm): version = ReportVersion.objects.create( report=self.instance, - content=self.cleaned_data['content'], + public_content=self.cleaned_data['public_content'], + private_content=self.cleaned_data['private_content'], submitted=timezone.now(), draft=is_draft, author=self.user, diff --git a/opentech/apply/projects/migrations/0031_add_public_private_content.py b/opentech/apply/projects/migrations/0031_add_public_private_content.py new file mode 100644 index 000000000..c4af728a8 --- /dev/null +++ b/opentech/apply/projects/migrations/0031_add_public_private_content.py @@ -0,0 +1,28 @@ +# Generated by Django 2.1.11 on 2019-10-31 22:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('application_projects', '0030_report_skipped'), + ] + + operations = [ + migrations.RenameField( + model_name='reportversion', + old_name='content', + new_name='public_content', + ), + migrations.RemoveField( + model_name='report', + name='public', + ), + migrations.AddField( + model_name='reportversion', + name='private_content', + field=models.TextField(default=''), + preserve_default=False, + ), + ] diff --git a/opentech/apply/projects/models.py b/opentech/apply/projects/models.py index b9a56ec3d..f047e1da7 100644 --- a/opentech/apply/projects/models.py +++ b/opentech/apply/projects/models.py @@ -653,7 +653,6 @@ class ReportQueryset(models.QuerySet): class Report(models.Model): - public = models.BooleanField(default=True) skipped = models.BooleanField(default=False) end_date = models.DateField() project = models.ForeignKey("Project", on_delete=models.CASCADE, related_name="reports") @@ -718,7 +717,8 @@ class Report(models.Model): class ReportVersion(models.Model): report = models.ForeignKey("Report", on_delete=models.CASCADE, related_name="versions") submitted = models.DateTimeField() - content = models.TextField() + public_content = models.TextField() + private_content = models.TextField() draft = models.BooleanField() author = models.ForeignKey( settings.AUTH_USER_MODEL, diff --git a/opentech/apply/projects/templates/application_projects/report_detail.html b/opentech/apply/projects/templates/application_projects/report_detail.html index d145729ed..dc075e40c 100644 --- a/opentech/apply/projects/templates/application_projects/report_detail.html +++ b/opentech/apply/projects/templates/application_projects/report_detail.html @@ -18,14 +18,14 @@ {% if report.skipped %} <h2>Report Skipped</h2> {% else%} - <h3> - {% if object.public %} + <h3> Public - {% else %} + </h3> + {{ object.current.public_content|bleach|safe }} + <h3> Private - {% endif %} - </h3> - {{ object.current.content|bleach|safe }} + </h3> + {{ object.current.private_content|bleach|safe }} {% for file in object.current.files.all %} {% if forloop.first %} <h4>Files</h4> diff --git a/opentech/apply/projects/tests/factories.py b/opentech/apply/projects/tests/factories.py index bdca5a9a5..1768a12cc 100644 --- a/opentech/apply/projects/tests/factories.py +++ b/opentech/apply/projects/tests/factories.py @@ -159,7 +159,8 @@ class ReportConfigFactory(factory.DjangoModelFactory): class ReportVersionFactory(factory.DjangoModelFactory): report = factory.SubFactory("opentech.apply.projects.tests.factories.ReportFactory") submitted = factory.LazyFunction(timezone.now) - content = factory.Faker('paragraph') + public_content = factory.Faker('paragraph') + private_content = factory.Faker('paragraph') draft = True class Meta: diff --git a/opentech/apply/projects/tests/test_views.py b/opentech/apply/projects/tests/test_views.py index 23b6f729c..a6364605a 100644 --- a/opentech/apply/projects/tests/test_views.py +++ b/opentech/apply/projects/tests/test_views.py @@ -1257,40 +1257,57 @@ class TestStaffSubmitReport(BaseViewTestCase): def test_submit_report(self): report = ReportFactory() - response = self.post_page(report, {'content': 'Some text', 'public': True}) + response = self.post_page(report, {'public_content': 'Some text'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.first().content, 'Some text') + self.assertEqual(report.versions.first().public_content, 'Some text') self.assertEqual(report.versions.first(), report.current) self.assertEqual(report.current.author, self.user) self.assertIsNone(report.draft) + def test_submit_private_report(self): + report = ReportFactory() + response = self.post_page(report, {'private_content': 'Some text'}) + report.refresh_from_db() + self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) + self.assertEqual(report.versions.first().private_content, 'Some text') + self.assertEqual(report.versions.first(), report.current) + self.assertEqual(report.current.author, self.user) + self.assertIsNone(report.draft) + + def test_cant_submit_blank_report(self): + report = ReportFactory() + response = self.post_page(report, {}) + report.refresh_from_db() + self.assertEqual(response.status_code, 200) + self.assertEqual(report.versions.count(), 0) + def test_save_report_draft(self): report = ReportFactory() - response = self.post_page(report, {'content': 'Some text', 'public': True, 'save': 'Save'}) + response = self.post_page(report, {'public_content': 'Some text', 'save': 'Save'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.first().content, 'Some text') + self.assertEqual(report.versions.first().public_content, 'Some text') self.assertEqual(report.versions.first(), report.draft) self.assertIsNone(report.current) def test_save_report_with_draft(self): report = ReportFactory(is_draft=True) self.assertEqual(report.versions.first(), report.draft) - response = self.post_page(report, {'content': 'Some text', 'public': True}) + response = self.post_page(report, {'public_content': 'Some text'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.last().content, 'Some text') + self.assertEqual(report.versions.last().public_content, 'Some text') self.assertEqual(report.versions.last(), report.current) self.assertIsNone(report.draft) def test_edit_submitted_report(self): report = ReportFactory(is_submitted=True) self.assertEqual(report.versions.first(), report.current) - response = self.post_page(report, {'content': 'Some text', 'public': True, 'save': ' Save'}) + response = self.post_page(report, {'public_content': 'Some text', 'save': ' Save'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.last().content, 'Some text') + self.assertEqual(report.versions.last().public_content, 'Some text') self.assertEqual(report.versions.last(), report.draft) self.assertEqual(report.versions.first(), report.current) @@ -1303,16 +1320,16 @@ class TestStaffSubmitReport(BaseViewTestCase): report.save() self.assertEqual(report.submitted, yesterday) self.assertEqual(report.versions.first(), report.current) - response = self.post_page(report, {'content': 'Some text', 'public': True}) + response = self.post_page(report, {'public_content': 'Some text'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.last().content, 'Some text') + self.assertEqual(report.versions.last().public_content, 'Some text') self.assertEqual(report.versions.last(), report.current) self.assertIsNone(report.draft) self.assertEqual(report.submitted.date(), yesterday.date()) self.assertEqual(report.current.submitted.date(), timezone.now().date()) - def test_can_submit_future_report(self): + def test_cant_submit_future_report(self): submitted_report = ReportFactory( end_date=timezone.now() + relativedelta(days=1), is_submitted=True, @@ -1321,26 +1338,9 @@ class TestStaffSubmitReport(BaseViewTestCase): end_date=timezone.now() + relativedelta(days=3), project=submitted_report.project, ) - response = self.post_page(future_report, {'content': 'Some text', 'public': True}) + response = self.post_page(future_report, {'public_content': 'Some text'}) self.assertEqual(response.status_code, 404) - def test_change_privacy(self): - report = ReportFactory(public=True) - response = self.post_page(report, {'content': 'Some text', 'public': False}) - report.refresh_from_db() - self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertFalse(report.public) - - def test_cant_change_privacy_submitted(self): - report = ReportFactory( - is_submitted=True, - public=True, - ) - response = self.post_page(report, {'content': 'Some text', 'public': False}) - report.refresh_from_db() - self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertTrue(report.public) - class TestApplicantSubmitReport(BaseViewTestCase): base_view_name = 'edit' @@ -1364,48 +1364,58 @@ class TestApplicantSubmitReport(BaseViewTestCase): def test_submit_own_report(self): report = ReportFactory(project__user=self.user) - response = self.post_page(report, {'content': 'Some text', 'public': True}) + response = self.post_page(report, {'public_content': 'Some text'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.first().content, 'Some text') + self.assertEqual(report.versions.first().public_content, 'Some text') self.assertEqual(report.versions.first(), report.current) self.assertEqual(report.current.author, self.user) - def test_change_privacy_own(self): + def test_submit_private_report(self): report = ReportFactory(project__user=self.user) - response = self.post_page(report, {'content': 'Some text', 'public': False}) + response = self.post_page(report, {'private_content': 'Some text'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertFalse(report.public) + self.assertEqual(report.versions.first().private_content, 'Some text') + self.assertEqual(report.versions.first(), report.current) + self.assertEqual(report.current.author, self.user) + self.assertIsNone(report.draft) + + def test_cant_submit_blank_report(self): + report = ReportFactory(project__user=self.user) + response = self.post_page(report, {}) + report.refresh_from_db() + self.assertEqual(response.status_code, 200) + self.assertEqual(report.versions.count(), 0) def test_save_report_draft(self): report = ReportFactory(project__user=self.user) - response = self.post_page(report, {'content': 'Some text', 'public': True, 'save': 'Save'}) + response = self.post_page(report, {'public_content': 'Some text', 'save': 'Save'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.first().content, 'Some text') + self.assertEqual(report.versions.first().public_content, 'Some text') self.assertEqual(report.versions.first(), report.draft) self.assertIsNone(report.current) def test_save_report_with_draft(self): report = ReportFactory(is_draft=True, project__user=self.user) self.assertEqual(report.versions.first(), report.draft) - response = self.post_page(report, {'content': 'Some text', 'public': True}) + response = self.post_page(report, {'public_content': 'Some text'}) report.refresh_from_db() self.assertRedirects(response, self.absolute_url(report.project.get_absolute_url())) - self.assertEqual(report.versions.last().content, 'Some text') + self.assertEqual(report.versions.last().public_content, 'Some text') self.assertEqual(report.versions.last(), report.current) self.assertIsNone(report.draft) def test_cant_edit_submitted_report(self): report = ReportFactory(is_submitted=True, project__user=self.user) self.assertEqual(report.versions.first(), report.current) - response = self.post_page(report, {'content': 'Some text', 'public': True, 'save': ' Save'}) + response = self.post_page(report, {'public_content': 'Some text', 'save': ' Save'}) self.assertEqual(response.status_code, 404) def test_cant_submit_other_report(self): report = ReportFactory() - response = self.post_page(report, {'content': 'Some text', 'public': True}) + response = self.post_page(report, {'public_content': 'Some text'}) self.assertEqual(response.status_code, 403) diff --git a/opentech/apply/projects/views/report.py b/opentech/apply/projects/views/report.py index d90767e37..2e804f7fe 100644 --- a/opentech/apply/projects/views/report.py +++ b/opentech/apply/projects/views/report.py @@ -75,7 +75,8 @@ class ReportUpdateView(ReportAccessMixin, UpdateView): if current: return { - 'content': current.content, + 'public_content': current.public_content, + 'private_content': current.private_content, 'file_list': current.files.all(), } @@ -167,6 +168,7 @@ class ReportSkipView(SingleObjectMixin, View): return redirect(report.project.get_absolute_url()) +@method_decorator(staff_required, name='dispatch') class ReportFrequencyUpdate(DelegatedViewMixin, UpdateView): form_class = ReportFrequencyForm context_name = 'update_frequency_form' -- GitLab