diff --git a/packages/zcli-themes/src/commands/themes/migrate.ts b/packages/zcli-themes/src/commands/themes/migrate.ts new file mode 100644 index 00000000..6777eca2 --- /dev/null +++ b/packages/zcli-themes/src/commands/themes/migrate.ts @@ -0,0 +1,26 @@ +import { Command } from '@oclif/core' +import * as path from 'path' +import migrate from '../../lib/migrate' + +export default class Migrate extends Command { + static description = 'migrate theme to the latest version of the templating api' + + static hidden = true + + static args = [ + { name: 'themeDirectory', required: true, default: '.' } + ] + + static examples = [ + '$ zcli themes:migrate ./copenhagen_theme' + ] + + static strict = false + + async run () { + const { flags, argv: [themeDirectory] } = await this.parse(Migrate) + const themePath = path.resolve(themeDirectory) + + await migrate(themePath, flags) + } +} diff --git a/packages/zcli-themes/src/lib/handleTemplateError.test.ts b/packages/zcli-themes/src/lib/handleTemplateError.test.ts new file mode 100644 index 00000000..f80ba76e --- /dev/null +++ b/packages/zcli-themes/src/lib/handleTemplateError.test.ts @@ -0,0 +1,104 @@ +import * as sinon from 'sinon' +import { expect } from '@oclif/test' +import * as validationErrorsToString from './validationErrorsToString' +import * as errors from '@oclif/core/lib/errors' +import handleTemplateError from './handleTemplateError' + +describe('handleTemplateError', () => { + beforeEach(() => { + sinon.restore() + }) + + it('transforms template identifiers to template paths and calls error', () => { + const validationErrorsToStringStub = sinon.stub(validationErrorsToString, 'default').returns('formatted errors') + const errorStub = sinon.stub(errors, 'error') + + const templateErrors = { + home_page: [ + { + description: 'not possible to access `names`', + line: 1, + column: 45, + length: 5 + } + ], + article_page: [ + { + description: "'articles' does not exist", + line: 21, + column: 16, + length: 11 + } + ] + } + + handleTemplateError('theme/path', templateErrors) + + expect(validationErrorsToStringStub.calledOnce).to.equal(true) + expect(validationErrorsToStringStub.firstCall.args[0]).to.equal('theme/path') + + const transformedErrors = validationErrorsToStringStub.firstCall.args[1] + expect(transformedErrors).to.have.property('templates/home_page.hbs') + expect(transformedErrors).to.have.property('templates/article_page.hbs') + expect(transformedErrors['templates/home_page.hbs']).to.deep.equal([ + { + description: 'not possible to access `names`', + line: 1, + column: 45, + length: 5 + } + ]) + + expect(errorStub.calledOnce).to.equal(true) + expect(errorStub.firstCall.args[0]).to.contain('InvalidTemplates') + expect(errorStub.firstCall.args[0]).to.contain('Template(s) with syntax error(s)') + expect(errorStub.firstCall.args[0]).to.contain('formatted errors') + }) + + it('handles empty template errors', () => { + const validationErrorsToStringStub = sinon.stub(validationErrorsToString, 'default').returns('') + const errorStub = sinon.stub(errors, 'error') + + handleTemplateError('theme/path', {}) + + expect(validationErrorsToStringStub.calledOnce).to.equal(true) + expect(validationErrorsToStringStub.firstCall.args[1]).to.deep.equal({}) + expect(errorStub.calledOnce).to.equal(true) + }) + + it('handles single template with multiple errors', () => { + const validationErrorsToStringStub = sinon.stub(validationErrorsToString, 'default').returns('multiple errors') + sinon.stub(errors, 'error') + + const templateErrors = { + new_request_page: [ + { + description: 'First error', + line: 1, + column: 1, + length: 5 + }, + { + description: 'Second error', + line: 10, + column: 5, + length: 3 + }, + { + description: 'Third error', + line: 20, + column: 10, + length: 7 + } + ] + } + + handleTemplateError('/my/theme', templateErrors) + + const transformedErrors = validationErrorsToStringStub.firstCall.args[1] + expect(transformedErrors['templates/new_request_page.hbs']).to.have.length(3) + expect(transformedErrors['templates/new_request_page.hbs'][0].description).to.equal('First error') + expect(transformedErrors['templates/new_request_page.hbs'][1].description).to.equal('Second error') + expect(transformedErrors['templates/new_request_page.hbs'][2].description).to.equal('Third error') + }) +}) diff --git a/packages/zcli-themes/src/lib/handleTemplateError.ts b/packages/zcli-themes/src/lib/handleTemplateError.ts new file mode 100644 index 00000000..bf000dbe --- /dev/null +++ b/packages/zcli-themes/src/lib/handleTemplateError.ts @@ -0,0 +1,19 @@ +import { ValidationError, ValidationErrors } from '../types' +import validationErrorsToString from './validationErrorsToString' +import * as chalk from 'chalk' +import { error } from '@oclif/core/lib/errors' + +export default function handleTemplateError (themePath: string, templateErrors: ValidationErrors) { + const validationErrors: ValidationErrors = {} + for (const [template, errors] of Object.entries(templateErrors)) { + // the theming endpoints return the template identifier as the 'key' instead of + // the template path. We must fix this so we can reuse `validationErrorsToString` + // and align with the job import error handling + validationErrors[`templates/${template}.hbs`] = errors as ValidationError[] + } + + const title = `${chalk.bold('InvalidTemplates')} - Template(s) with syntax error(s)` + const details = validationErrorsToString(themePath, validationErrors) + + error(`${title}\n${details}`) +} diff --git a/packages/zcli-themes/src/lib/handleThemeApiError.ts b/packages/zcli-themes/src/lib/handleThemeApiError.ts index 172aa80a..ef009220 100644 --- a/packages/zcli-themes/src/lib/handleThemeApiError.ts +++ b/packages/zcli-themes/src/lib/handleThemeApiError.ts @@ -1,14 +1,18 @@ import type { AxiosError, AxiosResponse } from 'axios' import * as chalk from 'chalk' import { error } from '@oclif/core/lib/errors' +import parseAxiosError from './parseAxiosError' export default function handleThemeApiError (e: AxiosError): never { - const { response, message } = e + const { message, response } = parseAxiosError(e) if (response) { - const { errors } = (response as AxiosResponse).data - for (const { code, title } of errors) { - error(`${chalk.bold(code)} - ${title}`) + const data = (response as AxiosResponse).data + + if (data && typeof data === 'object' && 'errors' in data && Array.isArray(data.errors)) { + for (const { code, title } of data.errors) { + error(`${chalk.bold(code)} - ${title}`) + } } } else if (message) { error(message) diff --git a/packages/zcli-themes/src/lib/migrate.test.ts b/packages/zcli-themes/src/lib/migrate.test.ts new file mode 100644 index 00000000..8bd4d01a --- /dev/null +++ b/packages/zcli-themes/src/lib/migrate.test.ts @@ -0,0 +1,249 @@ +import * as sinon from 'sinon' +import { expect } from '@oclif/test' +import * as getManifest from './getManifest' +import * as getTemplates from './getTemplates' +import * as getVariables from './getVariables' +import * as getAssets from './getAssets' +import * as rewriteManifest from './rewriteManifest' +import * as rewriteTemplates from './rewriteTemplates' +import * as axios from 'axios' +import { request } from '@zendesk/zcli-core' +import * as errors from '@oclif/core/lib/errors' +import migrate from './migrate' + +const manifest = { + name: 'Copenhagen theme', + author: 'Jane Doe', + version: '1.0.1', + api_version: 1, + settings: [ + { + variables: [ + { identifier: 'color', type: 'color', value: '#999' }, + { identifier: 'logo', type: 'file' } + ] + } + ] +} + +const flags = { + bind: 'localhost', + port: 1000, + logs: true, + livereload: false +} + +describe('migrate', () => { + beforeEach(() => { + sinon.restore() + }) + + it('calls the migrations endpoint with the correct payload and rewrites files', async () => { + const getManifestStub = sinon.stub(getManifest, 'default') + const getTemplatesStub = sinon.stub(getTemplates, 'default') + const getVariablesStub = sinon.stub(getVariables, 'default') + const getAssetsStub = sinon.stub(getAssets, 'default') + const rewriteManifestStub = sinon.stub(rewriteManifest, 'default') + const rewriteTemplatesStub = sinon.stub(rewriteTemplates, 'default') + const requestStub = sinon.stub(request, 'requestAPI') + + getManifestStub.withArgs('theme/path').returns(manifest) + getTemplatesStub.withArgs('theme/path').returns({ + home_page: '

Home

', + 'article_pages/product_updates': '

Product updates

', + 'custom_pages/faq': '

FAQ

' + }) + + getVariablesStub.withArgs('theme/path', manifest.settings, flags).returns([ + { identifier: 'color', type: 'color', value: '#999' }, + { + identifier: 'logo', + type: 'file', + value: 'http://localhost:1000/guide/settings/logo.png' + } + ]) + + getAssetsStub.withArgs('theme/path', flags).returns([ + [ + { + base: 'background.png', + dir: '', + ext: '.png', + name: 'background', + root: '' + }, + 'http://localhost:1000/guide/assets/background.png' + ] + ]) + + requestStub.returns( + Promise.resolve({ + status: 200, + statusText: 'OK', + data: { + metadata: { + api_version: 2 + }, + templates: { + home_page: '

Updated Home

', + 'article_pages/product_updates': '

Updated Product updates

', + 'custom_pages/faq': '

Updated FAQ

' + } + } + }) as axios.AxiosPromise + ) + + await migrate('theme/path', flags) + + expect( + requestStub.calledWith( + '/hc/api/internal/theming/migrations', + sinon.match({ + method: 'POST', + headers: { + 'X-Zendesk-Request-Originator': 'zcli themes:migrate' + }, + data: { + templates: { + home_page: '

Home

', + 'article_pages/product_updates': '

Product updates

', + 'custom_pages/faq': '

FAQ

', + assets: { + 'background.png': + 'http://localhost:1000/guide/assets/background.png' + }, + variables: { + color: '#999', + logo: 'http://localhost:1000/guide/settings/logo.png' + }, + metadata: { api_version: 1 } + } + } + }) + ) + ).to.equal(true) + + expect(rewriteManifestStub.calledWith('theme/path', 2)).to.equal(true) + expect( + rewriteTemplatesStub.calledWith('theme/path', { + home_page: '

Updated Home

', + 'article_pages/product_updates': '

Updated Product updates

', + 'custom_pages/faq': '

Updated FAQ

' + }) + ).to.equal(true) + }) + + it('throws an error when validation fails with template errors', async () => { + sinon.stub(getManifest, 'default').returns(manifest) + sinon.stub(getTemplates, 'default').returns({}) + sinon.stub(getVariables, 'default').returns([]) + sinon.stub(getAssets, 'default').returns([]) + + sinon.stub(request, 'requestAPI').throws({ + response: { + data: { + template_errors: { + home_page: [ + { + description: "'articles' does not exist", + line: 10, + column: 6, + length: 7 + } + ], + footer: [ + { + description: "'alternative_locales' does not exist", + line: 6, + column: 12, + length: 10 + } + ] + } + } + } + }) + + const errorStub = sinon.stub(errors, 'error').callThrough() + + try { + await migrate('theme/path', flags) + } catch { + const [call] = errorStub.getCalls() + const [error] = call.args + expect(error).to.contain('theme/path/templates/home_page.hbs:10:6') + expect(error).to.contain("'articles' does not exist") + expect(error).to.contain('theme/path/templates/footer.hbs:6:12') + expect(error).to.contain("'alternative_locales' does not exist") + } + }) + + it('throws an error when there is a general error', async () => { + sinon.stub(getManifest, 'default').returns(manifest) + sinon.stub(getTemplates, 'default').returns({}) + sinon.stub(getVariables, 'default').returns([]) + sinon.stub(getAssets, 'default').returns([]) + + sinon.stub(request, 'requestAPI').throws({ + response: { + data: { + general_error: 'Something went wrong' + } + } + }) + + const errorStub = sinon.stub(errors, 'error').callThrough() + + try { + await migrate('theme/path', flags) + } catch { + const [call] = errorStub.getCalls() + const [error] = call.args + expect(error).to.equal('Something went wrong') + } + }) + + it('throws an error when there is a response with a message', async () => { + sinon.stub(getManifest, 'default').returns(manifest) + sinon.stub(getTemplates, 'default').returns({}) + sinon.stub(getVariables, 'default').returns([]) + sinon.stub(getAssets, 'default').returns([]) + + sinon.stub(request, 'requestAPI').throws({ + response: { + data: {} + }, + message: 'Network error' + }) + + const errorStub = sinon.stub(errors, 'error').callThrough() + + try { + await migrate('theme/path', flags) + } catch { + const [call] = errorStub.getCalls() + const [error] = call.args + expect(error).to.equal('Network error') + } + }) + + it('throws an error when there is no response', async () => { + sinon.stub(getManifest, 'default').returns(manifest) + sinon.stub(getTemplates, 'default').returns({}) + sinon.stub(getVariables, 'default').returns([]) + sinon.stub(getAssets, 'default').returns([]) + + const axiosError = new Error('Connection refused') + sinon.stub(request, 'requestAPI').throws(axiosError) + + const errorStub = sinon.stub(errors, 'error').callThrough() + + try { + await migrate('theme/path', flags) + } catch { + const [call] = errorStub.getCalls() + const [error] = call.args + expect(error).to.equal(axiosError) + } + }) +}) diff --git a/packages/zcli-themes/src/lib/migrate.ts b/packages/zcli-themes/src/lib/migrate.ts new file mode 100644 index 00000000..f8d9db2b --- /dev/null +++ b/packages/zcli-themes/src/lib/migrate.ts @@ -0,0 +1,71 @@ +import type { Flags, ValidationErrors } from '../types' +import getManifest from './getManifest' +import getTemplates from './getTemplates' +import getVariables from './getVariables' +import getAssets from './getAssets' +import * as chalk from 'chalk' +import { request } from '@zendesk/zcli-core' +import { error } from '@oclif/core/lib/errors' +import { CliUx } from '@oclif/core' +import type { AxiosError } from 'axios' +import rewriteTemplates from './rewriteTemplates' +import rewriteManifest from './rewriteManifest' +import handleTemplateError from './handleTemplateError' +import parseAxiosError from './parseAxiosError' + +export default async function migrate (themePath: string, flags: Flags): Promise { + const manifest = getManifest(themePath) + const templates = getTemplates(themePath) + const variables = getVariables(themePath, manifest.settings, flags) + const assets = getAssets(themePath, flags) + + const variablesPayload = variables.reduce((payload, variable) => ({ + ...payload, + [variable.identifier]: variable.value + }), {}) + + const assetsPayload = assets.reduce((payload, [parsedPath, url]) => ({ + ...payload, + [parsedPath.base]: url + }), {}) + + const metadataPayload = { api_version: manifest.api_version } + + try { + CliUx.ux.action.start('Migrating theme') + const { data } = await request.requestAPI('/hc/api/internal/theming/migrations', { + method: 'POST', + headers: { + 'X-Zendesk-Request-Originator': 'zcli themes:migrate' + }, + data: { + templates: { + ...templates, + assets: assetsPayload, + variables: variablesPayload, + metadata: metadataPayload + } + }, + validateStatus: (status: number) => status === 200 + }) + rewriteManifest(themePath, data.metadata.api_version) + rewriteTemplates(themePath, data.templates) + CliUx.ux.action.stop('Ok') + } catch (e) { + CliUx.ux.action.stop(chalk.bold.red('!')) + const { message, response } = parseAxiosError(e as AxiosError) + + if (response) { + const { template_errors: templateErrors, general_error: generalError } = + response.data as { + template_errors: ValidationErrors; + general_error: string; + } + if (templateErrors) handleTemplateError(themePath, templateErrors) + else if (generalError) error(generalError) + else error(message) + } else { + error(e as AxiosError) + } + } +} diff --git a/packages/zcli-themes/src/lib/parseAxiosError.test.ts b/packages/zcli-themes/src/lib/parseAxiosError.test.ts new file mode 100644 index 00000000..c2393c65 --- /dev/null +++ b/packages/zcli-themes/src/lib/parseAxiosError.test.ts @@ -0,0 +1,246 @@ +import { expect } from '@oclif/test' +import parseAxiosError from './parseAxiosError' +import type { AxiosError, AxiosResponse, InternalAxiosRequestConfig } from 'axios' + +describe('parseAxiosError', () => { + it('extracts message and response from a standard AxiosError', () => { + const mockResponse: AxiosResponse = { + status: 400, + statusText: 'Bad Request', + data: { errors: [{ code: 'TestError', title: 'Test error' }] }, + headers: {}, + config: {} as InternalAxiosRequestConfig + } + + const error: AxiosError = { + message: 'Request failed with status code 400', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: mockResponse, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.message).to.equal('Request failed with status code 400') + expect(result.response).to.not.equal(undefined) + expect(result.response?.status).to.equal(400) + expect(result.response?.data).to.deep.equal({ + errors: [{ code: 'TestError', title: 'Test error' }] + }) + }) + + it('extracts response from error.cause when using fetch adapter', () => { + const mockResponse: AxiosResponse = { + status: 400, + statusText: 'Bad Request', + data: { errors: [{ code: 'FetchError', title: 'Fetch error' }] }, + headers: {}, + config: {} as InternalAxiosRequestConfig + } + + const error = { + message: 'Request failed with status code 400', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: undefined, + config: {} as InternalAxiosRequestConfig, + cause: { + response: mockResponse + } + } as unknown as AxiosError + + const result = parseAxiosError(error) + + expect(result.message).to.equal('Request failed with status code 400') + expect(result.response).to.not.equal(undefined) + expect(result.response?.status).to.equal(400) + expect(result.response?.data).to.deep.equal({ + errors: [{ code: 'FetchError', title: 'Fetch error' }] + }) + }) + + it('parses JSON string data from fetch adapter response', () => { + const mockResponse = { + status: 400, + statusText: 'Bad Request', + data: '{"errors":[{"code":"JSONError","title":"JSON error"}]}', + headers: {}, + config: {} as InternalAxiosRequestConfig + } as AxiosResponse + + const error = { + message: 'Request failed with status code 400', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: undefined, + config: {} as InternalAxiosRequestConfig, + cause: { + response: mockResponse + } + } as unknown as AxiosError + + const result = parseAxiosError(error) + + expect(result.message).to.equal('Request failed with status code 400') + expect(result.response).to.not.equal(undefined) + expect(result.response?.data).to.deep.equal({ + errors: [{ code: 'JSONError', title: 'JSON error' }] + }) + }) + + it('handles response with JSON string data directly on response', () => { + const mockResponse = { + status: 400, + statusText: 'Bad Request', + data: '{"template_errors":{"home_page":[{"description":"error"}]}}', + headers: {}, + config: {} as InternalAxiosRequestConfig + } as AxiosResponse + + const error: AxiosError = { + message: 'Request failed with status code 400', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: mockResponse, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.response?.data).to.deep.equal({ + template_errors: { + home_page: [{ description: 'error' }] + } + }) + }) + + it('keeps string data as-is when it is not JSON', () => { + const mockResponse = { + status: 500, + statusText: 'Internal Server Error', + data: 'Plain text error message', + headers: {}, + config: {} as InternalAxiosRequestConfig + } as AxiosResponse + + const error: AxiosError = { + message: 'Request failed with status code 500', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: mockResponse, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.response?.data).to.equal('Plain text error message') + }) + + it('keeps string data as-is when JSON parsing fails', () => { + const mockResponse = { + status: 400, + statusText: 'Bad Request', + data: '{invalid json}', + headers: {}, + config: {} as InternalAxiosRequestConfig + } as AxiosResponse + + const error: AxiosError = { + message: 'Request failed with status code 400', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: mockResponse, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.response?.data).to.equal('{invalid json}') + }) + + it('returns undefined response when no response exists', () => { + const error: AxiosError = { + message: 'Network Error', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: undefined, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.message).to.equal('Network Error') + expect(result.response === undefined).to.equal(true) + }) + + it('returns undefined response when neither response nor cause.response exists', () => { + const error = { + message: 'Network Error', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: undefined, + config: {} as InternalAxiosRequestConfig, + cause: {} + } as unknown as AxiosError + + const result = parseAxiosError(error) + + expect(result.message).to.equal('Network Error') + expect(result.response === undefined).to.equal(true) + }) + + it('parses JSON string when content-type is application/json', () => { + const mockResponse = { + status: 400, + statusText: 'Bad Request', + data: '{"error":"Server error"}', + headers: { 'content-type': 'application/json; charset=utf-8' }, + config: {} as InternalAxiosRequestConfig + } as AxiosResponse + + const error: AxiosError = { + message: 'Request failed', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: mockResponse, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.response?.data).to.deep.equal({ error: 'Server error' }) + }) + + it('parses JSON array string data', () => { + const mockResponse = { + status: 200, + statusText: 'OK', + data: '[{"id":1,"name":"test"}]', + headers: {}, + config: {} as InternalAxiosRequestConfig + } as AxiosResponse + + const error: AxiosError = { + message: 'Request failed', + name: 'AxiosError', + isAxiosError: true, + toJSON: () => ({}), + response: mockResponse, + config: {} as InternalAxiosRequestConfig + } + + const result = parseAxiosError(error) + + expect(result.response?.data).to.deep.equal([{ id: 1, name: 'test' }]) + }) +}) diff --git a/packages/zcli-themes/src/lib/parseAxiosError.ts b/packages/zcli-themes/src/lib/parseAxiosError.ts new file mode 100644 index 00000000..25074dd8 --- /dev/null +++ b/packages/zcli-themes/src/lib/parseAxiosError.ts @@ -0,0 +1,43 @@ +import type { AxiosError, AxiosResponse } from 'axios' + +/** + * Parses an AxiosError to extract message and response, handling quirks with the fetch adapter. + * + * When axios uses adapter: 'fetch', it creates nested errors where: + * - The actual response is in error.cause.response instead of error.response + * - The response.data is an unparsed JSON string instead of a parsed object + * + * This function normalizes the error structure across different axios configurations. + * + * @param error - The AxiosError to parse + * @returns An object containing the error message and normalized response (if any) + */ +export default function parseAxiosError (error: AxiosError): { + message: string; + response: AxiosResponse | undefined; +} { + let response = error.response + + // Handle axios fetch adapter quirk: response might be in error.cause.response + interface ErrorWithCause { + cause?: { response?: AxiosResponse }; + } + const errorWithCause = error as ErrorWithCause + if (!response && errorWithCause.cause?.response) { + response = errorWithCause.cause.response + } + + // Handle axios fetch adapter quirk: data might be unparsed JSON string + if (response && typeof response.data === 'string') { + try { + response.data = JSON.parse(response.data) + } catch { + // Keep as string if not valid JSON + } + } + + return { + message: error.message, + response + } +} diff --git a/packages/zcli-themes/src/lib/preview.ts b/packages/zcli-themes/src/lib/preview.ts index f6f0cf41..aaedc407 100644 --- a/packages/zcli-themes/src/lib/preview.ts +++ b/packages/zcli-themes/src/lib/preview.ts @@ -1,4 +1,4 @@ -import type { Flags, ValidationError, ValidationErrors } from '../types' +import type { Flags, ValidationErrors } from '../types' import getManifest from './getManifest' import getTemplates from './getTemplates' import getVariables from './getVariables' @@ -7,9 +7,9 @@ import * as chalk from 'chalk' import { request } from '@zendesk/zcli-core' import { error } from '@oclif/core/lib/errors' import { CliUx } from '@oclif/core' -import validationErrorsToString from './validationErrorsToString' import { getLocalServerBaseUrl } from './getLocalServerBaseUrl' import type { AxiosError } from 'axios' +import handleTemplateError from './handleTemplateError' export default async function preview (themePath: string, flags: Flags): Promise { const manifest = getManifest(themePath) @@ -68,7 +68,7 @@ export default async function preview (themePath: string, flags: Flags): Promise template_errors: ValidationErrors, general_error: string } - if (templateErrors) handlePreviewError(themePath, templateErrors) + if (templateErrors) handleTemplateError(themePath, templateErrors) else if (generalError) error(generalError) else error(message) } else { @@ -85,18 +85,3 @@ export function livereloadScript (flags: Flags) { })() ` } - -function handlePreviewError (themePath: string, templateErrors: ValidationErrors) { - const validationErrors: ValidationErrors = {} - for (const [template, errors] of Object.entries(templateErrors)) { - // the preview endpoint returns the template identifier as the 'key' instead of - // the template path. We must fix this so we can reuse `validationErrorsToString` - // and align with the job import error handling - validationErrors[`templates/${template}.hbs`] = errors as ValidationError[] - } - - const title = `${chalk.bold('InvalidTemplates')} - Template(s) with syntax error(s)` - const details = validationErrorsToString(themePath, validationErrors) - - error(`${title}\n${details}`) -} diff --git a/packages/zcli-themes/src/lib/rewriteManifest.test.ts b/packages/zcli-themes/src/lib/rewriteManifest.test.ts new file mode 100644 index 00000000..db35d982 --- /dev/null +++ b/packages/zcli-themes/src/lib/rewriteManifest.test.ts @@ -0,0 +1,128 @@ +import * as sinon from 'sinon' +import * as fs from 'fs' +import { expect } from '@oclif/test' +import rewriteManifest from './rewriteManifest' + +describe('rewriteManifest', () => { + beforeEach(() => { + sinon.restore() + }) + + it('updates the api_version in manifest.json', () => { + const readFileSyncStub = sinon.stub(fs, 'readFileSync') + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const manifestContent = JSON.stringify( + { + name: 'Copenhagen theme', + author: 'Jane Doe', + version: '1.0.1', + api_version: 1, + settings: [] + }, + null, + 2 + ) + + readFileSyncStub + .withArgs('theme/path/manifest.json', 'utf8') + .returns(manifestContent) + + rewriteManifest('theme/path', 2) + + expect(writeFileSyncStub.calledOnce).to.equal(true) + expect(writeFileSyncStub.firstCall.args[0]).to.equal( + 'theme/path/manifest.json' + ) + + const writtenContent = writeFileSyncStub.firstCall.args[1] as string + const parsedContent = JSON.parse(writtenContent) + expect(parsedContent.api_version).to.equal(2) + }) + + it('preserves formatting when updating api_version', () => { + const readFileSyncStub = sinon.stub(fs, 'readFileSync') + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const manifestContent = + '{\n "name": "Copenhagen",\n "api_version": 1,\n "version": "1.0.0"\n}' + + readFileSyncStub + .withArgs('theme/path/manifest.json', 'utf8') + .returns(manifestContent) + + rewriteManifest('theme/path', 3) + + const writtenContent = writeFileSyncStub.firstCall.args[1] + expect(writtenContent).to.equal( + '{\n "name": "Copenhagen",\n "api_version": 3,\n "version": "1.0.0"\n}' + ) + }) + + it('handles api_version with different spacing', () => { + const readFileSyncStub = sinon.stub(fs, 'readFileSync') + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const manifestContent = + '{"name": "Theme", "api_version" : 1, "version": "1.0"}' + + readFileSyncStub + .withArgs('theme/path/manifest.json', 'utf8') + .returns(manifestContent) + + rewriteManifest('theme/path', 5) + + const writtenContent = writeFileSyncStub.firstCall.args[1] + expect(writtenContent).to.include('"api_version": 5') + expect(writtenContent).to.include('"name": "Theme"') + }) + + it('throws an error when file cannot be read', () => { + const readFileSyncStub = sinon.stub(fs, 'readFileSync') + + readFileSyncStub + .withArgs('theme/path/manifest.json', 'utf8') + .throws(new Error('ENOENT: no such file or directory')) + + expect(() => { + rewriteManifest('theme/path', 2) + }).to.throw( + 'Failed to read or write manifest file: theme/path/manifest.json' + ) + }) + + it('throws an error when JSON is malformed', () => { + const readFileSyncStub = sinon.stub(fs, 'readFileSync') + + readFileSyncStub + .withArgs('theme/path/manifest.json', 'utf8') + .returns('{"name": "Theme",,,}') + + expect(() => { + rewriteManifest('theme/path', 2) + }).to.throw( + 'Failed to read or write manifest file: theme/path/manifest.json' + ) + }) + + it('throws an error when file cannot be written', () => { + const readFileSyncStub = sinon.stub(fs, 'readFileSync') + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const manifestContent = '{"name": "Theme", "api_version": 1}' + + readFileSyncStub + .withArgs('theme/path/manifest.json', 'utf8') + .returns(manifestContent) + + writeFileSyncStub + .withArgs('theme/path/manifest.json') + .throws(new Error('EACCES: permission denied')) + + expect(() => { + rewriteManifest('theme/path', 2) + }).to.throw( + 'Failed to read or write manifest file: theme/path/manifest.json' + ) + }) +}) diff --git a/packages/zcli-themes/src/lib/rewriteManifest.ts b/packages/zcli-themes/src/lib/rewriteManifest.ts new file mode 100644 index 00000000..007a9b02 --- /dev/null +++ b/packages/zcli-themes/src/lib/rewriteManifest.ts @@ -0,0 +1,18 @@ +import { CLIError } from '@oclif/core/lib/errors' +import * as fs from 'fs' +import * as chalk from 'chalk' + +export default function rewriteManifest (themePath: string, apiVersion: number) { + const manifestFilePath = `${themePath}/manifest.json` + + try { + const manifestFile = fs.readFileSync(manifestFilePath, 'utf8') + + // Rewrite with "replace" for minimal diff + const updatedContent = manifestFile.replace(/"api_version"\s*:\s*\d+/, `"api_version": ${apiVersion}`) + + fs.writeFileSync(manifestFilePath, updatedContent) + } catch (error) { + throw new CLIError(chalk.red(`Failed to read or write manifest file: ${manifestFilePath}`)) + } +} diff --git a/packages/zcli-themes/src/lib/rewriteTemplates.test.ts b/packages/zcli-themes/src/lib/rewriteTemplates.test.ts new file mode 100644 index 00000000..982f95e2 --- /dev/null +++ b/packages/zcli-themes/src/lib/rewriteTemplates.test.ts @@ -0,0 +1,85 @@ +import * as sinon from 'sinon' +import * as fs from 'fs' +import { expect } from '@oclif/test' +import rewriteTemplates from './rewriteTemplates' + +describe('rewriteTemplates', () => { + beforeEach(() => { + sinon.restore() + }) + + it('writes templates to the correct file paths', () => { + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const templates = { + home_page: '

Updated Home

', + article_page: '

Updated Article

', + 'custom_pages/faq': '

Updated FAQ

' + } + + rewriteTemplates('theme/path', templates) + + expect(writeFileSyncStub.callCount).to.equal(3) + expect(writeFileSyncStub.firstCall.args).to.deep.equal([ + 'theme/path/templates/home_page.hbs', + '

Updated Home

' + ]) + expect(writeFileSyncStub.secondCall.args).to.deep.equal([ + 'theme/path/templates/article_page.hbs', + '

Updated Article

' + ]) + expect(writeFileSyncStub.thirdCall.args).to.deep.equal([ + 'theme/path/templates/custom_pages/faq.hbs', + '

Updated FAQ

' + ]) + }) + + it('ignores write errors', () => { + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + writeFileSyncStub.onFirstCall().throws(new Error('Permission denied')) + writeFileSyncStub.onSecondCall().returns(undefined) + + const templates = { + home_page: '

Updated Home

', + article_page: '

Updated Article

' + } + + expect(() => { + rewriteTemplates('theme/path', templates) + }).to.not.throw() + + expect(writeFileSyncStub.callCount).to.equal(2) + }) + + it('handles empty templates object', () => { + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const templates = {} + + rewriteTemplates('theme/path', templates) + + expect(writeFileSyncStub.callCount).to.equal(0) + }) + + it('handles nested template paths correctly', () => { + const writeFileSyncStub = sinon.stub(fs, 'writeFileSync') + + const templates = { + 'article_pages/product_updates': '

Product Updates

', + 'custom_pages/deep/nested/template': '

Nested Template

' + } + + rewriteTemplates('theme/path', templates) + + expect(writeFileSyncStub.callCount).to.equal(2) + expect(writeFileSyncStub.firstCall.args).to.deep.equal([ + 'theme/path/templates/article_pages/product_updates.hbs', + '

Product Updates

' + ]) + expect(writeFileSyncStub.secondCall.args).to.deep.equal([ + 'theme/path/templates/custom_pages/deep/nested/template.hbs', + '

Nested Template

' + ]) + }) +}) diff --git a/packages/zcli-themes/src/lib/rewriteTemplates.ts b/packages/zcli-themes/src/lib/rewriteTemplates.ts new file mode 100644 index 00000000..93099a6a --- /dev/null +++ b/packages/zcli-themes/src/lib/rewriteTemplates.ts @@ -0,0 +1,15 @@ +import * as fs from 'fs' + +export default function rewriteTemplates (themePath: string, templates: Record) { + for (const [identifier, content] of Object.entries(templates)) { + const filePath = `${themePath}/templates/${identifier}.hbs` + + if (typeof content === 'string') { + try { + fs.writeFileSync(filePath, content) + } catch (error) { + // Ignore errors if file doesn't exist or can't be written + } + } + } +} diff --git a/packages/zcli-themes/tests/functional/delete.test.ts b/packages/zcli-themes/tests/functional/delete.test.ts index d66c7ae6..3643c919 100644 --- a/packages/zcli-themes/tests/functional/delete.test.ts +++ b/packages/zcli-themes/tests/functional/delete.test.ts @@ -2,6 +2,7 @@ import { expect, test } from '@oclif/test' import DeleteCommand from '../../src/commands/themes/delete' import env from './env' import * as sinon from 'sinon' +import { CLIError } from '@oclif/core/lib/errors' describe('themes:delete', function () { let fetchStub: sinon.SinonStub @@ -67,8 +68,8 @@ describe('themes:delete', function () { await DeleteCommand.run(['--themeId', '1234']) } catch (error) { expect(ctx.stderr).to.contain('!') - expect(error.message).to.contain('ThemeNotFound') - expect(error.message).to.contain('Invalid id') + expect((error as CLIError).message).to.contain('ThemeNotFound') + expect((error as CLIError).message).to.contain('Invalid id') } }) }) diff --git a/packages/zcli-themes/tests/functional/import.test.ts b/packages/zcli-themes/tests/functional/import.test.ts index df7a7d95..28b60020 100644 --- a/packages/zcli-themes/tests/functional/import.test.ts +++ b/packages/zcli-themes/tests/functional/import.test.ts @@ -4,6 +4,7 @@ import * as sinon from 'sinon' import * as path from 'path' import ImportCommand from '../../src/commands/themes/import' import env from './env' +import { CLIError } from '@oclif/core/lib/errors' describe('themes:import', function () { const baseThemePath = path.join(__dirname, 'mocks/base_theme') @@ -100,8 +101,8 @@ describe('themes:import', function () { await ImportCommand.run([baseThemePath, '--brandId', '1111']) } catch (error) { expect(ctx.stderr).to.contain('!') - expect(error.message).to.contain('TooManyThemes') - expect(error.message).to.contain('Maximum number of allowed themes reached') + expect((error as CLIError).message).to.contain('TooManyThemes') + expect((error as CLIError).message).to.contain('Maximum number of allowed themes reached') } }) @@ -161,10 +162,10 @@ describe('themes:import', function () { try { await ImportCommand.run([baseThemePath, '--brandId', '1111']) } catch (error) { - expect(error.message).to.contain('InvalidTemplates') - expect(error.message).to.contain('Template(s) with syntax error(s)') - expect(error.message).to.contain('Validation error') - expect(error.message).to.contain("'post_form' does not exist") + expect((error as CLIError).message).to.contain('InvalidTemplates') + expect((error as CLIError).message).to.contain('Template(s) with syntax error(s)') + expect((error as CLIError).message).to.contain('Validation error') + expect((error as CLIError).message).to.contain("'post_form' does not exist") } }) }) diff --git a/packages/zcli-themes/tests/functional/list.test.ts b/packages/zcli-themes/tests/functional/list.test.ts index af633238..4a3025c1 100644 --- a/packages/zcli-themes/tests/functional/list.test.ts +++ b/packages/zcli-themes/tests/functional/list.test.ts @@ -2,6 +2,7 @@ import { expect, test } from '@oclif/test' import * as sinon from 'sinon' import ListCommand from '../../src/commands/themes/list' import env from './env' +import { CLIError } from '@oclif/core/lib/errors' describe('themes:list', function () { let fetchStub: sinon.SinonStub @@ -67,8 +68,8 @@ describe('themes:list', function () { await ListCommand.run(['--brandId', '1111']) } catch (error) { expect(ctx.stderr).to.contain('!') - expect(error.message).to.contain('InternalError') - expect(error.message).to.contain('Something went wrong') + expect((error as CLIError).message).to.contain('InternalError') + expect((error as CLIError).message).to.contain('Something went wrong') } }) }) diff --git a/packages/zcli-themes/tests/functional/migrate.test.ts b/packages/zcli-themes/tests/functional/migrate.test.ts new file mode 100644 index 00000000..0e344e73 --- /dev/null +++ b/packages/zcli-themes/tests/functional/migrate.test.ts @@ -0,0 +1,171 @@ +import { expect, test } from '@oclif/test' +import MigrateCommand from '../../src/commands/themes/migrate' +import env from './env' +import * as sinon from 'sinon' +import * as path from 'path' +import * as fs from 'fs' +import { CLIError } from '@oclif/core/lib/errors' + +describe('themes:migrate', function () { + const baseThemePath = path.join(__dirname, 'mocks/base_theme') + let fetchStub: sinon.SinonStub + let manifestBackup: string + let templateBackup: string + + beforeEach(() => { + fetchStub = sinon.stub(global, 'fetch') + // Backup original files + manifestBackup = fs.readFileSync( + path.join(baseThemePath, 'manifest.json'), + 'utf8' + ) + templateBackup = fs.readFileSync( + path.join(baseThemePath, 'templates/document_head.hbs'), + 'utf8' + ) + }) + + afterEach(() => { + fetchStub.restore() + // Restore original files + fs.writeFileSync(path.join(baseThemePath, 'manifest.json'), manifestBackup) + fs.writeFileSync( + path.join(baseThemePath, 'templates/document_head.hbs'), + templateBackup + ) + }) + + describe('successful migration', () => { + const success = test.env(env).do(() => { + fetchStub + .withArgs( + sinon.match({ + url: 'https://z3ntest.zendesk.com/hc/api/internal/theming/migrations', + method: 'POST' + }) + ) + .resolves({ + status: 200, + ok: true, + text: () => + Promise.resolve( + JSON.stringify({ + metadata: { + api_version: 2 + }, + templates: { + document_head: '{{!chat (obsolete)}}' + } + }) + ) + }) + }) + + success + .stdout() + .it('should migrate theme successfully and update files', async () => { + await MigrateCommand.run([baseThemePath]) + + const manifest = JSON.parse( + fs.readFileSync(path.join(baseThemePath, 'manifest.json'), 'utf8') + ) + expect(manifest.api_version).to.equal(2) + + // Verify template was updated + const template = fs.readFileSync( + path.join(baseThemePath, 'templates/document_head.hbs'), + 'utf8' + ) + expect(template).to.contain('{{!chat (obsolete)}}') + }) + }) + + describe('migration with template errors', () => { + test + .env(env) + .stderr() + .do(() => { + fetchStub + .withArgs( + sinon.match({ + url: 'https://z3ntest.zendesk.com/hc/api/internal/theming/migrations', + method: 'POST' + }) + ) + .resolves({ + status: 400, + ok: false, + text: () => + Promise.resolve( + JSON.stringify({ + template_errors: { + document_head: [ + { + description: "'articles' does not exist", + line: 10, + column: 6, + length: 7 + } + ] + } + }) + ) + }) + }) + .it('should report template validation errors', async (ctx) => { + try { + await MigrateCommand.run([baseThemePath]) + throw new Error('Should have thrown an error') + } catch (error) { + if ( + error instanceof Error && + error.message === 'Should have thrown an error' + ) { + throw error + } + expect(ctx.stderr).to.contain('!') + expect((error as CLIError).oclif.exit).to.equal(2) + } + }) + }) + + describe('migration with general error', () => { + test + .env(env) + .stderr() + .do(() => { + fetchStub + .withArgs( + sinon.match({ + url: 'https://z3ntest.zendesk.com/hc/api/internal/theming/migrations', + method: 'POST' + }) + ) + .resolves({ + status: 400, + ok: false, + text: () => + Promise.resolve( + JSON.stringify({ + general_error: 'Theme migration failed' + }) + ) + }) + }) + .it('should report general errors', async (ctx) => { + try { + await MigrateCommand.run([baseThemePath]) + throw new Error('Should have thrown an error') + } catch (error) { + if ( + error instanceof Error && + error.message === 'Should have thrown an error' + ) { + throw error + } + expect(ctx.stderr).to.contain('!') + expect((error as CLIError).oclif.exit).to.equal(2) + } + }) + }) +}) diff --git a/packages/zcli-themes/tests/functional/preview.test.ts b/packages/zcli-themes/tests/functional/preview.test.ts index 61927ba4..d74fb572 100644 --- a/packages/zcli-themes/tests/functional/preview.test.ts +++ b/packages/zcli-themes/tests/functional/preview.test.ts @@ -21,7 +21,7 @@ describe('themes:preview', function () { }) describe('successful preview', () => { - let server + let server: { close: () => void } const preview = test .stdout() diff --git a/packages/zcli-themes/tests/functional/publish.test.ts b/packages/zcli-themes/tests/functional/publish.test.ts index 900c2f18..38250120 100644 --- a/packages/zcli-themes/tests/functional/publish.test.ts +++ b/packages/zcli-themes/tests/functional/publish.test.ts @@ -2,6 +2,7 @@ import { expect, test } from '@oclif/test' import * as sinon from 'sinon' import PublishCommand from '../../src/commands/themes/publish' import env from './env' +import { CLIError } from '@oclif/core/lib/errors' describe('themes:publish', function () { let fetchStub: sinon.SinonStub @@ -67,8 +68,8 @@ describe('themes:publish', function () { await PublishCommand.run(['--themeId', '1234']) } catch (error) { expect(ctx.stderr).to.contain('!') - expect(error.message).to.contain('ThemeNotFound') - expect(error.message).to.contain('Invalid id') + expect((error as CLIError).message).to.contain('ThemeNotFound') + expect((error as CLIError).message).to.contain('Invalid id') } }) }) diff --git a/packages/zcli-themes/tests/functional/update.test.ts b/packages/zcli-themes/tests/functional/update.test.ts index 7a9102eb..1f0d6d41 100644 --- a/packages/zcli-themes/tests/functional/update.test.ts +++ b/packages/zcli-themes/tests/functional/update.test.ts @@ -4,6 +4,7 @@ import * as path from 'path' import UpdateCommand from '../../src/commands/themes/update' import env from './env' import * as sinon from 'sinon' +import { CLIError } from '@oclif/core/lib/errors' describe('themes:update', function () { const baseThemePath = path.join(__dirname, 'mocks/base_theme') @@ -95,8 +96,8 @@ describe('themes:update', function () { await UpdateCommand.run([baseThemePath, '--themeId', '1234']) } catch (error) { expect(ctx.stderr).to.contain('!') - expect(error.message).to.contain('TooManyThemes') - expect(error.message).to.contain('Maximum number of allowed themes reached') + expect((error as CLIError).message).to.contain('TooManyThemes') + expect((error as CLIError).message).to.contain('Maximum number of allowed themes reached') } }) @@ -149,14 +150,14 @@ describe('themes:update', function () { text: () => Promise.resolve('') }) }) - .it('should report validation errors', async (ctx) => { + .it('should report validation errors', async () => { try { await UpdateCommand.run([baseThemePath, '--themeId', '1111']) } catch (error) { - expect(error.message).to.contain('InvalidTemplates') - expect(error.message).to.contain('Template(s) with syntax error(s)') - expect(error.message).to.contain('Validation error') - expect(error.message).to.contain("'request_fosrm' does not exist") + expect((error as CLIError).message).to.contain('InvalidTemplates') + expect((error as CLIError).message).to.contain('Template(s) with syntax error(s)') + expect((error as CLIError).message).to.contain('Validation error') + expect((error as CLIError).message).to.contain("'request_fosrm' does not exist") } }) })