Configuration used: CodeRabbit UI
Review profile: CHILL

Ginger/GingerCoreNET/GeneralLib/General.cs (2)

751-751: Add XML documentation for the Type property assignment.

The Type property assignment is crucial for the external field mapping functionality. Consider adding a comment to explain its significance.

+                // Preserve the field type from the external item field for proper RQM integration
                Type = externalItemField.Type,

---

Line range hint 734-763: Consider enhancing the MapExternalField method.

The method could benefit from the following improvements:

1. Add XML documentation to describe its purpose and parameters
2. Consider making it public to improve reusability
3. Add validation for required fields

Here's the suggested implementation:

-        private static ExternalItemFieldBase MapExternalField(ExternalItemFieldBase externalItemField)
+        /// <summary>
+        /// Maps properties from an external item field to a new instance while preserving existing selected values.
+        /// </summary>
+        /// <param name="externalItemField">The source external item field to map from.</param>
+        /// <returns>A new instance of ExternalItemFieldBase with mapped properties.</returns>
+        /// <exception cref="ArgumentNullException">Thrown when externalItemField is null.</exception>
+        public static ExternalItemFieldBase MapExternalField(ExternalItemFieldBase externalItemField)
         {
+            if (externalItemField == null)
+            {
+                throw new ArgumentNullException(nameof(externalItemField));
+            }
+
+            if (string.IsNullOrEmpty(externalItemField.Name))
+            {
+                throw new ArgumentException("Field name is required", nameof(externalItemField));
+            }
+
             var existingField = WorkSpace.Instance.Solution.ExternalItemsFields
                 .FirstOrDefault(x => x.Name.Equals(externalItemField.Name, StringComparison.CurrentCultureIgnoreCase));

             return new ExternalItemFieldBase
             {
                 Name = externalItemField.Name,
                 ID = externalItemField.ID,
                 ItemType = externalItemField.ItemType,
                 Type = externalItemField.Type,
                 Guid = externalItemField.Guid,
                 IsCustomField = externalItemField.IsCustomField,
                 SelectedValue = existingField != null && !string.IsNullOrEmpty(existingField.SelectedValue)
                     ? existingField.SelectedValue
                     : externalItemField.SelectedValue
             };
         }

Ginger/GingerCoreNET/ALMLib/RQM/ExportToRQM.cs (2)

111-118: Improve error message specificity.

The validation for predefined external fields is good, but the error message could be more specific about what TestCase fields are required.

Consider enhancing the error message to list the required TestCase fields:

-Reporter.ToUser(eUserMsgKey.StaticInfoMessage, "Current solution have no predefined values for RQM's mandatory fields. Please configure before doing export. ('ALM'-'ALM Items Fields Configuration')");
+Reporter.ToUser(eUserMsgKey.StaticInfoMessage, "Current solution has no predefined values for RQM's mandatory TestCase fields. Required fields: [list required fields here]. Please configure these fields in 'ALM'-'ALM Items Fields Configuration'.");

---

1142-1145: Fix indentation for consistency.

The indentation of these lines is inconsistent with the surrounding code.

-                        if (!propertiesList.ContainsKey(itemField.Name))
-                        {
-                            propertiesList.Add(itemField.Name, itemField.SelectedValue);
-                        }
+                    if (!propertiesList.ContainsKey(itemField.Name))
+                    {
+                        propertiesList.Add(itemField.Name, itemField.SelectedValue);
+                    }

Ginger/GingerCoreNET/ALMLib/RQM/ImportFromRQM.cs (2)

1364-1381: Extract duplicated XML namespace handling logic into a helper method.

The XML namespace handling code is duplicated. Consider extracting it into a reusable helper method to improve maintainability and reduce code duplication.

+ private static string GetCustomAttributeMandatory(XmlDocument customAttributeListing)
+ {
+     XmlNamespaceManager nsManager = new XmlNamespaceManager(customAttributeListing.NameTable);
+     nsManager.AddNamespace("ns2", "http://jazz.net/xmlns/alm/qm/v0.1/");
+     XmlNode requiredNode = customAttributeListing.SelectSingleNode("//ns2:required", nsManager);
+     return requiredNode?.InnerText ?? "false";
+ }

Usage:

- XmlNamespaceManager nsManager = new XmlNamespaceManager(CustomAttributeListing.NameTable);
- nsManager.AddNamespace("ns2", "http://jazz.net/xmlns/alm/qm/v0.1/");
- string xpath = "//ns2:required";
- XmlNode requiredNode = CustomAttributeListing.SelectSingleNode(xpath, nsManager);
- if (requiredNode != null)
- {
-     CustomAttributeMandatory = CustomAttributeListing.GetElementsByTagName("ns2:required").Item(0).InnerText;
- }
- else
- {
-     CustomAttributeMandatory = "false";
- }
+ CustomAttributeMandatory = GetCustomAttributeMandatory(CustomAttributeListing);

Also applies to: 1460-1477

---

1485-1513: Use constants for custom attribute types.

String literals for types are used in multiple places. Consider defining constants to improve maintainability and prevent typos.

+ private static class CustomAttributeTypes
+ {
+     public const string INTEGER = "INTEGER";
+     public const string MEDIUM_STRING = "MEDIUMSTRING";
+     public const string SMALL_STRING = "SMALLSTRING";
+     public const string TIMESTAMP = "TIMESTAMP";
+ }

