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

Fix profile image in push notifications #12427

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

briansztamfater
Copy link
Member

@briansztamfater briansztamfater commented Aug 5, 2021

fixes #12331

Summary

This PR aims to fix user profile picture is not shown in Push Notifications

Platforms

  • Android
Functional
  • 1-1 chats

Steps to test

  • Open Status
  • User A: open status, add User B to contacts
  • User B: set profile picture
  • User B: send message to User A
  • Verify if profile image is displaying in PN

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Aug 5, 2021

Jenkins Builds

Click to see older builds (83)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 8fe29fb #1 2021-08-05 20:46:04 ~15 min android 📦apk 📲
✔️ 8fe29fb #1 2021-08-05 20:46:40 ~15 min ios 📦ipa 📲
✔️ 8fe29fb #1 2021-08-05 20:48:38 ~17 min android-e2e 📦apk 📲
✔️ 5cf5958 #2 2021-08-05 23:19:57 ~13 min ios 📦ipa 📲
✔️ 5cf5958 #2 2021-08-05 23:23:22 ~17 min android-e2e 📦apk 📲
✔️ 5cf5958 #2 2021-08-05 23:23:32 ~17 min android 📦apk 📲
✔️ 9dd93b2 #3 2021-08-05 23:44:27 ~13 min ios 📦ipa 📲
✔️ 9dd93b2 #3 2021-08-05 23:44:51 ~13 min android-e2e 📦apk 📲
✔️ 9dd93b2 #3 2021-08-05 23:44:55 ~13 min android 📦apk 📲
b3a6611 #4 2021-08-06 19:27:39 ~31 sec android 📄log
b3a6611 #4 2021-08-06 19:27:39 ~31 sec android-e2e 📄log
b3a6611 #4 2021-08-06 19:27:50 ~36 sec ios 📄log
✔️ be91c60 #5 2021-08-06 19:51:08 ~15 min ios 📦ipa 📲
✔️ be91c60 #5 2021-08-06 19:55:21 ~19 min android-e2e 📦apk 📲
✔️ be91c60 #5 2021-08-06 19:55:29 ~19 min android 📦apk 📲
✔️ 867a38e #6 2021-08-06 22:07:04 ~20 min android-e2e 📦apk 📲
✔️ 867a38e #6 2021-08-06 22:07:10 ~20 min ios 📦ipa 📲
✔️ 867a38e #6 2021-08-06 22:07:11 ~20 min android 📦apk 📲
✔️ b2dec8a #7 2021-08-06 22:59:04 ~16 min ios 📦ipa 📲
✔️ b2dec8a #7 2021-08-06 23:01:35 ~18 min android-e2e 📦apk 📲
✔️ b2dec8a #7 2021-08-06 23:01:40 ~18 min android 📦apk 📲
✔️ da80aad #8 2021-08-07 05:20:07 ~15 min ios 📦ipa 📲
✔️ da80aad #8 2021-08-07 05:20:27 ~15 min android-e2e 📦apk 📲
✔️ da80aad #8 2021-08-07 05:20:36 ~16 min android 📦apk 📲
✔️ e57bd53 #9 2021-08-07 05:47:10 ~15 min ios 📦ipa 📲
✔️ e57bd53 #9 2021-08-07 05:48:30 ~16 min android-e2e 📦apk 📲
✔️ e57bd53 #9 2021-08-07 05:48:33 ~17 min android 📦apk 📲
✔️ 5b6a8d6 #10 2021-08-07 06:08:18 ~15 min ios 📦ipa 📲
✔️ 5b6a8d6 #10 2021-08-07 06:09:49 ~17 min android-e2e 📦apk 📲
✔️ 5b6a8d6 #10 2021-08-07 06:10:31 ~17 min android 📦apk 📲
✔️ c20d60e #11 2021-08-07 06:27:39 ~13 min android 📦apk 📲
✔️ c20d60e #11 2021-08-07 06:28:48 ~14 min android-e2e 📦apk 📲
✔️ c20d60e #11 2021-08-07 06:30:37 ~16 min ios 📦ipa 📲
✔️ 92cba78 #12 2021-08-07 06:32:57 ~15 min android-e2e 📦apk 📲
✔️ 92cba78 #12 2021-08-07 06:34:37 ~17 min ios 📦ipa 📲
✔️ 92cba78 #12 2021-08-07 06:35:39 ~18 min android 📦apk 📲
✔️ 9fce2f9 #13 2021-08-07 18:21:04 ~15 min ios 📦ipa 📲
✔️ 9fce2f9 #13 2021-08-07 18:22:52 ~17 min android-e2e 📦apk 📲
✔️ 9fce2f9 #13 2021-08-07 18:22:54 ~17 min android 📦apk 📲
e377a1c #14 2021-09-15 17:54:35 ~6 min android-e2e 📄log
e377a1c #14 2021-09-15 17:55:12 ~7 min android 📄log
e377a1c #14 2021-09-15 18:00:20 ~12 min ios 📄log
b9d23b4 #15 2021-09-15 18:13:25 ~3 min android 📄log
b9d23b4 #15 2021-09-15 18:13:40 ~3 min android-e2e 📄log
b9d23b4 #15 2021-09-15 18:25:20 ~15 min ios 📄log
✔️ 15b444f #16 2021-09-15 18:31:30 ~16 min android 📦apk 📲
✔️ 15b444f #16 2021-09-15 18:31:48 ~17 min android-e2e 📦apk 📲
✔️ 15b444f #16 2021-09-15 18:33:14 ~18 min ios 📦ipa 📲
47d0256 #18 2021-09-27 17:03:27 ~7 min android-e2e 📄log
47d0256 #18 2021-09-27 17:03:28 ~7 min android 📄log
✔️ ba123a0 #17 2021-09-27 17:11:59 ~16 min android 📦apk 📲
✔️ ba123a0 #17 2021-09-27 17:12:31 ~17 min android-e2e 📦apk 📲
9fe1781 #19 2021-09-30 14:28:48 ~1 min android 📄log
9fe1781 #19 2021-09-30 14:29:00 ~1 min android-e2e 📄log
9fe1781 #19 2021-09-30 14:57:49 ~30 min ios 📄log
✔️ e931cc5 #20 2021-09-30 15:26:29 ~17 min android-e2e 📦apk 📲
✔️ e931cc5 #20 2021-09-30 15:26:34 ~17 min android 📦apk 📲
✔️ e931cc5 #20 2021-09-30 15:28:09 ~19 min ios 📦ipa 📲
✔️ 2efe899 #21 2021-10-01 13:16:29 ~11 min android 📦apk 📲
✔️ 2efe899 #21 2021-10-01 13:17:25 ~12 min android-e2e 📦apk 📲
✔️ 2efe899 #21 2021-10-01 13:29:48 ~24 min ios 📦ipa 📲
✔️ 0affff5 #22 2021-10-01 15:15:27 ~14 min android-e2e 📦apk 📲
✔️ 0affff5 #22 2021-10-01 15:17:11 ~16 min ios 📦ipa 📲
✔️ 0affff5 #22 2021-10-01 15:18:05 ~17 min android 📦apk 📲
9954b34 #23 2021-10-01 21:18:39 ~6 min android-e2e 📄log
9954b34 #23 2021-10-01 21:20:18 ~8 min android 📄log
✔️ 9954b34 #23 2021-10-01 21:31:37 ~19 min ios 📦ipa 📲
5ad0839 #24 2021-10-01 21:23:03 ~3 min android-e2e 📄log
5ad0839 #24 2021-10-01 21:26:26 ~6 min android 📄log
✔️ 5ad0839 #24 2021-10-01 21:34:46 ~15 min ios 📦ipa 📲
✔️ 49f6004 #25 2021-10-01 21:36:13 ~11 min android 📦apk 📲
✔️ 49f6004 #25 2021-10-01 21:39:40 ~15 min android-e2e 📦apk 📲
✔️ 49f6004 #25 2021-10-01 21:44:32 ~20 min ios 📦ipa 📲
✔️ f39e814 #26 2021-10-01 21:41:30 ~11 min android 📦apk 📲
✔️ f39e814 #26 2021-10-01 21:41:50 ~12 min android-e2e 📦apk 📲
✔️ f39e814 #26 2021-10-01 21:44:36 ~14 min ios 📦ipa 📲
f39e814 #27 2021-10-05 14:08:37 ~17 sec ios 📄log
✔️ d2f80b7 #27 2021-10-05 14:44:07 ~11 min android-e2e 📦apk 📲
✔️ d2f80b7 #27 2021-10-05 14:44:09 ~11 min android 📦apk 📲
✔️ d2f80b7 #28 2021-10-05 14:48:25 ~15 min ios 📦ipa 📲
✔️ 5f8d806 #28 2021-10-05 17:27:00 ~14 min android 📦apk 📲
✔️ 5f8d806 #28 2021-10-05 17:29:41 ~16 min android-e2e 📦apk 📲
✔️ 5f8d806 #29 2021-10-05 17:30:19 ~17 min ios 📦ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 5e3b665 #29 2021-10-07 14:44:43 ~17 min android 📦apk 📲
✔️ 5e3b665 #29 2021-10-07 14:53:26 ~25 min android-e2e 📦apk 📲
✔️ 0afbe80 #30 2021-10-07 14:53:45 ~22 min android-e2e 📦apk 📲
✔️ 0afbe80 #31 2021-10-07 14:55:56 ~22 min android-e2e 📦apk 📲

@briansztamfater briansztamfater force-pushed the fix/push-notification-picture branch 2 times, most recently from 5cf5958 to 9dd93b2 Compare August 5, 2021 23:30
@status-github-bot status-github-bot bot moved this from REVIEW to CONTRIBUTOR in Pipeline for QA Aug 6, 2021
@briansztamfater briansztamfater force-pushed the fix/push-notification-picture branch 6 times, most recently from da80aad to e57bd53 Compare August 7, 2021 05:31
@status-im-auto
Copy link
Member

✔️ status-react/prs/ios/PR-12427#9 🔹 ~16 min 🔹 e57bd53 🔹 📦 ios package

@briansztamfater briansztamfater force-pushed the fix/push-notification-picture branch 2 times, most recently from 5b6a8d6 to c20d60e Compare August 7, 2021 06:13
@briansztamfater briansztamfater changed the title [WIP] Fix profile image in push notiications Fix profile image in push notiications Aug 7, 2021
@briansztamfater briansztamfater marked this pull request as ready for review August 7, 2021 06:14
@briansztamfater briansztamfater changed the title Fix profile image in push notiications Fix profile image in push notifications Aug 7, 2021
@briansztamfater briansztamfater moved this from CONTRIBUTOR to E2E Tests in Pipeline for QA Aug 17, 2021
@status-github-bot status-github-bot bot moved this from E2E Tests to CONTRIBUTOR in Pipeline for QA Aug 17, 2021
@briansztamfater briansztamfater moved this from CONTRIBUTOR to REVIEW in Pipeline for QA Sep 15, 2021
@briansztamfater
Copy link
Member Author

