From 4efe1672f63e94d271fa6cce0627eefcb77f72b2 Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Tue, 7 Nov 2017 17:43:15 +0000 Subject: [PATCH] Add content navigation multivariate test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start with 4 variants: original, no nav, taxonomy nav, mainstream nav * Follow “concern” pattern setup in #513 for task list AB test * Only content with the document types listed are part of the test * The content must also be tagged to a taxon * Use a “universal_navigation” variant for all multivariate tests except original – they are largely the same, significant differences with original shouldn’t be repeated --- .../content_navigation_ab_testable.rb | 86 +++++++++++++++++++ app/controllers/content_items_controller.rb | 19 ++-- app/presenters/content_item_presenter.rb | 4 + app/views/layouts/application.html.erb | 1 + ...b_testing_content_items_controller_test.rb | 69 +++++++++++++++ 5 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 app/controllers/concerns/content_navigation_ab_testable.rb create mode 100644 test/controllers/content_navigation_ab_testing_content_items_controller_test.rb diff --git a/app/controllers/concerns/content_navigation_ab_testable.rb b/app/controllers/concerns/content_navigation_ab_testable.rb new file mode 100644 index 0000000000..a2ff3e87b4 --- /dev/null +++ b/app/controllers/concerns/content_navigation_ab_testable.rb @@ -0,0 +1,86 @@ +module ContentNavigationABTestable + CONTENT_NAVIGATION_DIMENSION = 67 + CONTENT_NAVIGATION_ORIGINAL = "Original".freeze + CONTENT_NAVIGATION_UNIVERSAL_NO_NAVIGATION = "UniversalNoNav".freeze + CONTENT_NAVIGATION_UNIVERSAL_MAINSTREAM_NAVIGATION = "UniversalMainstreamNav".freeze + CONTENT_NAVIGATION_UNIVERSAL_TAXON_NAVIGATION = "UniversalTaxonNav".freeze + + CONTENT_NAVIGATION_ALLOWED_VARIANTS = [ + CONTENT_NAVIGATION_ORIGINAL, + CONTENT_NAVIGATION_UNIVERSAL_NO_NAVIGATION, + CONTENT_NAVIGATION_UNIVERSAL_MAINSTREAM_NAVIGATION, + CONTENT_NAVIGATION_UNIVERSAL_TAXON_NAVIGATION + ].freeze + + GUIDANCE_DOCUMENT_TYPES = %w( + answer + guide + detailed_guide + statutory_guidance + ).freeze + + def self.included(base) + base.helper_method( + :content_navigation_variant, + :content_navigation_ab_test_applies?, + :content_navigation_test_original_variant?, + :universal_navigation_without_nav?, + :universal_navigation_with_taxon_nav?, + :universal_navigation_with_mainstream_nav? + ) + + base.after_action :set_content_navigation_response_header + end + + def content_navigation_ab_test + @content_navigation_ab_test ||= + GovukAbTesting::AbTest.new( + "ContentNavigation", + dimension: CONTENT_NAVIGATION_DIMENSION, + allowed_variants: CONTENT_NAVIGATION_ALLOWED_VARIANTS + ) + end + + # Universal navigation: + # https://gov-uk.atlassian.net/wiki/spaces/GFED/pages/186351726/Sprint+1+designs + # + # Universal navigation must only be shown when: + # * in the multivariate test + # * not showing the original variant + def should_present_universal_navigation? + content_navigation_ab_test_applies? && !content_navigation_test_original_variant? + end + + def content_navigation_ab_test_applies? + @content_item.tagged_to_a_taxon? && content_document_type_is_guidance? + end + + def content_document_type_is_guidance? + GUIDANCE_DOCUMENT_TYPES.include? @content_item.document_type + end + + def content_navigation_test_original_variant? + content_navigation_variant.variant?(CONTENT_NAVIGATION_ORIGINAL) + end + + def universal_navigation_without_nav? + content_navigation_variant.variant?(CONTENT_NAVIGATION_UNIVERSAL_NO_NAVIGATION) + end + + def universal_navigation_with_taxon_nav? + content_navigation_variant.variant?(CONTENT_NAVIGATION_UNIVERSAL_TAXON_NAVIGATION) + end + + def universal_navigation_with_mainstream_nav? + content_navigation_variant.variant?(CONTENT_NAVIGATION_UNIVERSAL_MAINSTREAM_NAVIGATION) + end + + def content_navigation_variant + @content_navigation_variant ||= + content_navigation_ab_test.requested_variant(request.headers) + end + + def set_content_navigation_response_header + content_navigation_variant.configure_response(response) if content_navigation_ab_test_applies? + end +end diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index 494ef7d499..a00f6859c9 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -2,6 +2,7 @@ class ContentItemsController < ApplicationController class SpecialRouteReturned < StandardError; end include TasklistABTestable + include ContentNavigationABTestable rescue_from GdsApi::HTTPForbidden, with: :error_403 rescue_from GdsApi::HTTPNotFound, with: :error_notfound @@ -14,8 +15,9 @@ class SpecialRouteReturned < StandardError; end def show load_content_item - set_up_navigation setup_tasklist_ab_testing + setup_content_navigation_ab_testing + set_up_navigation set_expiry set_access_control_allow_origin_header if request.format.atom? set_guide_draft_access_token if @content_item.is_a?(GuidePresenter) @@ -89,8 +91,6 @@ def set_expiry end def set_up_navigation - @navigation = NavigationType.new(@content_item.content_item) - # Setting a variant on a request is a type of Rails Dark Magic that will # use a convention to automagically load an alternative partial, view or # layout. For example, if I set a variant of :taxonomy_navigation and we render @@ -98,8 +98,13 @@ def set_up_navigation # _breadcrumbs.html+taxonomy_navigation.erb instead. If this file doesn't exist, # then it falls back to _breadcrumbs.html.erb. See: # http://edgeguides.rubyonrails.org/4_1_release_notes.html#action-pack-variants - if @navigation.should_present_taxonomy_navigation? - request.variant = :taxonomy_navigation + if should_present_universal_navigation? + request.variant = :universal_navigation + else + @navigation = NavigationType.new(@content_item.content_item) + if @navigation.should_present_taxonomy_navigation? + request.variant = :taxonomy_navigation + end end end @@ -111,6 +116,10 @@ def setup_tasklist_ab_testing set_tasklist_response_header end + def setup_content_navigation_ab_testing + set_content_navigation_response_header + end + def content_item_path path_and_optional_locale = params .values_at(:path, :locale) diff --git a/app/presenters/content_item_presenter.rb b/app/presenters/content_item_presenter.rb index cfa6c49941..809a715ba5 100644 --- a/app/presenters/content_item_presenter.rb +++ b/app/presenters/content_item_presenter.rb @@ -62,6 +62,10 @@ def related_items @nav_helper.related_items end + def tagged_to_a_taxon? + content_item.dig("links", "taxons").present? + end + private def display_date(timestamp, format = "%-d %B %Y") diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 00f6c054c6..26292f011d 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -25,6 +25,7 @@ <% end %> <%= tasklist_variant.analytics_meta_tag.html_safe if tasklist_ab_test_applies? %> + <%= content_navigation_variant.analytics_meta_tag.html_safe if content_navigation_ab_test_applies? %> <%= yield :extra_head_content %> diff --git a/test/controllers/content_navigation_ab_testing_content_items_controller_test.rb b/test/controllers/content_navigation_ab_testing_content_items_controller_test.rb new file mode 100644 index 0000000000..847ae0c98c --- /dev/null +++ b/test/controllers/content_navigation_ab_testing_content_items_controller_test.rb @@ -0,0 +1,69 @@ +require 'test_helper' + +class ContentItemsControllerTest < ActionController::TestCase + include GdsApi::TestHelpers::ContentStore + include GovukAbTesting::MinitestHelpers + + ContentItemsController::GUIDANCE_DOCUMENT_TYPES.each do |document_type| + schema_name = document_type == 'statutory_guidance' ? 'publication' : document_type + + test "does not show universal navigation when no taxons are tagged to #{document_type}" do + content_item = content_store_has_schema_example(schema_name, schema_name) + path = "government/abtest/#{schema_name}" + content_item['base_path'] = "/#{path}" + content_item['links'] = {} + + content_store_has_item(content_item['base_path'], content_item) + + setup_ab_variant('ContentNavigation', ContentItemsController::CONTENT_NAVIGATION_ORIGINAL) + get :show, params: { path: path } + assert_equal [], @request.variant + assert_response_not_modified_for_ab_test('ContentNavigation') + end + + test "#{document_type} honours content navigation AB Testing cookie" do + content_item = content_store_has_schema_example(schema_name, schema_name) + path = "government/abtest/#{schema_name}" + content_item['document_type'] = document_type + content_item['base_path'] = "/#{path}" + content_item['links'] = { + 'taxons' => [ + { + 'title' => 'A Taxon', + 'base_path' => '/a-taxon', + } + ] + } + + content_store_has_item(content_item['base_path'], content_item) + + ContentItemsController::CONTENT_NAVIGATION_ALLOWED_VARIANTS.each do |variant| + with_variant ContentNavigation: variant do + get :show, params: { path: path } + requested = @controller.content_navigation_ab_test.requested_variant(request.headers) + assert_response 200 + assert requested.variant?(variant) + + if variant == ContentItemsController::CONTENT_NAVIGATION_ORIGINAL + assert_equal [:taxonomy_navigation], @request.variant + else + assert_equal [:universal_navigation], @request.variant + end + end + end + end + + test "defaults to original view without AB testing cookie for #{document_type}" do + content_item = content_store_has_schema_example(schema_name, schema_name) + path = "government/abtest/#{schema_name}" + content_item['document_type'] = document_type + content_item['base_path'] = "/#{path}" + content_item['links'] = {} + content_store_has_item(content_item['base_path'], content_item) + + get :show, params: { path: path } + assert_response 200 + assert_equal [], @request.variant + end + end +end