From 857b9c9adff7b83488ee61e3d3ddfea2ec110672 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 2 Aug 2021 14:34:18 -0400 Subject: [PATCH 1/5] Exclude 'fixme'/'help_request' warnings from changeset tags. They still appear in the issue list and in the entity editor. (closes #8603) --- modules/ui/commit.js | 5 ++++- modules/ui/commit_warnings.js | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/ui/commit.js b/modules/ui/commit.js index 5beec1448..b641ea3d8 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -193,7 +193,10 @@ export function uiCommit(context) { // add counts of warnings generated by the user's edits var warnings = context.validator() - .getIssuesBySeverity({ what: 'edited', where: 'all', includeIgnored: true, includeDisabledRules: true }).warning; + .getIssuesBySeverity({ what: 'edited', where: 'all', includeIgnored: true, includeDisabledRules: true }) + .warning + .filter(function(issue) { return issue.type !== 'help_request'; }); // exclude 'fixme' and similar - #8603 + addIssueCounts(warnings, 'warnings'); // add counts of issues resolved by the user's edits diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 5c968e421..e02656c2c 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -12,6 +12,11 @@ export function uiCommitWarnings(context) { for (var severity in issuesBySeverity) { var issues = issuesBySeverity[severity]; + + if (severity !== 'error') { // exclude 'fixme' and similar - #8603 + issues = issues.filter(function(issue) { return issue.type !== 'help_request'; }); + } + var section = severity + '-section'; var issueItem = severity + '-item'; From 0085c41876095652184df4267505ddea62e83cdb Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 2 Aug 2021 17:15:25 -0400 Subject: [PATCH 2/5] Store whether a result is provisional before filtering it Filtering returns a new array, which was clobbering the "provisional" flag. This was causing provisionally results to not be reprocessed later, which meant that certain "outdated_tags" results would not be in the baseCache. (cache of issues _before_ user edits). --- modules/core/validator.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/core/validator.js b/modules/core/validator.js index 15e059dbb..4f328fd1b 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -547,10 +547,11 @@ export function coreValidator(context) { return; } - const detected = fn(entity, graph).filter(applySeverityOverrides); + let detected = fn(entity, graph); if (detected.provisional) { // this validation should be run again later result.provisional = true; } + detected = detected.filter(applySeverityOverrides); result.issues = result.issues.concat(detected); } From 9f58f1fb5c7239789ca3500dba6473e5b8237f87 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 5 Aug 2021 12:45:39 -0400 Subject: [PATCH 3/5] Improve code for focusing a validation issue on a relation The "center" of the issue might be a spot of map that doesn't contain the relation. This code chooses a piece of the relation that has been downloaded and focuses on that. --- modules/core/validator.js | 46 ++++++++++++++++++++++++++++++++------- modules/util/index.js | 3 ++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/modules/core/validator.js b/modules/core/validator.js index 4f328fd1b..114fb971a 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -4,7 +4,7 @@ import { prefs } from './preferences'; import { coreDifference } from './difference'; import { geoExtent } from '../geo/extent'; import { modeSelect } from '../modes/select'; -import { utilArrayChunk, utilArrayGroupBy, utilRebind } from '../util'; +import { utilArrayChunk, utilArrayGroupBy, utilEntityAndDeepMemberIDs, utilRebind } from '../util'; import * as Validations from '../validations/index'; @@ -262,17 +262,47 @@ export function coreValidator(context) { // `issue` - the issue to focus on // validator.focusIssue = (issue) => { - const extent = issue.extent(context.graph()); - if (!extent) return; + const graph = context.graph(); + let selectID; + let focusCenter; - const setZoom = Math.max(context.map().zoom(), 19); - context.map().unobscuredCenterZoomEase(extent.center(), setZoom); + // Try to focus the map at the center of the issue.. + const issueExtent = issue.extent(graph); + if (issueExtent) { + focusCenter = issueExtent.center(); + } - // select the first entity + // Try to select the first entity in the issue.. if (issue.entityIds && issue.entityIds.length) { + selectID = issue.entityIds[0]; + + // If a relation, focus on one of its members instead. + // Otherwise we might be focusing on a part of map where the relation is not visible. + if (selectID && selectID.charAt(0) === 'r') { // relation + const ids = utilEntityAndDeepMemberIDs([selectID], graph); + let nodeID = ids.find(id => id.charAt(0) === 'n' && graph.hasEntity(id)); + + if (!nodeID) { // relation has no downloaded nodes to focus on + const wayID = ids.find(id => id.charAt(0) === 'w' && graph.hasEntity(id)); + if (wayID) { + nodeID = graph.entity(wayID).first(); // focus on the first node of this way + } + } + + if (nodeID) { + focusCenter = graph.entity(nodeID).loc; + } + } + } + + if (focusCenter) { // Adjust the view + const setZoom = Math.max(context.map().zoom(), 19); + context.map().unobscuredCenterZoomEase(focusCenter, setZoom); + } + + if (selectID) { // Enter select mode window.setTimeout(() => { - let ids = issue.entityIds; - context.enter(modeSelect(context, [ids[0]])); + context.enter(modeSelect(context, [selectID])); dispatch.call('focusedIssue', this, issue); }, 250); // after ease } diff --git a/modules/util/index.js b/modules/util/index.js index 683c476c2..3c0836aed 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -22,9 +22,10 @@ export { utilDisplayType } from './util'; export { utilDisplayLabel } from './util'; export { utilEntityRoot } from './util'; export { utilEditDistance } from './util'; -export { utilEntitySelector } from './util'; +export { utilEntityAndDeepMemberIDs } from './util'; export { utilEntityOrMemberSelector } from './util'; export { utilEntityOrDeepMemberSelector } from './util'; +export { utilEntitySelector } from './util'; export { utilFastMouse } from './util'; export { utilFetchJson } from './util'; export { utilFunctor } from './util'; From 3b0a8504007928643e0e15eab318c84e1a52bce3 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 5 Aug 2021 12:47:52 -0400 Subject: [PATCH 4/5] If undo'd back to the base graph, don't show head issues as user issues --- modules/core/validator.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/modules/core/validator.js b/modules/core/validator.js index 114fb971a..a95d5faf7 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -176,14 +176,14 @@ export function coreValidator(context) { validator.getIssues = (options) => { const opts = Object.assign({ what: 'all', where: 'all', includeIgnored: false, includeDisabledRules: false }, options); const view = context.map().extent(); + const baseGraph = context.history().base(); let issues = []; let seen = new Set(); // collect head issues - caused by user edits - let cache = _headCache; - if (_headGraph) { - Object.values(cache.issuesByIssueID).forEach(issue => { - if (!filter(issue, _headGraph, cache)) return; + if (_headGraph && _headGraph !== baseGraph) { + Object.values(_headCache.issuesByIssueID).forEach(issue => { + if (!filter(issue, _headGraph, _headCache)) return; seen.add(issue.id); issues.push(issue); }); @@ -191,9 +191,8 @@ export function coreValidator(context) { // collect base issues - not caused by user edits if (opts.what === 'all') { - cache = _baseCache; - Object.values(cache.issuesByIssueID).forEach(issue => { - if (!filter(issue, context.history().base(), cache)) return; + Object.values(_baseCache.issuesByIssueID).forEach(issue => { + if (!filter(issue, baseGraph, _baseCache)) return; seen.add(issue.id); issues.push(issue); }); From a46a345647920c51a9953e193ff0d74e7f8e71b9 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 5 Aug 2021 14:49:14 -0400 Subject: [PATCH 5/5] Only expand a validation set to include parent multipolygon relations The previous code was grabbing _all_ parent relations, which is too much. For example: if a user changed a road, the validator was treating it like the user had changed bus and highway routes along that road. (closes #8613) (helps a lot #8612) --- modules/core/validator.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/modules/core/validator.js b/modules/core/validator.js index a95d5faf7..b2cd289da 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -650,7 +650,7 @@ export function coreValidator(context) { checkParentRels.push(parentWay); }); - } else if (entity.type === 'relation') { + } else if (entity.type === 'relation' && entity.isMultipolygon()) { entity.members.forEach(member => collected.add(member.id)); // collect members } else if (entity.type === 'way') { @@ -662,7 +662,11 @@ export function coreValidator(context) { checkParentRels.forEach(entity => { // collect parent relations if (entity.type !== 'relation') { // but not super-relations - graph.parentRelations(entity).forEach(parentRelation => collected.add(parentRelation.id)); + graph.parentRelations(entity).forEach(parentRelation => { + if (parentRelation.isMultipolygon()) { + collected.add(parentRelation.id); + } + }); } }); });