Page MenuHomePhabricator

[lib] Save unsupported operations in the DB
ClosedPublic

Authored by tomek on Mon, Mar 17, 7:05 AM.
Tags
None
Referenced Files
F4947189: D14454.diff
Wed, Mar 19, 1:26 PM
Unknown Object (File)
Mon, Mar 17, 8:25 AM
Unknown Object (File)
Mon, Mar 17, 8:23 AM
Unknown Object (File)
Mon, Mar 17, 8:18 AM
Subscribers

Details

Summary

The unsupported operations aren't saved in Redux, but it's still beneficial to use the same approach because it's more consistent, allows easier modification in the future (if we decide to keep all the operations in the DB, and avoids introducing a new mechanism.

https://linear.app/comm/issue/ENG-10329/create-a-new-action-and-modify-the-reducedmoperationsqueue-reducer

Depends on D14453

Test Plan

Tested the whole stack on both native and web:

  1. Run the app on Redux version 86 with a modification to processDMOperation making all the INBOUND messages unsupported
  2. On one platform created a thread and sent a text message. These were visible on the sender, and invisible on the recipient
  3. Closed the recipient and added a migration that unshimms the thread creation operations, e.g.
unshimDMOperations(
  state,
  dmOperationTypes.CREATE_THREAD,
  handleReduxMigrationFailure,
): MigrationFunction<WebNavInfo, AppState>),
  1. Run the recipient and noticed the thread appearing
  2. Added another migration, this time unshimming text messages
  3. Run the recipient and noticed messages appearing

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.Mon, Mar 17, 7:33 AM
Harbormaster failed remote builds in B33730: Diff 47419!
tomek requested review of this revision.Tue, Mar 18, 4:00 AM
kamil added inline comments.
lib/shared/dm-ops/process-dm-ops.js
116

I think adding this here might cause calling confirmPeerToPeerMessage twice, here below and in DBOpsHandler, worth checking to avoid unnecessary messages sent to other peers.

lib/types/dm-ops.js
588

not sure if this needed but maybe worth be more specific

This revision is now accepted and ready to land.Tue, Mar 18, 7:57 AM

Remove extraneous confirmations

lib/shared/dm-ops/process-dm-ops.js
116

You're right! Tested it and a message was confirmed twice. Removing extraneous calls.

This revision was automatically updated to reflect the committed changes.