Skip to content

Commit

Permalink
Merge pull request #724 from OpenTechFund/feature/reviewers-access-al…
Browse files Browse the repository at this point in the history
…l-reviews

Allow reviewers to set visibility on reviews.
  • Loading branch information
frjo authored Apr 12, 2019
2 parents 524f539 + 7816de6 commit fcfd52b
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<div>-</div>
<div>-</div>
{% else %}
{% if request.user.is_apply_staff or request.user == reviewer %}
{% if request.user == reviewer or request.user.is_reviewer and review.reviewer_visibility or request.user.is_apply_staff %}
<div>
<a href="{% url 'apply:submissions:reviews:review' submission_pk=review.submission.id pk=review.id %}">
<div class="reviews-sidebar__name">
Expand Down
22 changes: 21 additions & 1 deletion opentech/apply/review/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

from django import forms

from django.utils.safestring import mark_safe
from django.utils.translation import ugettext_lazy as _

from wagtail.core.blocks import RichTextBlock

from opentech.apply.review.fields import ScoredAnswerField
from opentech.apply.review.options import RECOMMENDATION_CHOICES, RATE_CHOICES_DICT, RATE_CHOICE_NA
from opentech.apply.review.options import RECOMMENDATION_CHOICES, RATE_CHOICES_DICT, RATE_CHOICE_NA, VISIBILITY, VISIBILILTY_HELP_TEXT, PRIVATE
from opentech.apply.stream_forms.blocks import OptionalFormFieldBlock, CharFieldBlock, TextFieldBlock, CheckboxFieldBlock, DropdownFieldBlock
from opentech.apply.utils.blocks import CustomFormFieldsBlock, MustIncludeFieldBlock
from opentech.apply.utils.options import RICH_TEXT_WIDGET_SHORT
Expand Down Expand Up @@ -75,6 +76,25 @@ def get_field_kwargs(self, struct_value):
return kwargs


class VisibilityBlock(ReviewMustIncludeFieldBlock):
name = 'visibility'
description = 'Visibility'
field_class = forms.ChoiceField
widget = forms.RadioSelect()

class Meta:
icon = 'radio-empty'

def get_field_kwargs(self, struct_value):
kwargs = super(VisibilityBlock, self).get_field_kwargs(struct_value)
kwargs['choices'] = VISIBILITY.items()
kwargs['initial'] = PRIVATE
kwargs['help_text'] = mark_safe('<br>'.join(
[VISIBILITY[choice] + ': ' + VISIBILILTY_HELP_TEXT[choice] for choice in VISIBILITY]
))
return kwargs


class ReviewCustomFormFieldsBlock(CustomFormFieldsBlock):
char = CharFieldBlock(group=_('Fields'))
text = TextFieldBlock(group=_('Fields'))
Expand Down
11 changes: 9 additions & 2 deletions opentech/apply/review/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from opentech.apply.stream_forms.forms import StreamBaseForm

from .models import Review, ReviewOpinion
from .options import OPINION_CHOICES
from .options import OPINION_CHOICES, PRIVATE


class MixedMetaClass(type(StreamBaseForm), type(forms.ModelForm)):
Expand All @@ -18,13 +18,14 @@ class ReviewModelForm(StreamBaseForm, forms.ModelForm, metaclass=MixedMetaClass)

class Meta:
model = Review
fields = ['recommendation', 'score', 'submission', 'author']
fields = ['recommendation', 'visibility', 'score', 'submission', 'author']

widgets = {
'recommendation': forms.HiddenInput(),
'score': forms.HiddenInput(),
'submission': forms.HiddenInput(),
'author': forms.HiddenInput(),
'visibility': forms.HiddenInput(),
}

