[lib] Update `messageStore.messages` with `msgStoreOps` for processServerRequests action
ClosedPublic
Actions

Authored by atul on Jun 2 2022, 6:45 PM.

Details

Reviewers

• jacek
tomek

Commits

rCOMM8b9aceb81ad5: [lib] Update `messageStore.messages` with `msgStoreOps` for…

Summary

Instead of getting messageStore.messages from the messageStoreAfterReassignment returned by reassignMessagesToRealizedThreads(...)... we use the messageStoreOps returned by reassignMessagesToRealizedThreads(...) to compute the new messageStore.messages.

Note that a messageStore.messages is still being created within reassignMessagesToRealizedThreads(...) and will be removed in a subsequent diff

Depends on D4202

Test Plan

This should be a straightforward refactor given we've had assertMessageStoreMessagesAreEqual in place for some time now.

Diff Detail

Repository

rCOMM Comm

Branch

landjune21 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Jun 2 2022, 6:45 PM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptJun 2 2022, 6:45 PM

Harbormaster completed remote builds in B9608: Diff 13319.Jun 2 2022, 6:53 PM

atul requested review of this revision.Jun 2 2022, 6:53 PM

atul added a child revision: D4204: [lib] Update `messageStore.messages` with `msgStoreOps` for createLocalMessage.Jun 2 2022, 7:03 PM

tomek requested changes to this revision.Jun 3 2022, 4:30 AM

tomek added inline comments.

lib/reducers/message-reducer.js
1472–1475 ↗	(On Diff #13319)	We were checking if `processedMessageStore` is equal to `messageStoreAfterReassignment` so we should use `processedMessageStore`, right?

This revision now requires changes to proceed.Jun 3 2022, 4:30 AM

atul requested review of this revision.Jun 17 2022, 1:46 PM

atul added inline comments.

lib/reducers/message-reducer.js
1472–1475 ↗	(On Diff #13319)	Right now the "ops-approach" only modifies `messageStore.messages`, we'll need to "get the rest" of the messageStore using the old way `isEqual(messageStore, processedMessageStore) === false` `isEqual(messageStore.messages, processedMessageStore.messages) === true`

tomek accepted this revision.Jun 20 2022, 2:52 AM

tomek added inline comments.

lib/reducers/message-reducer.js
1472–1475 ↗	(On Diff #13319)	isEqual(messageStore, processedMessageStore) === false Ok, that makes sense. But in the older code we had: assertMessageStoreMessagesAreEqual( processedMessageStore, messageStoreAfterReassignment, action.type, ); So it should be possible to replace `messageStoreAfterReassignment` with `processedMessageStore`, right?

This revision is now accepted and ready to land.Jun 20 2022, 2:52 AM

atul added inline comments.Jun 21 2022, 11:53 PM

lib/reducers/message-reducer.js
1472–1475 ↗	(On Diff #13319)	So it should be possible to replace messageStoreAfterReassignment with processedMessageStore, right? We were asserting that `messageStoreAfterReassignment.messages` and `processedMessageStore.messages` were equal... but for example `messageStoreAfterReassignment.threads` and `processedMessageStore.threads` are not equal so we can't just swap them out.