Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor] Merge the post verification logic between verification panes. #8653

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Jun 18, 2024

Summary

  1. [Refactor] Took the opportunity to merge the post verification logic between verification panes. Functionally no changes (besides 1), and brings us 1 step closer to merge verification panes in the future as web does. See equivalent web code here.

Motivation

TBD

Testing

  • Added tests
  • Modified tests
  • Manually verified
image

Screenshots

test-nme

Changelog

@carlosmuvi-stripe carlosmuvi-stripe added the work-cli Added to pull requests created with #work-cli for usage tracking. label Jun 18, 2024
@carlosmuvi-stripe carlosmuvi-stripe self-assigned this Jun 18, 2024
Copy link
Contributor

github-actions bot commented Jun 18, 2024

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 │       2 MiB │       2 MiB │  0 B │     4.2 MiB │     4.2 MiB │  0 B 
     arsc │ 1,023.8 KiB │ 1,023.8 KiB │  0 B │ 1,023.7 KiB │ 1,023.7 KiB │  0 B 
 manifest │     2.3 KiB │     2.3 KiB │  0 B │       8 KiB │       8 KiB │  0 B 
      res │   301.5 KiB │   301.5 KiB │  0 B │     455 KiB │     455 KiB │  0 B 
   native │     6.2 MiB │     6.2 MiB │  0 B │    15.8 MiB │    15.8 MiB │  0 B 
    asset │     6.7 KiB │     6.7 KiB │  0 B │     6.5 KiB │     6.5 KiB │  0 B 
    other │    85.5 KiB │    85.5 KiB │  0 B │   158.7 KiB │   158.7 KiB │  0 B 
──────────┼─────────────┼─────────────┼──────┼─────────────┼─────────────┼──────
    total │     9.6 MiB │     9.6 MiB │  0 B │    21.6 MiB │    21.6 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 21305 │ 21305 │ 0 (+0 -0) 
   types │  6770 │  6770 │ 0 (+0 -0) 
 classes │  5559 │  5559 │ 0 (+0 -0) 
 methods │ 31121 │ 31121 │ 0 (+0 -0) 
  fields │ 18141 │ 18141 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3392 │ 3392 │  0
APK
   compressed    │   uncompressed   │                                           
──────────┬──────┼───────────┬──────┤                                           
 size     │ diff │ size      │ diff │ path                                      
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 28.3 KiB │ -5 B │  62.6 KiB │  0 B │ ∆ META-INF/CERT.SF                        
 25.1 KiB │ +5 B │  62.5 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
    271 B │ -1 B │     120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
  1.2 KiB │ +1 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA                       
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 54.9 KiB │  0 B │ 126.4 KiB │  0 B │ (total)

import com.stripe.android.financialconnections.repository.AttachedPaymentAccountRepository
import javax.inject.Inject

internal class CompleteVerification @Inject constructor(
Copy link
Collaborator Author

@carlosmuvi-stripe carlosmuvi-stripe Jun 28, 2024

Choose a reason for hiding this comment

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

equivalent web code: https://git.corp.stripe.com/stripe-internal/stripe-js-v3/blob/02e07063/src/linkedAccounts/inner/components/panes/v3/LinkVerificationPane/index.tsx#L177

Note this is just merging the logic across verification panes into a usecase. No new code here besides checking if an account of type bank_account has been previously attached.

@@ -45,15 +45,15 @@ internal data class PartnerAccountsList(
@Parcelize
internal data class PartnerAccount(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fields are nullable now to support NME accounts.

Comment on lines +15 to +20
internal class AttachedPaymentAccountRepository @Inject constructor(
savedStateHandle: SavedStateHandle,
private val logger: Logger,
) : PersistingRepository<AttachedPaymentAccountRepository.State>(
savedStateHandle = savedStateHandle,
) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is used to remember previously attached accounts locally. If an account of type bank_account (manually entered) was attached, we'll call save_accounts_to_link with null as accounts.

@@ -35,11 +37,13 @@ internal class PollAttachPaymentAccount @Inject constructor(
retryCondition = { exception -> exception.shouldRetry }
) {
try {
repository.postLinkAccountSessionPaymentAccount(
repository.postAttachPaymentAccountToLinkAccountSession(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

renamed this api request to be more precise.

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review July 1, 2024 18:12
@carlosmuvi-stripe carlosmuvi-stripe requested review from a team as code owners July 1, 2024 18:12
@carlosmuvi-stripe carlosmuvi-stripe changed the title [Android] Support for networking a manually entered account and signing up to Link [Refactor] Merge the post verification logic between verification panes. Jul 12, 2024
@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as draft July 12, 2024 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-cli Added to pull requests created with #work-cli for usage tracking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants