fix: retrieve package names output (#149)

* fix: retrieve package names output

* chore: throw error with non-existant packagename requested

* chore: move warnings to bottom of output

* chore: enable retrieve packagename NUTs

* chore: delete zipFile entry in JSON

* chore: delete zipFile entry in JSON

* chore: debug NUTs

* chore: extend base type

* chore: install package in NUTs to try and fix windows

* chore: install packages per test

* chore: full exec package install

* chore: silence exec

Author: WillieRuemmele

src/commands/force/source/retrieve.ts

@@ -6,14 +6,19 @@
  */
 
 import * as os from 'os';
+import { join } from 'path';
 import { flags, FlagsConfig } from '@salesforce/command';
-import { Messages } from '@salesforce/core';
+import { Messages, SfdxProject } from '@salesforce/core';
 import { Duration } from '@salesforce/kit';
 import { getString } from '@salesforce/ts-types';
 import { RetrieveResult } from '@salesforce/source-deploy-retrieve';
 import { RequestStatus } from '@salesforce/source-deploy-retrieve/lib/src/client/types';
 import { SourceCommand } from '../../../sourceCommand';
-import { RetrieveResultFormatter, RetrieveCommandResult } from '../../../formatters/retrieveResultFormatter';
+import {
+  RetrieveResultFormatter,
+  RetrieveCommandResult,
+  PackageRetrieval,
+} from '../../../formatters/retrieveResultFormatter';
 import { ComponentSetBuilder } from '../../../componentSetBuilder';
 
 Messages.importMessagesDirectory(__dirname);
@@ -105,10 +110,18 @@ export class Retrieve extends SourceCommand {
     }
   }
 
-  protected formatResult(): RetrieveCommandResult {
+  protected async formatResult(): Promise<RetrieveCommandResult> {
+    const packages: PackageRetrieval[] = [];
+    const projectPath = await SfdxProject.resolveProjectPath();
+
+    this.getFlag<string[]>('packagenames', []).forEach((name) => {
+      packages.push({ name, path: join(projectPath, name) });
+    });
+
     const formatterOptions = {
       waitTime: this.getFlag<Duration>('wait').quantity,
       verbose: this.getFlag<boolean>('verbose', false),
+      packages,
     };
     const formatter = new RetrieveResultFormatter(this.logger, this.ux, formatterOptions, this.retrieveResult);
 

src/formatters/retrieveResultFormatter.ts

@@ -7,7 +7,7 @@
 
 import { blue, yellow } from 'chalk';
 import { UX } from '@salesforce/command';
-import { Logger, Messages } from '@salesforce/core';
+import { Logger, Messages, SfdxError } from '@salesforce/core';
 import { get, getString, getNumber } from '@salesforce/ts-types';
 import { RetrieveResult, MetadataApiRetrieveStatus } from '@salesforce/source-deploy-retrieve';
 import {
@@ -26,6 +26,10 @@ export interface PackageRetrieval {
   path: string;
 }
 
+export interface RetrieveResultFormatterOptions extends ResultFormatterOptions {
+  packages?: PackageRetrieval[];
+}
+
 export interface RetrieveCommandResult {
   inboundFiles: FileResponse[];
   packages: PackageRetrieval[];
@@ -34,16 +38,20 @@ export interface RetrieveCommandResult {
 }
 
 export class RetrieveResultFormatter extends ResultFormatter {
+  protected packages: PackageRetrieval[] = [];
   protected result: RetrieveResult;
   protected fileResponses: FileResponse[];
   protected warnings: RetrieveMessage[];
 
-  public constructor(logger: Logger, ux: UX, options: ResultFormatterOptions, result: RetrieveResult) {
+  public constructor(logger: Logger, ux: UX, options: RetrieveResultFormatterOptions, result: RetrieveResult) {
     super(logger, ux, options);
     this.result = result;
     this.fileResponses = result?.getFileResponses ? result.getFileResponses() : [];
     const warnMessages = get(result, 'response.messages', []) as RetrieveMessage | RetrieveMessage[];
     this.warnings = Array.isArray(warnMessages) ? warnMessages : [warnMessages];
+    this.packages = options.packages || [];
+    // zipFile can become massive and unweildy with JSON parsing/terminal output and, isn't useful
+    delete this.result.response.zipFile;
   }
 
   /**
@@ -54,7 +62,7 @@ export class RetrieveResultFormatter extends ResultFormatter {
   public getJson(): RetrieveCommandResult {
     return {
       inboundFiles: this.fileResponses,
-      packages: [],
+      packages: this.packages,
       warnings: this.warnings,
       response: this.result.response,
     };
@@ -71,28 +79,28 @@ export class RetrieveResultFormatter extends ResultFormatter {
     }
 
     if (this.isSuccess()) {
-      if (this.warnings.length) {
-        this.displayWarnings();
-      }
       this.ux.styledHeader(blue(messages.getMessage('retrievedSourceHeader')));
       const retrievedFiles = this.fileResponses.filter((fr) => fr.state !== ComponentStatus.Failed);
       if (retrievedFiles?.length) {
         this.displaySuccesses(retrievedFiles);
       } else {
         this.ux.log(messages.getMessage('NoResultsFound'));
       }
+      if (this.warnings.length) {
+        this.displayWarnings();
+      }
     } else {
       this.displayErrors();
     }
 
     // Display any package retrievals
-    // if (results.packages && results.packages.length) {
-    //   this.logger.styledHeader(this.logger.color.blue('Retrieved Packages'));
-    //   results.packages.forEach(pkg => {
-    //     this.logger.log(`${pkg.name} package converted and retrieved to: ${pkg.path}`);
-    //   });
-    //   this.logger.log('');
-    // }
+    if (this.packages && this.packages.length) {
+      this.ux.styledHeader(blue('Retrieved Packages'));
+      this.packages.forEach((pkg) => {
+        this.ux.log(`${pkg.name} package converted and retrieved to: ${pkg.path}`);
+      });
+      this.ux.log('');
+    }
   }
 
   protected hasStatus(status: RequestStatus): boolean {
@@ -125,6 +133,11 @@ export class RetrieveResultFormatter extends ResultFormatter {
   }
 
   private displayErrors(): void {
+    // an invalid packagename retrieval will end up with a message in the `errorMessage` entry
+    const errorMessage = get(this.result.response, 'errorMessage') as string;
+    if (errorMessage) {
+      throw new SfdxError(errorMessage);
+    }
     const unknownMsg: RetrieveMessage[] = [{ fileName: 'unknown', problem: 'unknown' }];
     const responseMsgs = get(this.result, 'response.messages', unknownMsg) as RetrieveMessage | RetrieveMessage[];
     const errMsgs = Array.isArray(responseMsgs) ? responseMsgs : [responseMsgs];

test/commands/source/retrieve.test.ts

@@ -5,6 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
+import { join } from 'path';
 import * as sinon from 'sinon';
 import { expect } from 'chai';
 import { RetrieveOptions } from '@salesforce/source-deploy-retrieve';
@@ -56,6 +57,7 @@ describe('force:source:retrieve', () => {
 
   const runRetrieveCmd = async (params: string[]) => {
     const cmd = new TestRetrieve(params, oclifConfigStub);
+    stubMethod(sandbox, SfdxProject, 'resolveProjectPath').resolves(join('path', 'to', 'package'));
     stubMethod(sandbox, cmd, 'assignProject').callsFake(() => {
       const sfdxProjectStub = fromStub(
         stubInterface<SfdxProject>(sandbox, {
@@ -213,6 +215,7 @@ describe('force:source:retrieve', () => {
     const manifest = 'package.xml';
     const packagenames = ['package1'];
     const result = await runRetrieveCmd(['--manifest', manifest, '--packagenames', packagenames[0], '--json']);
+    expectedResults.packages.push({ name: packagenames[0], path: join('path', 'to', 'package', packagenames[0]) });
     expect(result).to.deep.equal(expectedResults);
     ensureCreateComponentSetArgs({
       packagenames,
@@ -223,6 +226,29 @@ describe('force:source:retrieve', () => {
     });
     ensureRetrieveArgs({ packageOptions: packagenames });
     ensureHookArgs();
+    // reset the packages for other tests
+    expectedResults.packages = [];
+  });
+
+  it('should pass along multiple packagenames', async () => {
+    const manifest = 'package.xml';
+    const packagenames = ['package1', 'package2'];
+    const result = await runRetrieveCmd(['--manifest', manifest, '--packagenames', packagenames.join(','), '--json']);
+    packagenames.forEach((pkg) => {
+      expectedResults.packages.push({ name: pkg, path: join('path', 'to', 'package', pkg) });
+    });
+    expect(result).to.deep.equal(expectedResults);
+    ensureCreateComponentSetArgs({
+      packagenames,
+      manifest: {
+        manifestPath: manifest,
+        directoryPaths: [defaultPackagePath],
+      },
+    });
+    ensureRetrieveArgs({ packageOptions: packagenames });
+    ensureHookArgs();
+    // reset the packages for other tests
+    expectedResults.packages = [];
   });
 
   it('should display output with no --json', async () => {

test/commands/source/retrieveResponses.ts

@@ -43,7 +43,6 @@ const baseRetrieveResponse = {
   id: '09S21000002jxznEAA',
   status: 'Succeeded',
   success: true,
-  zipFile: 'UEsDBBQA...some_long_string',
 };
 
 const warningMessage = "Entity of type 'ApexClass' named 'ProductController' cannot be found";

test/formatters/retrieveResultFormatter.test.ts

@@ -143,7 +143,7 @@ describe('RetrieveResultFormatter', () => {
       expect(styledHeaderStub.calledTwice).to.equal(true);
       expect(logStub.called).to.equal(true);
       expect(tableStub.calledOnce).to.equal(true);
-      expect(styledHeaderStub.firstCall.args[0]).to.contain('Retrieved Source Warnings');
+      expect(styledHeaderStub.secondCall.args[0]).to.contain('Retrieved Source Warnings');
       const warnMessages = retrieveResultWarnings.response.messages;
       const warnings = Array.isArray(warnMessages) ? warnMessages : [warnMessages];
       expect(tableStub.firstCall.args[0]).to.deep.equal(warnings);

test/nuts/seeds/retrieve.packagenames.seed.ts

@@ -7,14 +7,15 @@
 
 import * as path from 'path';
 import { SourceTestkit } from '@salesforce/source-testkit';
+import { exec } from 'shelljs';
 
 // DO NOT TOUCH. generateNuts.ts will insert these values
 const EXECUTABLE = '%EXECUTABLE%';
 
 const ELECTRON = { id: '04t6A000002zgKSQAY', name: 'ElectronBranding' };
 const SKUID = { id: '04t4A000000cESSQA2', name: 'Skuid' };
 
-context.skip('Retrieve packagenames NUTs [exec: %EXECUTABLE%]', () => {
+context('Retrieve packagenames NUTs [exec: %EXECUTABLE%]', () => {
   let testkit: SourceTestkit;
 
   before(async () => {
@@ -23,7 +24,6 @@ context.skip('Retrieve packagenames NUTs [exec: %EXECUTABLE%]', () => {
       executable: EXECUTABLE,
       nut: __filename,
     });
-    testkit.installPackage(ELECTRON.id);
     await testkit.deploy({ args: `--sourcepath ${testkit.packageNames.join(',')}` });
   });
 
@@ -39,17 +39,23 @@ context.skip('Retrieve packagenames NUTs [exec: %EXECUTABLE%]', () => {
 
   describe('--packagenames flag', () => {
     it('should retrieve an installed package', async () => {
+      exec(`sfdx force:package:install --noprompt --package ${ELECTRON.id} --wait 5 --json`, { silent: true });
+
       await testkit.retrieve({ args: `--packagenames "${ELECTRON.name}"` });
       await testkit.expect.packagesToBeRetrieved([ELECTRON.name]);
     });
 
     it('should retrieve two installed packages', async () => {
-      testkit.installPackage(SKUID.id);
+      exec(`sfdx force:package:install --noprompt --package ${ELECTRON.id} --wait 5 --json`, { silent: true });
+      exec(`sfdx force:package:install --noprompt --package ${SKUID.id} --wait 5 --json`, { silent: true });
+
       await testkit.retrieve({ args: `--packagenames "${ELECTRON.name}, ${SKUID.name}"` });
       await testkit.expect.packagesToBeRetrieved([ELECTRON.name, SKUID.name]);
     });
 
     it('should retrieve an installed package and sourcepath', async () => {
+      exec(`sfdx force:package:install --noprompt --package ${ELECTRON.id} --wait 5 --json`, { silent: true });
+
       await testkit.retrieve({
         args: `--packagenames "${ELECTRON.name}" --sourcepath "${path.join('force-app', 'main', 'default', 'apex')}"`,
       });