enhance "incompatible source" warnings for changesets

CHANGELOG.md

@@ -41,6 +41,8 @@ _Breaking developer changes, which may affect downstream projects or sites that
 #### :scissors: Operations
 #### :camera: Street-Level
 #### :white_check_mark: Validation
+* Show warnings for potentially incompatible sources also in changeset `source` tags ([#11334])
+* Include the number of changeset tags with incompatible sources in `warnings:incompatible_source` changeset tag ([#8400])
 #### :bug: Bugfixes
 #### :earth_asia: Localization
 #### :hourglass: Performance
@@ -49,7 +51,9 @@ _Breaking developer changes, which may affect downstream projects or sites that
 #### :mortar_board: Walkthrough / Help
 #### :hammer: Development
 
+[#8400]: https://github.com/openstreetmap/iD/issues/8400
 [#11141]: https://github.com/openstreetmap/iD/pull/11141
+[#11334]: https://github.com/openstreetmap/iD/pull/11334
 [#11353]: https://github.com/openstreetmap/iD/pull/11353
 
 

data/core.yaml

@@ -623,8 +623,10 @@ en:
     about_changeset_comments: About changeset comments
     about_changeset_comments_link: //wiki.openstreetmap.org/wiki/Good_changeset_comments
     changeset_comment_length_warning: "Changeset comments can have a maximum of {maxChars} characters."
-    google_warning: "You mentioned Google in this comment: remember that copying from Google Maps is strictly forbidden."
-    google_warning_link: https://www.openstreetmap.org/copyright
+    changeset_incompatible_source:
+      comment: 'You mentioned "{value}" in this changeset comment.'
+      source: 'You mentioned "{value}" as a source of this changeset.'
+      link: https://www.openstreetmap.org/copyright
   contributors:
     list: "Edits by {users}"
     truncated_list:

modules/ui/changeset_editor.js

@@ -8,6 +8,7 @@ import { uiCombobox} from './combobox';
 import { uiField } from './field';
 import { uiFormFields } from './form_fields';
 import { utilArrayUniqBy, utilCleanOsmString, utilRebind, utilTriggerEvent, utilUnicodeCharsCount } from '../util';
+import { getIncompatibleSources } from '../validations/incompatible_source';
 
 
 export function uiChangesetEditor(context) {
@@ -97,58 +98,73 @@ export function uiChangesetEditor(context) {
             }
         }
 
-        // Show warning(s) if comment mentions Google or comment length exceeds 255 chars
-        const warnings = [];
-        if (_tags.comment?.match(/google/i)) {
-            warnings.push({
-                id: 'contains "google"',
-                msg: t.append('commit.google_warning'),
-                link: t('commit.google_warning_link')
+        function findIncompatibleSources(str, which) {
+            const incompatibleSources = getIncompatibleSources(str);
+            if (!incompatibleSources) return false;
+            return incompatibleSources.map(rule => {
+                const value = rule.regex.exec(str)[1];
+                return {
+                    id: `incompatible_source.${which}.${rule.id}`,
+                    msg: selection => {
+                        selection.call(t.append(`commit.changeset_incompatible_source.${which}`, { value }));
+                        selection.append('br');
+                        selection
+                            .append('a')
+                            .attr('href', t('commit.changeset_incompatible_source.link'))
+                            .call(t.append(`issues.incompatible_source.reference.${rule.id}`));
+                    }
+                };
             });
         }
+
+        function renderWarnings(warnings, selection, klass) {
+            const entries = selection.selectAll(`.${klass}`)
+                .data(warnings, d => d.id);
+
+            entries.exit()
+                .transition()
+                .duration(200)
+                .style('opacity', 0)
+                .remove();
+
+            const enter = entries.enter()
+                .append('div')
+                .classed('field-warning', true)
+                .classed(klass, true)
+                .style('opacity', 0);
+
+            enter
+                .call(svgIcon('#iD-icon-alert', 'inline'))
+                .append('span');
+
+            enter
+                .transition()
+                .duration(200)
+                .style('opacity', 1);
+
+            entries.merge(enter).selectAll('div > span')
+                .text('')
+                .each(function(d) {
+                    d3_select(this).call(d.msg);
+                });
+        }
+
+        // Show warning(s) if comment mentions an invalid source
+        const commentWarnings = findIncompatibleSources(_tags.comment, 'comment');
+        // also show warning when comment length exceeds 255 chars
         const maxChars = context.maxCharsForTagValue();
         const strLen = utilUnicodeCharsCount(utilCleanOsmString(_tags.comment, Number.POSITIVE_INFINITY));
         if (strLen > maxChars || !true) {
-            warnings.push({
+            commentWarnings.push({
                 id: 'message too long',
                 msg: t.append('commit.changeset_comment_length_warning', { maxChars: maxChars }),
             });
         }
+        renderWarnings(commentWarnings, selection.select('.form-field-comment'), 'comment-warning');
 
-        var commentWarning = selection.select('.form-field-comment').selectAll('.comment-warning')
-            .data(warnings, d => d.id);
-
-        commentWarning.exit()
-            .transition()
-            .duration(200)
-            .style('opacity', 0)
-            .remove();
-
-        var commentEnter = commentWarning.enter()
-            .insert('div', '.comment-warning')
-            .attr('class', 'comment-warning field-warning')
-            .style('opacity', 0);
-
-        commentEnter
-            .call(svgIcon('#iD-icon-alert', 'inline'))
-            .append('span');
-
-        commentEnter
-            .transition()
-            .duration(200)
-            .style('opacity', 1);
-
-        commentWarning.merge(commentEnter).selectAll('div > span')
-            .text('')
-            .each(function(d) {
-                let selection = d3_select(this);
-                if (d.link) {
-                    selection = selection.append('a')
-                        .attr('target', '_blank')
-                        .attr('href', d.link);
-                }
-                selection.call(d.msg);
-            });
+        // Show warning(s) if sources contain an invalid source
+        const sourceWarnings = findIncompatibleSources(_tags.source, 'source');
+        renderWarnings(sourceWarnings, selection.select('.form-field-source'), 'source-warning');
     }
 
 

modules/ui/commit.js

@@ -14,6 +14,7 @@ import { uiCommitWarnings } from './commit_warnings';
 import { uiSectionRawTagEditor } from './sections/raw_tag_editor';
 import { utilArrayGroupBy, utilRebind, utilUniqueDomId } from '../util';
 import { utilDetect } from '../util/detect';
+import { getIncompatibleSources } from '../validations/incompatible_source';
 
 
 var readOnlyTags = [
@@ -55,8 +56,6 @@ export function uiCommit(context) {
         // Initialize changeset if one does not exist yet.
         if (!context.changeset) initChangeset();
 
-        loadDerivedChangesetTags();
-
         selection.call(render);
     }
 
@@ -185,11 +184,18 @@ export function uiCommit(context) {
         }
 
         // add counts of warnings generated by the user's edits
-        var warnings = context.validator()
+        const warnings = context.validator()
             .getIssuesBySeverity({ what: 'edited', where: 'all', includeIgnored: true, includeDisabledRules: true })
             .warning
             .filter(function(issue) { return issue.type !== 'help_request'; });    // exclude 'fixme' and similar - #8603
 
+        // also include "incompatible_source" warnings caused by changeset tags
+        [   ...getIncompatibleSources(tags.comment),
+            ...getIncompatibleSources(tags.source)
+        ].forEach(() => {
+            warnings.push({ type: 'incompatible_source' });
+        });
+
         addIssueCounts(warnings, 'warnings');
 
         // add counts of issues resolved by the user's edits
@@ -200,6 +206,7 @@ export function uiCommit(context) {
     }
 
     function render(selection) {
+        loadDerivedChangesetTags();
 
         var osm = context.connection();
         if (!osm) return;

modules/validations/incompatible_source.js

@@ -2,25 +2,36 @@ import { t } from '../core/localizer';
 import { utilDisplayLabel } from '../util/utilDisplayLabel';
 import { validationIssue, validationIssueFix } from '../core/validation';
 
+const incompatibleRules = [
+  {
+    id: 'amap',
+    regex: /(^amap$|^amap\.com|autonavi|mapabc|高德)/i
+  },
+  {
+    id: 'baidu',
+    regex: /(baidu|mapbar|百度)/i
+  },
+  {
+    id: 'google',
+    regex: /(google)/i,
+    exceptRegex: /((books|drive)\.google|google\s?(books|drive|plus))|(esri\/Google_Africa_Buildings)/i
+  }
+];
+
+/**
+ * @param {string} str String (e.g. tag value) to check for incompatible sources
+ * @returns {{id:string, regex: RegExp, exceptRegex?: RegExp}[]}
+ */
+export function getIncompatibleSources(str) {
+  return incompatibleRules
+    .filter(rule =>
+      rule.regex.test(str) &&
+      !rule.exceptRegex?.test(str)
+    );
+}
 
 export function validationIncompatibleSource() {
   const type = 'incompatible_source';
-  const incompatibleRules = [
-    {
-      id: 'amap',
-      regex: /(^amap$|^amap\.com|autonavi|mapabc|高德)/i
-    },
-    {
-      id: 'baidu',
-      regex: /(baidu|mapbar|百度)/i
-    },
-    {
-      id: 'google',
-      regex: /google/i,
-      exceptRegex: /((books|drive)\.google|google\s?(books|drive|plus))|(esri\/Google_Africa_Buildings)/i
-    }
-  ];
-
 
   const validation = function checkIncompatibleSource(entity) {
     const entitySources = entity.tags && entity.tags.source && entity.tags.source.split(';');
@@ -29,16 +40,8 @@ export function validationIncompatibleSource() {
     const entityID = entity.id;
 
     return entitySources
-      .map(source => {
-        const matchRule = incompatibleRules.find(rule => {
-          if (!rule.regex.test(source)) return false;
-          if (rule.exceptRegex && rule.exceptRegex.test(source)) return false;
-          return true;
-        });
-
-        if (!matchRule) return null;
-
-        return new validationIssue({
+      .flatMap(source => getIncompatibleSources(source)
+        .map(matchRule => new validationIssue({
           type: type,
           severity: 'warning',
           message: (context) => {
@@ -56,10 +59,8 @@ export function validationIncompatibleSource() {
               new validationIssueFix({ title: t.append('issues.fix.remove_proprietary_data.title') })
             ];
           }
-        });
-
-      }).filter(Boolean);
-
+        }))
+      );
 
       function getReference(id) {
         return function showReference(selection) {