Skip to content

Conversation

@shazarre
Copy link
Collaborator

@shazarre shazarre commented Jan 26, 2026

Description

This PR brings changes to our SDKs in order to be able to support first_name and last_name disclosure flags. This is not yet supported by the app.

Tested

  • ran tests locally
  • generated a QR code in a separate project

How to QA

  • generate a QR code
  • scan it using the app

Note

Adds optional first_name/last_name fields to SelfAppDisclosureConfig across common and SDKs, with corresponding docs updates and tests.

  • Export getSelectorDg1Passport/getSelectorDg1IdCard and add guards to ignore unknown disclosure attributes when building DG1 selectors
  • New unit tests validating type acceptance and selector behavior with first_name/last_name
  • README updates in qrcode and qrcode-angular documenting the new disclosure options

Written by Cursor Bugbot for commit 04545c5. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • New Features

    • Added first_name and last_name as optional disclosure fields across SDKs.
  • Public API

    • Exposed two selector utilities for external use; they now safely ignore unknown attributes to avoid errors.
  • Documentation

    • Updated SDK READMEs to document the new disclosure options.
  • Tests

    • Added unit tests covering first_name/last_name handling and the new selector utilities' behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@shazarre shazarre self-assigned this Jan 26, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds optional first_name and last_name boolean flags to the SelfAppDisclosureConfig interface across packages, exports selector helpers for DG1 passport/ID card with attribute guards, and includes unit tests and README updates reflecting the new disclosure fields.

Changes

Cohort / File(s) Summary
Interface Definitions
common/src/utils/appType.ts, sdk/qrcode-angular/src/lib/common.ts, sdk/sdk-common/index.ts
Added optional first_name?: boolean and last_name?: boolean to SelfAppDisclosureConfig in multiple package entry points.
Circuit Selector Helpers
common/src/utils/circuits/registerInputs.ts
Exported getSelectorDg1Passport and getSelectorDg1IdCard and added guards to skip unknown attributes when building selectors.
Unit Tests — appType
common/tests/appType.test.ts
New tests covering first_name/last_name handling (true/false/omitted) and multi-field disclosure combinations.
Unit Tests — registerInputs
common/tests/registerInputs.test.ts
New tests validating selector outputs for passport and ID card when first_name is included.
Documentation / SDK READMEs
sdk/qrcode-angular/README.md, sdk/qrcode/README.md
Documented new first_name and last_name boolean disclosure options in SDK README tables.

Sequence Diagram(s)

(omitted — changes are type additions, small control-flow export, and tests; no multi-component sequential flow requiring visualization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • App/eu id updates #638 — Modifies disclosure/identity-related utilities and types; strong overlap in SelfApp and selector/hash utilities.

Suggested reviewers

  • remicolin
  • transphorm

Poem

✨ Two name bits join the interface line,
First and last now optionally shine.
Tests hum a tune, selectors step light,
Docs nod along — a small, steady flight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding first_name and last_name disclosure flags across the SDKs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sdk/sdk-common/index.ts (1)

268-281: Multiple SDK packages have duplicate interface definitions that are already drifting.

SelfAppDisclosureConfig and SelfApp are duplicated in common/src/utils/appType.ts, sdk/sdk-common/index.ts, and sdk/qrcode-angular/src/lib/common.ts. The SelfApp interface in both SDK packages is missing the selfDefinedData field that exists in the common package and is actively used in the proving logic. This drift creates maintenance overhead and risks breaking changes as new fields are added.

Re-export these interfaces from the common package in SDK modules to maintain a single source of truth and prevent future inconsistencies.

🧹 Nitpick comments (1)
sdk/qrcode-angular/README.md (1)

157-158: Consider noting that these fields are not yet supported by the app.

Per the PR description, first_name and last_name flags are not yet supported by the mobile app. Adding a note (e.g., "⚠️ Coming soon" or similar) would help developers avoid confusion when these fields don't produce expected behavior.

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.

2 participants