From c8b2166cfd32b7b193a5f466586e332b28d933c2 Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Mon, 24 Feb 2020 17:04:01 +0800 Subject: [PATCH 1/2] Fix convert command so it correctly handles VB app converted from WB --- docs/index.html | 1 - packages/cli/README-source.md | 1 - packages/cli/README.md | 1 - packages/cli/docs/index.html | 1 - packages/cli/package.json | 1 - packages/cli/scripts/test-convert.js | 83 --------------- packages/cli/src/oclif/commands/convert.js | 6 +- packages/cli/src/tests/utils/convert.js | 42 +++++--- packages/cli/src/utils/convert.js | 113 ++++++++++----------- 9 files changed, 84 insertions(+), 165 deletions(-) delete mode 100755 packages/cli/scripts/test-convert.js diff --git a/docs/index.html b/docs/index.html index 728d07a7f..9fbc52a11 100644 --- a/docs/index.html +++ b/docs/index.html @@ -5095,7 +5095,6 @@

Commands

  • npm run watch for automatically building as you work
  • npm test for running tests (also runs npm run build)
  • npm link (while in this project's directory) and then use zapier command elsewhere
    • -
  • npm run test-convert for running integration tests for the zapier convert command
  • npm run docs for updating docs
  • npm run gen-completions for updating the auto complete scripts
    • diff --git a/packages/cli/README-source.md b/packages/cli/README-source.md index 9094fd7fb..10f53e0f7 100644 --- a/packages/cli/README-source.md +++ b/packages/cli/README-source.md @@ -1720,7 +1720,6 @@ This section is only relevant if you're editing the `zapier-platform-cli` packag - `npm run watch` for automatically building as you work - `npm test` for running tests (also runs `npm run build`) - `npm link` (while in this project's directory) and then use `zapier` command elsewhere -- `npm run test-convert` for running integration tests for the `zapier convert` command - `npm run docs` for updating docs - `npm run gen-completions` for updating the auto complete scripts diff --git a/packages/cli/README.md b/packages/cli/README.md index 519733ba9..6a53078f0 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -3114,7 +3114,6 @@ This section is only relevant if you're editing the `zapier-platform-cli` packag - `npm run watch` for automatically building as you work - `npm test` for running tests (also runs `npm run build`) - `npm link` (while in this project's directory) and then use `zapier` command elsewhere -- `npm run test-convert` for running integration tests for the `zapier convert` command - `npm run docs` for updating docs - `npm run gen-completions` for updating the auto complete scripts diff --git a/packages/cli/docs/index.html b/packages/cli/docs/index.html index 728d07a7f..9fbc52a11 100644 --- a/packages/cli/docs/index.html +++ b/packages/cli/docs/index.html @@ -5095,7 +5095,6 @@

    Commands

  • npm run watch for automatically building as you work
  • npm test for running tests (also runs npm run build)
  • npm link (while in this project's directory) and then use zapier command elsewhere
    • -
  • npm run test-convert for running integration tests for the zapier convert command
  • npm run docs for updating docs
  • npm run gen-completions for updating the auto complete scripts
    • diff --git a/packages/cli/package.json b/packages/cli/package.json index dedf6ee66..d63fda950 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -33,7 +33,6 @@ "smoke-test": "NODE_ENV=test mocha -t 120000 --recursive src/smoke-tests", "validate-templates": "./scripts/validate-app-templates.js", "set-template-versions": "./scripts/set-app-template-versions.js", - "test-convert": "./scripts/test-convert.js", "validate": "yarn test && yarn smoke-test && yarn lint" }, "dependencies": { diff --git a/packages/cli/scripts/test-convert.js b/packages/cli/scripts/test-convert.js deleted file mode 100755 index 76b4675a3..000000000 --- a/packages/cli/scripts/test-convert.js +++ /dev/null @@ -1,83 +0,0 @@ -#!/usr/bin/env node - -const _ = require('lodash'); -const path = require('path'); -const tmp = require('tmp'); -const { promisifyAll } = require('../src/utils/promisify'); - -const fse = require('fs-extra'); -const childProcess = promisifyAll(require('child_process')); - -const appsToConvert = [ - { id: 80082, name: 'simple-basic-auth' }, - { id: 80182, name: 'trigger-session-auth' }, - { id: 82052, name: 'simple-oauth' }, - { id: 82250, name: 'search-oauth' }, - { id: 82251, name: 'basic-api-header' }, - { id: 82460, name: 'custom-fields' }, - { id: 83195, name: 'search-or-create' }, - { id: 80444, name: 'custom-basic' }, - { id: 83073, name: 'send-in-json' }, - { id: 83342, name: 'replace-vars' }, - { id: 88339, name: 'create-oauth' }, - { id: 88722, name: 'no-custom-fields-url' }, - { id: 90353, name: 'auth-mapping-has-number' } - // TODO: Add more apps that use different scripting methods, as we start to support them -]; - -const testConvertedApp = (appToConvert, rootTmpDir) => { - const zapierCmd = path.resolve(__dirname, '../src/bin/run'); - // Prepare all env variables the apps might need - const exportCmd = - 'export CLIENT_ID=1234 CLIENT_SECRET=asdf USERNAME=user PASSWORD=secret API_KEY=secret SESSION_KEY=secret ACCESS_TOKEN=a_token REFRESH_TOKEN=a_refresh_token'; - - const logFile = path.resolve(__dirname, '..', `${appToConvert.name}.log`); - const logStream = fse.createWriteStream(logFile); - - console.log( - `Converting and testing ${appToConvert.name}, writing logs to ${logFile}` - ); - return fse - .ensureFile(logFile) - .then(() => { - return new Promise((resolve, reject) => { - const cmd = `${zapierCmd} convert ${appToConvert.id} ${appToConvert.name} --debug && cd ${appToConvert.name} && npm install && ${zapierCmd} validate && ${exportCmd} && ${zapierCmd} test --timeout=10000`; - const child = childProcess.exec(cmd, { cwd: rootTmpDir }, err => { - if (err) { - console.log('error starting child process:', err); - reject(err); - } - resolve(); - }); - child.stdout.pipe(logStream); - child.stderr.pipe(logStream); - }); - }) - .then(() => { - console.log(`${appToConvert.name} converted successfully`); - return null; - }) - .catch(() => { - console.error(`${appToConvert.name} conversion failed. See ${logFile}.`); - return appToConvert.name; - }); -}; - -global.argOpts = {}; - -const rootTmpDir = tmp.tmpNameSync(); -fse.removeSync(rootTmpDir); -fse.ensureDirSync(rootTmpDir); - -const tasks = _.map(appsToConvert, template => - testConvertedApp(template, rootTmpDir) -); - -Promise.all(tasks).then(results => { - const failures = _.filter(results, result => result !== null); - if (failures.length) { - console.error('these apps failed conversion:', failures.join(', ')); - } else { - console.log('apps converted successfully'); - } -}); diff --git a/packages/cli/src/oclif/commands/convert.js b/packages/cli/src/oclif/commands/convert.js index b95e50983..983707c7e 100644 --- a/packages/cli/src/oclif/commands/convert.js +++ b/packages/cli/src/oclif/commands/convert.js @@ -2,7 +2,7 @@ const BaseCommand = require('../ZapierBaseCommand'); const { buildFlags } = require('../buildFlags'); const { callAPI } = require('../../utils/api'); -const { convertVisualApp, convertLegacyApp } = require('../../utils/convert'); +const { convertApp } = require('../../utils/convert'); const { isExistingEmptyDir } = require('../../utils/files'); const { initApp } = require('../../utils/init'); const { BASE_ENDPOINT } = require('../../constants'); @@ -30,7 +30,7 @@ class ConvertCommand extends BaseCommand { } this.stopSpinner(); - return convertVisualApp( + return convertApp( appInfo, versionInfo.definition_override, tempAppDir @@ -62,7 +62,7 @@ class ConvertCommand extends BaseCommand { this.stopSpinner(); - return convertLegacyApp(legacyApp, appDefinition, tempAppDir); + return convertApp(legacyApp, appDefinition, tempAppDir); } catch (e) { if (e.status === 404) { this.error( diff --git a/packages/cli/src/tests/utils/convert.js b/packages/cli/src/tests/utils/convert.js index fc81f344d..a1c90f2c8 100644 --- a/packages/cli/src/tests/utils/convert.js +++ b/packages/cli/src/tests/utils/convert.js @@ -6,7 +6,7 @@ const path = require('path'); const { cloneDeep } = require('lodash'); const should = require('should'); -const { convertLegacyApp, convertVisualApp } = require('../../utils/convert'); +const { convertApp } = require('../../utils/convert'); const legacyAppDefinition = { beforeRequest: [ @@ -286,19 +286,21 @@ const setupTempWorkingDir = () => { }; describe('convert', () => { - let tempAppDir; + let tempAppDir, readTempFile; beforeEach(() => { tempAppDir = setupTempWorkingDir(); + readTempFile = fpath => + fs.readFileSync(path.join(tempAppDir, fpath), 'utf-8'); }); afterEach(() => { fs.removeSync(tempAppDir); }); - describe('legacy apps', () => { + describe('legacy web builder apps', () => { it('should create separate files', async () => { - await convertLegacyApp(legacyApp, legacyAppDefinition, tempAppDir); + await convertApp(legacyApp, legacyAppDefinition, tempAppDir); [ '.zapierapprc', '.gitignore', @@ -314,11 +316,9 @@ describe('convert', () => { }); }); - describe('visual apps apps', () => { + describe('visual builder apps', () => { it('should create separate files', async () => { - await convertVisualApp(visualApp, visualAppDefinition, tempAppDir); - const readTempFile = async fpath => - fs.readFile(path.join(tempAppDir, fpath), 'utf-8'); + await convertApp(visualApp, visualAppDefinition, tempAppDir); [ '.zapierapprc', '.gitignore', @@ -355,15 +355,15 @@ describe('convert', () => { ); should(pkg.version).eql('1.0.2'); - const rcFile = JSON.parse(await readTempFile('.zapierapprc')); + const rcFile = JSON.parse(readTempFile('.zapierapprc')); should(rcFile.id).eql(visualApp.id); should(rcFile.includeInBuild).be.undefined(); - const envFile = await readTempFile('.env'); + const envFile = readTempFile('.env'); should(envFile.includes('ACCESS_TOKEN')).be.true(); should(envFile.includes('REFRESH_TOKEN')).be.true(); - const idxFile = await readTempFile('index.js'); + const idxFile = readTempFile('index.js'); should(idxFile.includes("require('./package.json').version")).be.true(); should( @@ -376,7 +376,7 @@ describe('convert', () => { should(idx.platformVersion).eql(visualAppDefinition.platformVersion); // dynamic fields - const createFile = await readTempFile('creates/create_project.js'); + const createFile = readTempFile('creates/create_project.js'); should(createFile.includes('source:')).be.false(); should(createFile.includes('getInputFields = ')).be.true(); should(createFile.includes('getInputFields0')).be.false(); @@ -388,7 +388,23 @@ describe('convert', () => { const appDefinition = cloneDeep(visualAppDefinition); appDefinition.triggers.codemode.operation.perform.source += '\n// a comment'; - await convertVisualApp(visualApp, appDefinition, tempAppDir); + await convertApp(visualApp, appDefinition, tempAppDir); + }); + + it('should include legacy stuff if it was from web builder', async () => { + const appDefinition = cloneDeep(visualAppDefinition); + appDefinition.legacy = {}; // 'legacy' property makes it, well, "legacy" + + await convertApp(visualApp, appDefinition, tempAppDir); + + const rcFile = JSON.parse(readTempFile('.zapierapprc')); + const packageJson = JSON.parse(readTempFile('package.json')); + + should(rcFile.id).equal(visualApp.id); + should(rcFile.includeInBuild).deepEqual(['scripting.js']); + should.exist( + packageJson.dependencies['zapier-platform-legacy-scripting-runner'] + ); }); }); }); diff --git a/packages/cli/src/utils/convert.js b/packages/cli/src/utils/convert.js index 3b0aa9a8a..6f4aedd9b 100644 --- a/packages/cli/src/utils/convert.js +++ b/packages/cli/src/utils/convert.js @@ -97,35 +97,46 @@ const getAuthFieldKeys = appDefinition => { return fieldKeys; }; -const renderLegacyPackageJson = async (legacyApp, appDefinition) => { +const renderPackageJson = async (appInfo, appDefinition) => { + const name = _.kebabCase( + appInfo.title || _.get(appInfo, ['general', 'title']) + ); + // Not using escapeSpecialChars because we don't want to escape single quotes (not // allowed in JSON) - const description = legacyApp.general.description + const description = ( + appInfo.description || + _.get(appInfo, ['general', 'description']) || + '' + ) .replace(/\n/g, '\\n') .replace(/"/g, '\\"'); - const runnerVersion = await getPackageLatestVersion( - 'zapier-platform-legacy-scripting-runner' - ); + const version = appDefinition.version + ? semver.inc(appDefinition.version, 'patch') + : '1.0.0'; - const templateContext = { - name: _.kebabCase(legacyApp.general.title), - description, - appId: legacyApp.general.app_id || 'null', - cliVersion: PACKAGE_VERSION, - coreVersion: appDefinition.platformVersion, - runnerVersion + const dependencies = { + [PLATFORM_PACKAGE]: appDefinition.platformVersion }; + if (appDefinition.legacy) { + const RUNNER_PACKAGE = 'zapier-platform-legacy-scripting-runner'; + const runnerVersion = await getPackageLatestVersion(RUNNER_PACKAGE); + dependencies[RUNNER_PACKAGE] = runnerVersion; + } - const templateFile = path.join(TEMPLATE_DIR, '/package.template.json'); - return renderTemplate(templateFile, templateContext); -}; + const zapierMeta = { + convertedByCLIVersion: PACKAGE_VERSION + }; + const legacyAppId = _.get(appInfo, ['general', 'app_id']); + if (legacyAppId) { + zapierMeta.convertedFromAppID = legacyAppId; + } -const renderVisualPackageJson = (appInfo, appDefinition) => { const pkg = { - name: _.kebabCase(appInfo.title), - version: semver.inc(appDefinition.version, 'patch'), - description: appInfo.description, + name, + version, + description, main: 'index.js', scripts: { test: 'mocha --recursive -t 10000' @@ -134,14 +145,13 @@ const renderVisualPackageJson = (appInfo, appDefinition) => { node: `>=${LAMBDA_VERSION}`, npm: '>=5.6.0' }, - dependencies: { - [PLATFORM_PACKAGE]: appDefinition.platformVersion - }, + dependencies, devDependencies: { mocha: '^5.2.0', should: '^13.2.0' }, - private: true + private: true, + zapier: zapierMeta }; return prettifyJSON(pkg); @@ -418,10 +428,8 @@ const writeAuth = async (appDefinition, newAppDir) => { await createFile(content, 'authentication.js', newAppDir); }; -const writePackageJson = async (appInfo, appDefinition, newAppDir, legacy) => { - const content = legacy - ? await renderLegacyPackageJson(appInfo, appDefinition) - : renderVisualPackageJson(appInfo, appDefinition); +const writePackageJson = async (appInfo, appDefinition, newAppDir) => { + const content = await renderPackageJson(appInfo, appDefinition); await createFile(content, 'package.json', newAppDir); }; @@ -452,32 +460,21 @@ const writeGitIgnore = async newAppDir => { const srcPath = path.join(TEMPLATE_DIR, '/gitignore'); const destPath = path.join(newAppDir, '/.gitignore'); await copyFile(srcPath, destPath); - startSpinner('Writing .gitignore'); - endSpinner(); -}; - -const writeLegacyZapierAppRc = async newAppDir => { - const content = prettifyJSON({ - includeInBuild: ['scripting.js'] - }); - await createFile(content, '.zapierapprc', newAppDir); - startSpinner('Writing .zapierapprc'); - endSpinner(); }; -const writeVisualZapierAppRc = async (newAppDir, id) => { - const content = prettifyJSON({ - id - }); +const writeZapierAppRc = async (appInfo, appDefinition, newAppDir) => { + const json = {}; + if (appInfo.id) { + json.id = appInfo.id; + } + if (appDefinition.legacy) { + json.includeInBuild = ['scripting.js']; + } + const content = prettifyJSON(json); await createFile(content, '.zapierapprc', newAppDir); - startSpinner('Writing .zapierapprc'); - endSpinner(); }; -const convertApp = async (appInfo, appDefinition, newAppDir, opts = {}) => { - const defaultOpts = { legacy: true }; - const { legacy } = { ...defaultOpts, ...opts }; - +const convertApp = async (appInfo, appDefinition, newAppDir) => { if (IS_TESTING) { startSpinner = endSpinner = () => null; } @@ -486,8 +483,8 @@ const convertApp = async (appInfo, appDefinition, newAppDir, opts = {}) => { ['triggers', 'creates', 'searches'].forEach(stepType => { _.each(appDefinition[stepType], (definition, key) => { - promises.push(writeStep(stepType, definition, key, newAppDir)); promises.push( + writeStep(stepType, definition, key, newAppDir), writeStepTest(stepType, definition, key, appDefinition, newAppDir) ); }); @@ -503,24 +500,18 @@ const convertApp = async (appInfo, appDefinition, newAppDir, opts = {}) => { promises.push(writeScripting(appDefinition, newAppDir)); } - promises.push(writePackageJson(appInfo, appDefinition, newAppDir, legacy)); - promises.push(writeIndex(appDefinition, newAppDir)); - promises.push(writeEnvironment(appDefinition, newAppDir)); - promises.push(writeGitIgnore(newAppDir)); promises.push( - legacy - ? writeLegacyZapierAppRc(newAppDir) - : writeVisualZapierAppRc(newAppDir, appInfo.id) + writePackageJson(appInfo, appDefinition, newAppDir), + writeIndex(appDefinition, newAppDir), + writeEnvironment(appDefinition, newAppDir), + writeGitIgnore(newAppDir), + writeZapierAppRc(appInfo, appDefinition, newAppDir) ); return Promise.all(promises); }; -const convertVisualApp = _.partialRight(convertApp, { legacy: false }); -const convertLegacyApp = _.partialRight(convertApp, { legacy: true }); - module.exports = { renderTemplate, - convertLegacyApp, - convertVisualApp + convertApp }; From 5093a53437cd559082fdbf504ab26b3af022f0e1 Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Tue, 25 Feb 2020 10:28:10 +0800 Subject: [PATCH 2/2] Move legacy-scripting-runner package name to constants --- packages/cli/src/constants.js | 2 ++ packages/cli/src/utils/convert.js | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/constants.js b/packages/cli/src/constants.js index c53c678b2..c5fd01824 100644 --- a/packages/cli/src/constants.js +++ b/packages/cli/src/constants.js @@ -12,6 +12,7 @@ const AUTH_LOCATION = process.env.ZAPIER_AUTH_LOCATION || path.resolve(os.homedir(), '.zapierrc'); const CURRENT_APP_FILE = process.env.ZAPIER_CURRENT_APP_FILE || '.zapierapprc'; const PLATFORM_PACKAGE = 'zapier-platform-core'; +const LEGACY_RUNNER_PACKAGE = 'zapier-platform-legacy-scripting-runner'; const BUILD_DIR = 'build'; const DEFINITION_PATH = `${BUILD_DIR}/definition.json`; const BUILD_PATH = `${BUILD_DIR}/build.zip`; @@ -69,6 +70,7 @@ module.exports = { ENDPOINT, IS_TESTING, LAMBDA_VERSION, + LEGACY_RUNNER_PACKAGE, PACKAGE_NAME, PACKAGE_VERSION, PLATFORM_PACKAGE, diff --git a/packages/cli/src/utils/convert.js b/packages/cli/src/utils/convert.js index 6f4aedd9b..6f7b83364 100644 --- a/packages/cli/src/utils/convert.js +++ b/packages/cli/src/utils/convert.js @@ -8,6 +8,7 @@ const { PACKAGE_VERSION, PLATFORM_PACKAGE, LAMBDA_VERSION, + LEGACY_RUNNER_PACKAGE, IS_TESTING } = require('../constants'); const { copyFile, ensureDir, readFile, writeFile } = require('./files'); @@ -120,9 +121,8 @@ const renderPackageJson = async (appInfo, appDefinition) => { [PLATFORM_PACKAGE]: appDefinition.platformVersion }; if (appDefinition.legacy) { - const RUNNER_PACKAGE = 'zapier-platform-legacy-scripting-runner'; - const runnerVersion = await getPackageLatestVersion(RUNNER_PACKAGE); - dependencies[RUNNER_PACKAGE] = runnerVersion; + const runnerVersion = await getPackageLatestVersion(LEGACY_RUNNER_PACKAGE); + dependencies[LEGACY_RUNNER_PACKAGE] = runnerVersion; } const zapierMeta = {