Page MenuHomePhabricator

[web] introduce localID field to useOnClickReact hook
ClosedPublic

Authored by ginsu on Dec 30 2022, 3:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 1, 9:22 AM
Unknown Object (File)
Mon, Jul 1, 9:22 AM
Unknown Object (File)
Mon, Jul 1, 9:22 AM
Unknown Object (File)
Mon, Jul 1, 9:12 AM
Unknown Object (File)
Sat, Jun 29, 8:32 AM
Unknown Object (File)
Tue, Jun 25, 4:14 PM
Unknown Object (File)
Tue, Jun 25, 5:15 AM
Unknown Object (File)
Sun, Jun 23, 11:23 PM
Subscribers

Details

Summary

introduce localID field to useOnClickReact hook. This diff also handles creating the localID variable on the client using the localIDPrefix and nextLocalID state


Depends on D6118
Linear Task: ENG-2519

Test Plan

Please watch this demo video to show that we were able to get the localID succesfully from the client to db

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Dec 30 2022, 3:59 PM

We also need to increase the next local id when a message is sent, but I guess it is handled later in the stack.

This revision is now accepted and ready to land.Jan 2 2023, 5:17 AM
tomek requested changes to this revision.Jan 2 2023, 5:23 AM

Checked the rest of the stack and it doesn't seem like incrementing the id is handled. We should extend local-id-reducer to react on appropriate action.

To test this thoroughly, we can use a following scenario:

  1. Send a reaction request and add a sleep on server side
  2. Send a text message without a sleep (so that it finishes before reaction response)
  3. Finish sleeping on server side

This scenario might cause some issues when incrementation isn't performed.

This revision now requires changes to proceed.Jan 2 2023, 5:23 AM
ginsu requested review of this revision.Jan 3 2023, 10:29 AM

Checked the rest of the stack and it doesn't seem like incrementing the id is handled. We should extend local-id-reducer to react on appropriate action.

Just added D6123 to the stack which should address these concerns

This revision is now accepted and ready to land.Jan 5 2023, 4:00 AM
ginsu edited the summary of this revision. (Show Details)

rebase before landing