[HOLD Auth#21849] Re-apply secure private personal details + fix 4 blockers from previous revert#91646
[HOLD Auth#21849] Re-apply secure private personal details + fix 4 blockers from previous revert#91646jasperhuangg wants to merge 6 commits into
Conversation
…omplete and country auto-fill" This reverts commit 30d6f08.
- Preserve form draft across the navigation to the magic-code RHP. The prior cleanup cleared the draft on every unmount, so navigating to the magic-code screen wiped the payload before submission. For new users with no stored address that resulted in Auth returning an error on UpdatePrivatePersonalDetails (#91602). It also caused the country to fall back to the geolocation default when the user backed out of the magic-code page and remounted the form (#91608). Now the draft is cleared only when the form unmounts without navigating to the magic code screen, and the magic-code success effect clears the draft after the API write completes. - Seed selectedCountry / selectedState from the form draft (with the stored address and geolocation as further fallbacks) so a back-from- magic-code remount restores the user's choices instead of reverting to geolocation (#91608). - Pop the magic-code RHP with a plain Navigation.goBack() instead of goBack(ROUTES.SETTINGS_PRIVATE_PERSONAL_DETAILS). The route-based back path compares params, which never matched the existing PrivatePersonalDetails route (it carried a fieldToFocus param) and fell through to REPLACE, leaving a duplicate PrivatePersonalDetails on the RHP stack so the next back-press only popped the duplicate (#91611). - AddressSearch now accepts an autoFocus prop and forwards it to the underlying TextInput, so opening PrivatePersonalDetailsPage with ?fieldToFocus=addressLine1 actually focuses Address line 1 (#91604).
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
melvin-bot
Bot
requested review from
a team,
mountiny and
parasharrajat
and removed request for
a team
|
@parasharrajat @mountiny One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
npm has a |
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 92f50bf135e..972f9624a4c 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -3334,7 +3334,7 @@ ${amount} für ${merchant} – ${date}`,
enterPhoneNumber: 'Wie lautet deine Telefonnummer?',
personalDetails: 'Persönliche Angaben',
privateDataMessage: 'Diese Angaben werden für Reisen und Zahlungen verwendet. Sie werden niemals in deinem öffentlichen Profil angezeigt.',
- basicDetails: 'Grundlegende Angaben',
+ basicDetails: 'Grundlegende Details',
legalName: 'Rechtlicher Name',
legalFirstName: 'Rechtlicher Vorname',
legalLastName: 'Rechtlicher Nachname',
@@ -3529,7 +3529,7 @@ ${amount} für ${merchant} – ${date}`,
noBankAccountSelected: 'Bitte wähle ein Konto aus',
taxID: 'Bitte geben Sie eine gültige Steueridentifikationsnummer ein',
website: 'Bitte eine gültige Website eingeben',
- zipCode: `Bitte gib eine gültige Postleitzahl im folgenden Format ein: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
+ zipCode: `Bitte geben Sie eine gültige Postleitzahl im folgenden Format ein: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
phoneNumber: 'Bitte gib eine gültige Telefonnummer ein',
email: 'Bitte gib eine gültige E-Mail-Adresse ein',
companyName: 'Bitte gib einen gültigen Unternehmensnamen ein',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 3a8d9ce406c..7549e479462 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -3214,7 +3214,7 @@ ${amount} para ${merchant} - ${date}`,
enterPhoneNumber: '¿Cuál es tu número de teléfono?',
personalDetails: 'Datos personales',
privateDataMessage: 'Estos detalles se utilizan para viajes y pagos. Nunca se mostrarán en tu perfil público.',
- basicDetails: 'Datos básicos',
+ basicDetails: 'Detalles básicos',
legalName: 'Nombre completo',
legalFirstName: 'Nombre legal',
legalLastName: 'Apellidos legales',
@@ -3412,7 +3412,7 @@ ${amount} para ${merchant} - ${date}`,
noBankAccountSelected: 'Por favor, elige una cuenta bancaria',
taxID: 'Por favor, introduce un número de identificación fiscal válido',
website: 'Por favor, introduce un sitio web válido',
- zipCode: `Formato de código postal incorrecto. Formato aceptable: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}.`,
+ zipCode: `Por favor, introduce un código ZIP válido usando el formato: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
phoneNumber: 'Por favor, introduce un teléfono válido',
email: 'Por favor, introduce una dirección de correo electrónico válida',
companyName: 'Por favor, introduce un nombre comercial legal válido',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 8235e1a4c80..c95a2fa633b 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -3540,7 +3540,7 @@ ${amount} pour ${merchant} - ${date}`,
noBankAccountSelected: 'Veuillez choisir un compte',
taxID: 'Veuillez saisir un numéro d’identification fiscale valide',
website: 'Veuillez saisir un site web valide',
- zipCode: `Veuillez saisir un code postal valide au format : ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
+ zipCode: `Veuillez saisir un code ZIP valide en utilisant le format : ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
phoneNumber: 'Veuillez saisir un numéro de téléphone valide',
email: 'Veuillez saisir une adresse e-mail valide',
companyName: 'Veuillez saisir un nom d’entreprise valide',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index dd0e65638d8..4bd36a01cca 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -3492,7 +3492,7 @@ ${integrationName === CONST.ONBOARDING_ACCOUNTING_MAPPING.other ? 'あなたの'
noBankAccountSelected: 'アカウントを選択してください',
taxID: '有効な納税者番号を入力してください',
website: '有効なウェブサイトを入力してください',
- zipCode: `有効なZIPコードを、次の形式で入力してください: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
+ zipCode: `有効な ZIP コードを、次の形式で入力してください：${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
phoneNumber: '有効な電話番号を入力してください',
email: '有効なメールアドレスを入力してください',
companyName: '有効な会社名を入力してください',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 72296059032..9f511ae2130 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -3508,7 +3508,7 @@ ${amount} dla ${merchant} - ${date}`,
noBankAccountSelected: 'Wybierz konto',
taxID: 'Wprowadź prawidłowy numer identyfikacji podatkowej',
website: 'Wprowadź prawidłową stronę internetową',
- zipCode: `Wprowadź prawidłowy kod ZIP w formacie: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
+ zipCode: `Wpisz prawidłowy kod ZIP w formacie: ${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
phoneNumber: 'Wprowadź prawidłowy numer telefonu',
email: 'Wpisz prawidłowy adres e‑mail',
companyName: 'Wprowadź prawidłową nazwę firmy',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 0c9a4e0084e..dd4be17b0f6 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -3429,7 +3429,7 @@ ${amount}，商户：${merchant} - 日期：${date}`,
noBankAccountSelected: '请选择一个账户',
taxID: '请输入有效的税号',
website: '请输入有效的网站',
- zipCode: `请输入有效的邮政编码，格式为：${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
+ zipCode: `请按照以下格式输入有效的邮政编码：${COMMON_CONST.COUNTRY_ZIP_REGEX_DATA.US.samples}`,
phoneNumber: '请输入有效的电话号码',
email: '请输入有效的邮箱地址',
companyName: '请输入有效的公司名称',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a57604f84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…vatePersonalDetails-redo-v2
jasperhuangg
changed the title
…vatePersonalDetails-redo-v2
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33866eae94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Drop the `|| selectedState` / `|| selectedCountry` fallbacks in the PrivatePersonalDetailsPage validator. The form values already reflect the selectors (FormProvider syncs controlled `value` into inputValues), and on the non-US path the state TextInput has no handler to keep selectedState in sync — so clearing a prefilled state let the required check pass against the stale local value and submit an empty addressState. Add `address?.street2` to the precedence chain in getPrivatePersonalDetailsFormValues so a separately-stored line 2 is preserved when the form is rebuilt without a draft, matching the precedence the page itself uses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vatePersonalDetails-redo-v2
|
Found an odd behavior: If I fill out the form, close it out without submitting, then reopen it, the values I provided are still there. If I close it and reopen it again without advancing to the magic code screen, the values all disappear. Is that expected? Screen.Recording.2026-05-27.at.2.49.36.PM.mov |
rafecolton
left a comment
rafecolton
left a comment
There was a problem hiding this comment.
Other than the odd behavior noted in my prior comment, this looks fine to me, and all of the tests you laid out pass. However, given how many times this has been reverted, maybe we could have QA do regression tests on an ad-hoc build once the Auth PR is deployed?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b44ed7db9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // UI-prefilled values (geolocation country, normalized state) only live in component state until the user touches | ||
| // a field, so write the full form values to the draft before navigating so the confirm step submits them. | ||
| setDraftValues(ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM, values); | ||
| skipClearDraftOnUnmountRef.current = true; |
There was a problem hiding this comment.
If a user edits details, presses Save, then closes the magic-code screen instead of submitting, this ref remains true when they return to the details page. Pressing Back from there unmounts the page but skips clearDraftValues, so abandoned edits stay in personalDetailsFormDraft and prefill the form the next time the user opens private personal details. Reset the flag when the confirm screen is dismissed or when the details page regains focus.
Useful? React with 👍 / 👎.
| if (!stateValue.trim()) { | ||
| errors[INPUT_IDS.STATE] = translate('common.error.fieldRequired'); | ||
| } |
There was a problem hiding this comment.
When the country is US, this only checks that stateValue is non-empty, so an existing non-US province such as Ontario can remain in the form after switching the country to United States and pass client validation. This page's own helper notes that StateSelector keys on 2-letter codes, so the US path should either clear invalid values on country change or reject values that are not keys in COMMON_CONST.STATES before submitting addressState.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is valid feedback actually. I was able to submit the form like this:
Then I get a mysterious error on the magic code screen:
We should stop the user from submitting an invalid form
Good idea, I'll have them run through.. is there a list of pre-existing regression tests that you recommend I run through with them? Or would these just be ones I provide them myself? |
Hmm I see what you mean by it getting cleared out on the second time you close it, do you think this is something worth blocking on? |
I just tested existing behavior, and fields aren't saved at all, so no, I don't think so. It would be nice to fix though if it's easy Edit: Codex is suggesting we should clear these actually, so I'm not sure what the right behavior is. Maybe ask in Slack?
I'm not sure specifically. However the previous bugs were found I guess 😄 |
|
Ah I see, I think unless a PR hits staging, then there's no way for us to run the full regression suite on it. For AdHoc builds I think we need to specify a specific set of tests we want them to run. So it's fine if we ship it to staging, it would suck if it gets reverted again but that's kinda the whole point of having a staging environment right? |
|
Ad-hoc builds will run against the the staging or production backend depending on what's configured in the troubleshooting settings. I don't think we should have them run a full regression suite on that, just the ones relevant to this flow |
3 participants
Explanation of Change
Re-applies #91000 (reverted in #91606) and fixes the 4 blockers that caused the revert.
Fixed Issues
$ #91602
$ #91604
$ #91608
$ #91611
Tests
?fieldToFocus=addressLine1and the field is focused ([Due for payment 2026-06-02] Profile - Address line 1 is not auto focused when URL contains ?fieldToFocus=addressLine1 #91604).Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari