From dfd7f98129c2b9cb862ac414a84c5d82ffa407d0 Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Date: Sat, 8 Dec 2018 22:57:39 +0100
Subject: [PATCH] Core: don't remove dive from trip in add_dive_to_trip()

All callers of add_dive_to_trip() work on freshly generated dives,
with one exception, that was redundant anyway. Therefore it is not
necessary to remove the dive from a potential previous trip. Move the
responsibility of removing the dive from a trip to the caller,
respectively remove the redundant call. Add a warning message in the
case that trip is set.

Background: On import (either download or file-import) we might not
want to add trips to the global trip-list. For example to enable undo
of import but more generally to detangle that data flow. Thus,
add_dive_to_trip() should not mingle with the global trip-list,
which it has to do if a trip is deleted because the old dive was
removed.

Analysis of the add_dive_to_trip() callers:

1) core/dive.c

pick_trip():
    called on freshly generated merged dive.

finish_split():
    called on two freshly generated split dives.

2) core/divelist.c

create_and_hookup_trip_from_dive():
    called on freshly downloaded dive in dive_cb().
    called on freshly downloaded dive in record_uemis_dive().

autogroup_dives():
    called on dive from get_dives_to_autogroup(), which only
    finds dives that are outside of trips.

combine_trips():
    unused - removed in sibling commit.

try_to_merge_into():
    this call was actually erroneous - dive was already added
    to trip in try_to_merge(). Remove call.

3) core/libdivecomputer.c

dive_cb():
    called on freshly downloaded dive.

4) core/uemis_downloader.c

record_uemis_dive():
    called on freshly downloaded dive.

5) core/load_git.c

create_new_dive():
    called on freshly allocated dive.

6) core/parse.c

dive_end():
    called on freshly parsed dive.

7) desktop-widgets/command_divelist.cpp

DiveListBase::addDive():
    called on dive which is newly added to core.

moveDiveToTrip():
    called on dive that was removed from trip a few lines above.

8) mobile-widgets/qmlmanager.cpp

QMLManager::undoDelete():
    called on dive where divetrip was reset in the previous line.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
---
 core/divelist.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/core/divelist.c b/core/divelist.c
index 8f4e61707..1cf4f5e00 100644
--- a/core/divelist.c
+++ b/core/divelist.c
@@ -19,7 +19,6 @@
  * void insert_trip(dive_trip_t *dive_trip_p)
  * void unregister_trip(dive_trip_t *trip)
  * void free_trip(dive_trip_t *trip)
- * void remove_dive_from_trip(struct dive *dive)
  * void remove_dive_from_trip(struct dive *dive, bool was_autogen)
  * void add_dive_to_trip(struct dive *dive, dive_trip_t *trip)
  * dive_trip_t *create_and_hookup_trip_from_dive(struct dive *dive)
@@ -910,11 +909,14 @@ void remove_dive_from_trip(struct dive *dive, short was_autogen)
 		delete_trip(trip);
 }
 
+/* Add dive to a trip. Caller is responsible for removing dive
+ * from trip beforehand. */
 void add_dive_to_trip(struct dive *dive, dive_trip_t *trip)
 {
 	if (dive->divetrip == trip)
 		return;
-	remove_dive_from_trip(dive, false);
+	if (dive->divetrip)
+		fprintf(stderr, "Warning: adding dive to trip that has trip set\n");
 	add_dive_to_table(&trip->dives, -1, dive);
 	dive->divetrip = trip;
 }
@@ -1407,10 +1409,8 @@ static bool try_to_merge_into(struct dive *dive_to_add, int idx, bool prefer_imp
 	merged->id = old_dive->id;
 	merged->selected = old_dive->selected;
 	dive_table.dives[idx] = merged;
-	if (trip) {
+	if (trip)
 		remove_dive_from_trip(old_dive, false);
-		add_dive_to_trip(merged, trip);
-	}
 	free_dive(old_dive);
 	remove_dive_from_trip(dive_to_add, false);
 	free_dive(dive_to_add);