If everything else is the same, select older element as merge target

MarcusWolschon · Dec 8, 2024 · fb916bc · fb916bc
1 parent 32e6743
commit fb916bc
Show file tree

Hide file tree

Showing 6 changed files with 150 additions and 72 deletions.
diff --git a/src/androidTest/java/de/blau/android/TestUtils.java b/src/androidTest/java/de/blau/android/TestUtils.java
@@ -794,9 +794,10 @@ public static boolean clickTextContains(@NonNull UiDevice device, boolean clicka
             try {
                 if (waitForNewWindow) {
                     button.clickAndWaitForNewWindow();
+                    Log.w(DEBUG_TAG, ".... clicked waiting for new window");
                 } else {
                     button.click();
-                    Log.e(DEBUG_TAG, ".... clicked");
+                    Log.w(DEBUG_TAG, ".... clicked");
                 }
                 return true;
             } catch (UiObjectNotFoundException e) {

diff --git a/src/androidTest/java/de/blau/android/easyedit/NodeTest.java b/src/androidTest/java/de/blau/android/easyedit/NodeTest.java
@@ -181,6 +181,7 @@ public void unjoinMergeWays() {
     @Test
     public void unjoinMergeNodes() {
         TestUtils.zoomToLevel(device, main, 21);
+        TestUtils.unlock(device);
         TestUtils.clickAtCoordinates(device, map, 8.3866386, 47.3904394, true);
         assertTrue(TestUtils.findText(device, false, context.getString(R.string.actionmode_nodeselect)));
         Node node = App.getLogic().getSelectedNode();
@@ -195,11 +196,14 @@ public void unjoinMergeNodes() {
 
         TestUtils.clickAtCoordinates(device, map, 8.3866386, 47.3904394, false);
         TestUtils.clickAwayTip(device, context);
-        assertTrue(TestUtils.clickTextContains(device, false, " #-2221", false)); // the first node in the list
+        assertTrue(TestUtils.clickTextContains(device, false, " #-2221", true));
+
         assertTrue(TestUtils.clickMenuButton(device, context.getString(R.string.menu_merge), false, true));
 
         // merge all
-        assertTrue(TestUtils.clickText(device, false, context.getString(R.string.merge_with_all_nodes), false, false));
+        assertTrue(TestUtils.clickText(device, false, context.getString(R.string.merge_with_all_nodes), true, false));
+        TestUtils.textGone(device, context.getString(R.string.toast_merged), 2000);
+
         assertEquals(apiNodeCount + 1, delegator.getApiNodeCount());
 
         node = App.getLogic().getSelectedNode();

diff --git a/src/main/java/de/blau/android/Logic.java b/src/main/java/de/blau/android/Logic.java
@@ -2218,7 +2218,7 @@ public synchronized List<Result> performExtractSegment(@Nullable FragmentActivit
      * @param results a list of Result
      */
     private void checkForArea(@Nullable FragmentActivity activity, @NonNull Way way, @NonNull List<Result> results) {
-        if (way.hasTag(Tags.KEY_AREA, Tags.VALUE_YES) || ( activity != null && App.getAreaTags(activity).isImpliedArea(way.getTags()))) {
+        if (way.hasTag(Tags.KEY_AREA, Tags.VALUE_YES) || (activity != null && App.getAreaTags(activity).isImpliedArea(way.getTags()))) {
             results.get(0).addIssue(SplitIssue.SPLIT_AREA);
         }
     }
@@ -2521,6 +2521,7 @@ public List<OsmElement> findJoinableElements(@NonNull Node nodeToJoin) {
      */
     @NonNull
     public synchronized List<Result> performMergeNodes(@Nullable FragmentActivity activity, @NonNull List<OsmElement> elements, @NonNull Node nodeToJoin) {
+        Log.d(DEBUG_TAG, "performMergeNodes " + nodeToJoin.getOsmId() + " " + elements.size() + " targets");
         List<Result> overallResult = new ArrayList<>();
         if (elements.isEmpty()) {
             return overallResult;

diff --git a/src/main/java/de/blau/android/easyedit/NodeSelectionActionModeCallback.java b/src/main/java/de/blau/android/easyedit/NodeSelectionActionModeCallback.java
@@ -233,6 +233,7 @@ public boolean onActionItemClicked(ActionMode mode, MenuItem item) {
      * 
      */
     private void mergeNodeWith(@NonNull List<OsmElement> target) {
+        Log.d(DEBUG_TAG, "mergeNodesWith " + element.getDescription(main));
         try {
             List<Result> result = target.get(0) instanceof Way ? logic.performJoinNodeToWays(main, target, (Node) element)
                     : logic.performMergeNodes(main, target, (Node) element);
@@ -286,6 +287,7 @@ private <T extends OsmElement> void addElementsToContextMenu(@NonNull Disambigua
 
     @Override
     public void onItemClick(int position) {
+        Log.d(DEBUG_TAG, "onItemClick " + position);
         int itemId = position;
         if (action == MENUITEM_JOIN) {
             if (itemId == 0) {

diff --git a/src/main/java/de/blau/android/osm/MergeAction.java b/src/main/java/de/blau/android/osm/MergeAction.java
@@ -66,8 +66,10 @@ public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElemen
     public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElement mergeInto, @NonNull OsmElement mergeFrom, boolean swappable) {
         this.delegator = delegator;
         // first determine if one of the elements already has a valid id, if it is not and other node has valid id swap
-        // else check version numbers, the point of this is to preserve as much history as possible
-        if (swappable && (((mergeInto.getOsmId() < 0) && (mergeFrom.getOsmId() > 0)) || mergeInto.getOsmVersion() < mergeFrom.getOsmVersion())) {
+        // else check version numbers, then choose the older element. The point of this is to preserve as much history
+        // as possible
+        if (swappable && mergeFrom.getOsmId() > 0
+                && (mergeInto.getOsmId() < 0 || mergeInto.getOsmVersion() < mergeFrom.getOsmVersion() || mergeInto.getOsmId() > mergeFrom.getOsmId())) {
             // swap
             Log.d(DEBUG_TAG, "swap into #" + mergeInto.getOsmId() + " with from #" + mergeFrom.getOsmId());
             OsmElement tmpElement = mergeInto;