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

Better handle process death for all confirmation flow cases in DefaultConfirmationHandler #9754

Merged

Conversation

samer-stripe
Copy link
Collaborator

Summary

Better handle process death for all confirmation flow cases in DefaultConfirmationHandler.

Motivation

Our current process death handling works fine when a result is received in process but not great when a result is not received in process and results in a next step action

Testing

  • Added tests
  • Modified tests
  • Manually verified

@samer-stripe samer-stripe force-pushed the samer/refactor-process-death-handling-in-confirmation branch from a14e30e to 4c534c7 Compare December 6, 2024 00:20
Copy link
Contributor

github-actions bot commented Dec 6, 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.1 MiB │   4.1 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 301.8 KiB │ 301.8 KiB │  0 B │ 455.5 KiB │ 455.5 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.1 KiB │   7.1 KiB │  0 B │   6.9 KiB │   6.9 KiB │  0 B 
    other │  90.2 KiB │  90.2 KiB │ +4 B │ 170.3 KiB │ 170.3 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.6 MiB │   9.6 MiB │ +4 B │  21.5 MiB │  21.5 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 19966 │ 19966 │ 0 (+0 -0) 
   types │  6188 │  6188 │ 0 (+0 -0) 
 classes │  4979 │  4979 │ 0 (+0 -0) 
 methods │ 29759 │ 29759 │ 0 (+0 -0) 
  fields │ 17526 │ 17526 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3622 │ 3622 │  0
APK
   compressed    │  uncompressed   │                                           
──────────┬──────┼──────────┬──────┤                                           
 size     │ diff │ size     │ diff │ path                                      
──────────┼──────┼──────────┼──────┼───────────────────────────────────────────
 28.4 KiB │ +4 B │ 62.9 KiB │  0 B │ ∆ META-INF/CERT.SF                        
    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                       
──────────┼──────┼──────────┼──────┼───────────────────────────────────────────
 29.9 KiB │ +4 B │ 64.2 KiB │  0 B │ (total)

@samer-stripe samer-stripe force-pushed the samer/refactor-process-death-handling-in-confirmation branch from 4c534c7 to df88d90 Compare December 6, 2024 00:21
@@ -142,6 +145,7 @@ internal class DefaultConfirmationHandler(
when (val action = mediator.action(confirmationOption, intent)) {
is ConfirmationMediator.Action.Launch -> {
storeIsAwaitingForResult(
key = mediator.key,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I imagine we never will have a case like this but if we ever decide that the same definition can be looped through multiple times in a confirmation flow, this will break. Other option to use UUID instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like a fine trade off for now 👍

@samer-stripe samer-stripe marked this pull request as ready for review December 6, 2024 04:59
@samer-stripe samer-stripe requested review from a team as code owners December 6, 2024 04:59
@samer-stripe samer-stripe added the embedded-confirmation Related to Embedded project in terms of confirmation work label Dec 6, 2024
@@ -142,6 +145,7 @@ internal class DefaultConfirmationHandler(
when (val action = mediator.action(confirmationOption, intent)) {
is ConfirmationMediator.Action.Launch -> {
storeIsAwaitingForResult(
key = mediator.key,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like a fine trade off for now 👍

@samer-stripe samer-stripe merged commit 2b36058 into master Dec 6, 2024
13 checks passed
@samer-stripe samer-stripe deleted the samer/refactor-process-death-handling-in-confirmation branch December 6, 2024 16:04
simond-stripe pushed a commit that referenced this pull request Dec 6, 2024
simond-stripe added a commit that referenced this pull request Dec 12, 2024
* Initial camera implementation

# Conflicts:
#	connect-example/src/main/java/com/stripe/android/connect/example/ui/features/payouts/PayoutsExampleActivity.kt
#	connect/src/main/java/com/stripe/android/connect/PayoutsView.kt

# Conflicts:
#	connect/src/main/java/com/stripe/android/connect/PayoutsView.kt
#	connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewClient.kt

# Conflicts:
#	connect-example/src/main/java/com/stripe/android/connect/example/ui/features/payouts/PayoutsExampleActivity.kt
#	connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt
#	connect/src/main/java/com/stripe/android/connect/PayoutsView.kt

* Update request code

* Update example activity

* Refactor permission, handle within manager

* Fix leftover merge conflicts

# Conflicts:
#	connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainer.kt

* WIP

* Fix merge

* Fix build issues

* fix activity

* Fix suspend function

* refactor with activity oncreate

* Show promo badge in bank form (#9734)

* Show promo badge in bank form

* Address code review feedback

Fix layout issue with super-long bank name and validate with screenshot test.

* Add Embedded Appearance params to AppearanceBottomSheetDialogFragment (#9727)

* Add Embedded Appearance params to AppearanceBottomSheetDialogFragment

* [MOBILESDK-2480]update text style for bacs secondary button type (#9745)

* update text style for bacs secondary button type

* screenshots for screenshot tests

* Apply suggestions from code review

Capitalize comment and add period

Co-authored-by: Bella Koch <160939932+amk-stripe@users.noreply.github.com>

* crypto payment method (#9753)

* Better handle process death for all confirmation flow cases in `DefaultConfirmationHandler` (#9754)

* Fix lints, remove unneeded cancellation function

* Add tests

* Fix tests

* Add/fix logging

* Fix lint from log line

* Fix main thread

* Update connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainer.kt

Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>

* Update connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>

* unsupported behavior to log warning

* Fix docstrings

* Update crashing behavior for camera permission request

* Move functions around for clarity

* Update tests

* Update and fix tests

* Fix detekt lints

* Fix main activity

* Remove unnecessary fragment, move logger out of constructor

* Clean up lib restriction

* Remove unnecessary unconfined test dispatcher

* Update api

* Lint fixes

---------

Co-authored-by: Till Hellmund <tillh@stripe.com>
Co-authored-by: tjclawson-stripe <163896025+tjclawson-stripe@users.noreply.github.com>
Co-authored-by: Tian Zhao <tianzhao@stripe.com>
Co-authored-by: Bella Koch <160939932+amk-stripe@users.noreply.github.com>
Co-authored-by: ericzhang-stripe <94195995+ericzhang-stripe@users.noreply.github.com>
Co-authored-by: Samer Alabi <141707240+samer-stripe@users.noreply.github.com>
Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
embedded-confirmation Related to Embedded project in terms of confirmation work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants