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

Add send collectible fee validation (and small send collectible fixes) #22232

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

clauxx
Copy link
Member

@clauxx clauxx commented Mar 4, 2025

fixes #22213

Summary

  1. when not enough fee assets to send collectible:
    a. show the "Not enough assets to pay gas fees" alert banner
    b. highlight the fee
    c. disable the slider
  2. when no routes were found to send collectible:
    a. show the "No routes found ..." alert banner
    b. disable the slider
  3. while fetching routes for collectible, the footer should be consistent with the other flows, i.e.:
    a. removed activity loader
    b. show transaction details banner (est. time, max fees, recipient gets) in loading state
    c. show disabled slide button
  4. show the network summary when sending collectible (can be shown only after the routes were loaded)
  5. fixed the collectible name in the title overflowing outside the screen if it's too long (with ellipsis).
  6. show the estimated time (wasn't shown in any flow for some reason)
    a. instead of the exact minutes (e.g. 15 min), show the intervals (e.g. 1-2 min, 3-5 min, >5 min) like we do for dapp transactions

status: ready

@clauxx clauxx added the wallet-core Issues for mobile wallet team label Mar 4, 2025
@clauxx clauxx self-assigned this Mar 4, 2025
Comment on lines +47 to +58
(let [chosen-route (:best suggested-routes-data)
{:keys [token collectible token-display-name
receiver-network-values
sender-network-values tx-type]} (get-in db [:wallet :ui :send])
token-decimals (if collectible 0 (:decimals token))
native-token? (and token (= token-display-name "ETH"))
to-network-amounts-by-chain (send-utils/network-amounts-by-chain
{:route chosen-route
:token-decimals token-decimals
:native-token? native-token?
:receiver? true})]
Copy link
Member Author

Choose a reason for hiding this comment

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

the only change in this event is moving these definitions above the enough-assets? condition, so that we get the network information if there's no assets also.

@status-im-auto
Copy link
Member

status-im-auto commented Mar 4, 2025

Jenkins Builds

Click to see older builds (4)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 742df08 #1 2025-03-04 13:10:46 ~5 min tests 📄log
✔️ 742df08 #1 2025-03-04 13:12:55 ~7 min android-e2e 🤖apk 📲
✔️ 742df08 #1 2025-03-04 13:14:47 ~9 min android 🤖apk 📲
✔️ 742df08 #1 2025-03-04 13:17:57 ~12 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b1611b0 #2 2025-03-05 15:32:40 ~5 min tests 📄log
✔️ b1611b0 #2 2025-03-05 15:34:11 ~7 min android-e2e 🤖apk 📲
✔️ b1611b0 #2 2025-03-05 15:36:32 ~9 min android 🤖apk 📲
✔️ b1611b0 #2 2025-03-05 15:38:28 ~11 min ios 📱ipa 📲
✔️ 4b32d9d #4 2025-03-07 10:52:15 ~4 min tests 📄log
✔️ 4b32d9d #4 2025-03-07 10:55:34 ~8 min android-e2e 🤖apk 📲
✔️ 4b32d9d #4 2025-03-07 10:55:45 ~8 min android 🤖apk 📲
✔️ 4b32d9d #4 2025-03-07 10:59:04 ~11 min ios 📱ipa 📲

@status-im-auto
Copy link
Member

71% of end-end tests have passed

Total executed tests: 14
Failed tests: 4
Expected to fail tests: 0
Passed tests: 10
IDs of failed tests: 741841,741839,741840,741554 

Failed tests (4)

Click to expand
  • Rerun failed tests

  • Class TestWalletCollectibles:

    1. test_wallet_collectible_send_from_expanded_info_view, id: 741841

    Device 1: Find `Button` by `accessibility id`: `collectibles-tab`

    critical/wallet/test_collectibles.py:149: in test_wallet_collectible_send_from_expanded_info_view
        self.wallet_view.collectibles_tab.click()
    ../views/base_element.py:89: in click
        element = self.find_element()
    ../views/base_element.py:78: in find_element
        raise NoSuchElementException(
     Device 1: Button by accessibility id: `collectibles-tab` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
    



    2. test_wallet_collectibles_balance, id: 741839

    # STEP: Check BVL collectible info and image
    # STEP: Check Glitch Punks collectible info and image

    critical/wallet/test_collectibles.py:94: in test_wallet_collectibles_balance
        self.errors.verify_no_errors()
    base_test_case.py:179: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Device 1: Collectible 'BVL' is not displayed
    E    Device 1: Collectible 'Glitch Punks' is not displayed
    



    3. test_wallet_send_collectible, id: 741840

    Device 1: Find SlideButton by xpath: //*[@resource-id='slide-button-track']
    Device 1: Click system back button

    critical/wallet/test_collectibles.py:144: in test_wallet_send_collectible
        self.errors.verify_no_errors()
    base_test_case.py:179: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Device 1: Est. time text 3-5 min doesn't match expected  min on the Review Send page
    



    Class TestWalletOneDevice:

    1. test_wallet_send_flow_mainnet, id: 741554

    Device 1: Swiping right on element SlideButton
    Device 1: Find SlideButton by xpath: //*[@resource-id='slide-button-track']

    critical/wallet/test_wallet_mainnet.py:154: in test_wallet_send_flow_mainnet
        self.errors.verify_no_errors()
    base_test_case.py:179: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Device 1: Ether on Mainnet: Est. time text >5 min doesn't match expected  min on the Review Send page
    E    Device 1: Ether on Arbitrum: Est. time text 3-5 min doesn't match expected  min on the Review Send page
    E    Device 1: Status on Mainnet: Est. time text >5 min doesn't match expected  min on the Review Send page
    E    Device 1: Status on Optimism: Est. time text 3-5 min doesn't match expected  min on the Review Send page
    



    Passed tests (10)

    Click to expand

    Class TestWalletOneDevice:

    1. test_wallet_swap_flow_mainnet, id: 741555
    2. test_wallet_add_remove_regular_account, id: 727231
    3. test_wallet_bridge_flow_mainnet, id: 741612
    4. test_wallet_balance_mainnet, id: 740490

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843
    Device sessions

    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 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

    @clauxx clauxx force-pushed the cl-22213-send-collectible-bugs branch from b1611b0 to 3c4f2e2 Compare March 7, 2025 10:46
    @pavloburykh pavloburykh self-assigned this Mar 7, 2025
    @pavloburykh
    Copy link
    Contributor

    @clauxx thanks for the PR! Please take a look at the issues below.

    ISSUE 1 Fee validation is cleared after changing fee settings

    Preconditions: User has Account A which holds some collectibles but 0 eth (no eth to cover fee)

    Steps:

    1. Initiate collectible send flow from Account A
    2. Proceed to confirmation screen (fee validation should be applied)
    3. Change transaction fee setting (select another fee value)
    4. See if validation is still applied

    Actual result: validation is reset. Slider is active and user can proceed with signing transaction.

    telegram-cloud-document-2-5366074879118437717.mp4

    @pavloburykh
    Copy link
    Contributor

    pavloburykh commented Mar 10, 2025

    ISSUE 2 Est time is not shown on confirmation screen

    Steps:

    1. Proceed to transaction confirmation screen. It can be any type of transaction (send/swap/bridge)
    2. See if Est time is shown

    Actual result: Est time is not shown. Previously (in current develop) we have shown Est time, but there was no value which is also not okay. So from one point removing Est time seems to be a good temporary solution until we figure out which value we should pass there. But from PR description I see that the intention was to pass a valid value, that's why I am logging current behaviour as a bug.

    show the estimated time (wasn't shown in any flow for some reason)
    a. instead of the exact minutes (e.g. 15 min), show the intervals (e.g. 1-2 min, 3-5 min, >5 min) like we do for dapp transactions

    photo_2025-03-10 16 46 42

    Expected result: I am not sure what is the expected result here and which Est time value should we show. Should this value come from backend using the same method we use for dapp transactions or here we should use some other method? Or should we use hardcoded value of Est time?

    When we select fee in transaction setting, the time value seems to be hardcoded there (see screenshot below). Should we user the same hardcoded values from transaction settings on confirmation screen or rely on some estimated time that is calculated on the backend?

    photo_2025-03-10 16 46 38

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: IN TESTING
    Status: Code Review
    9 participants