Dive pictures: change removePicture() interface

The function removePicture() had a flag "last", which would indicate
that the called had finished removing pictures. Only then would
the model be recalculated.

This is a strange interface and, matter of fact, the caller was buggy:
if the last picture to be removed didn't have a proper url, removePicture()
was never called with "last" being set.

Change the interface to take a list of pictures to be deleted. This
will allow us to make picture deletion smarter in follow-up commits.

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

4 files changed, 27 insertions, 19 deletions

diff --git a/desktop-widgets/tab-widgets/TabDivePhotos.cpp b/desktop-widgets/tab-widgets/TabDivePhotos.cpp
index a2a41398f..1a5d233e5 100644
--- a/desktop-widgets/tab-widgets/TabDivePhotos.cpp
+++ b/desktop-widgets/tab-widgets/TabDivePhotos.cpp
@@ -60,23 +60,21 @@ void TabDivePhotos::contextMenuEvent(QContextMenuEvent *event)
 
 void TabDivePhotos::removeSelectedPhotos()
 {
-	bool last = false;
 	if (!ui->photosView->selectionModel()->hasSelection())
 		return;
 	QModelIndexList indexes =  ui->photosView->selectionModel()->selectedRows();
 	if (indexes.count() == 0)
 		indexes = ui->photosView->selectionModel()->selectedIndexes();
-	QModelIndex photo = indexes.first();
-	do {
-		photo = indexes.first();
-		last = indexes.count() == 1;
+	QVector<QString> photosToDelete;
+	photosToDelete.reserve(indexes.count());
+	for (const auto &photo: indexes) {
 		if (photo.isValid()) {
 			QString fileUrl = photo.data(Qt::DisplayPropertyRole).toString();
-			if (fileUrl.length() > 0)
-				DivePictureModel::instance()->removePicture(fileUrl, last);
+			if (!fileUrl.isEmpty())
+				photosToDelete.push_back(fileUrl);
 		}
-		indexes.removeFirst();
-	} while(!indexes.isEmpty());
+	}
+	DivePictureModel::instance()->removePictures(photosToDelete);
 }
 
 //TODO: This looks overly wrong. We shouldn't call MainWindow to retrieve the DiveList to add Images.
diff --git a/profile-widget/divepixmapitem.cpp b/profile-widget/divepixmapitem.cpp
index c81cd51e1..27fea611b 100644
--- a/profile-widget/divepixmapitem.cpp
+++ b/profile-widget/divepixmapitem.cpp
@@ -116,6 +116,6 @@ void DivePictureItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
 void DivePictureItem::removePicture()
 {
 #ifndef SUBSURFACE_MOBILE
-	DivePictureModel::instance()->removePicture(fileUrl, true);
+	DivePictureModel::instance()->removePictures({ fileUrl });
 #endif
 }
diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp
index 9b993cb5a..3fa1a07f4 100644
--- a/qt-models/divepicturemodel.cpp
+++ b/qt-models/divepicturemodel.cpp
@@ -127,19 +127,29 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const
 	return ret;
 }
 
-void DivePictureModel::removePicture(const QString &fileUrl, bool last)
+// Return true if we actually removed a picture
+static bool removePictureFromSelectedDive(const char *fileUrl)
 {
 	int i;
 	struct dive *dive;
 	for_each_dive (i, dive) {
-		if (dive->selected && dive_remove_picture(dive, qPrintable(fileUrl)))
-			break;
-	}
-	if (last) {
-		copy_dive(current_dive, &displayed_dive);
-		updateDivePictures();
-		mark_divelist_changed(true);
+		if (dive->selected && dive_remove_picture(dive, fileUrl))
+			return true;
 	}
+	return false;
+}
+
+void DivePictureModel::removePictures(const QVector<QString> &fileUrls)
+{
+	bool removed = false;
+	for (const QString &fileUrl: fileUrls)
+		removed |= removePictureFromSelectedDive(qPrintable(fileUrl));
+	if (!removed)
+		return;
+	copy_dive(current_dive, &displayed_dive);
+	mark_divelist_changed(true);
+
+	updateDivePictures();
 }
 
 int DivePictureModel::rowCount(const QModelIndex &parent) const
diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h
index 7f2cde6d0..61eb1d984 100644
--- a/qt-models/divepicturemodel.h
+++ b/qt-models/divepicturemodel.h
@@ -22,7 +22,7 @@ public:
 	virtual int rowCount(const QModelIndex &parent = QModelIndex()) const;
 	virtual void updateDivePictures();
 	void updateDivePicturesWhenDone(QList<QFuture<void>>);
-	void removePicture(const QString& fileUrl, bool last);
+	void removePictures(const QVector<QString> &fileUrls);
 	int rowDDStart, rowDDEnd;
 	void updateDivePictureOffset(const QString &filename, int offsetSeconds);
 public slots: