Flagfeature #1237

soridalac · 2024-10-14T21:05:54Z

What does this PR do?

feat: update flags and fields in definition file for cloning existing sandbox.

What issues does this PR fix or reference?

@W-16883274@
forcedotcom/cli#3036

jshackell-sfdc

see my suggestions and a question!

jshackell-sfdc · 2024-10-14T22:39:47Z

messages/clone.md

+
+# error.sandboxNameQueryFailed
+
+Unable to find the ID of the sandboxName "%s" that's defined in the definition file.


Suggested change

Unable to find the ID of the sandboxName "%s" that's defined in the definition file.

Unable to find the ID of the sandbox "%s" that's defined in the definition file.

jshackell-sfdc · 2024-10-14T22:42:26Z

messages/create.sandbox.md

-The value of --clone must be an existing sandbox. The existing sandbox, and the new sandbox specified with the --name flag, must both be associated with the production org (--target-org) that contains the sandbox licenses.
+The value of --source-sandbox-name must be an existing sandbox. The existing sandbox, and the new sandbox specified with the --name flag, must both be associated with the production org (--target-org) that contains the sandbox licenses.
+
+You can specify either --source-sandbox-name or --source-id for clone an existing sandbox but not both.


Suggested change

You can specify either --source-sandbox-name or --source-id for clone an existing sandbox but not both.

You can specify either --source-sandbox-name or --source-id when cloning an existing sandbox, but not both.

jshackell-sfdc · 2024-10-14T22:43:21Z

messages/create.sandbox.md

+
+The value of --source-id must be an existing sandbox. The existing sandbox, and the new sandbox specified with the --name flag, must both be associated with the production org (--target-org) that contains the sandbox licenses.
+
+You can specify either --source-sandbox-name or --source-id for clone an existing sandbox but not both.


Suggested change

You can specify either --source-sandbox-name or --source-id for clone an existing sandbox but not both.

You can specify either --source-sandbox-name or --source-id when cloning an existing sandbox, but not both.

jshackell-sfdc · 2024-10-14T22:44:28Z

messages/create.sandbox.md

@@ -120,6 +132,14 @@ The sandbox request configuration isn't acceptable.

 The poll interval (%d seconds) can't be larger that wait (%d in seconds)

-# error.noCloneSource
+# error.noCloneSourceName

 Could not find the clone sandbox name "%s" in the target org.


Suggested change

Could not find the clone sandbox name "%s" in the target org.

Couldn't find the clone sandbox name "%s" in the target org.

jshackell-sfdc · 2024-10-14T22:44:43Z

messages/create.sandbox.md

+
+# error.noCloneSourceId
+
+Could not find the clone sandbox id "%s" in the target org.


Suggested change

Could not find the clone sandbox id "%s" in the target org.

Couldn't find the clone sandbox ID "%s" in the target org.

jshackell-sfdc · 2024-10-14T22:52:02Z

messages/create.sandbox.md

+
+# error.cloneFlagsConflict
+
+You can not use clone Name or Id flags and clone Name or Id fields in the definition file at the same time.


I'm not sure I understand what this error message means. Can you elaborate? Thanks!

Based on our DMs, it sounds like this error occurs if users try to use --source-sandbox-name AND --definition-file AND the definition file contains SourceSandboxName. Ditto for the ID equivalents. And I guess this isn't allowed? If this is indeed the situation, here's my edit of the error:

You can't specify both the --source-sandbox-name and --definition-file flags, and also include the "SourceSandboxName" option in the definition file. Same with the --source-id flag and "SourceId" option. Pick one method of cloning a sandbox and try again.

cristiand391 · 2024-10-15T18:12:12Z

src/commands/force/org/create.ts

@@ -211,7 +211,7 @@ export class Create extends SfCommand<CreateResult> {
    });

    const { sandboxReq } = await requestFunctions.createSandboxRequest(
-      false,
+      // false,


remove this comment

cristiand391 · 2024-10-15T18:13:33Z

src/commands/org/create/sandbox.ts

@@ -20,13 +20,15 @@ const getLicenseTypes = (): string[] => Object.values(SandboxLicenseType);

 type SandboxConfirmData = SandboxRequest & { CloneSource?: string };

+// eslint-disable-next-line sf-plugin/only-extend-SfCommand


were these causing errors? if not, let's re-enable them 👍🏼 (warnings reminds us we have to do it anyway)

cristiand391 · 2024-10-15T18:15:40Z

src/commands/org/create/sandbox.ts

@@ -79,18 +81,25 @@ export default class CreateSandbox extends SandboxCommandBase<SandboxCommandResp
        return Promise.resolve(name);
      },
    }),
-    clone: Flags.string({
+    'source-sandbox-name': Flags.string({
      char: 'c',


the short flag -c should also be a deprecated alias.

cristiand391 · 2024-10-15T18:23:34Z

src/commands/org/create/sandbox.ts

+      deprecateAliases: true,
+      aliases: ['clone'],
+    }),
+    'source-id': Flags.string({


this can be a salesforceId flag, example:

plugin-org/src/commands/org/create/scratch.ts

Lines 148 to 155 in 0e7d3a7

'source-org': Flags.salesforceId({

summary: messages.getMessage('flags.source-org.summary'),

startsWith: '00D',

length: 15,

helpGroup: definitionFileHelpGroupName,

// salesforceId flag has `i` and that would be a conflict with client-id

char: undefined,

}),

I don't see the API mentions sandbox IDs start with a particular prefix so we can omit the startWith option.

Flags.salesforceId will validate that the ID length is 15 or 18.

cristiand391 · 2024-10-15T18:40:11Z

src/commands/org/create/sandbox.ts

+        ? { SourceId: await requestFunctions.getSrcIdByName(this.flags['target-org'].getConnection(), srcSandboxName) }
+        : {}),
+      ...(srcId ? { SourceId: srcId } : {}),
+      ...(this.flags['source-id'] ? { SourceId: await this.getSourceId() } : {}),


No need to check for flags and props, on L140 we already pass the source flags as requestOptions to requestFunctions.createSandboxRequest, srcSandboxName and srcId should have the flag/def. file values.

cristiand391 · 2024-10-15T18:43:11Z

src/commands/org/create/sandbox.ts

+        ? { CloneSource: this.flags['source-sandbox-name'] }
+        : this.flags['source-id']
+        ? { CloneSource: this.flags['source-id'] }
+        : {}),


CloneSource will only be shown if when using the source flags, what if I'm specifying the ID in the def. file instead?

Maybe CloneSource should always show the source ID (you can grab it from sandboxReq.SourceId, so we don't need to check flag/def. file values here.

@soridalac this is still unresolved ⬆️
we can ignore --source-sandbox-name, just check for sandboxReq.SourceId

cristiand391 · 2024-10-15T18:51:19Z

src/commands/org/create/sandbox.ts

+    const sourceSandboxNameInDefFile = !!defFileContent.SourceSandboxName;
+
+    if ((sourceIdFlag || sourceSandboxNameFlag) && (sourceIdInDefFile || sourceSandboxNameInDefFile)) {
+      throw messages.createError('error.cloneFlagsConflict');


these validations don't require any specific context other than reading the def.file, let's move these to each source flag, like this:

plugin-org/src/commands/org/create/sandbox.ts

Line 75 in 0e7d3a7

parse: (name: string): Promise<string> => {

you can do more specific error msgs like Can't use --source-id and define sourceId in the def. file at the same time.

cristiand391 · 2024-10-15T18:54:48Z

src/commands/org/create/sandbox.ts

      return sourceOrg.SandboxInfoId;
    } catch (err) {
-      throw messages.createError('error.noCloneSource', [this.flags.clone], [], err as Error);
+      throw messages.createError('error.noCloneSourceId', [this.flags['source-id']], [], err as Error);


this method can be removed, the value of --source-id or sourceId in the def. file is already a valid ID that we can pass to the API.

cristiand391

requested some changes.
Also can you add some UTs in test/shared/sandboxRequest.test.ts to cover the new scenarios? e.g. sourceId/sourceSandboxName throws, sourceId/sourceSandboxName trigger a clone.

cristiand391 · 2024-10-15T19:05:02Z

src/shared/sandboxRequest.ts

-    if (!SourceSandboxName) {
-      // error - we need SourceSandboxName to know which sandbox to clone from
+    if (!sandboxReqWithName.SourceSandboxName && !sandboxReqWithName.SourceId) {
+      // error - we need SourceSandboxName or SourceId to know which sandbox to clone from
      throw new SfError(
        cloneMessages.getMessage('missingSourceSandboxName', ['SourceSandboxName']),
        cloneMessages.getMessage('missingSourceSandboxNameAction', ['SourceSandboxName'])
      );


can we update these errors so they point if either SourceSandboxName or SourceId are being missed? the missingSourceSandboxName error will only mention SourceSandboxName

src/shared/sandboxRequest.ts

test/shared/sandboxRequest.test.ts

src/commands/org/create/sandbox.ts

cristiand391 · 2024-10-17T16:56:43Z

src/commands/org/create/sandbox.ts

    return {
      ...sandboxReq,
-      ...(this.flags.clone ? { SourceId: await this.getSourceId() } : {}),
+      ...(srcSandboxName ?? this.flags['source-sandbox-name']


no need to check for --source-sandbox-name here, srcSandboxName should have that value (requestFunctions.createSandboxRequest takes these flags as requestOptions and merges them with the def.file).

cristiand391 · 2024-10-17T16:58:36Z

src/commands/org/create/sandbox.ts

+      ...(srcSandboxName ?? this.flags['source-sandbox-name']
+        ? { SourceId: await requestFunctions.getSrcIdByName(this.flags['target-org'].getConnection(), srcSandboxName) }
+        : {}),
+      ...(srcId ?? this.flags['source-id'] ? { SourceId: srcId } : {}),


ditto, srcId should have the flag value, def. file value or undefined
https://github.com/salesforcecli/plugin-org/pull/1237/files#r1805117017

src/commands/org/create/sandbox.ts

cristiand391

almost there!
there are a few unresolved comments

src/shared/sandboxRequest.ts

test/shared/sandboxRequest.test.ts

cristiand391

lgtm, I'm approving code changes only, will ping Juliet for another review of the messages before merging.

cristiand391 · 2024-10-28T12:33:31Z

QA notes:

flags

✅ --source-id and --source-sandbox-name is blocked
✅ --source-id and sourceId in def. file is blocked
✅ --source-*** flags and --license-type is blocked.
✅ --source-sandbox-name and sourceSandboxName in def. file is blocked
✅ --clone/-c flag is deprecated
✅ --source-id works with 15/18-char IDs
✅ --source-sandbox-name is resolved to the correct sourceId
✅ throw user-friendly error if sourceSbxName -> sourceID can't be solved.

def. file

✅ sourceId and sourceSandboxName is blocked
❌ sourceId and licenseType is blocked

❌ sourceSandboxName and licenseType is blocked
✅ throw user-friendly error if sourceSbxName -> sourceID can't be solved.

command

✅ any source-id-related flag/prop triggers a clone
⚠️ tab with values is readable
CloneSource is always the sourceID even though it's not a real request value.

Since we'll always show the sourceId value when cloning let's remove CloneSource from the table.

⚠️ command help-text/examples are valid

sf org create sandbox --help still mentions --clone as the way to clone sandboxes and in examples:

cristiand391 · 2024-10-30T15:42:50Z

❌ sourceId and licenseType is blocked
❌ sourceSandboxName and licenseType is blocked
⚠️ command help-text/examples are valid

✅ fixed

soridalac added 8 commits September 23, 2024 15:03

feat: name fields for definitionfile

43b037d

fix: update the changes

8ebe71c

fix: definition-file refresh payload

59f8047

testing

2f14f34

feat: create flag and field for sandbox

86ae35a

fix: update the name and id for definition file

063e3a3

fix: sandbox name

4c3ee45

fix: update sourceSandboxName and Id

0c22746

soridalac requested a review from a team as a code owner October 14, 2024 21:05

jshackell-sfdc requested changes Oct 14, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into flagfeature

94a96ab

cristiand391 reviewed Oct 15, 2024

View reviewed changes

cristiand391 requested changes Oct 15, 2024

View reviewed changes

soridalac added 2 commits October 16, 2024 18:41

fix: update the fields and error messages

3f6fc8e

fix: update the error message

a8c511c

cristiand391 reviewed Oct 17, 2024

View reviewed changes

src/commands/org/create/sandbox.ts Show resolved Hide resolved

cristiand391 reviewed Oct 17, 2024

View reviewed changes

src/commands/org/create/sandbox.ts Outdated Show resolved Hide resolved

cristiand391 requested changes Oct 17, 2024

View reviewed changes

src/shared/sandboxRequest.ts Show resolved Hide resolved

test/shared/sandboxRequest.test.ts Show resolved Hide resolved

soridalac added 3 commits October 17, 2024 11:32

fix: update the error messages

50c1c38

fix: update the snapshot of alias clone

f5f3cf2

fix: error message

b57810b

soridalac added 3 commits October 17, 2024 12:16

fix: update clone message

345f6e0

fix: validate error messages for cloning

1858fd7

chore: ci-rerun

cad6958

cristiand391 approved these changes Oct 18, 2024

View reviewed changes

jshackell-sfdc approved these changes Oct 21, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into flagfeature

53f3f65

soridalac added 2 commits October 29, 2024 12:52

fix: add validation and messages

82d660f

fix: message description

de812a1

cristiand391 approved these changes Oct 30, 2024

View reviewed changes

cristiand391 merged commit a23ce06 into main Oct 30, 2024
11 checks passed

cristiand391 deleted the flagfeature branch October 30, 2024 15:43

iowillhoit mentioned this pull request Oct 31, 2024

Release PR for 2.66.0 as nightly salesforcecli/cli#1975

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flagfeature #1237

Flagfeature #1237

soridalac commented Oct 14, 2024 •

edited by cristiand391

Loading

jshackell-sfdc left a comment

jshackell-sfdc Oct 14, 2024

jshackell-sfdc Oct 14, 2024

jshackell-sfdc Oct 14, 2024

jshackell-sfdc Oct 14, 2024

jshackell-sfdc Oct 14, 2024

jshackell-sfdc Oct 14, 2024

jshackell-sfdc Oct 15, 2024

cristiand391 Oct 15, 2024

cristiand391 Oct 15, 2024

cristiand391 Oct 15, 2024

cristiand391 Oct 15, 2024

cristiand391 Oct 15, 2024

cristiand391 Oct 15, 2024 •

edited

Loading

cristiand391 Oct 17, 2024

cristiand391 Oct 15, 2024

cristiand391 Oct 15, 2024

cristiand391 left a comment

cristiand391 Oct 15, 2024

cristiand391 Oct 17, 2024

cristiand391 Oct 17, 2024

cristiand391 left a comment

cristiand391 left a comment

cristiand391 commented Oct 28, 2024

cristiand391 commented Oct 30, 2024


		# error.sandboxNameQueryFailed

		Unable to find the ID of the sandboxName "%s" that's defined in the definition file.

	Unable to find the ID of the sandboxName "%s" that's defined in the definition file.
	Unable to find the ID of the sandbox "%s" that's defined in the definition file.

	You can specify either --source-sandbox-name or --source-id for clone an existing sandbox but not both.
	You can specify either --source-sandbox-name or --source-id when cloning an existing sandbox, but not both.


		The value of --source-id must be an existing sandbox. The existing sandbox, and the new sandbox specified with the --name flag, must both be associated with the production org (--target-org) that contains the sandbox licenses.

		You can specify either --source-sandbox-name or --source-id for clone an existing sandbox but not both.

	Could not find the clone sandbox name "%s" in the target org.
	Couldn't find the clone sandbox name "%s" in the target org.


		# error.noCloneSourceId

		Could not find the clone sandbox id "%s" in the target org.

	Could not find the clone sandbox id "%s" in the target org.
	Couldn't find the clone sandbox ID "%s" in the target org.


		# error.cloneFlagsConflict

		You can not use clone Name or Id flags and clone Name or Id fields in the definition file at the same time.

		@@ -20,13 +20,15 @@ const getLicenseTypes = (): string[] => Object.values(SandboxLicenseType);

		type SandboxConfirmData = SandboxRequest & { CloneSource?: string };

		// eslint-disable-next-line sf-plugin/only-extend-SfCommand

	'source-org': Flags.salesforceId({
	summary: messages.getMessage('flags.source-org.summary'),
	startsWith: '00D',
	length: 15,
	helpGroup: definitionFileHelpGroupName,
	// salesforceId flag has `i` and that would be a conflict with client-id
	char: undefined,
	}),

Flagfeature #1237

Flagfeature #1237

Conversation

soridalac commented Oct 14, 2024 • edited by cristiand391 Loading

What does this PR do?

What issues does this PR fix or reference?

jshackell-sfdc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristiand391 Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristiand391 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristiand391 left a comment

Choose a reason for hiding this comment

cristiand391 left a comment

Choose a reason for hiding this comment

cristiand391 commented Oct 28, 2024

flags

def. file

command

cristiand391 commented Oct 30, 2024

soridalac commented Oct 14, 2024 •

edited by cristiand391

Loading

cristiand391 Oct 15, 2024 •

edited

Loading