themes:migrate: integrate with the new migrations endpoint contract

packages/zcli-themes/src/commands/themes/migrate.ts

@@ -1,6 +1,10 @@
-import { Command } from '@oclif/core'
+import { Command, CliUx } from '@oclif/core'
 import * as path from 'path'
+import * as chalk from 'chalk'
+import type { AxiosError } from 'axios'
 import migrate from '../../lib/migrate'
+import migrationReportToString from '../../lib/migrationReportToString'
+import handleThemeMigrationApiError from '../../lib/handleThemeMigrationApiError'
 
 export default class Migrate extends Command {
   static description = 'migrate theme to the latest version of the templating api'
@@ -21,6 +25,14 @@ export default class Migrate extends Command {
     const { argv: [themeDirectory] } = await this.parse(Migrate)
     const themePath = path.resolve(themeDirectory)
 
-    await migrate(themePath)
+    try {
+      CliUx.ux.action.start('Migrating theme')
+      const report = await migrate(themePath)
+      CliUx.ux.action.stop('Ok')
+      this.log(migrationReportToString(report))
+    } catch (e) {
+      CliUx.ux.action.stop(chalk.bold.red('!'))
+      handleThemeMigrationApiError(e as AxiosError, themePath)
+    }
   }
 }

packages/zcli-themes/src/lib/handleTemplateError.ts

