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

[#20472] hide account switcher in send flow #20892

Merged
merged 7 commits into from
Sep 9, 2024

Conversation

ulisesmac
Copy link
Contributor

fixes #20472

Important

This PR depends on the work done for:

So please, do the testing after that PR is merged

Summary

This PR hides the account switcher while the user is sending a collectible from any entry point.

Demo for ERC-721 for any entry point:

Screencast.from.2024-07-25.14-28-41.mp4

Demo for ERC-1155 for any entry point:

Screencast.from.2024-07-25.14-31-20.mp4

Additionally, this PR solves an existing bug in develop where the user was unable to pick the collectible amount for ERC-1155 collectibles:

Develop:

bug-develop.mp4

This PR:

solved-in-PR.mp4

Review notes

We should extend and improve the wizard mechanism, checking for something like "the user sending a collectible" has become a task hard to perform, since there are multiple flows depending on re-frame DB values. Depending on designs, we should design a new mechanism.

Testing notes

Please carefully test these flows to make sure nothing is broken, there are many possible cases, they were showed in the Demo videos, but there may be something forgotten.

Platforms

  • Android
  • iOS

Steps to test

  • Open Status
  • Please check the Demo videos and the issue description.

status: ready

@ulisesmac ulisesmac self-assigned this Jul 25, 2024
@@ -22,7 +22,7 @@
:skip-step? (fn [db] (or (token-selected? db) (collectible-selected? db)))}
{:screen-id :screen/wallet.send-input-amount
:skip-step? (fn [db]
(send-utils/tx-type-collectible? (get-in db [:wallet :ui :send :tx-type])))}
(-> db :wallet :ui :send :tx-type send-utils/tx-type-collectible?))}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just small code refactor

@status-im-auto
Copy link
Member

status-im-auto commented Jul 25, 2024

Jenkins Builds

Click to see older builds (17)
Commit #️⃣ Finished (UTC) Duration Platform Result
6bfe2a1 #1 2024-07-25 20:43:26 ~3 min tests 📄log
d61fb49 #2 2024-07-25 20:49:56 ~3 min tests 📄log
✔️ d61fb49 #2 2024-07-25 20:54:30 ~8 min android-e2e 🤖apk 📲
✔️ d61fb49 #2 2024-07-25 20:54:31 ~8 min android 🤖apk 📲
✔️ d61fb49 #2 2024-07-25 20:56:21 ~9 min ios 📱ipa 📲
bf290d1 #3 2024-08-01 21:35:05 ~3 min tests 📄log
✔️ bf290d1 #3 2024-08-01 21:38:36 ~6 min android-e2e 🤖apk 📲
✔️ bf290d1 #3 2024-08-01 21:38:51 ~6 min android 🤖apk 📲
✔️ bf290d1 #3 2024-08-01 21:47:29 ~15 min ios 📱ipa 📲
352c61d #4 2024-08-28 14:56:42 ~3 min tests 📄log
✔️ 352c61d #4 2024-08-28 15:00:34 ~6 min android 🤖apk 📲
✔️ 352c61d #4 2024-08-28 15:01:18 ~7 min android-e2e 🤖apk 📲
352c61d #4 2024-08-28 15:19:10 ~25 min ios 📄log
7901d3d #5 2024-09-04 13:26:48 ~3 min tests 📄log
✔️ 7901d3d #5 2024-09-04 13:30:21 ~6 min android-e2e 🤖apk 📲
✔️ 7901d3d #5 2024-09-04 13:31:25 ~7 min android 🤖apk 📲
✔️ 7901d3d #5 2024-09-04 13:40:07 ~16 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 98ab958 #6 2024-09-07 08:52:06 ~6 min android-e2e 🤖apk 📲
✔️ 98ab958 #6 2024-09-07 10:14:06 ~11 min ios 📱ipa 📲
✔️ 98ab958 #6 2024-09-07 10:49:09 ~6 min android 🤖apk 📲
✔️ 98ab958 #6 2024-09-07 11:05:44 ~4 min tests 📄log
✔️ a244dba #7 2024-09-09 20:07:22 ~4 min tests 📄log
✔️ a244dba #7 2024-09-09 20:09:20 ~6 min android-e2e 🤖apk 📲
✔️ a244dba #7 2024-09-09 20:10:53 ~7 min android 🤖apk 📲
✔️ a244dba #7 2024-09-09 20:14:58 ~11 min ios 📱ipa 📲

