Dive site: don't emit divesChanged signals when editing dive site

When editing the dive site, for certain fields a divesChanged signal
was emitted so that the dive-list can be updated.

Arguably it is wrong to decide which fields are relevant to the
dive list in the undo-command code. Therefore, let the list
catch the dive-site-edited signal and decide itself.

But the actual reason for this commit is that if the dive-site
field of a dive changes, we might have to reload the dive-location-model
because suddenly a new dive site appears. Now if this is done
in QML context on some Qt version (notably 5.9) we get crashes
later on. But that can happen if the user moves a flag. So in that
case only send a diveSiteChanged signal.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

3 files changed, 41 insertions, 17 deletions

diff --git a/desktop-widgets/command_divesite.cpp b/desktop-widgets/command_divesite.cpp
index 8c31959eb..3ef4231f8 100644
--- a/desktop-widgets/command_divesite.cpp
+++ b/desktop-widgets/command_divesite.cpp
@@ -186,19 +186,6 @@ static void swap(char *&c, QString &q)
 	q = s;
 }
 
-// Helper function: collect the dives that are at the given dive site
-static QVector<dive *> getDivesForSite(struct dive_site *ds)
-{
-	QVector<dive *> diveSiteDives;
-	diveSiteDives.reserve(ds->dives.nr);
-
-	for (int i = 0; i < ds->dives.nr; ++i)
-		diveSiteDives.push_back(ds->dives.dives[i]);
-
-	return diveSiteDives;
-}
-
-
 EditDiveSiteName::EditDiveSiteName(dive_site *dsIn, const QString &name) : ds(dsIn),
 	value(name)
 {
@@ -214,7 +201,6 @@ void EditDiveSiteName::redo()
 {
 	swap(ds->name, value);
 	emit diveListNotifier.diveSiteChanged(ds, LocationInformationModel::NAME); // Inform frontend of changed dive site.
-	emit diveListNotifier.divesChanged(getDivesForSite(ds), DiveField::DIVESITE); // dive site name can be shown in the dive list
 }
 
 void EditDiveSiteName::undo()
@@ -286,8 +272,6 @@ void EditDiveSiteCountry::redo()
 	taxonomy_set_country(&ds->taxonomy, copy_qstring(value), taxonomy_origin::GEOMANUAL);
 	value = old;
 	emit diveListNotifier.diveSiteChanged(ds, LocationInformationModel::TAXONOMY); // Inform frontend of changed dive site.
-	emit diveListNotifier.divesChanged(getDivesForSite(ds), DiveField::DIVESITE); // Country can be shown in the dive list
-
 }
 
 void EditDiveSiteCountry::undo()
@@ -315,7 +299,6 @@ void EditDiveSiteLocation::redo()
 {
 	std::swap(value, ds->location);
 	emit diveListNotifier.diveSiteChanged(ds, LocationInformationModel::LOCATION); // Inform frontend of changed dive site.
-	emit diveListNotifier.divesChanged(getDivesForSite(ds), DiveField::DIVESITE); // the globe icon in the dive list shows whether we have coordinates
 }
 
 void EditDiveSiteLocation::undo()
diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp
index 09c5c308d..30ab2175b 100644
--- a/qt-models/divetripmodel.cpp
+++ b/qt-models/divetripmodel.cpp
@@ -5,8 +5,10 @@
 #include "core/metrics.h"
 #include "core/trip.h"
 #include "core/qthelper.h"
+#include "core/divesite.h"
 #include "core/subsurface-string.h"
 #include "core/tag.h"
+#include "qt-models/divelocationmodel.h" // For the dive-site field ids
 #include "desktop-widgets/command.h"
 #include <QIcon>
 #include <QDebug>
@@ -548,6 +550,7 @@ DiveTripModelTree::DiveTripModelTree(QObject *parent) : DiveTripModelBase(parent
 	connect(&diveListNotifier, &DiveListNotifier::divesAdded, this, &DiveTripModelTree::divesAdded);
 	connect(&diveListNotifier, &DiveListNotifier::divesDeleted, this, &DiveTripModelTree::divesDeleted);
 	connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &DiveTripModelTree::divesChanged);
