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.

Sign up for GitHub

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

Jump to bottom

Update group-avatar component #16937

Merged
merged 7 commits into from
Aug 14, 2023
Merged

Update group-avatar component #16937

merged 7 commits into from
Aug 14, 2023

Conversation

codemaster115
Copy link
Contributor

@codemaster115 codemaster115 commented Aug 8, 2023 •
edited
Loading

fixes #16931

Summary

The current group-avatar component implementation has some missing features.

  • customization-color
  • avatar

It will affect to group-avatar-tag component so need manual-qa for testing.

Platforms

  • Android
  • iOS
  • macOS
  • Linux
  • Windows

status: ready

@codemaster115 codemaster115 linked an issue Aug 8, 2023 that may be closed by this pull request
Group Avatar component doesn't have picture format #16931
Closed
@codemaster115 codemaster115 self-assigned this Aug 8, 2023
@status-im-auto
Copy link
Member

status-im-auto commented Aug 8, 2023 •
edited
Loading

Jenkins Builds

Click to see older builds (29)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ aea9b72 #1 2023-08-08 20:58:51 ~5 min android 🤖apk 📲
✔️ aea9b72 #1 2023-08-08 20:58:54 ~5 min android-e2e 🤖apk 📲
✔️ aea9b72 #1 2023-08-08 21:01:47 ~8 min tests 📄log
✔️ aea9b72 #1 2023-08-08 21:02:50 ~9 min ios 📱ipa 📲
❌ d743147 #2 2023-08-10 14:04:20 ~2 min tests 📄log
✔️ 0ae5f09 #3 2023-08-10 14:10:53 ~6 min android 🤖apk 📲
✔️ 0ae5f09 #3 2023-08-10 14:10:53 ~6 min android-e2e 🤖apk 📲
✔️ 0ae5f09 #3 2023-08-10 14:11:33 ~6 min ios 📱ipa 📲
✔️ 0ae5f09 #3 2023-08-10 14:14:39 ~9 min tests 📄log
✔️ 308b435 #4 2023-08-10 14:46:24 ~6 min android-e2e 🤖apk 📲
✔️ 308b435 #4 2023-08-10 14:46:24 ~6 min ios 📱ipa 📲
✔️ 308b435 #4 2023-08-10 14:46:30 ~6 min android 🤖apk 📲
✔️ 308b435 #4 2023-08-10 14:49:07 ~8 min tests 📄log
✔️ f723f9b #5 2023-08-10 16:45:40 ~5 min android 🤖apk 📲
✔️ f723f9b #5 2023-08-10 16:47:14 ~7 min android-e2e 🤖apk 📲
✔️ f723f9b #5 2023-08-10 16:49:40 ~9 min tests 📄log
✔️ f723f9b #5 2023-08-10 16:51:35 ~11 min ios 📱ipa 📲
✔️ 8e80aca #6 2023-08-10 17:43:25 ~5 min android-e2e 🤖apk 📲
✔️ 8e80aca #6 2023-08-10 17:43:32 ~5 min android 🤖apk 📲
✔️ 8e80aca #6 2023-08-10 17:46:42 ~8 min tests 📄log
✔️ 8e80aca #6 2023-08-10 17:49:05 ~11 min ios 📱ipa 📲
✔️ 51e2bf3 #7 2023-08-11 10:13:05 ~5 min android 🤖apk 📲
✔️ 51e2bf3 #7 2023-08-11 10:15:22 ~7 min android-e2e 🤖apk 📲
✔️ 51e2bf3 #7 2023-08-11 10:16:55 ~9 min tests 📄log
✔️ 51e2bf3 #7 2023-08-11 10:18:45 ~11 min ios 📱ipa 📲
✔️ c21fbb3 #8 2023-08-11 20:46:59 ~5 min android-e2e 🤖apk 📲
✔️ c21fbb3 #8 2023-08-11 20:49:12 ~8 min android 🤖apk 📲
✔️ c21fbb3 #8 2023-08-11 20:50:18 ~9 min tests 📄log
✔️ c21fbb3 #8 2023-08-11 20:52:41 ~11 min ios 📱ipa 📲
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
❌ 98e2d8c #9 2023-08-14 17:17:42 ~2 min tests 📄log
✔️ 98e2d8c #9 2023-08-14 17:20:35 ~5 min android-e2e 🤖apk 📲
✔️ 98e2d8c #9 2023-08-14 17:21:07 ~6 min android 🤖apk 📲
✔️ 98e2d8c #9 2023-08-14 17:27:01 ~12 min ios 📱ipa 📲
✔️ 87a150c #10 2023-08-14 17:36:08 ~5 min android-e2e 🤖apk 📲
✔️ 87a150c #10 2023-08-14 17:38:12 ~7 min android 🤖apk 📲
✔️ 87a150c #10 2023-08-14 17:40:00 ~9 min tests 📄log
✔️ 87a150c #10 2023-08-14 17:44:52 ~14 min ios 📱ipa 📲

@codemaster115 codemaster115 force-pushed the 16931-group-avatar-component-doesnt-have-picture-format branch from aea9b72 to d743147 Compare August 10, 2023 14:01
@codemaster115 codemaster115 force-pushed the 16931-group-avatar-component-doesnt-have-picture-format branch from 308b435 to f723f9b Compare August 10, 2023 16:39
ilmotta
ilmotta approved these changes Aug 10, 2023
View reviewed changes
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.

Can't speak for the UI, but the code LGTM :)

@status-im-auto
Copy link
Member

88% of end-end tests have passed

Total executed tests: 43
Failed tests: 5
Passed tests: 38

