Page MenuHomePhabricator

[lib/native/web] handle send reaction message action failed case
ClosedPublic

Authored by ginsu on Jan 3 2023, 1:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 28, 11:25 AM
Unknown Object (File)
Thu, Nov 28, 11:25 AM
Unknown Object (File)
Thu, Nov 28, 11:25 AM
Unknown Object (File)
Tue, Nov 26, 9:41 AM
Unknown Object (File)
Tue, Nov 26, 9:41 AM
Unknown Object (File)
Tue, Nov 26, 7:16 AM
Unknown Object (File)
Sun, Nov 24, 11:56 AM
Unknown Object (File)
Mon, Nov 4, 5:09 AM
Subscribers

Details

Summary

handle send reaction message action failed case. Since there is no retry behavior for failed reaction messages right now, if a reaction message fails to get sent, I remove the local reaction message from the message store, and an alert telling the user that the message failed to send pops up


Depends on D6153
Linear Task: ENG-2600

Test Plan

Please watch the demo videos to see how the redux state changes when a sent reaction message action starts:

Native:

Web:

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.Jan 3 2023, 1:19 PM
Harbormaster failed remote builds in B15051: Diff 20532!
ginsu requested review of this revision.Jan 3 2023, 1:45 PM

(No reviewers here)

Was having issues with my demo videos should be resolved in < 5min

tomek requested changes to this revision.Jan 5 2023, 8:23 AM
tomek added inline comments.
lib/reducers/message-reducer.js
1015 ↗(On Diff #20532)

Shouldn't we also filter this? Are we adding pending reaction messages to it?

This revision now requires changes to proceed.Jan 5 2023, 8:23 AM
ginsu requested review of this revision.Jan 5 2023, 11:17 AM
ginsu added inline comments.
lib/reducers/message-reducer.js
1015 ↗(On Diff #20532)

We don't need to filter this because we are not adding pending reaction messages to it. Eventually we should add pending reaction messages to local when we want to implement retry behavior.

Screenshot 2023-01-05 at 2.14.49 PM.png (1×3 px, 1 MB)

tomek added inline comments.
lib/reducers/message-reducer.js
1015 ↗(On Diff #20532)

Ok, that makes sense. But I'm wondering now if it would be safer if we took local from the processed store.

return {
      messageStoreOperations,
      messageStore: {
        ...processedMessageStore,
        threads: {
          ...messageStore.threads,
          [threadID]: {
            ...messageStore.threads[threadID],
            messageIDs: newMessageIDs,
          },
        },
      },
    };
This revision is now accepted and ready to land.Jan 9 2023, 2:53 AM
ginsu edited the test plan for this revision. (Show Details)

address tomek's feedback