SDKS-4200: Support pre-filled phone field #396

ancheetah · 2025-09-03T04:06:31Z

JIRA Ticket

https://pingidentity.atlassian.net/browse/SDKS-4200

Description

Pre-fills both phoneNumber and countryCode in the PhoneCollector input and output when available. Updates unit tests and failing e2e tests.

Added changeset

changeset-bot · 2025-09-03T04:06:34Z

🦋 Changeset detected

Latest commit: 036f495

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@forgerock/davinci-client	Minor
@forgerock/oidc-client	Minor
@forgerock/protect	Minor
@forgerock/sdk-types	Minor
@forgerock/sdk-utilities	Minor
@forgerock/iframe-manager	Minor
@forgerock/sdk-logger	Minor
@forgerock/sdk-oidc	Minor
@forgerock/sdk-request-middleware	Minor
@forgerock/storage	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

nx-cloud · 2025-09-03T04:07:20Z

View your CI Pipeline Execution ↗ for commit 036f495

Command	Status	Duration	Result
`nx run-many -t build`	✅ Succeeded	<1s	View ↗
`nx affected -t build typecheck lint test e2e-ci`	✅ Succeeded	3m 58s	View ↗
`nx-cloud record -- nx format:check`	✅ Succeeded	1s	View ↗

☁️ Nx Cloud last updated this comment at 2025-09-04 22:51:18 UTC

cerebrl · 2025-09-03T19:27:22Z

