Changeset 18491 in josm for trunk/src/org

Timestamp:

2022-06-13T23:40:17+02:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #20823: Reject uploads that do not follow either comment policy or source policy (patch by ljdelight)

This fixes an issue where a the length of the comment or source was checked first,
and a warning instead of a rejection was generated when the upload policy would
have otherwise rejected the comment or source.

This also performs the validation check first, prior to the user being able to
make changes in the upload dialog UI, so that the initial dialog state matches
the upload policy.

Location:

trunk/src/org/openstreetmap/josm/gui/io

Files:

• UploadDialog.java (modified) (1 diff)
• UploadTextComponentValidator.java (modified) (4 diffs)

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

@@ -222 +222 @@
         );
 
+        // 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(

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

@@ -84 +84 @@
                 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 />" +
@@ -93 +102 @@
                         "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."));
             }
         }
@@ -121 +123 @@
                 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 />" +
@@ -129 +140 @@
                         "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!"));
             }
         }