Changeset 18839 in josm for trunk

Timestamp:

2023-09-21T19:04:05+02:00 (14 months ago)

Author:

taylor.smock

Message:

Fix #23179: Include changeset in note comment if feasible (patch by qeef, modified)

Modifications are as follows:

• Unit tests
• Better note matching
• Find multiple changesets referring the same note

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/NoteInputDialog.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/dialogs/NotesDialog.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/gui/dialogs/NotesDialogTest.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/gui/NoteInputDialog.java

@@ -45 +45 @@
      */
     public void showNoteDialog(String message, Icon icon) {
+        showNoteDialog(message, icon, "");
+    }
+
+    /**
+     * Displays the dialog to the user
+     * @param message Translated message to display to the user as input prompt
+     * @param icon Icon to display in the action button
+     * @param text Default text of the note's comment
+     * @since xxx
+     */
+    public void showNoteDialog(String message, Icon icon, String text) {
+        textArea.setText(text);
         textArea.setRows(6);
         textArea.setColumns(30);

trunk/src/org/openstreetmap/josm/gui/dialogs/NotesDialog.java

@@ -15 +15 @@
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Predicate;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -41 +43 @@
 import org.openstreetmap.josm.data.notes.Note.State;
 import org.openstreetmap.josm.data.notes.NoteComment;
+import org.openstreetmap.josm.data.osm.Changeset;
+import org.openstreetmap.josm.data.osm.ChangesetCache;
 import org.openstreetmap.josm.data.osm.NoteData;
 import org.openstreetmap.josm.data.osm.NoteData.NoteDataUpdateListener;
@@ -58 +62 @@
 import org.openstreetmap.josm.gui.widgets.JosmTextField;
 import org.openstreetmap.josm.gui.widgets.PopupMenuLauncher;
+import org.openstreetmap.josm.io.OsmApi;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.ImageProvider;
@@ -423 +428 @@
         @Override
         public void actionPerformed(ActionEvent e) {
+            Note note = displayList.getSelectedValue();
+            final List<String> changesetUrls = note == null ? Collections.emptyList() : getRelatedChangesetUrls(note.getId());
             NoteInputDialog dialog = new NoteInputDialog(MainApplication.getMainFrame(), tr("Close note"), tr("Close note"));
-            dialog.showNoteDialog(tr("Close note with message:"), ImageProvider.get("dialogs/notes", "note_closed"));
+            dialog.showNoteDialog(tr("Close note with message:"), ImageProvider.get("dialogs/notes", "note_closed"),
+                    String.join("\n", changesetUrls));
             if (dialog.getValue() != 1) {
                 return;
             }
-            Note note = displayList.getSelectedValue();
             if (note != null) {
                 int selectedIndex = displayList.getSelectedIndex();
@@ -443 +450 @@
 
     /**
+     * Get a list of changeset urls that may have fixed a note
+     * @param noteId The note ID to look for
+     * @return A list of changeset URLs
+     */
+    static List<String> getRelatedChangesetUrls(long noteId) {
+        final List<String> changesetUrls = new ArrayList<>();
+        final int patternFlags = Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS;
+        final Matcher noteMatcher = Pattern.compile("note " + noteId + "( |$)", patternFlags).matcher("");
+        final Matcher shortOsmMatcher = Pattern.compile("osm.org/note/" + noteId + "( |$)", patternFlags).matcher("");
+        final String hostUrl = Pattern.compile("https?://(www\\.)?")
+                .matcher(Config.getUrls().getBaseBrowseUrl())
+                .replaceFirst("");
+        final Matcher longOsmMatcher = Pattern.compile(hostUrl + "/note/" + noteId + "( |$)", patternFlags).matcher("");
+        final boolean isUsingDefaultOsmApi = Config.getUrls().getDefaultOsmApiUrl().equals(OsmApi.getOsmApi().getServerUrl());
+        for (Changeset cs: ChangesetCache.getInstance().getChangesets()) {
+            final String comment = cs.getComment();
+            if ((isUsingDefaultOsmApi && (shortOsmMatcher.reset(comment).find() || noteMatcher.reset(comment).find()))
+                    || longOsmMatcher.reset(comment).find()) {
+                changesetUrls.add(Config.getUrls().getBaseBrowseUrl() + "/changeset/" + cs.getId());
+            }
+        }
+        return changesetUrls;
+    }
+
+    /**
      * Create a new note
      */

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/NotesDialogTest.java

@@ -9 +9 @@
 
 import java.time.Instant;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
+import java.util.stream.Stream;
 
 import javax.swing.JLabel;
@@ -16 +19 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.platform.commons.util.ReflectionUtils;
 import org.openstreetmap.josm.TestUtils;
@@ -21 +27 @@
 import org.openstreetmap.josm.data.notes.Note;
 import org.openstreetmap.josm.data.notes.NoteComment;
+import org.openstreetmap.josm.data.osm.Changeset;
+import org.openstreetmap.josm.data.osm.ChangesetCache;
 import org.openstreetmap.josm.data.osm.User;
 import org.openstreetmap.josm.gui.ExtendedDialog;
@@ -27 +35 @@
 import org.openstreetmap.josm.gui.layer.NoteLayer;
 import org.openstreetmap.josm.gui.widgets.JosmTextField;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
@@ -97 +106 @@
         assertDoesNotThrow(() -> closeAction.actionPerformed(null));
     }
+
+    static Stream<Arguments> testCloseActionGetRelatedChangesetUrls() {
+        return Stream.of(
+                Arguments.of(1, 0, Collections.singletonList("/note/123")),
+                Arguments.of(1, 0, Collections.singletonList("/note/231")),
+                Arguments.of(1, 1, Collections.singletonList("/note/1")),
+                Arguments.of(1, 2, Arrays.asList("/note/1", "/note/1 again")),
+                Arguments.of(1, 2, Arrays.asList("/note/1", "/note/1 again", "/note/12 here")),
+                Arguments.of(1, 2, Arrays.asList("/note/1", "/note/12 again", "/note/1 here")),
+                Arguments.of(1, 2, Arrays.asList("/note/12", "/note/1 again", "/note/1 here")),
+                Arguments.of(1, 3, Arrays.asList("/note/1", "/note/1 again", "/note/1 here")),
+                Arguments.of(1, 3, Arrays.asList("note 1", "note 1 again", "note 1 here"))
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource
+    void testCloseActionGetRelatedChangesetUrls(long noteId, int expectedChangesets, List<String> changesetComments) {
+        try {
+            Config.getPref().put("osm-server.url", null);
+            final String[] apiList = {"osm.org", "openstreetmap.org", Config.getUrls().getBaseBrowseUrl()};
+            for (int i = 0; i < changesetComments.size(); i++) {
+                final String comment = changesetComments.get(i);
+                final Changeset cs = new Changeset(i + 1);
+                cs.put("comment", apiList[i % 3] + comment);
+                ChangesetCache.getInstance().update(cs);
+            }
+            final List<String> changesetUrls = NotesDialog.getRelatedChangesetUrls(noteId);
+            assertEquals(expectedChangesets, changesetUrls.size());
+        } finally {
+            ChangesetCache.getInstance().clear();
+        }
+    }
 }

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.dialogs.layer;