From 31cbd35862749a8d0dfb46950e80e230d0803f68 Mon Sep 17 00:00:00 2001
From: Magne Sjaastad <magne.sjaastad@ceetronsolutions.com>
Date: Tue, 19 Sep 2017 12:01:16 +0200
Subject: [PATCH] Added more tests and guard for null pointer access

---
 .../cafPdmUiTreeSelectionQModel.cpp           | 18 ++++++++-----
 .../cafPdmUiTreeSelectionQModelTest.cpp       | 26 +++++++++++++++++++
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiTreeSelectionQModel.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiTreeSelectionQModel.cpp
index ada804a68c..566333c17e 100644
--- a/Fwk/AppFwk/cafUserInterface/cafPdmUiTreeSelectionQModel.cpp
+++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiTreeSelectionQModel.cpp
@@ -80,6 +80,8 @@ int caf::PdmUiTreeSelectionQModel::headingRole()
 //--------------------------------------------------------------------------------------------------
 void caf::PdmUiTreeSelectionQModel::setCheckedStateForItems(const QModelIndexList& sourceModelIndices, bool checked)
 {
+    if (!m_uiFieldHandle || !m_uiFieldHandle->field()) return;
+
     std::set<unsigned int> selectedIndices;
     {
         QVariant fieldValue = m_uiFieldHandle->field()->uiValue();
@@ -282,8 +284,6 @@ QVariant caf::PdmUiTreeSelectionQModel::data(const QModelIndex &index, int role
         }
         else if (role == Qt::CheckStateRole && !optionItemInfo->isHeading())
         {
-            CAF_ASSERT(m_uiFieldHandle);
-
             if (m_uiFieldHandle && m_uiFieldHandle->field())
             {
                 QVariant fieldValue = m_uiFieldHandle->field()->uiValue();
@@ -327,8 +327,10 @@ QVariant caf::PdmUiTreeSelectionQModel::data(const QModelIndex &index, int role
 //--------------------------------------------------------------------------------------------------
 bool caf::PdmUiTreeSelectionQModel::setData(const QModelIndex &index, const QVariant &value, int role /*= Qt::EditRole*/)
 {
-     if (role == Qt::CheckStateRole)
-     {
+    if (!m_uiFieldHandle || !m_uiFieldHandle->field()) return false;
+
+    if (role == Qt::CheckStateRole)
+    {
         std::vector<unsigned int> selectedIndices;
         {
             QVariant fieldValue = m_uiFieldHandle->field()->uiValue();
@@ -371,10 +373,12 @@ bool caf::PdmUiTreeSelectionQModel::setData(const QModelIndex &index, const QVar
             fieldValueSelection.push_back(QVariant(v));
         }
 
-         PdmUiCommandSystemProxy::instance()->setUiValueToField(m_uiFieldHandle->field(), fieldValueSelection); 
+        PdmUiCommandSystemProxy::instance()->setUiValueToField(m_uiFieldHandle->field(), fieldValueSelection); 
 
-         return true;
-     }
+        emit dataChanged(index, index);
+
+        return true;
+    }
 
     return false;
 }
diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiTreeSelectionQModelTest.cpp b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiTreeSelectionQModelTest.cpp
index 8a246a7ceb..6dbf4f02b3 100644
--- a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiTreeSelectionQModelTest.cpp
+++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiTreeSelectionQModelTest.cpp
@@ -44,6 +44,9 @@ QList<caf::PdmOptionItemInfo> createOptions()
     return options;
 }
 
+
+
+
 //--------------------------------------------------------------------------------------------------
 /// 
 //--------------------------------------------------------------------------------------------------
@@ -54,6 +57,8 @@ TEST(PdmUiTreeSelectionQModelTest, BasicUsage)
     caf::PdmUiTreeSelectionQModel myModel;
     myModel.setOptions(nullptr, options);
 
+    EXPECT_EQ(options.size(), myModel.optionItemCount());
+
     EXPECT_EQ(4, myModel.rowCount(myModel.index(-1, -1)));
     
     EXPECT_EQ(0, myModel.rowCount(myModel.index(0, 0)));
@@ -105,3 +110,24 @@ TEST(PdmUiTreeSelectionQModelTest, SetDataAndSignal)
 
     myModel.setData(parentIndex, QVariant(true));
 }
+
+//--------------------------------------------------------------------------------------------------
+/// 
+//--------------------------------------------------------------------------------------------------
+TEST(PdmUiTreeSelectionQModelTest, SetCheckedStateForItems)
+{
+    QList<caf::PdmOptionItemInfo> options = createOptions();
+
+    caf::PdmUiTreeSelectionQModel myModel;
+    myModel.setOptions(nullptr, options);
+
+    QModelIndex parentIndex = myModel.index(1, 0);
+    QModelIndex firstChildIndex = myModel.index(0, 0, parentIndex);
+
+    QModelIndexList indices;
+    indices << firstChildIndex;
+
+    myModel.setCheckedStateForItems(indices, false);
+
+    // No test code for this behaviour, only making sure this runs without any errors
+}