From e0ca35904b8d7bfc643c79606f216eda55e2288b Mon Sep 17 00:00:00 2001
From: Alan Rynne <alan@speckle.systems>
Date: Fri, 3 Nov 2023 11:20:51 +0100
Subject: [PATCH] fix(rvt): Modify signature of
 `RevitConverter.UpdateParameter` to return AppObj (#3019)

* fix(rvt): Modify signature of `RevitConverter.UpdateParameter` to return AppObj

* fix: leftover formatting changes

* fix: No need for Report logging if we return AppObject

* fix: Remove unnecessary new Exception type
---
 .../ConverterRevitShared/ConversionUtils.cs   |  3 --
 .../ConverterRevitShared/ConverterRevit.cs    |  4 +-
 .../Partial Classes/UpdateParameter.cs        | 37 ++++++++++++-------
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/Objects/Converters/ConverterRevit/ConverterRevitShared/ConversionUtils.cs b/Objects/Converters/ConverterRevit/ConverterRevitShared/ConversionUtils.cs
index ce24870754..39dd8da75f 100644
--- a/Objects/Converters/ConverterRevit/ConverterRevitShared/ConversionUtils.cs
+++ b/Objects/Converters/ConverterRevit/ConverterRevitShared/ConversionUtils.cs
@@ -256,7 +256,6 @@ public void GetAllRevitParamsAndIds(Base speckleElement, DB.Element revitElement
 
       speckleElement["worksetId"] = revitElement.WorksetId.ToString();
 
-
       // 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.
       // In all other cases this should be the display value string (localized name) of the catogory
@@ -271,8 +270,6 @@ public void GetAllRevitParamsAndIds(Base speckleElement, DB.Element revitElement
       BuiltInCategory builtInCategory = Categories.GetBuiltInCategory(category);
       speckleElement["builtInCategory"] = builtInCategory.ToString();
 
-
-
       //NOTE: adds the quantities of all materials to an element
       var qs = MaterialQuantitiesToSpeckle(revitElement, speckleElement["units"] as string);
       if (qs != null)
diff --git a/Objects/Converters/ConverterRevit/ConverterRevitShared/ConverterRevit.cs b/Objects/Converters/ConverterRevit/ConverterRevitShared/ConverterRevit.cs
index 4098322776..f90f1943da 100644
--- a/Objects/Converters/ConverterRevit/ConverterRevitShared/ConverterRevit.cs
+++ b/Objects/Converters/ConverterRevit/ConverterRevitShared/ConverterRevit.cs
@@ -157,6 +157,7 @@ public void SetContextDocument(object doc)
     const string DetailLevelMedium = "Medium";
     const string DetailLevelFine = "Fine";
     public ViewDetailLevel DetailLevelSetting => GetDetailLevelSetting() ?? ViewDetailLevel.Fine;
+
     private ViewDetailLevel? GetDetailLevelSetting()
     {
       if (!Settings.TryGetValue("detail-level", out string detailLevel))
@@ -648,8 +649,7 @@ public object ConvertToNativeObject(Base @object)
           return RailingToNative(o);
 
         case BER.ParameterUpdater o:
-          UpdateParameter(o);
-          return null;
+          return UpdateParameter(o);
 
         case BE.View3D o:
           return ViewToNative(o);
diff --git a/Objects/Converters/ConverterRevit/ConverterRevitShared/Partial Classes/UpdateParameter.cs b/Objects/Converters/ConverterRevit/ConverterRevitShared/Partial Classes/UpdateParameter.cs
index fcdc114374..c93b910e8c 100644
--- a/Objects/Converters/ConverterRevit/ConverterRevitShared/Partial Classes/UpdateParameter.cs	
+++ b/Objects/Converters/ConverterRevit/ConverterRevitShared/Partial Classes/UpdateParameter.cs	
@@ -3,44 +3,53 @@
 using Objects.Geometry;
 using System.Collections.Generic;
 using System.Linq;
-
+using Speckle.Core.Helpers;
+using Speckle.Core.Models;
 
 namespace Objects.Converter.Revit
 {
   public partial class ConverterRevit
   {
-    public void UpdateParameter(ParameterUpdater paramUpdater)
+    public ApplicationObject UpdateParameter(ParameterUpdater paramUpdater)
     {
       //the below does not work because ApplicationIds are stored within a stream
       //try to get element using ApplicationId
       //this will only work if the element has been successfully been received in Revit before
       //var element = GetExistingElementByApplicationId(paramUpdater.revitId);
 
+      var appObj = new ApplicationObject(paramUpdater.id, paramUpdater.speckle_type)
+      {
+        applicationId = paramUpdater.applicationId
+      };
       Element element = null;
 
       //try to get element using ElementId
-      int intId;
-
-      if (int.TryParse(paramUpdater.elementId, out intId))
+      if (int.TryParse(paramUpdater.elementId, out int intId))
       {
         var elemId = new ElementId(intId);
         element = Doc.GetElement(elemId);
       }
 
       //try to get element using UniqueId
-      if (element == null)
+      element ??= Doc.GetElement(paramUpdater.elementId);
+
+      if (element != null)
       {
-        element = Doc.GetElement(paramUpdater.elementId);
+        SetInstanceParameters(element, paramUpdater);
+        appObj.Update(
+          status: ApplicationObject.State.Updated,
+          logItem: $"Successfully updated instance parameters for element {paramUpdater.elementId}"
+        );
       }
-
-      if (element == null)
+      else
       {
-        Report.LogConversionError(new System.Exception($"Could not find element to update: Element Id = {paramUpdater.elementId}"));
-        return;
+        appObj.Update(
+          status: ApplicationObject.State.Failed,
+          logItem: $"Could not find element to update: Element Id = {paramUpdater.elementId}"
+        );
       }
 
-      SetInstanceParameters(element, paramUpdater);
+      return appObj;
     }
-
   }
-}
\ No newline at end of file
+}