Allow dependent questions to depend on multiple values (#1336)

This commit is contained in:
Raphael Michel
2019-07-11 13:32:45 +02:00
committed by GitHub
parent d994fc674a
commit 53a0d62d93
17 changed files with 214 additions and 74 deletions

View File

@@ -201,15 +201,25 @@ class InlineQuestionOptionSerializer(I18nAwareModelSerializer):
fields = ('id', 'identifier', 'answer', 'position')
class LegacyDependencyValueField(serializers.CharField):
def to_representation(self, obj):
return obj[0] if obj else None
def to_internal_value(self, data):
return [data] if data else []
class QuestionSerializer(I18nAwareModelSerializer):
options = InlineQuestionOptionSerializer(many=True, required=False)
identifier = serializers.CharField(allow_null=True)
dependency_value = LegacyDependencyValueField(source='dependency_values', required=False, allow_null=True)
class Meta:
model = Question
fields = ('id', 'question', 'type', 'required', 'items', 'options', 'position',
'ask_during_checkin', 'identifier', 'dependency_question', 'dependency_value',
'hidden')
'ask_during_checkin', 'identifier', 'dependency_question', 'dependency_values',
'hidden', 'dependency_value')
def validate_identifier(self, value):
Question._clean_identifier(self.context['event'], value, self.instance)
@@ -263,6 +273,7 @@ class QuestionSerializer(I18nAwareModelSerializer):
def create(self, validated_data):
options_data = validated_data.pop('options') if 'options' in validated_data else []
items = validated_data.pop('items')
question = Question.objects.create(**validated_data)
question.items.set(items)
for opt_data in options_data:

View File

@@ -1,4 +1,5 @@
import copy
import json
import logging
from decimal import Decimal
from urllib.error import HTTPError
@@ -10,6 +11,7 @@ import vat_moss.id
from django import forms
from django.contrib import messages
from django.core.exceptions import ValidationError
from django.db.models import QuerySet
from django.utils.html import escape
from django.utils.safestring import mark_safe
from django.utils.translation import get_language, ugettext_lazy as _
@@ -25,6 +27,7 @@ from pretix.base.models.tax import EU_COUNTRIES
from pretix.base.settings import PERSON_NAME_SCHEMES
from pretix.base.templatetags.rich_text import rich_text
from pretix.control.forms import SplitDateTimeField
from pretix.helpers.escapejson import escapejson_attr
from pretix.helpers.i18n import get_format_without_seconds
from pretix.presale.signals import question_form_fields
@@ -278,7 +281,7 @@ class BaseQuestionsForm(forms.Form):
if q.dependency_question_id:
field.widget.attrs['data-question-dependency'] = q.dependency_question_id
field.widget.attrs['data-question-dependency-value'] = q.dependency_value
field.widget.attrs['data-question-dependency-values'] = escapejson_attr(json.dumps(q.dependency_values))
if q.type != 'M':
field.widget.attrs['required'] = q.required and not self.all_optional
field._required = q.required and not self.all_optional
@@ -299,26 +302,24 @@ class BaseQuestionsForm(forms.Form):
question_cache = {f.question.pk: f.question for f in self.fields.values() if getattr(f, 'question', None)}
def question_is_visible(parentid, qval):
def question_is_visible(parentid, qvals):
parentq = question_cache[parentid]
if parentq.dependency_question_id and not question_is_visible(parentq.dependency_question_id, parentq.dependency_value):
if parentq.dependency_question_id and not question_is_visible(parentq.dependency_question_id, parentq.dependency_values):
return False
if 'question_%d' % parentid not in d:
return False
dval = d.get('question_%d' % parentid)
if qval == 'True':
return dval
elif qval == 'False':
return not dval
elif isinstance(dval, QuestionOption):
return dval.identifier == qval
else:
return qval in [o.identifier for o in dval]
return (
('True' in qvals and dval)
or ('False' in qvals and not dval)
or (isinstance(dval, QuestionOption) and dval.identifier in qvals)
or (isinstance(dval, (list, QuerySet)) and any(qval in [o.identifier for o in dval] for qval in qvals))
)
def question_is_required(q):
return (
q.required and
(not q.dependency_question_id or question_is_visible(q.dependency_question_id, q.dependency_value))
(not q.dependency_question_id or question_is_visible(q.dependency_question_id, q.dependency_values))
)
if not self.all_optional:

View File

