fix: @W-19837693: Address the code review for delete case of OS and FC

Changes:
- Handle the delete use case with multiple versions of OS and FC
- Added test cases
- Updated the messages

TODO:
- Update the labels for CX review

Author: sf-kishore-kurri

messages/assess.json

@@ -20,7 +20,7 @@
   "duplicatedDrName": "Data Mapper with duplicate name found in this org. Rename your Data Mapper and try again.",
   "duplicatedOSName": "Omniscript with duplicate name, type, subtype, or language found in this org. Modify your Omniscript and try again.",
   "duplicatedName": "Duplicated name %s",
-  "lowerVersionDuplicateOmniscriptName": "A %s with name \"%s\" will not be migrated because lower version of same %s is marked as duplicate, which could lead to conflicts during migration.",
+  "lowerVersionDuplicateOmniscriptName": "A %s with name \"%s\" will not be migrated because lower version of same %s will be marked as duplicate, which could lead to conflicts during migration.",
   "errorWhileActivatingOs": "We couldn't activate your %s:",
   "errorWhileActivatingCard": "We couldn't activate your Flexcard:",
   "errorWhileUploadingCard": "We couldn't deploy your Flexcard to your org. Review these errors:",
@@ -39,10 +39,10 @@
   "processingDataRaptor": "Processing Data Mapper: %s",
   "processingOmniScript": "Processing Omniscript: %s",
   "foundDataRaptorsToAssess": "Found %s Data Mappers to assess.",
-  "foundOmniScriptsToAssess": "Found %s Omniscripts to assess",
+  "foundOmniScriptsToAssess": "Found %s %s to assess",
   "foundGlobalAutoNumbersToAssess": "Found %s Omni Global Auto Numbers to assess.",
   "startingDataRaptorAssessment": "Starting Data Mapper assessment",
-  "startingOmniScriptAssessment": "Starting Omniscript assessment",
+  "startingOmniScriptAssessment": "Starting %s assessment",
   "startingGlobalAutoNumberAssessment": "Starting Omni Global Auto Number assessment",
   "allVersionsInfo": "All versions: %s",
   "assessmentInitialization": "Assessment process started. Using namespace: %s",
@@ -98,7 +98,7 @@
   "integrationProcedureNameChangeMessage": "The Integration Procedure %s will be renamed to %s during migration.",
   "integrationProcedureManualUpdateMessage": "All references to %s in this Integration Procedure must be manually updated after migration.",
   "duplicateCardNameMessage": "A Flexcard with the same name \"%s\" already exists after name cleaning, which could lead to conflicts during migration.",
-  "lowerVersionDuplicateCardNameMessage": "A Flexcard with the name \"%s\" will not be migrated because lower version of same card is marked as duplicate, which could lead to conflicts during migration.",
+  "lowerVersionDuplicateCardNameMessage": "A Flexcard with the name \"%s\" will not be migrated because lower version of same card will be marked as duplicate, which could lead to conflicts during migration.",
   "startingCustomLabelAssessment": "Starting Custom Label assessment",
   "assessedCustomLabelsCount": "Found %s labels that need attention out of %s total",
   "customLabelAssessmentCompleted": "Custom Label assessment completed",

messages/migrate.json

@@ -19,11 +19,11 @@
   "couldNotTruncate": "We couldn't clear all %s records.",
   "couldNotTruncateOmnniProcess": "We couldn't clear all records of your %s because the %s is referenced in an Omniscript or a Flexcard.",
   "invalidOrRepeatingOmniscriptElementNames": "Omniscript with invalid or duplicate element names found. Rename your Omniscript elements and try again.",
-  "lowerVersionDuplicateCardName": "Flexcard with name %s can't be migrated because lower version of same card is marked as duplicate. Rename your Flexcard and try again.",
+  "lowerVersionDuplicateCardName": "Flexcard with name %s can't be migrated because lower version of same card is probable duplicate. Rename your Flexcard and try again.",
   "duplicatedCardName": "Flexcard with duplicate name %s found in this org. Rename your Flexcard and try again.",
   "duplicatedDrName": "Data Mapper with duplicate name found in this org. Rename your Data Mapper and try again.",
   "duplicatedOSName": "Omniscript with duplicate name, type, subtype, or language found in this org. Modify your Omniscript and try again.",
-  "lowerVersionDuplicateOSName": "%s with name %s can't be migrated because lower version of same %s is marked as duplicate. Rename your %s and try again.",
+  "lowerVersionDuplicateOSName": "%s with name %s can't be migrated because lower version of same %s is probable duplicate. Rename your %s and try again.",
   "errorWhileActivatingOs": "We couldn't activate your %s:",
   "errorWhileActivatingCard": "We couldn't activate your Flexcard:",
   "errorWhileUploadingCard": "We couldn't deploy your Flexcard to your org. Review these errors:",

src/migration/flexcard.ts

@@ -165,7 +165,7 @@ export class CardMigrationTool extends BaseMigrationTool implements MigrationToo
     const progressBar = createProgressBar('Assessing', 'Flexcards');
     progressBar.start(flexCards.length, progressCounter);
     const uniqueNames = new Set<string>();
-    const dupFlexCardNames: Set<string> = new Set<string>();
+    const dupFlexCardNames: Map<string, string> = new Map<string, string>();
 
     // Now process each OmniScript and its elements
     for (const flexCard of flexCards) {
@@ -200,7 +200,7 @@ export class CardMigrationTool extends BaseMigrationTool implements MigrationToo
   private async processFlexCard(
     flexCard: AnyJson,
     uniqueNames: Set<string>,
-    dupFlexCardNames: Set<String>
+    dupFlexCardNames: Map<string, string>
   ): Promise<FlexCardAssessmentInfo> {
     const flexCardName = flexCard['Name'];
     Logger.info(this.messages.getMessage('processingFlexCard', [flexCardName]));
@@ -235,24 +235,30 @@ export class CardMigrationTool extends BaseMigrationTool implements MigrationToo
     }
 
     // Check for duplicate names (include version when allVersions is true)
-    // A Set that can only store original flexcard names to avoid conflicts with cleaned name
     const uniqueCleanedName = this.allVersions ? `${cleanedName}_${version}` : cleanedName;
-    if (uniqueNames.has(uniqueCleanedName) || dupFlexCardNames.has(originalName)) {
-      // Add the original flexcard name to the set
-      if (this.allVersions && !dupFlexCardNames.has(originalName)) {
-        dupFlexCardNames.add(originalName);
-      }
-      if (uniqueNames.has(uniqueCleanedName)) {
-        flexCardAssessmentInfo.warnings.push(this.messages.getMessage('duplicateCardNameMessage', [uniqueCleanedName]));
-      } else {
+
+    // Check for exact duplicate (same name + same version)
+    if (uniqueNames.has(uniqueCleanedName)) {
+      flexCardAssessmentInfo.warnings.push(this.messages.getMessage('duplicateCardNameMessage', [uniqueCleanedName]));
+      assessmentStatus = getUpdatedAssessmentStatus(assessmentStatus, 'Needs manual intervention');
+    }
+    // Check for naming conflict: different original names cleaning to same name
+    else if (this.allVersions && dupFlexCardNames.has(cleanedName)) {
+      const existingOriginalName = dupFlexCardNames.get(cleanedName);
+      // Only flag if the original names are different (indicates a naming conflict)
+      if (existingOriginalName !== originalName) {
         flexCardAssessmentInfo.warnings.push(
           this.messages.getMessage('lowerVersionDuplicateCardNameMessage', [uniqueCleanedName])
         );
+        assessmentStatus = getUpdatedAssessmentStatus(assessmentStatus, 'Needs manual intervention');
       }
-      assessmentStatus = getUpdatedAssessmentStatus(assessmentStatus, 'Needs manual intervention');
     }
 
+    // Add to tracking structures
     uniqueNames.add(uniqueCleanedName);
+    if (this.allVersions && !dupFlexCardNames.has(cleanedName)) {
+      dupFlexCardNames.set(cleanedName, originalName);
+    }
 
     // Check for author name changes
     const originalAuthor = flexCard[this.namespacePrefix + 'Author__c'];
@@ -738,7 +744,8 @@ export class CardMigrationTool extends BaseMigrationTool implements MigrationToo
     const cardsUploadInfo = new Map<string, UploadRecordResult>();
     const originalRecords = new Map<string, any>();
     const uniqueNames = new Set<string>();
-    const dupFlexCardNames: Set<string> = new Set<string>();
+    // Map to track cleanedName -> originalName for duplicate detection
+    const dupFlexCardNames: Map<string, string> = new Map<string, string>();
 
     let progressCounter = 0;
     progressBar.start(cards.length, progressCounter);
@@ -760,7 +767,7 @@ export class CardMigrationTool extends BaseMigrationTool implements MigrationToo
     cardsUploadInfo: Map<string, UploadRecordResult>,
     originalRecords: Map<string, any>,
     uniqueNames: Set<string>,
-    dupFlexCardNames: Set<String>
+    dupFlexCardNames: Map<string, string>
   ) {
     const recordId = card['Id'];
 
@@ -802,22 +809,31 @@ export class CardMigrationTool extends BaseMigrationTool implements MigrationToo
         ? `${transformedCard['Name']}_${transformedCard['VersionNumber']}`
         : transformedCard['Name'];
 
-      if (uniqueNames.has(uniqueCheckName) || dupFlexCardNames.has(transformedCard['Name'])) {
-        if (this.allVersions && !dupFlexCardNames.has(transformedCard['Name'])) {
-          dupFlexCardNames.add(transformedCard['Name']);
-        }
-        // change error message
-        if (uniqueNames.has(uniqueCheckName)) {
-          this.setRecordErrors(card, this.messages.getMessage('duplicatedCardName', [uniqueCheckName]));
-        } else {
-          this.setRecordErrors(card, this.messages.getMessage('lowerVersionDuplicateCardName', [uniqueCheckName]));
-        }
+      const originalCardName = card['Name'];
+      const cleanedCardName = transformedCard['Name'];
+
+      // Check for exact duplicate (same name + same version)
+      if (uniqueNames.has(uniqueCheckName)) {
+        this.setRecordErrors(card, this.messages.getMessage('duplicatedCardName', [uniqueCheckName]));
         originalRecords.set(recordId, card);
         return;
       }
+      // Check for naming conflict: different original names cleaning to same name
+      else if (this.allVersions && dupFlexCardNames.has(cleanedCardName)) {
+        const existingOriginalName = dupFlexCardNames.get(cleanedCardName);
+        // Only flag if the original names are different (indicates a naming conflict)
+        if (existingOriginalName !== originalCardName) {
+          this.setRecordErrors(card, this.messages.getMessage('lowerVersionDuplicateCardName', [uniqueCheckName]));
+          originalRecords.set(recordId, card);
+          return;
+        }
+      }
 
-      // Save the name for duplicated names check (with version if allVersions is true)
+      // Add to tracking structures
       uniqueNames.add(uniqueCheckName);
+      if (this.allVersions && !dupFlexCardNames.has(cleanedCardName)) {
+        dupFlexCardNames.set(cleanedCardName, originalCardName);
+      }
 
       // Create a map of the original records
       originalRecords.set(recordId, card);

src/migration/omniscript.ts

@@ -238,7 +238,8 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
     const ipAssessmentInfos: IPAssessmentInfo[] = [];
 
     // Create a set to store existing OmniScript names and also extract DataRaptor and FlexCard names
-    const duplicateOmniscriptNames = new Set<string>();
+    // Map to track cleanedName (without version) -> originalName (without version) for duplicate detection
+    const duplicateOmniscriptNames: Map<string, string> = new Map<string, string>();
     const existingOmniscriptNames = new Set<string>();
     const existingDataRaptorNames = new Set(dataRaptorAssessmentInfos.map((info) => info.name));
     const existingFlexCardNames = new Set(flexCardAssessmentInfos.map((info) => info.name));
@@ -359,7 +360,7 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
     existingOmniscriptNames: Set<string>,
     existingDataRaptorNames: Set<string>,
     existingFlexCardNames: Set<string>,
-    duplicateOmniscriptNames: Set<string>
+    duplicateOmniscriptNames: Map<string, string>
   ): Promise<OSAssessmentInfo> {
     const elements = await this.getAllElementsForOmniScript(omniscript['Id']);
 
@@ -570,47 +571,43 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
       );
       assessmentStatus = 'Warnings';
     }
-    // Duplicate check logic:
-    // - When allVersions=false: check without version (using recordNameWithoutVersion)
-    // - When allVersions=true: check with version (using recordName)
+    // Duplicate check logic using Map to track originalName -> cleanedName
+    // This allows us to detect both exact duplicates and name cleaning conflicts
     const nameToCheck = this.allVersions ? recordName : recordNameWithoutVersion;
 
-    if (existingOmniscriptNames.has(nameToCheck) || duplicateOmniscriptNames.has(recordNameWithoutVersion)) {
-      // Mark the base name as duplicate for subsequent versions when allVersions=true
-      if (this.allVersions && !duplicateOmniscriptNames.has(recordNameWithoutVersion)) {
-        duplicateOmniscriptNames.add(recordNameWithoutVersion);
-      }
-
-      // Message selection logic:
-      // - When allVersions=false: always use "duplicatedName" (comparing without version)
-      // - When allVersions=true:
-      //   - If exact match in existingOmniscriptNames (same name+version): use "duplicatedName"
-      //   - If base name in duplicateOmniscriptNames (different version): use "lowerVersionDuplicateOmniscriptName"
-      let shouldUseDuplicatedNameMessage = false;
-
-      if (!this.allVersions) {
-        // allVersions=false: always use duplicatedName message
-        shouldUseDuplicatedNameMessage = true;
-      } else {
-        // allVersions=true: use duplicatedName only if exact match (same name+version)
-        shouldUseDuplicatedNameMessage = existingOmniscriptNames.has(recordName);
-      }
-
-      if (shouldUseDuplicatedNameMessage) {
-        warnings.push(this.messages.getMessage('duplicatedName', [recordName]));
-      } else {
+    // Get the original name parts (before cleaning)
+    const originalType = omniscript[this.namespacePrefix + 'Type__c'];
+    const originalSubType = omniscript[this.namespacePrefix + 'SubType__c'];
+    const originalLanguage = omniscript[this.namespacePrefix + 'Language__c'] || '';
+    const originalNameWithoutVersion = originalLanguage
+      ? `${originalType}_${originalSubType}_${originalLanguage}`
+      : `${originalType}_${originalSubType}`;
+
+    // Check for exact duplicate (same name + version)
+    if (existingOmniscriptNames.has(nameToCheck)) {
+      warnings.push(this.messages.getMessage('duplicatedName', [recordName]));
+      assessmentStatus = 'Needs manual intervention';
+    }
+    // Check for naming conflict: different original names cleaning to same name
+    else if (this.allVersions && duplicateOmniscriptNames.has(recordNameWithoutVersion)) {
+      const existingOriginalName = duplicateOmniscriptNames.get(recordNameWithoutVersion);
+      // Only flag if the original names are different (indicates a naming conflict)
+      if (existingOriginalName !== originalNameWithoutVersion) {
         warnings.push(
           this.messages.getMessage('lowerVersionDuplicateOmniscriptName', [
             this.getName(true),
             recordName,
             this.getName(true),
           ])
         );
+        assessmentStatus = 'Needs manual intervention';
       }
+    }
 
-      assessmentStatus = 'Needs manual intervention';
-    } else {
-      existingOmniscriptNames.add(nameToCheck);
+    // Add to tracking structures
+    existingOmniscriptNames.add(nameToCheck);
+    if (this.allVersions && !duplicateOmniscriptNames.has(recordNameWithoutVersion)) {
+      duplicateOmniscriptNames.set(recordNameWithoutVersion, originalNameWithoutVersion);
     }
 
     // Add warning for duplicate element names within the same OmniScript
@@ -736,7 +733,8 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
     populateRegexForFunctionMetadata(functionDefinitionMetadata);
 
     const duplicatedNames = new Set<string>();
-    const duplicateOmniscriptNames = new Set<string>();
+    // Map to track cleanedName (without version) -> originalName (without version) for duplicate detection
+    const duplicateOmniscriptNames: Map<string, string> = new Map<string, string>();
 
     // Variables to be returned After Migration
     let originalOsRecords = new Map<string, any>();
@@ -1048,24 +1046,17 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
           '_1';
       }
 
-      if (duplicatedNames.has(mappedOsName) || duplicateOmniscriptNames.has(mappedOsNameWithoutVersion)) {
-        originalOsRecords.set(recordId, omniscript);
-        if (this.allVersions && !duplicateOmniscriptNames.has(mappedOsNameWithoutVersion)) {
-          duplicateOmniscriptNames.add(mappedOsNameWithoutVersion);
-        }
-
-        let warningMessage: string;
-        if (duplicatedNames.has(mappedOsName)) {
-          warningMessage = this.messages.getMessage('duplicatedOSName', [this.getName(true), mappedOsName]);
-        } else {
-          warningMessage = this.messages.getMessage('lowerVersionDuplicateOSName', [
-            this.getName(true),
-            mappedOsName,
-            this.getName(true),
-            this.getName(true),
-          ]);
-        }
-
+      // Get original name parts for tracking
+      const originalType = omniscript[this.namespacePrefix + 'Type__c'];
+      const originalSubType = omniscript[this.namespacePrefix + 'SubType__c'];
+      const originalLanguage = omniscript[this.namespacePrefix + 'Language__c'] || '';
+      const originalNameWithoutVersion = originalLanguage
+        ? `${originalType}_${originalSubType}_${originalLanguage}`
+        : `${originalType}_${originalSubType}`;
+
+      // Check for exact duplicate (same name + same version)
+      if (duplicatedNames.has(mappedOsName)) {
+        const warningMessage = this.messages.getMessage('duplicatedOSName', [this.getName(true), mappedOsName]);
         const skippedResponse: UploadRecordResult = {
           referenceId: recordId,
           id: '',
@@ -1077,8 +1068,35 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
           skipped: true,
         };
         osUploadInfo.set(recordId, skippedResponse);
+        originalOsRecords.set(recordId, omniscript);
         continue;
       }
+      // Check for naming conflict: different original names cleaning to same name
+      else if (this.allVersions && duplicateOmniscriptNames.has(mappedOsNameWithoutVersion)) {
+        const existingOriginalName = duplicateOmniscriptNames.get(mappedOsNameWithoutVersion);
+        // Only flag if the original names are different (indicates a naming conflict)
+        if (existingOriginalName !== originalNameWithoutVersion) {
+          const warningMessage = this.messages.getMessage('lowerVersionDuplicateOSName', [
+            this.getName(true),
+            mappedOsName,
+            this.getName(true),
+            this.getName(true),
+          ]);
+          const skippedResponse: UploadRecordResult = {
+            referenceId: recordId,
+            id: '',
+            success: false,
+            hasErrors: false,
+            errors: [],
+            warnings: [warningMessage],
+            newName: '',
+            skipped: true,
+          };
+          osUploadInfo.set(recordId, skippedResponse);
+          originalOsRecords.set(recordId, omniscript);
+          continue;
+        }
+      }
 
       // Save the mapped record
       mappedRecords.push(mappedOmniScript);
@@ -1128,8 +1146,14 @@ export class OmniScriptMigrationTool extends BaseMigrationTool implements Migrat
             '_1';
         }
         // Always set the new name to show the migrated name
-        // Add the processednew name to the duplicated set
+        // Add the processed new name to the duplicated set
         duplicatedNames.add(mappedOsName);
+
+        // Add to map for tracking naming conflicts (only when allVersions=true)
+        if (this.allVersions && !duplicateOmniscriptNames.has(mappedOsNameWithoutVersion)) {
+          duplicateOmniscriptNames.set(mappedOsNameWithoutVersion, originalNameWithoutVersion);
+        }
+
         osUploadResponse.newName = mappedOsName;
 
         // Only add warning if the name was actually modified