Skip to content

Commit

Permalink
Only state variant if content navigation test applies
Browse files Browse the repository at this point in the history
The helper methods for checking the current variant (eg
`universal_navigation_without_nav?`) were not checking whether the AB
test applied or not.

If the CDN set the response header and this method was used on a page
that doesn’t qualify for the test, then it would give a false positive.

* Add test to confirm expected behaviour
* Update helpers to check AB test applies before checking variant
  • Loading branch information
fofr committed Nov 9, 2017
1 parent d23b8d8 commit 32b6634
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
19 changes: 9 additions & 10 deletions app/controllers/concerns/content_navigation_ab_testable.rb
@@ -1,4 +1,5 @@
module ContentNavigationABTestable
CONTENT_NAVIGATION_TEST_NAME = "ContentNavigation".freeze
CONTENT_NAVIGATION_DIMENSION = 67
CONTENT_NAVIGATION_ORIGINAL = "Original".freeze
CONTENT_NAVIGATION_UNIVERSAL_NO_NAVIGATION = "UniversalNoNav".freeze
Expand All @@ -23,7 +24,6 @@ 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?
Expand All @@ -35,7 +35,7 @@ def self.included(base)
def content_navigation_ab_test
@content_navigation_ab_test ||=
GovukAbTesting::AbTest.new(
"ContentNavigation",
CONTENT_NAVIGATION_TEST_NAME,
dimension: CONTENT_NAVIGATION_DIMENSION,
allowed_variants: CONTENT_NAVIGATION_ALLOWED_VARIANTS,
control_variant: CONTENT_NAVIGATION_ORIGINAL
Expand All @@ -49,7 +49,7 @@ def content_navigation_ab_test
# * in the multivariate test
# * not showing the original variant
def should_present_universal_navigation?
content_navigation_ab_test_applies? && !content_navigation_test_original_variant?
content_navigation_ab_test_applies? && !content_navigation_variant.variant?(CONTENT_NAVIGATION_ORIGINAL)
end

def content_navigation_ab_test_applies?
Expand All @@ -60,20 +60,19 @@ 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)
content_navigation_ab_test_applies? &&
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)
content_navigation_ab_test_applies? &&
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)
content_navigation_ab_test_applies? &&
content_navigation_variant.variant?(CONTENT_NAVIGATION_UNIVERSAL_MAINSTREAM_NAVIGATION)
end

def content_navigation_variant
Expand Down
Expand Up @@ -67,5 +67,25 @@ class ContentItemsControllerTest < ActionController::TestCase
assert_equal [], @request.variant
assert_equal ContentItemsController::CONTENT_NAVIGATION_ORIGINAL, requested_variant_name
end

test "#{document_type} does not mark itself as in variant when ab test does not apply" 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)

ContentItemsController::CONTENT_NAVIGATION_ALLOWED_VARIANTS.each do |variant|
request.headers["GOVUK-ABTest-#{ContentItemsController::CONTENT_NAVIGATION_TEST_NAME}"] = variant
get :show, params: { path: path }
assert_response 200
refute @controller.content_navigation_ab_test_applies?

refute @controller.universal_navigation_without_nav?
refute @controller.universal_navigation_with_taxon_nav?
refute @controller.universal_navigation_with_mainstream_nav?
end
end
end
end

0 comments on commit 32b6634

Please sign in to comment.