search

status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.92k stars 984 forks source link

feat(onboarding): Present Terms to users upgrading from v1 or those who need to accept updated Terms #21487

Closed ilmotta closed 1 month ago

ilmotta commented 1 month ago

Fixes https://github.com/status-im/status-mobile/issues/21113

Summary

This PR is 99% identical to https://github.com/status-im/status-mobile/pull/21124. We are cherry-picking from the 2.30 release branch revision https://github.com/status-im/status-mobile/commit/d45eb5ec20ae7757cd99554920e466eabac0bfb1 and applying to the status-go and status-mobile develop branches in preparation for the 2.31 release.

Related status-go PR: https://github.com/status-im/status-go/pull/5977

We will update the release branches once the develop branches are merged.

Steps to test

Steps to test are identical to the original PR https://github.com/status-im/status-mobile/pull/21124.

I re-tested and everything seems to work as well as in 2.30, where the original PR was already tested. I tested a scenario that's difficult for a QA to test, where I simulated a user who had Status v2 installed, had already accepted the Terms, then I forced a new migration in status-go to simulate an update to the Terms, and finally checked the user would need to re-accept terms before proceeding.

status: ready

status-im-auto commented 1 month ago

Jenkins Builds

:grey_question: Commit :hash: Finished (UTC) Duration Platform Result
:heavy_check_mark: 61ac8902 #1 2024-10-23 03:27:29 ~5 min tests :page_facing_up:log
:heavy_check_mark: 61ac8902 #1 2024-10-23 03:30:59 ~9 min android-e2e :robot:apk :calling:
:heavy_check_mark: 61ac8902 #1 2024-10-23 03:31:23 ~9 min android :robot:apk :calling:
:heavy_check_mark: 61ac8902 #1 2024-10-23 03:31:36 ~10 min ios :iphone:ipa :calling:
:heavy_check_mark: 61ac8902 #2 2024-10-23 10:52:56 ~9 min android :robot:apk :calling:
:heavy_check_mark: c47f317e #2 2024-10-23 14:17:17 ~4 min tests :page_facing_up:log
:heavy_check_mark: c47f317e #2 2024-10-23 14:20:12 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: c47f317e #3 2024-10-23 14:21:42 ~9 min android :robot:apk :calling:
:heavy_check_mark: c47f317e #2 2024-10-23 14:22:34 ~10 min ios :iphone:ipa :calling:
status-im-auto commented 1 month ago

75% of end-end tests have passed

Total executed tests: 8
Failed tests: 2
Expected to fail tests: 0
Passed tests: 6
IDs of failed tests: 703133,702843

Failed tests (2)

Click to expand
  • Rerun failed tests

    • Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    # STEP: Check that removed user is not shown in the list anymore
    Device 1: Wait for element `Button` for max 30s and click when it is available

    ``` critical/chats/test_public_chat_browsing.py:240: in test_restore_multiaccount_with_waku_backup_remove_switch self.sign_in.show_profiles_button.wait_and_click() ../views/base_element.py:100: in wait_and_click self.wait_for_visibility_of_element(sec) ../views/base_element.py:147: in wait_for_visibility_of_element raise TimeoutException( Device 1: Button by accessibility id:`show-profiles` is not found on the screen after wait_for_visibility_of_element ```

    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Device 2: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='chat-item']//android.widget.TextView[contains(@text,'https://status.app/c/')]`
    Device 2: Wait for element `Button` for max 120s and click when it is available

    ``` Test setup failed: critical/chats/test_public_chat_browsing.py:350: in prepare_devices self.community_2.join_community() ../views/chat_view.py:420: in join_community self.join_button.wait_and_click(120) ../views/base_element.py:100: in wait_and_click self.wait_for_visibility_of_element(sec) ../views/base_element.py:147: in wait_for_visibility_of_element raise TimeoutException( Device 2: Button by accessibility id:`show-request-to-join-screen-button` is not found on the screen after wait_for_visibility_of_element ```

    Device sessions

    Passed tests (6)

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

    1. test_wallet_add_remove_regular_account, id: 727231
    2. test_wallet_balance_mainnet, id: 740490

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    status-im-auto commented 1 month ago

    75% of end-end tests have passed

    Total executed tests: 8
