[Identity] Remove all parameters for Scan and Upload destinations

[ccen-stripe]

Summary

Remove all parameters passed to the following 6 types of destinations to capture document through camera/file upload

• DriverLicenseScanDestination
• PassportScanDestination
• IDScanDestination
  ⬆️ all replaced with DocumentScanDestination without params
• IDUploadDestination
• DriverLicenseUploadDestination
• PassportUploadDestination
  ⬆️ all replaced with DocumentUploadDestination without params

Now DocumentScanScreen and UploadScreen will not read the parameters to determine its UI, but instead use the states from IdentityViewModel and IdentityScanViewModel to determine them.

Motivation

This is one step further to clean up the legacy someFlow.collectLast call prior to compose. And is a prerequisite to Butter M1 UX change to remove document select page

Testing

• [] Added tests
• Modified tests
• Manually verified

Changelog

[ccen-stripe]

@stripe/stripe-identity-observers

[github-actions]

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │            compressed            │           uncompressed            
          ├───────────┬───────────┬──────────┼───────────┬───────────┬───────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff      
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼───────────
      dex │   1.9 MiB │   1.9 MiB │ -3.6 KiB │     4 MiB │     4 MiB │ -10.1 KiB 
     arsc │   1.4 MiB │   1.4 MiB │      0 B │   1.4 MiB │   1.4 MiB │       0 B 
 manifest │   2.3 KiB │   2.3 KiB │      0 B │   8.1 KiB │   8.1 KiB │       0 B 
      res │ 351.3 KiB │ 351.3 KiB │      0 B │ 481.9 KiB │ 481.9 KiB │       0 B 
   native │   6.2 MiB │   6.2 MiB │      0 B │  15.8 MiB │  15.8 MiB │       0 B 
    asset │  66.3 KiB │  67.4 KiB │ +1.1 KiB │  86.3 KiB │  87.4 KiB │  +1.1 KiB 
    other │ 113.2 KiB │ 113.2 KiB │     -7 B │ 260.5 KiB │ 260.5 KiB │       0 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼───────────
    total │    10 MiB │    10 MiB │ -2.5 KiB │    22 MiB │  21.9 MiB │    -9 KiB 

 DEX     │ old   │ new   │ diff              
─────────┼───────┼───────┼───────────────────
   files │     1 │     1 │   0               
 strings │ 20105 │ 20089 │ -16 (+79 -95)     
   types │  6175 │  6164 │ -11 (+18 -29)     
 classes │  5027 │  5017 │ -10 (+8 -18)      
 methods │ 30488 │ 30457 │ -31 (+2429 -2460) 
  fields │ 17352 │ 17266 │ -86 (+1491 -1577) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  261 │  261 │  0   
 entries │ 5177 │ 5177 │  0

APK

     compressed      │     uncompressed      │                                
──────────┬──────────┼───────────┬───────────┤                                
 size     │ diff     │ size      │ diff      │ path                           
──────────┼──────────┼───────────┼───────────┼────────────────────────────────
  1.9 MiB │ -3.6 KiB │     4 MiB │ -10.1 KiB │ ∆ classes.dex                  
  5.7 KiB │ +1.1 KiB │   5.5 KiB │  +1.1 KiB │ ∆ assets/dexopt/baseline.prof  
 49.5 KiB │     -5 B │ 113.2 KiB │       0 B │ ∆ META-INF/CERT.SF             
 31.3 KiB │     -2 B │ 113.1 KiB │       0 B │ ∆ META-INF/MANIFEST.MF         
    707 B │     +1 B │     575 B │      +1 B │ ∆ assets/dexopt/baseline.profm 
──────────┼──────────┼───────────┼───────────┼────────────────────────────────
    2 MiB │ -2.5 KiB │   4.2 MiB │    -9 KiB │ (total)

DEX