@briansztamfater can you please rebase it? Thanks

Sure, rebase done @qoqobolo

@qoqobolo qoqobolo moved this from IN TESTING to E2E Tests in Pipeline for QA Oct 1, 2021
@qoqobolo
Copy link
Contributor

qoqobolo commented Oct 1, 2021

Thanks, @briansztamfater!
Is it expected that there is no profile image in PN on the group invite?
I believe we could add profile images to this type of PNs ass well, because 3-random name of admin is displayed there. WDYT?
Screenshot 2021-10-01 at 16 15 20

@status-im-auto
Copy link
Member

99% of end-end tests have passed

Total executed tests: 67
Failed tests: 1
Passed tests: 66
IDs of failed tests: 3994 

Failed tests (1)

Click to expand
  • Rerun tests

  • 1. test_create_new_group_chat_messaging_pn_delivered, id: 3994

    Device 2: **Looking for a message by text: Message from device: 1**
    Device 2: **Looking for a message by text: Message from device: 2**

    Message from device '2' was not received

    Device sessions

    Passed tests (66)

    Click to expand

    1. test_image_in_one_to_one_send_save_reply_timeline, id: 6305
    Device sessions

    2. test_add_account_to_multiaccount_instance_generate_new, id: 6224
    Device sessions

    3. test_can_add_existing_ens_on_mainnet, id: 5502
    Device sessions

    4. test_keycard_can_see_all_transactions_in_history, id: 6291
    Device sessions

    5. test_sign_message_and_2tx_in_batch_and_transactions_filters_from_daap, id: 5342
    Device sessions

    6. test_add_and_delete_watch_only_account_to_multiaccount_instance, id: 6244
    Device sessions

    7. test_open_blocked_secure_not_secure_inlalid_offline_urls, id: 6210
    Device sessions

    8. test_open_public_chat_using_deep_link, id: 5396
    Device sessions

    9. test_offline_add_new_group_chat_member, id: 3998
    Device sessions

    10. test_send_non_english_message_to_newly_added_contact_on_different_networks, id: 5315
    Device sessions

    11. test_can_see_balance_and_all_transactions_history_on_cellular, id: 5314
    Device sessions

    12. test_insufficient_funds_wallet_positive_balance, id: 5412
    Device sessions

    13. test_delete_close_all_tabs, id: 5390
    Device sessions

    14. test_send_token_with_7_decimals, id: 5350
    Device sessions

    15. test_delete_chats_via_delete_button_rejoin, id: 5387
    Device sessions

    16. test_send_transaction_set_recipient_options, id: 6328
    Device sessions

    17. test_keycard_sign_message_and_transactions_from_daap, id: 6251
    Device sessions

    18. test_recover_account_from_new_user_seedphrase, id: 6296
    Device sessions

    19. test_keycard_can_recover_keycard_account_card_pairing, id: 5758
    Device sessions

    20. test_block_user_from_public_chat, id: 5786
    Device sessions

    21. test_send_audio_message_with_push_notification_check, id: 6316
    Device sessions

    22. test_redirect_to_public_chat_tapping_tag_message_fetch_more_history, id: 5675
    Device sessions

    23. test_restore_account_migrate_multiaccount_to_keycard, id: 6645
    Device sessions

    24. test_edit_delete_message_in_one_to_one_and_public_chats, id: 695843
    Device sessions

    25. test_ens_mentions_pn_and_nickname_in_public_and_1_1_chats, id: 6226
    Device sessions

    26. test_keycard_send_eth_from_wallet_to_address, id: 6289
    Device sessions

    27. test_keycard_create_login_restore_unlock_same_seed, id: 5689
    Device sessions

    28. test_add_account_to_wallet_private_key_and_seed_phrase, id: 6272
    Device sessions

    29. test_offline_is_shown_messaging_1_1_chat_sent_delivered, id: 5310
    Device sessions

    30. test_pair_devices_sync_one_to_one_contacts_nicknames_public_chat, id: 5762
    Device sessions

    31. test_long_press_to_delete_chat, id: 5319
    Device sessions

    32. test_copy_and_paste_messages, id: 5317
    Device sessions

    33. test_open_transaction_on_etherscan_copy_tx_hash, id: 5384
    Device sessions

    34. test_unread_messages_counter_public_chat, id: 5360
    Device sessions

    35. test_send_eth_from_wallet_to_address_incorrect_password, id: 5308
    Device sessions

    36. test_mobile_data_usage_complex_settings, id: 6228
    Device sessions

    37. test_start_chat_with_ens_mention_in_one_to_one, id: 5403
    Device sessions

    38. test_decline_transactions_in_1_1_chat_push_notification_changing_state, id: 6265
    Device sessions

    39. test_send_eth_in_1_1_chat_transaction_push, id: 6253
    Device sessions

    40. test_install_pack_and_send_sticker, id: 5782
    Device sessions

    41. test_switch_users_special_char_password_and_add_new_account_logcat, id: 5356
    Device sessions

    42. test_manage_assets, id: 5341
    Device sessions

    43. test_browser_managing_bookmarks, id: 6633
    Device sessions

    44. test_open_chat_by_pasting_chat_key_check_invalid_chat_key_cases, id: 5304
    Device sessions

    45. test_request_stt_from_daap, id: 5309
    Device sessions

    46. test_can_use_purchased_stickers_on_recovered_account, id: 5783
    Device sessions

    47. test_dapps_permissions, id: 5738
    Device sessions

    48. test_account_recovery_with_uppercase_whitespaces_seed_phrase_special_char_passw_logcat, id: 5394
    Device sessions

    49. test_send_eth_to_ens_in_chat, id: 6279
    Device sessions

    50. test_share_copy_contact_code_and_wallet_address, id: 5323
    Device sessions

    51. test_wallet_set_up, id: 5335
    Device sessions

    52. test_send_transaction_with_custom_token, id: 6208
    Device sessions

    53. test_home_view, id: 5379
    Device sessions

    54. test_logcat_backup_recovery_phrase, id: 5419
    Device sessions

    55. test_request_and_receive_stt_in_1_1_chat_offline, id: 6263
    Device sessions

    56. test_set_profile_picture, id: 6646
    Device sessions

    57. test_add_and_remove_mention_contact_with_nickname_from_public_chat, id: 5332
    Device sessions

    58. test_keycard_request_stt_from_daap, id: 6249
    Device sessions

    59. test_restore_account_from_mnemonic_to_keycard, id: 6240
    Device sessions

    60. test_pass_phrase_validation, id: 5363
    Device sessions

    61. test_keycard_send_eth_in_1_1_chat, id: 6293
    Device sessions

    62. test_send_and_open_links_with_previews, id: 5373
    Device sessions

    63. test_reactions_to_message_in_chats, id: 6315
    Device sessions

    64. test_fetching_balance_after_offline, id: 6237
    Device sessions

    65. test_push_notification_1_1_chat_no_pn_activity_center, id: 6283
    Device sessions

    66. test_public_chat_messaging_emojis_timestamps, id: 5313
    Device sessions

    @briansztamfater
    Copy link
    Member Author

    @qoqobolo I agree, that would be a nice addition. Updated PR with that change :)

    @qoqobolo qoqobolo moved this from E2E Tests to IN TESTING in Pipeline for QA Oct 1, 2021
    @qoqobolo
    Copy link
    Contributor

    qoqobolo commented Oct 1, 2021

    @briansztamfater PNs on group chat invites are not received in the latest build (0affff5) 👀

    @briansztamfater
    Copy link
    Member Author

    @briansztamfater PNs on group chat invites are not received in the latest build (0affff5) 👀

    @qoqobolo Hmm interesting, worked on my local build but didn't work on the PR apk. I'll dive into the problem and come back to you with previous deep testing. Sorry about that!

    @briansztamfater briansztamfater force-pushed the fix/push-notification-picture branch 3 times, most recently from 49f6004 to f39e814 Compare October 1, 2021 21:29
    @briansztamfater
    Copy link
    Member Author

    briansztamfater commented Oct 1, 2021

    @qoqobolo diving deeper into notifications UX, it seems that default behavior is to show icons on the right side since Android Lollipop, unless it is a conversation. As this is just an invite to a group and not an initiated chat / conversation, maybe it is ok to show it on the right side.

    What do you think?

    Captura de Pantalla 2021-10-01 a la(s) 19 40 57

    Captura de Pantalla 2021-10-01 a la(s) 19 40 32

    (Also, if you are OK with that, PR is ready to test)

    @churik
    Copy link
    Member

    churik commented Oct 4, 2021

    @briansztamfater
    It doesn't seem so important to me, as I don't think it is too often the situation when you have several PNs to notice the difference.
    However, we can fix it later.
    @hesterbruikman better to know your opinion about that.

    @hesterbruikman
    Copy link
    Contributor

    @briansztamfater to make sure I understand the issue:

    • Is the notification only treated differently when it's a group chat invite?
    • Do I understand correctly that because the notification is treated differently it can be shown with either no profile image or profile image on the right?
    • What happens when a user receives 2 group chat invites from different people at the same time? This makes me think that it's probably better to leave profile image out of group chat invites altogether

    Agree with @churik that in any case this would be an uncommon scenario. I do want to understand the issue a bit better though; The scenario might be more common on first time use. You join a community and get invited to a bunch of group chats by members

    @briansztamfater
    Copy link
    Member Author

    @briansztamfater to make sure I understand the issue:

    • Is the notification only treated differently when it's a group chat invite?
    • Do I understand correctly that because the notification is treated differently it can be shown with either no profile image or profile image on the right?
    • What happens when a user receives 2 group chat invites from different people at the same time? This makes me think that it's probably better to leave profile image out of group chat invites altogether

    Agree with @churik that in any case this would be an uncommon scenario. I do want to understand the issue a bit better though; The scenario might be more common on first time use. You join a community and get invited to a bunch of group chats by members

    @hesterbruikman thanks for your time, really appreciate your questions.

    • Exactly, but in reality, the conversation notifications are those treated differently, as the default behavior is to have the icon on the right since Android Lollipop.
    • Correct, we can show the notification with no profile image on the right, or with no profile image at all (that's how it is currently working in develop branch)
    • In that case user should receive two different notifications with different profile pictures at the right (if we decide to show them)

    @qoqobolo
    Copy link
    Contributor

    qoqobolo commented Oct 5, 2021

    @briansztamfater

    ISSUE 3: In the latest build d2f80b7 group invites from contacts are sent to the Activity center

    Steps:

    1. UserA and UserB add to contacts each other
    2. UserA creates a group chat with UserB

    Expected result: the group chat to which the user was invited by his contact is displayed on the home screen
    Actual result: the group chat is displayed in the Activity center

    @briansztamfater
    Copy link
    Member Author

    briansztamfater commented Oct 5, 2021

    @qoqobolo seems I also need to rebase status-go part due to some contacts logic modification, on it right now

    @briansztamfater
    Copy link
    Member Author

    @qoqobolo Done. Profile image is still showing on group invite notifications, so I'll stay tuned on whether we should remove it or not

    @qoqobolo
    Copy link
    Contributor

    qoqobolo commented Oct 6, 2021

    @briansztamfater let's keep it as it is, with the profile image on the right, it looks good!
    Also, tested the case with 2 group chat invites from different users and can confirm that in that case user receives two different notifications with different profile pictures at the right.

    Everything works fine now, thanks for the great work!
    PR can be merged.

    @qoqobolo qoqobolo moved this from IN TESTING to MERGE in Pipeline for QA Oct 6, 2021
    Signed-off-by: Brian Sztamfater <brian@status.im>
    @briansztamfater briansztamfater merged commit 0afbe80 into develop Oct 7, 2021
    Pipeline for QA automation moved this from MERGE to DONE Oct 7, 2021
    @briansztamfater briansztamfater deleted the fix/push-notification-picture branch October 7, 2021 14:33
    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.

    User profile picture is not shown in PNs
    6 participants