mobile/undo: create EditDive command

Command that just swaps two dives. This is rather complex,
as for example a dive site might be created.

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

commands/command.cpp

@@ -304,4 +304,11 @@ void editTripNotes(dive_trip *trip, const QString &s)
 	execute(new EditTripNotes(trip, s));
 }
 
+#ifdef SUBSURFACE_MOBILE
+void editDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *changeDs, location_t dsLocation)
+{
+	execute(new EditDive(oldDive, newDive, createDs, changeDs, dsLocation));
+}
+#endif // SUBSURFACE_MOBILE
+
 } // namespace Command

commands/command.h

@@ -89,6 +89,11 @@ void editProfile(dive *d); // dive computer(s) and cylinder(s) will be reset!
 int addWeight(bool currentDiveOnly);
 int removeWeight(int index, bool currentDiveOnly);
 int editWeight(int index, weightsystem_t ws, bool currentDiveOnly);
+#ifdef SUBSURFACE_MOBILE
+// Edits a dive and creates a divesite (if createDs != NULL) or edits a divesite (if changeDs != NULL).
+// Takes ownership of newDive and createDs!
+void editDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *changeDs, location_t dsLocation);
+#endif
 
 // 5) Trip editing commands
 

commands/command_edit.cpp

@@ -7,6 +7,9 @@
 #include "core/subsurface-string.h"
 #include "core/tag.h"
 #include "qt-models/weightsysteminfomodel.h"
+#ifdef SUBSURFACE_MOBILE
+#include "qt-models/divelocationmodel.h"
+#endif
 
 namespace Command {
 
@@ -1145,4 +1148,134 @@ void EditWeight::undo()
 	redo();
 }
 
+#ifdef SUBSURFACE_MOBILE
+
+EditDive::EditDive(dive *oldDiveIn, dive *newDiveIn, dive_site *createDs, dive_site *editDs, location_t dsLocationIn)
+	: oldDive(oldDiveIn)
+	, newDive(newDiveIn)
+	, changedFields(DiveField::NONE)
+	, siteToRemove(nullptr)
+	, siteToAdd(createDs)
+	, siteToEdit(editDs)
+	, dsLocation(dsLocationIn)
+{
+	if (!oldDive || ! newDive)
+		return;
+
+	setText(tr("Edit dive"));
+
+	// Calculate the fields that changed.
+	// Note: Probably not needed, as on mobile we don't have that granularity.
+	// However, for future-proofeness let's just do it.
+	changedFields = DiveField::NONE;
+	if (oldDive->number != newDive->number)
+		changedFields |= DiveField::NR;
+	if (oldDive->when != newDive->when)
+		changedFields |= DiveField::DATETIME;
+	if (oldDive->maxdepth.mm != newDive->maxdepth.mm)
+		changedFields |= DiveField::DEPTH;
+	if (oldDive->duration.seconds != newDive->duration.seconds)
+		changedFields |= DiveField::DURATION;
+	if (oldDive->airtemp.mkelvin != newDive->airtemp.mkelvin)
+		changedFields |= DiveField::AIR_TEMP;
+	if (oldDive->watertemp.mkelvin != newDive->watertemp.mkelvin)
+		changedFields |= DiveField::WATER_TEMP;
+	if (oldDive->surface_pressure.mbar != newDive->surface_pressure.mbar)
+		changedFields |= DiveField::ATM_PRESS;
+	if (oldDive->dive_site != newDive->dive_site)
+		changedFields |= DiveField::DIVESITE;
+	if (!same_string(oldDive->divemaster, newDive->divemaster))
+		changedFields |= DiveField::DIVEMASTER;
+	if (!same_string(oldDive->buddy, newDive->buddy))
+		changedFields |= DiveField::BUDDY;
+	if (oldDive->rating != newDive->rating)
+		changedFields |= DiveField::RATING;
+	if (oldDive->visibility != newDive->visibility)
+		changedFields |= DiveField::VISIBILITY;
+	if (oldDive->wavesize != newDive->wavesize)
+		changedFields |= DiveField::WAVESIZE;
+	if (oldDive->current != newDive->current)
+		changedFields |= DiveField::CURRENT;
+	if (oldDive->surge != newDive->surge)
+		changedFields |= DiveField::SURGE;
+	if (oldDive->chill != newDive->chill)
+		changedFields |= DiveField::CHILL;
+	if (!same_string(oldDive->suit, newDive->suit))
+		changedFields |= DiveField::SUIT;
+	if (get_taglist_string(oldDive->tag_list) != get_taglist_string(newDive->tag_list)) // This is cheating. Do we have a taglist comparison function?
+		changedFields |= DiveField::TAGS;
+	if (oldDive->dc.divemode != newDive->dc.divemode)
+		changedFields |= DiveField::MODE;
+	if (!same_string(oldDive->notes, newDive->notes))
+		changedFields |= DiveField::NOTES;
+	if (oldDive->salinity != newDive->salinity)
+		changedFields |= DiveField::SALINITY;
+}
+
+void EditDive::undo()
+{
+	if (siteToRemove) {
+		int idx = unregister_dive_site(siteToRemove);
+		siteToAdd.reset(siteToRemove);
+		emit diveListNotifier.diveSiteDeleted(siteToRemove, idx); // Inform frontend of removed dive site.
+	}
+
+	exchangeDives();
+	editDs();
+}
+
+void EditDive::redo()
+{
+	if (siteToAdd) {
+		siteToRemove = siteToAdd.get();
+		int idx = register_dive_site(siteToAdd.release()); // Return ownership to backend.
+		emit diveListNotifier.diveSiteAdded(siteToRemove, idx); // Inform frontend of new dive site.
+	}
+
+	exchangeDives();
+	editDs();
+}
+
+void EditDive::exchangeDives()
+{
+	// Bluntly exchange dive data by shallow copy
+	std::swap(*newDive, *oldDive);
+	invalidate_dive_cache(oldDive);
+
+	// Changing times may have unsorted the dive and trip tables
+	QVector<dive *> dives = { oldDive };
+	timestamp_t delta = oldDive->when - newDive->when;
+	if (delta != 0) {
+		sort_dive_table(&dive_table);
+		sort_trip_table(&trip_table);
+		if (newDive->divetrip != oldDive->divetrip)
+			qWarning("Command::EditDive::redo(): This command does not support moving between trips!");
+		if (oldDive->divetrip)
+			sort_dive_table(&newDive->divetrip->dives); // Keep the trip-table in order
+		emit diveListNotifier.divesTimeChanged(delta, dives);
+	}
+
+	// Send signals
+	emit diveListNotifier.divesChanged(dives, changedFields);
+
+	// Select the changed dives
+	setSelection( { oldDive }, oldDive);
+}
+
+void EditDive::editDs()
+{
+	if (siteToEdit) {
+		std::swap(siteToEdit->location, dsLocation);
+		emit diveListNotifier.diveSiteChanged(siteToEdit, LocationInformationModel::LOCATION); // Inform frontend of changed dive site.
+	}
+}
+
+bool EditDive::workToBeDone()
+{
+	// We trust the frontend that an EditDive command is only created if there are changes.
+	return true;
+}
+
+#endif // SUBSURFACE_MOBILE
+
 } // namespace Command

commands/command_edit.h

@@ -375,6 +375,33 @@ private:
 	void redo() override;
 };
 
+#ifdef SUBSURFACE_MOBILE
+// Edit a full dive. This is used on mobile where we don't have per-field granularity.
+// It may add or edit a dive site.
+class EditDive : public Base {
+public:
+	EditDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *editDs, location_t dsLocation); // Takes ownership of newDive
+private:
+	dive *oldDive; // Dive that is going to be overwritten
+	OwningDivePtr newDive; // New data
+	int changedFields;
+
+	dive_site *siteToRemove;
+	OwningDiveSitePtr siteToAdd;
+
+	dive_site *siteToEdit;
+	location_t dsLocation;
+
+	void undo() override;
+	void redo() override;
+	bool workToBeDone() override;
+
+	void exchangeDives();
+	void editDs();
+};
+
+#endif // SUBSURFACE_MOBILE
+
 } // namespace Command
 
 #endif

mobile-widgets/qml/DiveDetailsEdit.qml

@@ -112,29 +112,12 @@ Item {
 				      detailsEdit.depthText, detailsEdit.airtempText, detailsEdit.watertempText,
 				      suitBox.editText, buddyBox.editText, divemasterBox.editText,
 				      detailsEdit.weightText, detailsEdit.notesText, startpressure,
-				      endpressure, usedGas, usedCyl ,
+				      endpressure, usedGas, usedCyl,
 				      detailsEdit.rating,
 				      detailsEdit.visibility, state)
 		// trigger the profile to be redrawn
 		QMLProfile.diveId = dive_id
 
-		// apply the changes to the dive detail view - since the edit could have changed the order
-		// first make sure that we are looking at the correct dive - our model allows us to look
-		// up the index based on the unique dive_id
-		var newIdx = diveModel.getIdxForId(dive_id)
-		diveDetailsListView.currentIndex = newIdx
-		diveDetailsListView.currentItem.modelData.date = detailsEdit.dateText
-		diveDetailsListView.currentItem.modelData.location = locationBox.currentText
-		diveDetailsListView.currentItem.modelData.duration = detailsEdit.durationText
-		diveDetailsListView.currentItem.modelData.depth = detailsEdit.depthText
-		diveDetailsListView.currentItem.modelData.airtemp = detailsEdit.airtempText
-		diveDetailsListView.currentItem.modelData.watertemp = detailsEdit.watertempText
-		diveDetailsListView.currentItem.modelData.suit = suitBox.currentText
-		diveDetailsListView.currentItem.modelData.buddy = buddyBox.currentText
-		diveDetailsListView.currentItem.modelData.divemaster = divemasterBox.currentText
-		diveDetailsListView.currentItem.modelData.notes = detailsEdit.notesText
-		diveDetailsListView.currentItem.modelData.rating = detailsEdit.rating
-		diveDetailsListView.currentItem.modelData.visibility = detailsEdit.visibility
 		Qt.inputMethod.hide()
 		// now make sure we directly show the saved dive (this may be a new dive, or it may have moved)
 		clearDetailsEdit()

mobile-widgets/qmlmanager.cpp

@@ -49,6 +49,7 @@
 #include "core/worldmap-save.h"
 #include "core/uploadDiveLogsDE.h"
 #include "core/uploadDiveShare.h"
+#include "commands/command_base.h"
 #include "commands/command.h"
 
 QMLManager *QMLManager::m_instance = NULL;
@@ -874,20 +875,32 @@ void QMLManager::refreshDiveList()
 	MobileModels::instance()->reset();
 }
 
-void QMLManager::setupDivesite(struct dive *d, struct dive_site *ds, double lat, double lon, const char *locationtext)
+// Ouch. Editing a dive might create a dive site or change an existing dive site.
+// The following structure describes such a change caused by a dive edit.
+// Hopefully, we can remove this in due course by using finer-grained undo-commands.
+struct DiveSiteChange {
+	Command::OwningDiveSitePtr createdDs; // not-null if we created a dive site.
+
+	dive_site *editDs = nullptr; // not-null if we are supposed to edit an existing dive site.
+	location_t location = zero_location; // new value of the location if we edit an existing dive site.
+
+	bool changed = false; // true if either a dive site or the dive was changed.
+};
+
+static void setupDivesite(DiveSiteChange &res, struct dive *d, struct dive_site *ds, double lat, double lon, const char *locationtext)
 {
 	location_t location = create_location(lat, lon);
 	if (ds) {
-		ds->location = location;
+		res.editDs = ds;
+		res.location = location;
 	} else {
-		unregister_dive_from_dive_site(d);
-		add_dive_to_dive_site(d, create_dive_site_with_gps(locationtext, &location, &dive_site_table));
-		// We created a new dive site - let the dive site model know.
-		updateSiteList();
+		res.createdDs.reset(create_dive_site_with_gps(locationtext, &location, &dive_site_table));
+		add_dive_to_dive_site(d, res.createdDs.get());
 	}
+	res.changed = true;
 }
 
-bool QMLManager::checkDate(const DiveObjectHelper &myDive, struct dive * d, QString date)
+bool QMLManager::checkDate(const DiveObjectHelper &myDive, struct dive *d, QString date)
 {
 	QString oldDate = myDive.date() + " " + myDive.time();
 	if (date != oldDate) {
@@ -992,20 +1005,19 @@ parsed:
 	return false;
 }
 
-bool QMLManager::checkLocation(const DiveObjectHelper &myDive, struct dive *d, QString location, QString gps)
+bool QMLManager::checkLocation(DiveSiteChange &res, const DiveObjectHelper &myDive, struct dive *d, QString location, QString gps)
 {
-	bool diveChanged = false;
 	struct dive_site *ds = get_dive_site_for_dive(d);
 	qDebug() << "checkLocation" << location << "gps" << gps << "dive had" << myDive.location << "gps" << myDive.gas;
 	if (myDive.location != location) {
-		diveChanged = true;
 		ds = get_dive_site_by_name(qPrintable(location), &dive_site_table);
-		if (!ds && !location.isEmpty())
-			ds = create_dive_site(qPrintable(location), &dive_site_table);
+		if (!ds && !location.isEmpty()) {
+			res.createdDs.reset(create_dive_site(qPrintable(location), &dive_site_table));
+			res.changed = true;
+			ds = res.createdDs.get();
+		}
 		unregister_dive_from_dive_site(d);
 		add_dive_to_dive_site(d, ds);
-		// We created a new dive site - let the dive site model know.
-		updateSiteList();
 	}
 	// now make sure that the GPS coordinates match - if the user changed the name but not
 	// the GPS coordinates, this still does the right thing as the now new dive site will
@@ -1015,18 +1027,15 @@ bool QMLManager::checkLocation(const DiveObjectHelper &myDive, struct dive *d, Q
 		if (parseGpsText(gps, &lat, &lon)) {
 			qDebug() << "parsed GPS, using it";
 			// there are valid GPS coordinates - just use them
-			setupDivesite(d, ds, lat, lon, qPrintable(myDive.location));
-			diveChanged = true;
+			setupDivesite(res, d, ds, lat, lon, qPrintable(myDive.location));
 		} else if (gps == GPS_CURRENT_POS) {
 			qDebug() << "gps was our default text for no GPS";
 			// user asked to use current pos
 			QString gpsString = getCurrentPosition();
 			if (gpsString != GPS_CURRENT_POS) {
 				qDebug() << "but now I got a valid location" << gpsString;
-				if (parseGpsText(qPrintable(gpsString), &lat, &lon)) {
-					setupDivesite(d, ds, lat, lon, qPrintable(myDive.location));
-					diveChanged = true;
-				}
+				if (parseGpsText(qPrintable(gpsString), &lat, &lon))
+					setupDivesite(res, d, ds, lat, lon, qPrintable(myDive.location));
 			} else {
 				appendTextToLog("couldn't get GPS location in time");
 			}
@@ -1035,7 +1044,7 @@ bool QMLManager::checkLocation(const DiveObjectHelper &myDive, struct dive *d, Q
 			appendTextToLog(QString("wasn't able to parse gps string '%1'").arg(gps));
 		}
 	}
-	return diveChanged;
+	return res.changed;
 }
 
 bool QMLManager::checkDuration(const DiveObjectHelper &myDive, struct dive *d, QString duration)
@@ -1101,13 +1110,16 @@ void QMLManager::commitChanges(QString diveId, QString number, QString date, QSt
 			       QString airtemp, QString watertemp, QString suit, QString buddy, QString diveMaster, QString weight, QString notes,
 			       QStringList startpressure, QStringList endpressure, QStringList gasmix, QStringList usedCylinder, int rating, int visibility, QString state)
 {
-	struct dive *d = get_dive_by_uniq_id(diveId.toInt());
+	struct dive *orig = get_dive_by_uniq_id(diveId.toInt());
 
-	if (!d) {
+	if (!orig) {
 		appendTextToLog("cannot commit changes: no dive");
 		return;
 	}
 
+	Command::OwningDivePtr d_ptr(alloc_dive()); // Automatically delete dive if we exit early!
+	dive *d = d_ptr.get();
+	copy_dive(orig, d);
 	DiveObjectHelper myDive(d);
 
 	// notes comes back as rich text - let's convert this into plain text
@@ -1116,11 +1128,11 @@ void QMLManager::commitChanges(QString diveId, QString number, QString date, QSt
 	notes = doc.toPlainText();
 
 	bool diveChanged = false;
-	bool needResort = false;
 
-	diveChanged = needResort = checkDate(myDive, d, date);
+	diveChanged = checkDate(myDive, d, date);
 
-	diveChanged |= checkLocation(myDive, d, location, gps);
+	DiveSiteChange dsChange;
+	diveChanged |= checkLocation(dsChange, myDive, d, location, gps);
 
 	diveChanged |= checkDuration(myDive, d, duration);
 
@@ -1251,22 +1263,6 @@ void QMLManager::commitChanges(QString diveId, QString number, QString date, QSt
 	}
 	// now that we have it all figured out, let's see what we need
 	// to update
-	DiveListModel *dm = DiveListModel::instance();
-	int modelIdx = dm->getDiveIdx(d->id);
-	int oldIdx = get_idx_by_uniq_id(d->id);
-	if (needResort) {
-		// we know that the only thing that might happen in a resort is that
-		// this one dive moves to a different spot in the dive list
-		sort_dive_table(&dive_table);
-		sort_trip_table(&trip_table);
-		int newIdx = get_idx_by_uniq_id(d->id);
-		if (newIdx != oldIdx) {
-			DiveListModel::instance()->removeDive(modelIdx);
-			modelIdx += (newIdx - oldIdx);
-			DiveListModel::instance()->insertDive(modelIdx);
-			diveChanged = true; // because we already modified things
-		}
-	}
 	if (diveChanged) {
 		if (d->maxdepth.mm == d->dc.maxdepth.mm &&
 		    d->maxdepth.mm > 0 &&
@@ -1280,12 +1276,9 @@ void QMLManager::commitChanges(QString diveId, QString number, QString date, QSt
 			fake_dc(&d->dc);
 		}
 		fixup_dive(d);
-		DiveListModel::instance()->updateDive(modelIdx, d);
-		invalidate_dive_cache(d);
-		mark_divelist_changed(true);
-	}
-	if (diveChanged || needResort)
+		Command::editDive(orig, d_ptr.release(), dsChange.createdDs.release(), dsChange.editDs, dsChange.location); // With release() we're giving up ownership
 		changesNeedSaving();
+	}
 }
 
 void QMLManager::changesNeedSaving()

mobile-widgets/qmlmanager.h

@@ -20,6 +20,8 @@
 #include "core/subsurface-qt/divelistnotifier.h"
 
 class QAction;
+class DiveObjectHelper;
+class DiveSiteChange; // An obscure implementation artifact - remove in due course.
 
 class QMLManager : public QObject {
 	Q_OBJECT
@@ -251,7 +253,7 @@ private:
 	QElapsedTimer timer;
 	bool alreadySaving;
 	bool checkDate(const DiveObjectHelper &myDive, struct dive *d, QString date);
-	bool checkLocation(const DiveObjectHelper &myDive, struct dive *d, QString location, QString gps);
+	bool checkLocation(DiveSiteChange &change, const DiveObjectHelper &myDive, struct dive *d, QString location, QString gps);
 	bool checkDuration(const DiveObjectHelper &myDive, struct dive *d, QString duration);
 	bool checkDepth(const DiveObjectHelper &myDive, struct dive *d, QString depth);
 	bool currentGitLocalOnly;
@@ -260,7 +262,8 @@ private:
 	bool m_btEnabled;
 	void updateAllGlobalLists();
 	void updateSiteList();
-	void setupDivesite(struct dive *d, struct dive_site *ds, double lat, double lon, const char *locationtext);
+
+	location_t getGps(QString &gps);
 	QString m_pluggedInDeviceName;
 	bool m_showNonDiveComputers;
 	struct dive *m_copyPasteDive = NULL;