From 0035530f29410e3a5b6b3735d84c022ac545bf0b Mon Sep 17 00:00:00 2001
From: Ale Martinez <amtriathlon@gmail.com>
Date: Sat, 22 Feb 2020 20:58:49 -0300
Subject: [PATCH] Add some  validations to User Metrics creation and
 modification Enable accept only when boty symbol and name are non empty Warn
 on name and symbol collisions to avoid silently discarded metrics or metrics
 not usable from formulas Complements
 635eccd0015f34f7cffc18b5f89c4507d301c1b7, related to #2279

---
 src/Gui/EditUserMetricDialog.cpp | 30 ++++++++++++++++++++++++++++++
 src/Gui/Pages.cpp                |  8 ++++++++
 src/Metrics/UserMetricSettings.h |  2 ++
 3 files changed, 40 insertions(+)

diff --git a/src/Gui/EditUserMetricDialog.cpp b/src/Gui/EditUserMetricDialog.cpp
index b5cacbf66..d37f1d69c 100644
--- a/src/Gui/EditUserMetricDialog.cpp
+++ b/src/Gui/EditUserMetricDialog.cpp
@@ -27,9 +27,11 @@
 #include "DataFilter.h"
 #include "Zones.h"
 #include "HrZones.h"
+#include "RideMetric.h"
 
 #include <QFont>
 #include <QFontMetrics>
+#include <QMessageBox>
 
 static bool insensitiveLessThan(const QString &a, const QString &b)
 {
@@ -255,10 +257,35 @@ EditUserMetricDialog::EditUserMetricDialog(QWidget *parent, Context *context, Us
 
     mainLayout->addLayout(head);
 
+    connect(symbol, SIGNAL(textChanged(const QString &)), SLOT(enableOk()));
+    connect(name, SIGNAL(textChanged(const QString &)), SLOT(enableOk()));
+
     connect(test, SIGNAL(clicked()), this, SLOT(refreshStats()));
     connect(context, SIGNAL(rideSelected(RideItem*)), this, SLOT(refreshStats()));
     connect (cancelButton, SIGNAL(clicked()), this, SLOT(reject()));
     connect (okButton, SIGNAL(clicked()), this, SLOT(okClicked()));
+
+    // initialize button state
+    enableOk();
+}
+
+void
+EditUserMetricDialog::enableOk()
+{
+    okButton->setEnabled(!symbol->text().isEmpty() && !name->text().isEmpty());
+}
+
+bool
+EditUserMetricDialog::validSettings()
+{
+    // user metrics are silently discarded if the symbol is already in use
+    const RideMetric* metric = RideMetricFactory::instance().rideMetric(symbol->text());
+    if (metric && !metric->isUser()) {
+        QMessageBox::critical(this, tr("User Metric"), tr("Symbol already in use by a Builtin metric"));
+        return false;
+    }
+
+    return true;
 }
 
 void
@@ -284,6 +311,9 @@ EditUserMetricDialog::setSettings(UserMetricSettings &here)
 void
 EditUserMetricDialog::okClicked()
 {
+    // validate input
+    if (!validSettings()) return;
+
     // fetch current state
     setSettings(settings);
 
diff --git a/src/Gui/Pages.cpp b/src/Gui/Pages.cpp
index d3c4d4d0f..da668d449 100644
--- a/src/Gui/Pages.cpp
+++ b/src/Gui/Pages.cpp
@@ -3172,6 +3172,14 @@ CustomMetricsPage::refreshTable()
             continue;
         }
 
+        // user metrics are silently discarded if the symbol is already in use
+        if (!table->findItems(m.symbol, Qt::MatchExactly, 0).isEmpty())
+            QMessageBox::warning(this, tr("User Metrics"), tr("Duplicate Symbol: %1, one metric will be discarded").arg(m.symbol));
+
+        // duplicate names are allowed, but not recommended
+        if (!table->findItems(m.name, Qt::MatchExactly, 1).isEmpty())
+            QMessageBox::warning(this, tr("User Metrics"), tr("Duplicate Name: %1, one metric will not be acessible in formulas").arg(m.name));
+
         QTreeWidgetItem *add = new QTreeWidgetItem(table->invisibleRootItem());
         add->setText(0, m.symbol);
         add->setToolTip(0, m.description);
diff --git a/src/Metrics/UserMetricSettings.h b/src/Metrics/UserMetricSettings.h
index 6f1c190fa..04ba3304f 100644
--- a/src/Metrics/UserMetricSettings.h
+++ b/src/Metrics/UserMetricSettings.h
@@ -97,9 +97,11 @@ class EditUserMetricDialog : public QDialog {
         // the current ride, time to compute all rides)
         void refreshStats();
         void okClicked();
+        void enableOk();
 
     private:
 
+        bool validSettings();
         void setSettings(UserMetricSettings &);
 
         Context *context;