Expand annotation-author helper functions

src/sidebar/components/Annotation/AnnotationHeader.js

@@ -3,14 +3,13 @@ import { useMemo } from 'preact/hooks';
 
 import { withServices } from '../../service-context';
 import { useSidebarStore } from '../../store';
-import { isThirdPartyUser, username } from '../../helpers/account-id';
 import {
   domainAndTitle,
   isHighlight,
   isReply,
   hasBeenEdited,
 } from '../../helpers/annotation-metadata';
-import { annotationDisplayName } from '../../helpers/annotation-user';
+import { annotationAuthorInfo } from '../../helpers/annotation-user';
 import { isPrivate } from '../../helpers/permissions';
 
 import AnnotationDocumentInfo from './AnnotationDocumentInfo';
@@ -57,27 +56,11 @@ function AnnotationHeader({
   settings,
 }) {
   const store = useSidebarStore();
-  const defaultAuthority = store.defaultAuthority();
-  const displayNamesEnabled = store.isFeatureEnabled('client_display_names');
 
-  const isThirdParty = isThirdPartyUser(annotation.user, defaultAuthority);
-  const authorDisplayName = annotationDisplayName(
-    annotation,
-    isThirdParty,
-    displayNamesEnabled
-  );
-
-  const authorLink = (() => {
-    if (!isThirdParty) {
-      return store.getLink('user', { user: annotation.user });
-    } else {
-      return (
-        (settings.usernameUrl &&
-          `${settings.usernameUrl}${username(annotation.user)}`) ??
-        undefined
+  const { authorDisplayName, authorLink } = useMemo(
+    () => annotationAuthorInfo(annotation, store, settings),
+    [annotation, store, settings]
   );
-    }
-  })();
 
   const isCollapsedReply = isReply(annotation) && threadIsCollapsed;
 

src/sidebar/components/Annotation/test/AnnotationHeader-test.js

@@ -8,8 +8,7 @@ import { mockImportedComponents } from '../../../../test-util/mock-imported-comp
 import AnnotationHeader, { $imports } from '../AnnotationHeader';
 
 describe('AnnotationHeader', () => {
-  let fakeAccountId;
-  let fakeAnnotationDisplayName;
+  let fakeAnnotationAuthorInfo;
   let fakeDomainAndTitle;
   let fakeGroup;
   let fakeIsHighlight;
@@ -46,20 +45,15 @@ describe('AnnotationHeader', () => {
     fakeHasBeenEdited = sinon.stub().returns(false);
     fakeIsPrivate = sinon.stub();
 
-    fakeAccountId = {
-      isThirdPartyUser: sinon.stub().returns(false),
-      username: sinon.stub().returnsArg(0),
-    };
-
-    fakeAnnotationDisplayName = sinon.stub().returns('Robbie Burns');
+    fakeAnnotationAuthorInfo = sinon.stub().returns({
+      authorDisplayName: 'Robbie Burns',
+      authorLink: 'http://www.example.com',
+    });
 
     fakeSettings = { usernameUrl: 'http://foo.bar/' };
 
     fakeStore = {
-      defaultAuthority: sinon.stub().returns('foo.com'),
       getGroup: sinon.stub().returns(fakeGroup),
-      getLink: sinon.stub().returns('http://example.com'),
-      isFeatureEnabled: sinon.stub().returns(false),
       route: sinon.stub().returns('sidebar'),
       setExpanded: sinon.stub(),
     };
@@ -67,7 +61,6 @@ describe('AnnotationHeader', () => {
     $imports.$mock(mockImportedComponents());
     $imports.$mock({
       '../../store': { useSidebarStore: () => fakeStore },
-      '../../helpers/account-id': fakeAccountId,
       '../../helpers/annotation-metadata': {
         domainAndTitle: fakeDomainAndTitle,
         isHighlight: fakeIsHighlight,
@@ -75,7 +68,7 @@ describe('AnnotationHeader', () => {
         hasBeenEdited: fakeHasBeenEdited,
       },
       '../../helpers/annotation-user': {
-        annotationDisplayName: fakeAnnotationDisplayName,
+        annotationAuthorInfo: fakeAnnotationAuthorInfo,
       },
       '../../helpers/permissions': {
         isPrivate: fakeIsPrivate,
@@ -114,33 +107,22 @@ describe('AnnotationHeader', () => {
   });
 
   describe('annotation author (user) information', () => {
-    it('should link to author activity if first-party', () => {
-      fakeAccountId.isThirdPartyUser.returns(false);
-
+    it('should link to author activity if link available', () => {
       const wrapper = createAnnotationHeader();
 
       assert.equal(
         wrapper.find('AnnotationUser').props().authorLink,
-        'http://example.com'
+        'http://www.example.com'
       );
     });
 
-    it('should link to author activity if third-party and has settings URL', () => {
-      fakeAccountId.isThirdPartyUser.returns(true);
-      const fakeAnnotation = fixtures.defaultAnnotation();
-
-      const wrapper = createAnnotationHeader({ annotation: fakeAnnotation });
-
-      assert.equal(
-        wrapper.find('AnnotationUser').props().authorLink,
-        `http://foo.bar/${fakeAnnotation.user}`
-      );
+    it('should not link to author if none provided', () => {
+      fakeAnnotationAuthorInfo.returns({
+        authorDisplayName: 'Robbie Burns',
+        authorLink: undefined,
       });
 
-    it('should not link to author if third-party and no settings URL', () => {
-      fakeAccountId.isThirdPartyUser.returns(true);
-
-      const wrapper = createAnnotationHeader({ settings: {} });
+      const wrapper = createAnnotationHeader();
 
       assert.isUndefined(wrapper.find('AnnotationUser').props().authorLink);
     });

src/sidebar/components/hooks/test/use-filter-options-test.js

@@ -51,9 +51,7 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
 
     fakeStore = {
       allAnnotations: sinon.stub().returns([]),
-      defaultAuthority: sinon.stub().returns('foo.com'),
       getFocusFilters: sinon.stub().returns({}),
-      isFeatureEnabled: sinon.stub().returns(false),
       profile: sinon.stub().returns({}),
     };
 

src/sidebar/components/hooks/use-filter-options.js

 import { useMemo } from 'preact/hooks';
 
 import { useSidebarStore } from '../../store';
-import { isThirdPartyUser, username } from '../../helpers/account-id';
+import { username } from '../../helpers/account-id';
 import { annotationDisplayName } from '../../helpers/annotation-user';
 
 /** @typedef {import('../../store/modules/filters').FilterOption} FilterOption */
@@ -15,8 +15,6 @@ import { annotationDisplayName } from '../../helpers/annotation-user';
 export function useUserFilterOptions() {
   const store = useSidebarStore();
   const annotations = store.allAnnotations();
-  const defaultAuthority = store.defaultAuthority();
-  const displayNamesEnabled = store.isFeatureEnabled('client_display_names');
   const focusFilters = store.getFocusFilters();
   const currentUsername = username(store.profile().userid);
 
@@ -26,11 +24,7 @@ export function useUserFilterOptions() {
     const users = {};
     annotations.forEach(annotation => {
       const username_ = username(annotation.user);
-      users[username_] = annotationDisplayName(
-        annotation,
-        isThirdPartyUser(annotation.user, defaultAuthority),
-        displayNamesEnabled
-      );
+      users[username_] = annotationDisplayName(annotation, store);
     });
 
     // If user-focus is configured (even if not applied) add a filter
@@ -63,11 +57,5 @@ export function useUserFilterOptions() {
     });
 
     return userOptions;
-  }, [
-    annotations,
-    currentUsername,
-    defaultAuthority,
-    displayNamesEnabled,
-    focusFilters.user,
-  ]);
+  }, [annotations, currentUsername, focusFilters.user, store]);
 }

src/sidebar/helpers/annotation-user.js

 /**
  * @typedef {import("../../types/api").Annotation} Annotation
+ * @typedef {import('../../types/config').SidebarSettings} SidebarSettings
+ * @typedef {import('../store').SidebarStore} SidebarStore
  */
 
-import { username } from './account-id';
-
+import { isThirdPartyUser, username } from './account-id';
 /**
  * What string should we use to represent the author (user) of a given
  * annotation: a display name or a username?
@@ -17,18 +18,56 @@ import { username } from './account-id';
  * username or the display name.
  *
  * @param {Pick<Annotation, 'user'|'user_info'>} annotation
- * @param {boolean} isThirdPartyUser - Is the annotation's user third-party?
- * @param {boolean} isFeatureFlagEnabled - Is the `client_display_names`
- *   feature flag enabled
+ * @param {SidebarStore} store
+ *
  * @return {string}
  */
-export function annotationDisplayName(
-  annotation,
-  isThirdPartyUser,
-  isFeatureFlagEnabled
-) {
-  const useDisplayName = isFeatureFlagEnabled || isThirdPartyUser;
+export function annotationDisplayName(annotation, store) {
+  const defaultAuthority = store.defaultAuthority();
+  const isThirdParty = isThirdPartyUser(annotation.user, defaultAuthority);
+
+  const displayNamesEnabled = store.isFeatureEnabled('client_display_names');
+
+  const useDisplayName = displayNamesEnabled || isThirdParty;
   return useDisplayName && annotation.user_info?.display_name
     ? annotation.user_info.display_name
     : username(annotation.user);
 }
+
+/**
+ * Return a URL to the annotation author's user page, when available. Author
+ * links for third-party users are only available if a `usernameUrl` is
+ * provided in `settings`.
+ *
+ * @param {Pick<Annotation, 'user'>} annotation
+ * @param {SidebarStore} store
+ * @param {SidebarSettings} settings
+ */
+export function annotationAuthorLink(annotation, store, settings) {
+  const defaultAuthority = store.defaultAuthority();
+  const isThirdParty = isThirdPartyUser(annotation.user, defaultAuthority);
+
+  if (!isThirdParty) {
+    return store.getLink('user', { user: annotation.user });
+  }
+
+  return (
+    (settings.usernameUrl &&
+      `${settings.usernameUrl}${username(annotation.user)}`) ??
+    undefined
+  );
+}
+
+/**
+ * Retrieve both author display name and link.
+ *
+ * @param {Pick<Annotation, 'user'|'user_info'>} annotation
+ * @param {SidebarStore} store
+ * @param {SidebarSettings} settings
+ */
+export function annotationAuthorInfo(annotation, store, settings) {
+  return {
+    authorDisplayName: annotationDisplayName(annotation, store),
+    authorLink: annotationAuthorLink(annotation, store, settings),
+  };
+}

src/sidebar/helpers/test/annotation-user-test.js

-import { annotationDisplayName } from '../annotation-user';
+import {
+  annotationDisplayName,
+  annotationAuthorLink,
+  annotationAuthorInfo,
+  $imports,
+} from '../annotation-user';
 
 describe('sidebar/helpers/annotation-user', () => {
+  let fakeAccountId;
+  let fakeSettings;
+  let fakeStore;
+
+  beforeEach(() => {
+    fakeSettings = { usernameUrl: 'http://foo.bar/' };
+
+    fakeStore = {
+      defaultAuthority: sinon.stub().returns('foo.com'),
+      isFeatureEnabled: sinon.stub().returns(false),
+      getLink: sinon
+        .stub()
+        .withArgs('user')
+        .returns('http://www.example.com/user/'),
+    };
+
+    fakeAccountId = {
+      isThirdPartyUser: sinon.stub().returns(false),
+      username: sinon.stub().returns('albert'),
+    };
+
+    $imports.$mock({
+      './account-id': fakeAccountId,
+    });
+  });
+
   const fakeAnnotations = {
     withDisplayName: {
       user: 'acct:albert@victoriana.com',
@@ -15,52 +46,130 @@ describe('sidebar/helpers/annotation-user', () => {
     },
   };
 
+  describe('annotationDisplayName', () => {
+    context('annotation with first-party author', () => {
       [
         {
           annotation: fakeAnnotations.withDisplayName,
-      isThirdParty: false,
-      isFeatureEnabled: false,
           expected: 'albert',
         },
         {
-      annotation: fakeAnnotations.withDisplayName,
-      isThirdParty: true,
-      isFeatureEnabled: false,
-      expected: 'Albert, Prince Consort',
+          annotation: fakeAnnotations.noDisplayName,
+          expected: 'albert',
+        },
+        {
+          annotation: fakeAnnotations.noUserInfo,
+          expected: 'albert',
         },
+      ].forEach(testcase => {
+        it('should return author username if display-names feature flag is not enabled', () => {
+          fakeStore.isFeatureEnabled
+            .withArgs('client_display_names')
+            .returns(false);
+          assert.equal(
+            annotationDisplayName(testcase.annotation, fakeStore),
+            testcase.expected
+          );
+        });
+      });
+
+      [
         {
           annotation: fakeAnnotations.withDisplayName,
-      isThirdParty: false,
-      isFeatureEnabled: true,
           expected: 'Albert, Prince Consort',
         },
+        {
+          annotation: fakeAnnotations.noDisplayName,
+          expected: 'albert',
+        },
+        {
+          annotation: fakeAnnotations.noUserInfo,
+          expected: 'albert',
+        },
+      ].forEach(testcase => {
+        it('should return author display name when available if display-names feature flag is enabled', () => {
+          fakeStore.isFeatureEnabled
+            .withArgs('client_display_names')
+            .returns(true);
+          assert.equal(
+            annotationDisplayName(testcase.annotation, fakeStore),
+            testcase.expected
+          );
+        });
+      });
+    });
+
+    context('annotation with third-party author', () => {
+      [
         {
           annotation: fakeAnnotations.withDisplayName,
-      isThirdParty: true,
-      isFeatureEnabled: true,
           expected: 'Albert, Prince Consort',
         },
         {
           annotation: fakeAnnotations.noDisplayName,
-      isThirdParty: true,
-      isFeatureEnabled: true,
           expected: 'albert',
         },
         {
           annotation: fakeAnnotations.noUserInfo,
-      isThirdParty: true,
-      isFeatureEnabled: true,
           expected: 'albert',
         },
-  ].forEach(testCase => {
-    it('should return the appropriate author string for an annotation', () => {
+      ].forEach(testcase => {
+        it('should return author display name if available', () => {
+          fakeAccountId.isThirdPartyUser.returns(true);
+          assert.equal(
+            annotationDisplayName(testcase.annotation, fakeStore),
+            testcase.expected
+          );
+        });
+      });
+    });
+  });
+
+  describe('annotationAuthorLink', () => {
+    it('should return a URL for first-party users', () => {
+      fakeAccountId.isThirdPartyUser.returns(false);
       assert.equal(
-        annotationDisplayName(
-          testCase.annotation,
-          testCase.isThirdParty,
-          testCase.isFeatureEnabled
+        annotationAuthorLink(
+          fakeAnnotations.withDisplayName,
+          fakeStore,
+          fakeSettings
         ),
-        testCase.expected
+        'http://www.example.com/user/'
+      );
+    });
+
+    it('should return a URL for third-party users when configured with `usernameUrl`', () => {
+      fakeAccountId.isThirdPartyUser.returns(true);
+      assert.equal(
+        annotationAuthorLink(
+          fakeAnnotations.withDisplayName,
+          fakeStore,
+          fakeSettings
+        ),
+        'http://foo.bar/albert'
+      );
+    });
+
+    it('should not return a URL for third-party users if not configured with `usernameUrl`', () => {
+      fakeAccountId.isThirdPartyUser.returns(true);
+      assert.isUndefined(
+        annotationAuthorLink(fakeAnnotations.withDisplayName, fakeStore, {})
+      );
+    });
+  });
+
+  describe('annotationAuthorInfo', () => {
+    it('should return both a display name and a link for annotation author', () => {
+      assert.deepEqual(
+        annotationAuthorInfo(
+          fakeAnnotations.withDisplayName,
+          fakeStore,
+          fakeSettings
+        ),
+        {
+          authorDisplayName: 'albert',
+          authorLink: 'http://www.example.com/user/',
+        }
       );
     });
   });