Skip to content
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

fix: remove -o conflict, add short char UTs, add short chars to NUTs #821

Merged
merged 1 commit into from
Apr 18, 2023
Merged

fix: remove -o conflict, add short char UTs, add short chars to NUTs #821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions messages/deployCommand.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,11 @@ Deploying metadata to %s using the v%s %s API.
# apiVersionMsgDetailed

%s v%s metadata to %s using the v%s %s API.

# missingUsername

Unable to determine the username of the org to delete. Specify the username with the --target-org | -u flag.

# flags.targetusername.summary

Username or alias of the target org.
14 changes: 10 additions & 4 deletions src/commands/force/mdapi/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ import {
Flags,
loglevel,
orgApiVersionFlagWithDeprecations,
requiredOrgFlagWithDeprecations,
Ux,
} from '@salesforce/sf-plugins-core';
import { Interfaces } from '@oclif/core';
import { DeployCommand, getCoverageFormattersOptions, reportsFormatters, TestLevel } from '../../../deployCommand';
import {
DeployCommand,
getCoverageFormattersOptions,
reportsFormatters,
targetUsernameFlag,
TestLevel,
} from '../../../deployCommand';
import { DeployCommandAsyncResult } from '../../../formatters/source/deployAsyncResultFormatter';
import { MdDeployResult, MdDeployResultFormatter } from '../../../formatters/mdapi/mdDeployResultFormatter';
import { ProgressFormatter } from '../../../formatters/progressFormatter';
Expand Down Expand Up @@ -44,7 +49,7 @@ export class Deploy extends DeployCommand {
public static readonly flags = {
'api-version': orgApiVersionFlagWithDeprecations,
loglevel,
'target-org': requiredOrgFlagWithDeprecations,
'target-org': targetUsernameFlag,
checkonly: Flags.boolean({
char: 'c',
description: messages.getMessage('flags.checkOnly.description'),
Expand Down Expand Up @@ -75,6 +80,7 @@ export class Deploy extends DeployCommand {
summary: messages.getMessage('flags.runTests.summary'),
}),
ignoreerrors: Flags.boolean({
// break this
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this comment mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leftover - will remove

char: 'o',
description: messages.getMessage('flags.ignoreErrors.description'),
summary: messages.getMessage('flags.ignoreErrors.summary'),
Expand Down Expand Up @@ -134,7 +140,7 @@ export class Deploy extends DeployCommand {

public async run(): Promise<DeployResult> {
this.flags = (await this.parse(Deploy)).flags;
this.org = this.flags['target-org'];
this.org = await Org.create({ aliasOrUsername: this.flags['target-org'] });
await this.deploy();
this.resolveSuccess();
return this.formatResult();
Expand Down
14 changes: 10 additions & 4 deletions src/commands/force/source/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@ import {
Flags,
loglevel,
orgApiVersionFlagWithDeprecations,
requiredOrgFlagWithDeprecations,
Ux,
} from '@salesforce/sf-plugins-core';
import { Interfaces } from '@oclif/core';
import { DeployCommand, getCoverageFormattersOptions, reportsFormatters, TestLevel } from '../../../deployCommand';
import {
DeployCommand,
getCoverageFormattersOptions,
reportsFormatters,
targetUsernameFlag,
TestLevel,
} from '../../../deployCommand';
import { DeployCommandResult, DeployResultFormatter } from '../../../formatters/deployResultFormatter';
import {
DeployAsyncResultFormatter,
Expand Down Expand Up @@ -53,7 +58,7 @@ export class Deploy extends DeployCommand {
public static readonly flags = {
'api-version': orgApiVersionFlagWithDeprecations,
loglevel,
'target-org': requiredOrgFlagWithDeprecations,
'target-org': targetUsernameFlag,
checkonly: Flags.boolean({
char: 'c',
description: messages.getMessage('flags.checkonly.description'),
Expand Down Expand Up @@ -83,6 +88,7 @@ export class Deploy extends DeployCommand {
}),
ignoreerrors: Flags.boolean({
char: 'o',
// brfeak this
description: messages.getMessage('flags.ignoreErrors.description'),
summary: messages.getMessage('flags.ignoreErrors.summary'),
}),
Expand Down Expand Up @@ -159,7 +165,7 @@ export class Deploy extends DeployCommand {
private org: Org;
public async run(): Promise<DeployCommandCombinedResult> {
this.flags = (await this.parse(Deploy)).flags;
this.org = this.flags['target-org'];
this.org = await Org.create({ aliasOrUsername: this.flags['target-org'] });
await this.preChecks();
await this.deploy();
this.resolveSuccess();
Expand Down
37 changes: 36 additions & 1 deletion src/deployCommand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,17 @@ import {
MetadataApiDeployStatus,
RequestStatus,
} from '@salesforce/source-deploy-retrieve';
import { Connection, Messages, Org, PollingClient, SfdxPropertyKeys, SfError, StatusResult } from '@salesforce/core';
import {
ConfigAggregator,
Connection,
Messages,
Org,
PollingClient,
SfdxPropertyKeys,
SfError,
StateAggregator,
StatusResult,
} from '@salesforce/core';
import { AnyJson, getBoolean, isString } from '@salesforce/ts-types';
import { Duration, once, ensureArray } from '@salesforce/kit';
import {
Expand All @@ -25,6 +35,7 @@ import {
DefaultReportOptions,
JUnitReporter,
} from '@salesforce/apex-node';
import { Flags } from '@salesforce/sf-plugins-core';
import { SourceCommand } from './sourceCommand';
import { DeployData, Stash } from './stash';
import { transformCoverageToApexCoverage, transformDeployTestsResultsToTestResult } from './coverageUtils';
Expand Down Expand Up @@ -53,6 +64,7 @@ export abstract class DeployCommand extends SourceCommand {
const stashKey = Stash.getKey(this.id);
Stash.set(stashKey, { jobid: id });
});

/**
* Request a report of an in-progress or completed deployment.
*
Expand Down Expand Up @@ -259,3 +271,26 @@ export const getCoverageFormattersOptions = (formatters: string[] = []): Coverag
reportOptions,
};
};

export const targetUsernameFlag = Flags.string({
Copy link
Contributor

Choose a reason for hiding this comment

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

Having target-org with short char other than 'o' break sf guidelines. I ran into short char collision in plugin-user and to avoid the collision we ended up retaining the old commands that used the -o and created new ones that allows style adherence.

required: true,
char: 'u',
deprecateAliases: true,
// DO NOT alias to 'o', it will conflict with '--ignoreerrors'
aliases: ['targetusername', 'u'],
summary: messages.getMessage('flags.targetusername.summary'),
parse: async (input: string | undefined) => resolveUsername(input),
default: async () => resolveUsername(),
defaultHelp: async () => resolveUsername(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct for defaultHelp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is similar to what we did in plugin-org

https://github.com/salesforcecli/plugin-org/pull/646/files#diff-2cb023071675d0d3c17b86b021f81fd33622048a9eab2fb3d6623e52375f2690R38

Copy link
Contributor

Choose a reason for hiding this comment

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

This form can leak customer data into the manifest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peternhale ^

});

export const resolveUsername = async (usernameOrAlias?: string): Promise<string> => {
const stateAggregator = await StateAggregator.getInstance();
// we have a value, but don't know if it's a username or an alias
if (usernameOrAlias) return stateAggregator.aliases.resolveUsername(usernameOrAlias);
// we didn't get a value, so let's see if the config has a default target org
const configAggregator = await ConfigAggregator.create();
const defaultUsernameOrAlias: string = configAggregator.getPropertyValue('target-org');
if (defaultUsernameOrAlias) return stateAggregator.aliases.resolveUsername(defaultUsernameOrAlias);
throw new SfError(messages.getMessage('missingUsername'), 'MissingUsernameError');
};
Loading