diff --git a/opentech/apply/determinations/forms.py b/opentech/apply/determinations/forms.py index 14a8773936458ea485e7a3bb6c66bfbe93ada37c..8d7e85b58bc6f0e3a2d30862205c838176afe7be 100644 --- a/opentech/apply/determinations/forms.py +++ b/opentech/apply/determinations/forms.py @@ -1,13 +1,14 @@ from django import forms from django.core.exceptions import NON_FIELD_ERRORS +from opentech.apply.utils.options import RICH_TEXT_WIDGET + from .models import ( Determination, DETERMINATION_CHOICES, TRANSITION_DETERMINATION, ) - -from opentech.apply.utils.options import RICH_TEXT_WIDGET +from .utils import determination_actions class RichTextField(forms.CharField): @@ -59,7 +60,7 @@ class BaseDeterminationForm(forms.ModelForm): for field in self._meta.widgets: self.fields[field].disabled = True - self.fields['outcome'].choices = self.outcome_choices_for_phase(submission.phase) + self.fields['outcome'].choices = self.outcome_choices_for_phase(submission, user) if self.draft_button_name in self.data: for field in self.fields.values(): @@ -78,7 +79,7 @@ class BaseDeterminationForm(forms.ModelForm): return super().save(commit) - def outcome_choices_for_phase(self, phase): + def outcome_choices_for_phase(self, submission, user): """ Outcome choices correspond to Phase transitions. We need to filter out non-matching choices. @@ -87,7 +88,7 @@ class BaseDeterminationForm(forms.ModelForm): available_choices = set() choices = dict(self.fields['outcome'].choices) - for transition_name in phase.transitions: + for transition_name in determination_actions(user, submission): try: determination_type = TRANSITION_DETERMINATION[transition_name] except KeyError: diff --git a/opentech/apply/determinations/templates/determinations/determination_detail.html b/opentech/apply/determinations/templates/determinations/determination_detail.html index 4948b34b719b39f3704fa8f8eb3c23344900455c..cc968e9c94f2dcc80cb0366def45a54a27beb22d 100644 --- a/opentech/apply/determinations/templates/determinations/determination_detail.html +++ b/opentech/apply/determinations/templates/determinations/determination_detail.html @@ -4,7 +4,7 @@ {% block content %} <div class="admin-bar"> <div class="admin-bar__inner"> - <h2 class="heading heading--no-margin">Determination</h2> + <h2 class="heading heading--no-margin">Determination {% if determination.is_draft %}[DRAFT]{% endif %}</h2> <h5>For <a href="{% url "funds:submissions:detail" determination.submission.id %}">{{ determination.submission.title }}</a></h5> </div> </div> @@ -16,15 +16,13 @@ </div> </div> - {% if can_view_extended_data %} - <div class="rich-text rich-text--answers"> - {% for group in determination_data.values %} - <h4>{{ group.title }}</h4> - {% for question, answer in group.questions %} - <h5>{{ question }}</h5> - {% if answer %}{{ answer|bleach }}{% else %}-{% endif %} - {% endfor %} - {% endfor %} - </div> - {% endif %} +<div class="rich-text rich-text--answers"> + {% for group in determination.detailed_data.values %} + <h4>{{ group.title }}</h4> + {% for question, answer in group.questions %} + <h5>{{ question }}</h5> + {% if answer %}{{ answer|bleach }}{% else %}-{% endif %} + {% endfor %} + {% endfor %} +</div> {% endblock %} diff --git a/opentech/apply/determinations/templates/determinations/determination_form.html b/opentech/apply/determinations/templates/determinations/determination_form.html index 6f119037cc8d228d9f09883edb1ec4a6f828f43c..65e012705d27bcb408705140cffe02baa22fc3f8 100644 --- a/opentech/apply/determinations/templates/determinations/determination_form.html +++ b/opentech/apply/determinations/templates/determinations/determination_form.html @@ -4,13 +4,12 @@ {% block content %} <div class="admin-bar"> <div class="admin-bar__inner"> - <h2 class="heading heading--no-margin">{{ title|default:"Create Determination" }}</h2> + <h2 class="heading heading--no-margin">{% if object %}Update Determination draft{% else %}Add Determination{% endif %}</h2> <h5>For <a href="{% url "funds:submissions:detail" submission.id %}">{{ submission.title }}</a></h5> </div> </div> <div class="wrapper wrapper--medium wrapper--inner-space-medium"> -{% if not has_determination_response %} <form class="form form--with-p-tags" action="" method="post" novalidate> {{ form.media }} {% csrf_token %} @@ -43,8 +42,5 @@ {{ message|bleach }} </div> {% endfor %} -{% else %} - <p>You have already added a determination for this submission</p> -{% endif %} </div> {% endblock %} diff --git a/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html b/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html new file mode 100644 index 0000000000000000000000000000000000000000..6e361cbf7299d0f185e0829da6d6149f2da78400 --- /dev/null +++ b/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html @@ -0,0 +1,12 @@ +{% load determination_tags %} +{% if object.has_determination %} +<div class="sidebar__inner"> + <h5>Determination</h5> + <p> + <a class="link link--bold" href="{% url 'apply:submissions:determinations:detail' submission_pk=object.id %}"> + {{ object.determination.get_outcome_display }} + </a> + - {{ object.determination.updated_at|date:"Y-m-d" }} by {{ object.determination.author }} + </p> +</div> +{% endif %} diff --git a/opentech/apply/determinations/templates/determinations/includes/determination_block.html b/opentech/apply/determinations/templates/determinations/includes/determination_block.html index 1f64d25be86ea32a53ab97eb5907aae235912b9c..584e4047559d9e6fbf08d43c0e5f58b09c3dd482 100644 --- a/opentech/apply/determinations/templates/determinations/includes/determination_block.html +++ b/opentech/apply/determinations/templates/determinations/includes/determination_block.html @@ -1,15 +1,15 @@ {% load determination_tags %} -{% if not object|pending_determination:request.user %} - <div class="sidebar__inner"> - <h5>Determination</h5> - {% if object.determination %} - <p> - <a class="link link--bold" href="{% url 'apply:submissions:determinations:detail' submission_pk=object.id %}"> - {% if object|has_determination_draft:request.user %}[Draft] {% endif %}{{ object.determination.get_outcome_display }} - </a> - - {{ object.determination.updated_at|date:"Y-m-d" }} by {{ object.determination.author }} - </p> - {% endif %} - {% include 'determinations/includes/determination_button.html' with submission=object %} - </div> -{% endif %} +<div class="sidebar__inner"> + <h5>Determination</h5> + <p> + {% if object.determination %} + <a class="link link--bold" href="{% url 'apply:submissions:determinations:detail' submission_pk=object.id %}"> + {% if object.determination.is_draft %}[Draft] {% endif %}{{ object.determination.get_outcome_display }} + </a> + - {{ object.determination.updated_at|date:"Y-m-d" }} by {{ object.determination.author }} + {% else %} + Awaiting determination + {% endif %} + </p> + {% include 'determinations/includes/determination_button.html' with submission=object %} +</div> diff --git a/opentech/apply/determinations/templatetags/determination_tags.py b/opentech/apply/determinations/templatetags/determination_tags.py index 7b6098e596ef63c30251a49338152740c444d862..0f687f21e432a3001cb1ce2e8961a1326d0bd731 100644 --- a/opentech/apply/determinations/templatetags/determination_tags.py +++ b/opentech/apply/determinations/templatetags/determination_tags.py @@ -1,60 +1,10 @@ from django import template -from django.core.exceptions import ObjectDoesNotExist -from opentech.apply.determinations.models import NEEDS_MORE_INFO +from ..views import can_create_determination register = template.Library() @register.filter def can_add_determination(user, submission): - if not submission.can_have_determination: - return False - - if not submission.has_permission_to_add_determination(user): - return False - - return True - - -@register.filter -def has_determination_draft(submission, user): - try: - return submission.has_permission_to_add_determination(user) and submission.determination.is_draft - except ObjectDoesNotExist: - return False - - -@register.filter -def pending_determination(submission, user): - """ - Check when to show the determination block. - - The submission doesn't have a determination response (accepted / rejected), does not require more info - and is not in a determination state. - """ - if not submission.active or 'more_info' in submission.status: - return False - - if not submission.in_determination_phase: - return True - - try: - # No determination at all -> pending - if not submission.determination: - return True - - # When the determination is in draft, only the lead and admins can see that as such - if submission.has_permission_to_add_determination(user): - return False - - if submission.determination.is_draft: - return True - - # There is a determination, that is not draft. Yet we are in a determination phase (i.e. in discussion) - # If the determination outcome is that it needs - return submission.determination.outcome == NEEDS_MORE_INFO - - except ObjectDoesNotExist: - # No determination and we are ina determination phase - return not submission.has_permission_to_add_determination(user) + return can_create_determination(user, submission) diff --git a/opentech/apply/determinations/tests/factories.py b/opentech/apply/determinations/tests/factories.py index 7c4d6428bbeb4a504b17a82800d4678012f936c6..7b687ab0320079144fb987e7a25681aba1816d62 100644 --- a/opentech/apply/determinations/tests/factories.py +++ b/opentech/apply/determinations/tests/factories.py @@ -28,10 +28,9 @@ class DeterminationFactory(factory.DjangoModelFactory): model = Determination class Params: - submitted = factory.Trait(outcome=ACCEPTED, is_draft=False) accepted = factory.Trait(outcome=ACCEPTED) rejected = factory.Trait(outcome=REJECTED) - not_draft = factory.Trait(is_draft=False) + submitted = factory.Trait(is_draft=False) submission = factory.SubFactory(ApplicationSubmissionFactory) author = factory.SelfAttribute('submission.lead') diff --git a/opentech/apply/determinations/tests/test_views.py b/opentech/apply/determinations/tests/test_views.py index b5d8f49823dc05300e77ebbc06123f79a183ff9d..c63bb39634f14cb61aacbdd563e4dd592f984028 100644 --- a/opentech/apply/determinations/tests/test_views.py +++ b/opentech/apply/determinations/tests/test_views.py @@ -19,21 +19,19 @@ class StaffDeterminationsTestCase(BaseViewTestCase): def test_can_access_determination(self): submission = ApplicationSubmissionFactory(status='in_discussion') - determination = DeterminationFactory(submission=submission, author=self.user, not_draft=True) + determination = DeterminationFactory(submission=submission, author=self.user, submitted=True) response = self.get_page(determination) self.assertContains(response, determination.submission.title) self.assertContains(response, self.user.full_name) self.assertContains(response, reverse('funds:submissions:detail', kwargs={'pk': submission.id})) - self.assertFalse(response.context['can_view_extended_data']) def test_lead_can_access_determination(self): submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) - determination = DeterminationFactory(submission=submission, author=self.user, not_draft=True) + determination = DeterminationFactory(submission=submission, author=self.user, submitted=True) response = self.get_page(determination) self.assertContains(response, determination.submission.title) self.assertContains(response, self.user.full_name) self.assertContains(response, reverse('funds:submissions:detail', kwargs={'pk': submission.id})) - self.assertTrue(response.context['can_view_extended_data']) class DeterminationFormTestCase(BaseViewTestCase): @@ -50,22 +48,16 @@ class DeterminationFormTestCase(BaseViewTestCase): self.assertContains(response, submission.title) self.assertContains(response, reverse('funds:submissions:detail', kwargs={'pk': submission.id})) - def test_cannot_access_form_if_not_lead(self): - submission = ApplicationSubmissionFactory(status='in_discussion') - response = self.get_page(submission, 'form') - self.assertEqual(response.status_code, 403) - def test_cant_access_wrong_status(self): - submission = ApplicationSubmissionFactory() + submission = ApplicationSubmissionFactory(status='more_info') response = self.get_page(submission, 'form') self.assertEqual(response.status_code, 403) def test_cant_resubmit_determination(self): submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) - determination = DeterminationFactory(submission=submission, author=self.user, submitted=True) + determination = DeterminationFactory(submission=submission, author=self.user, accepted=True, submitted=True) response = self.post_page(submission, {'data': 'value', 'outcome': determination.outcome}, 'form') - self.assertTrue(response.context['has_determination_response']) - self.assertContains(response, 'You have already added a determination for this submission') + self.assertRedirects(response, self.url(submission)) def test_can_edit_draft_determination(self): submission = ApplicationSubmissionFactory(status='post_review_discussion', lead=self.user) @@ -80,11 +72,17 @@ class DeterminationFormTestCase(BaseViewTestCase): self.assertContains(response, reverse(self.url_name.format('form'), kwargs=self.get_kwargs(submission))) self.assertNotContains(response, 'Accepted determination draft message') + def test_can_edit_submitted_more_info(self): + submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) + DeterminationFactory(submission=submission, author=self.user, submitted=True) + response = self.get_page(submission, 'form') + self.assertContains(response, 'Update ') + def test_cannot_edit_draft_determination_if_not_lead(self): submission = ApplicationSubmissionFactory(status='in_discussion') - determination = DeterminationFactory(submission=submission, author=self.user) + determination = DeterminationFactory(submission=submission, author=self.user, accepted=True) response = self.post_page(submission, {'data': 'value', 'outcome': determination.outcome}, 'form') - self.assertEqual(response.status_code, 403) + self.assertRedirects(response, self.url(submission)) def test_sends_message_if_requires_more_info(self): submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) diff --git a/opentech/apply/determinations/utils.py b/opentech/apply/determinations/utils.py new file mode 100644 index 0000000000000000000000000000000000000000..f8642cccf9b12f59811e2702a652b38a2c0c3a36 --- /dev/null +++ b/opentech/apply/determinations/utils.py @@ -0,0 +1,29 @@ +from opentech.apply.funds.workflow import DETERMINATION_OUTCOMES + +from .models import TRANSITION_DETERMINATION + + +def determination_actions(user, submission): + return [action[0] for action in submission.get_actions_for_user(user)] + + +def transition_from_outcome(outcome, submission): + for transition_name in submission.phase.transitions: + try: + transition_type = TRANSITION_DETERMINATION[transition_name] + except KeyError: + pass + else: + if transition_type == outcome: + return transition_name + + +def can_edit_determination(user, determination, submission): + outcome = transition_from_outcome(determination.outcome, submission) + valid_outcomes = determination_actions(user, submission) + return outcome in valid_outcomes + + +def can_create_determination(user, submission): + actions = determination_actions(user, submission) + return any(action in DETERMINATION_OUTCOMES for action in actions) diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py index a8154435f3767e20d2e93d12a719dcde3c09287a..d537eb3edf7b389af325be5216baf65042ea8302 100644 --- a/opentech/apply/determinations/views.py +++ b/opentech/apply/determinations/views.py @@ -1,18 +1,23 @@ +from django.contrib import messages from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy from django.utils.decorators import method_decorator +from django.utils.translation import ugettext_lazy as _ from django.views.generic import DetailView from opentech.apply.activity.models import Activity from opentech.apply.activity.messaging import messenger, MESSAGES from opentech.apply.funds.models import ApplicationSubmission -from opentech.apply.utils.views import CreateOrUpdateView +from opentech.apply.utils.views import CreateOrUpdateView, ViewDispatcher +from opentech.apply.users.decorators import staff_required from .forms import ConceptDeterminationForm, ProposalDeterminationForm -from .models import Determination, TRANSITION_DETERMINATION, DeterminationMessageSettings, NEEDS_MORE_INFO +from .models import Determination, DeterminationMessageSettings, NEEDS_MORE_INFO + +from .utils import can_create_determination, can_edit_determination, transition_from_outcome def get_form_for_stage(submission): @@ -21,6 +26,7 @@ def get_form_for_stage(submission): return forms[index] +@method_decorator(staff_required, name='dispatch') class DeterminationCreateOrUpdateView(CreateOrUpdateView): model = Determination template_name = 'determinations/determination_form.html' @@ -31,28 +37,30 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): def dispatch(self, request, *args, **kwargs): self.submission = get_object_or_404(ApplicationSubmission, id=self.kwargs['submission_pk']) - if not self.submission.in_determination_phase \ - or not self.submission.has_permission_to_add_determination(request.user): + if not can_create_determination(request.user, self.submission): raise PermissionDenied() + if self.submission.has_determination: + messages.warning(request, _('A determination has already been submitted for that submission.')) + return HttpResponseRedirect(reverse_lazy('apply:submissions:determinations:detail', args=(self.submission.id,))) + try: - submitted = self.get_object().submitted + determination = self.get_object() except Determination.DoesNotExist: - submitted = False - - if self.request.POST and submitted: - return self.get(request, *args, **kwargs) + pass + else: + if not can_edit_determination(request.user, determination, self.submission): + messages.warning(request, _('There is a draft determination you do not have permission to edit.')) + return HttpResponseRedirect(reverse_lazy('apply:submissions:determinations:detail', args=(self.submission.id,))) return super().dispatch(request, *args, **kwargs) def get_context_data(self, **kwargs): - messages = DeterminationMessageSettings.for_site(self.request.site) + determination_messages = DeterminationMessageSettings.for_site(self.request.site) return super().get_context_data( submission=self.submission, - has_determination_response=self.submission.has_determination, - title="Update Determination draft" if self.object else 'Add Determination', - message_templates=messages.get_for_stage(self.submission.stage.name), + message_templates=determination_messages.get_for_stage(self.submission.stage.name), **kwargs ) @@ -79,7 +87,7 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): user=self.object.author, submission=self.object.submission, ) - transition = self.transition_from_outcome(int(form.cleaned_data.get('outcome'))) + transition = transition_from_outcome(int(form.cleaned_data.get('outcome')), self.submission) if self.object.outcome == NEEDS_MORE_INFO: # We keep a record of the message sent to the user in the comment @@ -107,19 +115,26 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): ) return HttpResponseRedirect(instance.get_absolute_url()) - def transition_from_outcome(self, outcome): - for transition_name in self.submission.phase.transitions: - try: - transition_type = TRANSITION_DETERMINATION[transition_name] - except KeyError: - pass - else: - if transition_type == outcome: - return transition_name + +@method_decorator(staff_required, name='dispatch') +class AdminDeterminationDetailView(DetailView): + model = Determination + + def get_object(self, queryset=None): + return self.model.objects.get(submission=self.submission) + + def dispatch(self, request, *args, **kwargs): + self.submission = get_object_or_404(ApplicationSubmission, id=self.kwargs['submission_pk']) + determination = self.get_object() + + if can_edit_determination(request.user, determination, self.submission) and determination.is_draft: + return HttpResponseRedirect(reverse_lazy('apply:submissions:determinations:form', args=(self.submission.id,))) + + return super().dispatch(request, *args, **kwargs) @method_decorator(login_required, name='dispatch') -class DeterminationDetailView(DetailView): +class ApplicantDeterminationDetailView(DetailView): model = Determination def get_object(self, queryset=None): @@ -129,20 +144,15 @@ class DeterminationDetailView(DetailView): self.submission = get_object_or_404(ApplicationSubmission, id=self.kwargs['submission_pk']) determination = self.get_object() - if request.user != self.submission.user and not request.user.is_apply_staff and not \ - self.submission.has_permission_to_add_determination(request.user): + if request.user != self.submission.user: raise PermissionDenied if determination.is_draft: - return HttpResponseRedirect(reverse_lazy('apply:submissions:determinations:form', args=(self.submission.id,))) + return HttpResponseRedirect(reverse_lazy('apply:submissions:determinations:detail', args=(self.submission.id,))) return super().dispatch(request, *args, **kwargs) - def get_context_data(self, **kwargs): - determination = self.get_object() - return super().get_context_data( - can_view_extended_data=determination.submission.has_permission_to_add_determination(self.request.user), - determination_data=determination.detailed_data, - **kwargs - ) +class DeterminationDetailView(ViewDispatcher): + admin_view = AdminDeterminationDetailView + applicant_view = ApplicantDeterminationDetailView diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py index 1953e26c7cb84306777f7674419850e927b2568f..779f551c64b28956b975dfca4cbf5ffe5c749b49 100644 --- a/opentech/apply/funds/models/submissions.py +++ b/opentech/apply/funds/models/submissions.py @@ -30,7 +30,6 @@ from .utils import LIMIT_TO_STAFF, LIMIT_TO_STAFF_AND_REVIEWERS, WorkflowHelpers from ..blocks import ApplicationCustomFormFieldsBlock, REQUIRED_BLOCK_NAMES from ..workflow import ( active_statuses, - DETERMINATION_PHASES, DETERMINATION_RESPONSE_PHASES, get_review_statuses, INITIAL_STATE, @@ -495,13 +494,6 @@ class ApplicationSubmission( return self.has_permission_to_review(user) - def has_permission_to_add_determination(self, user): - return user.is_superuser or self.lead == user - - @property - def in_determination_phase(self): - return self.status in DETERMINATION_PHASES - @property def has_determination(self): try: @@ -509,10 +501,6 @@ class ApplicationSubmission( except ObjectDoesNotExist: return False - @property - def can_have_determination(self): - return self.in_determination_phase and not self.has_determination - def prepare_search_values(self): for field_id in self.question_field_ids: field = self.field(field_id) diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html index de100c8b44bcd4b2a86f3166f9c4e650785a43a4..02c5284a13187c65a4d2c6117d78fed7124a402d 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html @@ -27,6 +27,10 @@ </div> {% endblock %} +{% block determination %} + {% include 'determinations/includes/determination_block.html' with submission=object %} +{% endblock %} + {% block extra_js %} {{ reviewer_form.media }} {% endblock %} diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html index 0b9288328f1af3e5a9f66c71e7fdb43a76c3423a..dbd743394d2100eb2763d835523f9d7059400223 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html @@ -83,7 +83,7 @@ {% endif %} {% block determination %} - {% include 'determinations/includes/determination_block.html' with submission=object %} + {% include 'determinations/includes/applicant_determination_block.html' with submission=object %} {% endblock %} {% block reviews %}