diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py index ad14f99adcbee394e90ad26e84d2a37e084b5d62..a067900c00be2986f45590e369374f1a173747cd 100644 --- a/opentech/apply/activity/messaging.py +++ b/opentech/apply/activity/messaging.py @@ -48,7 +48,7 @@ neat_related = { MESSAGES.EDIT: 'revision', MESSAGES.COMMENT: 'comment', MESSAGES.SCREENING: 'old_status', - MESSAGES.REVIEW_OPINION: 'review_opinion', + MESSAGES.REVIEW_OPINION: 'opinion', } @@ -194,7 +194,7 @@ class ActivityAdapter(AdapterBase): MESSAGES.NEW_REVIEW: 'Submitted a review', MESSAGES.OPENED_SEALED: 'Opened the submission while still sealed', MESSAGES.SCREENING: 'Screening status from {old_status} to {submission.screening_status}', - MESSAGES.REVIEW_OPINION: '{user} {opinion_verb} with {reviewer}''s review of {submission}' + MESSAGES.REVIEW_OPINION: '{user} {opinion.opinion_display}s with {opinion.review.author}''s review of {submission}' } def recipients(self, message_type, **kwargs): @@ -295,7 +295,7 @@ class SlackAdapter(AdapterBase): MESSAGES.NEW_REVIEW: '{user} has submitted a review for <{link}|{submission.title}>. Outcome: {review.outcome}, Score: {review.score}', MESSAGES.READY_FOR_REVIEW: 'notify_reviewers', MESSAGES.OPENED_SEALED: '{user} has opened the sealed submission: <{link}|{submission.title}>', - MESSAGES.REVIEW_OPINION: '{user} {opinion_verb} with {reviewer}''s review of {submission}', + MESSAGES.REVIEW_OPINION: '{user} {opinion.opinion_display}s with {opinion.review.author}''s review of {submission}', } def __init__(self): diff --git a/opentech/apply/review/forms.py b/opentech/apply/review/forms.py index eb00c505fbcc000ed1fa99ad38e2413e4ef3c9b9..0d29c09ea311ac510dadc7422f527ce0ad0dfc38 100644 --- a/opentech/apply/review/forms.py +++ b/opentech/apply/review/forms.py @@ -1,10 +1,12 @@ from django import forms from django.core.exceptions import NON_FIELD_ERRORS +from django.utils.html import escape from opentech.apply.review.options import NA from opentech.apply.stream_forms.forms import StreamBaseForm from .models import Review, ReviewOpinion +from .options import OPINION_CHOICES class MixedMetaClass(type(StreamBaseForm), type(forms.ModelForm)): @@ -86,13 +88,52 @@ class ReviewModelForm(StreamBaseForm, forms.ModelForm, metaclass=MixedMetaClass) return NA +class SubmitButtonWidget(forms.Widget): + def render(self, name, value, attrs=None): + disabled = 'disabled' if attrs.get('disabled') else '' + return '<input type="submit" name="{name}" value="{value}" class="button button--primary button--bottom-space" {disabled}>'.format( + disabled=disabled, + name=escape(name), + value=escape(name.title()), + ) + + +class OpinionField(forms.IntegerField): + def __init__(self, *args, opinion, **kwargs): + kwargs["widget"] = SubmitButtonWidget + self.opinion = opinion + kwargs['label'] = '' + super().__init__(*args, **kwargs) + + def clean(self, value): + if value: + return self.opinion + + class ReviewOpinionForm(forms.ModelForm): - agree = forms.IntegerField() + opinion = forms.IntegerField(required=False, widget=forms.HiddenInput()) class Meta: model = ReviewOpinion - fields = () + fields = ('opinion',) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + for value, opinion in OPINION_CHOICES: + self.fields[opinion.lower()] = OpinionField( + label=opinion.title(), + opinion=value, + disabled=self.instance.opinion == value, + ) def clean(self): cleaned_data = super().clean() - cleaned_data['opinion'] = cleaned_data['agree'] + opinions = [cleaned_data.get(opinion.lower()) for _, opinion in OPINION_CHOICES] + valid_opinions = [opinion for opinion in opinions if opinion is not None] + if len(valid_opinions) > 1: + self.add_error(None, 'Cant submit both an agreement and disagreement') + cleaned_data = {'opinion': valid_opinions[0]} + return cleaned_data + + def save(self, *args, **kwargs): + return super().save(*args, **kwargs) diff --git a/opentech/apply/review/models.py b/opentech/apply/review/models.py index 1f20b0e415509ad891d335f8c7ef800209d5f2f3..294b622e86c225e0ef4d4bb4e72d8e6d1f234ee4 100644 --- a/opentech/apply/review/models.py +++ b/opentech/apply/review/models.py @@ -180,3 +180,7 @@ class ReviewOpinion(models.Model): class Meta: unique_together = ('author', 'review') + + @property + def opinion_display(self): + return self.get_opinion_display() diff --git a/opentech/apply/review/templates/review/review_detail.html b/opentech/apply/review/templates/review/review_detail.html index f09f76cc442e0bc7815c8c21177f7dc0b2810a05..0e0c909c50804e9b8fe05d60a4a7e5092d08b68e 100644 --- a/opentech/apply/review/templates/review/review_detail.html +++ b/opentech/apply/review/templates/review/review_detail.html @@ -34,15 +34,10 @@ {{ object.output_answers|submission_links }} </div> -{% if opinion_choices %} +{% if form %} <form method="post"> {% csrf_token %} - {% for choice in opinion_choices %} - <button name="agree" type="submit" value="{{ choice.value }}" - class="button button--primary button--bottom-space {{ choice.disabled_class }}" {% if choice.disabled %}disabled{% endif %}> - {{ choice.label }} - </button> - {% endfor %} + {{ form }} </form> {% endif %} diff --git a/opentech/apply/review/tests/test_views.py b/opentech/apply/review/tests/test_views.py index a500da9c8111b5059047fb741a7b9ca40e5b60de..d58724ff3f442a40caa974033a3cbd9c7bc92570 100644 --- a/opentech/apply/review/tests/test_views.py +++ b/opentech/apply/review/tests/test_views.py @@ -226,19 +226,17 @@ class StaffReviewOpinionCase(BaseViewTestCase): staff = StaffFactory() review = ReviewFactory(submission=self.submission, author=staff, recommendation_yes=True) response = self.get_page(review) - self.assertContains(response, '<button name="agree"') - self.assertIn('opinion_choices', response.context_data) + self.assertContains(response, 'name="agree"') def test_cant_see_opinion_buttons_on_self_review(self): review = ReviewFactory(submission=self.submission, author=self.user, recommendation_yes=True) response = self.get_page(review) - self.assertNotContains(response, '<button name="agree"') - self.assertNotIn('opinion_choices', response.context_data) + self.assertNotContains(response, 'name="agree"') def test_can_add_opinion_to_others_review(self): staff = StaffFactory() review = ReviewFactory(submission=self.submission, author=staff, recommendation_yes=True) self.post_page(review, {'agree': AGREE}) - self.assertTrue('agrees' in Activity.objects.first().message) + self.assertTrue(review.opinions.first().opinion_display in Activity.objects.first().message) self.assertEqual(ReviewOpinion.objects.all().count(), 1) self.assertEqual(ReviewOpinion.objects.first().opinion, AGREE) diff --git a/opentech/apply/review/views.py b/opentech/apply/review/views.py index 57ea54fa831c754c2e7c0a57ec4864d41befaa62..d1ae45b51073ff6b5775e679bf6b7a3ba57ae8f1 100644 --- a/opentech/apply/review/views.py +++ b/opentech/apply/review/views.py @@ -6,8 +6,7 @@ from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy, reverse from django.utils.decorators import method_decorator -from django.views.generic import ListView, DetailView, FormView -from django.views.generic.detail import SingleObjectMixin +from django.views.generic import CreateView, ListView, DetailView from wagtail.core.blocks import RichTextBlock @@ -144,24 +143,17 @@ class ReviewDisplay(DetailView): model = Review def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - if self.get_object().author != self.request.user: - existing_opinion = ReviewOpinion.objects.filter( - author=self.request.user, review=self.get_object()).first() - opinion_choices = [] - for value, label in OPINION_CHOICES: - button_dict = { - 'value': value, - 'label': label, - 'disabled': False, - } - if existing_opinion and existing_opinion.opinion == value: - button_dict['disabled'] = True - button_dict['disabled_class'] = 'is-disabled' - opinion_choices.append(button_dict) - - context['opinion_choices'] = opinion_choices - return context + review = self.get_object() + if review.author != self.request.user: + consensus_form = ReviewOpinionForm( + instance=review.opinions.filter(author=self.request.user).first(), + ) + else: + consensus_form = None + return super().get_context_data( + form=consensus_form, + **kwargs, + ) def dispatch(self, request, *args, **kwargs): review = self.get_object() @@ -176,42 +168,35 @@ class ReviewDisplay(DetailView): return super().dispatch(request, *args, **kwargs) -class ReviewOpinionFormView(SingleObjectMixin, FormView): +class ReviewOpinionFormView(CreateView): template_name = 'review/review_detail.html' form_class = ReviewOpinionForm model = Review - def form_valid(self, form): + def get_form_kwargs(self): self.object = self.get_object() + kwargs = super().get_form_kwargs() + instance = kwargs['instance'] + kwargs['instance'] = instance.opinions.filter(author=self.request.user).first() + return kwargs + + def form_valid(self, form): + self.review = self.get_object() + form.instance.author = self.request.user + form.instance.review = self.review response = super().form_valid(form) - opinion = form.cleaned_data['opinion'] - existing_review = ReviewOpinion.objects.filter(author=self.request.user, review=self.object).first() - if existing_review: - existing_review.opinion = opinion - existing_review.save() - else: - ReviewOpinion.objects.create( - opinion=opinion, - author=self.request.user, - review=self.object) - if opinion == AGREE: - opinion_verb = 'agrees' - else: - opinion_verb = 'disagrees' messenger( MESSAGES.REVIEW_OPINION, request=self.request, user=self.request.user, - opinion_verb=opinion_verb, - reviewer=self.object.author, - submission=self.object.submission, - related=self.object, + submission=self.review.submission, + related=form.instance, ) return response def get_success_url(self): - return reverse('apply:submissions:reviews:review', args=(self.object.submission.pk, self.object.id,)) + return self.review.get_absolute_url() @method_decorator(login_required, name='dispatch')