fix: @W-18480479 : Code review comments

Author: sf-kishore-kurri

src/commands/omnistudio/migration/migrate.ts

@@ -208,7 +208,7 @@ export default class Migrate extends OmniStudioBaseCommand {
     );
 
     if (!migrateOnly) {
-      await postMigrate.setDesignersToUseStandardDataModel(namespace, actionItems);
+      await postMigrate.executeTasks(namespace, actionItems);
     }
     // From here also actionItems need to be collected
     await postMigrate.restoreExperienceAPIMetadataSettings(
@@ -437,7 +437,7 @@ export default class Migrate extends OmniStudioBaseCommand {
           allVersions
         ),
         new CardMigrationTool(namespace, conn, this.logger, messages, this.ux, allVersions),
-        new GlobalAutoNumberMigrationTool(namespace, conn, this.logger, messages, this.ux),
+        // new GlobalAutoNumberMigrationTool(namespace, conn, this.logger, messages, this.ux),
       ];
     } else {
       // For single component migration, the order doesn't matter as much

src/migration/postMigrate.ts

@@ -40,14 +40,19 @@ export class PostMigrate extends BaseMigrationTool {
     this.settingsPrefManager = new OmnistudioSettingsPrefManager(connection, messages);
   }
 
+  /**
+   * Execute post migration tasks with dependency handling.
+   */
+  public async executeTasks(namespaceToModify: string, userActionMessage: string[]): Promise<string[]> {
+    const designerOk = await this.enableDesignersToUseStandardDataModelIfNeeded(namespaceToModify, userActionMessage);
+    if (designerOk) {
+      await this.enableStandardRuntimeIfNeeded(userActionMessage);
+    }
+    return userActionMessage;
+  }
+
   /**
    * Checks if OmniStudio designers are already using the standard data model for the specific package.
-   *
-   * This method queries the OmniInteractionConfig table to check for specific DeveloperName values
-   * that indicate whether the standard data model is enabled for the package.
-   *
-   * @param namespaceToModify The namespace of the package being checked
-   * @returns {Promise<boolean>} True if designers are already using standard data model, false otherwise
    */
   private async isStandardDesignerEnabled(namespaceToModify: string): Promise<boolean> {
     try {
@@ -62,7 +67,6 @@ export class PostMigrate extends BaseMigrationTool {
         const records = result.records as Array<{ DeveloperName: string; Value: string }>;
 
         for (const record of records) {
-          // Check if the package matches the one we're migrating
           if (record.Value === namespaceToModify) {
             return true;
           }
@@ -74,15 +78,14 @@ export class PostMigrate extends BaseMigrationTool {
     } catch (error) {
       const errMsg = error instanceof Error ? error.message : String(error);
       Logger.error(this.messages.getMessage('errorCheckingStandardDesigner', [namespaceToModify, errMsg]));
-      // Assume not enabled if query fails
       return false;
     }
   }
 
-  public async setDesignersToUseStandardDataModel(
+  public async enableDesignersToUseStandardDataModelIfNeeded(
     namespaceToModify: string,
     userActionMessage: string[]
-  ): Promise<string[]> {
+  ): Promise<boolean> {
     try {
       Logger.logVerbose(this.messages.getMessage('settingDesignersToStandardModel'));
 
@@ -92,11 +95,7 @@ export class PostMigrate extends BaseMigrationTool {
 
       if (isAlreadyEnabled) {
         Logger.logVerbose(this.messages.getMessage('standardDesignerAlreadyEnabled', [namespaceToModify]));
-
-        // Even though designer is already enabled, we should still enable Standard Runtime
-        await this.enableStandardRuntimeIfNeeded(userActionMessage);
-
-        return userActionMessage;
+        return true;
       }
 
       const apexCode = `
@@ -113,39 +112,27 @@ export class PostMigrate extends BaseMigrationTool {
         userActionMessage.push(this.messages.getMessage('manuallySwitchDesignerToStandardDataModel'));
 
         // Do NOT attempt to enable standard runtime when setup fails
-        return userActionMessage;
+        return false;
       } else if (result?.success === true) {
         Logger.logVerbose(this.messages.getMessage('designersSetToStandardModel'));
-        Logger.logVerbose(this.messages.getMessage('enableStandardRuntimeAfterDesigner'));
-
-        // Enable Standard OmniStudio Runtime after successful standard designer setup
-        await this.enableStandardRuntimeIfNeeded(userActionMessage);
+        return true;
       } else {
         // Handle unexpected result structure
         Logger.error('Received unexpected response from Apex execution - unable to determine success status');
         userActionMessage.push(this.messages.getMessage('manuallySwitchDesignerToStandardDataModel'));
-
-        // Do NOT attempt to enable standard runtime when result is unclear
-        return userActionMessage;
+        return false;
       }
     } catch (ex) {
       const errorDetails = ex instanceof Error ? ex.message : JSON.stringify(ex);
-      Logger.error(this.messages.getMessage('exceptionSettingDesignersToStandardModel', [errorDetails]));
+      Logger.error(this.messages.getMessage('exceptionSettingDesignersToStandardDataModel', [errorDetails]));
       Logger.logVerbose(this.messages.getMessage('skipStandardRuntimeDueToFailure'));
       userActionMessage.push(this.messages.getMessage('manuallySwitchDesignerToStandardDataModel'));
-
-      // Do NOT attempt to enable standard runtime when exception occurs
+      return false;
     }
-    return userActionMessage;
   }
 
   /**
    * Enables Standard OmniStudio Runtime if it's currently disabled.
-   *
-   * This method is called after successfully setting designers to use standard data model.
-   * It checks if Standard Runtime is already enabled and only enables it if disabled.
-   *
-   * @param userActionMessage Array to collect user action messages for manual intervention
    */
   private async enableStandardRuntimeIfNeeded(userActionMessage: string[]): Promise<void> {
     try {

test/migration/postMigrate.test.ts

@@ -53,6 +53,7 @@ describe('PostMigrate', () => {
       tooling: {
         executeAnonymous: sandbox.stub(),
       },
+      query: sandbox.stub(),
     } as unknown as Connection;
 
     // Mock logger
@@ -92,6 +93,10 @@ describe('PostMigrate', () => {
       .withArgs('errorRevertingExperienceBundleMetadataAPI')
       .returns('Error reverting Experience Bundle Metadata API');
     getMessageStub.withArgs('errorDeployingComponents').returns('Error deploying components');
+    // New messages referenced by implementation
+    getMessageStub.withArgs('checkingStandardDesignerStatus', [testNamespace]).returns('Checking designer status');
+    getMessageStub.withArgs('standardDesignerAlreadyEnabled', [testNamespace]).returns('Designer already enabled');
+    getMessageStub.withArgs('skipStandardRuntimeDueToFailure').returns('Skip runtime due to failure');
 
     // Mock Logger static methods
     sandbox.stub(Logger, 'logVerbose');
@@ -156,7 +161,7 @@ describe('PostMigrate', () => {
       expect(deployerStub.called).to.be.false;
     });
 
-    xit('should handle deployment errors gracefully', () => {
+    it('should handle deployment errors gracefully', () => {
       // Arrange
       const error = new Error('Deployment failed');
       const deployerStub = sandbox.stub(Deployer.prototype, 'deploy').throws(error);
@@ -229,7 +234,10 @@ describe('PostMigrate', () => {
       const logVerboseStub = Logger.logVerbose as sinon.SinonStub;
 
       // Act
-      const result = await postMigrate.setDesignersToUseStandardDataModel(namespaceToModify, userActionMessage);
+      const result = await postMigrate.enableDesignersToUseStandardDataModelIfNeeded(
+        namespaceToModify,
+        userActionMessage
+      );
 
       // Assert
       expect(anonymousApexRunnerStub.calledOnce).to.be.true;
@@ -239,7 +247,8 @@ describe('PostMigrate', () => {
       );
       expect(logVerboseStub.calledWith('Setting designers to standard model...')).to.be.true;
       expect(logVerboseStub.calledWith('Designers set to standard model')).to.be.true;
-      expect(result).to.deep.equal(userActionMessage);
+      expect(result).to.equal(true);
+      expect(userActionMessage).to.deep.equal([]);
     });
 
     it('should handle unsuccessful anonymous apex execution', async () => {
@@ -258,12 +267,16 @@ describe('PostMigrate', () => {
       const anonymousApexRunnerStub = sandbox.stub(AnonymousApexRunner, 'run').resolves(mockResult);
 
       // Act
-      const result = await postMigrate.setDesignersToUseStandardDataModel(namespaceToModify, userActionMessage);
+      const result = await postMigrate.enableDesignersToUseStandardDataModelIfNeeded(
+        namespaceToModify,
+        userActionMessage
+      );
 
       // Assert
       expect(anonymousApexRunnerStub.calledOnce).to.be.true;
       expect(logErrorStub.called).to.be.true;
-      expect(result).to.include('Please manually switch designer to standard data model');
+      expect(result).to.equal(false);
+      expect(userActionMessage).to.include('Please manually switch designer to standard data model');
     });
 
     it('should handle exceptions during execution', async () => {
@@ -274,12 +287,78 @@ describe('PostMigrate', () => {
       const anonymousApexRunnerStub = sandbox.stub(AnonymousApexRunner, 'run').rejects(error);
 
       // Act
-      const result = await postMigrate.setDesignersToUseStandardDataModel(namespaceToModify, userActionMessage);
+      const result = await postMigrate.enableDesignersToUseStandardDataModelIfNeeded(
+        namespaceToModify,
+        userActionMessage
+      );
 
       // Assert
       expect(anonymousApexRunnerStub.calledOnce).to.be.true;
       expect(logErrorStub.called).to.be.true;
-      expect(result).to.include('Please manually switch designer to standard data model');
+      expect(result).to.equal(false);
+      expect(userActionMessage).to.include('Please manually switch designer to standard data model');
+    });
+
+    it('should return true when standard designer already enabled and skip Apex', async () => {
+      // Arrange
+      const namespaceToModify = 'test_namespace';
+      const userActionMessage: string[] = [];
+      // Stub SOQL query to indicate designer already enabled for this namespace
+      (connection.query as unknown as sinon.SinonStub).resolves({
+        totalSize: 2,
+        records: [
+          { DeveloperName: 'TheFirstInstalledOmniPackage', Value: namespaceToModify },
+          { DeveloperName: 'InstalledIndustryPackage', Value: 'other' },
+        ],
+      });
+      const anonymousApexRunnerStub = sandbox.stub(AnonymousApexRunner, 'run');
+
+      // Act
+      const result = await (postMigrate as any).enableDesignersToUseStandardDataModelIfNeeded(
+        namespaceToModify,
+        userActionMessage
+      );
+
+      // Assert
+      expect(result).to.equal(true);
+      expect(anonymousApexRunnerStub.called).to.be.false;
+      expect(userActionMessage).to.deep.equal([]);
+    });
+  });
+
+  describe('executeTasks', () => {
+    it('should enable runtime when designer step succeeds', async () => {
+      // Arrange
+      const enableDesignerStub = sandbox
+        .stub(postMigrate as any, 'enableDesignersToUseStandardDataModelIfNeeded')
+        .resolves(true);
+      const enableRuntimeSpy = sandbox.stub(postMigrate as any, 'enableStandardRuntimeIfNeeded').resolves();
+      const actionItems: string[] = [];
+
+      // Act
+      const res = await (postMigrate as any).executeTasks(testNamespace, actionItems);
+
+      // Assert
+      expect(enableDesignerStub.calledOnce).to.be.true;
+      expect(enableRuntimeSpy.calledOnce).to.be.true;
+      expect(res).to.equal(actionItems);
+    });
+
+    it('should not enable runtime when designer step fails', async () => {
+      // Arrange
+      const enableDesignerStub = sandbox
+        .stub(postMigrate as any, 'enableDesignersToUseStandardDataModelIfNeeded')
+        .resolves(false);
+      const enableRuntimeSpy = sandbox.stub(postMigrate as any, 'enableStandardRuntimeIfNeeded').resolves();
+      const actionItems: string[] = [];
+
+      // Act
+      const res = await (postMigrate as any).executeTasks(testNamespace, actionItems);
+
+      // Assert
+      expect(enableDesignerStub.calledOnce).to.be.true;
+      expect(enableRuntimeSpy.called).to.be.false;
+      expect(res).to.equal(actionItems);
     });
   });
 
@@ -465,7 +544,7 @@ describe('PostMigrate', () => {
 
       // Act
       postMigrate.deploy();
-      await postMigrate.setDesignersToUseStandardDataModel('test_namespace', []);
+      await (postMigrate as any).enableDesignersToUseStandardDataModelIfNeeded('test_namespace', []);
       await postMigrate.restoreExperienceAPIMetadataSettings({ value: true }, []);
 
       // Assert
@@ -500,7 +579,7 @@ describe('PostMigrate', () => {
 
       // Act
       postMigrateNoDeploy.deploy();
-      await postMigrateNoDeploy.setDesignersToUseStandardDataModel('test_namespace', []);
+      await (postMigrateNoDeploy as any).enableDesignersToUseStandardDataModelIfNeeded('test_namespace', []);
 
       // Assert
       expect(deployerStub.called).to.be.false;