IDs of failed tests: 702732,703133,702948,702786,702731

Failed tests (5)

Click to expand
  • Rerun failed tests

    • Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133 
    Test is not run, e2e blocker

    [[reason: [NOTRUN] Restoring communities issue: 16787; restoring contacts issue: 15500]]

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_pin_messages, id: 702732 

    Test is not run, e2e blocker

    [[reason: [NOTRUN] Pin feature is in development]]

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_hashtag_links_to_community_channels, id: 702948 

    Test is not run, e2e blocker

    [[reason: [NOTRUN] Can't invite user to closed community https://github.com//issues/16968]]

    2. test_community_mentions_push_notification, id: 702786

    # STEP: Admin gets push notification with the mention and tap it
    Device 1: Getting PN by 'user_1' 

    critical/test_public_chat_browsing.py:1082: in test_community_mentions_push_notification
    self.errors.verify_no_errors()
base_test_case.py:177: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Push notification with the mention was not received by admin



    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_pin_messages, id: 702731 

    Test is not run, e2e blocker

    [[reason: [NOTRUN] Pin feature is in development]]

    Passed tests (38)

    Click to expand

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_mute_chat, id: 703495
    Device sessions

    2. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    3. test_group_chat_reactions, id: 703202
    Device sessions

    4. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    5. test_group_chat_offline_pn, id: 702808
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_markdown_support, id: 702809
    Device sessions

    2. test_community_several_images_send_reply, id: 703194
    Device sessions

    3. test_community_one_image_send_reply, id: 702859
    Device sessions

    4. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    5. test_community_mark_all_messages_as_read, id: 703086
    Device sessions

    6. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    7. test_community_message_delete, id: 702839
    Device sessions

    8. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    9. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    10. test_community_message_edit, id: 702843
    Device sessions

    11. test_community_leave, id: 702845
    Device sessions

    12. test_community_unread_messages_badge, id: 702841
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_community_discovery, id: 703503
    Device sessions

    3. test_community_undo_delete_message, id: 702869
    Device sessions

    4. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    5. test_community_mute_community_and_channel, id: 703382
    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_activity_center_mentions, id: 702957
    Device sessions

    2. test_navigation_jump_to, id: 702936
    Device sessions

    3. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    4. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    2. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    3. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    4. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    5. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    6. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    7. test_1_1_chat_mute_chat, id: 703496
    Device sessions

    8. test_1_1_chat_edit_message, id: 702855
    Device sessions

    9. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    10. test_1_1_chat_message_reaction, id: 702730
    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

    @codemaster115 codemaster115 force-pushed the 16931-group-avatar-component-doesnt-have-picture-format branch from 51e2bf3 to c21fbb3 Compare August 11, 2023 20:40
    @codemaster115 codemaster115 force-pushed the 16931-group-avatar-component-doesnt-have-picture-format branch from c21fbb3 to 98e2d8c Compare August 14, 2023 17:14
    @codemaster115 codemaster115 merged commit 87a150c into develop Aug 14, 2023
    2 checks passed
    @codemaster115 codemaster115 deleted the 16931-group-avatar-component-doesnt-have-picture-format branch August 14, 2023 17:59
    @qoqobolo
    Copy link
    Contributor

    Hey @codemaster115, we noticed that this PR was also merged without QA approval, although the PR had the request-manual-QA label.
    Could you please clarify if there were any specific reasons for this? I'd like to remind you that at the moment the process for PRs is as follows:

    1. The PR must receive at least one approval from the dev review

    2. Then the PR should be moved to the e2e tests column to run autotests (for all PRs)

    3. If the developer is 100% sure that changes in the PR cannot introduce new issues or regression, the request-manual-QA label can be omitted. Otherwise, the label should be added.

    4. Then you should wait for manual testing from someone on the QA team. If issues are found, the QA will ping the developer in the comments and add the tested issues label.
      If no issues are found, the QA will add the tested-OK label.

    5. Then, depending on changes in the PR, this may also be sent for design review (corresponding column on the board).
      If the designer finds issues (now Francesca is doing it), they add the followup required label.
      After that, the PR is moved to the Merge column and can be merged.

    6. If a design review is not required, the QA moves the PR to the merge column immediately.

    You can also find the PR workflow instructions here (although it's a little outdated and doesn't include the new design review step).
    https://github.com/status-im/status-mobile/blob/develop/doc/pipeline_process.md

    Please feel free to contact us (in PRs or DM on Discord) with any questions!🙏

    @codemaster115
    Copy link
    Contributor Author

    codemaster115 commented Aug 17, 2023 •
    edited
    Loading

    @qoqobolo Sorry for that. I just assumed this PR doesn't require QA since just changes the API of the component.
    I will keep the process in the future. This PR is reverted now in #17039

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @Francesca-G Francesca-G Francesca-G approved these changes

    @ilmotta ilmotta ilmotta approved these changes

    @flexsurfer flexsurfer Awaiting requested review from flexsurfer

    @J-Son89 J-Son89 Awaiting requested review from J-Son89

    @FFFra FFFra Awaiting requested review from FFFra

    @Parveshdhull Parveshdhull Awaiting requested review from Parveshdhull

    @VolodLytvynenko VolodLytvynenko Awaiting requested review from VolodLytvynenko

    Assignees

    @codemaster115 codemaster115

    Labels
    design-system request-manual-qa
    Projects
    Pipeline for QA
    Archived in project
    Legacy QA Automation Project
    Archived in project
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    Group Avatar component doesn't have picture format
    6 participants
    @codemaster115 @status-im-auto @qoqobolo @ilmotta @J-Son89 @Francesca-G