Skip to content

fix: Unable to send token by reading a QR code #507

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

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

alexruzenhack
Copy link
Contributor

@alexruzenhack alexruzenhack commented Jul 11, 2024

Context

I have fixed token duplication on the Dashboard screen by transforming the registered tokens list into a map, this way we would never have a duplication. However, I failed to cover the necessary change and left one case untouched, which caused this bug.

See the previous fix at: #431

Acceptance Criteria

  • It should restore the ability to read QR code when sending token

Closes: https://github.com/HathorNetwork/internal-issues/issues/345

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@alexruzenhack alexruzenhack self-assigned this Jul 11, 2024
@alexruzenhack alexruzenhack requested a review from tuliomir July 11, 2024 20:42
@alexruzenhack alexruzenhack changed the title fix: unable to read QR code fix: Unable to send token by reading a QR code Jul 11, 2024
(stateToken) => stateToken.uid === qrcode.token.uid
);
if (isTokenRegistered) {
if (qrcode.token.uid in this.props.tokens) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion(non-blocking): Add a comment explaining that this is a search into the registered tokens.

Otherwise the reader would have to search what the props.tokens mean to understand what this rule is about. This was made clearer before by the old variable name itself.

@alexruzenhack alexruzenhack merged commit b96ec18 into release Jul 15, 2024
0 of 2 checks passed
@alexruzenhack alexruzenhack deleted the fix/unable-to-read-qrcode branch July 15, 2024 18:24
@alexruzenhack alexruzenhack mentioned this pull request Jul 31, 2024
6 tasks
@alexruzenhack alexruzenhack mentioned this pull request Aug 12, 2024
1 task
andreabadesso pushed a commit that referenced this pull request Aug 21, 2024
* fix: token registered condition after identify the qr code token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants