Update readme

README.md

@@ -0,0 +1,25 @@
+README 
+======
+
+# Usage
+## Download MapRoulette Tasks
+There are two methods to download tasks from MapRoulette.
+1. From the JOSM `Download data` window (`OpenStreetMap data`, `Raw GPS data`, `Notes`, ..., `MapRoulette Tasks`)
+2. From the `MapRoulette Tasks` toggle dialog (`Download Data`) which downloads data in the current viewport
+
+## Workflow
+1. Select a MapRoulette task in the mapview *or* in the `MapRoulette Tasks` window
+2. Click on `Start Task` -- this will "lock" the task on MapRoulette until you finish and click on `Stop Task`.
+   This helps ensure that you aren't mapping something concurrently with someone else.
+3. Perform the task. Instructions can be viewed in the `Current MapRoulette Task` toggle dialog.
+4. In `Current MapRoulette Task`, select the appropriate action (`False Positive`, `Too hard/Cannot see`, `Fixed`,
+   `Already Fixed/Not an issue`, or `Skipped`; some options are hidden in drop-down menus).
+5. Uploading a changeset or `Stop Task` will then upload the task status.
+
+If a task specifies an OSM primitive
+
+# Author
+* Taylor Smock <[email protected]>
+
+# License
+GPLv3 or any later version

src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java

@@ -1,7 +1,6 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.maproulette.actions.downloadtasks;
 
-import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.io.IOException;
@@ -12,21 +11,14 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import javax.swing.JOptionPane;
-import javax.swing.SwingUtilities;
-
 import org.openstreetmap.josm.actions.downloadtasks.AbstractDownloadTask;
 import org.openstreetmap.josm.actions.downloadtasks.DownloadParams;
 import org.openstreetmap.josm.data.Bounds;
-import org.openstreetmap.josm.data.UserIdentityManager;
-import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
+import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
-import org.openstreetmap.josm.gui.preferences.PreferenceDialog;
-import org.openstreetmap.josm.gui.preferences.server.ServerAccessPreference;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressTaskId;
-import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.OsmApiException;
 import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI;
