From f3ba54e337d8466a755ea5f524d783fe2573ee51 Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Tue, 10 Aug 2021 11:24:37 +0100 Subject: [PATCH] Return response headers for Explore AB test At present we're only returning Vary response headers when we get a B variant. We want to do this all the time or it's confusing for Fastly to know how to cache. I've added a test that ensures (using the `with_variant` helper) that the header is being set correctly, and also checks for analytics set up. It's trickier to assert things about the template because slimmer's testing is harder to figure out right now, but it'd be useful to add these at some point too. :-) --- .../ab_tests/explore_menu_ab_testable.rb | 4 +- app/controllers/content_items_controller.rb | 4 +- app/views/layouts/application.html.erb | 2 +- test/controllers/explore_ab_test.rb | 49 +++++++++++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 test/controllers/explore_ab_test.rb diff --git a/app/controllers/ab_tests/explore_menu_ab_testable.rb b/app/controllers/ab_tests/explore_menu_ab_testable.rb index f774acfcf..a05b3fea8 100644 --- a/app/controllers/ab_tests/explore_menu_ab_testable.rb +++ b/app/controllers/ab_tests/explore_menu_ab_testable.rb @@ -17,10 +17,10 @@ def explore_menu_variant end def set_explore_menu_response - explore_menu_variant.configure_response(response) if explore_menu_testable? + explore_menu_variant.configure_response(response) end - def explore_menu_testable? + def explore_menu_variant_b? explore_menu_variant.variant?("B") end end diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index f51d98aec..53af1ebc3 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -17,7 +17,7 @@ class ContentItemsController < ApplicationController before_action :set_explore_menu_response after_action :set_slimmer_template - helper_method :explore_menu_variant, :explore_menu_testable? + helper_method :explore_menu_variant, :explore_menu_variant_b? attr_accessor :content_item, :taxonomy_navigation @@ -57,7 +57,7 @@ def service_sign_in_options private def set_slimmer_template - if explore_menu_testable? + if explore_menu_variant_b? slimmer_template "core_layout_explore_header" else slimmer_template "core_layout" diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 4090da5fc..523b84ff9 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -33,7 +33,7 @@ <%= yield :extra_head_content %> - <% unless content_for(:simple_header) || explore_menu_testable? %> + <% unless content_for(:simple_header) || explore_menu_variant_b? %> <%= render 'govuk_publishing_components/components/government_navigation', active: active_proposition %> <% end %> diff --git a/test/controllers/explore_ab_test.rb b/test/controllers/explore_ab_test.rb new file mode 100644 index 000000000..8d3ef691d --- /dev/null +++ b/test/controllers/explore_ab_test.rb @@ -0,0 +1,49 @@ +require "test_helper" + +class ContentItemsControllerTest < ActionController::TestCase + include GovukAbTesting::MinitestHelpers + + test "shows new header for variant B" do + for_each_schema do |schema| + with_variant ExploreMenuAbTestable: "B" do + set_up_and_visit_content_item_for_schema(schema) + + assert_page_tracked_in_ab_test("ExploreMenuAbTestable", "B", 47) + end + end + end + + test "doesn't show new header for variant A" do + for_each_schema do |schema| + with_variant ExploreMenuAbTestable: "A" do + set_up_and_visit_content_item_for_schema(schema) + + assert_page_tracked_in_ab_test("ExploreMenuAbTestable", "A", 47) + end + end + end + + test "doesn't show new header for variant Z" do + for_each_schema do |schema| + with_variant ExploreMenuAbTestable: "Z" do + set_up_and_visit_content_item_for_schema(schema) + + assert_page_tracked_in_ab_test("ExploreMenuAbTestable", "Z", 47) + end + end + end + +private + + def set_up_and_visit_content_item_for_schema(schema) + content_item = content_store_has_schema_example(schema, schema) + stub_content_store_has_item(content_item["base_path"], content_item) + path = content_item["base_path"][1..] + + get :show, params: { path: path } + end + + def for_each_schema(&block) + %w[guide answer document_collection].each(&block) + end +end