Skip to content

Fix issue with "Change email" behavior in Link #10706

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tillh-stripe
Copy link
Collaborator

@tillh-stripe tillh-stripe commented Apr 23, 2025

Summary

This pull request fixes an issue with the “Change email” behavior in Link.

After selecting “Change email” and closing the flow, we now show the signup screen on re-open instead of the verification screen. This makes more sense, since the user already indicated that they don't want to use the email address that we pull from the CustomerInfo object.

Motivation

LINK_MOBILE-165

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screen recordings

Before

Screen_recording_20250423_113043.mp4

After

Screen_recording_20250423_112842.mp4

Changelog

get() {
val clearCustomerDetails = linkAccount == null
val effectiveCustomerInfo = if (clearCustomerDetails) {
LinkConfiguration.CustomerInfo(null, null, null, null)
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 was thinking of only setting the email address to null and keeping the other values as-is, but Web seems to fully clear the customer info for the same flow.

@@ -285,7 +296,7 @@ internal class LinkActivityViewModel @Inject constructor(

DaggerNativeLinkComponent
.builder()
.configuration(args.configuration)
.configuration(args.configurationWithUpdatedCustomerInfo)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might be weird that this happens here, but if we don't update the configuration, then this code will fetch the Link account associated with that email address.

@tillh-stripe tillh-stripe marked this pull request as ready for review April 23, 2025 15:28
@tillh-stripe tillh-stripe requested review from a team as code owners April 23, 2025 15:28
Copy link
Contributor

github-actions bot commented Apr 23, 2025

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.1 MiB │   2.1 MiB │  0 B │   4.3 MiB │   4.3 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 │ 302.9 KiB │ 302.9 KiB │  0 B │   457 KiB │   457 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.7 KiB │   7.7 KiB │  0 B │   7.4 KiB │   7.4 KiB │  0 B 
    other │  95.7 KiB │  95.7 KiB │ +5 B │ 183.5 KiB │ 183.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.8 MiB │   9.8 MiB │ +5 B │  21.8 MiB │  21.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20668 │ 20668 │ 0 (+0 -0) 
   types │  6493 │  6493 │ 0 (+0 -0) 
 classes │  5259 │  5259 │ 0 (+0 -0) 
 methods │ 31489 │ 31489 │ 0 (+0 -0) 
  fields │ 18221 │ 18221 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3646 │ 3646 │  0
APK
   compressed    │   uncompressed   │                        
──────────┬──────┼───────────┬──────┤                        
 size     │ diff │ size      │ diff │ path                   
──────────┼──────┼───────────┼──────┼────────────────────────
 29.2 KiB │ +4 B │  64.6 KiB │  0 B │ ∆ META-INF/CERT.SF     
  1.2 KiB │ +2 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA    
 25.9 KiB │ -1 B │  64.5 KiB │  0 B │ ∆ META-INF/MANIFEST.MF 
──────────┼──────┼───────────┼──────┼────────────────────────
 56.2 KiB │ +5 B │ 130.4 KiB │  0 B │ (total)

@tillh-stripe tillh-stripe force-pushed the tillh/fix-link-change-email-issue branch from f726bfa to e2f1285 Compare April 23, 2025 18:57
After selecting "Change email" and closing the flow, we now show the signup screen on re-open instead of the verification screen.
@tillh-stripe tillh-stripe force-pushed the tillh/fix-link-change-email-issue branch from e2f1285 to 3369932 Compare April 23, 2025 19:00
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe left a comment

Choose a reason for hiding this comment

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

LGTM! one small comment

@@ -271,3 +278,16 @@ internal sealed interface ScreenState {
}

internal class NoArgsException : IllegalArgumentException("NativeLinkArgs not found")

private val NativeLinkArgs.configurationWithUpdatedCustomerInfo: LinkConfiguration
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit - I think this is more understandable as an extension function of LinkConfiguration, so that it reads:

args.configuration.withUpdatedCustomerInfo(args.LinkAccount) - might be a bit more verbose on the other hand so leave it as is if it makes sense!

@tillh-stripe tillh-stripe marked this pull request as draft April 24, 2025 01:20
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.

3 participants