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

Fix/async call for mention suggestion #21171

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Fix/async call for mention suggestion #21171

merged 3 commits into from
Sep 19, 2024

Conversation

qfrank
Copy link
Contributor

@qfrank qfrank commented Sep 4, 2024 •
edited
Loading

Summary

This PR addresses an issue with asynchronous calls in the mention functionality. The main changes include:

  1. Enhanced mention result handling:

    • Added call-id and call-time to track individual calls
    • Updated the transfer-mention-result function to include these new fields

  2. Improved on-change-text event:

    • Introduced a call-id to uniquely identify each call
    • Added the current timestamp to the RPC call parameters

  3. Refined on-change-text-success event:

    • Implemented a check to ensure the response matches the most recent call
    • Added response time logging for performance monitoring
    • Only update the app state if the response is for the latest call

These changes aim to resolve race conditions and ensure that the UI always reflects the most recent user input when dealing with mentions. The addition of call tracking and response time logging will help with future performance optimizations.

Relate status-go PR

Testing notes

mention suggestion should work as before

Platforms

  • Android
  • iOS

status: ready

@qfrank qfrank requested review from ilmotta and flexsurfer September 4, 2024 09:22
@qfrank qfrank self-assigned this Sep 4, 2024
@qfrank qfrank mentioned this pull request Sep 4, 2024
Merged
@status-im-auto
Copy link
Member

status-im-auto commented Sep 4, 2024 •
edited
Loading

Jenkins Builds

Click to see older builds (27)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 4a3bb35 #1 2024-09-04 09:27:23 ~4 min tests 📄log
✔️ 4a3bb35 #1 2024-09-04 09:30:43 ~8 min android-e2e 🤖apk 📲
✔️ 4a3bb35 #1 2024-09-04 09:32:04 ~9 min android 🤖apk 📲
✔️ 4a3bb35 #1 2024-09-04 09:34:03 ~11 min ios 📱ipa 📲
✔️ 13e2c6c #3 2024-09-10 00:56:46 ~4 min tests 📄log
✔️ 13e2c6c #3 2024-09-10 00:58:34 ~6 min android-e2e 🤖apk 📲
✔️ 13e2c6c #3 2024-09-10 01:00:22 ~8 min android 🤖apk 📲
✔️ 13e2c6c #3 2024-09-10 01:02:02 ~9 min ios 📱ipa 📲
✔️ 1e167a4 #4 2024-09-10 02:03:10 ~5 min tests 📄log
✔️ 1e167a4 #4 2024-09-10 02:06:01 ~8 min android-e2e 🤖apk 📲
✔️ 1e167a4 #4 2024-09-10 02:06:30 ~9 min android 🤖apk 📲
✔️ 1e167a4 #4 2024-09-10 02:07:15 ~9 min ios 📱ipa 📲
✔️ ffc408c #5 2024-09-10 06:25:32 ~4 min tests 📄log
✔️ ffc408c #5 2024-09-10 06:29:08 ~8 min android-e2e 🤖apk 📲
✔️ ffc408c #5 2024-09-10 06:29:35 ~8 min android 🤖apk 📲
✔️ ffc408c #5 2024-09-10 06:34:35 ~13 min ios 📱ipa 📲
✔️ 41a0e3c #6 2024-09-10 09:36:36 ~4 min tests 📄log
✔️ 41a0e3c #6 2024-09-10 09:39:27 ~7 min android-e2e 🤖apk 📲
✔️ 41a0e3c #6 2024-09-10 09:42:26 ~10 min ios 📱ipa 📲
✔️ 41a0e3c #6 2024-09-10 09:43:34 ~11 min android 🤖apk 📲
❌ d097106 #7 2024-09-12 00:07:36 ~1 min android-e2e 📄log
❌ d097106 #7 2024-09-12 00:07:43 ~1 min ios 📄log
❌ d097106 #7 2024-09-12 00:08:51 ~2 min android 📄log
❌ d097106 #7 2024-09-12 00:09:57 ~3 min tests 📄log
❌ aa73e87 #8 2024-09-17 23:38:41 ~1 min android-e2e 📄log
❌ aa73e87 #8 2024-09-17 23:38:50 ~1 min android 📄log
❌ aa73e87 #8 2024-09-17 23:38:58 ~1 min ios 📄log
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 5626e17 #9 2024-09-17 23:44:42 ~5 min tests 📄log
✔️ 5626e17 #9 2024-09-17 23:46:22 ~6 min android-e2e 🤖apk 📲
✔️ 5626e17 #9 2024-09-17 23:48:44 ~9 min android 🤖apk 📲
✔️ 5626e17 #9 2024-09-17 23:51:14 ~11 min ios 📱ipa 📲
✔️ 0f015ce #10 2024-09-19 05:15:36 ~5 min tests 📄log
✔️ 0f015ce #10 2024-09-19 05:18:07 ~7 min android-e2e 🤖apk 📲
✔️ 0f015ce #10 2024-09-19 05:18:14 ~7 min android 🤖apk 📲
✔️ 0f015ce #10 2024-09-19 05:21:48 ~11 min ios 📱ipa 📲

