Update hook and component to use helper function for author names

src/sidebar/components/Annotation/AnnotationUser.js

 import { isThirdPartyUser, username } from '../../helpers/account-id';
+import { annotationDisplayName } from '../../helpers/annotation-user';
 import { withServices } from '../../service-context';
 
 /**
@@ -27,15 +28,11 @@ function AnnotationUser({ annotation, features, serviceUrl, settings }) {
   const username_ = username(user);
 
   // How should the user's name be displayed?
-  const displayName = (() => {
-    if (isFirstPartyUser && !features.flagEnabled('client_display_names')) {
-      return username_;
-    }
-    if (annotation.user_info && annotation.user_info.display_name) {
-      return annotation.user_info.display_name;
-    }
-    return username_;
-  })();
+  const displayName = annotationDisplayName(
+    annotation,
+    !isFirstPartyUser,
+    features.flagEnabled('client_display_names')
+  );
 
   const shouldLinkToActivity = isFirstPartyUser || settings.usernameUrl;
 

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

@@ -7,6 +7,7 @@ import AnnotationUser, { $imports } from '../AnnotationUser';
 
 describe('AnnotationUser', () => {
   let fakeAnnotation;
+  let fakeAnnotationUser;
   let fakeFeatures;
   let fakeIsThirdPartyUser;
   let fakeServiceUrl;
@@ -28,6 +29,11 @@ describe('AnnotationUser', () => {
     fakeAnnotation = {
       user: 'someone@hypothes.is',
     };
+    fakeAnnotationUser = {
+      annotationDisplayName: sinon
+        .stub()
+        .callsFake(annotation => annotation.user),
+    };
     fakeFeatures = { flagEnabled: sinon.stub() };
     fakeIsThirdPartyUser = sinon.stub().returns(false);
     fakeServiceUrl = sinon.stub();
@@ -40,6 +46,7 @@ describe('AnnotationUser', () => {
         isThirdPartyUser: fakeIsThirdPartyUser,
         username: fakeUsername,
       },
+      '../../helpers/annotation-user': fakeAnnotationUser,
     });
   });
 
@@ -89,58 +96,17 @@ describe('AnnotationUser', () => {
     });
   });
 
-  describe('rendered user name', () => {
-    context('feature flag on', () => {
-      beforeEach(() => {
-        fakeFeatures.flagEnabled.withArgs('client_display_names').returns(true);
-      });
-      it('should render a display name when feature flag on and info available', () => {
-        fakeAnnotation.user_info = {
-          display_name: 'Maple Oaks',
-        };
-
-        const wrapper = createAnnotationUser();
-        const linkEl = wrapper.find('a');
-
-        assert.equal(linkEl.text(), 'Maple Oaks');
-      });
-
-      it('should render a username when feature flag on but info not present', () => {
-        fakeUsername.returns('myusername');
+  it('renders the annotation author name', () => {
+    fakeFeatures.flagEnabled.withArgs('client_display_names').returns(true);
 
-        const wrapper = createAnnotationUser();
-        const linkEl = wrapper.find('a');
-
-        assert.equal(linkEl.text(), 'myusername');
-      });
-    });
+    createAnnotationUser();
 
-    context('feature flag off', () => {
-      it('should render a username for first-party users when feature flag off', () => {
-        fakeFeatures.flagEnabled.returns(false);
-        fakeUsername.returns('myusername');
-        fakeAnnotation.user_info = {
-          display_name: 'Maple Oaks',
-        };
-
-        const wrapper = createAnnotationUser();
-        const linkEl = wrapper.find('a');
-
-        assert.equal(linkEl.text(), 'myusername');
-      });
-
-      it('should render a display name for third-party users', () => {
-        fakeAnnotation.user_info = {
-          display_name: 'Maple Oaks',
-        };
-        fakeIsThirdPartyUser.returns(true);
-        fakeFeatures.flagEnabled.returns(false);
-
-        const wrapper = createAnnotationUser();
-
-        assert.equal(wrapper.text(), 'Maple Oaks');
-      });
-    });
+    assert.calledWith(
+      fakeAnnotationUser.annotationDisplayName,
+      fakeAnnotation,
+      false,
+      true
+    );
   });
 
   it(

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

@@ -4,9 +4,17 @@ import { useUserFilterOptions } from '../use-filter-options';
 import { $imports } from '../use-filter-options';
 
 describe('sidebar/components/hooks/use-user-filter-options', () => {
+  let fakeAccountId;
   let fakeStore;
+  let fakeAnnotationUser;
   let lastUserOptions;
 
+  // Mock `annotationDisplayName` as if it's returning display names
+  let fakeAnnotationUserDisplay = annotation =>
+    annotation.user_info.display_name;
+  // Mock `annotationDisplayName` as if it's returning usernames
+  let fakeAnnotationUserUsername = annotation => annotation.user;
+
   // Mount a dummy component to be able to use the hook
   function DummyComponent() {
     lastUserOptions = useUserFilterOptions();
@@ -15,33 +23,44 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
   function annotationFixtures() {
     return [
       {
-        user: 'acct:dingbat@localhost',
+        user: 'dingbat',
         user_info: { display_name: 'Ding Bat' },
       },
       {
-        user: 'acct:abalone@localhost',
+        user: 'abalone',
         user_info: { display_name: 'Aba Lone' },
       },
       {
-        user: 'acct:bananagram@localhost',
+        user: 'bananagram',
         user_info: { display_name: 'Zerk' },
       },
       {
-        user: 'acct:dingbat@localhost',
+        user: 'dingbat',
         user_info: { display_name: 'Ding Bat' },
       },
     ];
   }
 
   beforeEach(() => {
+    fakeAccountId = {
+      username: sinon.stub().returnsArg(0),
+    };
+
+    fakeAnnotationUser = {
+      annotationDisplayName: sinon.stub().callsFake(fakeAnnotationUserUsername),
+    };
+
     fakeStore = {
       allAnnotations: sinon.stub().returns([]),
+      authDomain: sinon.stub().returns('foo.com'),
       getFocusFilters: sinon.stub().returns({}),
       isFeatureEnabled: sinon.stub().returns(false),
       profile: sinon.stub().returns({}),
     };
 
     $imports.$mock({
+      '../../helpers/account-id': fakeAccountId,
+      '../../helpers/annotation-user': fakeAnnotationUser,
       '../../store/use-store': { useStoreProxy: () => fakeStore },
     });
   });
@@ -62,23 +81,10 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
     ]);
   });
 
-  it('should use display names if feature flag enabled', () => {
-    fakeStore.allAnnotations.returns(annotationFixtures());
-    fakeStore.isFeatureEnabled.withArgs('client_display_names').returns(true);
-
-    mount(<DummyComponent />);
-
-    assert.deepEqual(lastUserOptions, [
-      { value: 'abalone', display: 'Aba Lone' },
-      { value: 'dingbat', display: 'Ding Bat' },
-      { value: 'bananagram', display: 'Zerk' },
-    ]);
-  });
-
   it('sorts the current user to the front with " (Me)" suffix', () => {
     fakeStore.allAnnotations.returns(annotationFixtures());
     fakeStore.profile.returns({
-      userid: 'acct:bananagram@localhost',
+      userid: 'bananagram',
     });
 
     mount(<DummyComponent />);
@@ -114,7 +120,9 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
 
     it('should add focused-user filter information', () => {
       fakeStore.allAnnotations.returns(annotationFixtures());
-      fakeStore.isFeatureEnabled.withArgs('client_display_names').returns(true);
+      fakeAnnotationUser.annotationDisplayName.callsFake(
+        fakeAnnotationUserDisplay
+      );
 
       mount(<DummyComponent />);
 
@@ -128,9 +136,6 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
 
     it('always uses display name for focused user', () => {
       fakeStore.allAnnotations.returns(annotationFixtures());
-      fakeStore.isFeatureEnabled
-        .withArgs('client_display_names')
-        .returns(false);
       fakeStore.getFocusFilters.returns({
         user: { value: 'carrotNumberOne', display: 'Numero Uno Zanahoria' },
       });
@@ -146,10 +151,12 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
     });
 
     it('sorts the current user to the front with " (Me)" suffix', () => {
+      fakeAnnotationUser.annotationDisplayName.callsFake(
+        fakeAnnotationUserDisplay
+      );
       fakeStore.allAnnotations.returns(annotationFixtures());
-      fakeStore.isFeatureEnabled.withArgs('client_display_names').returns(true);
       fakeStore.profile.returns({
-        userid: 'acct:bananagram@localhost',
+        userid: 'bananagram',
       });
 
       mount(<DummyComponent />);
@@ -164,9 +171,11 @@ describe('sidebar/components/hooks/use-user-filter-options', () => {
 
     it('does not add (Me)" suffix if user has no annotations', () => {
       fakeStore.allAnnotations.returns(annotationFixtures());
-      fakeStore.isFeatureEnabled.withArgs('client_display_names').returns(true);
+      fakeAnnotationUser.annotationDisplayName.callsFake(
+        fakeAnnotationUserDisplay
+      );
       fakeStore.profile.returns({
-        userid: 'acct:fakeid@localhost',
+        userid: 'fakeid',
       });
 
       mount(<DummyComponent />);

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

 import { useMemo } from 'preact/hooks';
 
 import { useStoreProxy } from '../../store/use-store';
-import { username } from '../../helpers/account-id';
+import { isThirdPartyUser, username } from '../../helpers/account-id';
+import { annotationDisplayName } from '../../helpers/annotation-user';
 
 /** @typedef {import('../../store/modules/filters').FilterOption} FilterOption */
 
