Skip to content

Commit b5d68ee

Browse files
imnasnainaecclaude
andcommitted
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>
1 parent 63f2a0f commit b5d68ee

3 files changed

Lines changed: 45 additions & 30 deletions

File tree

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ describe('ProjectModals', () => {
511511
expect(setModal).toHaveBeenCalledWith('none');
512512
});
513513

514-
it('falls back to resetActiveProject and closes when backend project creation fails', async () => {
514+
it('falls back to resetActiveProject and keeps modal open when backend project creation fails', async () => {
515515
// Default mock returns undefined; JSON.parse(undefined) throws into the catch block.
516516
const newDraft = jest.fn();
517517
const setModal = jest.fn();
@@ -529,7 +529,7 @@ describe('ProjectModals', () => {
529529

530530
await userEvent.click(screen.getByTestId('create-submit'));
531531

532-
await waitFor(() => expect(setModal).toHaveBeenCalledWith('none'));
532+
await waitFor(() => expect(resetActiveProject).toHaveBeenCalledTimes(1));
533533
expect(newDraft).toHaveBeenCalledWith({
534534
analysisLanguages: ['en'],
535535
suggestedName: 'New',
@@ -543,14 +543,10 @@ describe('ProjectModals', () => {
543543
'New',
544544
'Desc',
545545
);
546-
expect(papi.notifications.send).toHaveBeenCalledWith({
547-
message: '%interlinearizer_error_create_project_failed%',
548-
severity: 'error',
549-
});
550-
expect(resetActiveProject).toHaveBeenCalledTimes(1);
546+
expect(setModal).not.toHaveBeenCalledWith('none');
551547
});
552548

553-
it('notifies and falls back to resetActiveProject when backend returns a non-project shape', async () => {
549+
it('notifies and falls back to resetActiveProject and keeps modal open when backend returns a non-project shape', async () => {
554550
jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify({ bad: true }));
555551
const setModal = jest.fn();
556552
const resetActiveProject = jest.fn();
@@ -566,12 +562,12 @@ describe('ProjectModals', () => {
566562

567563
await userEvent.click(screen.getByTestId('create-submit'));
568564

569-
await waitFor(() => expect(setModal).toHaveBeenCalledWith('none'));
565+
await waitFor(() => expect(resetActiveProject).toHaveBeenCalledTimes(1));
570566
expect(papi.notifications.send).toHaveBeenCalledWith({
571567
message: '%interlinearizer_error_create_project_failed%',
572568
severity: 'error',
573569
});
574-
expect(resetActiveProject).toHaveBeenCalledTimes(1);
570+
expect(setModal).not.toHaveBeenCalledWith('none');
575571
});
576572

577573
it('calls setModal with none when the create modal closes without a select source', async () => {

src/components/modals/ProjectModals.tsx

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -220,15 +220,17 @@ export default function ProjectModals({
220220
* project or the draft was empty — nothing is lost. When dirty is `true` the
221221
* {@link DiscardDraftConfirm} dialog has already obtained explicit user consent to discard.
222222
*
223-
* Sends an error notification on any failure path (matching the convention in {@link openProject})
224-
* so the user always gets feedback even when the backend error notification does not reach the
225-
* frontend (e.g. a transport-level rejection).
223+
* The `interlinearizer.createProject` command sends its own error notification before rethrowing,
224+
* so the catch block only needs to log — callers do not need to send a second notification. This
225+
* matches {@link handleSaveAsNew}, which uses the same command and follows the same pattern.
226226
*
227227
* @param config - The configuration collected by the New dialog.
228-
* @returns A promise that resolves once the project is created (or the failure is handled).
228+
* @returns `true` if the project was created and persisted successfully; `false` otherwise.
229+
* Callers are responsible for closing the modal on success and keeping it open on failure so
230+
* the user can retry without re-entering their inputs.
229231
*/
230232
const createAndPersistProject = useCallback(
231-
async (config: CreateDraftConfig) => {
233+
async (config: CreateDraftConfig): Promise<boolean> => {
232234
newDraft({
233235
analysisLanguages: config.analysisLanguages,
234236
...(config.name !== undefined && { suggestedName: config.name }),
@@ -254,19 +256,15 @@ export default function ProjectModals({
254256
}
255257
} catch (e) {
256258
logger.error('Interlinearizer: failed to create project from New dialog', e);
257-
await papi.notifications
258-
.send({ message: '%interlinearizer_error_create_project_failed%', severity: 'error' })
259-
.catch(() => {});
260259
}
261260
if (created) {
262261
setActiveProject(created);
263262
} else {
264263
resetActiveProject();
265264
}
266-
setCreateSourceIsSelect(false);
267-
setModal('none');
265+
return created !== undefined;
268266
},
269-
[newDraft, projectId, resetActiveProject, setActiveProject, setModal],
267+
[newDraft, projectId, resetActiveProject, setActiveProject],
270268
);
271269

272270
/**
@@ -286,7 +284,8 @@ export default function ProjectModals({
286284
/**
287285
* Called when the New dialog is submitted. Creates and persists the project immediately, or
288286
* defers behind the unsaved-changes confirmation when the draft is dirty. Disables the modal
289-
* buttons via `isCreating` during the backend round-trip.
287+
* buttons via `isCreating` during the backend round-trip. Closes on success; stays open on
288+
* failure so the user can retry without re-entering their inputs.
290289
*
291290
* @param config - The configuration collected by the New dialog.
292291
*/
@@ -298,15 +297,24 @@ export default function ProjectModals({
298297
}
299298
setIsCreating(true);
300299
try {
301-
await createAndPersistProject(config);
300+
const success = await createAndPersistProject(config);
301+
if (success) {
302+
setCreateSourceIsSelect(false);
303+
setModal('none');
304+
}
302305
} finally {
303306
setIsCreating(false);
304307
}
305308
},
306-
[createAndPersistProject, dirty],
309+
[createAndPersistProject, dirty, setCreateSourceIsSelect, setModal],
307310
);
308311

309-
/** Confirms the deferred draft-replacing action after the user accepts losing unsaved changes. */
312+
/**
313+
* Confirms the deferred draft-replacing action after the user accepts losing unsaved changes. For
314+
* a deferred Open, delegates entirely to {@link openProject}. For a deferred New, closes on
315+
* success; on failure the discard confirm is dismissed but the underlying create modal stays
316+
* visible so the user can retry.
317+
*/
310318
const handleConfirmReplace = useCallback(async () => {
311319
/* v8 ignore next -- the confirm only renders while a pending action exists */
312320
if (!pendingReplace) return;
@@ -321,7 +329,11 @@ export default function ProjectModals({
321329
} else {
322330
setIsCreating(true);
323331
try {
324-
await createAndPersistProject(pendingReplace.config);
332+
const success = await createAndPersistProject(pendingReplace.config);
333+
if (success) {
334+
setCreateSourceIsSelect(false);
335+
setModal('none');
336+
}
325337
} finally {
326338
setIsCreating(false);
327339
}
@@ -330,7 +342,14 @@ export default function ProjectModals({
330342
setIsReplacing(false);
331343
setPendingReplace(undefined);
332344
}
333-
}, [createAndPersistProject, isReplacing, openProject, pendingReplace]);
345+
}, [
346+
createAndPersistProject,
347+
isReplacing,
348+
openProject,
349+
pendingReplace,
350+
setCreateSourceIsSelect,
351+
setModal,
352+
]);
334353

335354
/** Cancels the deferred action, returning to the underlying modal with the draft untouched. */
336355
const handleCancelReplace = useCallback(() => setPendingReplace(undefined), []);

src/hooks/useDraftProject.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ export type UseDraftProjectResult = {
6868
loadFromProject: (project: OpenableProject) => void;
6969
/**
7070
* Starts a fresh, empty draft for the current source — the "New" flow. Seeds the chosen analysis
71-
* languages and retains the typed name/description as `suggestedName`/`suggestedDescription` to
72-
* prefill Save As; no backend project is created until the user explicitly saves. The new draft
73-
* is clean (`dirty: false`), so the unsaved-changes indicator stays clear until the first edit.
71+
* languages and retains the typed name/description as `suggestedName`/`suggestedDescription`.
72+
* The new draft is clean (`dirty: false`), so the unsaved-changes indicator stays clear until
73+
* the first edit. The caller is responsible for immediately persisting the project to the backend.
7474
*
7575
* @param config - The languages and optional suggested name/description from the New dialog.
7676
*/

0 commit comments

Comments
 (0)