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(group-chat): Missing group chat name when clearing chat history #21684

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ilmotta
Copy link
Contributor

@ilmotta ilmotta commented Nov 26, 2024

Fixes #21639

Summary

Fix missing group name when user chooses drawer option to clear chat history from within the group chat screen.

Areas that may be impacted

Chat option to clear history.

Steps to test

Please refer to the reproduction steps specified in the issue. With the fix applied, you should see the group chat name in the drawer when you try to clear the history from the list of group chats and from within each group chat options drawer.

status: ready

:empty-chat?
(not (:last-message current-chat)))))
(fn [chat]
{:able-to-send-message? (:able-to-send-message? chat)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In mobile apps using CLJS I prefer to reserve select-keys only for cases where the list of keys is dynamic. Here we know the fixed set of keys, so I extract directly. Destructuring made the code look longer because the destructured keys span over multiple lines, plus the returned map.

Copy link
Contributor

Choose a reason for hiding this comment

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

We know select-keys and merge are expensive operations, so this change makes a lot of sense 👍

@status-im-auto
Copy link
Member

status-im-auto commented Nov 26, 2024

Jenkins Builds

Click to see older builds (4)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ acc6321 #1 2024-11-26 12:36:55 ~4 min tests 📄log
✔️ acc6321 #1 2024-11-26 12:40:21 ~8 min android-e2e 🤖apk 📲
✔️ acc6321 #1 2024-11-26 12:41:06 ~8 min android 🤖apk 📲
✔️ acc6321 #1 2024-11-26 12:41:46 ~9 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b19b559 #2 2024-11-27 20:22:53 ~4 min tests 📄log
✔️ b19b559 #2 2024-11-27 20:26:17 ~7 min android-e2e 🤖apk 📲
✔️ b19b559 #2 2024-11-27 20:26:57 ~8 min android 🤖apk 📲
✔️ b19b559 #2 2024-11-27 20:27:28 ~8 min ios 📱ipa 📲
✔️ 100a26a #3 2024-11-29 09:44:29 ~4 min tests 📄log
✔️ 100a26a #3 2024-11-29 09:46:59 ~7 min android 🤖apk 📲
✔️ 100a26a #3 2024-11-29 09:47:45 ~7 min android-e2e 🤖apk 📲
✔️ 100a26a #3 2024-11-29 09:48:48 ~8 min ios 📱ipa 📲

Copy link
Contributor

@ulisesmac ulisesmac left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @ilmotta !

@ilmotta ilmotta force-pushed the ilmotta-21639/fix-missing-group-chat-name branch from acc6321 to b19b559 Compare November 27, 2024 20:18
@status-im-auto
Copy link
Member

88% of end-end tests have passed

Total executed tests: 8
Failed tests: 0
Expected to fail tests: 1
Passed tests: 7
IDs of expected to fail tests: 702843 

Expected to fail tests (1)

Click to expand

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Test is not run, e2e blocker  

[[reason: [NOTRUN] Skipped due to waku issue on staging fleet]]

Passed tests (7)

Click to expand

Class TestCommunityOneDeviceMerged:

1. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

2. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

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

@status-im-auto
Copy link
Member

88% of end-end tests have passed

Total executed tests: 8
Failed tests: 0
Expected to fail tests: 1
Passed tests: 7
IDs of expected to fail tests: 702843 

Expected to fail tests (1)

Click to expand

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Test is not run, e2e blocker  

[[reason: [NOTRUN] Skipped due to waku issue on staging fleet]]

Passed tests (7)

Click to expand

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

2. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

@Horupa-Olena Horupa-Olena self-assigned this Nov 29, 2024
@Horupa-Olena Horupa-Olena force-pushed the ilmotta-21639/fix-missing-group-chat-name branch from b19b559 to 100a26a Compare November 29, 2024 09:39
@Horupa-Olena
Copy link

@ilmotta Thank you for your PR!
Everything looks great! The PR is ready to be merged.

photo_2024-11-29_13-30-31
photo_2024-11-29_13-30-27

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

Successfully merging this pull request may close these issues.

Group chat name is missing in confirming clear history dialog
5 participants