Skip to content

Commit

Permalink
Add 'target-account-sid' flag for transferring phone numbers (#115)
Browse files Browse the repository at this point in the history
Also adds a warning messages when a command contains conflicting flags/params.
  • Loading branch information
childish-sambino authored Sep 5, 2019
1 parent 3b91b16 commit 3bc2238
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 16 deletions.
9 changes: 8 additions & 1 deletion src/base-commands/twilio-api-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand All @@ -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}'`);
Expand All @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions src/services/twilio-api/api-command-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"
Expand All @@ -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];
}
}
}
Expand Down
31 changes: 27 additions & 4 deletions src/services/twilio-api/get-flag-config.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
};
Expand Down
39 changes: 36 additions & 3 deletions test/base-commands/twilio-api-command.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -95,14 +109,15 @@ 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;
});

test
.twilioFakeProfile(ConfigData)
.twilioCliEnv(Config)
.stdout()
.stderr()
.twilioCreateCommand(getCommandClass(), [
'--from', '+15555555555',
'--to', '+14155555555',
Expand Down Expand Up @@ -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)
Expand All @@ -174,6 +206,7 @@ describe('base-commands', () => {
.twilioFakeProfile(ConfigData)
.twilioCliEnv(Config)
.stdout()
.stderr()
.twilioCreateCommand(getCommandClass(), [
'--skip-parameter-validation',
'--from', '+15555555555',
Expand Down
4 changes: 3 additions & 1 deletion test/hooks/init/twilio-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ===)
Expand Down

0 comments on commit 3bc2238

Please sign in to comment.