Failed tests: 2
Expected to fail tests: 0
Passed tests: 6
    IDs of failed tests: 703133,702843

    Failed tests (2)

    Click to expand
  • Rerun failed tests

    • Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Device 2: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='chat-item']//android.widget.TextView[contains(@text,'https://status.app/c/')]`
    Device 2: Wait for element `Button` for max 120s and click when it is available

    ``` Test setup failed: critical/chats/test_public_chat_browsing.py:350: in prepare_devices self.community_2.join_community() ../views/chat_view.py:420: in join_community self.join_button.wait_and_click(120) ../views/base_element.py:100: in wait_and_click self.wait_for_visibility_of_element(sec) ../views/base_element.py:147: in wait_for_visibility_of_element raise TimeoutException( Device 2: Button by accessibility id:`show-request-to-join-screen-button` is not found on the screen after wait_for_visibility_of_element ```

    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    # STEP: Check that removed user is not shown in the list anymore
    Device 1: Wait for element `Button` for max 30s and click when it is available

    ``` critical/chats/test_public_chat_browsing.py:240: in test_restore_multiaccount_with_waku_backup_remove_switch self.sign_in.show_profiles_button.wait_and_click() ../views/base_element.py:100: in wait_and_click self.wait_for_visibility_of_element(sec) ../views/base_element.py:147: in wait_for_visibility_of_element raise TimeoutException( Device 1: Button by accessibility id:`show-profiles` is not found on the screen after wait_for_visibility_of_element ```

    Device sessions

    Passed tests (6)

    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
    2. test_wallet_balance_mainnet, id: 740490

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230
    2. test_wallet_send_eth, id: 727229
    pavloburykh commented 1 month ago

    @ilmotta thanks for the PR. Please take a look at the issue below

    ISSUE 1 Terms screen appears after removing one of the existing Profiles despite terms have been already accepted before

    Steps:

    1. Create Profile A
    2. Close and reopen the app
    3. Create Profile B
    4. Close and reopen the app
    5. Go to Profiles list screen
    6. Remove Profile A
    7. Close and reopen the app
    8. Observe which screen is displayed after app reopening

    Actual result: Terms screen is displayed despite terms have been already accepted.

    https://github.com/user-attachments/assets/ccb2a061-d4a6-4026-adcc-e174e959f494

    ilmotta commented 1 month ago

    Thank you for promptly testing @pavloburykh 💯

    Unfortunately this is the behavior by "design" in the code. It's a known shortcoming of the implementation in status-go. The state about whether the user accepted the terms is stored in the first profile ever created. If you created two profiles, but deleted the first one, then the incorrect screen will appear. If you remove profile B instead of A, it work correctly because the state would still be stored in the local db by status-go.

    In release 2.32 we will make same pretty significant simplifications to onboarding. One of the changes under consideration is to remove this mandatory checkbox and use something like "By proceeding you agree with the terms" and possibly removing this extra screen saying "Explore new Status" because most users would have migrated from v1 anyway (according to app stores). It will be a matter of aligning with legal too, so let's see. Right now we can minimize our efforts regarding the privacy policy and terms of use UX because we may change everything soon.

    For now I would say this is a minor limitation we can live with until 2.32 is published.

    What do you think?

    pavloburykh commented 1 month ago

    What do you think?

    Agree @ilmotta! PR is ready for merge. Thank you!

    pavloburykh commented 3 weeks ago

    Hey @ilmotta! Also, it turned out that Terms screen is appearing for synced account.

    ISSUE 2 Terms screen appears after syncing the account

    Steps:

    1. Generate sync QR on Device A
    2. Scan QR by Device B to sync the devices
    3. Wait until sync is complete
    4. Close and reopen the app on Device B

    Actual result: user needs accept Terms

    https://github.com/user-attachments/assets/32d179c1-c0f6-4746-8d71-ec894e5a5810

    pavloburykh commented 3 weeks ago

    @ilmotta how do you think, do we want to fix ISSUE 1 and ISSUE 2 or we will leave it as it is. In case we want to fix it, I can combine and log them in one separate issue.

    ilmotta commented 3 weeks ago

    @ilmotta how do you think, do we want to fix ISSUE 1 and ISSUE 2 or we will leave it as it is. In case we want to fix it, I can combine and log them in one separate issue.

    @pavloburykh, ISSUE 2 is new to me, and it's an unfortunate bug we will have to live with in 2.31 (the bug should exist in 2.30 (?) because we basically cherry-picked).

    But to answer your question, onboarding simplifications have been defined and the user won't have to accept the checkbox (per screenshot). This means the existing (2.30 & 2.31) solution won't work well in 2.32 and we'll need to adjust. The business problem is: how do we tell the user the Terms of Service or Privacy Policy changed between releases? I believe whatever we do we will do in the least obtrusive way and the simplest way, therefore different than 2.30 & 2.31.

    Long story short, I think we shouldn't fix ISSUE 1 or ISSUE 2 now, but they will be attacked/solved in 2.32 once we define & implement what I shared above. I even have the impression we might completely remove the code from this PR (and from status-go), but let's see.

    pavloburykh commented 3 weeks ago

    (the bug should exist in 2.30 (?)

    Yes. it exists in 2.30, wondering how we (QAs) have missed it. Thanks for the answer @ilmotta Then, let's wait for implementing onboarding simplification.