error_messages = {
Expand Down Expand Up @@ -65,6 +66,12 @@ def save(self, commit=True):
self.instance.score = self.calculate_score(self.cleaned_data)
self.instance.recommendation = int(self.cleaned_data[self.instance.recommendation_field.id])
self.instance.is_draft = self.draft_button_name in self.data
# Old review forms do not have the requred visability field.
# This will set visibility to PRIVATE by default.
try:
self.instance.visibility = self.cleaned_data[self.instance.visibility_field.id]
except AttributeError:
self.instance.visibility = PRIVATE

self.instance.form_data = self.cleaned_data['form_data']

Expand Down
31 changes: 31 additions & 0 deletions opentech/apply/review/migrations/0016_review_visibility.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 2.0.9 on 2018-12-12 17:48

from django.db import migrations, models
import wagtail.core.blocks
import wagtail.core.blocks.static_block
import wagtail.core.fields


class Migration(migrations.Migration):

dependencies = [
('review', '0015_review_opinion'),
]

operations = [
migrations.AddField(
model_name='review',
name='visibility',
field=models.CharField(choices=[('private', 'Private'), ('reviewers', 'Reviewers and Staff')], default='private', max_length=10, verbose_name='Visibility'),
),
migrations.AlterField(
model_name='review',
name='form_fields',
field=wagtail.core.fields.StreamField([('rich_text', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.TextBlock(label='Default value', required=False))], group='Fields')), ('markdown_text', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.TextBlock(label='Default value', required=False))], group='Fields')), ('char', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('format', wagtail.core.blocks.ChoiceBlock(choices=[('email', 'Email'), ('url', 'URL')], label='Format', required=False)), ('default_value', wagtail.core.blocks.CharBlock(label='Default value', required=False))], group='Fields')), ('text', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.TextBlock(label='Default value', required=False))], group='Fields')), ('text_markup', wagtail.core.blocks.RichTextBlock(group='Fields', label='Paragraph')), ('score', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False))], group='Fields')), ('checkbox', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.BooleanBlock(required=False))], group='Fields')), ('dropdown', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('choices', wagtail.core.blocks.ListBlock(wagtail.core.blocks.CharBlock(label='Choice')))], group='Fields')), ('recommendation', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('info', wagtail.core.blocks.static_block.StaticBlock())], group=' Required')), ('comments', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('info', wagtail.core.blocks.static_block.StaticBlock())], group=' Required')), ('visibility', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('info', wagtail.core.blocks.static_block.StaticBlock())], group=' Required'))]),
),
migrations.AlterField(
model_name='reviewform',
name='form_fields',
field=wagtail.core.fields.StreamField([('rich_text', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.TextBlock(label='Default value', required=False))], group='Fields')), ('markdown_text', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.TextBlock(label='Default value', required=False))], group='Fields')), ('char', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('format', wagtail.core.blocks.ChoiceBlock(choices=[('email', 'Email'), ('url', 'URL')], label='Format', required=False)), ('default_value', wagtail.core.blocks.CharBlock(label='Default value', required=False))], group='Fields')), ('text', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.TextBlock(label='Default value', required=False))], group='Fields')), ('text_markup', wagtail.core.blocks.RichTextBlock(group='Fields', label='Paragraph')), ('score', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False))], group='Fields')), ('checkbox', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('default_value', wagtail.core.blocks.BooleanBlock(required=False))], group='Fields')), ('dropdown', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('required', wagtail.core.blocks.BooleanBlock(label='Required', required=False)), ('choices', wagtail.core.blocks.ListBlock(wagtail.core.blocks.CharBlock(label='Choice')))], group='Fields')), ('recommendation', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('info', wagtail.core.blocks.static_block.StaticBlock())], group=' Required')), ('comments', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('info', wagtail.core.blocks.static_block.StaticBlock())], group=' Required')), ('visibility', wagtail.core.blocks.StructBlock([('field_label', wagtail.core.blocks.CharBlock(label='Label')), ('help_text', wagtail.core.blocks.TextBlock(label='Help text', required=False)), ('info', wagtail.core.blocks.static_block.StaticBlock())], group=' Required'))]),
),
]
14 changes: 12 additions & 2 deletions opentech/apply/review/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
from django.db.models.signals import post_save
from django.dispatch import receiver
from django.urls import reverse
from django.utils.functional import cached_property
from django.utils.translation import ugettext_lazy as _
from wagtail.admin.edit_handlers import FieldPanel, StreamFieldPanel
from wagtail.core.fields import StreamField

