diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py index 4b70452923bb21f9b23d318908de04a9fa65c7c2..0561bfeb82e84f91032ed7641e502f9ef4416746 100644 --- a/opentech/apply/activity/messaging.py +++ b/opentech/apply/activity/messaging.py @@ -15,6 +15,7 @@ def link_to(target, request): class MESSAGES(Enum): UPDATE_LEAD = 'Update Lead' + EDIT = 'Edit' NEW_SUBMISSION = 'New Submission' TRANSITION = 'Transition' DETERMINATION_OUTCOME = 'Determination Outcome' @@ -107,6 +108,7 @@ class ActivityAdapter(AdapterBase): messages = { MESSAGES.TRANSITION: 'Progressed from {old_phase.display_name} to {submission.phase}', MESSAGES.NEW_SUBMISSION: 'Submitted {submission.title} for {submission.page.title}', + MESSAGES.EDIT: '{user} edited', MESSAGES.UPDATE_LEAD: 'Lead changed from {old.lead} to {submission.lead}', MESSAGES.DETERMINATION_OUTCOME: 'Sent a determination. Outcome: {submission.determination.clean_outcome}', MESSAGES.INVITED_TO_PROPOSAL: 'Invited to submit a proposal', @@ -154,6 +156,7 @@ class SlackAdapter(AdapterBase): MESSAGES.NEW_SUBMISSION: 'A new submission has been submitted for {submission.page.title}: <{link}|{submission.title}>', MESSAGES.UPDATE_LEAD: 'The lead of <{link}|{submission.title}> has been updated from {old.lead} to {submission.lead} by {user}', MESSAGES.COMMENT: 'A new comment has been posted on <{link}|{submission.title}>', + MESSAGES.EDIT: '{user} has edited <{link}|{submission.title}>', MESSAGES.REVIEWERS_UPDATED: '{user} has updated the reviewers on <{link}|{submission.title}>', MESSAGES.TRANSITION: '{user} has updated the status of <{link}|{submission.title}>: {old_phase.display_name} → {submission.phase}', MESSAGES.DETERMINATION_OUTCOME: 'A determination for <{link}|{submission.title}> was sent by email. Outcome: {submission.determination.clean_outcome}', @@ -226,6 +229,7 @@ class EmailAdapter(AdapterBase): messages = { MESSAGES.NEW_SUBMISSION: 'funds/email/confirmation.html', MESSAGES.COMMENT: 'notify_comment', + MESSAGES.EDIT: 'messages/email/edit.html', MESSAGES.TRANSITION: 'messages/email/transition.html', MESSAGES.DETERMINATION_OUTCOME: 'messages/email/determination.html', MESSAGES.INVITED_TO_PROPOSAL: 'messages/email/invited_to_proposal.html', diff --git a/opentech/apply/activity/migrations/0005_event.py b/opentech/apply/activity/migrations/0005_event.py index ffe2c1897edb991be3128ad5cc6ef04fc33c0d57..635b8afef5ec4d6eea269dfc57432a461552773f 100644 --- a/opentech/apply/activity/migrations/0005_event.py +++ b/opentech/apply/activity/migrations/0005_event.py @@ -17,7 +17,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('when', models.DateTimeField(auto_now_add=True)), - ('type', models.CharField(choices=[('UPDATE_LEAD', 'Update Lead'), ('NEW_SUBMISSION', 'New Submission'), ('TRANSITION', 'Transition'), ('DETERMINATION_OUTCOME', 'Determination Outcome'), ('INVITED_TO_PROPOSAL', 'Invited To Proposal'), ('REVIEWERS_UPDATED', 'Reviewers Updated'), ('READY_FOR_REVIEW', 'Ready For Review'), ('NEW_REVIEW', 'New Review'), ('COMMENT', 'Comment'), ('PROPOSAL_SUBMITTED', 'Proposal Submitted'), ('OPENED_SEALED', 'Opened Sealed Submission')], max_length=50)), + ('type', models.CharField(choices=[('UPDATE_LEAD', 'Update Lead'), ('EDIT', 'Edit'), ('NEW_SUBMISSION', 'New Submission'), ('TRANSITION', 'Transition'), ('DETERMINATION_OUTCOME', 'Determination Outcome'), ('INVITED_TO_PROPOSAL', 'Invited To Proposal'), ('REVIEWERS_UPDATED', 'Reviewers Updated'), ('READY_FOR_REVIEW', 'Ready For Review'), ('NEW_REVIEW', 'New Review'), ('COMMENT', 'Comment'), ('PROPOSAL_SUBMITTED', 'Proposal Submitted'), ('OPENED_SEALED', 'Opened Sealed Submission')], max_length=50)), ('by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL)), ('submission', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='funds.ApplicationSubmission', related_name='+')), ], diff --git a/opentech/apply/activity/templates/messages/email/edit.html b/opentech/apply/activity/templates/messages/email/edit.html new file mode 100644 index 0000000000000000000000000000000000000000..c93d54cbb0b7a119a8cc78296bb8e6ef0e059ec3 --- /dev/null +++ b/opentech/apply/activity/templates/messages/email/edit.html @@ -0,0 +1,5 @@ +{% extends "messages/email/applicant_base.html" %} + +{% block content %} +Your submission has been edited by a member of staff. +{% endblock %} diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py index 188bde203c21285a266002c6accdd268f0bae59e..1953e26c7cb84306777f7674419850e927b2568f 100644 --- a/opentech/apply/funds/models/submissions.py +++ b/opentech/apply/funds/models/submissions.py @@ -401,6 +401,7 @@ class ApplicationSubmission( return self def create_revision(self, draft=False, force=False, by=None, **kwargs): + # Will return True/False if the revision was created or not self.clean_submission() current_data = ApplicationSubmission.objects.get(id=self.id).form_data if current_data != self.form_data or force: @@ -419,6 +420,8 @@ class ApplicationSubmission( self.draft_revision = revision self.save() + return True + return False def clean_submission(self): self.process_form_data() diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html index dfb9782ec207c1aa959a887cb2a4175e95eee0cc..0b9288328f1af3e5a9f66c71e7fdb43a76c3423a 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html @@ -59,7 +59,7 @@ <h6 class="heading heading--submission-meta"> <span>Submitted: <strong>{{ object.submit_time.date }} by {{ object.user.get_full_name }}</strong></span> <span>Last edited: <strong>{{ object.last_edit.timestamp.date }} by {{ object.last_edit.user.get_full_name }}</strong></span> - {% if request.user|has_edit_perm:object and object.user == request.user %} + {% if request.user|has_edit_perm:object %} <a class="link link--edit-submission is-active" href="{% url 'funds:submissions:edit' object.id %}"> Edit <svg class="icon icon--pen"><use xlink:href="#pen"></use></svg> diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_form.html b/opentech/apply/funds/templates/funds/applicationsubmission_form.html index 8cfff8a972948aa669b610746ffce562abcdd34b..066afaeba8b8826abc513d8fbfcc937b1749d214 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_form.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_form.html @@ -34,17 +34,11 @@ {{ field }} {% endif %} {% endfor %} - {% for button_name, button_value in buttons %} - <input class="button button--primary" type="submit" name="{{ button_name }}" value="{{ button_value }}" /> + {% for button_name, button_type, button_value in buttons %} + <input class="button button--{{ button_type }}" type="submit" name="{{ button_name }}" value="{{ button_value }}" /> {% endfor %} </form> </div> - - <aside class="sidebar"> - {% block determination %} - {% include 'determinations/includes/determination_block.html' with submission=object %} - {% endblock %} - </aside> </div> {% endblock %} diff --git a/opentech/apply/funds/tests/test_views.py b/opentech/apply/funds/tests/test_views.py index 839d0a87e9edf3fd3a4053720638f9a74996fc3e..a1b986dfc4375236fb0d31a5d97188944620e22f 100644 --- a/opentech/apply/funds/tests/test_views.py +++ b/opentech/apply/funds/tests/test_views.py @@ -16,6 +16,32 @@ from opentech.apply.utils.testing.tests import BaseViewTestCase from ..models import ApplicationRevision +def prepare_form_data(submission, **kwargs): + data = submission.raw_data + + for field, value in kwargs.items(): + # convert named fields into id + field_id = submission.field(field).id + data[field_id] = value + + address_field = submission.must_include['address'] + address = data.pop(address_field) + data.update(**prepare_address(address, address_field)) + + return data + + +def prepare_address(address, field): + address = json.loads(address) + address['locality'] = { + 'localityname': address.pop('localityname'), + 'administrativearea': address.pop('administrativearea'), + 'postalcode': address.pop('postalcode'), + } + address = flatten_for_form(address, field, number=True) + return address + + class BaseSubmissionViewTestCase(BaseViewTestCase): url_name = 'funds:submissions:{}' base_view_name = 'detail' @@ -62,11 +88,37 @@ class TestStaffSubmissionView(BaseSubmissionViewTestCase): self.assertEqual(submission.status, 'concept_review_discussion') self.assertIsNone(submission.next) - def test_cant_access_edit_button_on_applicant_submission(self): + def test_cant_access_edit_button_when_applicant_editing(self): submission = ApplicationSubmissionFactory(status='more_info') response = self.get_page(submission) self.assertNotContains(response, self.url(submission, 'edit', absolute=False)) + def test_can_access_edit_button(self): + submission = ApplicationSubmissionFactory() + response = self.get_page(submission) + self.assertContains(response, self.url(submission, 'edit', absolute=False)) + + def test_can_access_edit(self): + submission = ApplicationSubmissionFactory() + response = self.get_page(submission, 'edit') + self.assertContains(response, submission.title) + + def test_can_edit_submission(self): + submission = ApplicationSubmissionFactory() + old_status = submission.status + new_title = 'A new Title' + data = prepare_form_data(submission, title=new_title) + response = self.post_page(submission, {'submit': True, **data}, 'edit') + + url = self.url_from_pattern('funds:submissions:detail', kwargs={'pk': submission.id}) + + self.assertRedirects(response, url) + submission = self.refresh(submission) + + # Staff edits don't affect the status + self.assertEqual(old_status, submission.status) + self.assertEqual(new_title, submission.title) + class TestApplicantSubmissionView(BaseSubmissionViewTestCase): user_factory = UserFactory @@ -109,6 +161,20 @@ class TestApplicantSubmissionView(BaseSubmissionViewTestCase): response = self.get_page(submission, 'edit') self.assertContains(response, submission.title) + def test_can_submit_submission(self): + submission = ApplicationSubmissionFactory(user=self.user, draft_proposal=True) + old_status = submission.status + + data = prepare_form_data(submission, title='This is different') + + response = self.post_page(submission, {'submit': True, **data}, 'edit') + + url = self.url_from_pattern('funds:submissions:detail', kwargs={'pk': submission.id}) + + self.assertRedirects(response, url) + submission = self.refresh(submission) + self.assertNotEqual(old_status, submission.status) + def test_gets_draft_on_edit_submission(self): submission = ApplicationSubmissionFactory(user=self.user, draft_proposal=True) draft_revision = ApplicationRevisionFactory(submission=submission) @@ -132,29 +198,12 @@ class TestApplicantSubmissionView(BaseSubmissionViewTestCase): class TestRevisionsView(BaseSubmissionViewTestCase): user_factory = UserFactory - def prepare_address(self, address, field): - address = json.loads(address) - address['locality'] = { - 'localityname': address.pop('localityname'), - 'administrativearea': address.pop('administrativearea'), - 'postalcode': address.pop('postalcode'), - } - address = flatten_for_form(address, field, number=True) - return address - def test_create_revisions_on_submit(self): submission = ApplicationSubmissionFactory(status='draft_proposal', workflow_stages=2, user=self.user) old_data = submission.form_data.copy() - new_data = submission.raw_data - new_title = 'New title' - new_data[submission.must_include['title']] = new_title - - address_id = submission.must_include['address'] - new_data.update(**self.prepare_address( - new_data[submission.must_include['address']], - address_id, - )) + new_title = 'New title' + new_data = prepare_form_data(submission, title=new_title) self.post_page(submission, {'submit': True, **new_data}, 'edit') @@ -171,16 +220,8 @@ class TestRevisionsView(BaseSubmissionViewTestCase): submission = ApplicationSubmissionFactory(status='draft_proposal', workflow_stages=2, user=self.user) old_data = submission.form_data.copy() - new_data = submission.raw_data - - address_id = submission.must_include['address'] + new_data = prepare_form_data(submission, title='New title') - new_data.update(**self.prepare_address( - new_data[submission.must_include['address']], - address_id, - )) - - new_data[submission.must_include['title']] = 'New title' self.post_page(submission, {'save': True, **new_data}, 'edit') submission = self.refresh(submission) @@ -193,23 +234,14 @@ class TestRevisionsView(BaseSubmissionViewTestCase): def test_existing_draft_edit_and_submit(self): submission = ApplicationSubmissionFactory(status='draft_proposal', workflow_stages=2, user=self.user) - draft_data = submission.raw_data.copy() - - address_id = submission.must_include['address'] - - draft_data.update(**self.prepare_address( - draft_data[submission.must_include['address']], - address_id, - )) - - draft_data[submission.must_include['title']] = 'New title' + draft_data = prepare_form_data(submission, title='A new title') self.post_page(submission, {'save': True, **draft_data}, 'edit') submission = self.refresh(submission) - new_title = 'Newer title' - draft_data[submission.must_include['title']] = new_title + newer_title = 'Newer title' + draft_data = prepare_form_data(submission, title=newer_title) self.post_page(submission, {'submit': True, **draft_data}, 'edit') submission = self.refresh(submission) @@ -219,7 +251,7 @@ class TestRevisionsView(BaseSubmissionViewTestCase): self.assertDictEqual(submission.draft_revision.form_data, submission.from_draft().form_data) self.assertDictEqual(submission.live_revision.form_data, submission.form_data) - self.assertEqual(submission.title, new_title) + self.assertEqual(submission.title, newer_title) class TestRevisionCompare(BaseSubmissionViewTestCase): diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py index 8c0e2f5fc91767a46f2a34a847b4fe47a953f283..6511df5d3b1fdc7aa508600b6f9764751cdcd9ff 100644 --- a/opentech/apply/funds/views.py +++ b/opentech/apply/funds/views.py @@ -247,8 +247,7 @@ class SubmissionDetailView(ViewDispatcher): return super().admin_check(request) -@method_decorator(login_required, name='dispatch') -class SubmissionEditView(UpdateView): +class BaseSubmissionEditView(UpdateView): """ Converts the data held on the submission into an editable format and knows how to save that back to the object. Shortcuts the normal update view save approach @@ -256,23 +255,13 @@ class SubmissionEditView(UpdateView): model = ApplicationSubmission def dispatch(self, request, *args, **kwargs): - if request.user != self.get_object().user: - raise PermissionDenied if not self.get_object().phase.permissions.can_edit(request.user): raise PermissionDenied return super().dispatch(request, *args, **kwargs) - @property - def transitions(self): - transitions = self.object.get_available_user_status_transitions(self.request.user) - return { - transition.name: transition - for transition in transitions - } - def buttons(self): - yield ('save', 'Save') - yield from ((transition, transition.title) for transition in self.transitions) + yield ('save', 'white', 'Save') + yield ('submit', 'primary', 'Submit') def get_form_kwargs(self): kwargs = super().get_form_kwargs() @@ -286,6 +275,44 @@ class SubmissionEditView(UpdateView): def get_form_class(self): return self.object.get_form_class() + +@method_decorator(staff_required, name='dispatch') +class AdminSubmissionEditView(BaseSubmissionEditView): + def form_valid(self, form): + self.object.new_data(form.cleaned_data) + + if 'save' in self.request.POST: + self.object.create_revision(draft=True, by=self.request.user) + return self.form_invalid(form) + + if 'submit' in self.request.POST: + created = self.object.create_revision(by=self.request.user) + if created: + messenger( + MESSAGES.EDIT, + request=self.request, + user=self.request.user, + submission=self.object, + ) + + return HttpResponseRedirect(self.get_success_url()) + + +@method_decorator(login_required, name='dispatch') +class ApplicantSubmissionEditView(BaseSubmissionEditView): + def dispatch(self, request, *args, **kwargs): + if request.user != self.get_object().user: + raise PermissionDenied + return super().dispatch(request, *args, **kwargs) + + @property + def transitions(self): + transitions = self.object.get_available_user_status_transitions(self.request.user) + return { + transition.name: transition + for transition in transitions + } + def form_valid(self, form): self.object.new_data(form.cleaned_data) @@ -301,6 +328,11 @@ class SubmissionEditView(UpdateView): return HttpResponseRedirect(self.get_success_url()) +class SubmissionEditView(ViewDispatcher): + admin_view = AdminSubmissionEditView + applicant_view = ApplicantSubmissionEditView + + @method_decorator(staff_required, name='dispatch') class RevisionListView(ListView): model = ApplicationRevision @@ -322,6 +354,7 @@ class RevisionListView(ListView): ) +@method_decorator(staff_required, name='dispatch') class RevisionCompareView(DetailView): model = ApplicationSubmission template_name = 'funds/revisions_compare.html' diff --git a/opentech/apply/funds/workflow.py b/opentech/apply/funds/workflow.py index 59291fdd156d891b95978928a870023f76645915..6eebaa6925ef4753e2a388e584879467137c4117 100644 --- a/opentech/apply/funds/workflow.py +++ b/opentech/apply/funds/workflow.py @@ -117,6 +117,9 @@ class DefaultPermissions(BasePermissions): def can_staff_review(self, user: 'User') -> bool: return True + def can_staff_edit(self, user: 'User') -> bool: + return True + class ReviewerReviewPermissions(DefaultPermissions): def can_reviewer_review(self, user: 'User') -> bool: @@ -127,6 +130,10 @@ class CanEditPermissions(DefaultPermissions): def can_applicant_edit(self, user: 'User') -> bool: return True + def can_staff_edit(self, user: 'User') -> bool: + # Prevent staff editing whilst with the user for edits + return False + Request = Stage('Request', False)