Prepare for Popular Tasks AB test #3740
Conversation
unoduetre
force-pushed
the
2783-set-up-ab-test-logic-in-collections-m-l
branch
from
c692d93
to
6a4e0b3
Compare
unoduetre
force-pushed
the
2783-set-up-ab-test-logic-in-collections-m-l
branch
from
6a4e0b3
to
7912eeb
Compare
unoduetre
force-pushed
the
2783-set-up-ab-test-logic-in-collections-m-l
branch
from
7912eeb
to
ab23894
Compare
unoduetre
force-pushed
the
2783-set-up-ab-test-logic-in-collections-m-l
branch
from
ab23894
to
c8015fe
Compare
unoduetre
force-pushed
the
2783-set-up-ab-test-logic-in-collections-m-l
branch
from
c8015fe
to
f732073
Compare
There was a problem hiding this comment.
Thanks for your work on this Mat
I've requested quite a few changes, as we need the popular tasks to be displayed for variants A and B on all 16 browse pages not just benefits and business. Sorry that wasn't made clearer to you.
I've made suggested changes which hopefully make sense and will help you get it ready for re-review - happy to pair on anything that's unclear.
| end | ||
|
|
||
| def popular_tasks_page_under_test? | ||
| request.path.starts_with?(%r{/browse/business|/browse/benefits}) |
There was a problem hiding this comment.
We're AB testing across all 16 mainstream browse pages. I didn't make this decision clear enough on the trello tickets, so I can see why you've assumed otherwise.
So you'll need some regex here. This kind of thing?
def browse_page_under_test?
/\/browse\/[a-z,-]*$/.match?(request.path)
end
app/helpers/browse_helper.rb
Outdated
| @@ -3,8 +3,44 @@ def display_popular_links_for_slug?(slug) | |||
| I18n.exists?(slug.to_s, scope: "browse.popular_links") | |||
| end | |||
|
|
|||
| def variant_a_popular_links(slug) | |||
| I18n.t(slug.to_s, scope: "browse.popular_links") | |||
There was a problem hiding this comment.
Is this meant to say I18n.t(slug.to_s, scope: "browse.popular_links_a") etc. Otherwise all these methods are fetching from the same key
There was a problem hiding this comment.
No, that's intentional. You mentioned that the work should be done in two stages, so when the actual tests are to be set up, another PR should simply switch those to e.g. popular_links_a etc. I can do it and now and copy the text instead, if that's your preference.
app/helpers/browse_helper.rb
Outdated
| end | ||
|
|
||
| def variant_popular_links_for_slug(slug) | ||
| default_variant_popular_links = variant_c_popular_links(slug) |
There was a problem hiding this comment.
This will work for the pages that already have popular tasks (business and benefits) but the c variant for the other 14 is no popular tasks section.
The method on line 2 (display_popular_links_for_slug?(slug)) can be rewritten to check if there's any control data in the locale file. If there's no data, the method returns false and hides the section.
config/locales/en/browse.yml
Outdated
| @@ -17,3 +17,47 @@ en: | |||
| url: /self-assessment-tax-returns | |||
| - title: "Pay employers' PAYE" | |||
| url: /pay-paye-tax | |||
| popular_links_a: | |||
There was a problem hiding this comment.
I think the way you've structured the locale file will make it hard to review the next PR to add the popular task links. We'll have to scroll down to find a slug in three different sections to check the popular tasks for each variant. As we have 16 pages, that's not an insignificant burden!
I think it might be easier if you could add a variant_a, variant_b within each existing slug instead. For business and benefits, there would also need to be a control key.
app/helpers/browse_helper.rb
Outdated
| def variant_popular_links_for_slug(slug) | ||
| default_variant_popular_links = variant_c_popular_links(slug) | ||
|
|
||
| if no_popular_tasks_ab_test? |
There was a problem hiding this comment.
I think we need
if popular_tasks_variant_a_page?
# show variant a links
elsif popular_tasks_variant_b_page?
# show variant b links
else (variant z or variant c)
# show original (links||nothing)
end
The links||nothing would come from default_variant_popular_links
unoduetre
force-pushed
the
2783-set-up-ab-test-logic-in-collections-m-l
branch
from
f732073
to
8a9d302
Compare
Follow these steps if you are doing a Rails upgrade.
What
Add AB testing logic to collection in preparation for the popular task AB test.
Allowed variants: A,B,C,Z
Control: C
Test name: PopularTasks alphagov/govuk-fastly#93
We should be able to merge this code now without users seeing any changes, ie before we have the list of links for each variants.
Why
So that all technical work is complete and ready to go.
Trello card