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

Update dapp request handling logic to properly determine starting screen #1119

Merged
merged 4 commits into from
Aug 6, 2024

Conversation

jakub-rdx
Copy link
Contributor

@jakub-rdx jakub-rdx commented Aug 2, 2024

Description

Re-arranged how we determine starting screen for request handling. Now it is as follow, when we hit one of those conditions, we don't examine the rest (when clause):

  • if there is ongoing accounts request that is not reset and we didn't grant account access to this type of request for the dapp - we start with account permission screen
  • if there is ongoing persona data request that is not reset and we didn't grant data access to this type of request for the dapp - we start with ongoing persona data screen
  • if there is one time accounts request - start with chosing account screen for one time data
  • if there is one time persona data request - start with persona data one time screen
    If none of the above are met, we have a request with either ongoing account and (or) persona data and we already granted data to such request in the past - ask for biometrics and complete request handling.

How to test

  1. Test scenarios described in the ticket description
  2. Test various random dapp requests using sandbox - it should work as it was before the changes

PR submission checklist

  • I have tested scenarios described in the ticket
  • I have tested data requests/one time requests using sandbox.

@jakub-rdx jakub-rdx marked this pull request as ready for review August 2, 2024 14:53
@jakub-rdx jakub-rdx changed the title Update dapp request handling logic to better handle situation when da… Update dapp request handling logic to properly determine starting screen Aug 5, 2024
Copy link
Contributor

@giannis-rdx giannis-rdx left a comment

Choose a reason for hiding this comment

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

Take a look at the one time data request with persona data.
If I send a plain Persona data one time request the wallet doesn't open the request. ❌

Copy link
Contributor

@sergiupuhalschi-rdx sergiupuhalschi-rdx left a comment

Choose a reason for hiding this comment

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

Works fine!

If none of the above are met, we have a request with either ongoing account and (or) persona data and we already granted data to such request in the past - ask for biometrics and complete request handling.

I'm not sure if we have to do anything about this or it works as expected but in this scenario I don't see the biometrics prompt, it goes straight to the success sheet.

@jakub-rdx
Copy link
Contributor Author

Works fine!

If none of the above are met, we have a request with either ongoing account and (or) persona data and we already granted data to such request in the past - ask for biometrics and complete request handling.

I'm not sure if we have to do anything about this or it works as expected but in this scenario I don't see the biometrics prompt, it goes straight to the success sheet.

You probably used requests without a proof, if there is a request with proof and signature needed, biometric prompt should appear

Copy link
Contributor

@giannis-rdx giannis-rdx left a comment

Choose a reason for hiding this comment

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

Copy link

sonarcloud bot commented Aug 6, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarCloud

@jakub-rdx jakub-rdx merged commit b26d74c into main Aug 6, 2024
8 of 9 checks passed
@jakub-rdx jakub-rdx deleted the fix/ABW-3662-update-dapp-request-handling branch August 6, 2024 08:45
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.

3 participants