from opentech.apply.funds.models.mixins import AccessFormData
from opentech.apply.review.options import YES, NO, MAYBE, RECOMMENDATION_CHOICES, OPINION_CHOICES
from opentech.apply.stream_forms.models import BaseStreamForm
from opentech.apply.users.models import User

Expand All @@ -19,8 +19,9 @@
RecommendationBlock,
RecommendationCommentsBlock,
ScoreFieldBlock,
VisibilityBlock,
)
from .options import NA
from .options import NA, YES, NO, MAYBE, RECOMMENDATION_CHOICES, OPINION_CHOICES, VISIBILITY, PRIVATE, REVIEWER


class ReviewFormFieldsMixin(models.Model):
Expand All @@ -37,6 +38,10 @@ def score_fields(self):
def recommendation_field(self):
return self._get_field_type(RecommendationBlock)

@property
def visibility_field(self):
return self._get_field_type(VisibilityBlock)

@property
def comment_field(self):
return self._get_field_type(RecommendationCommentsBlock)
Expand Down Expand Up @@ -128,6 +133,7 @@ class Review(ReviewFormFieldsMixin, BaseStreamForm, AccessFormData, models.Model
is_draft = models.BooleanField(default=False, verbose_name=_("Draft"))
created_at = models.DateTimeField(verbose_name=_("Creation time"), auto_now_add=True)
updated_at = models.DateTimeField(verbose_name=_("Update time"), auto_now=True)
visibility = models.CharField(verbose_name=_("Visibility"), choices=VISIBILITY.items(), default=PRIVATE, max_length=10)

# Meta: used for migration purposes only
drupal_id = models.IntegerField(null=True, blank=True, editable=False)
Expand Down Expand Up @@ -163,6 +169,10 @@ def for_latest(self):
def get_compare_url(self):
return self.revision.get_compare_url_to_latest()

@cached_property
def reviewer_visibility(self):
return self.visibility == REVIEWER


@receiver(post_save, sender=Review)
def update_submission_reviewers_list(sender, **kwargs):
Expand Down
13 changes: 13 additions & 0 deletions opentech/apply/review/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,16 @@
(AGREE, 'Agree'),
(DISAGREE, 'Disagree'),
)

PRIVATE = 'private'
REVIEWER = 'reviewers'

VISIBILILTY_HELP_TEXT = {
PRIVATE: 'Visible only to staff.',
REVIEWER: 'Visible to other reviewers and staff.',
}

VISIBILITY = {
PRIVATE: 'Private',
REVIEWER: 'Reviewers and Staff',
}
4 changes: 4 additions & 0 deletions opentech/apply/review/templates/review/review_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ <h5>Recommendation</h5>
<h5>Score</h5>
<p>{{ review.score }}</p>
</div>
<div>
<svg class="icon icon--eye"><use xlink:href="#eye"></use></svg>
{{ review.get_visibility_display }}
</div>
{% if not review.for_latest %}
<div>
<h5>Review was not against the latest version:</h5>
Expand Down
12 changes: 11 additions & 1 deletion opentech/apply/review/tests/factories/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import factory

from opentech.apply.review import blocks
from opentech.apply.review.options import YES, MAYBE, NO
from opentech.apply.review.options import YES, MAYBE, NO, PRIVATE, REVIEWER
from opentech.apply.stream_forms.testing.factories import FormFieldBlockFactory, CharFieldBlockFactory, \
StreamFieldUUIDFactory
from opentech.apply.utils.testing.factories import RichTextFieldBlockFactory
Expand All @@ -25,6 +25,15 @@ class Meta:
model = blocks.RecommendationCommentsBlock


