Skip to content

Add phone question a_b variable#9277

Merged
charleyf merged 7 commits intomainfrom
charley/lg-11090-add-new-a-b-variable-for-phone-question-test
Oct 2, 2023
Merged

Add phone question a_b variable#9277
charleyf merged 7 commits intomainfrom
charley/lg-11090-add-new-a-b-variable-for-phone-question-test

Conversation

@charleyf
Copy link
Contributor

@charleyf charleyf commented Sep 28, 2023

🎫 Ticket

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

🛠 Summary of changes

This work is intentionally small: Adding a new a_b testing variable, but not to using it yet.

When Sonia did this for team Ada (link) she included the variable addition in the PR that used it.

📜 Testing Plan

An engineer could make sure that the variable shows up by using the rails console? I'm not sure if that's necessary?

idv_acuant_sdk_upgrade_a_b_testing_enabled: false
idv_acuant_sdk_upgrade_a_b_testing_percent: 50
idv_getting_started_a_b_testing: '{"welcome_default":100, "welcome_new":0, "getting_started":0}'
idv_phone_question_a_b_testing: '{"hide_phone_question":100, "show_phone_question":0}'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am probably doing something wrong, but when I try running:

IdentityConfig.store.idv_phone_question_a_b_testing in the Rails console, I get NoMethodError: undefined method 'idv_phone_question_a_b_testing'

When I try other values from this file and from the PR linked from this one, I see a return value.

I tried make setup && make run on this branch first to make sure the changes were applied.

I'm probably doing something wrong, but I wanted to note it here since it was the testing plan and I'd be happy to learn how to make it work locally for me.

Copy link
Contributor

@aduth aduth Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be added to the list of configuration values in the IdentityConfig store for it to be accessible in code that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you both, added the new variable there (commit).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth when you say that change is needed to be accessible in code that way - is there a case where you wouldn't need to add it in both places?

@charleyf Thank you for making that change! I tested it again with the changes and saw the expected value in the console.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth when you say that change is needed to be accessible in code that way - is there a case where you wouldn't need to add it in both places?

Typically it should always be in both places. Since this ticket was already very limited in usefulness on its own, I wasn't sure just how far it was meant to be completed to be considered "functional".

@charleyf charleyf changed the title Add draft of phone question a_b variable Add phone question a_b variable Oct 2, 2023
Copy link
Contributor

@night-jellyfish night-jellyfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in the Rails console and saw the expected value.

@charleyf charleyf merged commit d83dcae into main Oct 2, 2023
@charleyf charleyf deleted the charley/lg-11090-add-new-a-b-variable-for-phone-question-test branch October 2, 2023 17:54
@jmhooper jmhooper mentioned this pull request Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants