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

fix: swap fees are not displayed if route estimated time is nil #22001

Merged
merged 1 commit into from
Jan 30, 2025
Merged

fix: swap fees are not displayed if route estimated time is nil #22001

merged 1 commit into from
Jan 30, 2025
+13 −15

Conversation

briansztamfater
Copy link
Member

@briansztamfater briansztamfater commented Jan 29, 2025 •
edited
Loading

fixes #21948

Summary

This PR fixes fees for swap transactions not being properly displayed (approvals and swaps themselves) when estimated-time comes nil from status-go. Also hides Estimated time if it comes with a nil value.

Before

After


Platforms

  • Android
  • iOS

Areas that may be impacted

Functional

  • wallet / transactions

Steps to test

  • Open Status
  • Try to perform a Swap with an ERC-20 token that required approval
  • Go to set spending cap confimation page
  • Check max fee and estimated time
    Also:
  • Go to swap confirmation page
  • Check max fee and estimated time

status: ready

@briansztamfater briansztamfater added request-manual-qa wallet-core Issues for mobile wallet team labels Jan 29, 2025
@briansztamfater briansztamfater added this to the 2.33.0 milestone Jan 29, 2025
@briansztamfater briansztamfater self-assigned this Jan 29, 2025
@status-im-auto
Copy link
Member

status-im-auto commented Jan 29, 2025 •
edited
Loading

Jenkins Builds

Click to see older builds (4)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 3de2ca6 #1 2025-01-29 17:39:37 ~4 min tests 📄log
✔️ 3de2ca6 #1 2025-01-29 17:43:04 ~7 min android-e2e 🤖apk 📲
✔️ 3de2ca6 #1 2025-01-29 17:43:30 ~8 min android 🤖apk 📲
✔️ 3de2ca6 #1 2025-01-29 17:46:02 ~10 min ios 📱ipa 📲
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 027b481 #3 2025-01-29 18:31:30 ~4 min tests 📄log
✔️ 027b481 #3 2025-01-29 18:34:48 ~7 min android-e2e 🤖apk 📲
✔️ 027b481 #3 2025-01-29 18:35:28 ~8 min android 🤖apk 📲
✔️ 027b481 #3 2025-01-29 18:38:13 ~11 min ios 📱ipa 📲
✔️ 0edfa1b #4 2025-01-30 12:59:25 ~4 min tests 📄log
✔️ 0edfa1b #4 2025-01-30 13:01:43 ~6 min android-e2e 🤖apk 📲
✔️ 0edfa1b #4 2025-01-30 13:03:16 ~8 min android 🤖apk 📲
✔️ 0edfa1b #4 2025-01-30 13:05:13 ~10 min ios 📱ipa 📲

@briansztamfater briansztamfater force-pushed the fix/swap-fees-not-displayed-if-estimated-time-is-nil branch 2 times, most recently from 4fc77e7 to 027b481 Compare January 29, 2025 18:26
@shivekkhurana
Copy link
Contributor

Hi, can you post the before an after screens? So the UI changes are logged.

@mariia-skrypnyk mariia-skrypnyk self-assigned this Jan 30, 2025
@status-im-auto
Copy link
Member

100% of end-end tests have passed

Total executed tests: 10
Failed tests: 0
Expected to fail tests: 0
Passed tests: 10

Passed tests (10)

Click to expand

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
2. test_wallet_swap_flow_mainnet, id: 741555
3. test_wallet_balance_mainnet, id: 740490
4. test_wallet_send_flow_mainnet, id: 741554

@status-im-auto
Copy link
Member

100% of end-end tests have passed

Total executed tests: 10
Failed tests: 0
Expected to fail tests: 0
Passed tests: 10

Passed tests (10)

Click to expand

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
2. test_wallet_swap_flow_mainnet, id: 741555
3. test_wallet_balance_mainnet, id: 740490
4. test_wallet_send_flow_mainnet, id: 741554

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions

@mariia-skrypnyk
Copy link

Hi @briansztamfater!

Testing Summary:

Objective: Verified fix for bug with swap fees and estimated time display when nil.
Platforms: iOS, Android.
Tokens: Tether on Optimism, USDC on Mainnet/Optimism.
Steps:

  1. Checked fee and estimated time display (hidden when nil) :
  • set spending cap confirmation page
  • swap confirmation page
  1. Verified swap success and send functionality on Optimism.
    Outcome: Issue fixed, all tested features working correctly.

E2E ok.
PR can be merged!

@briansztamfater
fix: swap fees are not displayed if route estimated time is nil
0edfa1b 
Signed-off-by: Brian Sztamfater <brian@status.im>
@briansztamfater briansztamfater force-pushed the fix/swap-fees-not-displayed-if-estimated-time-is-nil branch from 027b481 to 0edfa1b Compare January 30, 2025 12:54
@briansztamfater
Copy link
Member Author

Hi, can you post the before an after screens? So the UI changes are logged.

@shivekkhurana updated description with the screenshots

@mariia-skrypnyk Thanks for testing, will proceed to merge 👍

@briansztamfater briansztamfater merged commit 7289940 into develop Jan 30, 2025
5 checks passed
@briansztamfater briansztamfater deleted the fix/swap-fees-not-displayed-if-estimated-time-is-nil branch January 30, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smohamedjavid smohamedjavid smohamedjavid approved these changes

@alwx alwx Awaiting requested review from alwx

@shivekkhurana shivekkhurana Awaiting requested review from shivekkhurana

@vkjr vkjr Awaiting requested review from vkjr

@clauxx clauxx Awaiting requested review from clauxx

@mohsen-ghafouri mohsen-ghafouri Awaiting requested review from mohsen-ghafouri

Assignees

@briansztamfater briansztamfater

@mariia-skrypnyk mariia-skrypnyk

Labels
request-manual-qa Tested - OK wallet-core Issues for mobile wallet team
Projects
Pipeline for QA
Status: DONE
Status Desktop/Mobile Board
Status: Done
Milestone
2.33.0
Development

Successfully merging this pull request may close these issues.

Estimated time and max fee shown as "unknown" on approve and swap confirmation page
5 participants
@briansztamfater @status-im-auto @shivekkhurana @mariia-skrypnyk @smohamedjavid