From 434cff11a476cc53c566584016fffdc6a9626325 Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Tue, 14 May 2019 17:29:41 +0100
Subject: [PATCH] GH-1191 Make reviewer ordering consistent and tweak styles on
tables
---
opentech/apply/funds/models/submissions.py | 51 +++++++++++++++++--
.../funds/templates/funds/tables/table.html | 26 +++++++---
opentech/apply/review/views.py | 26 ++--------
.../src/sass/apply/components/_list.scss | 11 ++--
4 files changed, 75 insertions(+), 39 deletions(-)
diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py
index 87d634b37..5e5820331 100644
--- a/opentech/apply/funds/models/submissions.py
+++ b/opentech/apply/funds/models/submissions.py
@@ -22,7 +22,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 +30,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 +169,7 @@ 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 +184,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 +768,37 @@ 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(),
+ )
+ ).order_by(
+ 'role__order',
+ 'type_order',
+ 'review',
+ ).select_related(
+ 'reviewer',
+ 'role',
+ )
+
def with_roles(self):
return self.filter(role__isnull=False)
diff --git a/opentech/apply/funds/templates/funds/tables/table.html b/opentech/apply/funds/templates/funds/tables/table.html
index c992d2b7a..db8cf106f 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/review/views.py b/opentech/apply/review/views.py
index c31a7b700..f428bedc6 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,7 +16,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.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 .models import Review
@@ -26,27 +25,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()
diff --git a/opentech/static_src/src/sass/apply/components/_list.scss b/opentech/static_src/src/sass/apply/components/_list.scss
index 8b1f94a63..c965b40bb 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 {
--
GitLab