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.

Sign up for GitHub

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

Jump to bottom

Gh 1994 - Show pending step, show up to date values #2001

Merged
merged 3 commits into from
Aug 31, 2023
Merged

Gh 1994 - Show pending step, show up to date values #2001

merged 3 commits into from
Aug 31, 2023

Conversation

biafra23
Copy link
Contributor

Handles #1994

Changes proposed in this pull request:

  • Show pending step
  • Use different id to retrieve uncached tx details

@gnosis/mobile-devs

MouazAlzahabi
MouazAlzahabi reviewed Aug 31, 2023
View reviewed changes
app/src/main/java/io/gnosis/safe/ui/safe/send_funds/SendAssetReviewViewModel.kt
amountString,
selectedAsset.symbol
chain = activeSafe.chain,
txId = safeTxHash, // to get non cached updates later we need the txId not the hash
Copy link

Choose a reason for hiding this comment

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

I got confused by the naming here, txId and safeTxHash

Copy link

Choose a reason for hiding this comment

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

Are we sure that it's caching problem? why we don't have such on iOS ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because ios uses the txId:

txIds look likes this: multisig_0xfcFab1Eb61d3b1E8ff8F1eF1CD8e46F8be30b77f_0xc958935d43d24337ff7a425445ad05ca579231edf98eab9dd008db8447751f7d

txHashes look like this: 0xc958935d43d24337ff7a425445ad05ca579231edf98eab9dd008db8447751f7d

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But i didn't look into ios regarding the first time txId should be passed. In Android we do not know the txId at the time. It is returned in the txDetails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is an example request: https://safe-client.staging.5afe.dev/v1/chains/5/transactions/9f157207875105636c594bcce3ebea3eb5e0704f00cf64f59e3d286bc4258189

Copy link

Choose a reason for hiding this comment

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

I'm actually not sure about this change if it's correct or not,

@biafra23 biafra23 self-assigned this Aug 31, 2023
@biafra23 biafra23 merged commit 616ea14 into release Aug 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2023
@biafra23 biafra23 deleted the GH-1994/show-all-steps branch September 1, 2023 07:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@the-real-chrizzo the-real-chrizzo the-real-chrizzo approved these changes

@MouazAlzahabi MouazAlzahabi MouazAlzahabi approved these changes

Assignees

@biafra23 biafra23

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@biafra23 @MouazAlzahabi @the-real-chrizzo