Switching newer Acuant SDK to be the default version by eric-gade · Pull Request #7513 · 18F/identity-idp

eric-gade · 2022-12-19T21:12:41Z

Follow up to #7378 and #7392

The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default".

This change updates the doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off.

🛠 Summary of changes

We have added checking in the step method that provides variables to the frontend.

-- What The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default". This change updates the step doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off. changelog: Internal, Upgrades, Acuant SDK Upgrade

aduth · 2022-12-20T15:14:30Z

app/services/idv/steps/document_capture_step.rb

        bucket = AbTests::ACUANT_SDK.bucket(flow_session[:document_capture_session_uuid])
-        acuant_version = (bucket == :use_newer_sdk) ? '11.7.1' : '11.7.0'
+        testing_enabled = IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled
+        use_newer_sdk = (bucket == :use_newer_sdk)


If testing is disabled, this will be false despite the fact we're using the newer of the two versions. Is that expected?

What if instead of 'newer', we set default and alternate versions, and change the A/B test bucket to use_alt_sdk? Then it would be more explicit what the current default is meant to be, and we could test against an older version if we later discover what we suspect is a regression.

idv_acuant_sdk_version_default: <default_version> idv_acuant_sdk_version_alternate: <version_to_test>

Yeah I agree with @solipet -- "use newer" makes the logic more confusing. Having the two versions specified in the config also gives us fewer string replacements etc when changing Acuant versions for real. I'll whip something up

aduth · 2022-12-20T15:15:15Z

app/services/idv/steps/document_capture_step.rb

@@ -43,11 +43,13 @@ def native_camera_ab_testing_variables

      def acuant_sdk_upgrade_a_b_testing_variables
        bucket = AbTests::ACUANT_SDK.bucket(flow_session[:document_capture_session_uuid])


I wonder if we could simplify this by changing the logic of the bucket initialization to default to 100% if testing is disabled?

identity-idp/config/initializers/ab_tests.rb

Lines 16 to 18 in 28ac3ed

use_newer_sdk: IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled ?

IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_percent :

0,

aduth · 2022-12-20T15:17:08Z

app/services/idv/steps/document_capture_step.rb

          acuant_sdk_upgrade_a_b_testing_enabled:
-              IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled,
-          use_newer_sdk: (bucket == :use_newer_sdk),
+              testing_enabled,


Minor: We don't need to break this onto a separate line anymore.

Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade

changelog: Internal, Upgrades, Acuant SDK Upgrade

[skip changelog]

This reverts commit b420998. [skip changelog]

aduth · 2022-12-21T13:07:34Z

app/services/idv/steps/document_capture_step.rb

-        acuant_version = (bucket == :use_newer_sdk) ? '11.7.1' : '11.7.0'
+        testing_enabled = IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled
+        use_alternate_sdk = (bucket == :use_alternate_sdk)
+        if testing_enabled && use_alternate_sdk


Is testing_enabled && redundant here with an assumption that use_alternative_sdk could only be true if it were enabled?

[skip changelog]

@aduth

* Switching newer Acuant SDK to be the default version -- What The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default". This change updates the step doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off. changelog: Internal, Upgrades, Acuant SDK Upgrade * Specifying acuant versions in configs and updating names Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade * Acuant context should use config's acuant version by default changelog: Internal, Upgrades, Acuant SDK Upgrade * Fixing lints [skip changelog] * Revert "Acuant context should use config's acuant version by default" This reverts commit b420998. [skip changelog] * Removing redundant condition (via @aduth) [skip changelog]

@aduth