@@ -1,15 +1,14 @@
-import { ValidationError, ValidationErrors } from '../types'
+import { TemplateErrors, 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) {
+export default function handleTemplateError (themePath: string, templateErrors: TemplateErrors) {
+  // The theming endpoints key these by template identifier (e.g. `home_page`).
+  // `validationErrorsToString` expects a path-keyed shape, so we convert here.
   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[]
+  for (const [identifier, errors] of Object.entries(templateErrors)) {
+    validationErrors[`templates/${identifier}.hbs`] = errors
   }
 
   const title = `${chalk.bold('InvalidTemplates')} - Template(s) with syntax error(s)`

packages/zcli-themes/src/lib/handleThemeMigrationApiError.test.ts

@@ -0,0 +1,99 @@
+import * as sinon from 'sinon'
+import { expect } from '@oclif/test'
+import * as errors from '@oclif/core/lib/errors'
+import handleThemeMigrationApiError from './handleThemeMigrationApiError'
+import type { AxiosError } from 'axios'
+
+describe('handleThemeMigrationApiError', () => {
+  beforeEach(() => {
+    sinon.restore()
+  })
+
+  it('routes template_errors through handleTemplateError', () => {
+    const errorStub = sinon.stub(errors, 'error').callThrough()
+    const e = {
+      response: {
+        status: 400,
+        data: {
+          template_errors: {
+            home_page: [{ description: "'articles' does not exist", line: 10, column: 6, length: 7 }]
+          }
+        }
+      }
+    } as unknown as AxiosError
+
+    try {
+      handleThemeMigrationApiError(e, 'theme/path')
+    } catch {
+      const [call] = errorStub.getCalls()
+      const [thrown] = call.args
+      expect(thrown).to.contain('theme/path/templates/home_page.hbs:10:6')
+      expect(thrown).to.contain("'articles' does not exist")
+    }
+  })
+
+  it('renders the server general_error suffixed with a plain issues URL on a 5xx', () => {
+    const errorStub = sinon.stub(errors, 'error').callThrough()
+    const e = {
+      response: {
+        status: 500,
+        data: { general_error: 'Failed to migrate the theme.' }
+      }
+    } as unknown as AxiosError
+
+    try {
+      handleThemeMigrationApiError(e, 'theme/path')
+    } catch {
+      const [call] = errorStub.getCalls()
+      const thrown = call.args[0] as string
+      expect(thrown).to.match(/^Failed to migrate the theme\./)
+      expect(thrown).to.contain('Please open an issue at https://github.com/zendesk/zcli/issues/new')
+      expect(thrown).to.not.contain('themes-migrate')
+      expect(thrown).to.not.contain('## What happened')
+    }
+  })
+
+  it('throws the general_error verbatim on a 4xx', () => {
+    const errorStub = sinon.stub(errors, 'error').callThrough()
+    const e = {
+      response: {
+        status: 400,
+        data: { general_error: 'Theme is already on the latest supported version for migration' }
+      }
+    } as unknown as AxiosError
+
+    try {
+      handleThemeMigrationApiError(e, 'theme/path')
+    } catch {
+      const [call] = errorStub.getCalls()
+      expect(call.args[0]).to.equal('Theme is already on the latest supported version for migration')
+    }
+  })
+
+  it('falls back to the axios message when no body fields are present', () => {
+    const errorStub = sinon.stub(errors, 'error').callThrough()
+    const e = {
+      response: { status: 400, data: {} },
+      message: 'Network error'
+    } as unknown as AxiosError
+
+    try {
+      handleThemeMigrationApiError(e, 'theme/path')
+    } catch {
+      const [call] = errorStub.getCalls()
+      expect(call.args[0]).to.equal('Network error')
+    }
+  })
+
+  it('throws the original axios error when there is no response', () => {
+    const errorStub = sinon.stub(errors, 'error').callThrough()
+    const e = new Error('Connection refused') as AxiosError
+
+    try {
+      handleThemeMigrationApiError(e, 'theme/path')
+    } catch {
+      const [call] = errorStub.getCalls()
+      expect(call.args[0]).to.equal(e)
+    }
+  })
+})

packages/zcli-themes/src/lib/handleThemeMigrationApiError.ts

@@ -0,0 +1,25 @@
+import type { AxiosError } from 'axios'
+import { error } from '@oclif/core/lib/errors'
+import type { MigrateErrorBody } from '../types'
+import parseAxiosError from './parseAxiosError'
+import handleTemplateError from './handleTemplateError'
+
+export default function handleThemeMigrationApiError (e: AxiosError, themePath: string) {
+  const { message, response } = parseAxiosError(e)
+
+  if (!response) error(e)
+
+  const status = response.status
+  const body = (response.data ?? {}) as MigrateErrorBody
+  const { template_errors: templateErrors, general_error: generalError } = body
+
+  if (templateErrors) {
+    handleTemplateError(themePath, templateErrors)
+  } else if (status >= 500 && generalError) {
+    error(`${generalError} Please open an issue at https://github.com/zendesk/zcli/issues/new so we can investigate.`)
+  } else if (generalError) {
+    error(generalError)
+  } else {
+    error(message)
+  }
+}

packages/zcli-themes/src/lib/migrate.test.ts

@@ -9,7 +9,6 @@ import * as rewriteTemplates from './rewriteTemplates'
 import * as rewriteAssets from './rewriteAssets'
 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 = {
@@ -32,7 +31,7 @@ describe('migrate', () => {
     sinon.restore()
   })
 
-  it('calls the migrations endpoint with the correct payload and rewrites files', async () => {
+  it('calls the migrations endpoint, rewrites files, and returns the migration_report', async () => {
     const getManifestStub = sinon.stub(getManifest, 'default')
     const getTemplatesStub = sinon.stub(getTemplates, 'default')
     const getVariablesStub = sinon.stub(getVariables, 'default')
@@ -71,6 +70,12 @@ describe('migrate', () => {
       ]
     ])
 
+    const migrationReport = {
+      home_page: [
+        { target: 'logo_url', strategy: 'inline', description: 'desc', test_plan: 'test' }
+      ]
+    }
+
     requestStub.returns(
       Promise.resolve({
         status: 200,
@@ -86,12 +91,13 @@ describe('migrate', () => {
           },
           assets: {
             'category_tree.js': Buffer.from('console.log("tree")').toString('base64')
-          }
+          },
+          migration_report: migrationReport
         }
       }) as axios.AxiosPromise
     )
 
-    await migrate('theme/path')
+    const result = await migrate('theme/path')
 
     expect(
       requestStub.calledWith(
@@ -134,103 +140,11 @@ describe('migrate', () => {
         'category_tree.js': Buffer.from('console.log("tree")').toString('base64')
       })
     ).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')
-    } 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")
-    }
+    expect(result).to.deep.equal(migrationReport)
   })
 
-  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')
-    } 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')
-    } 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 () => {
+  it('propagates AxiosError on request failure', async () => {
     sinon.stub(getManifest, 'default').returns(manifest)
     sinon.stub(getTemplates, 'default').returns({})
     sinon.stub(getVariables, 'default').returns([])
@@ -239,14 +153,11 @@ describe('migrate', () => {
     const axiosError = new Error('Connection refused')
     sinon.stub(request, 'requestAPI').throws(axiosError)
 
-    const errorStub = sinon.stub(errors, 'error').callThrough()
-
     try {
       await migrate('theme/path')
-    } catch {
-      const [call] = errorStub.getCalls()
-      const [error] = call.args
-      expect(error).to.equal(axiosError)
+      throw new Error('Should have thrown')
+    } catch (e) {
+      expect(e).to.equal(axiosError)
     }
   })
 })