Open fast, indicate the probe, and make the offer report one door

Author: jasonleenaylor

__mocks__/platform-bible-react.tsx

@@ -623,6 +623,11 @@ export function RadioGroupItem({
  * {@link DialogContent} and {@link DialogTitle}, mirroring how the real Radix-based component
  * reaches its parts from the root.
  */
+/** Stub spinner: a marker element standing in for the platform's indeterminate spinner. */
+export function Spinner({ className }: { className?: string }) {
+  return <span className={className} data-testid="spinner" />;
+}
+
 const DialogContext = createContext<{ onOpenChange?: (open: boolean) => void; titleId?: string }>(
   {},
 );

contributions/localizedStrings.json

@@ -169,6 +169,7 @@
       "%interlinearizer_pt9ConvertPrompt_message%": "This project has Paratext 9 interlinear data. Would you like to convert it now?",
       "%interlinearizer_pt9ConvertPrompt_yes%": "Yes",
       "%interlinearizer_pt9ConvertPrompt_no%": "No",
+      "%interlinearizer_pt9ConvertPrompt_checking%": "Checking for Paratext 9 interlinear data…",
       "%interlinearizer_pt9ImportModal_title%": "Import from Paratext 9",
       "%interlinearizer_pt9ImportModal_syncTitle%": "Sync from Paratext 9",
       "%interlinearizer_pt9ImportModal_importing%": "Importing from Paratext 9…",

src/__tests__/components/InterlinearizerLoader.test.tsx

@@ -1151,7 +1151,15 @@ describe('InterlinearizerLoader', () => {
       expect(screen.queryByTestId('wipe-modal-panel')).not.toBeInTheDocument();
     });
 
+    /** Points the frontend PDP mock at a manifest, so the offer probe answers `available`. */
+    function mockOfferProbe(manifest: Record<string, string> = { 'Interlinear_en/x.xml': 'h' }) {
+      mockPdpGet.mockResolvedValue({
+        getPt9InterlinearManifest: jest.fn().mockResolvedValue(manifest),
+      });
+    }
+
     it('offers the first-open conversion and runs the import on Yes', async () => {
+      mockOfferProbe();
       mockImportCommands({
         importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT },
       });
@@ -1173,6 +1181,7 @@ describe('InterlinearizerLoader', () => {
     });
 
     it('returns to the plain view when an offer-run report is closed', async () => {
+      mockOfferProbe();
       mockImportCommands({
         importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT },
       });
@@ -1192,6 +1201,7 @@ describe('InterlinearizerLoader', () => {
     });
 
     it('persists the empty draft and runs no import on No', async () => {
+      mockOfferProbe();
       mockImportCommands();
       await act(async () => {
         renderLoader({ useWebViewState: makeWebViewState({ offerPt9Import: true }) });
@@ -1214,6 +1224,7 @@ describe('InterlinearizerLoader', () => {
     });
 
     it('holds the offer while the draft is still loading', async () => {
+      mockOfferProbe();
       mockSendCommand.mockImplementation(() => new Promise(() => {}));
       await act(async () => {
         renderLoader({ useWebViewState: makeWebViewState({ offerPt9Import: true }) });
@@ -1222,7 +1233,61 @@ describe('InterlinearizerLoader', () => {
       expect(screen.queryByTestId('pt9-convert-prompt-message')).not.toBeInTheDocument();
     });
 
+    it('never offers when the probe finds no convertible data', async () => {
+      mockOfferProbe({});
+      mockImportCommands();
+      await act(async () => {
+        renderLoader({ useWebViewState: makeWebViewState({ offerPt9Import: true }) });
+      });
+
+      expect(screen.queryByTestId('pt9-convert-prompt-message')).not.toBeInTheDocument();
+      expect(screen.queryByTestId('pt9-checking')).not.toBeInTheDocument();
+    });
+
+    it('shows the checking status only when the probe is still unanswered after the delay', async () => {
+      jest.useFakeTimers();
+      try {
+        mockPdpGet.mockResolvedValue({
+          getPt9InterlinearManifest: jest.fn(() => new Promise(() => {})),
+        });
+        mockImportCommands();
+        await act(async () => {
+          renderLoader({ useWebViewState: makeWebViewState({ offerPt9Import: true }) });
+        });
+        expect(screen.queryByTestId('pt9-checking')).not.toBeInTheDocument();
+
+        await act(async () => {
+          jest.advanceTimersByTime(400);
+        });
+
+        expect(screen.getByTestId('pt9-checking')).toBeInTheDocument();
+      } finally {
+        jest.useRealTimers();
+      }
+    });
+
+    it('never flashes the checking status when the probe answers fast', async () => {
+      jest.useFakeTimers();
+      try {
+        mockOfferProbe();
+        mockImportCommands();
+        await act(async () => {
+          renderLoader({ useWebViewState: makeWebViewState({ offerPt9Import: true }) });
+        });
+
+        await act(async () => {
+          jest.advanceTimersByTime(400);
+        });
+
+        expect(screen.queryByTestId('pt9-checking')).not.toBeInTheDocument();
+        expect(screen.getByTestId('pt9-convert-prompt-message')).toBeInTheDocument();
+      } finally {
+        jest.useRealTimers();
+      }
+    });
+
     it('hides the offer behind an open modal', async () => {
+      mockOfferProbe();
       mockImportCommands({
         importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT },
       });

src/__tests__/components/modals/Pt9ConvertPromptModal.test.tsx

@@ -4,14 +4,18 @@
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { useLocalizedStrings } from '@papi/frontend/react';
-import { Pt9ConvertPromptModal } from '../../../components/modals/Pt9ConvertPromptModal';
+import {
+  Pt9CheckingModal,
+  Pt9ConvertPromptModal,
+} from '../../../components/modals/Pt9ConvertPromptModal';
 
 const LOCALIZED: Record<string, string> = {
   '%interlinearizer_pt9ImportModal_title%': 'Import from Paratext 9',
   '%interlinearizer_pt9ConvertPrompt_message%':
     'This project has Paratext 9 interlinear data. Would you like to convert it now?',
   '%interlinearizer_pt9ConvertPrompt_yes%': 'Yes',
   '%interlinearizer_pt9ConvertPrompt_no%': 'No',
+  '%interlinearizer_pt9ConvertPrompt_checking%': 'Checking for Paratext 9 interlinear data…',
 };
 
 describe('Pt9ConvertPromptModal', () => {
@@ -50,3 +54,19 @@ describe('Pt9ConvertPromptModal', () => {
     expect(onNo).toHaveBeenCalledTimes(1);
   });
 });
+
+describe('Pt9CheckingModal', () => {
+  beforeEach(() => {
+    jest.mocked(useLocalizedStrings).mockReturnValue([LOCALIZED, false]);
+  });
+
+  it('shows the spinner and the checking status with no dismiss affordances', () => {
+    render(<Pt9CheckingModal />);
+
+    expect(screen.getByTestId('pt9-checking')).toHaveTextContent(
+      'Checking for Paratext 9 interlinear data…',
+    );
+    expect(screen.getByTestId('spinner')).toBeInTheDocument();
+    expect(screen.queryByRole('button')).not.toBeInTheDocument();
+  });
+});

src/__tests__/components/modals/Pt9ImportModal.test.tsx

@@ -56,7 +56,7 @@ function makeReport(): Pt9ImportReport {
             clustersTotal: 20,
             clustersConverted: 15,
             phrasesConverted: 2,
-            clusterDrops: { ...emptyDrops, formMismatch: 4, verseNotFound: 1 },
+            clusterDrops: { ...emptyDrops, formMismatch: 4, verseNotFound: 1, duplicateCluster: 1 },
             ambiguousAnchors: 0,
             punctuationEntriesIgnored: 0,
           },
@@ -160,7 +160,9 @@ describe('Pt9ImportModal', () => {
     expect(report).toHaveTextContent('Languages: en, fr');
     expect(report).toHaveTextContent('Books: MAT, MRK');
     expect(report).toHaveTextContent('15 of 25 clusters, 2 phrases');
-    expect(report).toHaveTextContent('10 clusters: 6 verse not found; 4 did not match the text');
+    expect(report).toHaveTextContent('11 clusters: 6 verse not found; 4 did not match the text');
+    // Only the top two reasons are named; the third stays in the JSON report.
+    expect(report).not.toHaveTextContent('duplicate data');
     expect(report).toHaveTextContent('Books with no text: MRK');
   });
 
@@ -205,6 +207,22 @@ describe('Pt9ImportModal', () => {
     expect(onOpen).toHaveBeenCalled();
   });
 
+  it('offers a single Open on an offer-run report and fires it', async () => {
+    const onOpen = jest.fn();
+    render(
+      <Pt9ImportModal
+        phase={{ kind: 'report', report: makeReport() }}
+        mode="offer"
+        onOpen={onOpen}
+        onClose={jest.fn()}
+      />,
+    );
+
+    expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument();
+    await userEvent.click(screen.getByRole('button', { name: 'Open' }));
+    expect(onOpen).toHaveBeenCalled();
+  });
+
   it('offers only Close on a sync report', () => {
     render(
       <Pt9ImportModal

src/__tests__/hooks/usePt9ImportAvailability.test.ts

@@ -2,7 +2,7 @@
 
 import { renderHook, waitFor } from '@testing-library/react';
 import papi from '@papi/frontend';
-import usePt9ImportAvailability from '../../hooks/usePt9ImportAvailability';
+import usePt9ImportAvailability, { usePt9ImportProbe } from '../../hooks/usePt9ImportAvailability';
 import { getMockedPdpGet, makeStubProject } from '../test-helpers';
 
 const mockPdpGet = getMockedPdpGet(papi);
@@ -58,6 +58,22 @@ describe('usePt9ImportAvailability', () => {
     expect(result.current).toBe(false);
   });
 
+  it('returns to false when a re-probe fails after an earlier success', async () => {
+    mockManifest({ 'Lexicon.xml': 'aaaa1111' });
+    const { result, rerender } = renderHook(
+      ({ loading }) => usePt9ImportAvailability('src-project', [], loading),
+      { initialProps: { loading: false } },
+    );
+    await waitFor(() => expect(result.current).toBe(true));
+
+    rerender({ loading: true });
+    mockPdpGet.mockRejectedValue(new Error('probe failed'));
+    rerender({ loading: false });
+
+    await waitFor(() => expect(mockPdpGet).toHaveBeenCalledTimes(2));
+    expect(result.current).toBe(false);
+  });
+
   it('ignores a probe that lands after unmount', async () => {
     let resolveManifest: (m: Record<string, string>) => void = () => {};
     mockPdpGet.mockResolvedValue({
@@ -75,3 +91,37 @@ describe('usePt9ImportAvailability', () => {
     // update after unmount would emit one) is the observable behavior.
   });
 });
+
+describe('usePt9ImportProbe', () => {
+  it('moves from pending to available when the manifest lists files', async () => {
+    mockManifest({ 'Lexicon.xml': 'aaaa1111' });
+
+    const { result } = renderHook(() => usePt9ImportProbe('src-project', true));
+
+    expect(result.current).toBe('pending');
+    await waitFor(() => expect(result.current).toBe('available'));
+  });
+
+  it('reports unavailable for an empty manifest', async () => {
+    mockManifest({});
+
+    const { result } = renderHook(() => usePt9ImportProbe('src-project', true));
+
+    await waitFor(() => expect(result.current).toBe('unavailable'));
+  });
+
+  it('reports unavailable when the probe fails', async () => {
+    mockPdpGet.mockRejectedValue(new Error('no such projectInterface'));
+
+    const { result } = renderHook(() => usePt9ImportProbe('src-project', true));
+
+    await waitFor(() => expect(result.current).toBe('unavailable'));
+  });
+
+  it('stays pending and never probes while disabled', () => {
+    const { result } = renderHook(() => usePt9ImportProbe('src-project', false));
+
+    expect(result.current).toBe('pending');
+    expect(mockPdpGet).not.toHaveBeenCalled();
+  });
+});

src/__tests__/main.test.ts

@@ -450,12 +450,12 @@ describe('main', () => {
         webViewType: 'someExtension.view',
         projectId: 'project-from-webview',
       });
-      jest.mocked(pt9ImportService.shouldOfferPt9Import).mockResolvedValue(true);
+      jest.mocked(pt9ImportService.hasNoInterlinearizerState).mockResolvedValue(true);
       const openForWebView = await getOpenForWebViewHandler();
 
       await openForWebView('some-webview');
 
-      expect(jest.mocked(pt9ImportService.shouldOfferPt9Import)).toHaveBeenCalledWith(
+      expect(jest.mocked(pt9ImportService.hasNoInterlinearizerState)).toHaveBeenCalledWith(
         expect.anything(),
         'project-from-webview',
       );
@@ -628,7 +628,7 @@ describe('main', () => {
         );
       });
 
-      it('does not probe for the convert offer when reusing an existing tab', async () => {
+      it('does not compute the convert offer when reusing an existing tab', async () => {
         __mockSelectProject.mockResolvedValue('my-project');
         const context = createTestActivationContext();
         await activate(context);
@@ -638,7 +638,7 @@ describe('main', () => {
 
         await findRegisteredHandler('interlinearizer.openForWebView')?.();
 
-        expect(jest.mocked(pt9ImportService.shouldOfferPt9Import)).not.toHaveBeenCalled();
+        expect(jest.mocked(pt9ImportService.hasNoInterlinearizerState)).not.toHaveBeenCalled();
         expect(__mockOpenWebView).toHaveBeenCalledWith(
           mainWebViewType,
           undefined,

src/__tests__/services/pt9ImportService.test.ts

@@ -5,7 +5,7 @@ import * as path from 'node:path';
 
 import papiBackendMock from '@papi/backend';
 import type { Pt9InterlinearProjectData } from 'platform-scripture';
-import { importPt9Project, shouldOfferPt9Import } from '../../services/pt9ImportService';
+import { hasNoInterlinearizerState, importPt9Project } from '../../services/pt9ImportService';
 import { resetQueuesForTesting } from '../../services/projectStorage';
 import { createTestActivationContext, enoentError, makeStubProject } from '../test-helpers';
 
@@ -281,67 +281,47 @@ describe('importPt9Project', () => {
   });
 });
 
-describe('shouldOfferPt9Import', () => {
+describe('hasNoInterlinearizerState', () => {
   beforeEach(() => {
     resetQueuesForTesting();
   });
 
   /** Seeds storage reads by key; unlisted keys read as never written. */
   function seedStorage(entries: Record<string, string>): void {
-    __mockReadUserData.mockImplementation(async (_token: unknown, key: string) => {
-      if (Object.hasOwn(entries, key)) return entries[key];
+    __mockReadUserData.mockImplementation(async (_token: unknown, key: unknown) => {
+      if (typeof key === 'string' && Object.hasOwn(entries, key)) return entries[key];
       throw enoentError();
     });
   }
 
-  /** Points the PT9 PDP mock at a manifest carrying the given file paths. */
-  function seedManifest(paths: string[]): void {
-    __mockProjectDataProvidersGet.mockResolvedValue({
-      getPt9InterlinearManifest: jest
-        .fn()
-        .mockResolvedValue(Object.fromEntries(paths.map((filePath) => [filePath, 'hash']))),
-    });
-  }
-
-  it('offers when nothing is stored and the source has an interlinear book file', async () => {
+  it('answers true when the source has no draft and no projects', async () => {
     seedStorage({});
-    seedManifest(['Lexicon.xml', 'Interlinear_en/Interlinear_en_MAT.xml']);
 
-    await expect(shouldOfferPt9Import(token, 'src-project')).resolves.toBe(true);
+    await expect(hasNoInterlinearizerState(token, 'src-project')).resolves.toBe(true);
   });
 
-  it('does not offer when a draft is already stored', async () => {
+  it('answers false when a draft is already stored', async () => {
     seedStorage({ 'draft:src-project': 'anything' });
 
-    await expect(shouldOfferPt9Import(token, 'src-project')).resolves.toBe(false);
-    expect(__mockProjectDataProvidersGet).not.toHaveBeenCalled();
+    await expect(hasNoInterlinearizerState(token, 'src-project')).resolves.toBe(false);
   });
 
-  it('does not offer when a project already exists for the source', async () => {
+  it('answers false when a project already exists for the source', async () => {
     seedStorage({
       projectIds: JSON.stringify(['p1']),
       'project:p1': JSON.stringify(makeStubProject('p1')),
     });
 
-    await expect(shouldOfferPt9Import(token, 'src-project')).resolves.toBe(false);
-    expect(__mockProjectDataProvidersGet).not.toHaveBeenCalled();
-  });
-
-  it('does not offer for a lexicon- or word-analyses-only manifest', async () => {
-    seedStorage({});
-    seedManifest(['Lexicon.xml', 'WordAnalyses.xml']);
-
-    await expect(shouldOfferPt9Import(token, 'src-project')).resolves.toBe(false);
+    await expect(hasNoInterlinearizerState(token, 'src-project')).resolves.toBe(false);
   });
 
-  it('answers false when the probe fails, and only warns', async () => {
-    seedStorage({});
-    __mockProjectDataProvidersGet.mockRejectedValue(new Error('interface unsupported'));
+  it('answers false when the state check fails, and only warns', async () => {
+    __mockReadUserData.mockRejectedValue(new Error('storage unavailable'));
 
-    await expect(shouldOfferPt9Import(token, 'src-project')).resolves.toBe(false);
+    await expect(hasNoInterlinearizerState(token, 'src-project')).resolves.toBe(false);
     expect(__mockLogger.warn).toHaveBeenCalledWith(
-      'Interlinearizer: Paratext 9 convert-offer probe failed; not offering',
-      expect.objectContaining({ message: 'interface unsupported' }),
+      'Interlinearizer: Paratext 9 convert-offer state check failed; not offering',
+      expect.objectContaining({ message: 'storage unavailable' }),
     );
   });
 });