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

ABW-1204 Assets appear to not transfer #565

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Conversation

kugel3
Copy link
Contributor

@kugel3 kugel3 commented Jun 8, 2023

JIRA ticket: ABW-1204

Description

When transferring assets, the asset details view was not reloaded automatically, which made it look like the transaction failed. This is fixed now. Also fixes the issue that NFT resources show up even if we have 0 tokens for them.

Video

RPReplay_Final1686217952.MP4

PR submission checklist

  • I have tested account to account transfer flow and have confirmed that it works

@kugel3 kugel3 changed the title Assets appear to not transfer ABW-1204 Assets appear to not transfer Jun 8, 2023
Task.detached {
try await hasTXBeenCommittedSuccessfully(txID)
if let changedAccounts {
// FIXME: Ideally we should only have to call the cacheClient here
Copy link
Contributor

Choose a reason for hiding this comment

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

well not necessarily, the accountsPortfolioClient does not react to cache changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well that is what I mean. I can't simply invalidate the cache since that is not enough to trigger the portfolio fetching, but it would be nicer if it did, because I don't think it's good to have to call accountsPortfolioClient from here.

let changedAccounts: [Profile.Network.Account.EntityAddress]?

do {
let decompiledNotarized = try engineToolkitClient.decompileNotarizedTransactionIntent(.init(
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not quite nice, but so be it for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part was already here, for the debug print. But is there another way to get the addresses?

@kugel3 kugel3 merged commit 8a76a0c into main Jun 8, 2023
@kugel3 kugel3 deleted the Assets-appear-to-not-transfer branch June 8, 2023 11:11
This pull request was closed.
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.

2 participants