Page MenuHomePhabricator

[keyserver] add dontAddViewerAsMember to createThread
AbandonedPublic

Authored by will on Wed, Oct 30, 12:12 AM.

Details

Reviewers
varun
ashoat
Summary

This diff adds a flag so we can skip adding a viewer as a member as part of creating a thread.

Depends on D13815

Test Plan

Confirmed that the sidebar that was created didn't have commbot in the member list

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will retitled this revision from add dontAddViewerAsMember to createThread to [keyserver] add dontAddViewerAsMember to createThread.
will requested review of this revision.Wed, Oct 30, 12:30 AM
ashoat requested changes to this revision.Wed, Oct 30, 5:01 AM
ashoat added inline comments.
keyserver/src/creators/thread-creator.js
376

I think there was a bit of miscommunication in D13812. I added this comment at a later point and I think it was missed:

(I suppose we'd need to add commbot as a ghost at least)

Basically, commbot needs to be a ghost so that users who join later can resolve its username. This is necessary if there is at least one message from commbot in the channel (I think this is currently the case?)

378

We should never have await inside an expression like this – it has the potentially of hiding it from the reader

We have a convention in our JS codebase that await is only allowed to appear in two places:

  1. As the first keyword in a statement
  2. As the first keyword in the right-hand-side of an assignment
This revision now requires changes to proceed.Wed, Oct 30, 5:01 AM
keyserver/src/creators/thread-creator.js
376

Got it. So I actually include this later as part of the sidebar creation code:

+  let viewer = commbotViewer;
+  if (taggerUserID) {
+    viewer = createScriptViewer(taggerUserID);
+    await joinThread(viewer, {
+      threadID: channelCommunityID,
+    });
+  } else {
+    const changeset = await changeRole(
+      channelCommunityID,
+      [commbot.userID],
+      -1,
+    );
+

Thinking now that the else block here can be simplified by including an option like addViewerAsGhost

Abandoning with new approach in setting the viewer as a ghost