From 4424b4d51ec71a66be41a74accb743ea3460b4e5 Mon Sep 17 00:00:00 2001
From: amzn-sean <75276488+amzn-sean@users.noreply.github.com>
Date: Tue, 30 Nov 2021 10:49:53 +0000
Subject: [PATCH] Fix Integer Spin Controller not sending
 PropertyEditorGUIMessages::Bus::Handler::OnEditingFinished (#5942)

* IntWidgetHandler::CreateUI now registers and sends editingFinished event
* add tests for int spin control valueChanged and editingFinished signals

Signed-off-by: amzn-sean <75276488+amzn-sean@users.noreply.github.com>
---
 .../UI/PropertyEditor/PropertyIntCtrlCommon.h |  4 ++
 .../Tests/PropertyIntCtrlCommonTests.cpp      |  4 +-
 .../Tests/PropertyIntCtrlCommonTests.h        | 10 +++
 .../Tests/PropertyIntSpinCtrlTests.cpp        | 67 +++++++++++++++++++
 .../Tests/aztoolsframeworktests_files.cmake   |  1 +
 5 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyIntCtrlCommon.h b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyIntCtrlCommon.h
index 01675e0044..bb7d03367b 100644
--- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyIntCtrlCommon.h
+++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyIntCtrlCommon.h
@@ -169,6 +169,10 @@ namespace AzToolsFramework
         {
             AzToolsFramework::PropertyEditorGUIMessages::Bus::Broadcast(&PropertyEditorGUIMessages::Bus::Events::RequestWrite, newCtrl);
         });
+        this->connect(newCtrl, &PropertyControl::editingFinished, this, [newCtrl]()
+        {
+            AzToolsFramework::PropertyEditorGUIMessages::Bus::Broadcast(&PropertyEditorGUIMessages::Bus::Handler::OnEditingFinished, newCtrl);
+        });
         // note:  Qt automatically disconnects objects from each other when either end is destroyed, no need to worry about delete.
 
         // Set the value range to that of ValueType as clamped to the range of QtWidgetValueType
diff --git a/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.cpp b/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.cpp
index 67f764d1b3..d95451441f 100644
--- a/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.cpp
+++ b/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.cpp
@@ -6,7 +6,9 @@
  *
  */
 
-#include "IntegerPrimtitiveTestConfig.h"
+#include <AzToolsFramework/UI/PropertyEditor/PropertyIntCtrlCommon.h>
+#include <AzToolsFramework/UI/PropertyEditor/QtWidgetLimits.h>
+#include <Tests/IntegerPrimtitiveTestConfig.h>
 
 namespace UnitTest
 {
diff --git a/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.h b/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.h
index 10aed57ebd..712eeb99b7 100644
--- a/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.h
+++ b/Code/Framework/AzToolsFramework/Tests/PropertyIntCtrlCommonTests.h
@@ -156,6 +156,16 @@ namespace UnitTest
             EXPECT_STREQ(tooltip.toStdString().c_str(), expected.str().c_str());
         }
 
+        void EmitWidgetValueChanged()
+        {
+            emit m_widget->valueChanged(ValueType(0));
+        }
+
+        void EmitWidgetEditingFinished()
+        {
+            emit m_widget->editingFinished();
+        }
+
         AZStd::unique_ptr<QWidget> m_dummyWidget;
         AZStd::unique_ptr<HandlerAPI> m_handler;
         WidgetType* m_widget;
diff --git a/Code/Framework/AzToolsFramework/Tests/PropertyIntSpinCtrlTests.cpp b/Code/Framework/AzToolsFramework/Tests/PropertyIntSpinCtrlTests.cpp
index 01f314bcc2..d7616e6bb8 100644
--- a/Code/Framework/AzToolsFramework/Tests/PropertyIntSpinCtrlTests.cpp
+++ b/Code/Framework/AzToolsFramework/Tests/PropertyIntSpinCtrlTests.cpp
@@ -47,4 +47,71 @@ namespace UnitTest
     {
         this->HandlerMinMaxLessLimit_ModifyHandler_ExpectSuccessAndValidLessLimitToolTipString();
     }
+
+    struct PropertyEditorHandler
+        : public AzToolsFramework::PropertyEditorGUIMessages::Bus::Handler
+    {
+        PropertyEditorHandler()
+        {
+            AzToolsFramework::PropertyEditorGUIMessages::Bus::Handler::BusConnect();
+        }
+
+        ~PropertyEditorHandler()
+        {
+            AzToolsFramework::PropertyEditorGUIMessages::Bus::Handler::BusDisconnect();
+        }
+
+        // AzToolsFramework::PropertyEditorGUIMessages::Bus overrides ...
+        void RequestWrite([[maybe_unused]] QWidget* editorGUI) override
+        {
+            m_requestWriteCallCount++;
+        }
+
+        void RequestRefresh([[maybe_unused]] PropertyModificationRefreshLevel level) override
+        {
+        }
+
+        void AddElementsToParentContainer(
+            [[maybe_unused]] QWidget* editorGUI,
+            [[maybe_unused]] size_t numElements,
+            [[maybe_unused]] const InstanceDataNode::FillDataClassCallback& fillDataCallback) override
+        {
+        }
+
+        void RequestPropertyNotify([[maybe_unused]] QWidget* editorGUI) override
+        {
+        }
+
+        void OnEditingFinished([[maybe_unused]]QWidget* editorGUI) override
+        {
+            m_onEditingFinishedCallCount++;
+        }
+
+        int m_requestWriteCallCount = 0;
+        int m_onEditingFinishedCallCount = 0;
+    };
+
+    TYPED_TEST(PropertySpinCtrlFixture, SpinBoxWidgetValueChangedInvokesPropertyEditorGUIMessages)
+    {
+        // setup the event handler
+        PropertyEditorHandler eventHandler;
+
+        // trigger the QT signal
+        this->EmitWidgetValueChanged();
+
+        // there should be at least 1 call to RequestWrite.
+        EXPECT_GT(eventHandler.m_requestWriteCallCount, 0);
+    }
+
+    TYPED_TEST(PropertySpinCtrlFixture, SpinBoxWidgetEditingFinishedInvokesPropertyEditorGUIMessages)
+    {
+        // setup the event handler
+        PropertyEditorHandler eventHandler;
+
+        // trigger the QT signal
+        this->EmitWidgetEditingFinished();
+
+        // there should be at least 1 call to OnEditingFinished.
+        EXPECT_GT(eventHandler.m_onEditingFinishedCallCount, 0);
+    }
 } // namespace UnitTest
diff --git a/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake b/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake
index 99a998abb8..9e052de983 100644
--- a/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake
+++ b/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake
@@ -100,6 +100,7 @@ set(FILES
     Prefab/SpawnableSortEntitiesTests.cpp
     Prefab/PrefabScriptingTests.cpp
     Prefab/ProceduralPrefabAssetTests.cpp
+    PropertyIntCtrlCommonTests.cpp
     PropertyIntCtrlCommonTests.h
     PropertyIntSliderCtrlTests.cpp
     PropertyIntSpinCtrlTests.cpp