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

Hotfix 1.11.1 #1279

Closed

Conversation

sergiupuhalschi-rdx
Copy link
Contributor

@sergiupuhalschi-rdx sergiupuhalschi-rdx commented Dec 11, 2024

Description

  • Include hidden assets for account deletion
    • Hide all assets for an account
    • Tap delete account from account settings
    • Check that we let the user to move their hidden assets from the account to be deleted to another one
  • Tx review small improvements
    • Fixed the issue of the tx review screen not being dismissed when an error which is not a subtype of DappRequestException, for example ReceivingAccountDoesNotAllowDeposits is encountered.
  • Non-enclosed pre-auth classification handling

Comment on lines +278 to +279
incomingRequestRepository.requestHandled(args.interactionId)
sendEvent(Event.Dismiss)
Copy link
Contributor

Choose a reason for hiding this comment

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

tbh i remember having some issues in the past with the order of those two but I can't remember what was exactly and if it was for transaction review screen or for the dapplogin screens.

is there any way we can test this?
Did you have the chance to test it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap, I tested this, you can test it too by following these steps: disable 3rd party deposits for an account, go to another account settings with assets and choose to delete it -> when asked to move assets to another account choose the one with the 3rd party deposits disallowed and tap continue -> you're going to see the tx review screen with an error -> when tapping it it goes through this code which dismisses the screen and marks the request as handled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, you can see in TransactionSubmitDelegate.onDismiss that the Dismiss event is sent before marking the request as handled, just like here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although I agree that it would make more sense first to mark the request as handled and then sending the dismiss event.

Copy link
Contributor

@giannis-rdx giannis-rdx left a comment

Choose a reason for hiding this comment

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

it works!

@sergiupuhalschi-rdx sergiupuhalschi-rdx changed the title Include hidden assets for account deletion. Tx review small improvements Hotfix 1.11.1 Dec 17, 2024
Copy link
Contributor

@giannis-rdx giannis-rdx left a comment

Choose a reason for hiding this comment

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

🚢 it

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube Cloud

@sergiupuhalschi-rdx sergiupuhalschi-rdx changed the base branch from main to hotfix/1.11.1 December 17, 2024 16:28
@sergiupuhalschi-rdx sergiupuhalschi-rdx changed the base branch from hotfix/1.11.1 to main December 17, 2024 16:47
@sergiupuhalschi-rdx
Copy link
Contributor Author

Closing in favor of #1286 to have the changes on top of the 1.11.0 tag commit.

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