From 29228641bfd7f044f3bb7662f9e28789da8704f6 Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Thu, 23 Aug 2018 13:07:55 +0100
Subject: [PATCH] Make sure we correctly store and handle the score values and
add tests
---
opentech/apply/review/blocks.py | 2 +-
opentech/apply/review/fields.py | 48 +++++--------------
opentech/apply/review/forms.py | 12 ++---
opentech/apply/review/tests/test_views.py | 17 +++++++
.../apply/stream_forms/testing/factories.py | 19 ++++++--
5 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/opentech/apply/review/blocks.py b/opentech/apply/review/blocks.py
index d271c864c..275f1601f 100644
--- a/opentech/apply/review/blocks.py
+++ b/opentech/apply/review/blocks.py
@@ -22,7 +22,7 @@ class ScoreFieldBlock(OptionalFormFieldBlock):
template = 'review/render_scored_answer_field.html'
def render(self, value, context=None):
- comment, score = json.loads(context['data'])
+ comment, score = context['data']
context.update(**{
'comment': comment,
'score': RATE_CHOICES_DICT.get(int(score), RATE_CHOICE_NA)
diff --git a/opentech/apply/review/fields.py b/opentech/apply/review/fields.py
index aeaeaa79e..8f1e8eee2 100644
--- a/opentech/apply/review/fields.py
+++ b/opentech/apply/review/fields.py
@@ -1,5 +1,3 @@
-import json
-
from django import forms
from tinymce import TinyMCE
@@ -19,44 +17,22 @@ class ScoredAnswerWidget(forms.MultiWidget):
super().__init__(_widgets, attrs)
def decompress(self, value):
+ # We should only hit this on initialisation where we set the default to a list of None
if value:
- return json.loads(value)
+ return value
return [None, None]
def render(self, name, value, attrs=None, renderer=None):
- """
- Render the widget as an HTML string.
- Required for the correct rendering of the TinyMCE widget.
- """
- if self.is_localized:
- for widget in self.widgets:
- widget.is_localized = self.is_localized
- # value is a list of values, each corresponding to a widget
- # in self.widgets.
- if not isinstance(value, list):
- value = self.decompress(value)
-
+ context = self.get_context(name, value, attrs)
rendered = []
- final_attrs = self.build_attrs(attrs)
- input_type = final_attrs.pop('type', None)
- id_ = final_attrs.get('id')
- for i, widget in enumerate(self.widgets):
- if input_type is not None:
- widget.input_type = input_type
- widget_name = '%s_%s' % (name, i)
- try:
- widget_value = value[i]
- except IndexError:
- widget_value = None
- if id_:
- widget_attrs = final_attrs.copy()
- widget_attrs['id'] = '%s_%s' % (id_, i)
- else:
- widget_attrs = final_attrs
-
- rendered.append(widget.render(widget_name, widget_value, widget_attrs, renderer))
-
- return ''.join([mark_safe(item) for item in rendered])
+ # We need to explicitly call the render method on the tinymce widget
+ # MultiValueWidget just passes all the context into the template
+ for kwargs, widget in zip(context['widget']['subwidgets'], self.widgets):
+ name = kwargs['name']
+ value = kwargs['value']
+ attrs = kwargs['attrs']
+ rendered.append(widget.render(name, value, attrs, renderer))
+ return mark_safe(''.join([widget for widget in rendered]))
class ScoredAnswerField(forms.MultiValueField):
@@ -71,4 +47,4 @@ class ScoredAnswerField(forms.MultiValueField):
super().__init__(fields=fields, *args, **kwargs)
def compress(self, data_list):
- return json.dumps(data_list)
+ return [data_list[0], int(data_list[1])]
diff --git a/opentech/apply/review/forms.py b/opentech/apply/review/forms.py
index 6ff588cd2..28a206afe 100644
--- a/opentech/apply/review/forms.py
+++ b/opentech/apply/review/forms.py
@@ -78,15 +78,9 @@ class ReviewModelForm(StreamBaseForm, forms.ModelForm, metaclass=MixedMetaClass)
scores = list()
for field in self.instance.score_fields:
- value = json.loads(data.get(field.id, '[null, null]'))
-
- try:
- score = int(value[1])
- except TypeError:
- pass
- else:
- if score != NA:
- scores.append(score)
+ score = data.get(field.id)[1]
+ if score != NA:
+ scores.append(score)
try:
return sum(scores) / len(scores)
diff --git a/opentech/apply/review/tests/test_views.py b/opentech/apply/review/tests/test_views.py
index 9b21dcabf..2a4ded257 100644
--- a/opentech/apply/review/tests/test_views.py
+++ b/opentech/apply/review/tests/test_views.py
@@ -115,6 +115,23 @@ class StaffReviewFormTestCase(BaseViewTestCase):
review = self.submission.reviews.first()
self.assertEqual(review.score, score)
+ def test_average_score_calculated(self):
+ form = ReviewFormFactory(form_fields__multiple__score=2)
+ review_form = self.submission.round.review_forms.first()
+ review_form.form = form
+ review_form.save()
+
+ score_1, score_2 = 1, 5
+
+ data = ReviewFormFieldsFactory.form_response(form.form_fields, {
+ field.id: {'score': score}
+ for field, score in zip(form.score_fields, [score_1, score_2])
+ })
+
+ self.post_page(self.submission, data, 'form')
+ review = self.submission.reviews.first()
+ self.assertEqual(review.score, (score_1 + score_2) / 2)
+
def test_no_score_is_NA(self):
form = ReviewFormFactory(form_fields__exclude__score=True)
review_form = self.submission.round.review_forms.first()
diff --git a/opentech/apply/stream_forms/testing/factories.py b/opentech/apply/stream_forms/testing/factories.py
index 398a54607..f742d6c50 100644
--- a/opentech/apply/stream_forms/testing/factories.py
+++ b/opentech/apply/stream_forms/testing/factories.py
@@ -177,29 +177,40 @@ class StreamFieldUUIDFactory(wagtail_factories.StreamFieldFactory):
def build_form(self, data):
extras = defaultdict(dict)
exclusions = []
+ multiples = dict()
for field, value in data.items():
# we dont care about position
name, attr = field.split('__')
if name == 'exclude':
exclusions.append(attr)
+ elif name == 'multiple':
+ multiples[attr] = value
else:
extras[name] = {attr: value}
+ defined_both = set(exclusions) & set(multiples)
+ if defined_both:
+ raise ValueError(
+ 'Cant exclude and provide multiple at the same time: {}'.format(', '.join(defined_both))
+ )
form_fields = {}
- for i, field in enumerate(self.factories):
+ field_count = 0
+ for field in self.factories:
if field == 'text_markup' or field in exclusions:
pass
else:
- form_fields[f'{i}__{field}__'] = ''
+ for _ in range(multiples.get(field, 1)):
+ form_fields[f'{field_count}__{field}__'] = ''
+ field_count += 1
for attr, value in extras[field].items():
- form_fields[f'{i}__{field}__{attr}'] = value
+ form_fields[f'{field_count}__{field}__{attr}'] = value
return form_fields
def form_response(self, fields, field_values=dict()):
data = {
- field.id: self.factories[field.block.name].make_form_answer(field_values.get(field, {}))
+ field.id: self.factories[field.block.name].make_form_answer(field_values.get(field.id, {}))
for field in fields
if hasattr(self.factories[field.block.name], 'make_form_answer')
}
--
GitLab