Page MenuHomePhabricator

[lib][native][web] Update useSidebarInfos to include lastUpdatedAtLeastTime
ClosedPublic

Authored by ashoat on Nov 12 2024, 12:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 18, 4:13 PM
Unknown Object (File)
Wed, Dec 18, 4:13 PM
Unknown Object (File)
Wed, Dec 18, 4:13 PM
Unknown Object (File)
Wed, Dec 18, 4:13 PM
Unknown Object (File)
Wed, Dec 18, 4:13 PM
Unknown Object (File)
Wed, Dec 18, 4:13 PM
Unknown Object (File)
Wed, Dec 18, 3:57 PM
Unknown Object (File)
Wed, Dec 18, 3:57 PM
Subscribers
None

Details

Summary

Before this diff, useSidebarInfos would return lastUpdatedAtLeastTime as lastUpdatedTime, as a temporary hack to help split up my work into smaller diffs.

In an earlier diff we updated lastUpdatedTime to be a Promise, and this allows us to avoid refactoring everything to handle the Promise.

In this diff, we address this temporary hack for sidebars, but for not for the ChatThreadItem itself (which will be handled later).

useSidebarInfos is used in places outside of chat-selectors.js, so this required some additional refactoring.

Depends on D13914

Test Plan

Tested in combination with the rest of the stack. See video in D13918

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 12 2024, 1:24 PM
Harbormaster failed remote builds in B32633: Diff 45769!
tomek added inline comments.
lib/hooks/search-threads.js
145–159

Wondering whether we shouldn't just use the deep equal from lodash instead. If we have some checks for the ID change, should we protect ourselves against the change to some other fields?

This revision is now accepted and ready to land.Nov 13 2024, 4:38 AM
lib/hooks/search-threads.js
145–159

In this case, we are comparing the results of getAllFinalSidebarItems against getAllInitialSidebarItems for the same input. Based on the implementation of those functions, the only field that might be different is lastUpdatedTime

This revision is now accepted and ready to land.Tue, Dec 10, 8:24 AM