@ulisesmac ulisesmac force-pushed the 20472-Hide-account-switcher-sned-flow branch from 6bfe2a1 to d61fb49 Compare July 25, 2024 20:46
src/test_helpers/unit.cljs Outdated Show resolved Hide resolved
@ulisesmac ulisesmac force-pushed the 20472-Hide-account-switcher-sned-flow branch from d61fb49 to bf290d1 Compare August 1, 2024 21:31
@churik churik force-pushed the 20472-Hide-account-switcher-sned-flow branch from bf290d1 to 352c61d Compare August 28, 2024 14:53
@VolodLytvynenko VolodLytvynenko self-assigned this Sep 4, 2024
@VolodLytvynenko VolodLytvynenko force-pushed the 20472-Hide-account-switcher-sned-flow branch from 352c61d to 7901d3d Compare September 4, 2024 13:23
@status-im-auto
Copy link
Member

57% of end-end tests have passed

Total executed tests: 7
Failed tests: 3
Expected to fail tests: 0
Passed tests: 4
IDs of failed tests: 727230,702843,727229 

Failed tests (3)

Click to expand
  • Rerun failed tests

  • Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230

    Device 1: `Text` is `0.43982 ETH`
    Device 2: `Text` is `0.04779 ETH`

    critical/test_wallet.py:190: in test_wallet_send_asset_from_drawer
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.4398 but expected to be 0.4399
    



    2. test_wallet_send_eth, id: 727229

    Device 2: Text is 0.04769 ETH
    Device 1: Text is 0.44003 ETH

    critical/test_wallet.py:159: in test_wallet_send_eth
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.44 but expected to be 0.4401
    



    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Device 2: Looking for a message by text: Message AFTER edit 2 (Edited)
    Device 2: Find ChatElementByText by xpath: //*[starts-with(@text,'Message AFTER edit 2 (Edited)')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']

    critical/chats/test_public_chat_browsing.py:378: in test_community_message_edit
        self.channel_2.set_reaction(message_text_after_edit)
    ../views/chat_view.py:1053: in set_reaction
        self.chat_element_by_text(message).long_press_until_element_is_shown(element)
    ../views/base_element.py:327: in long_press_until_element_is_shown
        element = self.find_element()
    ../views/chat_view.py:116: in find_element
        self.wait_for_visibility_of_element(20)
    ../views/base_element.py:147: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 2: ChatElementByText by xpath:`//*[starts-with(@text,'Message AFTER edit 2 (Edited)')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Passed tests (4)

    Click to expand

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

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    @status-im-auto
    Copy link
    Member

    57% of end-end tests have passed

    Total executed tests: 7
    Failed tests: 3
    Expected to fail tests: 0
    Passed tests: 4
    
    IDs of failed tests: 727230,727231,727229 
    

    Failed tests (3)

    Click to expand
  • Rerun failed tests

  • Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231

    Device 1: Tap on found: LogInButton
    Device 1: Find `Text` by `xpath`: `//*[@content-desc='account-avatar']/../following-sibling::android.widget.TextView[1]`

    critical/test_wallet.py:212: in test_wallet_add_remove_regular_account
        if self.wallet_view.account_name_text.text != new_account_name:
    ../views/base_element.py:419: in text
        text = self.find_element().text
    ../views/base_element.py:79: in find_element
        raise NoSuchElementException(
     Device 1: Text by xpath: `//*[@content-desc='account-avatar']/../following-sibling::android.widget.TextView[1]` 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
    



    Device sessions

    Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230

    Test setup failed: critical/test_wallet.py:23: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:329: in create_shared_drivers
        raise e
    base_test_case.py:319: in create_shared_drivers
        test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1
     '_asyncio.Future' object has no attribute 'session_id'
    



    2. test_wallet_send_eth, id: 727229

    Test setup failed: critical/test_wallet.py:23: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:329: in create_shared_drivers
        raise e
    base_test_case.py:319: in create_shared_drivers
        test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1
     '_asyncio.Future' object has no attribute 'session_id'
    



    Passed tests (4)

    Click to expand

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_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

    @VolodLytvynenko
    Copy link
    Contributor

    Hi @ulisesmac thank you for PR. Take a look at found issue

    ISSUE 1: "Objects are not valid as a React child" Error Displayed When Attempting to Send Multi-Collectible

    Steps:

    1. Select a multi-collectible.
    2. Go to the sending flow.
    3. Proceed with the sending process.

    Actual result:

    The error "Objects are not valid as a React child" is displayed

    multicollectible.mp4

    Expected result:

    The user should be navigated to the multi-collectible sending screen without errors.

    Devices:

    • Pixel 7a, Android 13
    • iPhone 11 Pro Max, IOS 17

    Logs:

    logs.zip

    @VolodLytvynenko
    Copy link
    Contributor

    ISSUE 2: [IOS] Token value and max fee not updated on the routes generation page

    Steps:

    1. Recover a user with tokens available on at least two networks.
    2. Go to the assets selection page.
    3. Select any asset (in my case I selected SNT with a total amount of 9).
    4. Proceed to the routes generation page.
    5. Change the selected token.
    6. Disable some networks in the "From" section.

    Actual Result:

    • The previous token's value is still displayed, even after selecting a different token.
    • The max value still reflects the total amount across all available networks, despite network disabling.
    assetsdelection.mp4

    Expected Result:

    The token value and max fee should update correctly based on the selected token and enabled/disabled networks.

    Devices:

    iPhone 11 Pro Max, iOS 17

    Logs:

    logs (3).zip

    @status-im-auto
    Copy link
    Member

    33% of end-end tests have passed

    Total executed tests: 3
    Failed tests: 2
    Expected to fail tests: 0
    Passed tests: 1
    
    IDs of failed tests: 727230,727229 
    

    Failed tests (2)

    Click to expand
  • Rerun failed tests

  • Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230

    Device 2: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]`
    Device 2: `Text` is `0.04819 ETH`

    critical/test_wallet.py:190: in test_wallet_send_asset_from_drawer
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.4392 but expected to be 0.4393
    



    2. test_wallet_send_eth, id: 727229

    Device 2: Find Text by xpath: //android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]
    Device 2: Text is 0.04809 ETH

    critical/test_wallet.py:159: in test_wallet_send_eth
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.4394 but expected to be 0.4395
    



    Passed tests (1)

    Click to expand

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231
    Device sessions

    @ulisesmac
    Copy link
    Contributor Author

    @VolodLytvynenko

    Thank you so much for testing this PR!

    I've checked these issues and I'm able to replicate them on develop, so this PR isn't introducing them.

    Do these issues block testing/merging this PR?
    OFC, I can investigate and solve them, but I'd prefer if they are created as separate issues.

    cc: @shivekkhurana

    @VolodLytvynenko
    Copy link
    Contributor

    Hi @ulisesmac, thank you for the update. That's strange -I couldn't reproduce the issue on the nightly build, possibly because it wasn't updated to the latest development yet. However, I can now reproduce it. Apologies for the confusion, the issues are not related to this PR. The PR can be merged

    @VolodLytvynenko
    Copy link
    Contributor

    VolodLytvynenko commented Sep 6, 2024

    @ulisesmac I've created issue 1 and issue 2 separately

    @ulisesmac ulisesmac force-pushed the 20472-Hide-account-switcher-sned-flow branch from 7901d3d to 98ab958 Compare September 7, 2024 00:58
    @ulisesmac ulisesmac merged commit 8f94332 into develop Sep 9, 2024
    6 checks passed
    @ulisesmac ulisesmac deleted the 20472-Hide-account-switcher-sned-flow branch September 9, 2024 20:24
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Archived in project
    Development

    Successfully merging this pull request may close these issues.

    Collectibles: Hide account switcher in send flow
    5 participants