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

fix: token duplication on dashboard #431

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Conversation

alexruzenhack
Copy link
Contributor

@alexruzenhack alexruzenhack commented Feb 16, 2024

Issue

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

@pedroferreira1 has investigated the these of migration be the culprit and has confirmed it. See the investigation note.

Despite we didn't need a fix anymore, we will proceed to avoid this problem in the future.

Acceptance Criteria

  • Should fix token duplication by replacing token list to token map

The following recording shows operations with token, register, unregister and creation to demonstrate these operations were not effected by the fix/refactoring:

RocketSim_Recording_iPhone_15_6.1_2024-02-16_14.37.24.mp4

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 Feb 16, 2024
@alexruzenhack alexruzenhack added the bug Something isn't working label Feb 16, 2024
* @param {unknown} props.header
* @param {boolean} props.renderArrow
* @param {function} props.onItemPress
*/
const TokenSelect = (props) => {
const tokens = Object.values(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.

Suggested change
const tokens = Object.values(props.tokens);
const tokens = getRegisteredTokenUids(props);

Copy link
Member

Choose a reason for hiding this comment

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

He is getting the token objects here. The method getRegisteredTokenUids gets only the uid, if I'm not wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct, @pedroferreira1. We should stick with the object here, not the uid.

* @param {unknown} props.header
* @param {boolean} props.renderArrow
* @param {function} props.onItemPress
*/
const TokenSelect = (props) => {
const tokens = Object.values(props.tokens);
Copy link
Member

Choose a reason for hiding this comment

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

He is getting the token objects here. The method getRegisteredTokenUids gets only the uid, if I'm not wrong.

@alexruzenhack alexruzenhack merged commit 527a75d into master Mar 4, 2024
2 checks passed
@alexruzenhack alexruzenhack deleted the fix/duplicate-htr-item branch March 4, 2024 19:22
This was referenced Mar 6, 2024
This was referenced May 9, 2024
alexruzenhack added a commit that referenced this pull request May 15, 2024
* fix: token duplication on dashboard

* lint: comply with rules
@alexruzenhack alexruzenhack mentioned this pull request May 15, 2024
alexruzenhack added a commit that referenced this pull request May 15, 2024
* fix: token duplication on dashboard

* lint: comply with rules
alexruzenhack added a commit that referenced this pull request May 15, 2024
* fix: token duplication on dashboard

* lint: comply with rules
alexruzenhack added a commit that referenced this pull request May 15, 2024
* fix: token duplication on dashboard

* lint: comply with rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants