From 97cb90b562cfac47795c1f4bfa1a8cb2e11b6e57 Mon Sep 17 00:00:00 2001
From: Quincy Morgan <quincy.morgan@icloud.com>
Date: Tue, 7 May 2019 14:48:30 -0400
Subject: [PATCH] Make issue messages dynamic (close #6331)

---
 data/core.yaml                             |  4 ++--
 dist/locales/en.json                       |  4 ++--
 modules/core/validation/models.js          |  2 +-
 modules/services/maprules.js               |  7 ++++--
 modules/ui/commit_warnings.js              |  4 ++--
 modules/ui/entity_issues.js                |  4 ++--
 modules/ui/issues.js                       |  4 ++--
 modules/validations/almost_junction.js     | 12 ++++++----
 modules/validations/close_nodes.js         |  5 ++++-
 modules/validations/crossing_ways.js       | 14 +++++++-----
 modules/validations/disconnected_way.js    | 16 +++++--------
 modules/validations/fixme_tag.js           |  5 ++++-
 modules/validations/generic_name.js        |  8 +++++--
 modules/validations/impossible_oneway.js   |  9 +++++---
 modules/validations/incompatible_source.js |  9 +++++---
 modules/validations/missing_role.js        | 12 ++++++----
 modules/validations/missing_tag.js         | 26 +++++++++-------------
 modules/validations/outdated_tags.js       | 11 ++++++---
 modules/validations/private_data.js        |  7 +++---
 modules/validations/tag_suggests_area.js   |  9 ++++++--
 modules/validations/unsquare_way.js        |  7 +++---
 21 files changed, 106 insertions(+), 73 deletions(-)

diff --git a/data/core.yaml b/data/core.yaml
index b7f7fbb1f..9ac8b5e62 100644
--- a/data/core.yaml
+++ b/data/core.yaml
@@ -1402,8 +1402,8 @@ en:
         message: "{feature} has no tags"
       descriptive:
         message: "{feature} has no descriptive tags"
-      specific:
-        message: '{feature} has no "{tag}" tag'
+      relation_type:
+        message: "{feature} is a relation without a type"
     old_multipolygon:
       message: "{multipolygon} has misplaced tags"
       reference: "Multipolygons should be tagged on their relation, not their outer way."
diff --git a/dist/locales/en.json b/dist/locales/en.json
index 081983136..da0da6702 100644
--- a/dist/locales/en.json
+++ b/dist/locales/en.json
@@ -1732,8 +1732,8 @@
                 "descriptive": {
                     "message": "{feature} has no descriptive tags"
                 },
-                "specific": {
-                    "message": "{feature} has no \"{tag}\" tag"
+                "relation_type": {
+                    "message": "{feature} is a relation without a type"
                 }
             },
             "old_multipolygon": {
diff --git a/modules/core/validation/models.js b/modules/core/validation/models.js
index 5a57fe284..683971d11 100644
--- a/modules/core/validation/models.js
+++ b/modules/core/validation/models.js
@@ -4,7 +4,7 @@ export function validationIssue(attrs) {
     this.type = attrs.type;                // required - name of rule that created the issue (e.g. 'missing_tag')
     this.subtype = attrs.subtype;          // optional - category of the issue within the type (e.g. 'relation_type' under 'missing_tag')
     this.severity = attrs.severity;        // required - 'warning' or 'error'
-    this.message = attrs.message;          // required - localized string
+    this.message = attrs.message;          // required - function returning localized string
     this.reference = attrs.reference;      // optional - function(selection) to render reference information
     this.entityIds = attrs.entityIds;      // optional - array of IDs of entities involved in the issue
     this.loc = attrs.loc;                  // optional - [lon, lat] to zoom in on to see the issue
diff --git a/modules/services/maprules.js b/modules/services/maprules.js
index 1f7c54298..8157639b9 100644
--- a/modules/services/maprules.js
+++ b/modules/services/maprules.js
@@ -218,11 +218,14 @@ export default {
                     var severity = Object.keys(selector).indexOf('error') > -1
                             ? 'error'
                             : 'warning';
+                    var message = selector[severity];
                     issues.push(new validationIssue({
                         type: 'maprules',
                         severity: severity,
-                        message: selector[severity],
-                        entityIds: [entity.id],
+                        message: function() {
+                            return message;
+                        },
+                        entityIds: [entity.id]
                     }));
                 }
             }
diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js
index 329e5bcb4..83edd3dc3 100644
--- a/modules/ui/commit_warnings.js
+++ b/modules/ui/commit_warnings.js
@@ -39,7 +39,7 @@ export function uiCommitWarnings(context) {
 
 
             var items = container.select('ul').selectAll('li')
-                .data(issues, function(d) { return d.id; });
+                .data(issues, function(d) { return d.message() + d.id; });
 
             items.exit()
                 .remove();
@@ -53,7 +53,7 @@ export function uiCommitWarnings(context) {
 
             itemsEnter
                 .append('strong')
-                .text(function(d) { return d.message; });
+                .text(function(d) { return d.message(); });
 
             itemsEnter.filter(function(d) { return d.tooltip; })
                 .call(tooltip()
diff --git a/modules/ui/entity_issues.js b/modules/ui/entity_issues.js
index a63e45215..c8c8d95c9 100644
--- a/modules/ui/entity_issues.js
+++ b/modules/ui/entity_issues.js
@@ -49,7 +49,7 @@ export function uiEntityIssues(context) {
 
 
         var containers = selection.selectAll('.issue-container')
-            .data(issues, function(d) { return d.id; });
+            .data(issues, function(d) { return d.message() + d.id; });
 
         // Exit
         containers.exit()
@@ -109,7 +109,7 @@ export function uiEntityIssues(context) {
         textEnter
             .append('span')
             .attr('class', 'issue-message')
-            .text(function(d) { return d.message; });
+            .text(function(d) { return d.message(); });
 
 
         var infoButton = labelsEnter
diff --git a/modules/ui/issues.js b/modules/ui/issues.js
index 645c397fb..50c1c2aba 100644
--- a/modules/ui/issues.js
+++ b/modules/ui/issues.js
@@ -73,7 +73,7 @@ export function uiIssues(context) {
 
 
         var items = list.selectAll('li')
-            .data(issues, function(d) { return d.id; });
+            .data(issues, function(d) { return d.message() + d.id; });
 
         // Exit
         items.exit()
@@ -127,7 +127,7 @@ export function uiIssues(context) {
         textEnter
             .append('span')
             .attr('class', 'issue-message')
-            .text(function(d) { return d.message; });
+            .text(function(d) { return d.message(); });
 
 
         labelsEnter
diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js
index 8d4903700..f05800d72 100644
--- a/modules/validations/almost_junction.js
+++ b/modules/validations/almost_junction.js
@@ -89,10 +89,14 @@ export function validationAlmostJunction() {
             issues.push(new validationIssue({
                 type: type,
                 severity: 'warning',
-                message: t('issues.almost_junction.message', {
-                    feature: utilDisplayLabel(entity, context),
-                    feature2: utilDisplayLabel(edgeHighway, context)
-                }),
+                message: function() {
+                    var entity1 = context.hasEntity(this.entityIds[0]),
+                        entity2 = context.hasEntity(this.entityIds[2]);
+                    return (entity && entity2) ? t('issues.almost_junction.message', {
+                        feature: utilDisplayLabel(entity1, context),
+                        feature2: utilDisplayLabel(entity2, context)
+                    }) : '';
+                },
                 reference: showReference,
                 entityIds: [entity.id, node.id, edgeHighway.id],
                 loc: extendableNodeInfo.node.loc,
diff --git a/modules/validations/close_nodes.js b/modules/validations/close_nodes.js
index ed9583a32..846d98e70 100644
--- a/modules/validations/close_nodes.js
+++ b/modules/validations/close_nodes.js
@@ -65,7 +65,10 @@ export function validationCloseNodes() {
         return new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.close_nodes.message', { way: utilDisplayLabel(way, context) }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.close_nodes.message', { way: utilDisplayLabel(entity, context) }) : '';
+            },
             reference: showReference,
             entityIds: [way.id, node1.id, node2.id],
             loc: node1.loc,
diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js
index edf2f52ba..e57054af1 100644
--- a/modules/validations/crossing_ways.js
+++ b/modules/validations/crossing_ways.js
@@ -403,11 +403,6 @@ export function validationCrossingWays() {
             crossingTypeID += '_connectable';
         }
 
-        var messageDict = {
-            feature: utilDisplayLabel(entities[0], context),
-            feature2: utilDisplayLabel(entities[1], context)
-        };
-
         var fixes = [];
         if (connectionTags) {
             fixes.push(new validationIssueFix({
@@ -485,7 +480,14 @@ export function validationCrossingWays() {
         return new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.crossing_ways.message', messageDict),
+            message: function() {
+                var entity1 = context.hasEntity(this.entityIds[0]),
+                    entity2 = context.hasEntity(this.entityIds[1]);
+                return (entity1 && entity2) ? t('issues.crossing_ways.message', {
+                    feature: utilDisplayLabel(entity1, context),
+                    feature2: utilDisplayLabel(entity2, context)
+                }) : '';
+            },
             reference: showReference,
             entityIds: entities.map(function(entity) {
                 return entity.id;
diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js
index 05841e5a9..262789bb0 100644
--- a/modules/validations/disconnected_way.js
+++ b/modules/validations/disconnected_way.js
@@ -29,7 +29,6 @@ export function validationDisconnectedWay() {
             return [];
         }
 
-        var entityLabel = utilDisplayLabel(entity, context);
         var fixes = [];
         var entityID = entity.id;
 
@@ -84,18 +83,15 @@ export function validationDisconnectedWay() {
                 }
             }));
         }
-
+        var suffix = isNewRoad(entity.id) ? '_new_road' : '';
         return [new validationIssue({
             type: type,
             severity: 'warning',
-            message: (isNewRoad(entity.id)
-                ? t('issues.disconnected_way.highway.message_new_road', { highway: entityLabel })
-                : t('issues.disconnected_way.highway.message', { highway: entityLabel })
-            ),
-            tooltip: (isNewRoad(entity.id)
-                ? t('issues.disconnected_way.highway.reference_new_road')
-                : t('issues.disconnected_way.highway.reference')
-            ),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.disconnected_way.highway.message' + suffix, { highway: utilDisplayLabel(entity, context) }) : '';
+            },
+            tooltip: t('issues.disconnected_way.highway.reference' + suffix),
             reference: showReference,
             entityIds: [entity.id],
             fixes: fixes
diff --git a/modules/validations/fixme_tag.js b/modules/validations/fixme_tag.js
index 5257ccdae..ea7be0039 100644
--- a/modules/validations/fixme_tag.js
+++ b/modules/validations/fixme_tag.js
@@ -23,7 +23,10 @@ export function validationFixmeTag() {
         return [new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.fixme_tag.message', { feature: utilDisplayLabel(entity, context) }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.fixme_tag.message', { feature: utilDisplayLabel(entity, context) }) : '';
+            },
             reference: showReference,
             entityIds: [entity.id]
         })];
diff --git a/modules/validations/generic_name.js b/modules/validations/generic_name.js
index 9b20f423f..8d31703cf 100644
--- a/modules/validations/generic_name.js
+++ b/modules/validations/generic_name.js
@@ -53,11 +53,15 @@ export function validationGenericName() {
         var generic = isGenericName(entity);
         if (!generic) return [];
 
-        var preset = utilPreset(entity, context);
         return [new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.generic_name.message', {feature: preset.name(), name: generic}),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                if (!entity) return '';
+                var preset = utilPreset(entity, context);
+                return t('issues.generic_name.message', { feature: preset.name(), name: generic });
+            },
             reference: showReference,
             entityIds: [entity.id],
             hash: generic,
diff --git a/modules/validations/impossible_oneway.js b/modules/validations/impossible_oneway.js
index 03443803e..e7b3d2f17 100644
--- a/modules/validations/impossible_oneway.js
+++ b/modules/validations/impossible_oneway.js
@@ -135,9 +135,12 @@ export function validationImpossibleOneway() {
             type: type,
             subtype: wayType,
             severity: 'warning',
-            message: t('issues.impossible_oneway.' + messageID + '.message', {
-                feature: utilDisplayLabel(way, context)
-            }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.impossible_oneway.' + messageID + '.message', {
+                    feature: utilDisplayLabel(entity, context)
+                }) : '';
+            },
             reference: getReference(referenceID),
             entityIds: [way.id, node.id],
             fixes: fixes
diff --git a/modules/validations/incompatible_source.js b/modules/validations/incompatible_source.js
index 5f369c643..50b0d3eed 100644
--- a/modules/validations/incompatible_source.js
+++ b/modules/validations/incompatible_source.js
@@ -18,9 +18,12 @@ export function validationIncompatibleSource() {
                     issues.push(new validationIssue({
                         type: type,
                         severity: 'warning',
-                        message: t('issues.incompatible_source.' + invalidSource.id + '.feature.message', {
-                            feature: utilDisplayLabel(entity, context),
-                        }),
+                        message: function() {
+                            var entity = context.hasEntity(this.entityIds[0]);
+                            return entity ? t('issues.incompatible_source.' + invalidSource.id + '.feature.message', {
+                                feature: utilDisplayLabel(entity, context)
+                            }) : '';
+                        },
                         reference: getReference(invalidSource.id),
                         entityIds: [entity.id],
                         fixes: [
diff --git a/modules/validations/missing_role.js b/modules/validations/missing_role.js
index 22ed2c25e..0ab0f8331 100644
--- a/modules/validations/missing_role.js
+++ b/modules/validations/missing_role.js
@@ -41,10 +41,14 @@ export function validationMissingRole() {
         return new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.missing_role.message', {
-                member: utilDisplayLabel(way, context),
-                relation: utilDisplayLabel(relation, context),
-            }),
+            message: function() {
+                var member = context.hasEntity(this.entityIds[1]),
+                    relation = context.hasEntity(this.entityIds[0]);
+                return (member && relation) ? t('issues.missing_role.message', {
+                    member: utilDisplayLabel(member, context),
+                    relation: utilDisplayLabel(relation, context)
+                }) : '';
+            },
             reference: showReference,
             entityIds: [relation.id, way.id],
             data: {
diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js
index 67b456f90..1e69ba43f 100644
--- a/modules/validations/missing_tag.js
+++ b/modules/validations/missing_tag.js
@@ -43,30 +43,21 @@ export function validationMissingTag() {
             return [];
         }
 
-        var messageObj = {};
-        var missingTagType;
         var subtype;
 
         if (Object.keys(entity.tags).length === 0) {
-            missingTagType = 'any';
             subtype = 'any';
         } else if (!hasDescriptiveTags(entity)) {
-            missingTagType = 'descriptive';
             subtype = 'descriptive';
         } else if (isUntypedRelation(entity)) {
-            missingTagType = 'specific';
-            messageObj.tag = 'type';
             subtype = 'relation_type';
         } else if (isUnknownRoad(entity)) {
-            missingTagType = 'unknown_road';
             subtype = 'highway_classification';
         }
 
-        if (!missingTagType) return [];
+        if (!subtype) return [];
 
-        messageObj.feature = utilDisplayLabel(entity, context);
-
-        var selectFixType = missingTagType === 'unknown_road' ? 'select_road_type' : 'select_preset';
+        var selectFixType = subtype === 'highway_classification' ? 'select_road_type' : 'select_preset';
 
         var fixes = [
             new validationIssueFix({
@@ -102,16 +93,21 @@ export function validationMissingTag() {
             );
         }
 
-        var messageID = missingTagType === 'unknown_road' ? 'unknown_road' : 'missing_tag.' + missingTagType;
-        var referenceID = missingTagType === 'unknown_road' ? 'unknown_road' : 'missing_tag';
+        var messageID = subtype === 'highway_classification' ? 'unknown_road' : 'missing_tag.' + subtype;
+        var referenceID = subtype === 'highway_classification' ? 'unknown_road' : 'missing_tag';
 
-        var severity = (canDelete && missingTagType !== 'unknown_road') ? 'error' : 'warning';
+        var severity = (canDelete && subtype !== 'highway_classification') ? 'error' : 'warning';
 
         return [new validationIssue({
             type: type,
             subtype: subtype,
             severity: severity,
-            message: t('issues.' + messageID + '.message', messageObj),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.' + messageID + '.message', {
+                    feature: utilDisplayLabel(entity, context)
+                }) : '';
+            },
             reference: showReference,
             entityIds: [entity.id],
             fixes: fixes
diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js
index 345b2c734..57cc1e7f6 100644
--- a/modules/validations/outdated_tags.js
+++ b/modules/validations/outdated_tags.js
@@ -55,7 +55,10 @@ export function validationOutdatedTags() {
             type: type,
             subtype: subtype,
             severity: 'warning',
-            message: t('issues.outdated_tags.message', { feature: utilDisplayLabel(entity, context) }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.outdated_tags.message', { feature: utilDisplayLabel(entity, context) }) : '';
+            },
             reference: showReference,
             entityIds: [entity.id],
             hash: JSON.stringify(tagDiff),
@@ -124,12 +127,14 @@ export function validationOutdatedTags() {
 
         if (!multipolygon || !outerWay) return [];
 
-        var multipolygonLabel = utilDisplayLabel(multipolygon, context);
         return [new validationIssue({
             type: type,
             subtype: 'old_multipolygon',
             severity: 'warning',
-            message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }),
+            message: function() {
+                var entity = context.hasEntity(this.issue.entityIds[1]);
+                return entity ? t('issues.old_multipolygon.message', { multipolygon: utilDisplayLabel(entity, context) }) : '';
+            },
             reference: showReference,
             entityIds: [outerWay.id, multipolygon.id],
             fixes: [
diff --git a/modules/validations/private_data.js b/modules/validations/private_data.js
index 9176adb35..9ae54241f 100644
--- a/modules/validations/private_data.js
+++ b/modules/validations/private_data.js
@@ -58,9 +58,10 @@ export function validationPrivateData() {
         return [new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.private_data.contact.message', {
-                feature: utilDisplayLabel(entity, context),
-            }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.private_data.contact.message', { feature: utilDisplayLabel(entity, context) }) : '';
+            },
             reference: showReference,
             entityIds: [entity.id],
             data: {
diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js
index 77c051497..59e1e79fd 100644
--- a/modules/validations/tag_suggests_area.js
+++ b/modules/validations/tag_suggests_area.js
@@ -94,11 +94,16 @@ export function validationTagSuggestsArea() {
             }
         }));
 
-        var featureLabel = utilDisplayLabel(entity, context);
         return [new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.tag_suggests_area.message', { feature: featureLabel, tag: tagText }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.tag_suggests_area.message', {
+                    feature: utilDisplayLabel(entity, context),
+                    tag: tagText
+                }) : '';
+            },
             reference: showReference,
             entityIds: [entity.id],
             hash: JSON.stringify(tagSuggestingArea),
diff --git a/modules/validations/unsquare_way.js b/modules/validations/unsquare_way.js
index ab4e9582c..368d4ca3b 100644
--- a/modules/validations/unsquare_way.js
+++ b/modules/validations/unsquare_way.js
@@ -60,9 +60,10 @@ export function validationUnsquareWay() {
         return [new validationIssue({
             type: type,
             severity: 'warning',
-            message: t('issues.unsquare_way.message', {
-                feature: utilDisplayLabel(entity, context)
-            }),
+            message: function() {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t('issues.unsquare_way.message', { feature: utilDisplayLabel(entity, context) }) : '';
+            },
             reference: showReference,
             entityIds: [entity.id],
             fixes: [