Merge pull request #1496 from hypothesis/rename-username-to-userid

Rename the username to userid in selection module

docs/publishers/config.rst

@@ -408,10 +408,8 @@ loads.
           user: {
             // required
             username: "foobar1234",
-
             // optional
             displayName: "Foo Bar",
-            authority: "example.com",
           }
         }
       };

src/sidebar/services/root-thread.js

@@ -58,7 +58,7 @@ function RootThread($rootScope, store, searchFilter, viewFilter) {
         state.selection.filterQuery,
         {
           // if a focus mode is applied (focused) and we're focusing on a user
-          user: store.focusModeFocused() && store.focusModeUsername(),
+          user: store.focusModeFocused() && store.focusModeUserId(),
         }
       );
 

src/sidebar/services/test/root-thread-test.js

@@ -67,7 +67,7 @@ describe('rootThread', function() {
       removeDraft: sinon.stub(),
       createAnnotation: sinon.stub(),
       focusModeFocused: sinon.stub().returns(false),
-      focusModeUsername: sinon.stub().returns({}),
+      focusModeUserId: sinon.stub().returns({}),
     };
 
     fakeBuildThread = sinon.stub().returns(fixtures.emptyThread);

src/sidebar/store/modules/selection.js

@@ -10,7 +10,7 @@
 
 /**
  * @typedef User
- * @property {string} username - Unique user's username
+ * @property {string} userid - Unique user's id
  * @property {string} displayName - User's display name
  */
 
@@ -441,16 +441,20 @@ function focusModeFocused(state) {
 }
 
 /**
- * Returns the username for a focused user or `null` if no focused user.
+ * Returns the `userid` for a focused user or `null` if no focused user.
  *
- * @return {object|null}
+ * @return {string|null}
  */
-function focusModeUsername(state) {
-  if (
-    state.selection.focusMode.config.user &&
-    state.selection.focusMode.config.user.username
-  ) {
-    return state.selection.focusMode.config.user.username;
+function focusModeUserId(state) {
+  if (state.selection.focusMode.config.user) {
+    if (state.selection.focusMode.config.user.userid) {
+      return state.selection.focusMode.config.user.userid;
+    }
+    if (state.selection.focusMode.config.user.username) {
+      // remove once LMS no longer sends username in RPC or config
+      // https://github.com/hypothesis/client/issues/1516
+      return state.selection.focusMode.config.user.username;
+    }
   }
   return null;
 }
@@ -462,11 +466,11 @@ function focusModeUsername(state) {
  * @return {boolean}
  */
 function focusModeHasUser(state) {
-  return focusModeEnabled(state) && !!focusModeUsername(state);
+  return focusModeEnabled(state) && !!focusModeUserId(state);
 }
 
 /**
- * Returns the display name for a user or the username
+ * Returns the display name for a user or the userid
  * if display name is not present. If both are missing
  * then this returns an empty string.
  *
@@ -478,7 +482,10 @@ function focusModeUserPrettyName(state) {
     return '';
   } else if (user.displayName) {
     return user.displayName;
+  } else if (user.userid) {
+    return user.userid;
   } else if (user.username) {
+    // remove once LMS no longer sends `username` in RPC
     return user.username;
   } else {
     return '';
@@ -491,19 +498,19 @@ module.exports = {
   update: update,
 
   actions: {
-    clearSelectedAnnotations: clearSelectedAnnotations,
-    clearSelection: clearSelection,
-    focusAnnotations: focusAnnotations,
-    highlightAnnotations: highlightAnnotations,
-    selectAnnotations: selectAnnotations,
-    selectTab: selectTab,
-    setCollapsed: setCollapsed,
-    setFilterQuery: setFilterQuery,
-    setFocusModeFocused: setFocusModeFocused,
-    changeFocusModeUser: changeFocusModeUser,
-    setForceVisible: setForceVisible,
-    setSortKey: setSortKey,
-    toggleSelectedAnnotations: toggleSelectedAnnotations,
+    clearSelectedAnnotations,
+    clearSelection,
+    focusAnnotations,
+    highlightAnnotations,
+    selectAnnotations,
+    selectTab,
+    setCollapsed,
+    setFilterQuery,
+    setFocusModeFocused,
+    changeFocusModeUser,
+    setForceVisible,
+    setSortKey,
+    toggleSelectedAnnotations,
   },
 
   selectors: {
@@ -512,7 +519,7 @@ module.exports = {
     focusModeFocused,
     focusModeEnabled,
     focusModeHasUser,
-    focusModeUsername,
+    focusModeUserId,
     focusModeUserPrettyName,
     isAnnotationSelected,
     getFirstSelectedAnnotationId,

src/sidebar/store/modules/test/selection-test.js

@@ -205,10 +205,10 @@ describe('sidebar/store/modules/selection', () => {
     it('sets the focused user and enables focus mode', function() {
       store.setFocusModeFocused(false);
       store.changeFocusModeUser({
-        username: 'testuser',
+        userid: 'testuser',
         displayName: 'Test User',
       });
-      assert.equal(store.focusModeUsername(), 'testuser');
+      assert.equal(store.focusModeUserId(), 'testuser');
       assert.equal(store.focusModeUserPrettyName(), 'Test User');
       assert.equal(store.focusModeFocused(), true);
       assert.equal(store.focusModeEnabled(), true);
@@ -256,14 +256,14 @@ describe('sidebar/store/modules/selection', () => {
     it('should return `true` if focus enabled and valid `user` object present', () => {
       store = createStore(
         [selection],
-        [{ focus: { user: { username: 'whatever' } } }]
+        [{ focus: { user: { userid: 'acct:userid@authority' } } }]
       );
       assert.isTrue(store.focusModeHasUser());
     });
     it('should return `false` if focus enabled but `user` object invalid', () => {
       store = createStore(
         [selection],
-        [{ focus: { user: { displayName: 'whatever' } } }] // `username` is required
+        [{ focus: { user: { displayName: 'FakeDisplayName' } } }] // `userid` is required
       );
       assert.isFalse(store.focusModeHasUser());
     });
@@ -274,43 +274,61 @@ describe('sidebar/store/modules/selection', () => {
   });
 
   describe('focusModeUserPrettyName()', function() {
-    it('should return false by default when focus mode is not enabled', function() {
+    it('returns false by default when focus mode is not enabled', function() {
       store = createStore(
         [selection],
         [{ focus: { user: { displayName: 'FakeDisplayName' } } }]
       );
       assert.equal(store.focusModeUserPrettyName(), 'FakeDisplayName');
     });
-    it('should the username when displayName is missing', function() {
+    it('returns the userid when displayName is missing', function() {
       store = createStore(
         [selection],
-        [{ focus: { user: { username: 'FakeUserName' } } }]
+        [{ focus: { user: { userid: 'acct:userid@authority' } } }]
       );
-      assert.equal(store.focusModeUserPrettyName(), 'FakeUserName');
+      assert.equal(store.focusModeUserPrettyName(), 'acct:userid@authority');
     });
-    it('should an return empty string when user object has no names', function() {
+    it('returns an empty string when user object has is empty', function() {
       store = createStore([selection], [{ focus: { user: {} } }]);
       assert.equal(store.focusModeUserPrettyName(), '');
     });
-    it('should an return empty string when there is no focus object', function() {
+    it('return an empty string when there is no focus object', function() {
       assert.equal(store.focusModeUserPrettyName(), '');
     });
+    it('returns the username when displayName and userid is missing', function() {
+      // remove once LMS no longer sends username in RPC or config
+      // https://github.com/hypothesis/client/issues/1516
+      store = createStore(
+        [selection],
+        [{ focus: { user: { username: 'fake_user_name' } } }]
+      );
+      assert.equal(store.focusModeUserPrettyName(), 'fake_user_name');
+    });
   });
 
-  describe('focusModeUsername()', function() {
-    it('should return the user name when present', function() {
+  describe('focusModeUserId()', function() {
+    it('should return the userid when present', function() {
       store = createStore(
         [selection],
-        [{ focus: { user: { username: 'FakeUserName' } } }]
+        [{ focus: { user: { userid: 'acct:userid@authority' } } }]
       );
-      assert.equal(store.focusModeUsername(), 'FakeUserName');
+      assert.equal(store.focusModeUserId(), 'acct:userid@authority');
     });
-    it('should return null when the username is not present', function() {
+    it('should return null when the userid is not present', function() {
       store = createStore([selection], [{ focus: { user: {} } }]);
-      assert.isNull(store.focusModeUsername());
+      assert.isNull(store.focusModeUserId());
     });
     it('should return null when the user object is not present', function() {
-      assert.isNull(store.focusModeUsername());
+      assert.isNull(store.focusModeUserId());
+    });
+    it('should return the username when present but no userid', function() {
+      // remove once LMS no longer sends username in RPC or config
+      // https://github.com/hypothesis/client/issues/1516
+      store = createStore(
+        [selection],
+        [{ focus: { user: { username: 'fake_user_name' } } }]
+      );
+      assert.equal(store.focusModeUserId(), 'fake_user_name');
     });
   });