Exclude replies from import

In the initial implementation of import, we have decided to exclude replies to
simplify the feature. In other words, the import feature only supports importing
top level annotations from a single user.

src/sidebar/components/ShareDialog/ImportAnnotations.tsx

@@ -2,6 +2,7 @@ import { Button, CardActions, Select } from '@hypothesis/frontend-shared';
 import { useCallback, useEffect, useId, useMemo, useState } from 'preact/hooks';
 
 import type { APIAnnotationData } from '../../../types/api';
+import { isReply } from '../../helpers/annotation-metadata';
 import { annotationDisplayName } from '../../helpers/annotation-user';
 import { readExportFile } from '../../helpers/import';
 import { withServices } from '../../service-context';
@@ -10,29 +11,37 @@ import { useSidebarStore } from '../../store';
 import FileInput from './FileInput';
 import LoadingSpinner from './LoadingSpinner';
 
-type UserAnnotationCount = {
+/** Details of a user and their annotations that are available to import. */
+type UserAnnotations = {
   userid: string;
   displayName: string;
-
-  /** Number of annotations made by this user. */
-  count: number;
+  annotations: APIAnnotationData[];
 };
 
 /**
- * Generate an alphabetized list of authors and their annotation counts.
+ * Generate an alphabetized list of authors and their importable annotations.
  */
-function annotationCountsByUser(
+function annotationsByUser(
   anns: APIAnnotationData[],
   getDisplayName: (ann: APIAnnotationData) => string,
-): UserAnnotationCount[] {
-  const userInfo = new Map<string, UserAnnotationCount>();
+): UserAnnotations[] {
+  const userInfo = new Map<string, UserAnnotations>();
   for (const ann of anns) {
+    if (isReply(ann)) {
+      // We decided to exclude replies from the initial implementation of
+      // annotation import, to simplify the feature.
+      continue;
+    }
     let info = userInfo.get(ann.user);
     if (!info) {
-      info = { userid: ann.user, displayName: getDisplayName(ann), count: 0 };
+      info = {
+        userid: ann.user,
+        displayName: getDisplayName(ann),
+        annotations: [],
+      };
       userInfo.set(ann.user, info);
     }
-    info.count += 1;
+    info.annotations.push(ann);
   }
   const userInfos = [...userInfo.values()];
   userInfos.sort((a, b) => a.displayName.localeCompare(b.displayName));
@@ -74,8 +83,7 @@ function ImportAnnotations({
     [defaultAuthority, displayNamesEnabled],
   );
   const userList = useMemo(
-    () =>
-      annotations ? annotationCountsByUser(annotations, getDisplayName) : null,
+    () => (annotations ? annotationsByUser(annotations, getDisplayName) : null),
     [annotations, getDisplayName],
   );
 
@@ -104,13 +112,17 @@ function ImportAnnotations({
   }, [currentUser, file]);
 
   let importAnnotations;
-  if (annotations && selectedUser) {
+  if (annotations && selectedUser && userList) {
     importAnnotations = async () => {
-      const annsToImport = annotations.filter(ann => ann.user === selectedUser);
+      const userEntry = userList.find(item => item.userid === selectedUser);
+      /* istanbul ignore next - This should never be triggered */
+      if (!userEntry) {
+        return;
+      }
 
       // nb. In the event of an error, `import` will report that directly via
       // a toast message, so we don't do that ourselves.
-      importAnnotationsService.import(annsToImport);
+      importAnnotationsService.import(userEntry.annotations);
     };
   }
 
@@ -154,7 +166,7 @@ function ImportAnnotations({
                 value={userInfo.userid}
                 selected={userInfo.userid === selectedUser}
               >
-                {userInfo.displayName} ({userInfo.count})
+                {userInfo.displayName} ({userInfo.annotations.length})
               </option>
             ))}
           </Select>

src/sidebar/components/ShareDialog/test/ImportAnnotations-test.js

@@ -74,35 +74,78 @@ describe('ImportAnnotations', () => {
     assert.isTrue(getImportButton(wrapper).prop('disabled'));
   });
 
-  it('displays user list when a valid file is selected', async () => {
-    const wrapper = createImportAnnotations();
-
-    selectFile(wrapper, [
-      {
-        user: 'acct:john@example.com',
-        user_info: {
-          display_name: 'John Smith',
+  [
+    // File with a mix of annotations and replies.
+    {
+      annotations: [
+        {
+          id: 'abc',
+          user: 'acct:john@example.com',
+          user_info: {
+            display_name: 'John Smith',
+          },
+          text: 'Test annotation',
         },
-        text: 'Test annotation',
-      },
-      {
-        user: 'acct:brian@example.com',
-        user_info: {
-          display_name: 'Brian Smith',
+
+        {
+          id: 'def',
+          user: 'acct:brian@example.com',
+          user_info: {
+            display_name: 'Brian Smith',
+          },
+          text: 'Test annotation',
         },
-        text: 'Test annotation',
-      },
-    ]);
 
-    const userList = await waitForElement(wrapper, 'Select');
-    const users = userList.find('option');
-    assert.equal(users.length, 3);
-    assert.equal(users.at(0).prop('value'), '');
-    assert.equal(users.at(0).text(), '');
-    assert.equal(users.at(1).prop('value'), 'acct:brian@example.com');
-    assert.equal(users.at(1).text(), 'Brian Smith (1)');
-    assert.equal(users.at(2).prop('value'), 'acct:john@example.com');
-    assert.equal(users.at(2).text(), 'John Smith (1)');
+        // A reply, this shouldn't be counted in the list.
+        {
+          id: 'xyz',
+          user: 'acct:brian@example.com',
+          user_info: {
+            display_name: 'Brian Smith',
+          },
+          text: 'Test annotation',
+          references: ['abc'],
+        },
+      ],
+      userEntries: [
+        { value: '', text: '' }, // "No user selected" entry
+        { value: 'acct:brian@example.com', text: 'Brian Smith (1)' },
+        { value: 'acct:john@example.com', text: 'John Smith (1)' },
+      ],
+    },
+
+    // File with a single reply.
+    {
+      annotations: [
+        {
+          id: 'xyz',
+          user: 'acct:brian@example.com',
+          user_info: {
+            display_name: 'Brian Smith',
+          },
+          text: 'Test annotation',
+          references: ['abc'],
+        },
+      ],
+      userEntries: [
+        { value: '', text: '' }, // "No user selected" entry
+      ],
+    },
+  ].forEach(({ annotations, userEntries }) => {
+    it('displays user list when a valid file is selected', async () => {
+      const wrapper = createImportAnnotations();
+
+      selectFile(wrapper, annotations);
+
+      const userList = await waitForElement(wrapper, 'Select');
+      const users = userList.find('option');
+      assert.equal(users.length, userEntries.length);
+
+      for (const [i, entry] of userEntries.entries()) {
+        assert.equal(users.at(i).prop('value'), entry.value);
+        assert.equal(users.at(i).text(), entry.text);
+      }
+    });
   });
 
   it('displays error if file is invalid', async () => {
@@ -156,7 +199,9 @@ describe('ImportAnnotations', () => {
   it('imports annotations when "Import" button is clicked', async () => {
     const wrapper = createImportAnnotations();
     const annotations = [
+      // Annotation by a different user. This should be ignored.
       {
+        id: 'abc',
         user: 'acct:john@example.com',
         user_info: {
           display_name: 'John Smith',
@@ -164,6 +209,17 @@ describe('ImportAnnotations', () => {
         text: 'Test annotation',
       },
       {
+        id: 'def',
+        user: 'acct:brian@example.com',
+        user_info: {
+          display_name: 'Brian Smith',
+        },
+        text: 'Test annotation',
+      },
+      // Reply by selected user. This should be ignored.
+      {
+        id: 'xyz',
+        references: ['abc'],
         user: 'acct:brian@example.com',
         user_info: {
           display_name: 'Brian Smith',
@@ -184,7 +240,9 @@ describe('ImportAnnotations', () => {
 
     assert.calledWith(
       fakeImportAnnotationsService.import,
-      annotations.filter(ann => ann.user === 'acct:brian@example.com'),
+      annotations.filter(
+        ann => ann.user === 'acct:brian@example.com' && !ann.references,
+      ),
     );
   });
 

src/sidebar/helpers/annotation-metadata.ts

@@ -121,7 +121,7 @@ function titleTextFromAnnotation(annotation: Annotation): string {
 /**
  * Return `true` if the given annotation is a reply, `false` otherwise.
  */
-export function isReply(annotation: Annotation): boolean {
+export function isReply(annotation: APIAnnotationData): boolean {
   return (annotation.references || []).length > 0;
 }