@@ -0,0 +1,25 @@
# Generated by Django 2.2.1 on 2019-07-11 07:05
from django.db import migrations
import pretix.base.models.fields
class Migration(migrations.Migration):
dependencies = [
('pretixbase', '0126_item_show_quota_left'),
]
operations = [
migrations.RenameField(
model_name='question',
old_name='dependency_value',
new_name='dependency_values',
),
migrations.AlterField(
model_name='question',
name='dependency_values',
field=pretix.base.models.fields.MultiStringField(default=['']),
),
]

View File

@@ -22,6 +22,7 @@ from i18nfield.fields import I18nCharField, I18nTextField
from pretix.base.models import fields
from pretix.base.models.base import LoggedModel
from pretix.base.models.fields import MultiStringField
from pretix.base.models.tax import TaxedPrice
from pretix.base.signals import quota_availability
@@ -934,8 +935,8 @@ class Question(LoggedModel):
:type identifier: str
:param dependency_question: This question will only show up if the referenced question is set to `dependency_value`.
:type dependency_question: Question
:param dependency_value: The value that `dependency_question` needs to be set to for this question to be applicable.
:type dependency_value: str
:param dependency_values: The values that `dependency_question` needs to be set to for this question to be applicable.
:type dependency_values: list[str]
"""
TYPE_NUMBER = "N"
TYPE_STRING = "S"
@@ -1015,7 +1016,7 @@ class Question(LoggedModel):
dependency_question = models.ForeignKey(
'Question', null=True, blank=True, on_delete=models.SET_NULL, related_name='dependent_questions'
)
dependency_value = models.TextField(null=True, blank=True)
dependency_values = MultiStringField(default=[])
objects = ScopedManager(organizer='event__organizer')

View File

@@ -1038,18 +1038,17 @@ class AbstractPosition(models.Model):
q.pk: q for q in questions
}
def question_is_visible(parentid, qval):
def question_is_visible(parentid, qvals):
parentq = question_cache[parentid]
if parentq.dependency_question_id and not question_is_visible(parentq.dependency_question_id, parentq.dependency_value):
if parentq.dependency_question_id and not question_is_visible(parentq.dependency_question_id, parentq.dependency_values):
return False
if parentid not in self.answ:
return False
if qval == 'True':
return self.answ[parentid].answer == 'True'
elif qval == 'False':
return self.answ[parentid].answer == 'False'
else:
return qval in [o.identifier for o in self.answ[parentid].options.all()]
return (
('True' in qvals and self.answ[parentid].answer == 'True')
or ('False' in qvals and self.answ[parentid].answer == 'False')
or (any(qval in [o.identifier for o in self.answ[parentid].options.all()] for qval in qvals))
)
self.questions = []
for q in questions:
@@ -1058,7 +1057,7 @@ class AbstractPosition(models.Model):
q.answer.question = q # cache object
else:
q.answer = ""
if not q.dependency_question_id or question_is_visible(q.dependency_question_id, q.dependency_value):
if not q.dependency_question_id or question_is_visible(q.dependency_question_id, q.dependency_values):
self.questions.append(q)
@property

View File

@@ -1,3 +1,5 @@
import json
from django import template
from django.template.defaultfilters import stringfilter
@@ -11,3 +13,9 @@ register = template.Library()
def escapejs_filter(value):
"""Hex encodes characters for use in a application/json type script."""
return escapejson(value)
@register.filter("escapejson_dumps")
def escapejs_dumps_filter(value):
"""Hex encodes characters for use in a application/json type script."""
return escapejson(json.dumps(value))

View File

@@ -55,8 +55,13 @@ class QuestionForm(I18nModelForm):
pk=self.instance.pk
)
self.fields['identifier'].required = False
self.fields['dependency_values'].required = False
self.fields['help_text'].widget.attrs['rows'] = 3
def clean_dependency_values(self):
val = self.data.getlist('dependency_values')
return val
def clean_dependency_question(self):
dep = val = self.cleaned_data.get('dependency_question')
if dep:
@@ -70,8 +75,8 @@ class QuestionForm(I18nModelForm):
def clean(self):
d = super().clean()
if d.get('dependency_question') and not d.get('dependency_value'):
raise ValidationError({'dependency_value': [_('This field is required')]})
if d.get('dependency_question') and not d.get('dependency_values'):
raise ValidationError({'dependency_values': [_('This field is required')]})
if d.get('dependency_question') and d.get('ask_during_checkin'):
raise ValidationError(_('Dependencies between questions are not supported during check-in.'))
return d
@@ -89,13 +94,13 @@ class QuestionForm(I18nModelForm):
'identifier',
'items',
'dependency_question',
'dependency_value'
'dependency_values'
]
widgets = {
'items': forms.CheckboxSelectMultiple(
attrs={'class': 'scrolling-multiple-choice'}
),
'dependency_value': forms.Select,
'dependency_values': forms.SelectMultiple,
}
field_classes = {
'items': SafeModelMultipleChoiceField,

View File

@@ -2,6 +2,7 @@
{% load i18n %}
{% load bootstrap3 %}
{% load formset_tags %}
{% load escapejson %}
{% block title %}
{% if question %}
{% blocktrans with name=question.question %}Question: {{ name }}{% endblocktrans %}
@@ -120,8 +121,8 @@
{% bootstrap_field form.dependency_question layout="inline" form_group_class="inner" %}
</div>
<div class="col-md-5">
<script type="text/plain" id="dependency_value_val">{{ form.instance.dependency_value }}</script>
{% bootstrap_field form.dependency_value layout="inline" form_group_class="inner" %}
<script type="text/plain" id="dependency_value_val">{{ form.instance.dependency_values|escapejson_dumps }}</script>
{% bootstrap_field form.dependency_values layout="inline" form_group_class="inner" %}
</div>
</div>
</fieldset>

View File

@@ -32,7 +32,7 @@
</a>
</div>
{% endif %}
<div class="form-group submit-group">
<div class="form-group submit-group">
<a href="{% url "control:event.items.quotas" organizer=request.event.organizer.slug event=request.event.slug %}" class="btn btn-default btn-cancel">
{% trans "Cancel" %}
</a>

View File

@@ -9,8 +9,23 @@ _json_escapes = {
ord('&'): '\\u0026',
}
_json_escapes_attr = {
ord('>'): '\\u003E',
ord('<'): '\\u003C',
ord('&'): '\\u0026',
ord('"'): '&#34;',
ord("'"): '&#39;',
ord("="): '&#61;',
}
@keep_lazy(six.text_type, SafeText)
def escapejson(value):
"""Hex encodes characters for use in a application/json type script."""
return mark_safe(force_text(value).translate(_json_escapes))
@keep_lazy(six.text_type, SafeText)
def escapejson_attr(value):
"""Hex encodes characters for use in a html attributw script."""
return mark_safe(force_text(value).translate(_json_escapes_attr))

View File

@@ -439,23 +439,22 @@ class QuestionsStep(QuestionsViewMixin, CartMixin, TemplateFlowStep):
q.pk: q for q in cp.item.questions_to_ask
}
def question_is_visible(parentid, qval):
def question_is_visible(parentid, qvals):
parentq = question_cache[parentid]
if parentq.dependency_question_id and not question_is_visible(parentq.dependency_question_id, parentq.dependency_value):
if parentq.dependency_question_id and not question_is_visible(parentq.dependency_question_id, parentq.dependency_values):
return False
if parentid not in answ:
return False
if qval == 'True':
return answ[parentid].answer == 'True'
elif qval == 'False':
return answ[parentid].answer == 'False'
else:
return qval in [o.identifier for o in answ[parentid].options.all()]
return (
('True' in qvals and answ[parentid].answer == 'True')
or ('False' in qvals and answ[parentid].answer == 'False')
or (any(qval in [o.identifier for o in answ[parentid].options.all()] for qval in qvals))
)
def question_is_required(q):
return (
q.required and
(not q.dependency_question_id or question_is_visible(q.dependency_question_id, q.dependency_value))
(not q.dependency_question_id or question_is_visible(q.dependency_question_id, q.dependency_values))
)
for q in cp.item.questions_to_ask:

View File

@@ -95,22 +95,22 @@ $(function () {
$(".alert-required-boolean").toggle(show);
}
var $val = $("#id_dependency_value");
var $val = $("#id_dependency_values");
var $dq = $("#id_dependency_question");
var oldval = $("#dependency_value_val").text();
var oldval = JSON.parse($("#dependency_value_val").text());
function update_dependency_options() {
$val.parent().find(".loading-indicator").remove();
$("#id_dependency_value option").remove();
$("#id_dependency_value").prop("required", false);
$("#id_dependency_values option").remove();
$("#id_dependency_values").prop("required", false);
var val = $dq.children("option:selected").val();
if (!val) {
$("#id_dependency_value").show();
$("#id_dependency_values").show();
$val.show();
return;
}
$("#id_dependency_value").prop("required", true);
$("#id_dependency_values").prop("required", true);
$val.hide();
$val.parent().append("<div class=\"help-block loading-indicator\"><span class=\"fa" +
" fa-cog fa-spin\"></span></div>");

View File

@@ -7,30 +7,34 @@ function questions_toggle_dependent(ev) {
}
var dependency_name = $el.attr("name").split("_")[0] + "_" + $el.attr("data-question-dependency");
var dependency_value = $el.attr("data-question-dependency-value");
var dependency_values = JSON.parse($el.attr("data-question-dependency-values"));
var $dependency_el;
if ($("select[name=" + dependency_name + "]").length) {
// dependency is type C
$dependency_el = $("select[name=" + dependency_name + "]");
if (!$dependency_el.closest(".form-group").hasClass("dependency-hidden")) { // do not show things that depend on hidden things
return q_should_be_shown($dependency_el) && $dependency_el.val() === dependency_value;
return q_should_be_shown($dependency_el) && $.inArray($dependency_el.val(), dependency_values) > -1;
}
} else if ($("input[type=checkbox][name=" + dependency_name + "]").length) {
// dependency type is B or M
if (dependency_value === "True" || dependency_value === "False") {
if ($.inArray("True", dependency_values) > -1 || $.inArray("False", dependency_values) > -1) {
$dependency_el = $("input[name=" + dependency_name + "]");
if (!$dependency_el.closest(".form-group").hasClass("dependency-hidden")) { // do not show things that depend on hidden things
if (dependency_value === "True") {
return q_should_be_shown($dependency_el) && $dependency_el.prop('checked');
} else {
return q_should_be_shown($dependency_el) && !$dependency_el.prop('checked');
}
return q_should_be_shown($dependency_el) && (
($.inArray("True", dependency_values) > -1 && $dependency_el.prop('checked'))
|| ($.inArray("False", dependency_values) > -1 && !$dependency_el.prop('checked'))
);
}
} else {
$dependency_el = $("input[value=" + dependency_value + "][name=" + dependency_name + "]");
var filter = "";
for (var i = 0; i < dependency_values.length; i++) {
if (filter) filter += ", ";
filter += "input[value=" + dependency_values[i] + "][name=" + dependency_name + "]:checked";
}
$dependency_el = $("input[value=" + dependency_values[0] + "][name=" + dependency_name + "]");
if (!$dependency_el.closest(".form-group").hasClass("dependency-hidden")) { // do not show things that depend on hidden things
return q_should_be_shown($dependency_el) && $dependency_el.prop('checked');
return q_should_be_shown($dependency_el) && $(filter).length;
}
}
}

View File

@@ -1651,6 +1651,7 @@ TEST_QUESTION_RES = {
"position": 0,
"dependency_question": None,
"dependency_value": None,
"dependency_values": [],
"options": [
{
"id": 0,
@@ -1883,6 +1884,49 @@ def test_question_delete(token_client, organizer, event, question):
assert not event.questions.filter(pk=question.id).exists()
@pytest.mark.django_db
def test_question_update_dependency_values(token_client, organizer, event, question):
resp = token_client.patch(
'/api/v1/organizers/{}/events/{}/questions/{}/'.format(organizer.slug, event.slug, question.pk),
{
"dependency_values": ["a", "b"]
},
format='json'
)
assert resp.status_code == 200
question.refresh_from_db()
assert question.dependency_values == ["a", "b"]
@pytest.mark.django_db
def test_question_update_dependency_value_legacy(token_client, organizer, event, question):
resp = token_client.patch(
'/api/v1/organizers/{}/events/{}/questions/{}/'.format(organizer.slug, event.slug, question.pk),
{
"dependency_value": "a"
},
format='json'
)
assert resp.status_code == 200
question.refresh_from_db()
assert question.dependency_values == ["a"]
@pytest.mark.django_db
def test_question_update_dependency_value_legacy_conflict(token_client, organizer, event, question):
resp = token_client.patch(
'/api/v1/organizers/{}/events/{}/questions/{}/'.format(organizer.slug, event.slug, question.pk),
{
"dependency_values": ["a", "b"],
"dependency_value": "a"
},
format='json'
)
assert resp.status_code == 200
question.refresh_from_db()
assert question.dependency_values == ["a"]
TEST_OPTIONS_RES = {
"identifier": "LVETRWVU",
"answer": {"en": "XL"},

View File

@@ -245,24 +245,24 @@ class QuestionsTest(ItemFormTest):
form_data = extract_form_fields(doc.select('.container-fluid form')[0])
form_data['items'] = self.item1.id
form_data['dependency_question'] = q1.pk
form_data['dependency_value'] = o1.identifier
form_data['dependency_values'] = o1.identifier
doc = self.post_doc('/control/event/%s/%s/questions/%s/change' % (self.orga1.slug, self.event1.slug, q2.id),
form_data)
assert doc.select(".alert-success")
q2.refresh_from_db()
assert q2.dependency_question == q1
assert q2.dependency_value == o1.identifier
assert q2.dependency_values == [o1.identifier]
def test_set_dependency_circular(self):
with scopes_disabled():
q1 = Question.objects.create(event=self.event1, question="What country are you from?", type="C", required=True)
o1 = q1.options.create(answer='Germany')
q2 = Question.objects.create(event=self.event1, question="What city are you from?", type="C", required=True,
dependency_question=q1, dependency_value=o1.identifier)
dependency_question=q1, dependency_values=[o1.identifier])
doc = self.get_doc('/control/event/%s/%s/questions/%s/change' % (self.orga1.slug, self.event1.slug, q1.id))
form_data = extract_form_fields(doc.select('.container-fluid form')[0])
form_data['dependency_question'] = q2.pk
form_data['dependency_value'] = '1'
form_data['dependency_values'] = '1'
doc = self.post_doc('/control/event/%s/%s/questions/%s/change' % (self.orga1.slug, self.event1.slug, q1.id),
form_data)
assert not doc.select(".alert-success")
@@ -274,7 +274,7 @@ class QuestionsTest(ItemFormTest):
doc = self.get_doc('/control/event/%s/%s/questions/%s/change' % (self.orga1.slug, self.event1.slug, q2.id))
form_data = extract_form_fields(doc.select('.container-fluid form')[0])
form_data['dependency_question'] = q1.pk
form_data['dependency_value'] = '1'
form_data['dependency_values'] = '1'
doc = self.post_doc('/control/event/%s/%s/questions/%s/change' % (self.orga1.slug, self.event1.slug, q2.id),
form_data)
assert not doc.select(".alert-success")

View File

@@ -2123,32 +2123,33 @@ class QuestionsTestCase(BaseCheckoutTestCase, TestCase):
)
self.q1.options.create(answer='Tech', identifier='TECH')
self.q1.options.create(answer='Health', identifier='HEALTH')
self.q1.options.create(answer='IT', identifier='IT')
self.q2a = self.event.questions.create(
event=self.event, question='What is your occupation?', type=Question.TYPE_CHOICE_MULTIPLE,
required=False, dependency_question=self.q1, dependency_value='TECH'
required=False, dependency_question=self.q1, dependency_values=['TECH', 'IT']
)
self.q2a.options.create(answer='Software developer', identifier='DEV')
self.q2a.options.create(answer='System administrator', identifier='ADMIN')
self.q2b = self.event.questions.create(
event=self.event, question='What is your occupation?', type=Question.TYPE_CHOICE_MULTIPLE,
required=True, dependency_question=self.q1, dependency_value='HEALTH'
required=True, dependency_question=self.q1, dependency_values=['HEALTH']
)
self.q2b.options.create(answer='Doctor', identifier='DOC')
self.q2b.options.create(answer='Nurse', identifier='NURSE')
self.q3 = self.event.questions.create(
event=self.event, question='Do you like Python?', type=Question.TYPE_BOOLEAN,
required=False, dependency_question=self.q2a, dependency_value='DEV'
required=False, dependency_question=self.q2a, dependency_values=['DEV']
)
self.q4a = self.event.questions.create(
event=self.event, question='Why?', type=Question.TYPE_TEXT,
required=True, dependency_question=self.q3, dependency_value='True'
required=True, dependency_question=self.q3, dependency_values=['True']
)
self.q4b = self.event.questions.create(
event=self.event, question='Why not?', type=Question.TYPE_TEXT,
required=True, dependency_question=self.q3, dependency_value='False'
required=True, dependency_question=self.q3, dependency_values=['False']
)
self.ticket.questions.add(self.q1)
self.ticket.questions.add(self.q2a)
@@ -2188,6 +2189,15 @@ class QuestionsTestCase(BaseCheckoutTestCase, TestCase):
self.q4a: 'No curly braces!'
}, should_fail=False)
def test_question_dependencies_second_path_alterative(self):
self._setup_dependency_questions()
self._test_question_input({
self.q1: 'IT',
self.q2a: 'DEV',
self.q3: 'True',
self.q4a: 'No curly braces!'
}, should_fail=False)
def test_question_dependencies_subitem_required(self):
self._setup_dependency_questions()
self._test_question_input({
@@ -2202,6 +2212,14 @@ class QuestionsTestCase(BaseCheckoutTestCase, TestCase):
self.q3: 'True',
}, should_fail=True)
def test_question_dependencies_subsubitem_required_alternative(self):
self._setup_dependency_questions()
self._test_question_input({
self.q1: 'IT',
self.q2a: 'DEV',
self.q3: 'True',
}, should_fail=True)
def test_question_dependencies_parent_not_required(self):
self._setup_dependency_questions()
self._test_question_input({