From 33ae009936132dca2381175eb6ef0f610b9c0c11 Mon Sep 17 00:00:00 2001
From: Quincy Morgan <quincy.morgan@icloud.com>
Date: Thu, 24 Jan 2019 11:01:41 -0500
Subject: [PATCH] Clarify wording of old_multipolygon issue and tooltip Add
 quick fix for old_multipolygon issue

---
 data/core.yaml                          |  7 +++--
 dist/locales/en.json                    |  8 ++++--
 modules/ui/entity_issues.js             |  5 +++-
 modules/ui/issues.js                    |  5 +++-
 modules/validations/old_multipolygon.js | 34 +++++++++++++++++++++----
 5 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/data/core.yaml b/data/core.yaml
index c14db4350..f1e9ffb0f 100644
--- a/data/core.yaml
+++ b/data/core.yaml
@@ -1152,8 +1152,8 @@ en:
       message: "{highway} is disconnected from other highways."
       tooltip: "Roads should be connected to other roads or building entrances."
     old_multipolygon:
-      message: Multipolygon tags on outer way
-      tooltip: "This style of multipolygon is deprecated. Please assign the tags to the parent multipolygon instead of the outer way."
+      message: "{multipolygon} has misplaced tags."
+      tooltip: "Multipolygons should be tagged on their relation, not their outer way."
     untagged_feature:
       message: "{feature} has no tags."
       tooltip: "Select a feature type that describes what this is."
@@ -1221,6 +1221,9 @@ en:
       remove_name:
         title: Remove the name
         undo_redo: Removed a generic name.
+      move_tags:
+        title: Move the tags
+        undo_redo: Moved tags.
   intro:
     done: done
     ok: OK
diff --git a/dist/locales/en.json b/dist/locales/en.json
index 4f4d1712b..2a90d352b 100644
--- a/dist/locales/en.json
+++ b/dist/locales/en.json
@@ -1397,8 +1397,8 @@
                 "tooltip": "Roads should be connected to other roads or building entrances."
             },
             "old_multipolygon": {
-                "message": "Multipolygon tags on outer way",
-                "tooltip": "This style of multipolygon is deprecated. Please assign the tags to the parent multipolygon instead of the outer way."
+                "message": "{multipolygon} has misplaced tags.",
+                "tooltip": "Multipolygons should be tagged on their relation, not their outer way."
             },
             "untagged_feature": {
                 "message": "{feature} has no tags.",
@@ -1490,6 +1490,10 @@
                 "remove_name": {
                     "title": "Remove the name",
                     "undo_redo": "Removed a generic name."
+                },
+                "move_tags": {
+                    "title": "Move the tags",
+                    "undo_redo": "Moved tags."
                 }
             }
         },
diff --git a/modules/ui/entity_issues.js b/modules/ui/entity_issues.js
index c9e0999d9..77dffd672 100644
--- a/modules/ui/entity_issues.js
+++ b/modules/ui/entity_issues.js
@@ -101,9 +101,12 @@ export function uiEntityIssues(context) {
 
                 issue.select('.label')
                     .on('click', function() {
-                        context.map().centerZoomEase(d.loc(), Math.max(context.map().zoom(), 18));
                         if (list.style('display') === 'none') {
                             list.style('display', 'block');
+                            var loc = d.loc();
+                            if (loc) {
+                                context.map().centerZoomEase(loc, Math.max(context.map().zoom(), 18));
+                            }
                         } else {
                             list.style('display', 'none');
                         }
diff --git a/modules/ui/issues.js b/modules/ui/issues.js
index b4baa1078..df3f4c303 100644
--- a/modules/ui/issues.js
+++ b/modules/ui/issues.js
@@ -130,7 +130,10 @@ export function uiIssues(context) {
                 .placement('bottom')
             )
             .on('click', function(d) {
-                context.map().centerZoomEase(d.loc(), Math.max(context.map().zoom(), 18));
+                var loc = d.loc();
+                if (loc) {
+                    context.map().centerZoomEase(loc, Math.max(context.map().zoom(), 18));
+                }
                 if (d.entities) {
                     context.enter(modeSelect(
                         context,
diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js
index 0681ab4f9..6305a9a90 100644
--- a/modules/validations/old_multipolygon.js
+++ b/modules/validations/old_multipolygon.js
@@ -1,25 +1,49 @@
 import { t } from '../util/locale';
 import { osmIsSimpleMultipolygonOuterMember } from '../osm';
+import { utilDisplayLabel } from '../util';
 import {
     ValidationIssueType,
     ValidationIssueSeverity,
     validationIssue,
+    validationIssueFix
 } from './validation_issue';
+import {
+    actionChangeTags
+} from '../actions';
 
-export function validationOldMultipolygon() {
+export function validationOldMultipolygon(context) {
 
     return function validation(changes, graph) {
         var issues = [];
         for (var i = 0; i < changes.created.length; i++) {
             var entity = changes.created[i];
-            var parent = osmIsSimpleMultipolygonOuterMember(entity, graph);
-            if (parent) {
+            var mistaggedMultipolygon = osmIsSimpleMultipolygonOuterMember(entity, graph);
+            if (mistaggedMultipolygon) {
+                var multipolygonLabel = utilDisplayLabel(mistaggedMultipolygon, context);
                 issues.push(new validationIssue({
                     type: ValidationIssueType.old_multipolygon,
                     severity: ValidationIssueSeverity.warning,
-                    message: t('issues.old_multipolygon.message'),
+                    message: t('issues.old_multipolygon.message', {multipolygon: multipolygonLabel}),
                     tooltip: t('issues.old_multipolygon.tooltip'),
-                    entities: [parent],
+                    entities: [entity, mistaggedMultipolygon],
+                    fixes: [
+                        new validationIssueFix({
+                            title: t('issues.fix.move_tags.title'),
+                            action: function() {
+                                var outerWay = this.issue.entities[0];
+                                var multipolygon =  this.issue.entities[1];
+                                context.perform(
+                                    function(graph) {
+                                        multipolygon = multipolygon.mergeTags(outerWay.tags);
+                                        graph = graph.replace(multipolygon);
+                                        graph = actionChangeTags(outerWay.id, {})(graph);
+                                        return graph;
+                                    },
+                                    t('issues.fix.move_tags.undo_redo')
+                                );
+                            }
+                        })
+                    ]
                 }));
             }
         }