@@ -14,8 +15,9 @@ import { username } from '../../helpers/account-id';
 export function useUserFilterOptions() {
   const store = useStoreProxy();
   const annotations = store.allAnnotations();
+  const authDomain = store.authDomain();
+  const displayNamesEnabled = store.isFeatureEnabled('client_display_names');
   const focusFilters = store.getFocusFilters();
-  const showDisplayNames = store.isFeatureEnabled('client_display_names');
   const currentUsername = username(store.profile().userid);
 
   return useMemo(() => {
@@ -23,11 +25,11 @@ export function useUserFilterOptions() {
     const users = {};
     annotations.forEach(annotation => {
       const username_ = username(annotation.user);
-      const displayValue =
-        showDisplayNames && annotation.user_info?.display_name
-          ? annotation.user_info.display_name
-          : username_;
-      users[username_] = displayValue;
+      users[username_] = annotationDisplayName(
+        annotation,
+        isThirdPartyUser(annotation.user, authDomain),
+        displayNamesEnabled
+      );
     });
 
     // If user-focus is configured (even if not applied) add a filter
@@ -60,5 +62,11 @@ export function useUserFilterOptions() {
     });
 
     return userOptions;
-  }, [annotations, currentUsername, focusFilters.user, showDisplayNames]);
+  }, [
+    annotations,
+    authDomain,
+    currentUsername,
+    displayNamesEnabled,
+    focusFilters.user,
+  ]);
 }