LG-10922: Display new headings for Hybrid Handoff page on AB test

[night-jellyfish]

🎫 Ticket

https://cm-jira.usa.gov/browse/LG-10922

🛠 Summary of changes

This adds a change of heading text to the Hybrid Handoff page, if idv_phone_question_a_b_testing is set to 1. If it's set to 0, (the default), the old copy should remain.

📜 Testing Plan

• First, check that the old copy still remains:
  • Follow the flow to the /verify/hybrid_handoff page
  • Check that the h1 has the hybrid_handoff text ("How would you like to add your ID?" in English)
  • Check that the h2 has the upload_from_phone text ("Use your phone to take photos" in English)
• Second, check that the new copy is there:
  • Set idv_phone_question_a_b_testing: '{"bypass_phone_question":0, "show_phone_question":100}' in your application.yml
  • Follow the flow to the /verify/hybrid_handoff page
  • Check that the h1 has the upload_from_phone text ("Use your phone to take photos" in English)
  • Check that the h2 has the upload_from_phone text ("Switch to your phone" in English)

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before / with `idv_phone_question_a_b_testing: '{"bypass_phone_question":100, "show_phone_question":0}'`:

After / with `idv_phone_question_a_b_testing: '{"bypass_phone_question":0, "show_phone_question":100}'`

[kellular]

This LGTM! I just wanted to confirm that we have the French and Spanish translations in place for the 1 version: "Use your phone to take photos"? I only saw the "Switch to your phone" translations in the .yml files

[night-jellyfish]

@kellular good question (and it made me realize I had a couple of typos in my PR message (fixed now))!

We have Spanish here and French here. Luckily since "Use your phone to take photos" was already used as an h2 in our control / current text, the translations were already there, and we're just bumping it to an h1 for this change.

[amirbey]

looks great @night-jellyfish 👍🏿 ... lastly, can we add a few a/b content checks to feature tests?

[night-jellyfish]

@amirbey I added some feature tests in 7f4e0f4, but please let me know if you think more are needed!

[amirbey]

Thanks for adding these checks @night-jellyfish ... is it possible these checks could be inserted into one of the already existing tests to reduce the time it takes for our long running batch of specs?

[night-jellyfish]

That's a good point. I struggled with that myself but I wasn't sure how to incorporate them into another spec and keep the reasoning for testing clear. I sometimes think about tests like documentation, and I liked that the context and it blocks let me communicate exactly what I'm testing / what I expect the code to do. It seems like it might help towards your other point of making the test easy to delete, as it's encapsulated.

But, I understand your point about test speed as well, as slow tests can cause problems and pain as well.

Have you found a way to merge the gap between those two pulls (clarity vs speed)?

My thoughts were, since there aren't other tests focused on copy, if I added it to a test, the it block would become a little strange, ex: it 'rate limits sending the link and displays the expected headings', which aren't really related. But, maybe I'm overthinking it.

[amirbey]

i also contemplate the same 🤔 .... i've often left the it/context heading unchanged but commented within ... it can be a tough call ... in this case this is more verifying the presence of the text and not so much a result of actions which makes me a bit more comfortable with combining it in a test that is really trying to test a scenario

[night-jellyfish]

Thank you for the feedback! As discussed over Slack, I agree that since it's just a text change, it's fine to move into the context of other test to reduce impact on test time. I've pushed up this change in 3f7b868.

Since you approved, I'll merge if CI passes - but please let me know if you have any hesitancy on that!

[amirbey]

LGTM ... thanks @night-jellyfish 👍🏿