STRINGS:

   old   │ new   │ diff          
  ───────┼───────┼───────────────
   20105 │ 20089 │ -16 (+79 -95) 
  + A2
  + B2
  + C2
  + D2
  + DOC_BACK
  + DOC_FRONT
  + E2
  + F2
  + G2
  + H2
  + I2
  + J2
  + K2
  + L2
  + Lk7/y2;
  + Lk7/z2;
  + Lm7/g1;
  + Lm7/h1;
  + Lm7/i1;
  + Lo0/v2;
  + Lo6/f;
  + Lw/w1;
  + M2
  + N2
  + O2
  + P2
  + Q2
  + R2
  + S2
  + Scan
  + T2
  + U2
  + Upload
  + V2
  + VLLLLLLLLLI
  + VLZLLLII
  + VZZLLIZI
  + VZZZLLLI
  + VZZZLLLLI
  + W2
  + X2
  + Y2
  + Z2
  + [Lk7/c1;
  + [Lk7/i1;
  + [Lk7/p;
  + [Lk7/q2;
  + [Lk7/u1;
  + [Lr7/a2;
  + [Lr7/s1;
  + [Lr7/y1;
  + [Lw/t1;
  + [Lx0/m1;
  + a3
  + additionalParams
  + b3
  + c3
  + d3
  + doc_front
  + e3
  + f3
  + fromSelfie
  + g3
  + incorrect category: 
  + live_capture
  + o2
  + p2
  + pageAndModelFiles
  + q2
  + r2
  + s2
  + t2
  + u2
  + v2
  + w2
  + x2
  + y2
  + z2
  + ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:e841d9d,r8-mode:full,version:8.1.65}
  
  -  should not be null when trying to scan from back
  - , backMessageStringRes=
  - , backTitleStringRes=
  - , checkmarkContentDescriptionRes=
  - , frontMessageStringRes=
  - , scanType=
  - DL_BACK
  - DL_FRONT
  - DocumentScanMessageRes(frontTitleStringRes=
  - DocumentUploadSideInfo(descriptionRes=
  - DriverLicenseScan
  - DriverLicenseUpload
  - IDScan
  - IDUpload
  - LLZZ
  - Lg7/j0;
  - Lg7/k0;
  - Lg7/l0;
  - Lg7/m0;
  - Lg7/n0;
  - Lg7/o0;
  - Lg7/p0;
  - Lg7/q0;
  - Lg7/r0;
  - Lg7/s0;
  - Lg7/t0;
  - Lg7/u0;
  - Ll7/f;
  - Ll7/g;
  - Ll7/h;
  - Lr7/c3;
  - Lw6/x;
  - Ly5/e;
  - PassportScan
  - PassportUpload
  - SELFIE doesn't support upload
  - VLLLIIIIIL
  - VLLLLIILLI
  - VLLLLIILLLI
  - VLLLLLIIILLL
  - VLLLLLLILLZLLL
  - VLLLLLLLLLL
  - VLLLLLZLLLI
  - VLLLLLZLLLLI
  - VLLLLZII
  - VLLLLZLII
  - VLLZLLLL
  - VLZLLIIIIL
  - VLZLLIZI
  - VLZZLLLLI
  - VZLLIIIIIL
  - VZLLLLL
  - VZZLLIIIIL
  - [Lg7/j0;
  - [Lk7/a1;
  - [Lk7/g1;
  - [Lk7/m;
  - [Lk7/n2;
  - [Lk7/s1;
  - [Lr7/b2;
  - [Lr7/t1;
  - [Lr7/z1;
  - [Lw/s1;
  - [Ly5/e;
  - backCheckMarkDescriptionRes
  - backDescriptionRes
  - backMessageStringRes
  - backScanType
  - backScanType is null while still missing back
  - backTitleStringRes
  - back_scan_type
  - contextRes
  - driver_license
  - file_upload_driver_license
  - file_upload_id
  - file_upload_passport
  - frontCheckMarkDescriptionRes
  - frontDescriptionRes
  - frontInfo
  - frontMessageStringRes
  - frontScanType
  - frontTitleStringRes
  - front_scan_type
  - invalid scan type: 
  - live_capture_driver_license
  - live_capture_id
  - live_capture_passport
  - messageRes
  - null cannot be cast to non-null type com.stripe.android.identity.states.IdentityScanState.ScanType
  - requireLiveCapture
  - startFromBack
  - targetScanType
  - titleRes
  - uploadInfo
  - ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:ef41134,r8-mode:full,version:8.1.65}
  

TYPES:

   old  │ new  │ diff          
  ──────┼──────┼───────────────
   6175 │ 6164 │ -11 (+18 -29) 
  + Lk7/y2;
  + Lk7/z2;
  + Lm7/g1;
  + Lm7/h1;
  + Lm7/i1;
  + Lo0/v2;
  + Lo6/f;
  + Lw/w1;
  + [Lk7/c1;
  + [Lk7/i1;
  + [Lk7/p;
  + [Lk7/q2;
  + [Lk7/u1;
  + [Lr7/a2;
  + [Lr7/s1;
  + [Lr7/y1;
  + [Lw/t1;
  + [Lx0/m1;
  
  - Lg7/j0;
  - Lg7/k0;
  - Lg7/l0;
  - Lg7/m0;
  - Lg7/n0;
  - Lg7/o0;
  - Lg7/p0;
  - Lg7/q0;
  - Lg7/r0;
  - Lg7/s0;
  - Lg7/t0;
  - Lg7/u0;
  - Ll7/f;
  - Ll7/g;
  - Ll7/h;
  - Lr7/c3;
  - Lw6/x;
  - Ly5/e;
  - [Lg7/j0;
  - [Lk7/a1;
  - [Lk7/g1;
  - [Lk7/m;
  - [Lk7/n2;
  - [Lk7/s1;
  - [Lr7/b2;
  - [Lr7/t1;
  - [Lr7/z1;
  - [Lw/s1;
  - [Ly5/e;
  

METHODS:

   old   │ new   │ diff              
  ───────┼───────┼───────────────────
   30488 │ 30457 │ -31 (+2429 -2460) 
  + a0.h A(m0, a) → Object
  + a0.h B(f, float)
  + a0.h C(int) → char
  + a0.h D(int) → char
  + a0.h E(long, int, int) → int
  + a0.h F(b, w) → h0
  + a0.h G(String, int) → String
  + a0.h H(String, int, String) → String
  + a0.h I(String, String) → String
  + a0.h J(StringBuilder, String, String) → String
  + a0.h K(String, int, String) → StringBuilder
  + a0.h L(int, b, m2, w, int)
  + a0.h M(Parcelable)
  + a0.h O(j)
  + a0.h P(String, String) → String
  + a0.h Q(Object)
  + a0.h T(int) → String
  + a0.h U(int) → String
  + a0.h V(int) → String
  + a0.h W(String) → int
  + a0.h g(w, o, n, int) → int
  + a0.h h(v, o, n, int) → int
  + a0.h i(h0, o, List, int) → int
  + a
...✂

[carlosmuvi-stripe]

Great cleanup! just some feedback on using UI to communicate viewmodels.

[carlosmuvi-stripe]

looks like we're using launched effects (UI layer) to communicate viewModels (more no the controller layer).

This works but it might be hard to maintain long term. A potential alternative would be to have a coordinator that can be injected on both the parent and the child viewmodel, so the UI does not need to be involved.

This is how we do this on Connections:

• Coordinator:

  stripe-android/financial-connections/src/main/java/com/stripe/android/financialconnections/domain/NativeAuthFlowCoordinator.kt

  Line 15 in 437c9c1

  internal class NativeAuthFlowCoordinator @Inject constructor() {

• Child sending a message:

  stripe-android/financial-connections/src/main/java/com/stripe/android/financialconnections/features/manualentry/ManualEntryViewModel.kt

  Line 152 in 728b2da

  companion object :

• Parent viewmodel observing messages:

  stripe-android/financial-connections/src/main/java/com/stripe/android/financialconnections/presentation/FinancialConnectionsSheetNativeViewModel.kt

  Line 85 in 728b2da

  nativeAuthFlowCoordinator().collect { message ->

[ccen-stripe]

Thanks for the pointers, this LaunchedEffect is a bit wonky as it's mostly inherited from the legacy architecture with Fragment and navigation components.

The essential goal is to remove any observe and collectLatest and totally rely on IdentityScanViewModel's new State to drive UI, and handle these non-UI logics within identityScanViewModel itself without the need to reference the parent IdentityViewModel.

• E.g we'll directly inject the fpsTracker into IdentityScanViewModel and handle the trackFrame within IdentityScanViewModel.onInterimResult callback.

Need to double check but I think with the decoupling there will be need to send messages from childIdentityScanViewModel to parent IdentityViewModel, but I have added your reference in the follow up ticket here incase we do need that!