Skip to content
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

feat: dcql and a lot of UI stuff #227

Merged
merged 6 commits into from
Nov 25, 2024
Merged

feat: dcql and a lot of UI stuff #227

merged 6 commits into from
Nov 25, 2024

Conversation

TimoGlastra
Copy link
Member

This PR grew a lot bigger than anticipated but it hasA LOT of great improvements:

  • Add DCQL support (some issues with showing disclosues, but flow is working and the logic is there)
  • Remove all the custom pid display rendering. Instead it's now stored on the record as metadata
  • Clean up a lot of the hacky display logic. It's way more unified now
  • Pin working in all flows based on whether eID is used (todo in next pr: handle invalid pin, need to send 403 or some way to show in the wallet that the pin is invaild)

Copy link
Contributor

@janrtvld janrtvld left a comment

Choose a reason for hiding this comment

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

nice

apps/easypid/src/agent/initialize.ts Outdated Show resolved Hide resolved
@@ -132,6 +139,12 @@ export function FunkeCredentialNotificationScreen() {
[agent]
)

// TODO: Should we add this to the activitiy? We also don't do it for issuance
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay. Reason for the comment is that i see this more as a failed issuance. But yeah if that's also on the list.

We don't support failed issuance though? And just adding this as failed proof request is weird i think as it's during issuance

// We only have the attribute paths, no way to know how to render
// TODO: we could look at the vct?
// TODO: we should maybe support partial matches (i.e. vct matches), as then we can
// show a much better UI (you have the cred, but age is not valid, or this param is missing)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean then rendering the credential card but with 'date_of_birth' in red when it doesn't match the required age?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes exactly!

@@ -67,7 +76,7 @@ export class ReceivePidUseCaseCFlow extends ReceivePidUseCaseFlow {
resolvedCredentialOffer: this.resolvedCredentialOffer,
credentialConfigurationIdsToRequest,
clientId: ReceivePidUseCaseCFlow.CLIENT_ID,
requestBatch: getShouldUseCloudHsm() ? 10 : false,
requestBatch: getShouldUseCloudHsm() ? 2 : false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this for testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Need to update WSP. It doesn't batch soo is way too slow at 10

@@ -93,7 +94,7 @@ export class RefreshPidUseCase {
resolvedCredentialOffer: this.resolvedCredentialOffer,
credentialConfigurationIdsToRequest,
clientId: RefreshPidUseCase.CLIENT_ID,
requestBatch: 10,
requestBatch: 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

packages/ui/src/base/Headings.tsx Outdated Show resolved Hide resolved
@TimoGlastra TimoGlastra marked this pull request as ready for review November 25, 2024 13:36
@TimoGlastra TimoGlastra merged commit d02e10d into main Nov 25, 2024
1 check passed
@TimoGlastra TimoGlastra deleted the feat/mdcql branch November 25, 2024 13:36
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