diff --git a/.travis.yml b/.travis.yml index 092de86..753cb20 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,8 +25,6 @@ script: - mkdir testapp/testapp/templates/core - cp molo/surveys/test_templates/*.html testapp/testapp/templates/core/ - cp molo/surveys/test_templates/base.html testapp/testapp/templates/base.html - - mkdir testapp/testapp/templates/wagtailsurveys - - cp molo/surveys/test_templates/wagtailsurveys/*.html testapp/testapp/templates/wagtailsurveys/ - flake8 testapp - pip install -e testapp - py.test --cov=molo.surveys --cov-report=term diff --git a/CHANGES.rst b/CHANGES.rst index acb242c..05cdabd 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,5 +1,15 @@ CHANGE LOG ========== + +6.7.6 +----- +- Bugfix: Ensure results of surveys, that are children of articles, pull through to the admin view + +6.7.5 +----- +- Bugfix: Handle errors when testing invalid rules +- Upgrade wagtail-personalisation-molo to 0.11.3 + 6.7.4 ----- - Upgrade wagtail-personalisation-molo to 0.11.2 diff --git a/VERSION b/VERSION index 29310f4..2ed16f4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.7.4 +6.7.6 diff --git a/molo/surveys/admin_urls.py b/molo/surveys/admin_urls.py new file mode 100644 index 0000000..1c32b8a --- /dev/null +++ b/molo/surveys/admin_urls.py @@ -0,0 +1,8 @@ +from django.conf.urls import url + +from molo.surveys.views import index + +urlpatterns = [ + # re-route to overwritten index view, originally in wagtailsurveys + url(r'^$', index, name='index'), +] diff --git a/molo/surveys/rules.py b/molo/surveys/rules.py index 1a90801..0ca1b97 100644 --- a/molo/surveys/rules.py +++ b/molo/surveys/rules.py @@ -4,7 +4,7 @@ from django import forms from django.apps import apps from django.conf import settings -from django.core.exceptions import ValidationError +from django.core.exceptions import ValidationError, ObjectDoesNotExist from django.db import models from django.test.client import RequestFactory from django.utils import six, timezone @@ -297,7 +297,11 @@ def test_user(self, request, user=None): if not user: return False - submission_class = self.survey.get_submission_class() + # Django formsets don't honour 'required' fields so check rule is valid + try: + submission_class = self.survey.get_submission_class() + except ObjectDoesNotExist: + return False return submission_class.objects.filter( user=user, @@ -463,6 +467,12 @@ def test_user(self, request, user=None): # Return false if we don't have a user or a request return False + # Django formsets don't honour 'required' fields so check rule is valid + try: + self.tag + except ObjectDoesNotExist: + return False + from wagtail_personalisation.adapters import get_segment_adapter operator = self.OPERATORS[self.operator] adapter = get_segment_adapter(request) diff --git a/molo/surveys/templates/wagtailsurveys/list_forms.html b/molo/surveys/templates/wagtailsurveys/list_forms.html deleted file mode 100644 index b01d89a..0000000 --- a/molo/surveys/templates/wagtailsurveys/list_forms.html +++ /dev/null @@ -1,24 +0,0 @@ -{% load i18n molo_survey_tags %} - - - - - - - - - - - {% get_survey_list_for_site as survey_pages %} - {% for sp in survey_pages %} - - - - - {% endfor %} - -
{% trans "Title" %}{% trans "Origin" %}
-

{{ sp|capfirst }}

-
- {{ sp.content_type.name |capfirst }} ({{ sp.content_type.app_label }}.{{ sp.content_type.model }}) -
diff --git a/molo/surveys/templatetags/molo_survey_tags.py b/molo/surveys/templatetags/molo_survey_tags.py index a4a0a92..0517e53 100644 --- a/molo/surveys/templatetags/molo_survey_tags.py +++ b/molo/surveys/templatetags/molo_survey_tags.py @@ -98,18 +98,6 @@ def surveys_list(context, pk=None, only_linked_surveys=False, return add_form_objects_to_surveys(context) -@register.simple_tag(takes_context=True) -def get_survey_list_for_site(context): - context = copy(context) - main = context['request'].site.root_page - page = SurveysIndexPage.objects.child_of(main).live().first() - if page: - return ( - MoloSurveyPage.objects.child_of(page).filter( - languages__language__is_main_language=True).specific()) - return None - - @register.simple_tag(takes_context=True) def submission_has_article(context, survey_id, submission_id): survey_page = get_object_or_404(Page, id=survey_id).specific diff --git a/molo/surveys/test_templates/wagtailsurveys/list_forms.html b/molo/surveys/test_templates/wagtailsurveys/list_forms.html deleted file mode 100644 index b01d89a..0000000 --- a/molo/surveys/test_templates/wagtailsurveys/list_forms.html +++ /dev/null @@ -1,24 +0,0 @@ -{% load i18n molo_survey_tags %} - - - - - - - - - - - {% get_survey_list_for_site as survey_pages %} - {% for sp in survey_pages %} - - - - - {% endfor %} - -
{% trans "Title" %}{% trans "Origin" %}
-

{{ sp|capfirst }}

-
- {{ sp.content_type.name |capfirst }} ({{ sp.content_type.app_label }}.{{ sp.content_type.model }}) -
diff --git a/molo/surveys/tests/base.py b/molo/surveys/tests/base.py new file mode 100644 index 0000000..8811236 --- /dev/null +++ b/molo/surveys/tests/base.py @@ -0,0 +1,51 @@ +from molo.surveys.models import ( + MoloSurveyFormField, + MoloSurveyPage, + SurveysIndexPage, +) +from .utils import skip_logic_data + + +def create_molo_survey_form_field(survey, sort_order, obj): + if obj['type'] == 'radio': + skip_logic = skip_logic_data(choices=obj['choices']) + else: + skip_logic = None + + return MoloSurveyFormField.objects.create( + page=survey, + sort_order=sort_order, + label=obj["question"], + field_type=obj["type"], + required=obj["required"], + page_break=obj["page_break"], + admin_label=obj["question"].lower().replace(" ", "_"), + skip_logic=skip_logic + ) + + +def create_molo_survey_page( + parent, title="Test Survey", slug='test-survey', **kwargs): + molo_survey_page = MoloSurveyPage( + title=title, slug=slug, + introduction='Introduction to Test Survey ...', + thank_you_text='Thank you for taking the Test Survey', + submit_text='survey submission text', + **kwargs + ) + + parent.add_child(instance=molo_survey_page) + molo_survey_page.save_revision().publish() + + return molo_survey_page + + +def create_survey(fields={}, **kwargs): + survey = create_molo_survey_page(SurveysIndexPage.objects.first()) + + if not fields == {}: + num_questions = len(fields) + for index, field in enumerate(reversed(fields)): + sort_order = num_questions - (index + 1) + create_molo_survey_form_field(survey, sort_order, field) + return survey diff --git a/molo/surveys/tests/test_admin.py b/molo/surveys/tests/test_admin.py index bee8ff8..27ce4f8 100644 --- a/molo/surveys/tests/test_admin.py +++ b/molo/surveys/tests/test_admin.py @@ -17,6 +17,8 @@ from wagtail_personalisation.models import Segment from wagtail_personalisation.rules import UserIsLoggedInRule +from .base import create_molo_survey_page + User = get_user_model() @@ -219,3 +221,21 @@ def test_export_submission_personalisable_survey(self): self.assertContains(response, self.user.username) self.assertContains(response, answer) + + def test_survey_index_view_displays_all_surveys(self): + child_of_index_page = create_molo_survey_page( + self.surveys_index, + title="Child of SurveysIndexPage Survey", + slug="child-of-surveysindexpage-survey" + ) + + child_of_article_page = create_molo_survey_page( + self.article, + title="Child of Article Survey", + slug="child-of-article-survey" + ) + + self.client.force_login(self.super_user) + response = self.client.get('/admin/surveys/') + self.assertContains(response, child_of_index_page.title) + self.assertContains(response, child_of_article_page.title) diff --git a/molo/surveys/tests/test_models.py b/molo/surveys/tests/test_models.py index 0df9189..b1789b1 100644 --- a/molo/surveys/tests/test_models.py +++ b/molo/surveys/tests/test_models.py @@ -12,6 +12,7 @@ ) from .utils import skip_logic_block_data, skip_logic_data +from .base import create_survey class TestSurveyModels(TestCase, MoloTestCaseMixin): @@ -140,50 +141,6 @@ def test_question_passes_with_object(self): self.assertEqual(cleaned_data['question'], 1) -def create_molo_survey_form_field(survey, sort_order, obj): - if obj['type'] == 'radio': - skip_logic = skip_logic_data(choices=obj['choices']) - else: - skip_logic = None - - return MoloSurveyFormField.objects.create( - page=survey, - sort_order=sort_order, - label=obj["question"], - field_type=obj["type"], - required=obj["required"], - page_break=obj["page_break"], - admin_label=obj["question"].lower().replace(" ", "_"), - skip_logic=skip_logic - ) - - -def create_molo_survey_page(parent, **kwargs): - molo_survey_page = MoloSurveyPage( - title='Test Survey', slug='test-survey', - introduction='Introduction to Test Survey ...', - thank_you_text='Thank you for taking the Test Survey', - submit_text='survey submission text', - **kwargs - ) - - parent.add_child(instance=molo_survey_page) - molo_survey_page.save_revision().publish() - - return molo_survey_page - - -def create_survey(fields={}, **kwargs): - survey = create_molo_survey_page(SurveysIndexPage.objects.first()) - - if not fields == {}: - num_questions = len(fields) - for index, field in enumerate(reversed(fields)): - sort_order = num_questions - (index + 1) - create_molo_survey_form_field(survey, sort_order, field) - return survey - - class TestPageBreakWithTwoQuestionsInOneStep(TestCase, MoloTestCaseMixin): def setUp(self): self.mk_main() diff --git a/molo/surveys/tests/test_rules.py b/molo/surveys/tests/test_rules.py index 29e80cd..18aab0c 100644 --- a/molo/surveys/tests/test_rules.py +++ b/molo/surveys/tests/test_rules.py @@ -415,6 +415,11 @@ def test_other_user_submitted_fails(self): self.request.user = new_user self.assertTrue(rule.test_user(self.request)) + def test_call_test_user_on_invalid_rule_fails(self): + self.submit_survey(self.survey, self.user) + rule = SurveyResponseRule() + self.assertFalse(rule.test_user(None, self.request.user)) + def test_call_test_user_without_request(self): self.submit_survey(self.survey, self.user) rule = SurveyResponseRule(survey=self.survey) @@ -644,6 +649,11 @@ def test_visting_non_tagged_page_isnt_error(self): self.adapter.add_page_visit(self.main) self.assertFalse(self.request.session['tag_count']) + def test_call_test_user_on_invalid_rule_fails(self): + rule = ArticleTagRule() + self.adapter.add_page_visit(self.article) + self.assertFalse(rule.test_user(None, self.request.user)) + def test_call_test_user_without_request(self): rule = ArticleTagRule( tag=self.tag, diff --git a/molo/surveys/views.py b/molo/surveys/views.py index a09cc66..0fee445 100644 --- a/molo/surveys/views.py +++ b/molo/surveys/views.py @@ -15,15 +15,32 @@ from django.shortcuts import render from django.http import JsonResponse from django.utils.translation import ugettext as _ +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin import messages from wagtail.wagtailadmin.utils import permission_required from wagtail_personalisation.forms import SegmentAdminForm from wagtail_personalisation.models import Segment +from wagtailsurveys.models import get_surveys_for_user + from .forms import CSVGroupCreationForm +def index(request): + survey_pages = get_surveys_for_user(request.user) + survey_pages = ( + survey_pages.descendant_of(request.site.root_page) + .filter(languages__language__is_main_language=True) + .specific() + ) + paginator, survey_pages = paginate(request, survey_pages) + + return render(request, 'wagtailsurveys/index.html', { + 'survey_pages': survey_pages, + }) + + class SegmentCountForm(SegmentAdminForm): class Meta: model = Segment diff --git a/molo/surveys/wagtail_hooks.py b/molo/surveys/wagtail_hooks.py index 5a934cc..c8547a4 100644 --- a/molo/surveys/wagtail_hooks.py +++ b/molo/surveys/wagtail_hooks.py @@ -1,4 +1,5 @@ from django.conf import settings +from django.conf.urls import include, url from django.utils.html import format_html_join from django.contrib.auth.models import User @@ -6,6 +7,7 @@ from wagtail.wagtailcore import hooks from molo.surveys.models import MoloSurveyPage, SurveyTermsConditions +from molo.surveys import admin_urls from molo.core.models import ArticlePage from .admin import SegmentUserGroupAdmin @@ -52,3 +54,12 @@ def create_new_page_relations(request, page, new_page): .first() relation.terms_and_conditions = new_article relation.save() + + +# This overrwrites the wagtailsuveys admin urls in order to use custom +# survey index view +@hooks.register('register_admin_urls') +def register_admin_urls(): + return [ + url(r'^surveys/', include(admin_urls)), + ] diff --git a/requirements.txt b/requirements.txt index b13554b..e15ea06 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ celery<4.0 dateparser>=0.7.0,<0.8.0 django-celery wagtailsurveys==0.1.1 -wagtail-personalisation-molo==0.11.2 +wagtail-personalisation-molo==0.11.3 psycopg2 html5lib==0.9999999 six==1.11.0