Ticket #20823: patch-20823-3.patch

src/org/openstreetmap/josm/gui/io/UploadDialog.java

@@ -221 +221 @@
                 () -> tpConfigPanels.setSelectedIndex(2)
         );
 
+        // Set the initial state of the upload button
+        btnUpload.setEnabled(pnlBasicUploadSettings.getUploadTextValidators()
+                .stream().noneMatch(UploadTextComponentValidator::isUploadRejected));
+
         // Enable/disable the upload button if at least an upload validator rejects upload
         pnlBasicUploadSettings.getUploadTextValidators().forEach(v -> v.addChangeListener(e -> btnUpload.setEnabled(
                 pnlBasicUploadSettings.getUploadTextValidators().stream().noneMatch(UploadTextComponentValidator::isUploadRejected))));

src/org/openstreetmap/josm/gui/io/UploadTextComponentValidator.java

@@ -83 +83 @@
                 feedbackDisabled();
                 return;
             }
-            String uploadComment = getComponent().getText();
-            if (UploadDialog.UploadAction.isUploadCommentTooShort(uploadComment)) {
+            final String uploadComment = getComponent().getText();
+            final String rejection = UploadDialog.UploadAction.validateUploadTag(uploadComment, "upload.comment",
+                    Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
+
+            // Reject the upload if tags are required and are not in the input. If the tags exist or are not
+            // required, then check the length of the input and warn if it's too short (a short msg is not a rejection)
+            uploadRejected = rejection != null;
+
+            if (uploadRejected) {
+                feedbackWarning(tr("Your upload comment is <i>rejected</i>.") + "<br />" + rejection);
+            } else if (UploadDialog.UploadAction.isUploadCommentTooShort(uploadComment)) {
                 feedbackWarning(tr("Your upload comment is <i>empty</i>, or <i>very short</i>.<br /><br />" +
                         "This is technically allowed, but please consider that many users who are<br />" +
                         "watching changes in their area depend on meaningful changeset comments<br />" +
@@ -92 +101 @@
                         "If you spend a minute now to explain your change, you will make life<br />" +
                         "easier for many other mappers.").replace("<br />", " "));
             } else {
-                String rejection = UploadDialog.UploadAction.validateUploadTag(uploadComment, "upload.comment",
-                        Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
-                uploadRejected = rejection != null;
-                if (uploadRejected) {
-                    feedbackWarning(tr("Your upload comment is <i>rejected</i>.") + "<br />" + rejection);
-                } else {
-                    feedbackValid(tr("Thank you for providing a changeset comment! " +
-                            "This gives other mappers a better understanding of your intent."));
-                }
+                feedbackValid(tr("Thank you for providing a changeset comment! " +
+                        "This gives other mappers a better understanding of your intent."));
             }
         }
     }
@@ -120 +122 @@
                 feedbackDisabled();
                 return;
             }
-            String uploadSource = getComponent().getText();
-            if (Utils.isStripEmpty(uploadSource)) {
+            final String uploadSource = getComponent().getText();
+            final String rejection = UploadDialog.UploadAction.validateUploadTag(
+                    uploadSource, "upload.source", Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
+
+            // Reject the upload only if tags are required and are not in the input. If the tags exist or are not
+            // required, then check the length of the input and warn if it's too short (a short msg is not a rejection)
+            uploadRejected = rejection != null;
+
+            if (uploadRejected) {
+                feedbackWarning(tr("Your changeset source is <i>rejected</i>.") + "<br />" + rejection);
+            } else if (Utils.isStripEmpty(uploadSource)) {
                 feedbackWarning(tr("You did not specify a source for your changes.<br />" +
                         "It is technically allowed, but this information helps<br />" +
                         "other users to understand the origins of the data.<br /><br />" +
                         "If you spend a minute now to explain your change, you will make life<br />" +
                         "easier for many other mappers.").replace("<br />", " "));
             } else {
-                final String rejection = UploadDialog.UploadAction.validateUploadTag(
-                        uploadSource, "upload.source", Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
-                uploadRejected = rejection != null;
-                if (uploadRejected) {
-                    feedbackWarning(tr("Your changeset source is <i>rejected</i>.") + "<br />" + rejection);
-                } else {
-                    feedbackValid(tr("Thank you for providing the data source!"));
-                }
+                feedbackValid(tr("Thank you for providing the data source!"));
             }
         }
     }

test/unit/org/openstreetmap/josm/gui/io/UploadTextComponentValidatorTest.java

@@ -8 +8 @@
 import javax.swing.JTextField;
 
 import org.junit.jupiter.api.Test;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 
+import java.util.Arrays;
+import java.util.Collections;
+
 @BasicPreferences
 class UploadTextComponentValidatorTest {
     /**
@@ -39 +43 @@
         textField.setText("a comment long enough");
         assertThat(feedback.getText(), containsString("Thank you for providing the data source"));
     }
+
+    /**
+     * Unit test of {@link UploadTextComponentValidator.UploadCommentValidator} with mandatory terms
+     */
+    @Test
+    void testUploadCommentWithMandatoryTerm() {
+        testUploadWithMandatoryTermHelper("upload.comment.mandatory-terms");
+    }
+
+
+    /**
+     * Unit test of {@link UploadTextComponentValidator.UploadSourceValidator} with mandatory terms
+     */
+    @Test
+    void testUploadSourceWithMandatoryTerm() {
+        testUploadWithMandatoryTermHelper("upload.source.mandatory-terms");
+    }
+
+    void testUploadWithMandatoryTermHelper(String confPref) {
+        Config.getPref().putList(confPref, Arrays.asList("myrequired", "xyz"));
+        JTextField textField = new JTextField("");
+        JLabel feedback = new JLabel();
+
+        if ("upload.comment.mandatory-terms".equalsIgnoreCase(confPref)) {
+            new UploadTextComponentValidator.UploadCommentValidator(textField, feedback);
+        } else if ("upload.source.mandatory-terms".equalsIgnoreCase(confPref)) {
+            new UploadTextComponentValidator.UploadSourceValidator(textField, feedback);
+        } else {
+            assertThat("Invalid configuration pref", false);
+        }
+
+        // A too-short string should fail validation
+        textField.setText("");
+        assertThat(feedback.getText(), containsString("The following required terms are missing: [myrequired, xyz]"));
+
+        // A long enough string without the mandatory terms should claim that the required terms are missing
+        textField.setText("a string long enough but missing the mandatory term");
+        assertThat(feedback.getText(), containsString("The following required terms are missing: [myrequired, xyz]"));
+
+        // A valid string should pass
+        textField.setText("a string long enough with the mandatory term #myrequired #xyz");
+        if ("upload.comment.mandatory-terms".equalsIgnoreCase(confPref)) {
+            assertThat(feedback.getText(), containsString("Thank you for providing a changeset comment"));
+        } else {
+            assertThat(feedback.getText(), containsString("Thank you for providing the data source"));
+        }
+
+        Config.getPref().putList(confPref, Collections.emptyList());
+    }
 }