Page MenuHomePhabricator

Conpherence - make sure real time updates still work if you switch threads
ClosedPublic

Authored by btrahan on May 7 2015, 7:37 PM.
Tags
None
Referenced Files
F14063452: D12758.diff
Mon, Nov 18, 6:37 PM
F14054818: D12758.diff
Sat, Nov 16, 5:27 AM
F14042649: D12758.diff
Tue, Nov 12, 5:09 AM
F14026754: D12758.diff
Fri, Nov 8, 2:03 AM
F14006865: D12758.diff
Mon, Oct 28, 7:31 PM
F13997160: D12758.id30663.diff
Thu, Oct 24, 2:42 AM
F13980799: D12758.diff
Oct 19 2024, 12:36 PM
F13969567: D12758.diff
Oct 17 2024, 3:09 AM
Subscribers

Details

Summary

Fixes T8118. Turns out this also was broken in the main view if e.g. you went to just /conpherence/. The fix is to make sure the threadmanager js class explicitly manages subscriptions as the loaded thread changes.

Test Plan
  • from /conpherence/ was able to receive messages in real time.
  • from /conpherence/ changed threads and still received messages in real time
  • from durable column switched threads and received messages in real time

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Conpherence - make sure real time updates still work if you switch threads.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js
285–287

whoops, should be PHID not ID

epriestley edited edge metadata.
epriestley added inline comments.
webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js
282–304

Maybe simpler like this?

var old_subs = client.getSubscriptions();
var new_subs = [];
for (var ii = 0; ii < old_subs.length; ii++) {
  if (old_subs[ii] == this._loadedThreadPHID) {
    new_subs.push(r.threadPHID);
  } else {
    new_subs.push(old_subs[ii]);
  }
}
This revision is now accepted and ready to land.May 7 2015, 7:40 PM
btrahan edited edge metadata.
  • ID => PHID
  • require aphlict package in thread manager
  • celerity map update
webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js
281–303

I'll see if I can make it better, but the subscriptions don't necessarily have this._loadedThreadPHID in them. e.g. the /conpherence/ case

This revision was automatically updated to reflect the committed changes.