Skip to content

SDKS-5268 flutter-davinci sample app - #145

Open
rodrigoareis wants to merge 3 commits into
flutterfrom
SDKS-5268
Open

rodrigoareis wants to merge 3 commits into
flutterfrom
SDKS-5268

Conversation

@rodrigoareis

Copy link
Copy Markdown
Contributor

Summary

Adds flutter-davinci, a Flutter sample app that runs a PingOne DaVinci flow natively through the ping_davinci bridge (#144)). The branch also carries one hardening commit for the bridge, found while writing and runtime-checking the sample.

  • flutter/flutter-davinci/ — the new sample app (Android/iOS, native Ping SDK 2.2.0):
    • Env reads PINGONE_DISCOVERY_ENDPOINT, PINGONE_CLIENT_ID, PINGONE_REDIRECT_URI, PINGONE_SCOPES and PINGONE_ACR_VALUES through String.fromEnvironment, with TODO placeholders as defaults. flutter run --dart-define-from-file=../.env works with nothing edited
    • A start screen where the acr_values flow policy id can be changed per run (persisted with shared_preferences)
    • CollectorForm renders each ContinueNode dynamically, one widget per collector branch: text, password (with verify), submit and flow buttons, labels, read-only text, single/multi select, boolean (a SwitchListTile when the server asks for SWITCH), phone number, image, polling, QR code, device registration/authentication and metadata
    • Local validate before submit. An ErrorNode's per-field errors render on the same form so the user can correct and retry; errors with no field target (a wrong password) show as a banner
    • Fields the app can't render (unsupportedFields) are listed with their reason instead of failing the form
    • Success screen: authorization code, tokens and userinfo, token reload (the native SDK refreshes if expired), and sign-off
    • MVVM structure (data/ repository, ui/features/*/view_models + views, routing/), go_router, a shared theme
    • Android sets the appRedirectUriScheme manifest placeholder that native 2.2.0's browser module requires. iOS needs no Info.plist change
    • Ships no tests, by design, like the other samples. Coverage lives in the ping_davinci unit and integration suites; the README says so
  • flutter-journey / flutter-oidc — Env now reads build-time PINGAIC_* dart-defines too (same pattern as above), and their READMEs and config views are updated to match. Defaults and behaviour are unchanged when no define is passed. flutter-journey's oidcConfig is now null until clientId is set
  • flutter/flutter-sdk-bridge/ — hardening, found by the sample:
    • ping_davinci: every call that reads or moves a flow (start, next, validate, startPolling, signOff, dispose) is serialized per handle, and dispose removes the handle first. A next or startPolling in flight during dispose can no longer leave a cached node or a running poll behind
    • ping_davinci iOS: detaching the Flutter engine stops every running poll, as Android's does
    • New diagnostic host method retainedState() (handles, cached nodes/sessions, flow locks, polls), used by a new configure-to-dispose leak test (davinci_lifecycle_leak_test). It is not part of the public Dart API
    • ping_core: Registry reports its size (size on Android, count() on iOS) for the leak check
    • Both CHANGELOGs updated; flutter-sdk-bridge/README.md now documents ping_davinci (module description, "DaVinci collectors vs. Journey callbacks", how to run its integration suite)
  • flutter/pubspec.yaml — adds flutter-davinci to the workspace

Review note: much of the bridge diff outside the items above is dart format output (ping_core's oidc_config*.dart and its tests, ping_journey's messages.g.dart, ping_davinci's collector_test.dart). Pigeon's output isn't formatted, so these had drifted. No behaviour change in those files. The sample's android/, ios/ and asset files are flutter create output.

Test plan

  • flutter analyze clean (workspace root)
  • ping_davinci unit tests, Android native unit tests and iOS RunnerTests pass after the hardening change
  • ping_davinci integration suite (hermetic mock DaVinci server) re-run after the hardening change, one file per process, on Android (Pixel 5 API 33) and the iOS simulator, including the new davinci_lifecycle_leak_test
  • Sample verified at runtime on both platforms against the live DaVinci test tenant, driving the real app with a throwaway WidgetTester driver (removed afterwards, since the sample ships no tests): sign-on, wrong password banner, inline field error on register, form-fields flow, continue and finish polling, QR code, device registration, metadata, success screen (code, tokens, userinfo, reload), sign-off. One section per collector branch; all pass on both platforms
  • flutter build apk --debug and flutter build ios --debug --no-codesign from a clean git worktree checkout of the branch
  • DEVICE_AUTHENTICATION is rendered but not walked live: the tenant offers it only to a signed-in user with an enrolled device
  • On iOS, a polling timeout shows as an API error (HTTP 400) failure page, where Android shows a recoverable error (SDKS-5461, pre-existing)

@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@spetrov spetrov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Test coverage for the behaviour this app demonstrates lives in the bridge package instead:

- **Unit tests** — `../flutter-sdk-bridge/ping_davinci/test/` (collector and node mapping,
validation, error translation). Run with `flutter test` from the `flutter/` workspace root.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

run "from the flutter/ workspace root"

must run from flutter-sdk-bridge/ping_davinci/ ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants