Skip to content

Commit 39fa413

Browse files
imnasnainaecclaude
andauthored
Fix broken new-interlinear-project (#122)
* Fix broken new-interlinear-project * Address code review feedback on fix-create-proj - Restructure createAndPersistProject to eliminate duplicate setCreateSourceIsSelect/setModal calls via a single exit path - Set isCreating during the discard-confirm createAndPersistProject path so the create modal buttons are disabled in that flow too - Update stale JSDoc: PendingReplace comment and component summary now accurately describe the immediate-persist New behavior - Split the old New-flow test into success and failure cases so resetActiveProject is asserted for the right reason in each - Add validation-failure test (parse succeeds, non-project shape) to cover lines 242-245 and restore 100% coverage - Fix no-type-assertion lint error in makeWebViewStateWithActiveProjectSpies - Update discard-confirm test to assert createProject is called after confirm Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Send error notification on all createAndPersistProject failure paths Matches the openProject convention: the catch block now sends the same notification as the validation-failure path, so transport-level rejections (where the backend may not have reached its own notification send) also surface visible feedback to the user. Expanded the JSDoc to document why the pre-round-trip newDraft() call is safe: dirty=false means all draft data is committed to the active project (or the draft was empty), and dirty=true means the user has already confirmed discarding via DiscardDraftConfirm. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix duplicate notification and keep create modal open on failure - Remove papi.notifications.send from createAndPersistProject catch block; interlinearizer.createProject already sends its own notification before rethrowing, so the second call caused a duplicate toast (handleSaveAsNew follows the same convention) - createAndPersistProject now returns boolean success; callers own the modal transition so the create modal stays open on failure, preserving the user name/description/language inputs for retry - useDraftProject.ts newDraft JSDoc updated to reflect that the caller is responsible for immediately persisting to the backend Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Cover discard-confirm -> create success path in tests The handleConfirmReplace else branch was uncovered: the existing test used the default mock (undefined -> JSON.parse throws -> success false), leaving the if (success) close path unreached. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix false-negative in discard-confirm pre-submit assertion expect.anything() does not match undefined, so not.toHaveBeenCalledWith with five expect.anything() placeholders passed even when createProject was called with undefined as arg 4. Replace with the exact expected argument values so the assertion correctly guards the not-yet-called path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Restore expect.anything() for non-undefined args in createProject guard Only arg 4 (undefined targetProjectId) needed to be explicit; the other arguments are non-undefined so expect.anything() is correct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 31ee21d commit 39fa413

3 files changed

Lines changed: 240 additions & 41 deletions

File tree

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

Lines changed: 134 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,25 @@ function makeWebViewStateWithResetSpy(resetActiveProject: () => void) {
292292
];
293293
}
294294