…7524) * changelog: Internal, Authentication, Remove show_account_recovery_recovery_options feature flag since feature is permanently turned on * LG-7489: rspec fixes * remove unneeded spec * LG-8089: Update to support notifying user of OTP format in message (#7486) * Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect (LG-8089) * update voice * translation update * add check to otp_sender * changelog: Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect * fix spec * Revert "LG-8432: SAML - Update ForceAuthn check (#7514)" (#7520) This reverts commit a7577b6. [skip changelog] * LG-8388: Confirm ThreatMetrix doesn't block IPP (#7521) * LG-8388: Confirm ThreatMetrix doesn't block IPP Spec to confirm that IPP is not blocked by a ThreatMetrix result of 'Reject' [skip changelog] * ensure the threatmetrix profiling is enabled * lints * Update spec/features/idv/in_person_spec.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Switching newer Acuant SDK to be the default version (#7513) * Switching newer Acuant SDK to be the default version -- What The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default". This change updates the step doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off. changelog: Internal, Upgrades, Acuant SDK Upgrade * Specifying acuant versions in configs and updating names Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade * Acuant context should use config's acuant version by default changelog: Internal, Upgrades, Acuant SDK Upgrade * Fixing lints [skip changelog] * Revert "Acuant context should use config's acuant version by default" This reverts commit b420998. [skip changelog] * Removing redundant condition (via @aduth) [skip changelog] * Add logging criteria, abstract logging decision into method (#7516) * Abstract logging decision into method * [skip changelog] * lintfix * Test logic in should_log? method * lintfix * Add filename fingerprint to JavaScript locale data script (#7527) * Add filename fingerprint to JavaScript locale data script changelog: Internal, Build Tooling, Invalidate cache for content-only revisions to JavaScript bundles * Update specs * LG-8438: Update language on pages related to OTP (#7515) Update OTP language changelog: Improvements, Multi-Factor Authentication, Update OTP language * Doc auth retry timer should decrease (#7498) * Move timeout calculation to helper * Fix calculation of attempted_at and expires_at * changelog: Bug Fixes, DocAuth, Modify retry timer to decrease * Remove instance variable from helper * Revert helper * Indent Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * build-sass: Add CLI flag for additional load paths (#7525) changelog: Internal, Build Tooling, Improve Sass compilation tool to support additional load paths Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>

@aduth

* build-sass: Add CLI flag for additional load paths (#7525) changelog: Internal, Build Tooling, Improve Sass compilation tool to support additional load paths * Updating ThreatMetrix failure (sad face) screen content (#7529) * Updating ThreatMetrix failure (sad face) screen content changelog: User Facing Improvements, UX, ThreatMetrix failure screen content * Lints [skipe changelog] * Adding contact center number to config and setting dummy default [skip changelog] * Ignore non-yaml files when normalizing yamls (#7531) [skip changelog] * Lg-8264 po search frontend updates (#7519) * show updated ui for location page when arcgis is enabled * add mile(s) away to distance * changelog: Improvements, Location page, update ui on location pg when arcgis is enabled * normalize yaml * fix missing translation test failure * update strings to include count to handle pluralization * update collection item test * add es translation for tty to untranslated keys * refactor * Fix flakey in-person feature test (#7533) * make it okay for the interstitial to not show it's possible for the interstitial to disappear before the matcher can catch it. changelog: Internal, Automated Testing, Improve reliability of successful automated tests * Make `lintfix` fix erb, javascript, and CSS files too (#7528) * Make lintfix fix erb files too * Autofix javascript and CSS files * [skip changelog] * Mention that we're fixing things in status updates * Fix untranslated email input in password reset (#7537) * Fix untranslated email input in password reset changelog: Bug Fixes, Internationalization, Fix untranslated email input in password reset * fix specs * Update app/views/devise/passwords/new.html.erb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Move favicons to asset pipeline (#7532) * Move favicons to asset pipeline changelog: Internal, Assets, Move favicons to asset pipeline * use existing design system assets * LG-7489: Remove show_account_recovery_recovery_options Feature flag (#7524) * changelog: Internal, Authentication, Remove show_account_recovery_recovery_options feature flag since feature is permanently turned on * LG-7489: rspec fixes * remove unneeded spec * LG-8089: Update to support notifying user of OTP format in message (#7486) * Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect (LG-8089) * update voice * translation update * add check to otp_sender * changelog: Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect * fix spec * Revert "LG-8432: SAML - Update ForceAuthn check (#7514)" (#7520) This reverts commit a7577b6. [skip changelog] * LG-8388: Confirm ThreatMetrix doesn't block IPP (#7521) * LG-8388: Confirm ThreatMetrix doesn't block IPP Spec to confirm that IPP is not blocked by a ThreatMetrix result of 'Reject' [skip changelog] * ensure the threatmetrix profiling is enabled * lints * Update spec/features/idv/in_person_spec.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Switching newer Acuant SDK to be the default version (#7513) * Switching newer Acuant SDK to be the default version -- What The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default". This change updates the step doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off. changelog: Internal, Upgrades, Acuant SDK Upgrade * Specifying acuant versions in configs and updating names Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade * Acuant context should use config's acuant version by default changelog: Internal, Upgrades, Acuant SDK Upgrade * Fixing lints [skip changelog] * Revert "Acuant context should use config's acuant version by default" This reverts commit b420998. [skip changelog] * Removing redundant condition (via @aduth) [skip changelog] * Add logging criteria, abstract logging decision into method (#7516) * Abstract logging decision into method * [skip changelog] * lintfix * Test logic in should_log? method * lintfix * Add filename fingerprint to JavaScript locale data script (#7527) * Add filename fingerprint to JavaScript locale data script changelog: Internal, Build Tooling, Invalidate cache for content-only revisions to JavaScript bundles * Update specs * LG-8438: Update language on pages related to OTP (#7515) Update OTP language changelog: Improvements, Multi-Factor Authentication, Update OTP language * Doc auth retry timer should decrease (#7498) * Move timeout calculation to helper * Fix calculation of attempted_at and expires_at * changelog: Bug Fixes, DocAuth, Modify retry timer to decrease * Remove instance variable from helper * Revert helper * Indent Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * build-sass: Add CLI flag for additional load paths (#7525) changelog: Internal, Build Tooling, Improve Sass compilation tool to support additional load paths Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * LG-8414 Remove flag from batch job (#7534) * LG-8414 Remove flag from batch job changelog: Internal, Attempts API, Events Batch work * LG-8390 Write rake tasks to pass or reject users (#7536) * initial steps for rake script to review profile * rewrite tasks for passing and rejecting * add tests for review_profile rake task * add changelog changelog: Internal, IdV, Make script to pass or reject TMX review * activate profile upon passing review also changed the deactivation reason upon rejection to :threatmetrix_review_rejected * check for deactivation reason in passing Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov> Co-authored-by: Shannon A <20867088+svalexander@users.noreply.github.com> Co-authored-by: Tomas Apodaca <thomas.apodaca@gsa.gov> Co-authored-by: Malick Diarra <malick.diarra@gsa.gov> Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: Luis H. Matos <ThatSpaceGuy@users.noreply.github.com> Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov>

eric-gade requested a review from a team December 19, 2022 21:12

aduth approved these changes Dec 20, 2022

View reviewed changes

eric-gade added 4 commits December 20, 2022 14:40

Specifying acuant versions in configs and updating names

443179c

Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade

Acuant context should use config's acuant version by default

b420998

changelog: Internal, Upgrades, Acuant SDK Upgrade

Fixing lints

f5e94e7

[skip changelog]

Revert "Acuant context should use config's acuant version by default"

83550cb

This reverts commit b420998. [skip changelog]

aduth approved these changes Dec 21, 2022

View reviewed changes

Removing redundant condition (via @aduth)

a6a433b

[skip changelog]

eric-gade merged commit b5d7f6b into main Dec 21, 2022

eric-gade deleted the eric-acuant-ab-followup branch December 21, 2022 17:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switching newer Acuant SDK to be the default version#7513

Switching newer Acuant SDK to be the default version#7513
eric-gade merged 6 commits intomainfrom
eric-acuant-ab-followup

eric-gade commented Dec 19, 2022

Uh oh!

aduth Dec 20, 2022

Uh oh!

solipet Dec 20, 2022

Uh oh!

eric-gade Dec 20, 2022

Uh oh!

aduth Dec 20, 2022

Uh oh!

aduth Dec 20, 2022

Uh oh!

aduth Dec 21, 2022

Uh oh!

eric-gade Dec 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -43,11 +43,13 @@ def native_camera_ab_testing_variables

		def acuant_sdk_upgrade_a_b_testing_variables
		bucket = AbTests::ACUANT_SDK.bucket(flow_session[:document_capture_session_uuid])

	use_newer_sdk: IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled ?
	IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_percent :
	0,

Conversation

eric-gade commented Dec 19, 2022

🛠 Summary of changes

Uh oh!

aduth Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

solipet Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

eric-gade Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

aduth Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

aduth Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

aduth Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

eric-gade Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants