Skip to content

fix: remove short flag -j for json and return response to global json handler #465

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jul 20, 2022
Merged

fix: remove short flag -j for json and return response to global json handler #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8c723ee
fix: remove short flag -j for json
RasPhilCo Jul 19, 2022
74aa6ff
fix: rm json from flag builder
RasPhilCo Jul 19, 2022
113e729
fix: return value to json handler
RasPhilCo Jul 20, 2022
a1d97be
refactor: env var set error handling (#463)
RasPhilCo Jul 20, 2022
fc3591c
fix: comments
RasPhilCo Jul 20, 2022
4532791
fix: return string not empty array
RasPhilCo Jul 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions src/commands/deploy/functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export default class DeployFunctions extends Command {
description: messages.getMessage('flags.quiet.summary'),
char: 'q',
}),
json: FunctionsFlagBuilder.json,
};

async run() {
Expand Down Expand Up @@ -188,16 +187,9 @@ export default class DeployFunctions extends Command {
cli.action.stop();

if (shouldExitNonZero) {
cli.exit(1);
}
if (flags.json) {
cli.styledJSON({
status: 0,
result: {
results,
},
warnings: [],
});
this.exit(1);
}

return results;
}
}
2 changes: 2 additions & 0 deletions src/commands/env/compute/collaborator/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ 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');
Expand Down
43 changes: 15 additions & 28 deletions src/commands/env/create/compute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export default class EnvCreateCompute extends Command {
char: 'a',
description: messages.getMessage('flags.alias.summary'),
}),
json: FunctionsFlagBuilder.json,
};

async run() {
Expand All @@ -64,9 +63,7 @@ export default class EnvCreateCompute extends Command {
)} to the "features" list in your scratch org definition JSON file, e.g. "features": ["Functions"]`
);
}
if (!flags.json) {
cli.action.start(`Creating compute environment for org ID ${orgId}`);
}
Comment on lines -67 to -69
Copy link
Contributor

@RodEsp RodEsp Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you QA this?
I'm not sure that cli.action.start() has the same --json checks that this.log() does.

Copy link
Contributor Author

@RasPhilCo RasPhilCo Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actions go out on stderr, so this shouldn't matter.

Copy link
Contributor Author

@RasPhilCo RasPhilCo Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"He says 99% sure."

Copy link
Contributor Author

@RasPhilCo RasPhilCo Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/oclif/cli-ux/blob/21e8525497e6b4e9b4fa6cc1e2006bae2ff402eb/src/action/base.ts#L29

$ ./bin/dev logout functions --json 1>/dev/null
Logging out of Salesforce Functions... done

$ ./bin/dev logout functions --json 2>/dev/null
{
  "status": 0,
  "result": "Logged out",
  "warnings": []
}

cli.action.start(`Creating compute environment for org ID ${orgId}`);
const project = await fetchSfdxProject();
const projectName = project.name;

Expand Down Expand Up @@ -140,25 +137,21 @@ export default class EnvCreateCompute extends Command {

cli.action.stop();

if (!flags.json) {
this.log(`New compute environment created with ID ${app.name}`);
this.log(`New compute environment created with ID ${app.name}`);

cli.action.start('Connecting environments');
}
cli.action.start('Connecting environments');

if (alias) {
this.stateAggregator.aliases.set(alias, app.id!);
await this.stateAggregator.aliases.write();
}

cli.action.stop();
if (!flags.json) {
this.log(
alias
? `Your compute environment with local alias ${herokuColor.cyan(alias)} is ready.`
: 'Your compute environment is ready.'
);
}
this.log(
alias
? `Your compute environment with local alias ${herokuColor.cyan(alias)} is ready.`
: 'Your compute environment is ready.'
);
} catch (err) {
const DUPLICATE_PROJECT_MESSAGE =
'There is already a project with the same name in the same namespace for this org';
Expand All @@ -181,18 +174,12 @@ export default class EnvCreateCompute extends Command {
this.error(`${error.data.message}`);
}
const app = await fetchAppForProject(this.client, projectName, org.getUsername());
if (flags.json) {
cli.styledJSON({
status: 0,
result: {
alias,
projectName,
connectedOrgAlias: '',
connectedOrgId: orgId,
computeEnvironmentName: app.name,
},
warnings: [],
});
}
return {
alias,
projectName,
connectedOrgAlias: '',
connectedOrgId: orgId,
computeEnvironmentName: app.name,
};
}
}
11 changes: 2 additions & 9 deletions src/commands/env/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export default class EnvDelete extends Command {
hidden: true,
}),
confirm: confirmationFlag,
json: FunctionsFlagBuilder.json,
};

async run() {
Expand All @@ -59,7 +58,7 @@ export default class EnvDelete extends Command {
}

if (flags.environment) {
cli.warn(messages.getMessage('flags.environment.deprecation'));
this.warn(messages.getMessage('flags.environment.deprecation'));
}

await this.confirmRemovePrompt('environment', targetCompute, flags.confirm);
Expand Down Expand Up @@ -158,12 +157,6 @@ export default class EnvDelete extends Command {

cli.action.stop();

if (flags.json) {
cli.styledJSON({
status: 0,
result: 'Environment deleted.',
warnings: [],
});
}
return 'Environment deleted.';
}
}
4 changes: 3 additions & 1 deletion src/commands/env/log.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const messages = Messages.loadMessages('@salesforce/plugin-functions', 'env.log'

export default class Log extends Command {
public static readonly state = 'beta';
static enableJsonFlag = false;

static summary = messages.getMessage('summary');

static examples = messages.getMessages('examples');
Expand Down Expand Up @@ -54,7 +56,7 @@ export default class Log extends Command {
}

if (flags.environment) {
cli.warn(messages.getMessage('flags.environment.deprecation'));
this.warn(messages.getMessage('flags.environment.deprecation'));
}

const appName = await resolveAppNameForEnvironment(targetCompute);
Expand Down
4 changes: 3 additions & 1 deletion src/commands/env/log/tail.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/plugin-functions', 'env.log.tail');

export default class LogTail extends Command {
static enableJsonFlag = false;

static summary = messages.getMessage('summary');

static examples = messages.getMessages('examples');
Expand Down Expand Up @@ -48,7 +50,7 @@ export default class LogTail extends Command {
}

if (flags.environment) {
cli.warn(messages.getMessage('flags.environment.deprecation'));
this.warn(messages.getMessage('flags.environment.deprecation'));
}

const appName = await resolveAppNameForEnvironment(targetCompute);
Expand Down
49 changes: 21 additions & 28 deletions src/commands/env/logdrain/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export default class LogDrainAdd extends Command {
description: messages.getMessage('flags.drain-url.summary'),
hidden: true,
}),
json: FunctionsFlagBuilder.json,
};

async run() {
Expand Down Expand Up @@ -71,58 +70,52 @@ export default class LogDrainAdd extends Command {
}

if (flags.environment) {
cli.warn(messages.getMessage('flags.environment.deprecation'));
this.warn(messages.getMessage('flags.environment.deprecation'));
}

if (flags.url) {
cli.warn(messages.getMessage('flags.url.deprecation'));
this.warn(messages.getMessage('flags.url.deprecation'));
}
const appName = await resolveAppNameForEnvironment(targetCompute);

try {
cli.action.start(`Creating drain for environment ${herokuColor.app(targetCompute)}`);

const result = await this.client.post<Heroku.LogDrain>(`/apps/${appName}/log-drains`, {
data: {
url,
},
});

if (flags.json) {
cli.styledJSON({
status: 0,
result: [
{
addon: null,
created_at: result.data.created_at,
id: result.data.id,
token: result.data.token,
updated_at: result.data.updated_at,
url: result.data.url,
},
],
warnings: [],
});
} else {
cli.action.start(`Creating drain for environment ${herokuColor.app(targetCompute)}`);

cli.action.stop();
}
cli.action.stop();

return [
{
addon: null,
created_at: result.data.created_at,
id: result.data.id,
token: result.data.token,
updated_at: result.data.updated_at,
url: result.data.url,
},
];
} catch (e) {
const error = e as { data: { message?: string } };

if (error.data?.message?.includes('Url is invalid')) {
this.error(new Error(`URL is invalid <${url}>`));
this.error(new Error(`URL is invalid ${url}`));
}

if (error.data?.message?.includes('Url has already been taken')) {
this.error(new Error(`Logdrain URL is already added <${url}>`));
this.error(new Error(`Logdrain URL is already added ${url}`));
}

if (error.data?.message?.includes("Couldn't find that app.")) {
this.error(new Error(`Could not find environment <${appName}>`));
if (error.data?.message?.includes("Couldn't find that app")) {
this.error(new Error(`Could not find environment ${appName}`));
}

if (error.data?.message?.includes("You've reached the limit")) {
this.error(new Error(`You've reached the limit of 5 log drains on <${appName}>`));
this.error(new Error(`You've reached the limit of 5 log drains on ${appName}`));
}

if (error.data?.message?.includes('401')) {
Expand Down
28 changes: 5 additions & 23 deletions src/commands/env/logdrain/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export default class LogDrainList extends Command {
exclusive: ['target-compute'],
hidden: true,
}),
json: FunctionsFlagBuilder.json,
};

async run() {
Expand All @@ -49,35 +48,16 @@ export default class LogDrainList extends Command {
}

if (flags.environment) {
cli.warn(messages.getMessage('flags.environment.deprecation'));
this.warn(messages.getMessage('flags.environment.deprecation'));
}

const appName = await resolveAppNameForEnvironment(targetCompute);

const { data: drains } = await this.client.get<Heroku.LogDrain[]>(`/apps/${appName}/log-drains`);

if (flags.json) {
if (drains.length === 0) {
cli.styledJSON({
status: 0,
result: [],
warnings: [`No logdrain found for environment <${appName}>`],
});
return;
}

cli.styledJSON({
status: 0,
result: drains,
warnings: [],
});
return;
if (drains.length === 0) {
this.warn(`No log-drains found for environment ${targetCompute}`);
} else {
if (drains.length === 0) {
this.log(`No log drains found for environment ${targetCompute}.`);
return;
}

cli.table<Heroku.LogDrain>(
drains,
{
Expand All @@ -96,5 +76,7 @@ export default class LogDrainList extends Command {
}
);
}

return drains;
}
}
29 changes: 11 additions & 18 deletions src/commands/env/logdrain/remove.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export default class LogDrainRemove extends Command {
description: messages.getMessage('flags.drain-url.summary'),
hidden: true,
}),
json: FunctionsFlagBuilder.json,
};

async run() {
Expand All @@ -63,41 +62,35 @@ export default class LogDrainRemove extends Command {
}

if (!url) {
this.error(new Error('Missing required flag: -u, --url Logdrain url to remove'));
this.error(new Error('Missing required flag: -l, --drain-url Logdrain url to remove'));
}

if (flags.environment) {
cli.warn(messages.getMessage('flags.environment.deprecation'));
this.warn(messages.getMessage('flags.environment.deprecation'));
}

if (flags.url) {
cli.warn(messages.getMessage('flags.url.deprecation'));
this.warn(messages.getMessage('flags.url.deprecation'));
}

const appName = await resolveAppNameForEnvironment(targetCompute);
try {
cli.action.start(`Deleting drain for environment ${herokuColor.app(targetCompute)}`);

await this.client.delete<Heroku.LogDrain>(`/apps/${appName}/log-drains/${encodeURIComponent(url)}`);
if (flags.json) {
cli.styledJSON({
status: 0,
result: null,
warnings: [],
});
return;
} else {
cli.action.start(`Deleting drain for environment ${herokuColor.app(targetCompute)}`);

cli.action.stop();
}

cli.action.stop();

return 'Removed drain-url';
} catch (e) {
const error = e as { data: { message?: string } };

if (error.data?.message?.includes('Url is invalid')) {
this.error(new Error(`URL is invalid <${url}>`));
this.error(new Error(`URL is invalid ${url}`));
}

if (error.data?.message?.includes("Couldn't find that app.")) {
this.error(new Error(`Couldn't find that app <${appName}>`));
this.error(new Error(`Couldn't find that app ${appName}`));
}

this.error(e as Error);
Expand Down
Loading