Skip to content

Rename doc_auth_selfie_capture config to doc_auth_selfie_capture_enabled#9782

Merged
jmhooper merged 2 commits intomainfrom
jmhooper-rename-config
Dec 18, 2023
Merged

Rename doc_auth_selfie_capture config to doc_auth_selfie_capture_enabled#9782
jmhooper merged 2 commits intomainfrom
jmhooper-rename-config

Conversation

@jmhooper
Copy link
Contributor

The doc_auth_selfie_capture is a JSON hash that was intended to manage multiple configs for document selfie capture. Prior to this commit it contains a single value: enabled.

Using an unstructured hash this way means we do not get many of the benefits of using IdentityConfig e.g. type validation for configs and warnings/errors when configs are missing.

This commit moves the enabled config from doc_auth_selfie_capture to doc_auth_selfie_capture_enabled and deletes the doc_auth_selfie_capture config. Future configs for selfie capture can use the doc_auth_selfie_capture prefix.

…nabled`

The `doc_auth_selfie_capture` is a JSON hash that was intended to manage multiple configs for document selfie capture. Prior to this commit it contains a single value: `enabled`.

Using an unstructured hash this way means we do not get many of the benefits of using IdentityConfig e.g. type valudation for configs and warnings/errors when configs are missing.

This commit moves the enabled config from `doc_auth_selfie_capture` to `doc_auth_selfie_capture_enabled` and deletes the `doc_auth_selfie_capture` config. Future configs for selfie capture can use the `doc_auth_selfie_capture` prefix.

[skip changelog]
@jmhooper jmhooper requested review from a team and eileen-nava and removed request for a team December 18, 2023 14:45
@jmhooper
Copy link
Contributor Author

Before this gets merged I am going to look at the application.yml files in deployed envs to make sure that doc_auth_selfie_capture_enabled is true anywhere that doc_auth_selfie_capture.enabled is currently true.

Comment on lines +39 to +41
doc_auth_selfie_capture: {
enabled: IdentityConfig.store.doc_auth_selfie_capture_enabled,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd also be nice to update this in the JavaScript code as well. Currently we bypass type-checking on this data attribute and perform a JSON.parse without error checking. I feel like we could get rid of that function altogether if we passed through the individual value.

diff --git a/app/javascript/packs/document-capture.tsx b/app/javascript/packs/document-capture.tsx
index 1a27313ef..d5df599ec 100644
--- a/app/javascript/packs/document-capture.tsx
+++ b/app/javascript/packs/document-capture.tsx
@@ -31,2 +31,3 @@ interface AppRootData {
   acuantVersion: string;
+  docAuthSelfieCaptureEnabled: 'true' | 'false';
   flowPath: FlowPath;
@@ -54,8 +55,2 @@ function getServiceProvider() {
 
-function getSelfieCaptureEnabled() {
-  const { docAuthSelfieCapture } = appRoot.dataset;
-  const docAuthSelfieCaptureObject = docAuthSelfieCapture ? JSON.parse(docAuthSelfieCapture) : {};
-  return !!docAuthSelfieCaptureObject?.enabled;
-}
-
 function getMetaContent(name): string | null {
@@ -95,2 +90,3 @@ const {
   acuantVersion,
+  docAuthSelfieCaptureEnabled,
   flowPath,
@@ -112,2 +108,4 @@ const {
 
+const selfieCaptureEnabled = docAuthSelfieCaptureEnabled === 'true';
+
 let parsedUsStatesTerritories = [];
@@ -144,3 +142,3 @@ const App = composeComponents(
       passiveLivenessOpenCVSrc: acuantVersion && `/acuant/${acuantVersion}/opencv.min.js`,
-      passiveLivenessSrc: getSelfieCaptureEnabled()
+      passiveLivenessSrc: selfieCaptureEnabled
         ? acuantVersion && `/acuant/${acuantVersion}/AcuantPassiveLiveness.min.js`
@@ -194,3 +192,3 @@ const App = composeComponents(
         exitQuestionSectionEnabled: String(uiExitQuestionSectionEnabled) === 'true',
-        selfieCaptureEnabled: getSelfieCaptureEnabled(),
+        selfieCaptureEnabled,
       },
diff --git a/app/views/idv/shared/_document_capture.html.erb b/app/views/idv/shared/_document_capture.html.erb
index d82035650..a33b0a70c 100644
--- a/app/views/idv/shared/_document_capture.html.erb
+++ b/app/views/idv/shared/_document_capture.html.erb
@@ -38,5 +38,3 @@
       us_states_territories: us_states_territories,
-      doc_auth_selfie_capture: {
-        enabled: IdentityConfig.store.doc_auth_selfie_capture_enabled,
-      },
+      doc_auth_selfie_capture_enabled: IdentityConfig.store.doc_auth_selfie_capture_enabled,
       skip_doc_auth: skip_doc_auth,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll tag in @charleyf here since updating that portion of this seems firmly within the Timnit portion of this effort

Copy link
Contributor

@charleyf charleyf left a comment

Choose a reason for hiding this comment

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

Approving based on our discussion in Slack. I just went and verified that the way getSelfieCaptureEnabled is written on the FE makes it so that I don't see a failure mode where we accidentally turn the feature on (my primary concern).

@jmhooper jmhooper merged commit b6cffc8 into main Dec 18, 2023
@jmhooper jmhooper deleted the jmhooper-rename-config branch December 18, 2023 15:20
@eileen-nava
Copy link
Contributor

This looks good, thank you, @jmhooper!

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.

4 participants