Page MenuHomePhabricator

[server] Support pending threads when calculating `finalNavInfo`
ClosedPublic

Authored by jacek on Jul 4 2022, 8:37 AM.
Tags
None
Referenced Files
F1786971: D4435.id14146.diff
Sat, May 18, 6:25 PM
Unknown Object (File)
Wed, May 1, 1:31 AM
Unknown Object (File)
Wed, May 1, 1:31 AM
Unknown Object (File)
Wed, May 1, 1:31 AM
Unknown Object (File)
Wed, May 1, 1:28 AM
Unknown Object (File)
Wed, May 1, 1:10 AM
Unknown Object (File)
Apr 5 2024, 5:02 PM
Unknown Object (File)
Apr 4 2024, 4:53 PM

Details

Summary

Currently, on server side, when calculating initial navInfo, there is a check that redirects user to latest thread, if the user has no permission to read the thread.
However, there is no check if thread with ID calculated from URL even exist in store. As we want to support navigation with pending threads, the non-existing thread IDs should not be removed on server side.

Test Plan

The app behavior remains unchanged, if non-existing ID is entered, the user is still redirected correctly. Support for pending threads on React side will be introduced in following diffs.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat requested changes to this revision.Jul 4 2022, 1:10 PM

Do you think we can check threadIsPending instead? I think it's probably good that we unset the activeChatThreadID if it is non-pending but does not exist in threadInfos.

This revision now requires changes to proceed.Jul 4 2022, 1:10 PM

Do you think we can check threadIsPending instead? I think it's probably good that we unset the activeChatThreadID if it is non-pending but does not exist in threadInfos.

Agree - the isPending check is even more precise condition here. Although, there is no much difference in both cases - when checking threadInfos[requestedActiveChatThreadID] the ID for non-existing non-pending thread was unset in frontend side. But it's probably better to handle this case here in responder.

Change check to threadIsPending

This revision is now accepted and ready to land.Jul 5 2022, 11:54 AM