diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py index 3ba3154b72a98f21875c752744ac8d68bffe74e1..edb540b2130358a560fcd71ed60f63730b332cca 100644 --- a/opentech/apply/activity/messaging.py +++ b/opentech/apply/activity/messaging.py @@ -112,7 +112,7 @@ class ActivityAdapter(AdapterBase): MESSAGES.EDIT: 'Edited', MESSAGES.APPLICANT_EDIT: 'Edited', MESSAGES.UPDATE_LEAD: 'Lead changed from {old.lead} to {submission.lead}', - MESSAGES.DETERMINATION_OUTCOME: 'Sent a determination. Outcome: {submission.determination.clean_outcome}', + MESSAGES.DETERMINATION_OUTCOME: 'Sent a determination. Outcome: {determination.clean_outcome}', MESSAGES.INVITED_TO_PROPOSAL: 'Invited to submit a proposal', MESSAGES.REVIEWERS_UPDATED: 'reviewers_updated', MESSAGES.NEW_REVIEW: 'Submitted a review', @@ -162,7 +162,7 @@ class SlackAdapter(AdapterBase): MESSAGES.APPLICANT_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}', + MESSAGES.DETERMINATION_OUTCOME: 'A determination for <{link}|{submission.title}> was sent by email. Outcome: {determination.clean_outcome}', MESSAGES.PROPOSAL_SUBMITTED: 'A proposal has been submitted for review: <{link}|{submission.title}>', MESSAGES.INVITED_TO_PROPOSAL: '<{link}|{submission.title}> by {submission.user} has been invited to submit a proposal', MESSAGES.NEW_REVIEW: '{user} has submitted a review for <{link}|{submission.title}>. Outcome: {review.outcome}, Score: {review.score}', diff --git a/opentech/apply/activity/templates/messages/email/determination.html b/opentech/apply/activity/templates/messages/email/determination.html index 3350013084a9e255324ceb63d94358e8fb849210..483a897a9a53777fd00fcab5c1afabdceb78079c 100644 --- a/opentech/apply/activity/templates/messages/email/determination.html +++ b/opentech/apply/activity/templates/messages/email/determination.html @@ -4,19 +4,7 @@ {% block title %}Determination for {{ object.title }}{% endblock %} {% block content %} -Your application has been reviewed and the outcome is: {{ submission.determination.clean_outcome }} +Your application has been reviewed and the outcome is: {{ determination.clean_outcome }} -{{ submission.determination.message|bleach }} -{% endblock %} - - -{% block post_signature_content %} -{% if submission.determination.submitted %} - {% for group in submission.determination.detailed_data.values %} - {% for question, answer in group.questions %} - <h5>{{ question }}</h5> - {% if answer %}{{ answer|bleach }}{% else %}-{% endif %} - {% endfor %} - {% endfor %} -{% endif %} +{{ determination.message|bleach }} {% endblock %} diff --git a/opentech/apply/determinations/migrations/0006_allow_multiple_determinations.py b/opentech/apply/determinations/migrations/0006_allow_multiple_determinations.py new file mode 100644 index 0000000000000000000000000000000000000000..7a4bddebee0c13c3d035edb88e92f3831ea2a045 --- /dev/null +++ b/opentech/apply/determinations/migrations/0006_allow_multiple_determinations.py @@ -0,0 +1,19 @@ +# Generated by Django 2.0.2 on 2018-09-05 13:27 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('determinations', '0005_determination_drupal_id'), + ] + + operations = [ + migrations.AlterField( + model_name='determination', + name='submission', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='determinations', to='funds.ApplicationSubmission'), + ), + ] diff --git a/opentech/apply/determinations/models.py b/opentech/apply/determinations/models.py index 2824c367a2205aa086e598c0d1e7fd2c24508945..0051c20bb4bd97797b7d826ab5df49e958cc6dd5 100644 --- a/opentech/apply/determinations/models.py +++ b/opentech/apply/determinations/models.py @@ -34,11 +34,23 @@ TRANSITION_DETERMINATION = { } +class DeterminationQuerySet(models.QuerySet): + def active(self): + # Designed to be used with a queryset related to submissions + return self.get(is_draft=True) + + def submitted(self): + return self.filter(is_draft=False) + + def final(self): + return self.submitted().filter(outcome__in=[ACCEPTED, REJECTED]) + + class Determination(models.Model): - submission = models.OneToOneField( + submission = models.ForeignKey( 'funds.ApplicationSubmission', on_delete=models.CASCADE, - related_name='determination' + related_name='determinations' ) author = models.ForeignKey( settings.AUTH_USER_MODEL, @@ -55,6 +67,8 @@ class Determination(models.Model): # Meta: used for migration purposes only drupal_id = models.IntegerField(null=True, blank=True, editable=False) + objects = DeterminationQuerySet.as_manager() + @property def stripped_message(self): return bleach.clean(self.message, tags=[], strip=True) @@ -64,11 +78,11 @@ class Determination(models.Model): return self.get_outcome_display() def get_absolute_url(self): - return reverse('apply:submissions:determinations:detail', args=(self.id,)) + return reverse('apply:submissions:determinations:detail', args=(self.submission.id, self.id)) @property def submitted(self): - return self.outcome != NEEDS_MORE_INFO and not self.is_draft + return not self.is_draft def __str__(self): return f'Determination for {self.submission.title} by {self.author!s}' diff --git a/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html b/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html index 6e361cbf7299d0f185e0829da6d6149f2da78400..ef275a4492b974b77f895ac3edf218bc0e4309d5 100644 --- a/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html +++ b/opentech/apply/determinations/templates/determinations/includes/applicant_determination_block.html @@ -1,12 +1,13 @@ -{% 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> + {% for determination in object.determinations.submitted %} + <p> + <a class="link link--bold" href="{% url 'apply:submissions:determinations:detail' submission_pk=object.id pk=determination.id %}"> + {{ determination.get_outcome_display }} + </a> + - {{ determination.updated_at|date:"Y-m-d" }} by {{ determination.author }} + </p> + {% empty %} + <p>Awaiting determination</p> + {% endfor %} </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 584e4047559d9e6fbf08d43c0e5f58b09c3dd482..92eb6baf1aeb3d9ba3bbc0d75032dfcb29297a0c 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 %} <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> + {% for determination in object.determinations.all %} + <p> + <a class="link link--bold" href="{% url 'apply:submissions:determinations:detail' submission_pk=object.id pk=determination.id %}"> + {% if determination.is_draft %}[Draft] {% endif %}{{ determination.get_outcome_display }} + </a> + - {{ determination.updated_at|date:"Y-m-d" }} by {{ determination.author }} + </p> + {% empty %} + <p>Awaiting determination</p> + {% endfor %} {% 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 d13b1baae741d5adf832e6be9be199ca63a510ca..7ca1a8e98eae98b51b650a12abbb9d1a24bed18f 100644 --- a/opentech/apply/determinations/templatetags/determination_tags.py +++ b/opentech/apply/determinations/templatetags/determination_tags.py @@ -9,6 +9,6 @@ register = template.Library() @register.filter def show_determination_button(user, submission): try: - return can_edit_determination(user, submission.determination, submission) + return can_edit_determination(user, submission.determinations.active(), submission) except ObjectDoesNotExist: return can_create_determination(user, submission) diff --git a/opentech/apply/determinations/tests/test_views.py b/opentech/apply/determinations/tests/test_views.py index c63bb39634f14cb61aacbdd563e4dd592f984028..5004cb5d5a8f26a7d9d901052907ac764a8c8753 100644 --- a/opentech/apply/determinations/tests/test_views.py +++ b/opentech/apply/determinations/tests/test_views.py @@ -1,5 +1,3 @@ -from django.urls import reverse - from opentech.apply.activity.models import Activity from opentech.apply.determinations.models import ACCEPTED from opentech.apply.users.tests.factories import StaffFactory, UserFactory @@ -15,7 +13,7 @@ class StaffDeterminationsTestCase(BaseViewTestCase): base_view_name = 'detail' def get_kwargs(self, instance): - return {'submission_pk': instance.submission.id} + return {'submission_pk': instance.submission.id, 'pk': instance.pk} def test_can_access_determination(self): submission = ApplicationSubmissionFactory(status='in_discussion') @@ -23,7 +21,7 @@ class StaffDeterminationsTestCase(BaseViewTestCase): 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.assertContains(response, submission.get_absolute_url()) def test_lead_can_access_determination(self): submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) @@ -31,7 +29,7 @@ class StaffDeterminationsTestCase(BaseViewTestCase): 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.assertContains(response, submission.get_absolute_url()) class DeterminationFormTestCase(BaseViewTestCase): @@ -40,24 +38,27 @@ class DeterminationFormTestCase(BaseViewTestCase): base_view_name = 'detail' def get_kwargs(self, instance): + return {'submission_pk': instance.id, 'pk': instance.determinations.first().id} + + def get_form_kwargs(self, instance): return {'submission_pk': instance.id} def test_can_access_form_if_lead(self): submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) response = self.get_page(submission, 'form') self.assertContains(response, submission.title) - self.assertContains(response, reverse('funds:submissions:detail', kwargs={'pk': submission.id})) + self.assertContains(response, submission.get_absolute_url()) def test_cant_access_wrong_status(self): submission = ApplicationSubmissionFactory(status='more_info') response = self.get_page(submission, 'form') - self.assertEqual(response.status_code, 403) + self.assertRedirects(response, self.absolute_url(submission.get_absolute_url())) def test_cant_resubmit_determination(self): submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user) determination = DeterminationFactory(submission=submission, author=self.user, accepted=True, submitted=True) response = self.post_page(submission, {'data': 'value', 'outcome': determination.outcome}, 'form') - self.assertRedirects(response, self.url(submission)) + self.assertRedirects(response, self.absolute_url(submission.get_absolute_url())) def test_can_edit_draft_determination(self): submission = ApplicationSubmissionFactory(status='post_review_discussion', lead=self.user) @@ -69,14 +70,14 @@ class DeterminationFormTestCase(BaseViewTestCase): 'save_draft': True, }, 'form') self.assertContains(response, '[Draft] Approved') - self.assertContains(response, reverse(self.url_name.format('form'), kwargs=self.get_kwargs(submission))) + self.assertContains(response, self.url(submission, 'form', absolute=False)) self.assertNotContains(response, 'Accepted determination draft message') - def test_can_edit_submitted_more_info(self): + def test_cant_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 ') + self.assertNotContains(response, 'Update ') def test_cannot_edit_draft_determination_if_not_lead(self): submission = ApplicationSubmissionFactory(status='in_discussion') @@ -110,7 +111,7 @@ class DeterminationFormTestCase(BaseViewTestCase): submission_next = submission_original.next # Cannot use self.url() as that uses a different base. - url = reverse('funds:submissions:detail', kwargs={'pk': submission_next.id}) + url = submission_next.get_absolute_url() self.assertRedirects(response, self.factory.get(url, secure=True).build_absolute_uri(url)) self.assertEqual(submission_original.status, 'invited_to_proposal') self.assertEqual(submission_next.status, 'draft_proposal') diff --git a/opentech/apply/determinations/urls.py b/opentech/apply/determinations/urls.py index b2e59b256c67ded0c9344e1dff1b2e972bc0b928..4c0775e9f1629268a81326acfb00a3a3a1ea049a 100644 --- a/opentech/apply/determinations/urls.py +++ b/opentech/apply/determinations/urls.py @@ -5,6 +5,6 @@ from .views import DeterminationCreateOrUpdateView, DeterminationDetailView app_name = 'determinations' urlpatterns = [ - path('determination/', DeterminationDetailView.as_view(), name="detail"), + path('determination/<int:pk>/', DeterminationDetailView.as_view(), name="detail"), path('determination/add/', DeterminationCreateOrUpdateView.as_view(), name="form"), ] diff --git a/opentech/apply/determinations/utils.py b/opentech/apply/determinations/utils.py index f8642cccf9b12f59811e2702a652b38a2c0c3a36..e698fae5cd391434ad925083469957509267b9bc 100644 --- a/opentech/apply/determinations/utils.py +++ b/opentech/apply/determinations/utils.py @@ -27,3 +27,7 @@ def can_edit_determination(user, determination, submission): def can_create_determination(user, submission): actions = determination_actions(user, submission) return any(action in DETERMINATION_OUTCOMES for action in actions) + + +def has_final_determination(submission): + return submission.determinations.final().exists() diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py index 0e9a3447790d2dc6e6069a62d9682f547f102b1c..2225a20aed84cf1d0b2ef5777802bddbd1959d78 100644 --- a/opentech/apply/determinations/views.py +++ b/opentech/apply/determinations/views.py @@ -17,7 +17,7 @@ from opentech.apply.users.decorators import staff_required from .forms import ConceptDeterminationForm, ProposalDeterminationForm from .models import Determination, DeterminationMessageSettings, NEEDS_MORE_INFO -from .utils import can_create_determination, can_edit_determination, transition_from_outcome +from .utils import can_create_determination, can_edit_determination, has_final_determination, transition_from_outcome def get_form_for_stage(submission): @@ -32,29 +32,35 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): template_name = 'determinations/determination_form.html' def get_object(self, queryset=None): - return self.model.objects.get(submission=self.submission) + return self.model.objects.get(submission=self.submission, is_draft=True) def dispatch(self, request, *args, **kwargs): self.submission = get_object_or_404(ApplicationSubmission, id=self.kwargs['submission_pk']) if not can_create_determination(request.user, self.submission): - raise PermissionDenied() + return self.back_to_submission(_('You do not have permission to create that determination.')) - 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,))) + if has_final_determination(self.submission): + return self.back_to_submission(_('A final determination has already been submitted.')) try: - determination = self.get_object() + self.determination = self.get_object() except Determination.DoesNotExist: 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,))) + if not can_edit_determination(request.user, self.determination, self.submission): + return self.back_to_detail(_('There is a draft determination you do not have permission to edit.')) return super().dispatch(request, *args, **kwargs) + def back_to_submission(self, message): + messages.warning(self.request, message) + return HttpResponseRedirect(self.submission.get_absolute_url()) + + def back_to_detail(self, message): + messages.warning(self.request, message) + return HttpResponseRedirect(self.determination.get_absolute_url()) + def get_context_data(self, **kwargs): determination_messages = DeterminationMessageSettings.for_site(self.request.site) @@ -86,6 +92,7 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): request=self.request, user=self.object.author, submission=self.object.submission, + determination=self.object, ) transition = transition_from_outcome(int(form.cleaned_data.get('outcome')), self.submission) @@ -107,7 +114,7 @@ class AdminDeterminationDetailView(DetailView): model = Determination def get_object(self, queryset=None): - return self.model.objects.get(submission=self.submission) + return self.model.objects.get(submission=self.submission, id=self.kwargs['pk']) def dispatch(self, request, *args, **kwargs): self.submission = get_object_or_404(ApplicationSubmission, id=self.kwargs['submission_pk']) diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py index 8af1b3568a67e549a23670bcdcb01859ec003d4d..f4fcb4a04c7ef714a25d62c95b1e9945ba7da5d9 100644 --- a/opentech/apply/funds/models/submissions.py +++ b/opentech/apply/funds/models/submissions.py @@ -8,7 +8,7 @@ from django.core.exceptions import PermissionDenied from django.core.files.storage import get_storage_class from django.core.serializers.json import DjangoJSONEncoder from django.db import models -from django.db.models import Count, IntegerField, OuterRef, ObjectDoesNotExist, Subquery, Sum +from django.db.models import Count, IntegerField, OuterRef, Subquery, Sum from django.db.models.expressions import RawSQL, OrderBy from django.db.models.functions import Coalesce from django.dispatch import receiver @@ -563,13 +563,6 @@ class ApplicationSubmission( return self.has_permission_to_review(user) - @property - def has_determination(self): - try: - return self.determination.submitted - except ObjectDoesNotExist: - return False - def prepare_search_values(self): for field_id in self.question_field_ids: field = self.field(field_id) diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py index 31b040a4fb7ec426a9421161b0bff7d7903c5080..eb27dc0dd2223153aea20c107748975e8a337ad7 100644 --- a/opentech/apply/funds/views.py +++ b/opentech/apply/funds/views.py @@ -80,7 +80,7 @@ class ProgressSubmissionView(DelegatedViewMixin, UpdateView): def form_valid(self, form): action = form.cleaned_data.get('action') # Defer to the determination form for any of the determination transitions - if action in DETERMINATION_OUTCOMES and not self.object.has_determination: + if action in DETERMINATION_OUTCOMES: return HttpResponseRedirect(reverse_lazy( 'apply:submissions:determinations:form', args=(form.instance.id,)) + "?action=" + action) diff --git a/opentech/apply/utils/testing/tests.py b/opentech/apply/utils/testing/tests.py index d24705586891813426f16270582d78d46016f466..bba277b3d117141767a458d942b252b9a2cd63b6 100644 --- a/opentech/apply/utils/testing/tests.py +++ b/opentech/apply/utils/testing/tests.py @@ -35,15 +35,25 @@ class BaseViewTestCase(TestCase): def get_kwargs(self, instance): return {} - def url(self, instance, view_name=None, absolute=True): - full_url_name = self.url_name.format(view_name or self.base_view_name) - return self.url_from_pattern(full_url_name, self.get_kwargs(instance), secure=True, absolute=absolute) + def url(self, instance, view_name=None, absolute=True, kwargs=dict()): + view = view_name or self.base_view_name + full_url_name = self.url_name.format(view) + kwargs_method = f'get_{view}_kwargs' + if hasattr(self, kwargs_method): + kwargs = getattr(self, kwargs_method)(instance) + else: + kwargs = self.get_kwargs(instance) + return self.url_from_pattern(full_url_name, kwargs, secure=True, absolute=absolute) + + def absolute_url(self, location, secure=True): + request = self.factory.get(location, secure=secure) + return request.build_absolute_uri() def url_from_pattern(self, pattern, kwargs=None, secure=True, absolute=True): url = reverse(pattern, kwargs=kwargs) - request = self.factory.get(url, secure=secure) if absolute: - return request.build_absolute_uri() + return self.absolute_url(url) + request = self.factory.get(url, secure=secure) return request.path def get_page(self, instance=None, view_name=None):