+	connect(&diveListNotifier, &DiveListNotifier::diveSiteChanged, this, &DiveTripModelTree::diveSiteChanged);
 	connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModelTree::divesMovedBetweenTrips);
 	connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModelTree::divesTimeChanged);
 	connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &DiveTripModelTree::divesSelected);
@@ -928,6 +931,34 @@ void processByTrip(QVector<dive *> dives, Function action)
 	}
 }
 
+// Helper function: collect the dives that are at the given dive site
+static QVector<dive *> getDivesForSite(struct dive_site *ds)
+{
+	QVector<dive *> diveSiteDives;
+	diveSiteDives.reserve(ds->dives.nr);
+
+	for (int i = 0; i < ds->dives.nr; ++i)
+		diveSiteDives.push_back(ds->dives.dives[i]);
+
+	return diveSiteDives;
+}
+
+// On the change of which dive site field should we update the
+// dive in the list?
+static bool isInterestingDiveSiteField(int field)
+{
+	return field == LocationInformationModel::NAME		// dive site name can is shown in the dive list
+	    || field == LocationInformationModel::TAXONOMY	// country is shown in the dive list
+	    || field == LocationInformationModel::LOCATION;	// the globe icon in the dive list shows whether we have coordinates
+}
+
+void DiveTripModelTree::diveSiteChanged(dive_site *ds, int field)
+{
+	if (!isInterestingDiveSiteField(field))
+		return;
+	divesChanged(getDivesForSite(ds));
+}
+
 void DiveTripModelTree::divesChanged(const QVector<dive *> &dives)
 {
 	processByTrip(dives, [this] (dive_trip *trip, const QVector<dive *> &divesInTrip)
@@ -1161,6 +1192,7 @@ DiveTripModelList::DiveTripModelList(QObject *parent) : DiveTripModelBase(parent
 	connect(&diveListNotifier, &DiveListNotifier::divesAdded, this, &DiveTripModelList::divesAdded);
 	connect(&diveListNotifier, &DiveListNotifier::divesDeleted, this, &DiveTripModelList::divesDeleted);
 	connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &DiveTripModelList::divesChanged);
+	connect(&diveListNotifier, &DiveListNotifier::diveSiteChanged, this, &DiveTripModelList::diveSiteChanged);
 	// Does nothing in list-view
 	//connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModelList::divesMovedBetweenTrips);
 	connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModelList::divesTimeChanged);
@@ -1249,6 +1281,13 @@ void DiveTripModelList::divesDeleted(dive_trip *, bool, const QVector<dive *> &d
 				 });
 }
 
+void DiveTripModelList::diveSiteChanged(dive_site *ds, int field)
+{
+	if (!isInterestingDiveSiteField(field))
+		return;
+	divesChanged(getDivesForSite(ds));
+}
+
 void DiveTripModelList::divesChanged(const QVector<dive *> &divesIn)
 {
 	QVector<dive *> dives = divesIn;
diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h
index 8ddcf9615..b63456236 100644
--- a/qt-models/divetripmodel.h
+++ b/qt-models/divetripmodel.h
@@ -106,6 +106,7 @@ public slots:
 	void divesAdded(dive_trip *trip, bool addTrip, const QVector<dive *> &dives);
 	void divesDeleted(dive_trip *trip, bool deleteTrip, const QVector<dive *> &dives);
 	void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives);
+	void diveSiteChanged(dive_site *ds, int field);
 	void divesChanged(const QVector<dive *> &dives);
 	void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
 	void divesSelected(const QVector<dive *> &dives, dive *current);
@@ -170,6 +171,7 @@ class DiveTripModelList : public DiveTripModelBase
 public slots:
 	void divesAdded(dive_trip *trip, bool addTrip, const QVector<dive *> &dives);
 	void divesDeleted(dive_trip *trip, bool deleteTrip, const QVector<dive *> &dives);
+	void diveSiteChanged(dive_site *ds, int field);
 	void divesChanged(const QVector<dive *> &dives);
 	void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
 	// Does nothing in list view.