Cleanup settings remove/reset behavior

Instead of letting removeValues() and friends just literally wipe out
all settings, remove them only if they're not described in
settings_descriptions.json. If they are described, then merely reset
them to the default.

This also removes some code which reset values to QVariant(), instead of
the default value.

Rename some instances of "remove" to "reset", because that's what it
actually does.

In addition, send change notifications in cases when resetting a value
or a section changes the values (such as changing it back to default).
Entirely removed values are not included in the change notifications -
the assumption is that nothing cares about them anymore.

All these changes should allow web-client to reset any section safely,
even sections that contain host values.

src/settings/SettingsComponent.cpp

@@ -315,7 +315,7 @@ void SettingsComponent::setValues(const QVariantMap& options)
     }
 
     if (values.isNull())
-      section->removeValues();
+      section->resetValues();
     else
       section->setValues(values);
 
@@ -343,7 +343,7 @@ void SettingsComponent::removeValue(const QString &sectionOrKey)
   {
     // we want to remove a full section
     // dont remove the section, but remove all keys
-    section->removeValues();
+    section->resetValues();
     saveSection(section);
   }
   else
@@ -351,7 +351,7 @@ void SettingsComponent::removeValue(const QString &sectionOrKey)
     // we want to remove a root key from webclient
     // which is stored in webclient section
     section = m_sections[SETTINGS_SECTION_WEBCLIENT];
-    section->removeValue(sectionOrKey);
+    section->resetValue(sectionOrKey);
     saveSection(section);
   }
 }
@@ -362,7 +362,10 @@ void SettingsComponent::resetToDefault(const QString &sectionID)
   SettingsSection* section = getSection(sectionID);
 
   if (section)
-    section->resetToDefault();
+  {
+    section->resetValues();
+    saveSection(section);
+  }
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
@@ -370,7 +373,8 @@ void SettingsComponent::resetToDefault()
 {
   for(SettingsSection *section : m_sections)
   {
-   section->resetToDefault();
+    section->resetValues();
+    saveSection(section);
   }
 }
 
@@ -490,6 +494,7 @@ void SettingsComponent::parseSection(const QJsonObject& sectionObject)
 
     int vPlatformMask = platformMaskFromObject(valobj);
     SettingsValue* setting = new SettingsValue(valobj.value("value").toString(), defaultval, (quint8)vPlatformMask, this);
+    setting->setHasDescription(true);
     setting->setHidden(valobj.value("hidden").toBool(false));
     setting->setIndexOrder(order ++);
 

src/settings/SettingsComponent.h

@@ -48,6 +48,9 @@ public:
   Q_INVOKABLE void setValues(const QVariantMap& options);
   Q_INVOKABLE QVariant value(const QString& sectionID, const QString& key);
   Q_INVOKABLE QVariant allValues(const QString& section = "");
+  // Note: the naming "remove" is a lie - it will remove the affected keys only if they are not
+  //       declared in settings_descriptions.json. Also, sections are never removed, even if they
+  //       remain empty.
   Q_INVOKABLE void removeValue(const QString& sectionOrKey);
   Q_INVOKABLE void resetToDefault();
   Q_INVOKABLE void resetToDefault(const QString& sectionID);

src/settings/SettingsSection.cpp

@@ -27,11 +27,11 @@ void SettingsSection::setValues(const QVariant& values)
   QVariantMap map = values.toMap();
   QVariantMap updatedValues;
 
-  // values not included in the map are reset to default
+  // values not included in the map are "removed"
   for(const QString& key : m_values.keys())
   {
     if (!map.contains(key))
-      map[key] = m_values[key]->defaultValue();
+      resetValueNoNotify(key, updatedValues);
   }
 
   for(const QString& key : map.keys())
