From a93c152eca2e0c84349fc038744f15971be68457 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Mon, 19 Mar 2018 12:58:49 +0200 Subject: [PATCH 1/4] [Refactor] Move survey test functions into base file This makes functions more resuable and is a first step in attempting to consolidate the testing functions used for surveys and the projects that uses molo.surveys --- molo/surveys/tests/base.py | 50 +++++++++++++++++++++++++++++++ molo/surveys/tests/test_models.py | 45 +--------------------------- 2 files changed, 51 insertions(+), 44 deletions(-) create mode 100644 molo/surveys/tests/base.py diff --git a/molo/surveys/tests/base.py b/molo/surveys/tests/base.py new file mode 100644 index 0000000..35f2c9f --- /dev/null +++ b/molo/surveys/tests/base.py @@ -0,0 +1,50 @@ +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, **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 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() From 4a8dbee4219015cbb85b6bf65003a09b04191e6c Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Mon, 19 Mar 2018 13:04:51 +0200 Subject: [PATCH 2/4] Write failing test to show that article-child surveys not in admin view --- molo/surveys/tests/base.py | 5 +++-- molo/surveys/tests/test_admin.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/molo/surveys/tests/base.py b/molo/surveys/tests/base.py index 35f2c9f..8811236 100644 --- a/molo/surveys/tests/base.py +++ b/molo/surveys/tests/base.py @@ -24,9 +24,10 @@ def create_molo_survey_form_field(survey, sort_order, obj): ) -def create_molo_survey_page(parent, **kwargs): +def create_molo_survey_page( + parent, title="Test Survey", slug='test-survey', **kwargs): molo_survey_page = MoloSurveyPage( - title='Test Survey', slug='test-survey', + title=title, slug=slug, introduction='Introduction to Test Survey ...', thank_you_text='Thank you for taking the Test Survey', submit_text='survey submission text', 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) From 8282efd602ea0ecf99c3d4bef855d649ef0a4746 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Mon, 19 Mar 2018 09:45:26 +0200 Subject: [PATCH 3/4] Override wagtailsurveys index view and remove templatetag to get surveys Using a templatetag and overriding the default template is an anti-pattern, as it means that we are fetching surveys twice when dispalying them. It also meant that we were not fetching all of the surveys within a site - only those that were children of SurveyIndexPage. --- .travis.yml | 2 -- molo/surveys/admin_urls.py | 8 +++++++ .../templates/wagtailsurveys/list_forms.html | 24 ------------------- .../wagtailsurveys/list_forms.html | 24 ------------------- molo/surveys/views.py | 17 +++++++++++++ molo/surveys/wagtail_hooks.py | 11 +++++++++ 6 files changed, 36 insertions(+), 50 deletions(-) create mode 100644 molo/surveys/admin_urls.py delete mode 100644 molo/surveys/templates/wagtailsurveys/list_forms.html delete mode 100644 molo/surveys/test_templates/wagtailsurveys/list_forms.html 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/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/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/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/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)), + ] From 89d7a4ad270d96419f23a78e1f2595f97a92c5e7 Mon Sep 17 00:00:00 2001 From: nathanbegbie Date: Mon, 19 Mar 2018 17:11:13 +0200 Subject: [PATCH 4/4] Remove unused templatetag function get_survey_list_for_site --- molo/surveys/templatetags/molo_survey_tags.py | 12 ------------ 1 file changed, 12 deletions(-) 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