class VisibilityBlockFactory(FormFieldBlockFactory):
class Meta:
model = blocks.VisibilityBlock

@classmethod
def make_answer(cls, params=dict()):
return random.choices([PRIVATE, REVIEWER])


class ScoreFieldBlockFactory(FormFieldBlockFactory):
class Meta:
model = blocks.ScoreFieldBlock
Expand All @@ -49,4 +58,5 @@ def make_form_answer(cls, params=dict()):
'score': ScoreFieldBlockFactory,
'recommendation': RecommendationBlockFactory,
'comments': RecommendationCommentsBlockFactory,
'visibility': VisibilityBlockFactory,
})
4 changes: 3 additions & 1 deletion opentech/apply/review/tests/factories/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from opentech.apply.stream_forms.testing.factories import FormDataFactory
from opentech.apply.users.tests.factories import StaffFactory

from ...options import YES, NO, MAYBE, AGREE, DISAGREE
from ...options import YES, NO, MAYBE, AGREE, DISAGREE, PRIVATE, REVIEWER
from ...models import Review, ReviewForm, ReviewOpinion

from . import blocks
Expand All @@ -24,6 +24,8 @@ class Params:
recommendation_yes = factory.Trait(recommendation=YES)
recommendation_maybe = factory.Trait(recommendation=MAYBE)
draft = factory.Trait(is_draft=True)
visibility_private = factory.Trait(visibility=PRIVATE)
visibility_reviewer = factory.Trait(visibility=REVIEWER)

submission = factory.SubFactory(ApplicationSubmissionFactory)
revision = factory.SelfAttribute('submission.live_revision')
Expand Down
25 changes: 24 additions & 1 deletion opentech/apply/review/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from opentech.apply.activity.models import Activity
from opentech.apply.funds.tests.factories.models import ApplicationSubmissionFactory
from opentech.apply.users.tests.factories import StaffFactory, UserFactory
from opentech.apply.users.tests.factories import ReviewerFactory, StaffFactory, UserFactory
from opentech.apply.utils.testing.tests import BaseViewTestCase

from .factories import ReviewFactory, ReviewFormFieldsFactory, ReviewFormFactory, ReviewOpinionFactory
Expand Down Expand Up @@ -296,3 +296,26 @@ def test_nonstaff_cant_post_opinion_to_review(self):
review = ReviewFactory(submission=self.submission, author=staff, recommendation_yes=True)
response = self.post_page(review, {'agree': AGREE})
self.assertEqual(response.status_code, 403)


class ReviewDetailVisibilityTestCase(BaseViewTestCase):
user_factory = ReviewerFactory
url_name = 'funds:submissions:reviews:{}'
base_view_name = 'review'

def get_kwargs(self, instance):
return {'pk': instance.id, 'submission_pk': instance.submission.id}

def test_review_detail_visibility_private(self):
submission = ApplicationSubmissionFactory(status='external_review', workflow_stages=2)
review = ReviewFactory(submission=submission, author=self.user, visibility_private=True)
self.client.force_login(self.user_factory())
response = self.get_page(review)
self.assertEqual(response.status_code, 403)

def test_review_detail_visibility_reviewer(self):
submission = ApplicationSubmissionFactory(status='external_review', workflow_stages=2)
review = ReviewFactory(submission=submission, author=self.user, visibility_reviewer=True)
self.client.force_login(self.user_factory())
response = self.get_page(review)
self.assertEqual(response.status_code, 200)
3 changes: 2 additions & 1 deletion opentech/apply/review/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,10 @@ def get_context_data(self, **kwargs):

def dispatch(self, request, *args, **kwargs):
review = self.get_object()
user = request.user
author = review.author

if request.user != author and not request.user.is_superuser and not request.user.is_apply_staff:
if user != author and not (user.is_reviewer and review.reviewer_visibility) and not user.is_apply_staff:
raise PermissionDenied

if review.is_draft:
Expand Down

0 comments on commit fcfd52b

Please sign in to comment.