diff --git a/packages/visual-editor/src/cli/commands/internal/deploy/api.ts b/packages/visual-editor/src/cli/commands/internal/deploy/api.ts index dde22ac4d..ee57e17a1 100644 --- a/packages/visual-editor/src/cli/commands/internal/deploy/api.ts +++ b/packages/visual-editor/src/cli/commands/internal/deploy/api.ts @@ -30,7 +30,8 @@ export async function yextApiRequest( path: string, config: DeployConfig, verbose: boolean = false, - data?: object + data?: object, + expectedStatusMessages: Partial> = {} ): Promise { const url = new URL(`${API_PATH_PREFIX}${path}`, config.apiHost); url.searchParams.set("v", "20260819"); @@ -44,7 +45,13 @@ export async function yextApiRequest( requestInit.body = JSON.stringify(data); } - const finishLog = logApiCall(logAction, method, url, verbose); + const finishLog = logApiCall( + logAction, + method, + url, + verbose, + expectedStatusMessages + ); try { const response = await fetch(url, new Request(url, requestInit)); diff --git a/packages/visual-editor/src/cli/commands/internal/deploy/deploy.test.ts b/packages/visual-editor/src/cli/commands/internal/deploy/deploy.test.ts index faa4861d9..035cc5b5e 100644 --- a/packages/visual-editor/src/cli/commands/internal/deploy/deploy.test.ts +++ b/packages/visual-editor/src/cli/commands/internal/deploy/deploy.test.ts @@ -4,11 +4,25 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { execFileSync } from "node:child_process"; import prompts from "prompts"; +import ora from "ora"; import { deploy } from "./deploy.ts"; import type { DeployConfig } from "./config.ts"; import type { SectionLibraryRevision } from "./sectionLibraryApi.ts"; vi.mock("prompts"); +vi.mock("ora", () => ({ + default: vi.fn((options) => { + const spinner = { + text: typeof options === "string" ? options : options.text, + start: vi.fn(), + succeed: vi.fn(), + fail: vi.fn(), + info: vi.fn(), + }; + spinner.start.mockReturnValue(spinner); + return spinner; + }), +})); const config: DeployConfig = { accountId: "123", @@ -38,6 +52,7 @@ let sourceCommitHash: string; beforeEach(() => { vi.mocked(prompts).mockReset(); + vi.mocked(ora).mockClear(); vi.spyOn(console, "log").mockImplementation(() => {}); vi.spyOn(console, "warn").mockImplementation(() => {}); rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "deploy-templates-test-")); @@ -292,6 +307,68 @@ describe("deploy", () => { expect(error).not.toHaveBeenCalled(); }); + it.each([ + { + name: "built-in library rejection", + message: + "Provided argument 'section_library_revision' is invalid: cannot create revision for built-in section library", + expectedMessage: + 'Cannot create a revision for a built-in section library. Check the "id" field in src/library/library.json. To deploy your own library, remove the reserved "yext_" prefix and retry.', + verbose: false, + }, + { + name: "built-in library rejection in verbose mode", + message: + "Provided argument 'section_library_revision' is invalid: cannot create revision for built-in section library", + expectedMessage: + 'Cannot create a revision for a built-in section library. Check the "id" field in src/library/library.json. To deploy your own library, remove the reserved "yext_" prefix and retry.', + verbose: true, + }, + { + name: "unrelated rejection with the same error code", + message: "The exact API error message.", + expectedMessage: "The exact API error message.", + verbose: false, + }, + ])("handles $name during revision creation", async (testCase) => { + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}); + const apiError = { + code: 104001, + type: "BAD_REQUEST", + message: testCase.message, + name: "invalidRequest", + }; + vi.stubGlobal( + "fetch", + vi + .fn() + .mockResolvedValueOnce( + new Response(successfulResponse(sectionLibrary), { status: 200 }) + ) + .mockResolvedValueOnce( + new Response(successfulResponse({ sectionLibraryRevisions: [] }), { + status: 200, + }) + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ meta: { errors: [apiError] } }), { + status: 400, + }) + ) + ); + + await expect(deploy(config, testCase.verbose)).rejects.toThrow( + testCase.expectedMessage + ); + if (testCase.verbose) { + expect(errorLog).toHaveBeenCalledWith( + `[debug] API Errors: ${JSON.stringify([apiError])}` + ); + } else { + expect(errorLog).not.toHaveBeenCalled(); + } + }); + it("explains how to resolve missing section library write permissions", async () => { vi.stubGlobal( "fetch", @@ -354,6 +431,11 @@ describe("deploy", () => { await expect(deploy(config)).rejects.toThrow( /Yext API key does not have required permissions\./ ); + const spinner = vi.mocked(ora).mock.results[0].value; + expect(spinner.fail).toHaveBeenCalledWith( + "Fetching Section Library... error" + ); + expect(spinner.info).not.toHaveBeenCalled(); }); it("uses the API message for error code 104001", async () => { @@ -477,6 +559,9 @@ describe("deploy", () => { await deploy(config, false, { isInteractive: true }); + const spinner = vi.mocked(ora).mock.results[0].value; + expect(spinner.info).toHaveBeenCalledWith("Section library not found"); + expect(spinner.fail).not.toHaveBeenCalled(); expect(fetchMock).toHaveBeenNthCalledWith( 2, new URL( @@ -507,6 +592,9 @@ describe("deploy", () => { deploy(config, false, { isInteractive: false }) ).rejects.toThrow(/Section library "library\/123" does not exist/); + const spinner = vi.mocked(ora).mock.results[0].value; + expect(spinner.info).toHaveBeenCalledWith("Section library not found"); + expect(spinner.fail).not.toHaveBeenCalled(); expect(fetchMock).toHaveBeenCalledTimes(1); expect(prompts).not.toHaveBeenCalled(); }); diff --git a/packages/visual-editor/src/cli/commands/internal/deploy/logging.ts b/packages/visual-editor/src/cli/commands/internal/deploy/logging.ts index 299c29189..c0c3c2e58 100644 --- a/packages/visual-editor/src/cli/commands/internal/deploy/logging.ts +++ b/packages/visual-editor/src/cli/commands/internal/deploy/logging.ts @@ -5,14 +5,18 @@ export function logApiCall( text: string, method: string, url: URL, - verbose: boolean = false + verbose: boolean = false, + expectedStatusMessages: Partial> = {} ): (res?: YextApiResponse) => void { const spinner = text.length > 0 ? ora(text).start() : undefined; const finishVerboseLog = verbose ? verboseLogApiCall(method, url) : undefined; return (res?: YextApiResponse) => { if (spinner) { - if (res?.ok) { + const expectedMessage = res && expectedStatusMessages[res.status]; + if (expectedMessage) { + spinner.info(expectedMessage); + } else if (res?.ok) { spinner.succeed(`${text} ${color("done", true)}`); } else { spinner.fail(`${text} ${color("error", false)}`); diff --git a/packages/visual-editor/src/cli/commands/internal/deploy/sectionLibraryApi.ts b/packages/visual-editor/src/cli/commands/internal/deploy/sectionLibraryApi.ts index 0687b22a4..ecc0c9814 100644 --- a/packages/visual-editor/src/cli/commands/internal/deploy/sectionLibraryApi.ts +++ b/packages/visual-editor/src/cli/commands/internal/deploy/sectionLibraryApi.ts @@ -93,6 +93,15 @@ export async function createSectionLibraryRevision( ); if (!result.ok) { + if ( + result.errors[0]?.message.includes( + "cannot create revision for built-in section library" + ) + ) { + throw new Error( + 'Cannot create a revision for a built-in section library. Check the "id" field in src/library/library.json. To deploy your own library, remove the reserved "yext_" prefix and retry.' + ); + } throw sectionLibraryApiError( result.errors, "Failed to upload current commit as a Section Library Revision" @@ -170,7 +179,9 @@ export async function getSectionLibrary( "GET", `accounts/me/sectionLibraries/${encodeURIComponent(libraryId)}`, config, - verbose + verbose, + undefined, + { 404: "Section library not found" } ); if (!result.ok && result.status !== 404) { diff --git a/packages/visual-editor/src/cli/yextve.test.ts b/packages/visual-editor/src/cli/yextve.test.ts index 7ef88bc3d..c049bd277 100644 --- a/packages/visual-editor/src/cli/yextve.test.ts +++ b/packages/visual-editor/src/cli/yextve.test.ts @@ -300,6 +300,27 @@ describe("yextve", () => { }); }); + it("rejects a library id containing an underscore during validation", async () => { + const rootDir = createTempRoot(); + fs.outputJsonSync(path.join(rootDir, "src", "library", "library.json"), { + schemaVersion: 1, + id: "foo_test", + displayName: "Library", + }); + + const result = await invoke( + ["validate", "--skip-repo-structure-check", "--skip-code-check"], + rootDir + ); + + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain("src/library/library.json"); + expect(result.stdout).toContain( + "id must be 2–63 characters, contain only lowercase letters, numbers, and hyphens, start with a lowercase letter, and end with a letter or number." + ); + expect(result.stdout).toContain("Validation failed. 1 error."); + }); + it("renders all skipped stages and succeeds", async () => { const result = await invoke([ "validate", diff --git a/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.test.ts b/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.test.ts index 2bd3f96b0..c665ac119 100644 --- a/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.test.ts +++ b/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.test.ts @@ -71,15 +71,43 @@ describe("validateLibraryMetadata", () => { ); }); - it("reports an unsafe id", () => { + it.each([ + "unsafe id", + "foo_test", + "safe-id\n", + "Safe-id", + "safe-Id", + "1safe-id", + "-safe-id", + "safe-id-", + "a", + "a".repeat(64), + ])("reports an unsafe id %s", (id) => { const rootDir = createTempRoot(); - writeLibraryJson(rootDir, validMetadata({ id: "unsafe id" })); + writeLibraryJson(rootDir, validMetadata({ id })); expect(validateLibraryMetadata(rootDir).issues).toContainEqual( - expect.objectContaining({ rule: "field/id/safe" }) + expect.objectContaining({ + rule: "field/id/safe", + message: + "id must be 2–63 characters, contain only lowercase letters, numbers, and hyphens, start with a lowercase letter, and end with a letter or number.", + }) ); }); + it.each(["ab", "a0", "a-b", "a".repeat(63)])( + "accepts a valid id %s", + (id) => { + const rootDir = createTempRoot(); + writeLibraryJson(rootDir, validMetadata({ id })); + + expect(validateLibraryMetadata(rootDir)).toEqual({ + issues: [], + metadata: validMetadata({ id }), + }); + } + ); + it("aggregates independent field errors", () => { const rootDir = createTempRoot(); writeLibraryJson(rootDir, { diff --git a/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.ts b/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.ts index 7e90e7ce1..2e2fdb800 100644 --- a/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.ts +++ b/packages/visual-editor/src/internal/sectionLibraryValidation/stages/api/libraryMetadata.ts @@ -3,7 +3,8 @@ import fs from "fs-extra"; import type { LibraryMetadata } from "../../../../types/sectionLibrary.ts"; import type { ValidationIssue } from "../../types.ts"; -const safeIdPattern = /^[A-Za-z0-9_-]{1,64}$/; +// Require the actual end of input; JavaScript $ also matches before a trailing newline. +const safeIdPattern = /^[a-z][a-z0-9-]{0,61}[a-z0-9](?![\s\S])/; const descriptionMaxLength = 1024; /** validateLibraryMetadata validates that the library.json has the required fields. */ @@ -82,7 +83,7 @@ export const validateLibraryMetadata = ( if (id && !safeIdPattern.test(id)) { addIssue( "field/id/safe", - "id must be at most 64 characters and may contain only letters, numbers, underscores, and hyphens." + "id must be 2–63 characters, contain only lowercase letters, numbers, and hyphens, start with a lowercase letter, and end with a letter or number." ); }