diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py
index 87d634b371a58f4c774f8f18ec57d755db2b6970..661bb355b493215643e8dd2fd83a20f70c565a16 100644
--- a/opentech/apply/funds/models/submissions.py
+++ b/opentech/apply/funds/models/submissions.py
@@ -7,7 +7,18 @@ from django.contrib.auth.models import Group
from django.contrib.postgres.fields import JSONField
from django.core.exceptions import PermissionDenied
from django.db import models
-from django.db.models import Case, Count, IntegerField, OuterRef, Subquery, Sum, Q, Prefetch, When
+from django.db.models import (
+ Case,
+ Count,
+ IntegerField,
+ F,
+ OuterRef,
+ Prefetch,
+ Q,
+ Subquery,
+ Sum,
+ When,
+)
from django.db.models.expressions import RawSQL, OrderBy
from django.db.models.functions import Coalesce
from django.dispatch import receiver
@@ -22,7 +33,7 @@ from wagtail.contrib.forms.models import AbstractFormSubmission
from opentech.apply.activity.messaging import messenger, MESSAGES
from opentech.apply.determinations.models import Determination
-from opentech.apply.review.models import Review, ReviewOpinion
+from opentech.apply.review.models import ReviewOpinion
from opentech.apply.review.options import MAYBE, AGREE, DISAGREE
from opentech.apply.stream_forms.blocks import UploadableMediaBlock
from opentech.apply.stream_forms.files import StreamFieldDataEncoder
@@ -30,9 +41,11 @@ from opentech.apply.stream_forms.models import BaseStreamForm
from .mixins import AccessFormData
from .utils import (
+ COMMUNITY_REVIEWER_GROUP_NAME,
LIMIT_TO_STAFF,
LIMIT_TO_REVIEWER_GROUPS,
LIMIT_TO_PARTNERS,
+ PARTNER_GROUP_NAME,
REVIEW_GROUPS,
REVIEWER_GROUP_NAME,
STAFF_GROUP_NAME,
@@ -167,7 +180,9 @@ class ApplicationSubmissionQueryset(JSONOrderable):
output_field=IntegerField(),
),
review_submitted_count=Subquery(
- reviewers.reviewed().values('submission').annotate(count=Count('pk')).values('count'),
+ reviewers.reviewed().values('submission').annotate(
+ count=Count('pk', distinct=True)
+ ).values('count'),
output_field=IntegerField(),
),
review_recommendation=Case(
@@ -182,10 +197,18 @@ class ApplicationSubmissionQueryset(JSONOrderable):
role_icon=Subquery(roles_for_review[:1].values('role__icon')),
).prefetch_related(
Prefetch(
- 'reviews', queryset=Review.objects.select_related('author').prefetch_related(
- Prefetch('opinions', queryset=ReviewOpinion.objects.select_related('author'))
- )
+ 'assigned',
+ queryset=AssignedReviewers.objects.reviewed().review_order().prefetch_related(
+ Prefetch('opinions', queryset=ReviewOpinion.objects.select_related('author__reviewer'))
+ ),
+ to_attr='has_reviewed'
+ ),
+ Prefetch(
+ 'assigned',
+ queryset=AssignedReviewers.objects.not_reviewed().staff(),
+ to_attr='hasnt_reviewed'
)
+
).select_related(
'page',
'round',
@@ -758,6 +781,43 @@ class ApplicationRevision(AccessFormData, models.Model):
class AssignedReviewersQuerySet(models.QuerySet):
+ def review_order(self):
+ review_order = [
+ STAFF_GROUP_NAME,
+ PARTNER_GROUP_NAME,
+ COMMUNITY_REVIEWER_GROUP_NAME,
+ REVIEWER_GROUP_NAME,
+ ]
+
+ ordering = [
+ models.When(type__name=review_type, then=models.Value(i))
+ for i, review_type in enumerate(review_order)
+ ]
+ return self.exclude(
+ # Remove people from the list who are opinionated but
+ # didn't review, they appear elsewhere
+ opinions__isnull=False,
+ review__isnull=True,
+ ).annotate(
+ type_order=models.Case(
+ *ordering,
+ output_field=models.IntegerField(),
+ ),
+ has_review=models.Case(
+ models.When(review__isnull=True, then=models.Value(1)),
+ models.When(review__is_draft=True, then=models.Value(1)),
+ default=models.Value(0),
+ output_field=models.IntegerField(),
+ )
+ ).order_by(
+ 'type_order',
+ 'has_review',
+ F('role__order').asc(nulls_last=True),
+ ).select_related(
+ 'reviewer',
+ 'role',
+ )
+
def with_roles(self):
return self.filter(role__isnull=False)
@@ -766,13 +826,14 @@ class AssignedReviewersQuerySet(models.QuerySet):
def reviewed(self):
return self.filter(
- Q(opinions__isnull=False) | Q(Q(review__isnull=False) & Q(review__is_draft=False))
+ Q(opinions__opinion=AGREE) |
+ Q(Q(review__isnull=False) & Q(review__is_draft=False))
).distinct()
def not_reviewed(self):
return self.filter(
Q(review__isnull=True) | Q(review__is_draft=True),
- opinions__isnull=True,
+ Q(opinions__isnull=True) | Q(opinions__opinion=DISAGREE),
).distinct()
def never_tried_to_review(self):
diff --git a/opentech/apply/funds/templates/funds/includes/review_sidebar.html b/opentech/apply/funds/templates/funds/includes/review_sidebar.html
index c812b7d9b54a62b7f319daab4f1bdbd20fcf1918..e3c5f1752d8e874d4416f7964bc23821fa8ad7fb 100644
--- a/opentech/apply/funds/templates/funds/includes/review_sidebar.html
+++ b/opentech/apply/funds/templates/funds/includes/review_sidebar.html
@@ -31,15 +31,4 @@
{% endif %}
{% endfor %}
{% endfor %}
-
- {% if reviews_block.external_reviewed or reviews_block.external_not_reviewed %}
- {% for review_data in reviews_block.external_reviewed %}
- {% include 'funds/includes/review_sidebar_item.html' with review=review_data.review reviewer=review_data.reviewer opinions=review_data.opinions %}
- {% 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 %}
-
- {% endif %}
</ul>
diff --git a/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html b/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html
index 3a6b8db9ed20a185ca19f18e19c9d549e295b5c5..955624c17038d68fbca3a8ae66cd45b6eb8c6565 100644
--- a/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html
+++ b/opentech/apply/funds/templates/funds/includes/review_sidebar_item.html
@@ -1,7 +1,7 @@
{% load wagtailimages_tags %}
<li class="reviews-sidebar__item {% if hidden and not reviewer.review %}is-hidden {% endif %}{% if not reviewer.review %}no-response {% endif %}">
- {% if not reviewer.review %}
+ {% if not reviewer.review or reviewer.review.is_draft %}
<div class="reviews-sidebar__name">
<span>{{ reviewer}}</span>
{% if reviewer.role %}{% image reviewer.role.icon max-12x12 %}{% endif %}
diff --git a/opentech/apply/funds/templates/funds/tables/table.html b/opentech/apply/funds/templates/funds/tables/table.html
index c992d2b7a18d28937da2dbdece15eb91f280d2b8..db8cf106f06157ff258857e4a0c7ff67861cdb2b 100644
--- a/opentech/apply/funds/templates/funds/tables/table.html
+++ b/opentech/apply/funds/templates/funds/tables/table.html
@@ -1,5 +1,5 @@
{% extends 'django_tables2/table.html' %}
-{% load django_tables2 table_tags review_tags %}
+{% load django_tables2 table_tags review_tags wagtailimages_tags %}
{% block table.tbody.row %}
<tr {{ row.attrs.as_html }}>
@@ -36,20 +36,27 @@
<td>
<strong>{{ submission.screening_status|default:"Awaiting Screen status" }}</strong>
</td>
+
<td>
<ul class="list list--no-margin">
- {% for review in submission.reviews.submitted %}
+ {% for reviewer in submission.has_reviewed %}
<li class="list__item list__item--reviewer">
- <span class="list__item--reviewer-name">{{ review.author }}</span>
- <span class="list__item list__item--reviewer-outcome">{{ review.get_recommendation_display }}</span>
+ <span class="list__item--reviewer-name">
+ {{ reviewer }}
+ {% if reviewer.role %}{% image reviewer.role.icon max-12x12 %}{% endif %}
+ </span>
+ <span class="list__item list__item--reviewer-outcome">{{ reviewer.review.get_recommendation_display }}</span>
</li>
- {% for opinion in review.opinions.all %}
+ {% for opinion in reviewer.review.opinions.all %}
{% if forloop.first %}
<ul class="list list--opinion">
{% endif %}
<li class="list__item list__item--reviewer list__item--opinion">
- <span class="list__item--reviewer-name">{{ opinion.author }}</span>
+ <span class="list__item--reviewer-name">
+ {{ opinion.author }}
+ {% if opinion.author.role %}{% image opinion.author.role.icon max-12x12 %}{% endif %}
+ </span>
<span class="list__item list__item--reviewer-outcome {{ opinion.get_opinion_display|lower }}">{{ opinion.get_opinion_display }}</span>
</li>
@@ -58,9 +65,12 @@
{% endif %}
{% endfor %}
{% endfor %}
- {% for reviewer in submission.staff_not_reviewed %}
+ {% for reviewer in submission.hasnt_reviewed %}
<li class="list__item list__item--reviewer">
- <span class="list__item--reviewer-name">{{ reviewer }}</span>
+ <span class="list__item--reviewer-name">
+ {{ reviewer }}
+ {% if reviewer.role %}{% image reviewer.role.icon max-12x12 %}{% endif %}
+ </span>
<span class="list__item list__item--reviewer-outcome">—</span>
</li>
{% endfor %}
diff --git a/opentech/apply/funds/tests/test_models.py b/opentech/apply/funds/tests/test_models.py
index 545ce1c57ad7faf5fe48e3dfc66055f23254e591..eaa22338c1c81d12aabff0d25395dc18916a5c1f 100644
--- a/opentech/apply/funds/tests/test_models.py
+++ b/opentech/apply/funds/tests/test_models.py
@@ -535,7 +535,23 @@ class TestForTableQueryset(TestCase):
submission = qs[0]
self.assertEqual(submission.opinion_disagree, 1)
self.assertEqual(submission.review_count, 2)
- self.assertEqual(submission.review_submitted_count, 2)
+ # Reviewers that disagree are not counted
+ self.assertEqual(submission.review_submitted_count, 1)
+ self.assertEqual(submission.review_recommendation, MAYBE)
+
+ def test_opinionated_slash_confused_reviewer(self):
+ staff = StaffFactory()
+ submission = ApplicationSubmissionFactory()
+ review_one = ReviewFactory(submission=submission)
+ review_two = ReviewFactory(submission=submission)
+ ReviewOpinionFactory(opinion_disagree=True, review=review_one, author__reviewer=staff)
+ ReviewOpinionFactory(opinion_agree=True, review=review_two, author__reviewer=staff)
+ qs = ApplicationSubmission.objects.for_table(user=staff)
+ submission = qs[0]
+ self.assertEqual(submission.opinion_disagree, 1)
+ self.assertEqual(submission.review_count, 3)
+ # Reviewers that disagree are not counted
+ self.assertEqual(submission.review_submitted_count, 3)
self.assertEqual(submission.review_recommendation, MAYBE)
def test_dont_double_count_review_and_opinion(self):
@@ -570,7 +586,8 @@ class TestForTableQueryset(TestCase):
self.assertEqual(submission, submission_one)
self.assertEqual(submission.opinion_disagree, 1)
self.assertEqual(submission.review_count, 2)
- self.assertEqual(submission.review_submitted_count, 2)
+ # Reviewers that disagree are not counted
+ self.assertEqual(submission.review_submitted_count, 1)
self.assertEqual(submission.review_recommendation, MAYBE)
submission = qs[1]
diff --git a/opentech/apply/review/templates/review/review_list.html b/opentech/apply/review/templates/review/review_list.html
index 7487a763ff508614413959e5e3ff19857deec10c..8576b3f564ac3da2b6e4cd10bf83864d826b6230 100644
--- a/opentech/apply/review/templates/review/review_list.html
+++ b/opentech/apply/review/templates/review/review_list.html
@@ -23,7 +23,9 @@
<tr class="reviews-list__tr">
<th class="reviews-list__th">{{ answers.question }}</th>
{% for answer in answers.answers %}
- {% if answers.question == "Opinions" %}
+ {% if forloop.parentloop.first %}
+ <td class="reviews-list__td reviews-list__td--author">{{ answer|safe }}</td>
+ {% elif answers.question == "Opinions"%}
<td class="reviews-list__td">{{ answer }}</td>
{% else %}
<td class="reviews-list__td">{{ answer|bleach }}</td>
diff --git a/opentech/apply/review/views.py b/opentech/apply/review/views.py
index c31a7b7001664ef8c2256f28ea184ff6c12972c1..a7dc4778f49ba12f7624faceac84c17270f9791d 100644
--- a/opentech/apply/review/views.py
+++ b/opentech/apply/review/views.py
@@ -1,7 +1,6 @@
from django.contrib.auth.decorators import login_required
from django.contrib.auth.mixins import UserPassesTestMixin
from django.core.exceptions import PermissionDenied
-from django.db import models
from django.http import HttpResponseRedirect
from django.shortcuts import get_object_or_404
from django.template.loader import get_template
@@ -17,8 +16,9 @@ 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.groups import REVIEWER_GROUP_NAME, STAFF_GROUP_NAME, PARTNER_GROUP_NAME, COMMUNITY_REVIEWER_GROUP_NAME
+from opentech.apply.users.groups import REVIEWER_GROUP_NAME
from opentech.apply.utils.views import CreateOrUpdateView
+from opentech.apply.utils.image import generate_image_tag
from .models import Review
from .options import DISAGREE
@@ -26,27 +26,8 @@ from .options import DISAGREE
class ReviewContextMixin:
def get_context_data(self, **kwargs):
- review_order = [
- STAFF_GROUP_NAME,
- COMMUNITY_REVIEWER_GROUP_NAME,
- PARTNER_GROUP_NAME,
- REVIEWER_GROUP_NAME,
- ]
-
- ordering = [models.When(type__name=review_type, then=models.Value(i)) for i, review_type in enumerate(review_order)]
- assigned_reviewers = self.object.assigned.annotate(
- type_order=models.Case(
- *ordering,
- output_field=models.IntegerField(),
- )
- ).order_by(
- 'role__order',
- 'review',
- 'type_order'
- ).select_related(
- 'reviewer',
- 'role',
- )
+ assigned_reviewers = self.object.assigned.review_order()
+
if not self.object.stage.has_external_review:
assigned_reviewers = assigned_reviewers.staff()
@@ -286,9 +267,17 @@ class ReviewListView(ListView):
review_data['comments'] = {'question': 'Comments', 'answers': list()}
responses = self.object_list.count()
+ ordered_reviewers = AssignedReviewers.objects.filter(submission=self.submission).reviewed().review_order()
+
+ reviews = {review.author: review for review in self.object_list}
+ for i, reviewer in enumerate(ordered_reviewers):
+ review = reviews[reviewer]
+ author = '<a href="{}"><span>{}</span></a>'.format(review.get_absolute_url(), review.author)
+ if review.author.role:
+ author += generate_image_tag(review.author.role.icon, '12x12')
+ author = f'<div>{author}</div>'
- for i, review in enumerate(self.object_list):
- review_data['title']['answers'].append('<a href="{}">{}</a>'.format(review.get_absolute_url(), review.author))
+ review_data['title']['answers'].append(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)
diff --git a/opentech/static_src/src/sass/apply/components/_list.scss b/opentech/static_src/src/sass/apply/components/_list.scss
index 8b1f94a63f1662e1df1c028868acd98776e4fcf4..c965b40bb60e088c7773ce646b1e69f46b1ab5a0 100644
--- a/opentech/static_src/src/sass/apply/components/_list.scss
+++ b/opentech/static_src/src/sass/apply/components/_list.scss
@@ -1,5 +1,6 @@
.list {
$root: &;
+ max-width: 290px;
&--no-margin {
margin: 0;
@@ -12,7 +13,6 @@
border-bottom: 1px solid $color--mid-grey;
margin: 10px 0;
padding: 5px 0;
- max-width: 150px;
#{$root}__item--opinion:first-child {
span:last-child {
@@ -39,14 +39,19 @@
&--reviewer {
display: flex;
justify-content: space-between;
- max-width: 150px;
}
&--reviewer-name {
- max-width: 100px;
+ max-width: 190px;
overflow: hidden;
font-weight: $weight--bold;
text-overflow: ellipsis;
+ display: flex;
+ align-items: center;
+
+ > img {
+ margin-left: 10px;
+ }
// show truncated emails on hover
&:hover {
diff --git a/opentech/static_src/src/sass/apply/components/_reviews-list.scss b/opentech/static_src/src/sass/apply/components/_reviews-list.scss
index 3d4e066342f396fe7c83e4714a36ffec8367da4d..e3b41fe543f81e598fdebdf34f98283c39760c9b 100644
--- a/opentech/static_src/src/sass/apply/components/_reviews-list.scss
+++ b/opentech/static_src/src/sass/apply/components/_reviews-list.scss
@@ -21,6 +21,17 @@
max-width: 340px;
min-width: 240px;
padding: 20px;
+
+ &--author {
+ > div {
+ display: flex;
+ align-items: center;
+
+ img {
+ margin-left: 7px;
+ }
+ }
+ }
}
&__th,