-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add --json to compute add collaborator #469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -16,8 +16,6 @@ Messages.importMessagesDirectory(__dirname); | |||
const messages = Messages.loadMessages('@salesforce/plugin-functions', 'env.compute.collaborator.add'); | ||||
|
||||
export default class ComputeCollaboratorAdd extends Command { | ||||
static enableJsonFlag = false; | ||||
|
||||
static summary = messages.getMessage('summary'); | ||||
|
||||
static examples = messages.getMessages('examples'); | ||||
|
@@ -64,7 +62,7 @@ export default class ComputeCollaboratorAdd extends Command { | |||
} | ||||
|
||||
if (error.message?.includes('404')) { | ||||
jheikes515 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
this.error(`${herokuColor.heroku(herokuUser)} does not exist.`); | ||||
this.error(`Couldn't find Heroku user ${herokuColor.heroku(herokuUser)}.`); | ||||
jheikes515 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
} | ||||
|
||||
this.error(error.message); | ||||
|
@@ -74,5 +72,8 @@ export default class ComputeCollaboratorAdd extends Command { | |||
this.log( | ||||
'For more information about attaching Heroku add-ons to your compute environments, run $ heroku addons:attach --help.' | ||||
); | ||||
return { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the user includes the --json flag, the error output should be in json also. Try adding in:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example here
|
||||
added: [`${herokuUser}`], | ||||
}; | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,14 @@ import herokuColor from '@heroku-cli/color'; | |
|
||
const HEROKU_USER = '[email protected]'; | ||
|
||
const jsonSuccess = { | ||
status: 0, | ||
result: { | ||
added: [HEROKU_USER], | ||
}, | ||
warnings: [], | ||
}; | ||
|
||
describe('sf env compute collaborator add', () => { | ||
test | ||
.stdout() | ||
|
@@ -33,13 +41,25 @@ describe('sf env compute collaborator add', () => { | |
); | ||
}); | ||
|
||
test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add in a failing testing for the --json flag |
||
.stdout() | ||
.stderr() | ||
.nock('https://api.heroku.com', (api) => api.post('/salesforce-orgs/collaborators').reply(200, {})) | ||
.command(['env:compute:collaborator:add', '-h', HEROKU_USER, '--json']) | ||
jheikes515 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.it('will show json output with success', (ctx) => { | ||
const succJSON = JSON.parse(ctx.stdout); | ||
|
||
expect(succJSON.status).to.deep.equal(jsonSuccess.status); | ||
expect(succJSON.result).to.eql(jsonSuccess.result); | ||
}); | ||
|
||
test | ||
.stdout() | ||
.stderr() | ||
.nock('https://api.heroku.com', (api) => api.post('/salesforce-orgs/collaborators').reply(404, {})) | ||
.command(['env:compute:collaborator:add', '-h', HEROKU_USER]) | ||
jheikes515 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.catch((error) => { | ||
expect(error.message).contains(`${herokuColor.heroku(HEROKU_USER)} does not exist.`); | ||
expect(error.message).contains(`Couldn't find Heroku user ${herokuColor.heroku(HEROKU_USER)}.`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See previous comment about this change. I'm not sure it's needed. |
||
}) | ||
.it('alerts user if user entered does not exist'); | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.