diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html index d39cf28ada9ae8697491c16f859ae05c0674b285..c58e19266b5a20e2a00a23bd570848f68582d935 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html @@ -22,7 +22,7 @@ <div class="sidebar__inner"> <h5>Reviews & assignees</h5> - {% include 'funds/includes/review_table.html' %} + {% include 'funds/includes/review_sidebar.html' %} <div class="wrapper wrapper--sidebar-buttons"> {% include 'review/includes/review_button.html' with submission=object class="button--half-width" %} diff --git a/opentech/apply/funds/templates/funds/includes/review_sidebar.html b/opentech/apply/funds/templates/funds/includes/review_sidebar.html new file mode 100644 index 0000000000000000000000000000000000000000..210777f13044bdfb6af5c30524e362b17f064fa8 --- /dev/null +++ b/opentech/apply/funds/templates/funds/includes/review_sidebar.html @@ -0,0 +1,46 @@ +{% load review_tags %} +<ul class="reviews-sidebar js-reviews-sidebar"> + {% if reviews_exist %} + <li class="reviews-sidebar__item reviews-sidebar__item--header"> + <div></div> + <div>{{ recommendation|traffic_light }}</div> + <div></div> + </li> + {% endif %} + + {% if not assigned_staff %} + <li class="reviews-sidebar__no-reviews">No reviews yet</li> + {% endif %} + + {% for review_data in reviews_block.role_reviewed %} + {% include 'funds/includes/review_sidebar_item.html' with review=review_data.review reviewer=review_data.reviewer role=review_data.role %} + {% endfor %} + + {% for review_data in reviews_block.staff_reviewed %} + {% include 'funds/includes/review_sidebar_item.html' with review=review_data.review reviewer=review_data.reviewer %} + {% endfor %} + + {% for review_data in reviews_block.role_not_reviewed %} + {% include 'funds/includes/review_sidebar_item.html' with reviewer=review_data.reviewer role=review_data.role missing=True %} + {% endfor %} + + {% for review_data in reviews_block.staff_not_reviewed %} + {% include 'funds/includes/review_sidebar_item.html' with reviewer=review_data.reviewer missing=True %} + {% endfor %} + + {% if object.stage.has_external_review %} + {% if reviews_block.external_reviewed or reviews_block.external_not_reviewed %} + <hr class="reviews-sidebar__split"> + + {% for review_data in reviews_block.external_reviewed %} + {% include 'funds/includes/review_sidebar_item.html' with review=review_data.review reviewer=review_data.reviewer %} + {% endfor %} + + {% for review_data in reviews_block.external_not_reviewed %} + {% include 'funds/includes/review_sidebar_item.html' with reviewer=review_data.reviewer missing=True class="is-hidden" %} + {% endfor %} + + <li><a class="link link--bold link--underlined js-toggle-reviewers" href="#">All Assigned Advisors</a></li> + {% endif %} + {% endif %} +</ul> \ No newline at end of file diff --git a/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html b/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html new file mode 100644 index 0000000000000000000000000000000000000000..6a11896af038d955c35fa1dd3cdf94bea5c46c21 --- /dev/null +++ b/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html @@ -0,0 +1,42 @@ +{% load wagtailimages_tags %} + +<li class="reviews-sidebar__item {{ class }} {% if missing %}no-response{% endif %}"> + {% if missing %} + <div class="reviews-sidebar__name"> + <span>{{ reviewer }}</span> + {% if role %}{% image role.icon max-12x12 %}{% endif %} + </div> + <div>-</div> + <div>-</div> + {% else %} + {% if request.user.is_apply_staff or request.user == reviewer %} + <div> + <a href="{% url 'apply:submissions:reviews:review' submission_pk=review.submission.id pk=review.id %}"> + <div class="reviews-sidebar__name"> + <span>{{ reviewer }}</span> + {% if role %}{% image role.icon max-12x12 %}{% endif %} + </div> + </a> + <div class="reviews-sidebar__date">{{ review.updated_at|date:"Y-m-d" }}</div> + </div> + {% else %} + <div class="reviews-sidebar__name"> + <span>{{ reviewer }}</span> + {% if role %}{% image role.icon max-12x12 %}{% endif %} + </div> + {% endif %} + <div>{{ review.get_recommendation_display }}</div> + <div>{{ review.get_score_display }}</div> + {% endif %} +</li> + +{% for opinion in review.opinions.all %} +<li class="reviews-sidebar__item {{ class }}"> + <div class="reviews-sidebar__name"> + <span>{{ opinion.author }}</span> + {% if opinion.get_author_assignment %}{% image opinion.get_author_assignment.icon max-12x12 %}{% endif %} + </div> + <div></div> + <div>{{ opinion.get_opinion_display }}</div> +</li> +{% endfor %} \ No newline at end of file diff --git a/opentech/apply/funds/templates/funds/includes/review_table.html b/opentech/apply/funds/templates/funds/includes/review_table.html deleted file mode 100644 index ef85008ac12605eb1e73e7b6b318644b97d77355..0000000000000000000000000000000000000000 --- a/opentech/apply/funds/templates/funds/includes/review_table.html +++ /dev/null @@ -1,47 +0,0 @@ -{% load review_tags %} -<table class="reviews-sidebar js-reviews-sidebar"> - {% if reviews_exist %} - <tr class="tr tr--subchild light-grey-bg"> - <th colspan="2"></th> - <th>{{ recommendation|traffic_light }}</th> - <th></th> - </tr> - {% endif %} - - {% if not assigned_staff %} - <tr><td colspan="4" class="reviews-sidebar__no-reviews">No reviews yet</td></tr> - {% endif %} - - {% for review_data in reviews_block.role_reviewed %} - {% include 'funds/includes/review_table_row.html' with review=review_data.review reviewer=review_data.reviewer role=review_data.role %} - {% endfor %} - - {% for review_data in reviews_block.staff_reviewed %} - {% include 'funds/includes/review_table_row.html' with review=review_data.review reviewer=review_data.reviewer %} - {% endfor %} - - {% for review_data in reviews_block.role_not_reviewed %} - {% include 'funds/includes/review_table_row.html' with reviewer=review_data.reviewer role=review_data.role missing=True %} - {% endfor %} - - {% for review_data in reviews_block.staff_not_reviewed %} - {% include 'funds/includes/review_table_row.html' with reviewer=review_data.reviewer missing=True %} - {% endfor %} - - {% if object.stage.has_external_review %} - <tr class="tr tr--subchild"><td colspan="4"><hr></td></tr> - {% if reviews_block.external_reviewed or reviews_block.external_not_reviewed %} - - {% for review_data in reviews_block.external_reviewed %} - {% include 'funds/includes/review_table_row.html' with review=review_data.review reviewer=review_data.reviewer %} - {% endfor %} - - {% for review_data in reviews_block.external_not_reviewed %} - {% include 'funds/includes/review_table_row.html' with reviewer=review_data.reviewer missing=True class="hidden" %} - {% endfor %} - - <tr><td colspan="4"><a class="link link--bold link--underlined js-toggle-reviewers" href="#">All Assigned Advisors</a></td></tr> - {% endif %} - {% endif %} - -</table> diff --git a/opentech/apply/funds/templates/funds/includes/review_table_row.html b/opentech/apply/funds/templates/funds/includes/review_table_row.html deleted file mode 100644 index 8e5190c29c6a6f449602af0b86c1b421b44ba415..0000000000000000000000000000000000000000 --- a/opentech/apply/funds/templates/funds/includes/review_table_row.html +++ /dev/null @@ -1,33 +0,0 @@ -{% load wagtailimages_tags %} - -<tr class="tr--subchild {{ class }} {% if missing %}no-response{% endif %}"> - {% if missing %} - <td class="reviews-sidebar__author" colspan="2"> - <span> - {{ reviewer }} - {% if role %}{% image role.icon max-12x12 %}{% endif %} - </span> - </td> - <td>-</td> - <td>-</td> - {% else %} - <td class="reviews-sidebar__author" colspan="2"> - {% if request.user.is_apply_staff or request.user == reviewer %} - <a href="{% url 'apply:submissions:reviews:review' submission_pk=review.submission.id pk=review.id %}"> - <span> - {{ reviewer }} - {% if role %}{% image role.icon max-12x12 %}{% endif %} - </span> - </a> - <div class="reviews-sidebar__date">{{ review.updated_at|date:"Y-m-d" }}</div> - {% else %} - <span> - {{ reviewer }} - {% if role %}{% image role.icon max-12x12 %}{% endif %} - </span> - {% endif %} - </td> - <td>{{ review.get_recommendation_display }}</td> - <td>{{ review.get_score_display }}</td> - {% endif %} -</tr> diff --git a/opentech/apply/funds/templates/funds/tables/table.html b/opentech/apply/funds/templates/funds/tables/table.html index 66599d9371b55d206e44642fe6df8ed0f9000a23..3111c4aef4aaa2434bb32dba3e24c9394b3f0314 100644 --- a/opentech/apply/funds/templates/funds/tables/table.html +++ b/opentech/apply/funds/templates/funds/tables/table.html @@ -39,6 +39,12 @@ <span class="list__item--reviewer-name">{{ review.author }}</span> <span class="list__item list__item--reviewer-outcome">{{ review.get_recommendation_display }}</span> </li> + {% for opinion in review.opinions.all %} + <li class="list__item list__item--reviewer"> + <span class="list__item--reviewer-name">{{ opinion.author }}</span> + <span class="list__item list__item--reviewer-outcome">{{ opinion.get_opinion_display }}</span> + </li> + {% endfor %} {% endfor %} </ul> </td> diff --git a/opentech/apply/funds/tests/test_views.py b/opentech/apply/funds/tests/test_views.py index 2eee0123a4632af75ee2309c5faebb6c45e5f2d4..024aea5327c214f103701e6b7361a52eef62ddc2 100644 --- a/opentech/apply/funds/tests/test_views.py +++ b/opentech/apply/funds/tests/test_views.py @@ -2,6 +2,7 @@ from datetime import datetime, timedelta import json from django.test import TestCase +from django.utils.text import slugify from opentech.apply.activity.models import Activity, INTERNAL from opentech.apply.determinations.tests.factories import DeterminationFactory @@ -215,7 +216,7 @@ class TestReviewersUpdateView(BaseSubmissionViewTestCase): } data.update( **{ - f'role_reviewer_{str(role)}': reviewer.id + f'role_reviewer_{slugify(str(role))}': reviewer.id for role, reviewer in zip(self.roles, reviewer_roles) } ) diff --git a/opentech/apply/review/models.py b/opentech/apply/review/models.py index 294b622e86c225e0ef4d4bb4e72d8e6d1f234ee4..2b9ce85ddd8ac92927420705ff2ad715bba63f4d 100644 --- a/opentech/apply/review/models.py +++ b/opentech/apply/review/models.py @@ -107,6 +107,9 @@ class ReviewQuerySet(models.QuerySet): else: return MAYBE + def opinions(self): + return ReviewOpinion.objects.filter(review__id__in=self.values_list('id')) + class Review(ReviewFormFieldsMixin, BaseStreamForm, AccessFormData, models.Model): submission = models.ForeignKey('funds.ApplicationSubmission', on_delete=models.CASCADE, related_name='reviews') @@ -184,3 +187,7 @@ class ReviewOpinion(models.Model): @property def opinion_display(self): return self.get_opinion_display() + + def get_author_role(self): + assignment = self.review.submission.assigned.with_roles().filter(reviewer=self.author).first() + return assignment.role if assignment else None diff --git a/opentech/apply/review/templates/review/includes/review_opinions_list.html b/opentech/apply/review/templates/review/includes/review_opinions_list.html new file mode 100644 index 0000000000000000000000000000000000000000..0cc942d17bf8bd1c4536731dbeeed1e69a967e37 --- /dev/null +++ b/opentech/apply/review/templates/review/includes/review_opinions_list.html @@ -0,0 +1,13 @@ +{% load wagtailimages_tags %} + +{% if opinions %} + <ul> + {% for opinion in opinions %} + <li> + {{ opinion.author }} + {% if opinion.get_author_assignment %}{% image opinion.get_author_assignment.icon max-12x12 %}{% endif %} + {{ opinion.get_opinion_display }}s + </li> + {% endfor %} + </ul> +{% endif %} \ No newline at end of file diff --git a/opentech/apply/review/templates/review/review_detail.html b/opentech/apply/review/templates/review/review_detail.html index 0e0c909c50804e9b8fe05d60a4a7e5092d08b68e..ef40757784276d254162cf1db75854d36e3bd698 100644 --- a/opentech/apply/review/templates/review/review_detail.html +++ b/opentech/apply/review/templates/review/review_detail.html @@ -6,6 +6,8 @@ <div class="admin-bar__inner"> <h1 class="beta heading heading--no-margin heading--bold">Review</h1> <h5>For <a href="{% url "funds:submissions:detail" review.submission.id %}">{{ review.submission.title }}</a> by {{ review.author }} at {{ review.updated_at|date:"Y-m-d" }}</h5> + + {% include 'review/includes/review_opinions_list.html' with opinions=review.opinions.all %} </div> </div> diff --git a/opentech/apply/review/templates/review/review_list.html b/opentech/apply/review/templates/review/review_list.html index a8bdbdf556f9a7fa81347f57d6886dcb43f25e03..7487a763ff508614413959e5e3ff19857deec10c 100644 --- a/opentech/apply/review/templates/review/review_list.html +++ b/opentech/apply/review/templates/review/review_list.html @@ -23,7 +23,11 @@ <tr class="reviews-list__tr"> <th class="reviews-list__th">{{ answers.question }}</th> {% for answer in answers.answers %} - <td class="reviews-list__td">{{ answer|bleach }}</td> + {% if answers.question == "Opinions" %} + <td class="reviews-list__td">{{ answer }}</td> + {% else %} + <td class="reviews-list__td">{{ answer|bleach }}</td> + {% endif %} {% endfor %} </tr> {% endfor %} diff --git a/opentech/apply/review/tests/factories/models.py b/opentech/apply/review/tests/factories/models.py index 2d59912a3ead56fc6f9d6389e9b221c96b212173..a6d825394a52951c3a37d9e2d409e8da67916c58 100644 --- a/opentech/apply/review/tests/factories/models.py +++ b/opentech/apply/review/tests/factories/models.py @@ -4,12 +4,12 @@ from opentech.apply.funds.tests.factories import ApplicationSubmissionFactory from opentech.apply.stream_forms.testing.factories import FormDataFactory from opentech.apply.users.tests.factories import StaffFactory -from ...options import YES, NO, MAYBE -from ...models import Review, ReviewForm +from ...options import YES, NO, MAYBE, AGREE, DISAGREE +from ...models import Review, ReviewForm, ReviewOpinion from . import blocks -__all__ = ['ReviewFactory', 'ReviewFormFactory'] +__all__ = ['ReviewFactory', 'ReviewFormFactory', 'ReviewOpinionFactory'] class ReviewFormDataFactory(FormDataFactory): @@ -38,6 +38,19 @@ class ReviewFactory(factory.DjangoModelFactory): score = 0 +class ReviewOpinionFactory(factory.DjangoModelFactory): + class Meta: + model = ReviewOpinion + + class Params: + opinion_agree = factory.Trait(opinion=AGREE) + opinion_disagree = factory.Trait(opinion=DISAGREE) + + review = factory.SubFactory(ReviewFactory) + author = factory.SubFactory(StaffFactory) + opinion = DISAGREE + + class ReviewFormFactory(factory.DjangoModelFactory): class Meta: model = ReviewForm diff --git a/opentech/apply/review/tests/test_views.py b/opentech/apply/review/tests/test_views.py index d58724ff3f442a40caa974033a3cbd9c7bc92570..8d0c1fe20f3131fc0158bfd151c40401e659555d 100644 --- a/opentech/apply/review/tests/test_views.py +++ b/opentech/apply/review/tests/test_views.py @@ -5,7 +5,7 @@ from opentech.apply.funds.tests.factories.models import ApplicationSubmissionFac from opentech.apply.users.tests.factories import StaffFactory, UserFactory from opentech.apply.utils.testing.tests import BaseViewTestCase -from .factories import ReviewFactory, ReviewFormFieldsFactory, ReviewFormFactory +from .factories import ReviewFactory, ReviewFormFieldsFactory, ReviewFormFactory, ReviewOpinionFactory from ..models import Review, ReviewOpinion from ..options import NA, AGREE @@ -208,6 +208,33 @@ class ReviewDetailTestCase(BaseViewTestCase): self.assertContains(response, submission.title) self.assertContains(response, "<p>Yes</p>") + def test_review_detail_opinion(self): + staff = StaffFactory() + submission = ApplicationSubmissionFactory(status='draft_proposal', workflow_stages=2) + review = ReviewFactory(submission=submission, author=self.user, recommendation_yes=True) + ReviewOpinionFactory(review=review, author=staff, opinion_disagree=True) + response = self.get_page(review) + self.assertContains(response, "Disagrees") + + +class ReviewListTestCase(BaseViewTestCase): + user_factory = StaffFactory + url_name = 'funds:submissions:reviews:{}' + base_view_name = 'list' + + def get_kwargs(self, instance): + return {'submission_pk': instance.submission.id} + + def test_review_list_opinion(self): + staff = StaffFactory() + submission = ApplicationSubmissionFactory(status='draft_proposal', workflow_stages=2) + review = ReviewFactory(submission=submission, author=self.user, recommendation_yes=True) + ReviewOpinionFactory(review=review, author=staff, opinion_disagree=True) + response = self.get_page(review) + response_opinion = response.context['review_data']['opinions']['answers'][0] + self.assertIn("Disagrees", response_opinion) + self.assertIn(str(staff), response_opinion) + class StaffReviewOpinionCase(BaseViewTestCase): user_factory = StaffFactory @@ -240,3 +267,23 @@ class StaffReviewOpinionCase(BaseViewTestCase): 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) + + +class NonStaffReviewOpinionCase(BaseViewTestCase): + user_factory = UserFactory + url_name = 'funds:submissions:reviews:{}' + base_view_name = 'review' + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.submission = ApplicationSubmissionFactory(status='draft_proposal', workflow_stages=2) + + def get_kwargs(self, instance): + return {'pk': instance.id, 'submission_pk': instance.submission.id} + + def nonstaff_cant_post_opinion_to_review(self): + staff = StaffFactory() + review = ReviewFactory(submission=self.submission, author=staff, recommendation_yes=True) + response = self.post_page(review, {'agree': AGREE}) + self.assertEqual(response.status_code, 403) diff --git a/opentech/apply/review/views.py b/opentech/apply/review/views.py index c8605b2eb93cd743e650bd6efec6689df71117cb..09752f6aee1a8d8d3f95ee6efcae9e74e5c4a857 100644 --- a/opentech/apply/review/views.py +++ b/opentech/apply/review/views.py @@ -4,6 +4,7 @@ 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.template.loader import get_template from django.urls import reverse_lazy from django.utils.decorators import method_decorator from django.views.generic import CreateView, ListView, DetailView @@ -16,6 +17,7 @@ from opentech.apply.review.blocks import RecommendationBlock, RecommendationComm from opentech.apply.review.forms import ReviewModelForm, ReviewOpinionForm from opentech.apply.stream_forms.models import BaseStreamForm from opentech.apply.users.decorators import staff_required +from opentech.apply.users.models import User from opentech.apply.utils.views import CreateOrUpdateView from .models import Review @@ -30,32 +32,37 @@ class ReviewContextMixin: for review in reviews: reviews_dict[review.author.pk] = review + # Get all the authors of opinions, these authors should not show up in the 'xxx_not_reviewed' lists + opinion_authors = User.objects.filter(pk__in=self.object.reviews.opinions().values('author')).distinct() + reviews_block = defaultdict(list) for assigned_reviewer in assigned: reviewer = assigned_reviewer.reviewer role = assigned_reviewer.role review = reviews_dict.get(reviewer.pk, None) + key = None if role: if review: key = 'role_reviewed' - else: + elif reviewer not in opinion_authors: key = 'role_not_reviewed' elif reviewer.is_apply_staff: if review: key = 'staff_reviewed' - else: + elif review not in opinion_authors: key = 'staff_not_reviewed' else: if review: key = 'external_reviewed' else: key = 'external_not_reviewed' - - reviews_block[key].append({ - 'reviewer': reviewer, - 'review': review, - 'role': role, - }) + if key: # Do not add this reviewer to any list if they haven't reviewed but have left an opinion + review_info_dict = { + 'reviewer': reviewer, + 'review': review, + 'role': role, + } + reviews_block[key].append(review_info_dict) # Calculate the recommendation based on role and staff reviews recommendation = self.object.reviews.by_staff().recommendation() @@ -226,6 +233,7 @@ class ReviewListView(ListView): # Add the header rows review_data['title'] = {'question': '', 'answers': list()} + review_data['opinions'] = {'question': 'Opinions', 'answers': list()} review_data['score'] = {'question': 'Overall Score', 'answers': list()} review_data['recommendation'] = {'question': 'Recommendation', 'answers': list()} review_data['revision'] = {'question': 'Revision', 'answers': list()} @@ -235,6 +243,9 @@ class ReviewListView(ListView): for i, review in enumerate(self.object_list): review_data['title']['answers'].append('<a href="{}">{}</a>'.format(review.get_absolute_url(), review.author)) + opinions_template = get_template('review/includes/review_opinions_list.html') + opinions_html = opinions_template.render({'opinions': review.opinions.select_related('author').all()}) + review_data['opinions']['answers'].append(opinions_html) review_data['score']['answers'].append(str(review.get_score_display())) review_data['recommendation']['answers'].append(review.get_recommendation_display()) review_data['comments']['answers'].append(review.get_comments_display(include_question=False))