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

feat: dapp connected notification #20905

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

yqrashawn
Copy link
Contributor

@yqrashawn yqrashawn commented Jul 29, 2024

status-im/status-go@4a43b2b...9bc67a0


fixes #20467

Summary

Pressing on the notification to execute its default action will be a separate GH issue.

  • add dapp connected notification type
  • fetch notification unread count when wallet connect session added
  • add status-im.contexts.shell.activity-center.notification.dapp-connection.view to render the notification

Review notes

Testing notes

Platforms

  • Android
  • iOS

Areas that maybe impacted

activity center

Steps to test

  • Open Status
  • enable wallet connect feature flag
  • scan the QR code at https://react-app.walletconnect.com/
  • connect on mobile
  • unread notification count should increase
  • open activity center
  • check the notification
  • try mark notification read or delete the notification

status: ready

@yqrashawn yqrashawn force-pushed the feat/dapp-connected-notification branch 2 times, most recently from 43efe77 to 849ac8f Compare July 29, 2024 07:28
@yqrashawn yqrashawn self-assigned this Jul 29, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Jul 29, 2024

Jenkins Builds

Click to see older builds (30)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 849ac8f #3 2024-07-29 07:33:16 ~4 min tests 📄log
✔️ 849ac8f #3 2024-07-29 07:36:42 ~8 min android-e2e 🤖apk 📲
✔️ 849ac8f #3 2024-07-29 07:37:55 ~9 min android 🤖apk 📲
849ac8f #3 2024-07-29 07:41:30 ~12 min ios 📄log
✔️ 77ad8a9 #5 2024-07-29 08:05:26 ~4 min tests 📄log
✔️ 77ad8a9 #5 2024-07-29 08:06:57 ~6 min android-e2e 🤖apk 📲
✔️ 77ad8a9 #5 2024-07-29 08:07:13 ~6 min android 🤖apk 📲
77ad8a9 #5 2024-07-29 08:12:45 ~12 min ios 📄log
✔️ 834a529 #6 2024-07-29 08:22:33 ~4 min tests 📄log
✔️ 834a529 #6 2024-07-29 08:24:39 ~6 min android-e2e 🤖apk 📲
✔️ 834a529 #6 2024-07-29 08:26:26 ~8 min android 🤖apk 📲
✔️ 834a529 #6 2024-07-29 08:34:02 ~15 min ios 📱ipa 📲
✔️ e714064 #8 2024-07-29 11:49:04 ~4 min tests 📄log
✔️ e714064 #8 2024-07-29 11:50:40 ~6 min android-e2e 🤖apk 📲
✔️ 731f2ab #9 2024-07-29 11:55:48 ~4 min tests 📄log
✔️ 731f2ab #9 2024-07-29 11:58:51 ~7 min android-e2e 🤖apk 📲
✔️ 731f2ab #9 2024-07-29 12:00:12 ~9 min android 🤖apk 📲
✔️ 731f2ab #9 2024-07-29 12:10:19 ~19 min ios 📱ipa 📲
✔️ 1b39c36 #10 2024-07-31 03:10:52 ~5 min tests 📄log
✔️ 1b39c36 #10 2024-07-31 03:12:18 ~6 min android-e2e 🤖apk 📲
✔️ 1b39c36 #10 2024-07-31 03:14:59 ~9 min android 🤖apk 📲
✔️ 1b39c36 #10 2024-07-31 03:19:56 ~14 min ios 📱ipa 📲
✔️ 63acb32 #11 2024-08-28 15:23:15 ~5 min tests 📄log
✔️ 63acb32 #11 2024-08-28 15:24:25 ~6 min android-e2e 🤖apk 📲
✔️ 63acb32 #11 2024-08-28 15:27:50 ~10 min android 🤖apk 📲
✔️ 63acb32 #11 2024-08-28 15:35:40 ~18 min ios 📱ipa 📲
82769ee #13 2024-09-02 16:01:20 ~2 min tests 📄log
✔️ 82769ee #13 2024-09-02 16:05:45 ~7 min android-e2e 🤖apk 📲
✔️ 82769ee #13 2024-09-02 16:07:41 ~8 min android 🤖apk 📲
✔️ 82769ee #13 2024-09-02 16:09:31 ~10 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ cf6eef2 #14 2024-09-03 08:25:42 ~5 min tests 📄log
✔️ cf6eef2 #14 2024-09-03 08:27:06 ~6 min android 🤖apk 📲
✔️ cf6eef2 #14 2024-09-03 08:27:53 ~7 min android-e2e 🤖apk 📲
✔️ cf6eef2 #14 2024-09-03 08:30:24 ~9 min ios 📱ipa 📲
✔️ 43a7049 #15 2024-09-03 16:30:55 ~5 min tests 📄log
✔️ 43a7049 #15 2024-09-03 16:32:30 ~6 min android-e2e 🤖apk 📲
✔️ 43a7049 #15 2024-09-03 16:33:26 ~7 min android 🤖apk 📲
✔️ 43a7049 #15 2024-09-03 16:36:49 ~11 min ios 📱ipa 📲

@yqrashawn yqrashawn force-pushed the feat/dapp-connected-notification branch 2 times, most recently from feaa59c to 77ad8a9 Compare July 29, 2024 08:00
@yqrashawn yqrashawn force-pushed the feat/dapp-connected-notification branch from 77ad8a9 to 834a529 Compare July 29, 2024 08:17
@yqrashawn yqrashawn marked this pull request as ready for review July 29, 2024 08:18
Copy link
Member

@clauxx clauxx left a comment

Choose a reason for hiding this comment

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

@yqrashawn thank you for the PR! Just a reminder, since we agreed to not include it in the 2.30 release, we should hold on with merging it till after we cut the release branch.

@yqrashawn yqrashawn force-pushed the feat/dapp-connected-notification branch 3 times, most recently from e714064 to 731f2ab Compare July 29, 2024 11:50
Copy link
Contributor

@ilmotta ilmotta left a comment

Choose a reason for hiding this comment

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

LGTM @yqrashawn. I left a minor comment. As @clauxx suggested it's a good idea to hold the merge if it's not critical for 2.30.

@yqrashawn yqrashawn force-pushed the feat/dapp-connected-notification branch from 731f2ab to 1b39c36 Compare July 31, 2024 03:05
@yqrashawn
Copy link
Contributor Author

Will request for manual QA after 2.30

@yqrashawn
Copy link
Contributor Author

next up: #20467 (comment)

@yqrashawn yqrashawn removed their assignment Aug 6, 2024
@yqrashawn yqrashawn removed their assignment Aug 19, 2024
@mohsen-ghafouri mohsen-ghafouri self-assigned this Aug 28, 2024
@pavloburykh pavloburykh self-assigned this Aug 30, 2024
@pavloburykh
Copy link
Contributor

Hey @pavloburykh, could you please check these 2 issues again? should be resolved.

@mohsen-ghafouri thank you! Both issues are fixed.

One thing that we still need to do is to rebase status go branch status-im/status-go#5616 , then update mobile PR with updated status go commit and re-run e2e tests make sure no issues will leak from status go develop to mobile branch. Could you please ping me up once branches are updated and then I will trigger e2e. Thank you!

@mohsen-ghafouri
Copy link
Contributor

Thank you @pavloburykh, i will ping you once it's ready

@mohsen-ghafouri
Copy link
Contributor

Hey @pavloburykh, I updated branches. please if you can, do another test as status-go side had a slight changes.

@status-im-auto
Copy link
Member

98% of end-end tests have passed

Total executed tests: 51
Failed tests: 1
Expected to fail tests: 0
Passed tests: 50
IDs of failed tests: 727229 

Failed tests (1)

