Page MenuHomePhabricator

[lib/native] fix @-mentioning in a create new pending GENSIS chat crashes app
ClosedPublic

Authored by ginsu on Feb 5 2024, 3:02 PM.
Tags
None
Referenced Files
F2167127: D10957.id36793.diff
Tue, Jul 2, 5:55 AM
Unknown Object (File)
Mon, Jul 1, 3:45 AM
Unknown Object (File)
Sun, Jun 30, 3:25 AM
Unknown Object (File)
Wed, Jun 26, 9:38 AM
Unknown Object (File)
Mon, Jun 24, 9:29 PM
Unknown Object (File)
Mon, Jun 24, 10:19 AM
Unknown Object (File)
Fri, Jun 21, 2:44 PM
Unknown Object (File)
Fri, Jun 21, 12:44 AM
Subscribers

Details

Summary

After putting up a solution for fixing @ mentioning in pending GENESIS sidebars in D10956, as a sanity check I wanted to check other places where we have pending threads and see if these also have issues when using the @ mentioning feature. I'm glad I did, because I also found an issue when creating a new standard pending GENESIS chat and using the @ mentioning would cause the app to crash.

The cause of the crash was the same as in D10956 where we were attempting to get the chat mention search index from the communityThreadIDForGenesisThreads, but since this new pending thread wasn't resolved yet, it is not part of communityThreadIDForGenesisThreads so we were returning an chatSearchIndex as undefined even though in useMentionTypeaheadChatSuggestions we guarantee that there is a value there.

A pending standard GENESIS chat shouldn't have any resolved sidebars or subchannels, so we really shouldn't be returning anything from useMentionTypeaheadChatSuggestions in the first place. A solution that made sense to me here was to type the chatSearchIndex parameter as optional and have a check where if the chatSearchIndex is not set we just return the blank array that we initialized in the beginning as our list of MentionTypeaheadChatSuggestionItems.

Depends on D10956

Test Plan

Please see demo video below (also confirmed that native counterpart works too)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 5 2024, 3:13 PM
Harbormaster failed remote builds in B26522: Diff 36657!
ginsu retitled this revision from [lib] fix @-mentining in a create new pending GENSIS chat crashes app to [lib/native] fix @-mentining in a create new pending GENSIS chat crashes app.Feb 5 2024, 3:29 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu retitled this revision from [lib/native] fix @-mentining in a create new pending GENSIS chat crashes app to [lib/native] fix @-mentioning in a create new pending GENSIS chat crashes app.Feb 5 2024, 3:31 PM
ginsu requested review of this revision.Feb 5 2024, 3:45 PM

Seems reasonable, but I don't have a ton of context. Adding @ashoat as blocking since he's reviewing D10956 as well.

This revision is now accepted and ready to land.Feb 6 2024, 10:03 AM
This revision was landed with ongoing or failed builds.Feb 7 2024, 11:11 AM
This revision was automatically updated to reflect the committed changes.