diff --git a/sharedUtils/bookNameValidation.ts b/sharedUtils/bookNameValidation.ts new file mode 100644 index 000000000..5cb90e0e1 --- /dev/null +++ b/sharedUtils/bookNameValidation.ts @@ -0,0 +1,71 @@ +/** + * Validation rules for user-edited book display names (`fileDisplayName`). + * + * Keep this list narrow and conservative. The primary motivation (issue #1013) + * is that earlier metadata-cleanup logic ran `path.extname()` over the display + * name and treated anything after a `.` as a strippable extension, silently + * truncating names like "1. New Items" to "1" on every sync. The safest fix + * is to disallow filesystem-unsafe characters at the entry point so the name + * round-trips cleanly through metadata, file IO, and any future tooling that + * may interpret the value as a path. + * + * Shared between the extension host (defensive validation in providers) and + * the webview (live input warnings + disabled save). + */ + +/** + * Characters that must not appear in a book display name. + * + * `.` is the bug from issue #1013. The remaining characters are reserved on + * common filesystems / URLs and would similarly create downstream surprises + * if they leaked into the display name. + */ +export const DISALLOWED_BOOK_NAME_CHARS: readonly string[] = [ + ".", + "/", + "\\", + ":", + "*", + "?", + '"', + "<", + ">", + "|", +]; + +/** + * Returns the subset of {@link DISALLOWED_BOOK_NAME_CHARS} present in `name`. + * Returns an empty array if the name is valid (or empty). + */ +export function findDisallowedBookNameChars(name: string): string[] { + if (!name) return []; + const seen = new Set(); + for (const ch of name) { + if (DISALLOWED_BOOK_NAME_CHARS.includes(ch)) { + seen.add(ch); + } + } + return Array.from(seen); +} + +/** + * True if the name contains any disallowed character. Whitespace-only / empty + * names are NOT considered "invalid" by this function — those are handled + * separately by callers that already trim and reject empty input. + */ +export function bookNameHasDisallowedChars(name: string): boolean { + return findDisallowedBookNameChars(name).length > 0; +} + +/** + * Human-friendly explanation of why a name is rejected, suitable for showing + * inline beneath an input field. Returns `null` when the name is valid. + */ +export function getBookNameValidationMessage(name: string): string | null { + const offenders = findDisallowedBookNameChars(name); + if (offenders.length === 0) return null; + + const list = offenders.map((c) => `"${c}"`).join(", "); + const noun = offenders.length === 1 ? "character" : "characters"; + return `${list} ${offenders.length === 1 ? "is" : "are"} not allowed in book names. Use dashes (-) or underscores (_) instead.`; +} diff --git a/sharedUtils/index.ts b/sharedUtils/index.ts index 8369f2f51..fba35af20 100644 --- a/sharedUtils/index.ts +++ b/sharedUtils/index.ts @@ -283,3 +283,4 @@ export const deriveTargetPathFromSource = (sourcePath: string): string => { // Re-export corpus utilities export * from "./corpusUtils"; export * from "./exportOptionsEligibility"; +export * from "./bookNameValidation"; diff --git a/src/providers/navigationWebview/navigationWebviewProvider.ts b/src/providers/navigationWebview/navigationWebviewProvider.ts index 17afcf099..076f44664 100644 --- a/src/providers/navigationWebview/navigationWebviewProvider.ts +++ b/src/providers/navigationWebview/navigationWebviewProvider.ts @@ -7,7 +7,7 @@ import { BaseWebviewProvider } from "../../globalProvider"; import { getWebviewHtml } from "../../utils/webviewTemplate"; import { safePostMessageToView } from "../../utils/webviewUtils"; import { CodexItem } from "types"; -import { getCellValueData, cellHasAudioUsingAttachments, computeValidationStats, computeProgressPercents, shouldExcludeCellFromProgress, shouldExcludeQuillCellFromProgress, countActiveValidations, hasTextContent } from "../../../sharedUtils"; +import { getCellValueData, cellHasAudioUsingAttachments, computeValidationStats, computeProgressPercents, shouldExcludeCellFromProgress, shouldExcludeQuillCellFromProgress, countActiveValidations, hasTextContent, getBookNameValidationMessage } from "../../../sharedUtils"; import { normalizeCorpusMarker } from "../../utils/corpusMarkerUtils"; import { addMetadataEdit, addProjectMetadataEdit, EditMapUtils } from "../../utils/editMapUtils"; import { MetadataManager } from "../../utils/metadataManager"; @@ -957,6 +957,21 @@ export class NavigationWebviewProvider extends BaseWebviewProvider { return; } + // Defensive: reject names containing characters that would later be misinterpreted + // by filesystem-style cleanup (see issue #1013) or are otherwise unsafe in paths. + // The webview also blocks these client-side, but we re-validate here in case a + // stale webview, command, or future caller bypasses that check. + const trimmedNewBookName = (newBookName ?? "").trim(); + if (!trimmedNewBookName) { + vscode.window.showErrorMessage("Book name cannot be empty."); + return; + } + const validationError = getBookNameValidationMessage(trimmedNewBookName); + if (validationError) { + vscode.window.showErrorMessage(`Cannot update book name: ${validationError}`); + return; + } + try { // Find codex files matching the book abbreviation and read metadata from first file const { matchingUris, corpusMarker } = await findCodexFilesByBookAbbr(bookAbbr, { readMetadata: true }); @@ -1019,15 +1034,15 @@ export class NavigationWebviewProvider extends BaseWebviewProvider { const oldValue = metadata.fileDisplayName; // Only add edit if value is actually changing - if (oldValue !== newBookName) { + if (oldValue !== trimmedNewBookName) { // Add edit history entry before updating metadata - addMetadataEdit(metadata, "fileDisplayName", newBookName, currentUser); + addMetadataEdit(metadata, "fileDisplayName", trimmedNewBookName, currentUser); } // Update metadata to add fileDisplayName (preserve originalName) notebookData.metadata = { ...metadata, - fileDisplayName: newBookName, + fileDisplayName: trimmedNewBookName, // Preserve originalName if it exists, don't modify it }; @@ -1068,15 +1083,15 @@ export class NavigationWebviewProvider extends BaseWebviewProvider { const sourceOldValue = sourceMetadata.fileDisplayName; // Only add edit if value is actually changing - if (sourceOldValue !== newBookName) { + if (sourceOldValue !== trimmedNewBookName) { // Add edit history entry before updating metadata - addMetadataEdit(sourceMetadata, "fileDisplayName", newBookName, currentUser); + addMetadataEdit(sourceMetadata, "fileDisplayName", trimmedNewBookName, currentUser); } // Update metadata to add fileDisplayName (preserve originalName) sourceNotebookData.metadata = { ...sourceMetadata, - fileDisplayName: newBookName, + fileDisplayName: trimmedNewBookName, // Preserve originalName if it exists, don't modify it }; @@ -1104,11 +1119,11 @@ export class NavigationWebviewProvider extends BaseWebviewProvider { if (updatedCount > 0) { vscode.window.showInformationMessage( - `Book name updated: "${bookAbbr}" → "${newBookName}" (${updatedCount} file(s) updated)` + `Book name updated: "${bookAbbr}" → "${trimmedNewBookName}" (${updatedCount} file(s) updated)` ); } else { vscode.window.showInformationMessage( - `Book name updated: "${bookAbbr}" → "${newBookName}" (no matching codex files found)` + `Book name updated: "${bookAbbr}" → "${trimmedNewBookName}" (no matching codex files found)` ); } } catch (error) { diff --git a/src/test/suite/bookNameValidation.test.ts b/src/test/suite/bookNameValidation.test.ts new file mode 100644 index 000000000..75f261045 --- /dev/null +++ b/src/test/suite/bookNameValidation.test.ts @@ -0,0 +1,78 @@ +import * as assert from "assert"; +import { + DISALLOWED_BOOK_NAME_CHARS, + findDisallowedBookNameChars, + bookNameHasDisallowedChars, + getBookNameValidationMessage, +} from "../../../sharedUtils/bookNameValidation"; + +suite("bookNameValidation Test Suite", () => { + test("DISALLOWED_BOOK_NAME_CHARS includes the period that triggered issue #1013", () => { + assert.ok( + DISALLOWED_BOOK_NAME_CHARS.includes("."), + "'.' must be in the disallowed list (root cause of #1013)" + ); + }); + + test("DISALLOWED_BOOK_NAME_CHARS includes filesystem-unsafe characters", () => { + const expected = ["/", "\\", ":", "*", "?", '"', "<", ">", "|"]; + for (const ch of expected) { + assert.ok( + DISALLOWED_BOOK_NAME_CHARS.includes(ch), + `'${ch}' should be disallowed in book names` + ); + } + }); + + test("findDisallowedBookNameChars returns offenders without duplicates", () => { + assert.deepStrictEqual(findDisallowedBookNameChars("1. New Items"), ["."]); + assert.deepStrictEqual( + findDisallowedBookNameChars("a/b\\c/d"), + ["/", "\\"], + "Repeated offenders should be deduplicated" + ); + assert.deepStrictEqual(findDisallowedBookNameChars("My Book"), []); + assert.deepStrictEqual(findDisallowedBookNameChars(""), []); + }); + + test("bookNameHasDisallowedChars distinguishes valid and invalid names", () => { + assert.strictEqual(bookNameHasDisallowedChars("1. New Items"), true); + assert.strictEqual(bookNameHasDisallowedChars("1.New"), true); + assert.strictEqual(bookNameHasDisallowedChars(".leading"), true); + assert.strictEqual(bookNameHasDisallowedChars("trailing."), true); + + assert.strictEqual(bookNameHasDisallowedChars("1 New Items"), false); + assert.strictEqual(bookNameHasDisallowedChars("1-New-Items"), false); + assert.strictEqual(bookNameHasDisallowedChars("1_New_Items"), false); + assert.strictEqual(bookNameHasDisallowedChars("Genesis"), false); + assert.strictEqual(bookNameHasDisallowedChars(""), false); + }); + + test("getBookNameValidationMessage returns null for valid names", () => { + assert.strictEqual(getBookNameValidationMessage("Genesis"), null); + assert.strictEqual(getBookNameValidationMessage("1-New-Items"), null); + assert.strictEqual(getBookNameValidationMessage("1_New_Items"), null); + assert.strictEqual(getBookNameValidationMessage(""), null); + }); + + test("getBookNameValidationMessage describes the offending characters", () => { + const message = getBookNameValidationMessage("1. New Items"); + assert.ok(message, "Should return a non-null message for invalid names"); + assert.ok(message!.includes('"."'), "Message should reference the period"); + assert.ok( + message!.toLowerCase().includes("dash") || message!.includes("-"), + "Message should suggest dashes as an alternative" + ); + assert.ok( + message!.toLowerCase().includes("underscore") || message!.includes("_"), + "Message should suggest underscores as an alternative" + ); + }); + + test("getBookNameValidationMessage handles multiple offenders", () => { + const message = getBookNameValidationMessage("a/b.c"); + assert.ok(message, "Should return a non-null message for invalid names"); + assert.ok(message!.includes('"/"'), "Message should reference '/'"); + assert.ok(message!.includes('"."'), "Message should reference '.'"); + }); +}); diff --git a/src/test/suite/navigationWebviewProvider.test.ts b/src/test/suite/navigationWebviewProvider.test.ts index 65fb7dec6..0c7406f7b 100644 --- a/src/test/suite/navigationWebviewProvider.test.ts +++ b/src/test/suite/navigationWebviewProvider.test.ts @@ -445,6 +445,95 @@ suite("NavigationWebviewProvider Test Suite", () => { } }); + test("updateBookName rejects names containing periods (issue #1013)", async () => { + if (!vscode.workspace.workspaceFolders?.[0]) { + return; + } + + const codexUri = await createCodexFileWithMetadata("GEN", { + fileDisplayName: "Genesis", + }); + + const loadBibleBookMap = (provider as any).loadBibleBookMap.bind(provider); + loadBibleBookMap(); + + const showErrorMessageStub = sinon.stub(vscode.window, "showErrorMessage"); + const showInformationMessageStub = sinon.stub(vscode.window, "showInformationMessage"); + const withProgressStub = sinon.stub(vscode.window, "withProgress").callsFake(async (options, callback) => { + return callback({ report: () => { } } as any, new vscode.CancellationTokenSource().token); + }); + + try { + const updateBookName = (provider as any).updateBookName.bind(provider); + await updateBookName("GEN", "1. New Items"); + + assert.ok( + showErrorMessageStub.called, + "Should show an error when the new name contains a disallowed character" + ); + + // The original fileDisplayName should be untouched on disk. + const content = await vscode.workspace.fs.readFile(codexUri); + const serializer = new CodexContentSerializer(); + const notebookData = await serializer.deserializeNotebook( + content, + new vscode.CancellationTokenSource().token + ); + assert.strictEqual( + (notebookData.metadata as any).fileDisplayName, + "Genesis", + "fileDisplayName must not be changed when validation fails" + ); + } finally { + showErrorMessageStub.restore(); + showInformationMessageStub.restore(); + withProgressStub.restore(); + } + }); + + test("updateBookName accepts names with dashes and underscores", async () => { + if (!vscode.workspace.workspaceFolders?.[0]) { + return; + } + + const codexUri = await createCodexFileWithMetadata("EXO", {}); + + const loadBibleBookMap = (provider as any).loadBibleBookMap.bind(provider); + loadBibleBookMap(); + + const showErrorMessageStub = sinon.stub(vscode.window, "showErrorMessage"); + const showInformationMessageStub = sinon.stub(vscode.window, "showInformationMessage"); + const withProgressStub = sinon.stub(vscode.window, "withProgress").callsFake(async (options, callback) => { + return callback({ report: () => { } } as any, new vscode.CancellationTokenSource().token); + }); + + try { + const updateBookName = (provider as any).updateBookName.bind(provider); + await updateBookName("EXO", "1-New_Items"); + + assert.ok( + !showErrorMessageStub.called, + "Should not show an error for a valid name with dashes/underscores" + ); + + const content = await vscode.workspace.fs.readFile(codexUri); + const serializer = new CodexContentSerializer(); + const notebookData = await serializer.deserializeNotebook( + content, + new vscode.CancellationTokenSource().token + ); + assert.strictEqual( + (notebookData.metadata as any).fileDisplayName, + "1-New_Items", + "fileDisplayName should be updated to the valid value" + ); + } finally { + showErrorMessageStub.restore(); + showInformationMessageStub.restore(); + withProgressStub.restore(); + } + }); + test("deleteFile closes open webview panels for codex file", async () => { // Skip if no workspace folder if (!vscode.workspace.workspaceFolders?.[0]) { diff --git a/src/test/suite/notebookMetadataManager.test.ts b/src/test/suite/notebookMetadataManager.test.ts index 9340cdd6c..edb084ac2 100644 --- a/src/test/suite/notebookMetadataManager.test.ts +++ b/src/test/suite/notebookMetadataManager.test.ts @@ -678,6 +678,76 @@ suite("NotebookMetadataManager Test Suite", function () { ); }); + test("should preserve fileDisplayName containing periods (issue #1013)", async function () { + this.timeout(5000); + if (!workspaceFolder) { + return; + } + + // Regression test: previously, stripFileExtensionFromDisplayName used path.extname() + // and treated ". New Items" as a strippable extension on every loadMetadata call, + // truncating the user-edited name "1. New Items" to "1" after each sync. + const userEditedName = "1. New Items"; + const codexUri = await createNotebookFile("ACT", false, { + fileDisplayName: userEditedName, + originalName: "Acts", + }); + + await manager.initialize(); + await ensureFileProcessedByLoadMetadata(codexUri); + + // Force a second loadMetadata pass to mimic what happens after a sync + // triggers the file watcher (the original bug only manifested after + // the cleanup pass ran a second time). + await manager.loadMetadata(); + + const serializer = new CodexContentSerializer(); + const content = await vscode.workspace.fs.readFile(codexUri); + const notebookData = await serializer.deserializeNotebook( + content, + new vscode.CancellationTokenSource().token + ); + const metadata = notebookData.metadata as CustomNotebookMetadata; + + assert.strictEqual( + metadata.fileDisplayName, + userEditedName, + "fileDisplayName containing a period should not be truncated" + ); + }); + + test("should still strip known file extensions from fileDisplayName", async function () { + this.timeout(5000); + if (!workspaceFolder) { + return; + } + + // The cleanup is intentionally narrowed to known extensions, but it + // must still kick in for legitimate cases (e.g. an importer accidentally + // wrote "Genesis.codex" into fileDisplayName). + const codexUri = await createNotebookFile("GE2", false, { + fileDisplayName: "Genesis.codex", + originalName: "Genesis", + }); + + await manager.initialize(); + await ensureFileProcessedByLoadMetadata(codexUri); + + const serializer = new CodexContentSerializer(); + const content = await vscode.workspace.fs.readFile(codexUri); + const notebookData = await serializer.deserializeNotebook( + content, + new vscode.CancellationTokenSource().token + ); + const metadata = notebookData.metadata as CustomNotebookMetadata; + + assert.strictEqual( + metadata.fileDisplayName, + "Genesis", + "Known file extensions should be stripped from fileDisplayName" + ); + }); + test("should update both .codex and .source files for same book", async function () { this.timeout(5000); if (!workspaceFolder) { diff --git a/src/utils/notebookMetadataManager.ts b/src/utils/notebookMetadataManager.ts index 790ed5002..f7bb3fbb1 100644 --- a/src/utils/notebookMetadataManager.ts +++ b/src/utils/notebookMetadataManager.ts @@ -33,6 +33,35 @@ interface MetadataValidationResult { errors: string[]; } +/** + * File extensions that may legitimately appear at the tail of a fileDisplayName + * (e.g. when the display name was originally derived from a source filename). + * These are stripped on load so the UI shows a clean name. Anything else after + * a `.` is treated as part of the user's chosen name and preserved as-is. + */ +const KNOWN_DISPLAY_NAME_EXTENSIONS: readonly string[] = [ + ".codex", + ".source", + ".scripture", + ".bible", + ".usfm", + ".sfm", + ".vtt", + ".srt", + ".mp4", + ".mp3", + ".wav", + ".m4a", + ".ogg", + ".webm", + ".txt", + ".md", + ".json", + ".csv", + ".tsv", + ".pdf", +]; + export function getNotebookMetadataManager(): NotebookMetadataManager { return NotebookMetadataManager.getManager(); } @@ -577,10 +606,16 @@ export class NotebookMetadataManager { } /** - * Strips file extensions from a display name if detected. - * Common extensions include: .vtt, .mp4, .mp3, .wav, .srt, .codex, .source, etc. + * Strips a known file extension from the end of a display name. + * + * Only the extensions in {@link KNOWN_DISPLAY_NAME_EXTENSIONS} are stripped. + * Using `path.extname` here would treat any text after the last `.` as an + * extension (e.g. `"1. New Items"` → strip `". New Items"` → `"1"`), which + * silently corrupts user-edited book names on every metadata reload after + * sync. See issue #1013. + * * @param displayName - The display name to clean - * @returns The display name without file extension + * @returns The display name without a known file extension */ private stripFileExtensionFromDisplayName(displayName: string): string { const trimmed = displayName.trim(); @@ -588,11 +623,11 @@ export class NotebookMetadataManager { return trimmed; } - // Check if the display name has a file extension - const ext = path.extname(trimmed); - if (ext && ext.length > 0) { - // Strip the extension using path.basename - return path.basename(trimmed, ext); + const lower = trimmed.toLowerCase(); + for (const ext of KNOWN_DISPLAY_NAME_EXTENSIONS) { + if (lower.endsWith(ext) && trimmed.length > ext.length) { + return trimmed.slice(0, -ext.length); + } } return trimmed; diff --git a/webviews/codex-webviews/src/NavigationView/index.tsx b/webviews/codex-webviews/src/NavigationView/index.tsx index d5b28112a..c103f6db9 100644 --- a/webviews/codex-webviews/src/NavigationView/index.tsx +++ b/webviews/codex-webviews/src/NavigationView/index.tsx @@ -16,6 +16,7 @@ import { Languages, Mic, ShieldCheck } from "lucide-react"; import { Switch } from "../components/ui/switch"; import { Label } from "../components/ui/label"; import { RenameModal } from "../components/RenameModal"; +import { getBookNameValidationMessage } from "@sharedUtils"; import { Dialog, DialogContent, @@ -657,16 +658,22 @@ function NavigationView() { const handleBookNameModalConfirm = () => { const { item, newName } = state.bookNameModal; - if (item && newName.trim() !== "") { + const trimmed = newName.trim(); + if (item && trimmed !== "") { + // Defensive: refuse to send an invalid name. The modal disables Save + // when this returns non-null, so users normally shouldn't reach here. + if (getBookNameValidationMessage(trimmed)) { + return; + } // Use fileDisplayName from metadata if available, otherwise fall back to formatted label const currentDisplayName = item.fileDisplayName || formatLabel(item.label, state.bibleBookMap || new Map()); - if (newName.trim() !== currentDisplayName) { + if (trimmed !== currentDisplayName) { vscode.postMessage({ command: "editBookName", content: { bookAbbr: item.label, - newBookName: newName.trim(), + newBookName: trimmed, }, }); } @@ -1106,6 +1113,7 @@ function NavigationView() { placeholder="Enter new book name" confirmButtonLabel="Save" disabled={disableBookNameButton} + validate={getBookNameValidationMessage} onClose={handleBookNameModalClose} onConfirm={handleBookNameModalConfirm} onValueChange={handleBookNameModalInputChange} diff --git a/webviews/codex-webviews/src/components/RenameModal.tsx b/webviews/codex-webviews/src/components/RenameModal.tsx index e281f910f..8af510357 100644 --- a/webviews/codex-webviews/src/components/RenameModal.tsx +++ b/webviews/codex-webviews/src/components/RenameModal.tsx @@ -19,6 +19,12 @@ interface RenameModalProps { placeholder?: string; confirmButtonLabel?: string; disabled?: boolean; + /** + * Optional inline validation. Returning a non-null string will display the + * message under the input and disable the confirm button. The component + * still respects `disabled` for non-validation reasons (e.g. empty input). + */ + validate?: (value: string) => string | null; onClose: () => void; onConfirm: () => void; onValueChange: (value: string) => void; @@ -33,10 +39,14 @@ export const RenameModal: React.FC = ({ placeholder, confirmButtonLabel = "Save", disabled = false, + validate, onClose, onConfirm, onValueChange, }) => { + const validationMessage = validate ? validate(value) : null; + const isInvalid = validationMessage !== null; + const isConfirmDisabled = disabled || isInvalid; const inputRef = useRef(null); // Focus input when modal opens @@ -53,7 +63,7 @@ export const RenameModal: React.FC = ({ const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === "Enter") { e.preventDefault(); - if (!disabled) { + if (!isConfirmDisabled) { onConfirm(); } } else if (e.key === "Escape") { @@ -104,22 +114,40 @@ export const RenameModal: React.FC = ({ onChange={(e) => onValueChange(e.target.value)} onKeyDown={handleKeyDown} placeholder={placeholder} - className="w-full mb-5 bg-vscode-input-background placeholder:text-gray-500 text-vscode-input-foreground border-vscode-input-border" + aria-invalid={isInvalid} + aria-describedby={isInvalid ? "rename-modal-error" : undefined} + className="w-full bg-vscode-input-background placeholder:text-gray-500 text-vscode-input-foreground border-vscode-input-border" style={{ padding: "8px", fontSize: "14px", backgroundColor: "var(--vscode-input-background)", color: "var(--vscode-input-foreground)", - borderColor: "var(--vscode-input-border)", + borderColor: isInvalid + ? "var(--vscode-inputValidation-errorBorder, var(--vscode-errorForeground))" + : "var(--vscode-input-border)", borderRadius: "6px", - marginBottom: "20px", + marginBottom: isInvalid ? "8px" : "20px", }} /> + {isInvalid && ( + + )} - diff --git a/webviews/codex-webviews/src/components/__tests__/RenameModal.test.tsx b/webviews/codex-webviews/src/components/__tests__/RenameModal.test.tsx new file mode 100644 index 000000000..09d3a2330 --- /dev/null +++ b/webviews/codex-webviews/src/components/__tests__/RenameModal.test.tsx @@ -0,0 +1,92 @@ +import React from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import "@testing-library/jest-dom"; +import { RenameModal } from "../RenameModal"; +import { getBookNameValidationMessage } from "@sharedUtils"; + +describe("RenameModal validation", () => { + const onClose = vi.fn(); + const onConfirm = vi.fn(); + const onValueChange = vi.fn(); + + beforeEach(() => { + onClose.mockReset(); + onConfirm.mockReset(); + onValueChange.mockReset(); + }); + + const renderModal = (value: string) => + render( + + ); + + it("does not show a warning for a valid name", () => { + renderModal("1-New-Items"); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("shows a warning when the value contains a period", () => { + renderModal("1. New Items"); + const alert = screen.getByRole("alert"); + expect(alert).toBeInTheDocument(); + expect(alert.textContent).toContain('"."'); + expect(alert.textContent?.toLowerCase()).toContain("not allowed"); + }); + + it("disables the Save button when the value is invalid", () => { + renderModal("1. New Items"); + const saveButton = screen.getByRole("button", { name: /save/i }); + expect(saveButton).toBeDisabled(); + }); + + it("re-enables Save once the period is removed", () => { + const { rerender } = renderModal("1. New Items"); + expect(screen.getByRole("button", { name: /save/i })).toBeDisabled(); + + rerender( + + ); + + expect(screen.queryByRole("alert")).toBeNull(); + expect(screen.getByRole("button", { name: /save/i })).not.toBeDisabled(); + }); + + it("ignores Enter when the value is invalid", () => { + renderModal("1. New Items"); + const input = screen.getByPlaceholderText("Enter new book name"); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onConfirm).not.toHaveBeenCalled(); + }); + + it("invokes onConfirm on Enter when the value is valid", () => { + renderModal("1-New-Items"); + const input = screen.getByPlaceholderText("Enter new book name"); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onConfirm).toHaveBeenCalledTimes(1); + }); +});