DEV: Refactor presence manager to deal with multiple composer states.

This change amends it so we use a static service to keep track of
the typing presence.

It correct various edge cases the initial implementation had

- Faster close messages
- When composing on topic 1 and viewing topic 2 we had incorrect
  presence
- Changing a running composer to reply as new topic or reply to a
  differet topic would not correctly shift presence

Authored by tgxworld, with contributions by sam

plugins/discourse-presence/assets/javascripts/discourse/components/composer-presence-display.js.es6

@@ -1,11 +1,17 @@
 import Component from "@ember/component";
+import { getOwner } from "@ember/application";
 import { cancel } from "@ember/runloop";
-import { equal, gt, readOnly } from "@ember/object/computed";
+import { equal, gt } from "@ember/object/computed";
 import discourseComputed, {
   observes,
   on
 } from "discourse-common/utils/decorators";
-import { REPLYING, CLOSED, EDITING } from "../lib/presence-manager";
+import {
+  REPLYING,
+  CLOSED,
+  EDITING,
+  COMPOSER_TYPE
+} from "../lib/presence-manager";
 import { REPLY, EDIT } from "discourse/models/composer";
 
 export default Component.extend({
@@ -16,46 +22,72 @@ export default Component.extend({
   reply: null,
   title: null,
   isWhispering: null,
+  presenceManager: null,
+
+  init() {
+    this._super(...arguments);
+
+    this.setProperties({
+      presenceManager: getOwner(this).lookup("presence-manager:main")
+    });
+  },
+
+  @discourseComputed("topic.id")
+  users(topicId) {
+    return this.presenceManager.users(topicId);
+  },
+
+  @discourseComputed("topic.id")
+  editingUsers(topicId) {
+    return this.presenceManager.editingUsers(topicId);
+  },
 
-  presenceManager: readOnly("topic.presenceManager"),
-  users: readOnly("presenceManager.users"),
-  editingUsers: readOnly("presenceManager.editingUsers"),
   isReply: equal("action", "reply"),
 
   @on("didInsertElement")
   subscribe() {
-    this.presenceManager.subscribe();
+    this.presenceManager.subscribe(this.get("topic.id"), COMPOSER_TYPE);
   },
 
   @discourseComputed(
     "post.id",
     "editingUsers.@each.last_seen",
-    "users.@each.last_seen"
+    "users.@each.last_seen",
+    "action"
   )
-  presenceUsers(postId, editingUsers, users) {
+  presenceUsers(postId, editingUsers, users, action) {
-    if (postId) {
+    if (action === EDIT) {
       return editingUsers.filterBy("post_id", postId);
-    } else {
+    } else if (action === REPLY) {
       return users;
     }
+    return [];
   },
 
   shouldDisplay: gt("presenceUsers.length", 0),
 
   @observes("reply", "title")
   typing() {
-    let action = this.action;
+    const action = this.action;
 
     if (action !== REPLY && action !== EDIT) {
       return;
     }
 
-    const postId = this.get("post.id");
+    let data = {
+      topicId: this.get("topic.id"),
+      state: action === EDIT ? EDITING : REPLYING,
+      whisper: this.whisper,
+      postId: this.get("post.id")
+    };
+
+    this._prevPublishData = data;
 
     this._throttle = this.presenceManager.throttlePublish(
-      action === EDIT ? EDITING : REPLYING,
+      data.topicId,
-      this.whisper,
+      data.state,
-      action === EDIT ? postId : undefined
+      data.whisper,
+      data.postId
     );
   },
 
@@ -64,20 +96,30 @@ export default Component.extend({
     this._cancelThrottle();
   },
 
-  @observes("post.id")
+  @observes("action", "topic.id")
-  stopEditing() {
+  composerState() {
-    if (!this.get("post.id")) {
+    if (this._prevPublishData) {
-      this.presenceManager.publish(CLOSED, this.whisper);
+      this.presenceManager.publish(
+        this._prevPublishData.topicId,
+        CLOSED,
+        this._prevPublishData.whisper,
+        this._prevPublishData.postId
+      );
+      this._prevPublishData = null;
     }
   },
 
   @on("willDestroyElement")
-  composerClosing() {
+  closeComposer() {
     this._cancelThrottle();
-    this.presenceManager.publish(CLOSED, this.whisper);
+    this._prevPublishData = null;
+    this.presenceManager.cleanUpPresence(COMPOSER_TYPE);
   },
 
   _cancelThrottle() {
-    cancel(this._throttle);
+    if (this._throttle) {
+      cancel(this._throttle);
+      this._throttle = null;
+    }
   }
 });

plugins/discourse-presence/assets/javascripts/discourse/components/topic-presence-display.js.es6

@@ -1,21 +1,32 @@
 import Component from "@ember/component";
-import { gt, readOnly } from "@ember/object/computed";
+import { getOwner } from "@ember/application";
-import { on } from "discourse-common/utils/decorators";
+import { gt } from "@ember/object/computed";
+import discourseComputed, { on } from "discourse-common/utils/decorators";
+import { TOPIC_TYPE } from "../lib/presence-manager";
 
 export default Component.extend({
   topic: null,
+  presenceManager: null,
+
+  init() {
+    this._super(...arguments);
+    this.set("presenceManager", getOwner(this).lookup("presence-manager:main"));
+  },
+
+  @discourseComputed("topic.id")
+  users(topicId) {
+    return this.presenceManager.users(topicId);
+  },
 
-  presenceManager: readOnly("topic.presenceManager"),
-  users: readOnly("presenceManager.users"),
   shouldDisplay: gt("users.length", 0),
 
   @on("didInsertElement")
   subscribe() {
-    this.presenceManager.subscribe();
+    this.presenceManager.subscribe(this.get("topic.id"), TOPIC_TYPE);
   },
 
   @on("willDestroyElement")
   _destroyed() {
-    this.presenceManager.unsubscribe();
+    this.presenceManager.unsubscribe(this.get("topic.id"), TOPIC_TYPE);
   }
 });

plugins/discourse-presence/assets/javascripts/discourse/initializers/discourse-presence.js.es6

@@ -0,0 +1,32 @@
+import { withPluginApi } from "discourse/lib/plugin-api";
+import PresenceManager from "../lib/presence-manager";
+import ENV from "discourse-common/config/environment";
+
+function initializeDiscoursePresence(api, { app }) {
+  const currentUser = api.getCurrentUser();
+
+  if (currentUser) {
+    app.register(
+      "presence-manager:main",
+      PresenceManager.create({
+        currentUser,
+        messageBus: api.container.lookup("message-bus:main"),
+        siteSettings: api.container.lookup("site-settings:main")
+      }),
+      { instantiate: false }
+    );
+  }
+}
+
+export default {
+  name: "discourse-presence",
+  after: "message-bus",
+
+  initialize(container, app) {
+    const siteSettings = container.lookup("site-settings:main");
+
+    if (siteSettings.presence_enabled && ENV.environment !== "test") {
+      withPluginApi("0.8.40", initializeDiscoursePresence, { app });
+    }
+  }
+};

plugins/discourse-presence/assets/javascripts/discourse/lib/presence-manager.js.es6

@@ -26,11 +26,14 @@ export const REPLYING = "replying";
 export const EDITING = "editing";
 export const CLOSED = "closed";
 
-const PresenceManager = EmberObject.extend({
+export const TOPIC_TYPE = "topic";
+export const COMPOSER_TYPE = "composer";
+
+const Presence = EmberObject.extend({
   users: null,
   editingUsers: null,
-  subscribed: null,
+  subscribers: null,
-  topic: null,
+  topicId: null,
   currentUser: null,
   messageBus: null,
   siteSettings: null,
@@ -41,46 +44,57 @@ const PresenceManager = EmberObject.extend({
     this.setProperties({
       users: [],
       editingUsers: [],
-      subscribed: false
+      subscribers: new Set()
     });
   },
 
-  subscribe() {
+  subscribe(type) {
-    if (this.subscribed) return;
+    if (this.subscribers.size === 0) {
+      this.messageBus.subscribe(
+        this.channel,
+        message => {
+          const { user, state } = message;
+          if (this.get("currentUser.id") === user.id) return;
 
-    this.messageBus.subscribe(
+          switch (state) {
-      this.channel,
+            case REPLYING:
-      message => {
+              this._appendUser(this.users, user);
-        const { user, state } = message;
+              break;
-        if (this.get("currentUser.id") === user.id) return;
+            case EDITING:
+              this._appendUser(this.editingUsers, user, {
+                post_id: parseInt(message.post_id, 10)
+              });
+              break;
+            case CLOSED:
+              this._removeUser(user);
+              break;
+          }
+        },
+        MESSAGE_BUS_LAST_ID
+      );
+    }
 
-        switch (state) {
+    this.subscribers.add(type);
-          case REPLYING:
-            this._appendUser(this.users, user);
-            break;
-          case EDITING:
-            this._appendUser(this.editingUsers, user, {
-              post_id: parseInt(message.post_id, 10)
-            });
-            break;
-          case CLOSED:
-            this._removeUser(user);
-            break;
-        }
-      },
-      MESSAGE_BUS_LAST_ID
-    );
-
-    this.set("subscribed", true);
   },
 
-  unsubscribe() {
+  unsubscribe(type) {
-    this.messageBus.unsubscribe(this.channel);
+    this.subscribers.delete(type);
-    this._stopTimer();
+    const noSubscribers = this.subscribers.size === 0;
-    this.set("subscribed", false);
+
+    if (noSubscribers) {
+      this.messageBus.unsubscribe(this.channel);
+      this._stopTimer();
+
+      this.setProperties({
+        users: [],
+        editingUsers: []
+      });
+    }
+
+    return noSubscribers;
   },
 
-  @discourseComputed("topic.id")
+  @discourseComputed("topicId")
   channel(topicId) {
     return `/presence/${topicId}`;
   },
@@ -101,14 +115,14 @@ const PresenceManager = EmberObject.extend({
 
     const data = {
       state,
-      topic_id: this.get("topic.id")
+      topic_id: this.topicId
     };
 
     if (whisper) {
       data.is_whisper = 1;
     }
 
-    if (postId) {
+    if (postId && state === EDITING) {
       data.post_id = postId;
     }
 
@@ -200,4 +214,73 @@ const PresenceManager = EmberObject.extend({
   }
 });
 
+const PresenceManager = EmberObject.extend({
+  presences: null,
+  currentUser: null,
+  messageBus: null,
+  siteSettings: null,
+
+  init() {
+    this._super(...arguments);
+
+    this.setProperties({
+      presences: {}
+    });
+  },
+
+  subscribe(topicId, type) {
+    if (!topicId) return;
+    this._getPresence(topicId).subscribe(type);
+  },
+
+  unsubscribe(topicId, type) {
+    if (!topicId) return;
+    const presence = this._getPresence(topicId);
+
+    if (presence.unsubscribe(type)) {
+      delete this.presences[topicId];
+    }
+  },
+
+  users(topicId) {
+    if (!topicId) return [];
+    return this._getPresence(topicId).users;
+  },
+
+  editingUsers(topicId) {
+    if (!topicId) return [];
+    return this._getPresence(topicId).editingUsers;
+  },
+
+  throttlePublish(topicId, state, whisper, postId) {
+    if (!topicId) return;
+    return this._getPresence(topicId).throttlePublish(state, whisper, postId);
+  },
+
+  publish(topicId, state, whisper, postId) {
+    if (!topicId) return;
+    return this._getPresence(topicId).publish(state, whisper, postId);
+  },
+
+  cleanUpPresence(type) {
+    Object.keys(this.presences).forEach(key => {
+      this.publish(key, CLOSED);
+      this.unsubscribe(key, type);
+    });
+  },
+
+  _getPresence(topicId) {
+    if (!this.presences[topicId]) {
+      this.presences[topicId] = Presence.create({
+        messageBus: this.messageBus,
+        siteSettings: this.siteSettings,
+        currentUser: this.currentUser,
+        topicId
+      });
+    }
+
+    return this.presences[topicId];
+  }
+});
+
 export default PresenceManager;

plugins/discourse-presence/assets/javascripts/discourse/templates/connectors/composer-fields/presence.js.es6

@@ -1,9 +1,5 @@
 export default {
-  shouldRender(args, component) {
+  shouldRender(_, component) {
-    return (
+    return component.siteSettings.presence_enabled;
-      component.siteSettings.presence_enabled &&
-      args.model.topic &&
-      args.model.topic.presenceManager
-    );
   }
 };

plugins/discourse-presence/assets/javascripts/discourse/templates/connectors/topic-above-footer-buttons/presence.js.es6

@@ -1,7 +1,5 @@
 export default {
-  shouldRender(args, component) {
+  shouldRender(_, component) {
-    return (
+    return component.siteSettings.presence_enabled;
-      component.siteSettings.presence_enabled && args.model.presenceManager
-    );
   }
 };

plugins/discourse-presence/assets/javascripts/initializers/discourse-presence.js.es6

@@ -1,43 +0,0 @@
-import { withPluginApi } from "discourse/lib/plugin-api";
-import PresenceManager from "../discourse/lib/presence-manager";
-import ENV from "discourse-common/config/environment";
-
-function initializeDiscoursePresence(api) {
-  const currentUser = api.getCurrentUser();
-  const siteSettings = api.container.lookup("site-settings:main");
-
-  if (currentUser) {
-    api.modifyClass("model:topic", {
-      presenceManager: null
-    });
-
-    api.modifyClass("route:topic-from-params", {
-      setupController() {
-        this._super(...arguments);
-
-        this.modelFor("topic").set(
-          "presenceManager",
-          PresenceManager.create({
-            topic: this.modelFor("topic"),
-            currentUser,
-            messageBus: api.container.lookup("message-bus:main"),
-            siteSettings
-          })
-        );
-      }
-    });
-  }
-}
-
-export default {
-  name: "discourse-presence",
-  after: "message-bus",
-
-  initialize(container) {
-    const siteSettings = container.lookup("site-settings:main");
-
-    if (siteSettings.presence_enabled && ENV.environment !== "test") {
-      withPluginApi("0.8.40", initializeDiscoursePresence);
-    }
-  }
-};