This differential implements synchronizing thread subscription in dm threads.
Details
Details
- Reviewers
kamil tomek - Commits
- rCOMMa0bb60037921: Implement updating DM thread subscription
- Create dm thread and add another user. (Use UI button implemented by Kamil)
- Play around with changing thread subscription and sending notifs.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-8522
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/dm-ops/change-thread-subscription.js | ||
|---|---|---|
| 38–46 ↗ | (On Diff #43879) | We need to introduce a new timestamp that should be used to check whether the subscription should be updated. |
| lib/shared/thread-settings-notifications-utils.js | ||
| 6–7 ↗ | (On Diff #43879) | Can be merged |
| 178–180 ↗ | (On Diff #43879) | We should use dispatchActionPromise so that the status can be tracked. |
Comment Actions
- Address review
- Refactor to execute DM logic in hook instead of component to match latest convention
| lib/shared/dm-ops/change-thread-subscription.js | ||
|---|---|---|
| 38–46 ↗ | (On Diff #43879) | Discussed it offline. We don't need to introduce a new timestamp, because we already have one in members[viewerID]. We don't want to duplicate it because then it will become possible to break the consistency. |
| lib/shared/thread-settings-notifications-utils.js | ||
| 178–180 ↗ | (On Diff #43879) | Should we use the dispatchActionPromise? |
| lib/shared/thread-settings-notifications-utils.js | ||
|---|---|---|
| 178–180 ↗ | (On Diff #43879) | This question refers to the old version of this diff. Latest version doesn't have this issue anymore. |