packages/davinci-client/src/lib/node.reducer.ts

              case 'PHONE_NUMBER': {
-                // No data to send
-                return returnObjectValueCollector(field, idx);
+                const defaultValues = data as PhoneNumberOutputValue;


This is a bit semantic, but the data represented here is the existing or "prefilled" data, rather than the default values which are found directly on the collector field, yeah?

That's right, let me change this to pre-filled data

cerebrl · 2025-09-03T19:28:46Z

packages/davinci-client/src/lib/davinci.types.ts

+  required: boolean; // TODO: add to phone collector
+  validatePhoneNumber: boolean; // TODO: add to phone collector


Do these TODOs still need to be completed?

I wasn't sure if we wanted to do this within the scope of this ticket. required was there before but we didn't do anything with it. validatePhoneNumber is new but I'm not sure if it's of value to the user or if DaVinci automatically validates on their end. And if we add them to the collector then where should they go in the output?

If required has always been there, then it's a bug that we don't include it. It should be added to the output just like the other collectors.

As for validatePhoneNumber, if it's new, then we need to communicate this to the team and understand its purpose.

We'll handle required and validatePhoneNumber in other tickets:
https://pingidentity.atlassian.net/browse/SDKS-4371
https://pingidentity.atlassian.net/browse/SDKS-4373

cerebrl · 2025-09-03T19:58:10Z

e2e/davinci-suites/src/phone-number-field.test.ts

+    // await expect(page.locator('#countryCode')).toBeVisible();
+    // await page.locator('#countryCode').selectOption('US');


Was the DaVinci flow this relies on changed?

Yes, the form that the flow used was changed but it doesn't really have to do with this. If you look at the component that handles the phone collector in the app you'll find that it doesn't actually do anything with the #countryCode dropdown field. It just uses the country code value from the collector's input (which I've changed to output). That country code dropdown is separate from the phone number field and it was too messy to pass the dropdown value to the phone collector. So I opted to use the prefilled country code value from the collector.

cerebrl · 2025-09-03T19:59:31Z

e2e/davinci-suites/src/phone-number-field.test.ts

Some of these tests are growing pretty long and complex. That makes me a bit anxious that they will get more and more fragile.

I agree, I also tried to reduce the amount of lines in these tests as much as I could. The last step in the tests, authentication with the registered device, doesn't seem to actually do anything according to the flow other than redirect back to the start of the flow so honestly we could remove the device authentication segment all together.

Okay, let's remove anything that doesn't directly test the collectors themselves ... just to make them a tad more resilient.

codecov-commenter · 2025-09-03T20:52:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.63%. Comparing base (7176dd5) to head (036f495).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #396      +/-   ##
==========================================
+ Coverage   55.47%   55.63%   +0.15%     
==========================================
  Files          32       32              
  Lines        2044     2051       +7     
  Branches      340      344       +4     
==========================================
+ Hits         1134     1141       +7     
  Misses        910      910

Files with missing lines	Coverage Δ
packages/davinci-client/src/lib/collector.types.ts	`100.00% <ø> (ø)`
packages/davinci-client/src/lib/collector.utils.ts	`70.99% <100.00%> (+0.41%)`	⬆️
packages/davinci-client/src/lib/davinci.types.ts	`100.00% <ø> (ø)`
packages/davinci-client/src/lib/node.reducer.ts	`72.66% <100.00%> (+0.18%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-09-03T20:52:23Z

Deployed 002fc3d to https://ForgeRock.github.io/ping-javascript-sdk/pr-396/002fc3dc0e71aaacce501dc176570d07ad4ae07d branch gh-pages in ForgeRock/ping-javascript-sdk

github-actions · 2025-09-03T20:52:27Z

📦 Bundle Size Analysis

📊 Minor Changes

📈 @forgerock/davinci-client - 34.2 KB (+0.1 KB)

➖ No Changes

➖ @forgerock/sdk-utilities - 4.0 KB
➖ @forgerock/device-client - 9.2 KB
➖ @forgerock/sdk-types - 5.9 KB
➖ @forgerock/protect - 150.1 KB
➖ @forgerock/sdk-oidc - 2.7 KB
➖ @forgerock/sdk-logger - 1.6 KB
➖ @forgerock/storage - 1.4 KB
➖ @forgerock/sdk-request-middleware - 4.4 KB
➖ @forgerock/iframe-manager - 2.4 KB
➖ @forgerock/oidc-client - 22.3 KB

11 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated

Current Size: Total gzipped size of all files in the package's dist directory
Baseline: Comparison against the latest build from the main branch
Files included: All build outputs except source maps and TypeScript build cache
Exclusions: .map, .tsbuildinfo, and .d.ts.map files

_{🔄 Updated automatically on each push to this PR}

ancheetah force-pushed the SDKS-4200-phone-prefill branch from d670f1d to af9d7cb Compare September 3, 2025 04:13

cerebrl reviewed Sep 3, 2025

View reviewed changes

feat(davinci-client): support prefilled phone field

036f495

ancheetah force-pushed the SDKS-4200-phone-prefill branch from af9d7cb to 036f495 Compare September 4, 2025 22:45

ancheetah requested a review from cerebrl September 4, 2025 22:52

cerebrl approved these changes Sep 4, 2025

View reviewed changes

ancheetah merged commit 52ba723 into main Sep 8, 2025
3 checks passed

ancheetah deleted the SDKS-4200-phone-prefill branch September 8, 2025 22:21

ryanbas21 mentioned this pull request Sep 8, 2025

Release PR #345

Open

		required: boolean; // TODO: add to phone collector
		validatePhoneNumber: boolean; // TODO: add to phone collector

		// await expect(page.locator('#countryCode')).toBeVisible();
		// await page.locator('#countryCode').selectOption('US');

SDKS-4200: Support pre-filled phone field #396

SDKS-4200: Support pre-filled phone field #396

Uh oh!

Conversation

ancheetah commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Ticket

Description

Uh oh!

changeset-bot bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

nx-cloud bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Bundle Size Analysis

📦 Bundle Size Analysis

Legend

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

ancheetah commented Sep 3, 2025 •

edited

Loading

changeset-bot bot commented Sep 3, 2025 •

edited

Loading

nx-cloud bot commented Sep 3, 2025 •

edited

Loading

codecov-commenter commented Sep 3, 2025 •

edited

Loading

github-actions bot commented Sep 3, 2025 •

edited

Loading

github-actions bot commented Sep 3, 2025 •

edited

Loading