@flexsurfer
Copy link
Member

thank you @qfrank , there is a PR with composer simplification, #20125 , there were changes and improvements for mentions as well, i would wait for it merged first

ilmotta
ilmotta reviewed Sep 10, 2024
View reviewed changes
src/legacy/status_im/chat/models/mentions.cljs Outdated Show resolved Hide resolved
src/legacy/status_im/chat/models/mentions.cljs Show resolved Hide resolved
src/legacy/status_im/chat/models/mentions.cljs Outdated Show resolved Hide resolved
ios/Podfile.lock Outdated Show resolved Hide resolved
@qfrank qfrank force-pushed the fix/async_call_for_mention branch from ffc408c to 41a0e3c Compare September 10, 2024 09:31
@churik churik added the blocked label Sep 10, 2024
@churik
Copy link
Member

churik commented Sep 10, 2024

see #21171 (comment)

@qfrank qfrank force-pushed the fix/async_call_for_mention branch from 41a0e3c to d097106 Compare September 12, 2024 00:06
@churik churik removed the blocked label Sep 17, 2024
@churik
Copy link
Member

churik commented Sep 17, 2024

@qfrank not sure why builds are failed, can you have a look?
Thanks!

@pavloburykh
Copy link
Contributor

@qfrank not sure why builds are failed, can you have a look? Thanks!

@qfrank could you please also rebase corresponding go PR and update mobile PR as well? Thank you!

@qfrank qfrank force-pushed the fix/async_call_for_mention branch 2 times, most recently from aa73e87 to 5626e17 Compare September 17, 2024 23:39
@qfrank
Copy link
Contributor Author

qfrank commented Sep 17, 2024

Sorry for the delay. I'm just back from holiday. Fixed @churik @pavloburykh

@pavloburykh pavloburykh self-assigned this Sep 18, 2024
@status-im-auto
Copy link
Member

100% of end-end tests have passed

Total executed tests: 51
Failed tests: 0
Expected to fail tests: 0
Passed tests: 51

Passed tests (51)

Click to expand

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

1. test_activity_center_admin_notification_accept_swipe, id: 702958
Device sessions

2. test_activity_center_mentions, id: 702957
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 TestActivityMultipleDevicePR:

1. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
Device sessions

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

@pavloburykh
Copy link
Contributor

@qfrank thank you for the PR. No issues from my side. Ready for merge.

@qfrank qfrank force-pushed the fix/async_call_for_mention branch from 5626e17 to 0f015ce Compare September 19, 2024 05:10
@qfrank qfrank merged commit 2e61f94 into develop Sep 19, 2024
6 checks passed
@qfrank qfrank deleted the fix/async_call_for_mention branch September 19, 2024 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siddarthkay siddarthkay siddarthkay left review comments

@ilmotta ilmotta ilmotta approved these changes

@flexsurfer flexsurfer flexsurfer approved these changes

Assignees

@qfrank qfrank

@pavloburykh pavloburykh

Labels
request-manual-qa status-go Tested - OK
Projects
Pipeline for QA
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@qfrank @status-im-auto @flexsurfer @churik @pavloburykh @ilmotta @siddarthkay