feat: support retrieves in metadata format #313

mdonnalley · 2022-08-04T18:17:18Z

What does this PR do?

Adds metadata format support to retrieve metadata

New flags:

--target-metadata-dir, -t (depends on one of source-dir, manifest, and metadata)
--single-package (depends on target-metadata-dir)
--unzip, -z (depends on target-metadata-dir)
--zip-file-name (depends on target-metadata-dir)

TODO:

Tests

What issues does this PR fix or reference?

@W-10748963@

mshanemc

some questions, and one real change on the output.ts comment

mshanemc · 2022-08-08T17:07:30Z

src/utils/flags.ts

+  let resolvedPath: string;
+  if (trimmedPath?.length) {
+    resolvedPath = resolve(trimmedPath);
+  }


avoids let

explicit that resolvedPath could be undefined (I'm not sure that's how it's supposed to work, but that's possible in both the existing and suggested code.

Suggested change

let resolvedPath: string;

if (trimmedPath?.length) {

resolvedPath = resolve(trimmedPath);

}

const resolvedPath = trimmedPath?.length ? resolve(trimmedPath) : undefined;

mshanemc · 2022-08-08T17:10:06Z

src/utils/flags.ts

+
+  try {
+    const stats = fs.statSync(resolvedPath);
+    if (type !== 'any') {


why might it be 'any' ? If it is 'any', then nothing should happen?

it's happy as long as something exists there?

any denotes that it could be a file or a directory

For context, I took this directly from plugin-source: https://github.com/salesforcecli/plugin-source/blob/d8192203b018df2ed6af68ed695ca96e759a89c3/src/sourceCommand.ts#L81 without any modification

if we don't have a use case for that in mind, I'd be happy omitting it

mshanemc · 2022-08-08T17:15:41Z

src/utils/output.ts

+  public async display(): Promise<void> {
+    CliUx.ux.log(retrieveMessages.getMessage('info.WroteZipFile', [this.zipFilePath]));
+    if (this.opts.unzip) {
+      const extractPath = path.join(this.opts['target-metadata-dir'], path.parse(this.opts['zip-file-name']).name);


on line 473, we set this.zipFilePath to the default unpackaged.zip but it doesn't get defaulted on this.opts. It's always safer to do that kind of stuff in the constructor (have it set defaults on the options object) to avoid this accidental potential undefined.

If you strongly prefer to not modify the options, you could put a private zipFileName and have the constructor set that and refer to it here.

There's actually not a need to default it anywhere since the flag does that already:

plugin-deploy-retrieve/src/utils/flags.ts

Line 149 in 6df9c94

export const zipFileFlag = (opts: ZipFileOpts): Interfaces.OptionFlag<string | undefined> => {

even better

mshanemc · 2022-08-08T17:22:40Z

src/utils/flags.ts

+): Interfaces.OptionFlag<TestLevel | undefined> => {
+  return Flags.build<TestLevel | undefined>({
+    char: 'l',
+    parse: (input: string) => Promise.resolve(input as TestLevel),


What happens without the parse function that's doing nothing (it's either undefined or one of the options!?). Or is it necessary to coerce the type via the assertion?

Flags.build requires the parse to be present so you have to put something.

If you did parse: (input: string) => Promise.resolve(input), it would complain that you're returning a string when it expects a TestLevel

that feels like an oclif problem.

mshanemc · 2022-08-10T21:16:16Z

[side note: is this PR description wrong?]

--target-metadata-dir, -t (exclusive of source-dir, manifest, and metadata)
--zip-file-name (depends on target-metadata-dir)

QA notes:

👎🏻 there should be examples for this mdapi format stuff

👎🏻 retrieve metadata -t mdapiOut
Error (1): No components in the package to retrieve.
Not sure what to do. without examples and with confusion from the PR description...I'm digging through code to understand what flags --target-metadata-dir needs.

retrieve metadata -t mdapiOut --source-dir force-app
✅ retrieves zip file to a dir that doesn't exist yet (creates)
[runs command again] ✅ same end result (replaces zip file)
[delete zip file and run again] ✅ same end result (creates zip file in existing folder)

retrieve metadata -t package.json --source-dir force-app
✅ good error message. 👎🏻 the extra line seems odd. Feels like these go together in the same line?

Error (1): Invalid path specified: package.json

Expected a directory but found a file.

retrieve metadata -t mdapiOut2 --source-dir force-app -z
❓ I think this is wrong not to delete the zip file. I can't imagine why I'd want both when I say -z. I see sfdx does that.

retrieve metadata -t mdapiOut3 --source-dir force-app -z --single-package

💡 why not mdapiOut/pkg1/etc, mdapiOut/pkg2/etc (or just pkg1 if single-package)? If I'm giving the target folder (mdapiOut) you could put each pkg in the folder I said (so it supports multiple) OR just put the contents of the single-package in there, so that it's mdapiOut/package.xml.

Maybe there's a good reason for this (mdapi deploy expects it?), but I've always avoided mdapi and don't understand the logic. Or maybe it's awkward but better to keep it like sfdx does it. 🤷🏻

--zip-file-name foo
✅ produces zip file named foo, which unzips to /foo. cool
✅ can be safely repeated on the same dir (overwrites zip and unzipped stuff)

--api-version 44.0
✅ version is correct in package.xml

-c
👎🏻 I think these mdapi-only flags should be exclusive of -c, --ignore-conflicts, just to keep users from misunderstanding.

mshanemc · 2022-09-06T15:24:18Z

QA (primarily rechecking flag config and interactions given the oclif changes)
✅ help has examples
✅ exclusivity for -c (also 👍🏻 for the error message extra line being gone!)

Error: The following errors occurred:
  --ignore-conflicts=true cannot also be provided when using --target-metadata-dir
  --ignore-conflicts=true cannot also be provided when using --unzip

💡 idea for oclif: what if the error message included the short char, like

 --ignore-conflicts=true (-c) cannot also be provided when using --target-metadata-dir (-t)

push to org
retrieve metadata --source-dir force-app --target-metadata-dir output --unzip
❌ all conflicts in a table, then Error: There are changes in your local files that conflict with the org changes you're trying to retrieve.. see 773c3a3

✅ after fix

retrieve metadata --source-dir force-app --target-metadata-dir output --unzip
Warning: This command is currently in beta. Any aspect of this command can change without advanced notice. Don't use beta commands in your scripts.
Preparing retrieve request... Succeeded
Wrote retrieve zip file to /Users/shane.mclaughlin/eng/repros/qa-mdapi-retrieve/output/unpackaged.zip.
Extracted /Users/shane.mclaughlin/eng/repros/qa-mdapi-retrieve/output/unpackaged.zip to /Users/shane.mclaughlin/eng/repros/qa-mdapi-retrieve/output/unpackaged.

✅ retrieving to a zip file works fine without a name (=> unpackaged.zip)
✅ retrieving to a zip file works fine with a name (--zip-file-name myZip.zip => myZip.zip)
✅ retrieving to a zip file works fine with a name but no extension (--zip-file-name myOtherZip => myOtherZip.zip)

feat: support retrieves in metadata format

6df9c94

mdonnalley self-assigned this Aug 4, 2022

mdonnalley requested a review from mshanemc August 4, 2022 18:17

mshanemc requested changes Aug 8, 2022

View reviewed changes

chore: code review

434f5f7

mshanemc approved these changes Aug 8, 2022

View reviewed changes

mdonnalley added 6 commits August 8, 2022 15:28

fix: add overloads to custom flags

c6efcb7

chore: update flags

1fd03a5

chore: bump oclif/core

be6e041

chore: bump oclif/core

41f84ef

chore: nuts and unit tests

9f5a526

chore: nuts

a497d58

mdonnalley added 6 commits August 17, 2022 14:35

chore: code review

5facb06

Merge branch 'main' into mdonnalley/retireve-metadata-format

a7d6db5

chore: bump oclif/core

eb615da

chore: regenerate yarn.lock

958fcc8

chore: exclude ignore-conflicts on mdapi flags

0f5a489

chore: remove zip when --unzip is provided

4321256

mshanemc approved these changes Sep 6, 2022

View reviewed changes

mshanemc added 2 commits September 6, 2022 09:57

chore: bump for eslint bug

60722bb

fix: ignoreConflicts when format is metadata

773c3a3

mshanemc merged commit 380c978 into main Sep 6, 2022

mshanemc deleted the mdonnalley/retireve-metadata-format branch September 6, 2022 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support retrieves in metadata format #313

feat: support retrieves in metadata format #313

mdonnalley commented Aug 4, 2022 •

edited

mshanemc left a comment

mshanemc Aug 8, 2022

mshanemc Aug 8, 2022

mshanemc Aug 8, 2022

mdonnalley Aug 8, 2022

mshanemc Aug 8, 2022

mshanemc Aug 8, 2022

mdonnalley Aug 8, 2022

mshanemc Aug 8, 2022

mshanemc Aug 8, 2022

mdonnalley Aug 8, 2022

mshanemc Aug 8, 2022

mshanemc commented Aug 10, 2022

mshanemc commented Sep 6, 2022

feat: support retrieves in metadata format #313

feat: support retrieves in metadata format #313

Conversation

mdonnalley commented Aug 4, 2022 • edited

What does this PR do?

What issues does this PR fix or reference?

mshanemc 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

mshanemc commented Aug 10, 2022

mshanemc commented Sep 6, 2022

mdonnalley commented Aug 4, 2022 •

edited