- if(itemfield.Type.Equals("INTEGER",StringComparison.CurrentCultureIgnoreCase))
+ if(itemfield.Type.Equals(CustomAttributeTypes.INTEGER, StringComparison.CurrentCultureIgnoreCase))

Configuration used: CodeRabbit UI
Review profile: CHILL

- itemfield.SelectedValue = DateTime.Now.ToString("yyyy-mm-dd");
+ itemfield.SelectedValue = DateTime.Now.ToString("yyyy-MM-dd");

Ginger/GingerCoreNET/ALMLib/RQM/ExportToRQM.cs (2)

928-937: Improved duplicate detection in external fields conversion.

The updated logic now properly handles field properties and uses ID-based comparison to prevent duplicates. Consider adding null checks for the field properties to improve robustness.

 field.ID = fields[indx].ID;
-field.Type = fields[indx].Type;
-field.TypeIdentifier = fields[indx].TypeIdentifier;
-field.IsMultiple = fields[indx].IsMultiple;
+field.Type = fields[indx].Type ?? string.Empty;
+field.TypeIdentifier = fields[indx].TypeIdentifier ?? string.Empty;
+field.IsMultiple = fields[indx].IsMultiple;

---

Line range hint 789-818: Consider using more specific exception types.

The current implementation uses generic Exception catching. Consider catching specific exceptions to handle different error scenarios appropriately.

-try
-{
-    //Create (RQM)TestCase for each Ginger ActivityGroup and add it to RQM TestCase List
-    testPlan.Activities = [];//3
-    foreach (ActivitiesGroup ag in businessFlow.ActivitiesGroups)
-    {
-        if (projEnvironment != null)
-        {
-            IValueExpression magVE = new GingerCore.ValueExpression(projEnvironment, businessFlow, [], false, "", false);
-            ag.CalculateExternalId(magVE);
-        }
-        testPlan.Activities.Add(GetTestCaseFromActivityGroup(ag));
-    }
-}
-catch (Exception ex)
+try
+{
+    //Create (RQM)TestCase for each Ginger ActivityGroup and add it to RQM TestCase List
+    testPlan.Activities = [];//3
+    foreach (ActivitiesGroup ag in businessFlow.ActivitiesGroups)
+    {
+        if (projEnvironment != null)
+        {
+            IValueExpression magVE = new GingerCore.ValueExpression(projEnvironment, businessFlow, [], false, "", false);
+            ag.CalculateExternalId(magVE);
+        }
+        testPlan.Activities.Add(GetTestCaseFromActivityGroup(ag));
+    }
+}
+catch (InvalidOperationException ex)
+{
+    Reporter.ToLog(eLogLevel.ERROR, $"Failed to calculate external ID: {ex.Message}", ex);
+    throw;
+}
+catch (Exception ex)

Ginger/GingerCoreNET/ALMLib/RQM/ImportFromRQM.cs (1)

1509-1536: Inconsistent null handling for SelectedValue.

The code has redundant null checks and assignments for SelectedValue. The default value assignment should be consolidated into a single location.

Consider refactoring to:

- if (CustomAttributeMandatory.Equals("true",StringComparison.CurrentCultureIgnoreCase))
- {
-     // ... existing type-specific value assignments ...
- }
- else
- {
-     itemfield.ToUpdate = false;
-     itemfield.Mandatory = false;
- }
- if (itemfield.SelectedValue == null)
- {
-     itemfield.SelectedValue = "Unassigned";
- }

+ if (CustomAttributeMandatory.Equals("true",StringComparison.CurrentCultureIgnoreCase))
+ {
+     // ... existing type-specific value assignments ...
+ }
+ else
+ {
+     itemfield.ToUpdate = false;
+     itemfield.Mandatory = false;
+     itemfield.SelectedValue = "Unassigned";
+ }

Configuration used: CodeRabbit UI
Review profile: CHILL

Ginger/GingerCoreNET/GeneralLib/General.cs (1)

754-781: Simplify the nested conditional logic.

The current implementation has deeply nested if conditions that make the code hard to follow. Consider simplifying the logic by combining conditions and extracting the value determination logic into a separate method.

-if (existingField == null)
-{
-    if (externalItemField.Mandatory)
-    {
-        if (!string.IsNullOrEmpty(externalItemField.SelectedValue))
-        {
-            value = externalItemField.SelectedValue;
-        }
-        else
-        {
-            value = GetDefaultValue(externalItemField);
-        }
-    }
-}
-else
-{
-    if (externalItemField.Mandatory)
-    {
-        if (!string.IsNullOrEmpty(existingField.SelectedValue))
-        {
-            value = existingField.SelectedValue;
-        }
-        else
-        {
-            value = GetDefaultValue(externalItemField);
-        }
-    }
-}
+string value = string.Empty;
+if (externalItemField.Mandatory)
+{
+    value = existingField?.SelectedValue ?? externalItemField.SelectedValue;
+    if (string.IsNullOrEmpty(value))
+    {
+        value = GetDefaultValue(externalItemField);
+    }
+}

Configuration used: CodeRabbit UI
Review profile: CHILL