295+
/**
296+
* Builds a `useWebViewState` stub with spies on both the setter and the reset for the
297+
* `'activeProject'` key, so tests can assert which path was taken after a create attempt.
298+
*
299+
* @param setActiveProject - Spy invoked when the `'activeProject'` slot is set.
300+
* @param resetActiveProject - Spy invoked when the `'activeProject'` slot is reset.
301+
* @returns A `useWebViewState`-shaped hook stub.
302+
*/
303+
function makeWebViewStateWithActiveProjectSpies(
304+
setActiveProject: jest.Mock,
305+
resetActiveProject: jest.Mock,
306+
) {
307+
return <T,>(key: string, defaultValue: T): [T, (v: T) => void, () => void] => [
308+
defaultValue,
309+
key === 'activeProject' ? setActiveProject : () => {},
310+
key === 'activeProject' ? resetActiveProject : () => {},
311+
];
312+
}
313+
295314
describe('ProjectModals', () => {
296315
beforeEach(() => {
297316
jest.mocked(papi.notifications.send).mockResolvedValue('notification-id');
@@ -452,42 +471,105 @@ describe('ProjectModals', () => {
452471
});
453472

454473
describe('new (create) flow', () => {
455-
it('starts an empty draft, clears the active project, and closes when not dirty', async () => {
474+
it('seeds the draft, calls createProject on the backend, and closes', async () => {
475+
jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(MOCK_PROJECT));
456476
const newDraft = jest.fn();
457477
const setModal = jest.fn();
478+
const setActiveProject = jest.fn();
458479
const resetActiveProject = jest.fn();
459480
render(
460481
<ProjectModals
461482
{...buildProps({
462483
modal: 'create',
463484
newDraft,
464485
setModal,
465-
useWebViewState: makeWebViewStateWithResetSpy(resetActiveProject),
486+
useWebViewState: makeWebViewStateWithActiveProjectSpies(
487+
setActiveProject,
488+
resetActiveProject,
489+
),
466490
})}
467491
/>,
468492
);
469493

470494
await userEvent.click(screen.getByTestId('create-submit'));
471495

472-
// New starts a draft locally (carrying the typed name/description for the Save As prefill) and
473-
// clears the active project so Save routes to Save As; no backend project is created.
496+
await waitFor(() => expect(setActiveProject).toHaveBeenCalledWith(MOCK_PROJECT));
474497
expect(newDraft).toHaveBeenCalledWith({
475498
analysisLanguages: ['en'],
476499
suggestedName: 'New',
477500
suggestedDescription: 'Desc',
478501
});
479-
expect(papi.commands.sendCommand).not.toHaveBeenCalledWith(
502+
expect(papi.commands.sendCommand).toHaveBeenCalledWith(
480503
'interlinearizer.createProject',
481-
expect.anything(),
482-
expect.anything(),
483-
expect.anything(),
484-
expect.anything(),
485-
expect.anything(),
504+
'source-proj',
505+
['en'],
506+
undefined,
507+
'New',
508+
'Desc',
486509
);
487-
expect(resetActiveProject).toHaveBeenCalledTimes(1);
510+
expect(resetActiveProject).not.toHaveBeenCalled();
488511
expect(setModal).toHaveBeenCalledWith('none');
489512
});
490513

514+
it('falls back to resetActiveProject and keeps modal open when backend project creation fails', async () => {
515+
// Default mock returns undefined; JSON.parse(undefined) throws into the catch block.
516+
const newDraft = jest.fn();
517+
const setModal = jest.fn();
518+
const resetActiveProject = jest.fn();
519+
render(
520+
<ProjectModals
521+
{...buildProps({
522+
modal: 'create',
523+
newDraft,
524+
setModal,
525+
useWebViewState: makeWebViewStateWithResetSpy(resetActiveProject),
526+
})}
527+
/>,
528+
);
529+
530+
await userEvent.click(screen.getByTestId('create-submit'));
531+
532+
await waitFor(() => expect(resetActiveProject).toHaveBeenCalledTimes(1));
533+
expect(newDraft).toHaveBeenCalledWith({
534+
analysisLanguages: ['en'],
535+
suggestedName: 'New',
536+
suggestedDescription: 'Desc',
537+
});
538+
expect(papi.commands.sendCommand).toHaveBeenCalledWith(
539+
'interlinearizer.createProject',
540+
'source-proj',
541+
['en'],
542+
undefined,
543+
'New',
544+
'Desc',
545+
);
546+
expect(setModal).not.toHaveBeenCalledWith('none');
547+
});
548+
549+
it('notifies and falls back to resetActiveProject and keeps modal open when backend returns a non-project shape', async () => {
550+
jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify({ bad: true }));
551+
const setModal = jest.fn();
552+
const resetActiveProject = jest.fn();
553+
render(
554+
<ProjectModals
555+
{...buildProps({
556+
modal: 'create',
557+
setModal,
558+
useWebViewState: makeWebViewStateWithResetSpy(resetActiveProject),
559+
})}
560+
/>,
561+
);
562+
563+
await userEvent.click(screen.getByTestId('create-submit'));
564+
565+
await waitFor(() => expect(resetActiveProject).toHaveBeenCalledTimes(1));
566+
expect(papi.notifications.send).toHaveBeenCalledWith({
567+
message: '%interlinearizer_error_create_project_failed%',
568+
severity: 'error',
569+
});
570+
expect(setModal).not.toHaveBeenCalledWith('none');
571+
});
572+
491573
it('calls setModal with none when the create modal closes without a select source', async () => {
492574
const setModal = jest.fn();
493575
render(<ProjectModals {...buildProps({ modal: 'create', setModal })} />);
@@ -540,14 +622,22 @@ describe('ProjectModals', () => {
540622
expect(loadFromProject).not.toHaveBeenCalled();
541623
});
542624

543-
it('confirms before starting a new draft when the draft is dirty', async () => {
625+
it('confirms before creating a project when the draft is dirty', async () => {
544626
const newDraft = jest.fn();
545627
render(<ProjectModals {...buildProps({ modal: 'create', dirty: true, newDraft })} />);
546628

547629
await userEvent.click(screen.getByTestId('create-submit'));
548630
expect(screen.getByTestId('discard-modal')).toBeInTheDocument();
549-
// The new draft must not start until the user confirms discarding the current one.
631+
// Neither draft nor project creation should start until the user confirms discarding.
550632
expect(newDraft).not.toHaveBeenCalled();
633+
expect(papi.commands.sendCommand).not.toHaveBeenCalledWith(
634+
'interlinearizer.createProject',
635+
expect.anything(),
636+
expect.anything(),
637+
undefined,
638+
expect.anything(),
639+
expect.anything(),
640+
);
551641

552642
await userEvent.click(screen.getByTestId('discard-confirm'));
553643
await waitFor(() =>
@@ -557,6 +647,37 @@ describe('ProjectModals', () => {
557647
suggestedDescription: 'Desc',
558648
}),
559649
);
650+
expect(papi.commands.sendCommand).toHaveBeenCalledWith(
651+
'interlinearizer.createProject',
652+
'source-proj',
653+
['en'],
654+
undefined,
655+
'New',
656+
'Desc',
657+
);
658+
});
659+
660+
it('creates the project and closes after confirming the discard on a dirty draft', async () => {
661+
jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(MOCK_PROJECT));
662+
const setModal = jest.fn();
663+
const setActiveProject = jest.fn();
664+
render(
665+
<ProjectModals
666+
{...buildProps({
667+
modal: 'create',
668+
dirty: true,
669+
setModal,
670+
useWebViewState: makeWebViewStateWithActiveProjectSpies(setActiveProject, jest.fn()),
671+
})}
672+
/>,
673+
);
674+
675+
await userEvent.click(screen.getByTestId('create-submit'));
676+
expect(screen.getByTestId('discard-modal')).toBeInTheDocument();
677+
678+
await userEvent.click(screen.getByTestId('discard-confirm'));
679+
await waitFor(() => expect(setModal).toHaveBeenCalledWith('none'));
680+
expect(setActiveProject).toHaveBeenCalledWith(MOCK_PROJECT);
560681
});
561682

562683
it('disables the discard-confirm button while an open is in flight', async () => {

0 commit comments

Comments
 (0)