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

Chore/native/use add coin returns error #13214

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

vytick
Copy link
Contributor

@vytick vytick commented Jul 4, 2024

Description

  • addAndDiscoverNetworkAccountThunk now rejects or fulfils with value
  • fixed typo in component name

Related Issue

Resolve #13163

@vytick vytick added the mobile Suite Lite issues and PRs label Jul 4, 2024
@vytick vytick requested a review from juriczech July 4, 2024 20:51
@vytick vytick requested a review from a team as a code owner July 4, 2024 20:51
Copy link
Contributor

@juriczech juriczech left a comment

Choose a reason for hiding this comment

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

This is not working as expected. First we need to refactor fetchBundleDescriptorsThunk and it's return value from connect. The thunk returns empty array if no bundle was found (if there was error). We need to change that so that it doesn't return empty array but an error. Than we propagate the error to the callers.

You've actually made matters worse a little bit since add coin on develop returns at least basic error, now it actually doesn't do anything and just hangs on a loading screen.

If you'd like, maybe we could huddle and I might explain better looking at the code.

@vytick vytick force-pushed the chore/native/use-add-coin-returns-error branch from 31838cc to fb5a9c3 Compare July 8, 2024 13:16
@vytick vytick requested a review from juriczech July 8, 2024 14:01
@vytick
Copy link
Contributor Author

vytick commented Jul 9, 2024

/rebase

Copy link

github-actions bot commented Jul 9, 2024

@trezor-ci trezor-ci force-pushed the chore/native/use-add-coin-returns-error branch from 79f1c26 to 2b8d54e Compare July 9, 2024 19:38
@vytick vytick requested review from juriczech and removed request for juriczech July 9, 2024 19:39
@vytick vytick force-pushed the chore/native/use-add-coin-returns-error branch from 2b8d54e to fe64724 Compare July 10, 2024 12:34
Copy link
Contributor

@juriczech juriczech left a comment

Choose a reason for hiding this comment

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

LGTM. Great job 💪

@vytick vytick merged commit f0f2abb into develop Jul 11, 2024
13 checks passed
@vytick vytick deleted the chore/native/use-add-coin-returns-error branch July 11, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Suite Lite issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use add account receives an actual error
2 participants