fix(revit): skips category assignment when category already exists on…

… send (#2923) * fixes category assignment logic * Update ConversionUtils.cs * Update ConversionUtils.cs * Update ConversionUtils.cs * removes switch in favor of in-line comments * Update ConversionUtils.cs
specklesystems · Sep 14, 2023 · df09e01 · df09e01
1 parent 0592ce3
commit df09e01
Showing 1 changed file with 36 additions and 33 deletions.
diff --git a/Objects/Converters/ConverterRevit/ConverterRevitShared/ConversionUtils.cs b/Objects/Converters/ConverterRevit/ConverterRevitShared/ConversionUtils.cs
@@ -251,17 +251,18 @@ public void GetAllRevitParamsAndIds(Base speckleElement, DB.Element revitElement
 
       speckleElement["worksetId"] = revitElement.WorksetId.ToString();
 
-      // for the category, we should be mirroring how we handle parsing of built-in-categories for revit 2023/2024
-      // this is because different built-in-categories may have the same name, eg "OST_Railings" and "OST_StairsRailing" both have a Category name of "Railing"
+      // assign the category if it is null
+      // WARN: DirectShapes have a `category` prop of type `RevitCategory` (enum), NOT `string`. This is the only exception as of 2.16.
+      // If the null check is removed, the DirectShape case needs to be handled.
       var category = revitElement.Category;
-      if (category is not null)
+      if (speckleElement["category"] is null && category is not null)
       {
-        var categoryName = revitElement.Category.Name;
+        var categoryName = category.Name;
+        // we should use RevitCategory values for BuiltInCategory strings where possible (revit 2023+)
+        // different BuiltInCategory may have the same name, eg "OST_Railings" and "OST_StairsRailing" both have a Category name of "Railing"
 #if !(REVIT2020 || REVIT2021 || REVIT2022)
         if (Categories.GetRevitCategoryFromBuiltInCategory(category.BuiltInCategory, out RevitCategory c))
-        {
           categoryName = c.ToString();
-        }
 #endif
         speckleElement["category"] = categoryName;
       }
@@ -345,29 +346,32 @@ private Parameter ParameterToSpeckle(
       // TODO : could add some generic getOrAdd overloads to avoid creating closures
       var paramData = revitDocumentAggregateCache
         .GetOrInitializeEmptyCacheOfType<ParameterToSpeckleData>(out _)
-        .GetOrAdd(paramInternalName, () =>
-        {
-          var definition = rp.Definition;
-          var newParamData = new ParameterToSpeckleData()
+        .GetOrAdd(
+          paramInternalName,
+          () =>
           {
-            Definition = definition,
-            InternalName = paramInternalName,
-            IsReadOnly = rp.IsReadOnly,
-            IsShared = rp.IsShared,
-            IsTypeParameter = isTypeParameter,
-            Name = definition.Name,
-            UnitType = definition.GetUnityTypeString(),
-          };
-          if (rp.StorageType == StorageType.Double)
-          {
-            unitTypeId = rp.GetUnitTypeId();
-            newParamData.UnitsSymbol = GetSymbolUnit(rp, definition, unitTypeId);
-            newParamData.ApplicationUnits = unitsOverride != null
-              ? UnitsToNative(unitsOverride).ToUniqueString()
-              : unitTypeId.ToUniqueString();
-          }
-          return newParamData;
-        }, out _);
+            var definition = rp.Definition;
+            var newParamData = new ParameterToSpeckleData()
+            {
+              Definition = definition,
+              InternalName = paramInternalName,
+              IsReadOnly = rp.IsReadOnly,
+              IsShared = rp.IsShared,
+              IsTypeParameter = isTypeParameter,
+              Name = definition.Name,
+              UnitType = definition.GetUnityTypeString(),
+            };
+            if (rp.StorageType == StorageType.Double)
+            {
+              unitTypeId = rp.GetUnitTypeId();
+              newParamData.UnitsSymbol = GetSymbolUnit(rp, definition, unitTypeId);
+              newParamData.ApplicationUnits =
+                unitsOverride != null ? UnitsToNative(unitsOverride).ToUniqueString() : unitTypeId.ToUniqueString();
+            }
+            return newParamData;
+          },
+          out _
+        );
 
       return paramData.GetParameterObjectWithValue(rp.GetValue(paramData.Definition, unitTypeId));
     }
@@ -382,9 +386,7 @@ private Parameter ParameterToSpeckle(
     /// <param name="cache"></param>
     /// <param name="forgeTypeId"></param>
     /// <returns></returns>
-    public string GetSymbolUnit(
-      DB.Parameter parameter,
-      DB.Definition definition,
+    public string GetSymbolUnit(DB.Parameter parameter, DB.Definition definition,
 #if REVIT2020
       DisplayUnitType unitTypeId
 #else
@@ -619,7 +621,7 @@ private Phase GetRevitPhase(DB.Document document, string phaseName)
 
       var cachedIds = PreviouslyReceivedObjectIds?.GetCreatedIdsFromConvertedId(applicationId);
       // TODO: we may not want just the first one
-      return  cachedIds == null ? null : Doc.GetElement(cachedIds.First());
+      return cachedIds == null ? null : Doc.GetElement(cachedIds.First());
     }
 
     public IEnumerable<DB.Element?> GetExistingElementsByApplicationId(string applicationId)
@@ -628,7 +630,8 @@ private Phase GetRevitPhase(DB.Document document, string phaseName)
         yield break;
 
       var cachedIds = PreviouslyReceivedObjectIds?.GetCreatedIdsFromConvertedId(applicationId);
-      if (cachedIds == null) yield break;
+      if (cachedIds == null)
+        yield break;
       foreach (var id in cachedIds)
         yield return Doc.GetElement(id);
     }