[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

Branch

jacek/chat-fetch-issue

Lint

No Lint Coverage

Unit

No Test Coverage

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	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?