Skip to content

Commit

Permalink
More robust replacement of AB test content
Browse files Browse the repository at this point in the history
As suggested by @hannako, we can make the AB test replacement less
fragile by looking for a specific replacement placeholder in the content
item, instead of replacing a particular bit of content.

I've gone for a [mustache](https://mustache.github.io/) style
replacement string of {{ab_test_find_utr_number_video_links}}. There's a
possible future thought of doing other kinds of templating, so I think
starting to socialise this syntax is a good idea.

I've checked in the govspeak preview app, and the `{{...}}` syntax is ignored
by govspeak so there should be no issue entering it in the publishing
app.

I've pulled the replacement strings out into the translation file so
that they're easier to review / keep up to date.

I've also switched to three A / B / Z variants, so we can do splits like
5% / 5% / 90% where we're measuring the difference between the two 5%
samples and ignoring the 90% sample. This test is probably going to be
50% / 50%, so not strictly necessary, but this seems to be the idiomatic
way to do it.
  • Loading branch information
richardTowers committed Oct 18, 2023
1 parent 588a57a commit 0f30c12
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 20 deletions.
27 changes: 11 additions & 16 deletions app/controllers/content_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,21 @@ def show
ab_test = GovukAbTesting::AbTest.new(
"find_utr_number_video_links",
dimension: 300, # TODO: which dimension should we use?
allowed_variants: %w[HelpText VideoLink],
control_variant: "HelpText",
allowed_variants: %w[A B Z],
control_variant: "Z",
)
@requested_variant = ab_test.requested_variant(request.headers)
@requested_variant.configure_response(response)

if @requested_variant.variant? "VideoLink"
# NOTE: this is a fragile way of doing an AB test on content.
#
# Any change to the base content, or to the way the content is rendered
# could potentially break the B variant of the test, and result in both
# variants being the same.
# We're aware of this risk, and we're going to be careful in this one off
# situation. This is not a sustainable way of doing AB tests in the
# future.
@content_item.body.sub!(
/<li>\s*in\ the\s+<a\ href="[^"]*"><abbr\ title="[^"]+">HMRC<\/abbr>\s+app<\/a>\s*<\/li>/,
'<li>in the <a href="https://www.gov.uk/guidance/download-the-hmrc-app"><abbr title="HM Revenue and Customs">HMRC</abbr> app</a> - watch a <a href="https://www.youtube.com/watch?v=LXw9ily9rTo">video about finding your UTR number in the app</a></li>',
)
end
replacement = case @requested_variant.variant_name
when "A"
I18n.t("ab_tests.find_utr_number_video_links.A")
when "B"
I18n.t("ab_tests.find_utr_number_video_links.B")
else
I18n.t("ab_tests.find_utr_number_video_links.Z")
end
@content_item.body.sub!("{{ab_test_find_utr_number_video_links}}", replacement)
end
# /TEMPORARY

Expand Down
5 changes: 5 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
---
en:
ab_tests:
find_utr_number_video_links:
A: 'in the <a href="/guidance/download-the-hmrc-app"><abbr title="HM Revenue and Customs">HMRC</abbr> app</a>'
B: 'in the <abbr title="HM Revenue and Customs">HMRC</abbr> app - watch a <a href="https://www.youtube.com/watch?v=LXw9ily9rTo">video about finding your UTR number in the app</a>'
Z: 'in the <a href="/guidance/download-the-hmrc-app"><abbr title="HM Revenue and Customs">HMRC</abbr> app</a>'
call_for_evidence:
and: and
another_website_html: This call for evidence %{closed} held on <a href="%{url}">another website</a>
Expand Down
25 changes: 21 additions & 4 deletions test/controllers/content_items_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -364,19 +364,36 @@ class ContentItemsControllerTest < ActionController::TestCase
assert_equal "true", @response.headers[Slimmer::Headers::REMOVE_SEARCH_HEADER]
end

test "AB test replaces content on the find-utr-number page" do
test "AB test replaces content on the find-utr-number page with default" do
content_item = content_store_has_schema_example("answer", "answer")
content_item["base_path"] = "/find-utr-number"
content_item["details"]["body"] = "<li>in the <a href=\"\"><abbr title=\"HM Revenue and Customs\">HMRC</abbr> app</a>\n</li>"
content_item["details"]["body"] = "<li>{{ab_test_find_utr_number_video_links}}</li>"
content_item["locale"] = "en"

stub_content_store_has_item(content_item["base_path"], content_item)

request.headers["HTTP_GOVUK_ABTEST_FIND_UTR_NUMBER_VIDEO_LINKS"] = "VideoLink"
request.headers["HTTP_GOVUK_ABTEST_FIND_UTR_NUMBER_VIDEO_LINKS"] = nil

get :show, params: { path: path_for(content_item) }
assert_response :success
assert_match 'watch a <a href="https://www.youtube.com/watch?v=LXw9ily9rTo">video about finding your UTR number in the app</a>', response.body
assert_no_match "{{ab_test_find_utr_number_video_links}}", response.body
assert_match "<li>#{I18n.t('ab_tests.find_utr_number_video_links.Z')}</li>", response.body
end

test "AB test replaces content on the find-utr-number page with variant B" do
content_item = content_store_has_schema_example("answer", "answer")
content_item["base_path"] = "/find-utr-number"
content_item["details"]["body"] = "<li>{{ab_test_find_utr_number_video_links}}</li>"
content_item["locale"] = "en"

stub_content_store_has_item(content_item["base_path"], content_item)

request.headers["HTTP_GOVUK_ABTEST_FIND_UTR_NUMBER_VIDEO_LINKS"] = "B"

get :show, params: { path: path_for(content_item) }
assert_response :success
assert_no_match "{{ab_test_find_utr_number_video_links}}", response.body
assert_match "<li>#{I18n.t('ab_tests.find_utr_number_video_links.B')}</li>", response.body
end

def path_for(content_item, locale = nil)
Expand Down

0 comments on commit 0f30c12

Please sign in to comment.