Page MenuHomePhabricator

Implement DMActivityHandler
ClosedPublic

Authored by marcin on Thu, Sep 5, 6:15 AM.
Tags
None
Referenced Files
F2711363: D13246.id43999.diff
Sun, Sep 15, 11:11 PM
F2711308: D13246.id44001.diff
Sun, Sep 15, 10:37 PM
F2708728: D13246.id44000.diff
Sun, Sep 15, 4:17 PM
F2708702: D13246.id43896.diff
Sun, Sep 15, 4:03 PM
F2708178: D13246.id43992.diff
Sun, Sep 15, 1:49 PM
F2707618: D13246.id43895.diff
Sun, Sep 15, 12:29 PM
F2704761: D13246.id.diff
Sun, Sep 15, 4:30 AM
Unknown Object (File)
Sat, Sep 14, 5:56 PM
Subscribers

Details

Summary

This differential implements DMActivityHandler component. Its role is the same as ActivityHandler but it works thick threads. I opted for new compoment instead of modifying ActivityHandler since we only need one component for thick threads while there are many ActivityHandlers (one for each keyserver). Therefore I couldn't see an easy way to reuse ActivityHandler.

Test Plan
  1. Create thick thread between users A and B.
  2. Log in as user A on two devices (on web and on native) and log in as user B on third device.
  3. Send message from B to A. Open the thread as A on web. Ensure rescinding happens on native.
  4. Open the thread as A on native. Change opened thread as A on web. Send messages from B to A. Ensure that thread remains read on web.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Remove unwanted change in version-utils.js

marcin requested review of this revision.Thu, Sep 5, 6:34 AM
tomek added inline comments.
lib/handlers/dm-activity-handler.js
4–5 ↗(On Diff #43896)

Can be merged.

This revision is now accepted and ready to land.Fri, Sep 6, 8:29 AM

Enable tracking update activity action for thick threads

This revision was landed with ongoing or failed builds.Mon, Sep 9, 9:06 AM
This revision was automatically updated to reflect the committed changes.
lib/handlers/dm-activity-handler.js
92–95

What's the point of this? Does processAndSendDMOperation already dispatch actions? Given you're returning empty collections on line 46, it seems like no reducers will have any changes as a result of these actions... so probably best to remove these lines.

@marcin, if this makes sense to you, can you remove these lines as part of the same stack as D13260, and land the fixes together?

native/components/dm-activity-handler.react.js
1

Nit: we always have an extra newline after this

12–21

Looks like most of this was copy-pasted from native/socket.react.js. Could you factor it out in a follow-up diff?

web/components/dm-activity-handler.react.js
1

Same here

lib/handlers/dm-activity-handler.js
92–95

I figured that we might want to task status of this action. This is the approach that we are already using in, for example, useChangeThreadSettings. If we are running for thick threads we return empty collections so that we can call it from dispatchActionPromise and track its status.

lib/handlers/dm-activity-handler.js
92–95

Unless we have a reason for it (such as tracking loadingStatus), I think we should remove the dispatch

and track its status

Do we track the status of updateActivityActionTypes?