@@ -96,18 +88,7 @@ protected void realRun() throws IOException, OsmTransferException {
                     TaskCache.isHidden(task);
                 }
             } catch (UnauthorizedException unauthorizedException) {
-                final var message = UserIdentityManager.getInstance().isAnonymous()
-                        ? tr("Please log in to OpenStreetMap in JOSM")
-                        : tr("Please log in to the MapRoulette instance at least once: {0}", getBaseUrl());
-                GuiHelper.runInEDT(() -> {
-                    ConditionalOptionPaneUtil.showMessageDialog("maproulette.user.not.logged.in",
-                            MainApplication.getMainFrame(), message, message, JOptionPane.ERROR_MESSAGE);
-                    if (UserIdentityManager.getInstance().isAnonymous()) {
-                        final var p = new PreferenceDialog(MainApplication.getMainFrame());
-                        SwingUtilities.invokeLater(() -> p.selectPreferencesTabByClass(ServerAccessPreference.class));
-                        p.setVisible(true);
-                    }
-                });
+                ExceptionDialogUtil.explainException(unauthorizedException);
                 // This is specifically so that user's don't get a bug report message
                 final var transferException = new OsmApiException(unauthorizedException);
                 transferException.setUrl(MapRouletteConfig.getBaseUrl());

src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java

@@ -119,6 +119,7 @@ public static Task get(long task) throws IOException {
      * @param task The task to start
      * @return The updated task
      * @throws IOException if there was a problem communicating with the server
+     * @throws UnauthorizedException If we aren't authorized for the server
      */
     public static Task start(long task) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task + "/start");

src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java

@@ -8,29 +8,22 @@
 import java.io.IOException;
 import java.io.Serial;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
 
-import javax.swing.JOptionPane;
 import javax.swing.JTable;
 
 import org.openstreetmap.josm.actions.AutoScaleAction;
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.data.osm.IPrimitive;
-import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI;
-import org.openstreetmap.josm.plugins.maproulette.api.UnauthorizedException;
-import org.openstreetmap.josm.plugins.maproulette.api.enums.TaskStatus;
 import org.openstreetmap.josm.plugins.maproulette.api.model.Task;
 import org.openstreetmap.josm.plugins.maproulette.gui.ModifiedObjects;
 import org.openstreetmap.josm.plugins.maproulette.gui.layer.MapRouletteClusteredPointLayer;
 import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil;
 import org.openstreetmap.josm.tools.ImageProvider;
-import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Shortcut;
 
 /**
@@ -77,26 +70,17 @@ private void lockTasks() {
         final var selected = this.table.getSelectionModel().getSelectedIndices();
         final var model = (TaskTableModel) this.table.getModel();
         final var selectedTasks = new ArrayList<Task>();
-        final var messages = new HashSet<String>();
         for (int index : selected) {
             try {
                 final var i = table.getRowSorter().convertRowIndexToModel(index);
                 final var task = model.get(i);
                 final var lockedTask = TaskAPI.start(task.id());
                 ModifiedObjects.addLockedTask(lockedTask);
                 selectedTasks.add(lockedTask);
-            } catch (UnauthorizedException unauthorizedException) {
-                Logging.trace(unauthorizedException);
-                messages.add(unauthorizedException.getMessage());
             } catch (IOException e) {
                 ExceptionDialogUtil.explainException(e);
             }
         }
-        if (!messages.isEmpty()) {
-            ConditionalOptionPaneUtil.showMessageDialog("maproulette.tasks.locked", MainApplication.getMainFrame(),
-                    "<html>" + messages.stream().sorted().collect(Collectors.joining("<br>")) + "</html>",
-                    tr("Could not lock tasks"), JOptionPane.INFORMATION_MESSAGE);
-        }
 
         ((TaskTableModel) this.table.getModel()).fireTableDataChanged();
         reselect(selected);
@@ -127,7 +111,7 @@ private void unlockTasks() {
                 final var unlockedTask = TaskAPI.release(cluster.id());
                 if (task != null && task.id() == unlockedTask.id()) {
                     final var modified = ModifiedObjects.getModifiedTask(task.id());
-                    if (modified != null && modified.status() != TaskStatus.FIXED) {
+                    if (modified != null) {
                         TaskAPI.updateStatus(task.id(), modified.status(), modified.comment(), modified.tags(),
                                 modified.reviewRequested(), modified.completionResponses());
                         ModifiedObjects.removeModifiedTask(modified);

src/main/java/org/openstreetmap/josm/plugins/maproulette/io/upload/EarlyUploadHook.java

@@ -107,8 +107,8 @@ public boolean checkUpload(APIDataSet apiDataSet) {
                 final var descriptivePanel = createDescriptivePanel(task, apiDataSet);
                 final var didFix = ConditionalOptionPaneUtil.showConfirmationDialog(PREF_CHECK_IF_FINISHED,
                         MainApplication.getMainFrame(), descriptivePanel,
-                        tr("Did you finish the following MapRoulette Task?"), JOptionPane.YES_NO_CANCEL_OPTION,
-                        JOptionPane.QUESTION_MESSAGE, JOptionPane.YES_OPTION);
+                        tr("Did you finish the following MapRoulette Task: {0}?", task.id()),
+                        JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.QUESTION_MESSAGE, JOptionPane.YES_OPTION);
                 if (didFix) {
                     final var doc = (HTMLDocument) ((JosmEditorPane) descriptivePanel.getComponent(1)).getDocument();
                     ModifiedObjects.addModifiedTask(

src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java

@@ -2,6 +2,7 @@
 package org.openstreetmap.josm.plugins.maproulette.util;
 
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
+import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.Component;
@@ -10,9 +11,16 @@
 import java.net.UnknownHostException;
 
 import javax.swing.JOptionPane;
+import javax.swing.SwingUtilities;
 
+import org.openstreetmap.josm.data.UserIdentityManager;
+import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.preferences.PreferenceDialog;
+import org.openstreetmap.josm.gui.preferences.server.ServerAccessPreference;
+import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.plugins.maproulette.api.UnauthorizedException;
 import org.openstreetmap.josm.tools.ExceptionUtil;
 
 /**
@@ -28,7 +36,23 @@ private ExceptionDialogUtil() {
      * @param exception The exception to explain to the user
      */
     public static void explainException(Exception exception) {
-        if (exception instanceof SocketException socketException) {
+        if (exception instanceof UnauthorizedException) {
+            OsmPreferenceUtils.clearCachedKey();
+            final var message = UserIdentityManager.getInstance().isAnonymous()
+                    ? tr("Please log in to OpenStreetMap in JOSM")
+                    : tr("Please log in to the MapRoulette instance at least once: {0}\n NOTE: you may need to reset your MapRoulette API key",
+                            getBaseUrl());
+            GuiHelper.runInEDT(() -> {
+                ConditionalOptionPaneUtil.showMessageDialog("maproulette.user.not.logged.in",
+                        MainApplication.getMainFrame(), message, message, JOptionPane.ERROR_MESSAGE);
+                if (UserIdentityManager.getInstance().isAnonymous()) {
+                    final var p = new PreferenceDialog(MainApplication.getMainFrame());
+                    SwingUtilities.invokeLater(() -> p.selectPreferencesTabByClass(ServerAccessPreference.class));
+                    p.setVisible(true);
+                }
+            });
+            showErrorDialog(message, tr("Unauthorized"), null);
+        } else if (exception instanceof SocketException socketException) {
             final var message = tr("<html>Failed to open a connection to the remote server<br>" + "''{0}''.<br>"
                     + "Please check your internet connection.", socketException.getMessage()) + "</html>";
             showErrorDialog(message, tr("Network exception"), ht("/ErrorMessages#NestedSocketException"));

src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java

@@ -37,16 +37,11 @@ private OsmPreferenceUtils() {
      */
     static String getMapRouletteApiKey() throws UnauthorizedException {
         final var user = UserIdentityManager.getInstance().getUserInfo();
-        final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList());
         if (user == null) {
-            // Right now JOSM doesn't support multiple users, so just wipe everything. If JOSM ever supports multiple users
-            for (var userId : userList.get()) {
-                final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + userId;
-                Config.getPref().put(preferenceKey, null);
-            }
+            clearCachedKey();
             throw new UnauthorizedException("User is not logged in");
         }
-        final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + "." + user.getId();
+        final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + '.' + user.getId();
         final var possibleApiKey = Config.getPref().get(preferenceKey);
         if (!Utils.isStripEmpty(possibleApiKey) && !"Couldn't authenticate you".equals(possibleApiKey)) {
             return possibleApiKey;
@@ -55,12 +50,14 @@ static String getMapRouletteApiKey() throws UnauthorizedException {
                 "maproulette_apikey_v2");
         final var reader = new OsmServerUserPreferencesReader();
         final var monitor = new PleaseWaitProgressMonitor(tr("Fetching OpenStreetMap User Preferences"));
+        final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList());
         try {
             final var key = reader.fetchUserPreferences(monitor, tr("Getting MapRoulette API Key"))
                     .getOrDefault(osmServerKey, null);
+            final String userId = String.valueOf(user.getId());
             List<String> userIds = new ArrayList<>(userList.get());
-            if (!userIds.contains(String.valueOf(user.getId()))) {
-                userIds.add(String.valueOf(user.getId()));
+            if (!userIds.contains(userId)) {
+                userIds.add(userId);
                 userList.put(userIds);
             }
             Config.getPref().put(preferenceKey, key);
@@ -71,4 +68,16 @@ static String getMapRouletteApiKey() throws UnauthorizedException {
             monitor.close();
         }
     }
+
+    /**
+     * Remove all cached keys (this can happen due to auth failure)
+     */
+    static void clearCachedKey() {
+        final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList());
+        // Right now JOSM doesn't support multiple users, so just wipe everything. If JOSM ever supports multiple users
+        for (var userId : userList.get()) {
+            final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + '.' + userId;
+            Config.getPref().put(preferenceKey, null);
+        }
+    }
 }