From 2188f3f1dd64fa6ffa5e09da30ee194673c15753 Mon Sep 17 00:00:00 2001 From: Sam Harrison Date: Wed, 4 Sep 2019 09:43:36 -0500 Subject: [PATCH] Add 'target-account-sid' flag for transferring phone numbers Also adds a warning messages when a command contains conflicting flags/params. --- src/base-commands/twilio-api-command.js | 9 ++++- src/services/twilio-api/api-command-runner.js | 14 +++---- src/services/twilio-api/get-flag-config.js | 31 +++++++++++++-- test/base-commands/twilio-api-command.test.js | 39 +++++++++++++++++-- test/hooks/init/twilio-api.test.js | 4 +- 5 files changed, 81 insertions(+), 16 deletions(-) diff --git a/src/base-commands/twilio-api-command.js b/src/base-commands/twilio-api-command.js index ad0652cd6..9835a925f 100644 --- a/src/base-commands/twilio-api-command.js +++ b/src/base-commands/twilio-api-command.js @@ -60,6 +60,7 @@ TwilioApiCommand.flags = Object.assign( TwilioApiCommand.setUpNewCommandClass = NewCommandClass => { const resource = NewCommandClass.actionDefinition.resource; const action = NewCommandClass.actionDefinition.action; + const commandId = NewCommandClass.actionDefinition.topicName + ':' + NewCommandClass.actionDefinition.commandName; // Parameters let cmdFlags = {}; @@ -84,6 +85,12 @@ TwilioApiCommand.setUpNewCommandClass = NewCommandClass => { } if (flagType) { + // If the flag already exists, issue a warning. We're not equipped to + // handle such issues at the moment. + if (cmdFlags[flagConfig.name]) { + logger.warn(`The command "${commandId}" contains a conflicting flag: "--${flagConfig.name}"`); + } + cmdFlags[flagConfig.name] = flagType(flagConfig); } else { logger.error(`Unknown parameter type '${param.schema.type}' for parameter '${param.name}'`); @@ -107,7 +114,7 @@ TwilioApiCommand.setUpNewCommandClass = NewCommandClass => { } // Class statics - NewCommandClass.id = NewCommandClass.actionDefinition.topicName + ':' + NewCommandClass.actionDefinition.commandName; + NewCommandClass.id = commandId; NewCommandClass.args = []; NewCommandClass.flags = Object.assign(cmdFlags, TwilioApiCommand.flags); NewCommandClass.description = getActionDescription(NewCommandClass.actionDefinition); diff --git a/src/services/twilio-api/api-command-runner.js b/src/services/twilio-api/api-command-runner.js index 8948c3c78..b4236a1c1 100644 --- a/src/services/twilio-api/api-command-runner.js +++ b/src/services/twilio-api/api-command-runner.js @@ -7,7 +7,7 @@ const { TwilioCliError } = require('@twilio/cli-core').services.error; const { logger } = require('@twilio/cli-core').services.logging; const { TwilioApiFlags } = require('@twilio/cli-core').services.TwilioApi; const { validateSchema } = require('../api-schema/schema-validator'); -const { getFlagName } = require('./get-flag-config'); +const { getFlagConfig } = require('./get-flag-config'); class ApiCommandRunner { constructor(twilioClient, actionDefinition, flagDefinitions, flagValues) { @@ -73,12 +73,12 @@ class ApiCommandRunner { // Build the path and query params based on the parameters defined with the API action. actionParams.forEach(parameter => { const destination = (parameter.in === 'path' ? pathParams : queryParams); - this.addParameter(parameter.name, destination); + this.addParameter(parameter, destination); }); // Also add any API "snowflake" flags to the query params. Object.values(TwilioApiFlags).forEach(apiFlagName => { - this.addParameter(apiFlagName, queryParams); + this.addParameter({ name: apiFlagName }, queryParams); }); // TODO: Possible extender event: "beforeInvokeApi" @@ -93,11 +93,11 @@ class ApiCommandRunner { // TODO: Possible extender event: "afterInvokeApi" } - addParameter(parameterName, params) { - const cliFlagName = getFlagName(parameterName); + addParameter(parameter, params) { + const flag = getFlagConfig(parameter, this.actionDefinition); - if (doesObjectHaveProperty(this.flagValues, cliFlagName)) { - params[parameterName] = this.flagValues[cliFlagName]; + if (doesObjectHaveProperty(this.flagValues, flag.name)) { + params[parameter.name] = this.flagValues[flag.name]; } } } diff --git a/src/services/twilio-api/get-flag-config.js b/src/services/twilio-api/get-flag-config.js index bed53cdb3..394094f55 100644 --- a/src/services/twilio-api/get-flag-config.js +++ b/src/services/twilio-api/get-flag-config.js @@ -1,9 +1,16 @@ const { kebabCase } = require('@twilio/cli-core').services.namingConventions; -const { BASE_TOPIC_NAME } = require('./get-topic-name'); +const { TOPIC_SEPARATOR, BASE_TOPIC_NAME, CORE_TOPIC_NAME } = require('./get-topic-name'); // AccountSid is a special snowflake const ACCOUNT_SID_FLAG = 'AccountSid'; +const UPDATE_PHONE_NUMBER_COMMAND = [ + BASE_TOPIC_NAME, + CORE_TOPIC_NAME, + 'incoming-phone-numbers', + 'update' +].join(TOPIC_SEPARATOR); + const getFlagName = paramName => { return kebabCase( paramName @@ -16,14 +23,30 @@ const getFlagName = paramName => { * Converts a Twilio API parameter into a Twilio CLI flag. */ const getFlagConfig = (parameter, actionDefinition) => { + let flagName = getFlagName(parameter.name); + let flagDescription = parameter.description || ''; + + const commandId = [actionDefinition.topicName, actionDefinition.commandName].join(TOPIC_SEPARATOR); const isCoreAccountSidFlag = (parameter.name === ACCOUNT_SID_FLAG && actionDefinition.domainName === BASE_TOPIC_NAME); + if (isCoreAccountSidFlag && commandId === UPDATE_PHONE_NUMBER_COMMAND) { + if (parameter.in === 'query') { + // Turn 'account-sid' into 'target-account-sid' + flagName = 'target-' + flagName; + // Converting something like this: + // "The SID of the Account that created the IncomingPhoneNumber resource to update." + // to: + // "The SID of the Account that you wish to own the number." + flagDescription = flagDescription.replace(/that created.*?\./, 'that you wish to own the number.'); + } + } + return { - name: getFlagName(parameter.name), - description: parameter.description, + name: flagName, + description: flagDescription, // AccountSid on 'api:core' not required, we can get from the current profile required: isCoreAccountSidFlag ? false : parameter.required, - multiple: parameter.schema.type === 'array', + multiple: parameter.schema && parameter.schema.type === 'array', // Allow negated booleans ('-no' option) allowNo: true }; diff --git a/test/base-commands/twilio-api-command.test.js b/test/base-commands/twilio-api-command.test.js index ee5ce7153..3247e4fdb 100644 --- a/test/base-commands/twilio-api-command.test.js +++ b/test/base-commands/twilio-api-command.test.js @@ -43,10 +43,24 @@ describe('base-commands', () => { } }; + const numberUpdateActionDefinition = { + domainName: 'api', + commandName: 'update', + path: '/2010-04-01/Accounts/{AccountSid}/IncomingPhoneNumbers/{Sid}.json', + actionName: 'update', + action: { + parameters: [ + { name: 'Sid', in: 'path', schema: { type: 'string' } }, + { name: 'AccountSid', in: 'path', schema: { type: 'string' } }, + { name: 'AccountSid', in: 'query', schema: { type: 'string' } } + ] + } + }; + const getCommandClass = (actionDefinition = callCreateActionDefinition) => { const NewCommandClass = class extends TwilioApiCommand { }; NewCommandClass.actionDefinition = actionDefinition; - NewCommandClass.actionDefinition.topicName = getTopicName(NewCommandClass.actionDefinition); + NewCommandClass.actionDefinition.topicName = 'api:' + getTopicName(NewCommandClass.actionDefinition); TwilioApiCommand.setUpNewCommandClass(NewCommandClass); return NewCommandClass; @@ -55,7 +69,7 @@ describe('base-commands', () => { test.it('setUpNewCommandClass', () => { const NewCommandClass = getCommandClass(); - expect(NewCommandClass.id).to.equal('core:calls:create'); + expect(NewCommandClass.id).to.equal('api:core:calls:create'); expect(NewCommandClass.description).to.equal(fakeResource.actions.create.description); expect(NewCommandClass.load()).to.equal(NewCommandClass); @@ -95,7 +109,7 @@ describe('base-commands', () => { test.it('handles remove action', () => { const NewCommandClass = getCommandClass(callRemoveActionDefinition); - expect(NewCommandClass.id).to.equal('core:calls:remove'); + expect(NewCommandClass.id).to.equal('api:core:calls:remove'); expect(NewCommandClass.flags.properties).to.be.undefined; }); @@ -103,6 +117,7 @@ describe('base-commands', () => { .twilioFakeProfile(ConfigData) .twilioCliEnv(Config) .stdout() + .stderr() .twilioCreateCommand(getCommandClass(), [ '--from', '+15555555555', '--to', '+14155555555', @@ -155,6 +170,23 @@ describe('base-commands', () => { expect(ctx.stderr).to.contain('success'); }); + test + .twilioFakeProfile(ConfigData) + .twilioCliEnv(Config) + .stderr() + .twilioCreateCommand(getCommandClass(numberUpdateActionDefinition), [ + '--sid', fakeCallResponse.sid, + '--target-account-sid', 'AC123' + ]) + .do(ctx => { + ctx.testCmd.twilioApi = { update: sinon.stub().returns({}) }; + return ctx.testCmd.run(); + }) + .it('updates a call', ctx => { + const callOptions = ctx.testCmd.twilioApi.update.firstCall.args[0]; + expect(callOptions.data).to.include({ AccountSid: 'AC123' }); + }); + test .twilioFakeProfile(ConfigData) .twilioCliEnv(Config) @@ -174,6 +206,7 @@ describe('base-commands', () => { .twilioFakeProfile(ConfigData) .twilioCliEnv(Config) .stdout() + .stderr() .twilioCreateCommand(getCommandClass(), [ '--skip-parameter-validation', '--from', '+15555555555', diff --git a/test/hooks/init/twilio-api.test.js b/test/hooks/init/twilio-api.test.js index 1ab20fbd7..4d0cdefce 100644 --- a/test/hooks/init/twilio-api.test.js +++ b/test/hooks/init/twilio-api.test.js @@ -6,10 +6,12 @@ const getFakeConfig = () => ({ plugins: [] }); describe('hooks', () => { describe('init', () => { describe('twilio-api', () => { - test.it('provides multiple resources and actions', ctx => { + test.stderr().it('provides multiple resources and actions', ctx => { ctx.config = getFakeConfig(); pluginFunc.call(ctx); + expect(ctx.stderr).to.be.empty; // Think conflicting command flag warnings + const plugin = ctx.config.plugins[0]; expect(plugin.hooks).to.eql({}); // eql is for comparing objects (== instead of ===)