fix: set correct status on file responses (#465)

* fix: set correct status on file responses

* fix: use constructor shorthand and add comments

* refactor: Apply suggestions from code review

* ci: dependency fix on external NUTs

* ci: delete before build

* ci: module deletes had a cwd

Co-authored-by: Willie Ruemmele <willieruemmele@gmail.com>
Co-authored-by: Shane McLaughlin <shane.mclaughlin@salesforce.com>

.circleci/config.yml

@@ -64,8 +64,12 @@ jobs:
       - run:
           name: install/build <<parameters.external_project_git_url>> in node_modules
           # why doesn't SDR put the registry.json in the lib when run from inside a node module?  I don't know.
+          # prevent dependency conflicts between plugin's top-level imports and imported SDR's deps by deleting them
+          # If there are real conflicts, we'll catch them when bumping a version in the plugin (same nuts)
           command: |
             yarn install
+            rm -rf node_modules/@salesforce/kit
+            rm -rf node_modules/@salesforce/core
             yarn build
             cp src/registry/registry.json lib/src/registry
           working_directory: node_modules/@salesforce/source-deploy-retrieve
@@ -140,4 +144,4 @@ workflows:
           filters:
             branches:
               only: main
-          context: CLI_CTC
+          context: CLI_CTC

src/client/metadataApiRetrieve.ts

@@ -29,16 +29,31 @@ export type MetadataApiRetrieveOptions = MetadataTransferOptions &
   RetrieveOptions & { registry?: RegistryAccess };
 
 export class RetrieveResult implements MetadataTransferResult {
-  public readonly response: MetadataApiRetrieveStatus;
-  public readonly components: ComponentSet;
+  // This ComponentSet is most likely just the components on the local file
+  // system and is used to set the state of a SourceComponent to "Created"
+  // rather than "Changed".
+  private localComponents: ComponentSet;
+  private fileResponses: FileResponse[];
 
-  constructor(response: MetadataApiRetrieveStatus, components: ComponentSet) {
-    this.response = response;
-    this.components = components;
+  /**
+   * @param response The metadata retrieve response from the server
+   * @param components The ComponentSet of retrieved source components
+   * @param localComponents The ComponentSet used to create the retrieve request
+   */
+  constructor(
+    public readonly response: MetadataApiRetrieveStatus,
+    public readonly components: ComponentSet,
+    localComponents?: ComponentSet
+  ) {
+    this.localComponents = new ComponentSet(localComponents?.getSourceComponents());
   }
 
   public getFileResponses(): FileResponse[] {
-    const responses: FileResponse[] = [];
+    if (this.response && this.fileResponses) {
+      return this.fileResponses;
+    }
+
+    this.fileResponses = [];
 
     // construct failures
     if (this.response.messages) {
@@ -49,15 +64,15 @@ export class RetrieveResult implements MetadataTransferResult {
         const matches = message.problem.match(/.+'(.+)'.+'(.+)'/);
         if (matches) {
           const [typeName, fullName] = matches.slice(1);
-          responses.push({
+          this.fileResponses.push({
             fullName,
             type: typeName,
             state: ComponentStatus.Failed,
             error: message.problem,
             problemType: 'Error',
           });
         } else {
-          responses.push({
+          this.fileResponses.push({
             fullName: '',
             type: '',
             problemType: 'Error',
@@ -74,21 +89,23 @@ export class RetrieveResult implements MetadataTransferResult {
       const baseResponse: FileResponse = {
         fullName,
         type: type.name,
-        state: ComponentStatus.Changed,
+        state: this.localComponents.has(retrievedComponent)
+          ? ComponentStatus.Changed
+          : ComponentStatus.Created,
       };
 
       if (!type.children) {
         for (const filePath of retrievedComponent.walkContent()) {
-          responses.push(Object.assign({}, baseResponse, { filePath }));
+          this.fileResponses.push(Object.assign({}, baseResponse, { filePath }));
         }
       }
 
       if (xml) {
-        responses.push(Object.assign({}, baseResponse, { filePath: xml }));
+        this.fileResponses.push(Object.assign({}, baseResponse, { filePath: xml }));
       }
     }
 
-    return responses;
+    return this.fileResponses;
   }
 }
 
@@ -175,7 +192,7 @@ export class MetadataApiRetrieve extends MetadataTransfer<
 
     await this.maybeSaveTempDirectory('source', components);
 
-    return new RetrieveResult(result, components);
+    return new RetrieveResult(result, components, this.components);
   }
 
   private getPackageNames(): string[] {

test/client/metadataApiRetrieve.test.ts

@@ -351,7 +351,7 @@ describe('MetadataApiRetrieve', async () => {
 
         await operation.start();
         const result = await operation.pollStatus();
-        const expected = new RetrieveResult(response, toRetrieve);
+        const expected = new RetrieveResult(response, toRetrieve, toRetrieve);
 
         expect(result).to.deep.equal(expected);
       });
@@ -385,7 +385,8 @@ describe('MetadataApiRetrieve', async () => {
 
         await operation.start();
         const result = await operation.pollStatus();
-        const expected = new RetrieveResult(response, new ComponentSet(undefined, mockRegistry));
+        const compSet = new ComponentSet(undefined, mockRegistry);
+        const expected = new RetrieveResult(response, compSet, toRetrieve);
 
         expect(result).to.deep.equal(expected);
       });
@@ -433,7 +434,11 @@ describe('MetadataApiRetrieve', async () => {
         const component = COMPONENT;
         const retrievedSet = new ComponentSet([component]);
         const apiStatus = {};
-        const result = new RetrieveResult(apiStatus as MetadataApiRetrieveStatus, retrievedSet);
+        const result = new RetrieveResult(
+          apiStatus as MetadataApiRetrieveStatus,
+          retrievedSet,
+          retrievedSet
+        );
 
         const responses = result.getFileResponses();
         const baseResponse: FileResponse = {
@@ -450,6 +455,41 @@ describe('MetadataApiRetrieve', async () => {
       });
     });
 
+    it('should report correct file status', () => {
+      const component = COMPONENT;
+      const newComponent = DECOMPOSED_COMPONENT;
+      const retrievedSet = new ComponentSet([component, newComponent]);
+      const localSet = new ComponentSet([component]);
+      const apiStatus = {};
+      const result = new RetrieveResult(
+        apiStatus as MetadataApiRetrieveStatus,
+        retrievedSet,
+        localSet
+      );
+
+      const responses = result.getFileResponses();
+      const baseResponse: FileResponse = {
+        state: ComponentStatus.Changed,
+        fullName: component.fullName,
+        type: component.type.name,
+      };
+      // Since the DECOMPOSED_COMPONENT was in the retrieved ComponentSet but
+      // not the local source ComponentSet it should have a state of 'Created'
+      // rather than 'Changed'.
+      const expected: FileResponse[] = [
+        Object.assign({}, baseResponse, { filePath: component.content }),
+        Object.assign({}, baseResponse, { filePath: component.xml }),
+        {
+          fullName: DECOMPOSED_COMPONENT.fullName,
+          filePath: DECOMPOSED_COMPONENT.xml,
+          state: ComponentStatus.Created,
+          type: DECOMPOSED_COMPONENT.type.name,
+        },
+      ];
+
+      expect(responses).to.deep.equal(expected);
+    });
+
     it('should report one failure if component does not exist', () => {
       const component = COMPONENT;
       const retrievedSet = new ComponentSet();
@@ -487,7 +527,11 @@ describe('MetadataApiRetrieve', async () => {
           },
         ],
       };
-      const result = new RetrieveResult(apiStatus as MetadataApiRetrieveStatus, retrievedSet);
+      const result = new RetrieveResult(
+        apiStatus as MetadataApiRetrieveStatus,
+        retrievedSet,
+        retrievedSet
+      );
 
       const responses = result.getFileResponses();
       const expected: FileResponse[] = [
@@ -543,7 +587,11 @@ describe('MetadataApiRetrieve', async () => {
       const component = DECOMPOSED_COMPONENT;
       const retrievedSet = new ComponentSet([component]);
       const apiStatus = {};
-      const result = new RetrieveResult(apiStatus as MetadataApiRetrieveStatus, retrievedSet);
+      const result = new RetrieveResult(
+        apiStatus as MetadataApiRetrieveStatus,
+        retrievedSet,
+        retrievedSet
+      );
 
       const responses = result.getFileResponses();
       const expected: FileResponse[] = [
@@ -569,7 +617,11 @@ describe('MetadataApiRetrieve', async () => {
       );
       const retrievedSet = new ComponentSet([component]);
       const apiStatus = {};
-      const result = new RetrieveResult(apiStatus as MetadataApiRetrieveStatus, retrievedSet);
+      const result = new RetrieveResult(
+        apiStatus as MetadataApiRetrieveStatus,
+        retrievedSet,
+        retrievedSet
+      );
 
       const responses = result.getFileResponses();
       const expected: FileResponse[] = [