Click to expand
  • Rerun failed tests

  • Class TestWalletMultipleDevice:

    1. 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.04649 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.4422 but expected to be 0.4423
    



    Passed tests (50)

    Click to expand

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    1. test_1_1_chat_mute_chat, id: 703496
    Device sessions

    2. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    3. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231
    Device sessions

    Class TestDeepLinksOneDevice:

    1. test_links_open_universal_links_from_chat, id: 704613
    Device sessions

    2. test_links_deep_links_profile, id: 702775
    Device sessions

    3. test_deep_links_communities, id: 739307
    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_leave, id: 702845
    Device sessions

    2. test_community_mentions_push_notification, id: 702786
    Device sessions

    3. test_community_markdown_support, id: 702809
    Device sessions

    4. test_community_hashtag_links_to_community_channels, id: 702948
    Device sessions

    5. test_community_join_when_node_owner_offline, id: 703629
    Device sessions

    Class TestActivityMultipleDevicePRTwo:

    1. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    2. test_activity_center_mentions, id: 702957
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_edit_message, id: 702855
    Device sessions

    2. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    3. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    4. test_1_1_chat_pin_messages, id: 702731
    Device sessions

    5. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    6. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    7. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    8. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_reactions, id: 703202
    Device sessions

    2. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    3. test_group_chat_offline_pn, id: 702808
    Device sessions

    4. test_group_chat_pin_messages, id: 702732
    Device sessions

    5. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    6. test_group_chat_mute_chat, id: 703495
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    3. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    4. test_community_undo_delete_message, id: 702869
    Device sessions

    5. test_community_mute_community_and_channel, id: 703382
    Device sessions

    6. test_community_discovery, id: 703503
    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    2. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    3. test_add_contact_field_validation, id: 702777
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    2. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    3. test_community_mark_all_messages_as_read, id: 703086
    Device sessions

    4. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    5. test_community_unread_messages_badge, id: 702841
    Device sessions

    6. test_community_message_delete, id: 702839
    Device sessions

    7. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    8. test_community_edit_delete_message_when_offline, id: 704615
    Device sessions

    9. test_community_one_image_send_reply, id: 702859
    Device sessions

    10. test_community_message_edit, id: 702843
    Device sessions

    11. test_community_several_images_send_reply, id: 703194
    Device sessions

    Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230

    Class TestActivityMultipleDevicePR:

    1. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    @pavloburykh
    Copy link
    Contributor

    Hey @pavloburykh, I updated branches. please if you can, do another test as status-go side had a slight changes.

    thanks @mohsen-ghafouri! Tested, PR is ready for merge.

    @mohsen-ghafouri
    Copy link
    Contributor

    Thank you for your time and checking this PR @pavloburykh.

    @pavloburykh
    Copy link
    Contributor

    Hey @mohsen-ghafouri! I see that current PR hasn't been merged because corresponding status go PR is blocked. Once it will be unblocked we will need to rebase both branches once again and re run e2e, because there are lot of new commits in go develop since the last test e2e run and we need to make sure they will not brake mobile develop.

    Also, I see that new conflict has appeared in src/status_im/contexts/shell/activity_center/view.cljs Please ping me up in case PR will need to be re-tested as a result of resolving this conflict. Thank you.

    @mohsen-ghafouri
    Copy link
    Contributor

    Hi @pavloburykh, sure. There are ongoing discussions about changes in status-go, and we might implement some updates. I’ll let you know once it’s ready for testing.

    @mariia-skrypnyk
    Copy link

    Hi @pavloburykh, sure. There are ongoing discussions about changes in status-go, and we might implement some updates. I’ll let you know once it’s ready for testing.

    Thanks @mohsen-ghafouri .

    Pavlo is AFK this week. I took this PR instead.

    @mohsen-ghafouri
    Copy link
    Contributor

    Thanks @mariia-skrypnyk, i will let you know once it's ready for test.

    @mariia-skrypnyk
    Copy link

    Hi @mohsen-ghafouri !

    Any update on this?

    @mohsen-ghafouri
    Copy link
    Contributor

    Hi @mariia-skrypnyk, no and i think it can takes time for a while. I will move it to contribution step and remove manual-qa label until it being ready again.

    @clauxx
    Copy link
    Member

    clauxx commented Sep 16, 2024

    I think it might be best to park/close this after all, since we picked it up with the assumption that not much more has to be done (as it's not a priority for the current release), but it seems like there are implications on status-go as well and it might take a while to get it in.

    cc: @mohsen-ghafouri @shivekkhurana

    @mohsen-ghafouri mohsen-ghafouri marked this pull request as draft September 16, 2024 10:49
    @mohsen-ghafouri
    Copy link
    Contributor

    mohsen-ghafouri commented Sep 16, 2024

    Hey @clauxx, make sense, i just marked as draft until we apply status-go changes. hopefully that won't require any more changes in mobile PR.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: CONTRIBUTOR
    Development

    Successfully merging this pull request may close these issues.

    [dApp Notifications] Show successful dApp connection in Activity Center
    8 participants