[web] Fix issue with fetching messages after creating pending thread
ClosedPublic
Actions

Authored by • jacek on May 20 2022, 4:37 AM.

Details

Reviewers

tomek
atul
• benschac

Commits

rCOMM51edd510d5db: [web] Fix issue with fetching messages after creating pending thread

Summary

Fixes the issue: https://linear.app/comm/issue/ENG-1170/after-creating-any-pending-threadsidebar-on-web-app-dont-fetch-any
There are two changes introduced in the diff:

remove pendingThread from navInfo in nav-reducer if activeChatThreadID is not pending
replace checking pendingThread with checking if current threadInfo is pending in chat-message-list.react.js

Each of these things should fix the original issue, but I think both of them should be introduced to prevent accidentally re-introducing the issue in the future

Test Plan

Follow steps described in Linear task. Messages should be always fetched from server

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jacek created this revision.May 20 2022, 4:37 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 20 2022, 4:37 AM

Harbormaster completed remote builds in B9364: Diff 12977.May 20 2022, 4:42 AM

• jacek requested review of this revision.May 20 2022, 4:42 AM

Thanks for including the Linear link. It was helpful to see a description of the issue, a video of the issue, and a discussion of solutions. Made the diff easy to review.

web/redux/nav-reducer.js
39 ↗	(On Diff #12977)	Doesn't really matter, but I wonder if switching these positionally eg if (state.pendingThread && !threadIsPending(state.activeChatThreadID) { makes a tiny bit more sense? Because it seems like if there's no `state.pendingThread` we can short-circuit and skip checking if the active chat thread id is pending? Not that it makes any performance difference, but I think it's kind of like an "early return" in that you get the more critical condition "out of the way" first?