@@ -114,9 +114,21 @@ bool SettingsSection::setValue(const QString& key, const QVariant& value)
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
-void SettingsSection::removeValue(const QString &key)
+void SettingsSection::resetValueNoNotify(const QString& key, QVariantMap& updatedValues)
 {
-  if (m_values.contains(key))
+  if (!m_values.contains(key))
+    return;
+
+  SettingsValue* val = m_values[key];
+
+  if (val->hasDescription())
+  {
+    if (val->value() == val->defaultValue())
+      return;
+    val->setValue(val->defaultValue());
+    updatedValues[key] = val->value();
+  }
+  else
   {
     m_values[key]->deleteLater();
     m_values.remove(key);
@@ -124,9 +136,26 @@ void SettingsSection::removeValue(const QString &key)
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
-void SettingsSection::removeValues()
+void SettingsSection::resetValue(const QString &key)
 {
-  m_values.clear();
+  QVariantMap updatedValues;
+
+  resetValueNoNotify(key, updatedValues);
+
+  if (updatedValues.size() > 0)
+    emit valuesUpdated(updatedValues);
+}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+void SettingsSection::resetValues()
+{
+  QVariantMap updatedValues;
+
+  for (auto key : m_values.keys())
+    resetValueNoNotify(key, updatedValues);
+
+  if (updatedValues.size() > 0)
+    emit valuesUpdated(updatedValues);
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
@@ -169,13 +198,6 @@ const QVariantMap SettingsSection::descriptions() const
   return map;
 }
 
-///////////////////////////////////////////////////////////////////////////////////////////////////
-void SettingsSection::resetToDefault()
-{
-  for(const QString& key : m_values.keys())
-    m_values[key]->reset();
-}
-
 /////////////////////////////////////////////////////////////////////////////////////////
 bool SettingsSection::isHidden() const
 {

src/settings/SettingsSection.h

@@ -20,9 +20,8 @@ public:
 
   void setValues(const QVariant& values);
   bool setValue(const QString& key, const QVariant& value);
-  void removeValue(const QString& key);
-  void removeValues();
-  void resetToDefault();
+  void resetValue(const QString& key);
+  void resetValues();
   void registerSetting(SettingsValue* value);
   bool isHidden() const;
 
@@ -56,6 +55,9 @@ public:
   Q_SIGNAL void valuesUpdated(const QVariantMap& values);
 
 protected:
+  // if the value is _not_ removed, _and_ changes, it's added to updatedValues
+  void resetValueNoNotify(const QString& key, QVariantMap& updatedValues);
+
   QHash<QString, SettingsValue*> m_values;
   QString m_sectionID;
   int m_orderIndex;

src/settings/SettingsValue.h

@@ -17,6 +17,8 @@ public:
     : QObject(parent)
     , m_platform(PLATFORM_UNKNOWN)
     , m_hidden(true)
+    , m_indexOrder(0)
+    , m_hasDescription(false)
   {}
 
   explicit SettingsValue(const QString& _key, QVariant _defaultValue=QVariant(), quint8 platforms = PLATFORM_ANY, QObject* parent = nullptr)
@@ -26,6 +28,8 @@ public:
     , m_defaultValue(_defaultValue)
     , m_platform(platforms)
     , m_hidden(true)
+    , m_indexOrder(0)
+    , m_hasDescription(false)
   {}
 
   const QString& key() const { return m_key; }
@@ -96,12 +100,6 @@ public:
     m_possibleValues << entry;
   }
 
-  // goes back to use the default
-  void reset()
-  {
-    m_value.clear();
-  }
-
   QVariantMap descriptions() const
   {
     QVariantMap ret;
@@ -123,6 +121,13 @@ public:
 
   int indexOrder() const { return m_indexOrder; }
 
+  void setHasDescription(bool hasDescription)
+  {
+    m_hasDescription = hasDescription;
+  }
+
+  bool hasDescription() { return m_hasDescription; }
+
 private:
   QString m_key;
   QVariant m_value;
@@ -133,6 +138,7 @@ private:
   QString m_inputType;
 
   int m_indexOrder;
+  bool m_hasDescription;
 };
 
 #endif //KONVERGO_SETTINGS_VALUE_H