From e11d97b38cb52fddec470bc68a6c6d58f952770b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 7 Dec 2019 15:35:51 +0000 Subject: [PATCH 01/38] Add Osmose QA layer and service Initial implementation - need to add UI for the errors and correctly set up support for the desired error types provided by osmose. --- css/65_data.css | 10 +- data/core.yaml | 7 +- dist/locales/en.json | 6 +- modules/modes/select_error.js | 13 +- modules/services/index.js | 5 +- modules/services/osmose.js | 238 +++++++++++++++++++++++++++++++ modules/svg/layers.js | 4 +- modules/svg/osmose.js | 261 ++++++++++++++++++++++++++++++++++ modules/ui/commit.js | 8 +- modules/ui/map_data.js | 4 +- test/spec/svg/layers.js | 21 +-- 11 files changed, 556 insertions(+), 21 deletions(-) create mode 100644 modules/services/osmose.js create mode 100644 modules/svg/osmose.js diff --git a/css/65_data.css b/css/65_data.css index 8de9ca09cb..837cc9688a 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -3,7 +3,8 @@ .error-header-icon .qa_error-fill, .layer-keepRight .qa_error .qa_error-fill, -.layer-improveOSM .qa_error .qa_error-fill { +.layer-improveOSM .qa_error .qa_error-fill, +.layer-osmose .qa_error .qa_error-fill { stroke: #333; stroke-width: 1.3px; /* NOTE: likely a better way to scale the icon stroke */ } @@ -152,6 +153,11 @@ color: #EC1C24; } +/* Osmose Errors +------------------------------------------------------- */ +.osmose { + color: #FFFFFF; +} /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { @@ -211,4 +217,4 @@ stroke: #000; stroke-width: 5px; stroke-miterlimit: 1; -} +} \ No newline at end of file diff --git a/data/core.yaml b/data/core.yaml index d68008f9af..e030339e9a 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -630,6 +630,9 @@ en: improveOSM: tooltip: Missing data automatically detected by improveosm.org title: ImproveOSM Issues + osmose: + tooltip: Automatically detected map issues from osmose.openstreetmap.fr + title: Osmose Issues custom: tooltip: "Drag and drop a data file onto the page, or click the button to setup" title: Custom Map Data @@ -1343,7 +1346,7 @@ en: title: Quality Assurance intro: "*Quality Assurance* (Q/A) tools can find improper tags, disconnected roads, and other issues with OpenStreetMap, which mappers can then fix. To view existing Q/A issues, click the {data} **Map data** panel to enable a specific Q/A layer." tools_h: "Tools" - tools: "The following tools are currently supported: [KeepRight](https://www.keepright.at/) and [ImproveOSM](https://improveosm.org/en/). Expect iD to support [Osmose](https://osmose.openstreetmap.fr/) and more Q/A tools in the future." + tools: "The following tools are currently supported: [KeepRight](https://www.keepright.at/), [ImproveOSM](https://improveosm.org/en/) and [Osmose](https://osmose.openstreetmap.fr/). Expect iD to support more Q/A tools in the future." issues_h: "Handling Issues" issues: "Handling Q/A issues is similar to handling notes. Click on a marker to view the issue details in the sidebar. Each tool has its own capabilities, but generally you can comment and/or close an issue." field: @@ -2058,4 +2061,4 @@ en: wikidata: identifier: "Identifier" label: "Label" - description: "Description" + description: "Description" \ No newline at end of file diff --git a/dist/locales/en.json b/dist/locales/en.json index 1aa7361e5d..31a21fdba0 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -782,6 +782,10 @@ "tooltip": "Missing data automatically detected by improveosm.org", "title": "ImproveOSM Issues" }, + "osmose": { + "tooltip": "Automatically detected map issues from osmose.openstreetmap.fr", + "title": "Osmose Issues" + }, "custom": { "tooltip": "Drag and drop a data file onto the page, or click the button to setup", "title": "Custom Map Data", @@ -1653,7 +1657,7 @@ "title": "Quality Assurance", "intro": "*Quality Assurance* (Q/A) tools can find improper tags, disconnected roads, and other issues with OpenStreetMap, which mappers can then fix. To view existing Q/A issues, click the {data} **Map data** panel to enable a specific Q/A layer.", "tools_h": "Tools", - "tools": "The following tools are currently supported: [KeepRight](https://www.keepright.at/) and [ImproveOSM](https://improveosm.org/en/). Expect iD to support [Osmose](https://osmose.openstreetmap.fr/) and more Q/A tools in the future.", + "tools": "The following tools are currently supported: [KeepRight](https://www.keepright.at/), [ImproveOSM](https://improveosm.org/en/) and [Osmose](https://osmose.openstreetmap.fr/). Expect iD to support more Q/A tools in the future.", "issues_h": "Handling Issues", "issues": "Handling Q/A issues is similar to handling notes. Click on a marker to view the issue details in the sidebar. Each tool has its own capabilities, but generally you can comment and/or close an issue." }, diff --git a/modules/modes/select_error.js b/modules/modes/select_error.js index 774822b686..0af8b3acb7 100644 --- a/modules/modes/select_error.js +++ b/modules/modes/select_error.js @@ -15,6 +15,7 @@ import { modeDragNode } from './drag_node'; import { modeDragNote } from './drag_note'; import { uiImproveOsmEditor } from '../ui/improveOSM_editor'; import { uiKeepRightEditor } from '../ui/keepRight_editor'; +import { uiOsmoseEditor } from '../ui/osmose_editor'; import { utilKeybinding } from '../util'; @@ -49,6 +50,16 @@ export function modeSelectError(context, selectedErrorID, selectedErrorService) .show(errorEditor.error(error)); }); break; + case 'osmose': + errorEditor = uiOsmoseEditor(context) + .on('change', function() { + context.map().pan([0,0]); // trigger a redraw + var error = checkSelectedID(); + if (!error) return; + context.ui().sidebar + .show(errorEditor.error(error)); + }); + break; } @@ -154,4 +165,4 @@ export function modeSelectError(context, selectedErrorID, selectedErrorService) return mode; -} +} \ No newline at end of file diff --git a/modules/services/index.js b/modules/services/index.js index 838ed435f0..ab9aa55034 100644 --- a/modules/services/index.js +++ b/modules/services/index.js @@ -1,5 +1,6 @@ import serviceKeepRight from './keepRight'; import serviceImproveOSM from './improveOSM'; +import serviceOsmose from './osmose'; import serviceMapillary from './mapillary'; import serviceMapRules from './maprules'; import serviceNominatim from './nominatim'; @@ -17,6 +18,7 @@ export var services = { geocoder: serviceNominatim, keepRight: serviceKeepRight, improveOSM: serviceImproveOSM, + osmose: serviceOsmose, mapillary: serviceMapillary, openstreetcam: serviceOpenstreetcam, osm: serviceOsm, @@ -32,6 +34,7 @@ export var services = { export { serviceKeepRight, serviceImproveOSM, + serviceOsmose, serviceMapillary, serviceMapRules, serviceNominatim, @@ -43,4 +46,4 @@ export { serviceVectorTile, serviceWikidata, serviceWikipedia -}; +}; \ No newline at end of file diff --git a/modules/services/osmose.js b/modules/services/osmose.js new file mode 100644 index 0000000000..36181b5792 --- /dev/null +++ b/modules/services/osmose.js @@ -0,0 +1,238 @@ +import RBush from 'rbush'; + +import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { json as d3_json } from 'd3-fetch'; + +import { geoExtent, geoVecAdd, geoVecScale } from '../geo'; +import { qaError } from '../osm'; +import { t } from '../util/locale'; +import { utilRebind, utilTiler, utilQsString } from '../util'; + + +var tiler = utilTiler(); +var dispatch = d3_dispatch('loaded'); + +var _erCache; +var _erZoom = 14; + +var _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta/'; + +function abortRequest(controller) { + if (controller) { + controller.abort(); + } +} + +function abortUnwantedRequests(cache, tiles) { + Object.keys(cache.inflightTile).forEach(function(k) { + var wanted = tiles.find(function(tile) { return k === tile.id; }); + if (!wanted) { + abortRequest(cache.inflightTile[k]); + delete cache.inflightTile[k]; + } + }); +} + + +function encodeErrorRtree(d) { + return { minX: d.loc[0], minY: d.loc[1], maxX: d.loc[0], maxY: d.loc[1], data: d }; +} + + +// replace or remove error from rtree +function updateRtree(item, replace) { + _erCache.rtree.remove(item, function isEql(a, b) { + return a.data.id === b.data.id; + }); + + if (replace) { + _erCache.rtree.insert(item); + } +} + +function linkErrorObject(d) { + return '' + d + ''; +} + +function linkEntity(d) { + return '' + d + ''; +} + +// Errors shouldn't obscure eachother +function preventCoincident(loc, bumpUp) { + var coincident = false; + do { + // first time, move marker up. after that, move marker right. + var delta = coincident ? [0.00001, 0] : (bumpUp ? [0, 0.00001] : [0, 0]); + loc = geoVecAdd(loc, delta); + var bbox = geoExtent(loc).bbox(); + coincident = _erCache.rtree.search(bbox).length; + } while (coincident); + + return loc; +} + +export default { + init: function() { + if (!_erCache) { + this.reset(); + } + + this.event = utilRebind(this, dispatch, 'on'); + }, + + reset: function() { + if (_erCache) { + Object.values(_erCache.inflightTile).forEach(abortRequest); + } + _erCache = { + data: {}, + loadedTile: {}, + inflightTile: {}, + inflightPost: {}, + closed: {}, + rtree: new RBush() + }; + }, + + loadErrors: function(projection) { + var options = { + full: 'true', // Returns element IDs + level: '1,2,3', + zoom: '19' + }; + + // determine the needed tiles to cover the view + var tiles = tiler + .zoomExtent([_erZoom, _erZoom]) + .getTiles(projection); + + // abort inflight requests that are no longer needed + abortUnwantedRequests(_erCache, tiles); + + // issue new requests.. + tiles.forEach(function(tile) { + if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; + + var rect = tile.extent.rectangle(); // E, N, W, S + var params = Object.assign({}, options, { bbox: [rect[0], rect[1], rect[2], rect[3]].join() }); + + var url = _osmoseUrlRoot + 'issues?' + utilQsString(params); + + var controller = new AbortController(); + _erCache.inflightTile[tile.id] = controller; + + d3_json(url, { signal: controller.signal }) + .then(function(data) { + delete _erCache.inflightTile[tile.id]; + _erCache.loadedTile[tile.id] = true; + + if (data.issues) { + data.issues.forEach(function(issue) { + // Elements provided as string, separated by _ character + var elems = issue.elems.split('_'); + var loc = [issue.lon, issue.lat]; + + loc = preventCoincident(loc, true); + + var d = new qaError({ + // Info required for every error + loc: loc, + service: 'osmose', + error_type: [issue.item, issue.classs].join('-'), + // Extra details needed for this service + identifier: issue.id, // this is used to post changes to the error + elems: elems + //object_id: elems[0], + //object_type: elems[0].substring(0,1) + }); + + // Variables used in the description + d.replacements = { + }; + + _erCache.data[d.id] = d; + _erCache.rtree.insert(encodeErrorRtree(d)); + }); + } + }) + .catch(function() { + delete _erCache.inflightTile[tile.id]; + _erCache.loadedTile[tile.id] = true; + }); + }); + }, + + postUpdate: function(d, callback) { + if (_erCache.inflightPost[d.id]) { + return callback({ message: 'Error update already inflight', status: -2 }, d); + } + + var that = this; + + if (err) { return callback(err, d); } + + // UI sets the status to either '/done' or '/false' + var url = _osmoseUrlRoot + 'issue/' + d.identifier + d.newStatus; + + var controller = new AbortController(); + _erCache.inflightPost[d.id] = controller; + + fetch(url, { method: 'POST', signal: controller.signal }) + .then(function() { + delete _erCache.inflightPost[d.id]; + + that.removeError(d); + if (d.newStatus === '/done') { + // No pretty identifier, so we just use coordinates + var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); + _erCache.closed[key + ':' + closedID] = true; + } + if (callback) callback(null, d); + }) + .catch(function(err) { + delete _erCache.inflightPost[d.id]; + if (callback) callback(err.message); + }); + }, + + + // get all cached errors covering the viewport + getErrors: function(projection) { + var viewport = projection.clipExtent(); + var min = [viewport[0][0], viewport[1][1]]; + var max = [viewport[1][0], viewport[0][1]]; + var bbox = geoExtent(projection.invert(min), projection.invert(max)).bbox(); + + return _erCache.rtree.search(bbox).map(function(d) { + return d.data; + }); + }, + + // get a single error from the cache + getError: function(id) { + return _erCache.data[id]; + }, + + // replace a single error in the cache + replaceError: function(error) { + if (!(error instanceof qaError) || !error.id) return; + + _erCache.data[error.id] = error; + updateRtree(encodeErrorRtree(error), true); // true = replace + return error; + }, + + // remove a single error from the cache + removeError: function(error) { + if (!(error instanceof qaError) || !error.id) return; + + delete _erCache.data[error.id]; + updateRtree(encodeErrorRtree(error), false); // false = remove + }, + + // Used to populate `closed:osmose` changeset tag + getClosedIDs: function() { + return Object.keys(_erCache.closed).sort(); + } +}; \ No newline at end of file diff --git a/modules/svg/layers.js b/modules/svg/layers.js index 12595a7d7b..d0c46df537 100644 --- a/modules/svg/layers.js +++ b/modules/svg/layers.js @@ -6,6 +6,7 @@ import { svgDebug } from './debug'; import { svgGeolocate } from './geolocate'; import { svgKeepRight } from './keepRight'; import { svgImproveOSM } from './improveOSM'; +import { svgOsmose } from './osmose'; import { svgStreetside } from './streetside'; import { svgMapillaryImages } from './mapillary_images'; import { svgMapillarySigns } from './mapillary_signs'; @@ -27,6 +28,7 @@ export function svgLayers(projection, context) { { id: 'data', layer: svgData(projection, context, dispatch) }, { id: 'keepRight', layer: svgKeepRight(projection, context, dispatch) }, { id: 'improveOSM', layer: svgImproveOSM(projection, context, dispatch) }, + { id: 'osmose', layer: svgOsmose(projection, context, dispatch) }, { id: 'streetside', layer: svgStreetside(projection, context, dispatch)}, { id: 'mapillary', layer: svgMapillaryImages(projection, context, dispatch) }, { id: 'mapillary-map-features', layer: svgMapillaryMapFeatures(projection, context, dispatch) }, @@ -116,4 +118,4 @@ export function svgLayers(projection, context) { return utilRebind(drawLayers, dispatch, 'on'); -} +} \ No newline at end of file diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js new file mode 100644 index 0000000000..1126087d68 --- /dev/null +++ b/modules/svg/osmose.js @@ -0,0 +1,261 @@ +import _throttle from 'lodash-es/throttle'; +import { select as d3_select } from 'd3-selection'; + +import { modeBrowse } from '../modes/browse'; +import { svgPointTransform } from './helpers'; +import { services } from '../services'; + +var _osmoseEnabled = false; +var _errorService; + + +export function svgOsmose(projection, context, dispatch) { + var throttledRedraw = _throttle(function () { dispatch.call('change'); }, 1000); + var minZoom = 12; + var touchLayer = d3_select(null); + var drawLayer = d3_select(null); + var _osmoseVisible = false; + + function markerPath(selection, klass) { + selection + .attr('class', klass) + .attr('transform', 'translate(-10, -28)') + .attr('points', '16,3 4,3 1,6 1,17 4,20 7,20 10,27 13,20 16,20 19,17.033 19,6'); + } + + + // Loosely-coupled osmose service for fetching errors. + function getService() { + if (services.osmose && !_errorService) { + _errorService = services.osmose; + _errorService.on('loaded', throttledRedraw); + } else if (!services.osmose && _errorService) { + _errorService = null; + } + + return _errorService; + } + + + // Show the errors + function editOn() { + if (!_osmoseVisible) { + _osmoseVisible = true; + drawLayer + .style('display', 'block'); + } + } + + + // Immediately remove the errors and their touch targets + function editOff() { + if (_osmoseVisible) { + _osmoseVisible = false; + drawLayer + .style('display', 'none'); + drawLayer.selectAll('.qa_error.osmose') + .remove(); + touchLayer.selectAll('.qa_error.osmose') + .remove(); + } + } + + + // Enable the layer. This shows the errors and transitions them to visible. + function layerOn() { + editOn(); + + drawLayer + .style('opacity', 0) + .transition() + .duration(250) + .style('opacity', 1) + .on('end interrupt', function () { + dispatch.call('change'); + }); + } + + + // Disable the layer. This transitions the layer invisible and then hides the errors. + function layerOff() { + throttledRedraw.cancel(); + drawLayer.interrupt(); + touchLayer.selectAll('.qa_error.osmose') + .remove(); + + drawLayer + .transition() + .duration(250) + .style('opacity', 0) + .on('end interrupt', function () { + editOff(); + dispatch.call('change'); + }); + } + + + // Update the error markers + function updateMarkers() { + if (!_osmoseVisible || !_osmoseEnabled) return; + + var service = getService(); + var selectedID = context.selectedErrorID(); + var data = (service ? service.getErrors(projection) : []); + var getTransform = svgPointTransform(projection); + + // Draw markers.. + var markers = drawLayer.selectAll('.qa_error.osmose') + .data(data, function(d) { return d.id; }); + + // exit + markers.exit() + .remove(); + + // enter + var markersEnter = markers.enter() + .append('g') + .attr('class', function(d) { + return [ + 'qa_error', + d.service, + 'error_id-' + d.id, + 'error_type-' + d.error_type, + 'category-' + d.category + ].join(' '); + }); + + markersEnter + .append('polygon') + .call(markerPath, 'shadow'); + + markersEnter + .append('ellipse') + .attr('cx', 0) + .attr('cy', 0) + .attr('rx', 4.5) + .attr('ry', 2) + .attr('class', 'stroke'); + + markersEnter + .append('polygon') + .attr('fill', 'currentColor') + .call(markerPath, 'qa_error-fill'); + + markersEnter + .append('use') + .attr('transform', 'translate(-5.5, -21)') + .attr('class', 'icon-annotation') + .attr('width', '11px') + .attr('height', '11px') + .attr('xlink:href', function(d) { + var picon = d.icon; + + if (!picon) { + return ''; + } else { + var isMaki = /^maki-/.test(picon); + return '#' + picon + (isMaki ? '-11' : ''); + } + }); + + // update + markers + .merge(markersEnter) + .sort(sortY) + .classed('selected', function(d) { return d.id === selectedID; }) + .attr('transform', getTransform); + + + // Draw targets.. + if (touchLayer.empty()) return; + var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor '; + + var targets = touchLayer.selectAll('.qa_error.osmose') + .data(data, function(d) { return d.id; }); + + // exit + targets.exit() + .remove(); + + // enter/update + targets.enter() + .append('rect') + .attr('width', '20px') + .attr('height', '30px') + .attr('x', '-10px') + .attr('y', '-28px') + .merge(targets) + .sort(sortY) + .attr('class', function(d) { + return 'qa_error ' + d.service + ' target error_id-' + d.id + ' ' + fillClass; + }) + .attr('transform', getTransform); + + + function sortY(a, b) { + return (a.id === selectedID) ? 1 + : (b.id === selectedID) ? -1 + : b.loc[1] - a.loc[1]; + } + } + + + // Draw the Osmose layer and schedule loading errors and updating markers. + function drawOsmose(selection) { + var service = getService(); + + var surface = context.surface(); + if (surface && !surface.empty()) { + touchLayer = surface.selectAll('.data-layer.touch .layer-touch.markers'); + } + + drawLayer = selection.selectAll('.layer-osmose') + .data(service ? [0] : []); + + drawLayer.exit() + .remove(); + + drawLayer = drawLayer.enter() + .append('g') + .attr('class', 'layer-osmose') + .style('display', _osmoseEnabled ? 'block' : 'none') + .merge(drawLayer); + + if (_osmoseEnabled) { + if (service && ~~context.map().zoom() >= minZoom) { + editOn(); + service.loadErrors(projection); + updateMarkers(); + } else { + editOff(); + } + } + } + + + // Toggles the layer on and off + drawOsmose.enabled = function(val) { + if (!arguments.length) return _osmoseEnabled; + + _osmoseEnabled = val; + if (_osmoseEnabled) { + layerOn(); + } else { + layerOff(); + if (context.selectedErrorID()) { + context.enter(modeBrowse(context)); + } + } + + dispatch.call('change'); + return this; + }; + + + drawOsmose.supported = function() { + return !!getService(); + }; + + + return drawOsmose; +} \ No newline at end of file diff --git a/modules/ui/commit.js b/modules/ui/commit.js index 3713705df7..6f5e293d8f 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -149,6 +149,12 @@ export function uiCommit(context) { tags['closed:improveosm'] = iOsmClosed.join(';').substr(0, tagCharLimit); } } + if (services.osmose) { + var osmoseClosed = services.osmose.getClosedIDs(); + if (osmoseClosed.length) { + tags['closed:osmose'] = osmoseClosed.join(';').substr(0, 255); + } + } // remove existing issue counts for (var key in tags) { @@ -585,4 +591,4 @@ export function uiCommit(context) { return utilRebind(commit, dispatch, 'on'); -} +} \ No newline at end of file diff --git a/modules/ui/map_data.js b/modules/ui/map_data.js index 2f1fa57bdf..d6362a5182 100644 --- a/modules/ui/map_data.js +++ b/modules/ui/map_data.js @@ -341,7 +341,7 @@ export function uiMapData(context) { function drawQAItems(selection) { - var qaKeys = ['keepRight', 'improveOSM']; + var qaKeys = ['keepRight', 'improveOSM', 'osmose']; var qaLayers = layers.all().filter(function(obj) { return qaKeys.indexOf(obj.id) !== -1; }); var ul = selection @@ -916,4 +916,4 @@ export function uiMapData(context) { }; return uiMapData; -} +} \ No newline at end of file diff --git a/test/spec/svg/layers.js b/test/spec/svg/layers.js index 09b2b85ce7..4a24eaacbe 100644 --- a/test/spec/svg/layers.js +++ b/test/spec/svg/layers.js @@ -26,20 +26,21 @@ describe('iD.svgLayers', function () { it('creates default data layers', function () { container.call(iD.svgLayers(projection, context)); var nodes = container.selectAll('svg .data-layer').nodes(); - expect(nodes.length).to.eql(13); + expect(nodes.length).to.eql(14); expect(d3.select(nodes[0]).classed('osm')).to.be.true; expect(d3.select(nodes[1]).classed('notes')).to.be.true; expect(d3.select(nodes[2]).classed('data')).to.be.true; expect(d3.select(nodes[3]).classed('keepRight')).to.be.true; expect(d3.select(nodes[4]).classed('improveOSM')).to.be.true; - expect(d3.select(nodes[5]).classed('streetside')).to.be.true; - expect(d3.select(nodes[6]).classed('mapillary')).to.be.true; - expect(d3.select(nodes[7]).classed('mapillary-map-features')).to.be.true; - expect(d3.select(nodes[8]).classed('mapillary-signs')).to.be.true; - expect(d3.select(nodes[9]).classed('openstreetcam')).to.be.true; - expect(d3.select(nodes[10]).classed('debug')).to.be.true; - expect(d3.select(nodes[11]).classed('geolocate')).to.be.true; - expect(d3.select(nodes[12]).classed('touch')).to.be.true; + expect(d3.select(nodes[5]).classed('osmose')).to.be.true; + expect(d3.select(nodes[6]).classed('streetside')).to.be.true; + expect(d3.select(nodes[7]).classed('mapillary')).to.be.true; + expect(d3.select(nodes[8]).classed('mapillary-map-features')).to.be.true; + expect(d3.select(nodes[9]).classed('mapillary-signs')).to.be.true; + expect(d3.select(nodes[10]).classed('openstreetcam')).to.be.true; + expect(d3.select(nodes[11]).classed('debug')).to.be.true; + expect(d3.select(nodes[12]).classed('geolocate')).to.be.true; + expect(d3.select(nodes[13]).classed('touch')).to.be.true; }); -}); +}); \ No newline at end of file From 09e7b236658e08fde82c0b1a345cb4426ca57edb Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 7 Dec 2019 19:07:53 +0000 Subject: [PATCH 02/38] Add Osmose issues UI and filtering Filters out errors not present in the data .json file to enable selective support since Osmose has a wide variety of errors which may be too advanced for iD. Also added processing for the elements associated with an error for forced visibility and highlighting. --- css/65_data.css | 4 + data/core.yaml | 6 ++ data/qa_errors.json | 8 ++ dist/locales/en.json | 9 ++ modules/services/osmose.js | 54 ++++++----- modules/ui/osmose_details.js | 135 ++++++++++++++++++++++++++++ modules/ui/osmose_editor.js | 170 +++++++++++++++++++++++++++++++++++ modules/ui/osmose_header.js | 97 ++++++++++++++++++++ modules/ui/sidebar.js | 15 +++- 9 files changed, 472 insertions(+), 26 deletions(-) create mode 100644 modules/ui/osmose_details.js create mode 100644 modules/ui/osmose_editor.js create mode 100644 modules/ui/osmose_header.js diff --git a/css/65_data.css b/css/65_data.css index 837cc9688a..39c1c1603d 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -159,6 +159,10 @@ color: #FFFFFF; } +.osmose.category-structure { + color: #F9C700; +} + /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { pointer-events: none; diff --git a/data/core.yaml b/data/core.yaml index e030339e9a..b5b4c0af93 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -827,6 +827,12 @@ en: cannot_zoom: "Cannot zoom out further in current mode." full_screen: Toggle Full Screen QA: + osmose: + title: Osmose + error_types: + 1070-1: + title: 'Highway intersecting building' + description: '{1} intersects with {0}.' improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/data/qa_errors.json b/data/qa_errors.json index d14306dcdc..aad4b3328a 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -32,6 +32,14 @@ "errorTypes": { } + }, + "osmose": { + "errorTypes": { + "1070-1": { + "icon": "maki-home", + "category": "structure" + } + } } } } \ No newline at end of file diff --git a/dist/locales/en.json b/dist/locales/en.json index 31a21fdba0..a38ff70e6a 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1031,6 +1031,15 @@ "cannot_zoom": "Cannot zoom out further in current mode.", "full_screen": "Toggle Full Screen", "QA": { + "osmose": { + "title": "Osmose", + "error_types": { + "1070-1": { + "title": "Highway intersecting building", + "description": "{1} intersects with {0}." + } + } + }, "improveOSM": { "title": "ImproveOSM Detection", "geometry_types": { diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 36181b5792..91aa5c2bdf 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -7,7 +7,7 @@ import { geoExtent, geoVecAdd, geoVecScale } from '../geo'; import { qaError } from '../osm'; import { t } from '../util/locale'; import { utilRebind, utilTiler, utilQsString } from '../util'; - +import { services } from '../../data/qa_errors.json'; var tiler = utilTiler(); var dispatch = d3_dispatch('loaded'); @@ -130,29 +130,37 @@ export default { if (data.issues) { data.issues.forEach(function(issue) { // Elements provided as string, separated by _ character - var elems = issue.elems.split('_'); + var elems = issue.elems.split('_').map(function(i) { + return i.substring(0,1) + i.replace(/node|way|relation/, '') + }); var loc = [issue.lon, issue.lat]; - - loc = preventCoincident(loc, true); - - var d = new qaError({ - // Info required for every error - loc: loc, - service: 'osmose', - error_type: [issue.item, issue.classs].join('-'), - // Extra details needed for this service - identifier: issue.id, // this is used to post changes to the error - elems: elems - //object_id: elems[0], - //object_type: elems[0].substring(0,1) - }); - - // Variables used in the description - d.replacements = { - }; - - _erCache.data[d.id] = d; - _erCache.rtree.insert(encodeErrorRtree(d)); + // Item is the type of error, w/ class tells us the sub-type + var type = [issue.item, issue.classs].join('-'); + + // Filter out unsupported error types (some are too specific or advanced) + if (services.osmose.errorTypes[type]) { + loc = preventCoincident(loc, true); + + var d = new qaError({ + // Info required for every error + loc: loc, + service: 'osmose', + error_type: type, + // Extra details needed for this service + identifier: issue.id, // this is used to post changes to the error + elems: elems, + object_id: elems.length ? elems[0].substring(1) : '', + object_type: elems.length ? elems[0].substring(0,1) : '' + }); + + // Variables used in the description + d.replacements = elems.map(function(i) { + return linkEntity(i) + }); + + _erCache.data[d.id] = d; + _erCache.rtree.insert(encodeErrorRtree(d)); + } }); } }) diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js new file mode 100644 index 0000000000..9c8dbb8749 --- /dev/null +++ b/modules/ui/osmose_details.js @@ -0,0 +1,135 @@ +import { + event as d3_event, + select as d3_select +} from 'd3-selection'; + +import { dataEn } from '../../data'; +import { modeSelect } from '../modes/select'; +import { t } from '../util/locale'; +import { utilDisplayName, utilEntityOrMemberSelector, utilEntityRoot } from '../util'; + + +export function uiOsmoseDetails(context) { + var _error; + + + function errorDetail(d) { + var unknown = t('inspector.unknown'); + + if (!d) return unknown; + + if (d.desc) return d.desc; + + var errorType = d.error_type; + var et = dataEn.QA.osmose.error_types[errorType]; + + var detail; + if (et && et.description) { + detail = t('QA.osmose.error_types.' + errorType + '.description', d.replacements); + } else { + detail = unknown; + } + + return detail; + } + + + function osmoseDetails(selection) { + var details = selection.selectAll('.error-details') + .data( + (_error ? [_error] : []), + function(d) { return d.id + '-' + (d.status || 0); } + ); + + details.exit() + .remove(); + + var detailsEnter = details.enter() + .append('div') + .attr('class', 'error-details error-details-container'); + + + // description + var descriptionEnter = detailsEnter + .append('div') + .attr('class', 'error-details-description'); + + descriptionEnter + .append('h4') + .text(function() { return t('QA.keepRight.detail_description'); }); + + descriptionEnter + .append('div') + .attr('class', 'error-details-description-text') + .html(errorDetail); + + // If there are entity links in the error message.. + var relatedEntities = _error.elems; + descriptionEnter.selectAll('.error_entity_link, .error_object_link') + .each(function() { + var link = d3_select(this); + var isObjectLink = link.classed('error_object_link'); + var entityID = isObjectLink ? + (_error.object_type + _error.object_id) + : this.textContent; + var entity = context.hasEntity(entityID); + + // Add click handler + link + .on('mouseenter', function() { + context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) + .classed('hover', true); + }) + .on('mouseleave', function() { + context.surface().selectAll('.hover') + .classed('hover', false); + }) + .on('click', function() { + d3_event.preventDefault(); + var osmlayer = context.layers().layer('osm'); + if (!osmlayer.enabled()) { + osmlayer.enabled(true); + } + + context.map().centerZoom(_error.loc, 20); + + if (entity) { + context.enter(modeSelect(context, [entityID])); + } else { + context.loadEntity(entityID, function() { + context.enter(modeSelect(context, [entityID])); + }); + } + }); + + // Replace with friendly name if possible + // (The entity may not yet be loaded into the graph) + if (entity) { + var name = utilDisplayName(entity); // try to use common name + + if (!name && !isObjectLink) { + var preset = context.presets().match(entity, context.graph()); + name = preset && !preset.isFallback() && preset.name(); // fallback to preset name + } + + if (name) { + this.innerText = name; + } + } + }); + + // Don't hide entities related to this error - #5880 + context.features().forceVisible(relatedEntities); + context.map().pan([0,0]); // trigger a redraw + } + + + osmoseDetails.error = function(val) { + if (!arguments.length) return _error; + _error = val; + return osmoseDetails; + }; + + + return osmoseDetails; +} \ No newline at end of file diff --git a/modules/ui/osmose_editor.js b/modules/ui/osmose_editor.js new file mode 100644 index 0000000000..102144dbf9 --- /dev/null +++ b/modules/ui/osmose_editor.js @@ -0,0 +1,170 @@ +import { dispatch as d3_dispatch } from 'd3-dispatch'; + +import { t } from '../util/locale'; +import { services } from '../services'; +import { modeBrowse } from '../modes/browse'; +import { svgIcon } from '../svg/icon'; + +import { uiOsmoseDetails } from './osmose_details'; +import { uiOsmoseHeader } from './osmose_header'; +import { uiQuickLinks } from './quick_links'; +import { uiTooltipHtml } from './tooltipHtml'; + +import { utilRebind } from '../util'; + + +export function uiOsmoseEditor(context) { + var dispatch = d3_dispatch('change'); + var errorDetails = uiOsmoseDetails(context); + var errorHeader = uiOsmoseHeader(context); + var quickLinks = uiQuickLinks(); + + var _error; + + + function osmoseEditor(selection) { + // quick links + var choices = [{ + id: 'zoom_to', + label: 'inspector.zoom_to.title', + tooltip: function() { + return uiTooltipHtml(t('inspector.zoom_to.tooltip_issue'), t('inspector.zoom_to.key')); + }, + click: function zoomTo() { + context.mode().zoomToSelected(); + } + }]; + + + var header = selection.selectAll('.header') + .data([0]); + + var headerEnter = header.enter() + .append('div') + .attr('class', 'header fillL'); + + headerEnter + .append('button') + .attr('class', 'fr error-editor-close') + .on('click', function() { + context.enter(modeBrowse(context)); + }) + .call(svgIcon('#iD-icon-close')); + + headerEnter + .append('h3') + .text(t('QA.osmose.title')); + + + var body = selection.selectAll('.body') + .data([0]); + + body = body.enter() + .append('div') + .attr('class', 'body') + .merge(body); + + var editor = body.selectAll('.error-editor') + .data([0]); + + editor.enter() + .append('div') + .attr('class', 'modal-section error-editor') + .merge(editor) + .call(errorHeader.error(_error)) + .call(quickLinks.choices(choices)) + .call(errorDetails.error(_error)) + .call(osmoseSaveSection); + } + + function osmoseSaveSection(selection) { + var isSelected = (_error && _error.id === context.selectedErrorID()); + var isShown = (_error && isSelected); + var saveSection = selection.selectAll('.error-save') + .data( + (isShown ? [_error] : []), + function(d) { return d.id + '-' + (d.status || 0); } + ); + + // exit + saveSection.exit() + .remove(); + + // enter + var saveSectionEnter = saveSection.enter() + .append('div') + .attr('class', 'error-save save-section cf'); + + // update + saveSection = saveSectionEnter + .merge(saveSection) + .call(errorSaveButtons); + } + + function errorSaveButtons(selection) { + var isSelected = (_error && _error.id === context.selectedErrorID()); + var buttonSection = selection.selectAll('.buttons') + .data((isSelected ? [_error] : []), function(d) { return d.status + d.id; }); + + // exit + buttonSection.exit() + .remove(); + + // enter + var buttonEnter = buttonSection.enter() + .append('div') + .attr('class', 'buttons'); + + buttonEnter + .append('button') + .attr('class', 'button close-button action'); + + buttonEnter + .append('button') + .attr('class', 'button ignore-button action'); + + + // update + buttonSection = buttonSection + .merge(buttonEnter); + + buttonSection.select('.close-button') + .text(function(d) { + return t('QA.keepRight.close'); + }) + .on('click.close', function(d) { + this.blur(); // avoid keeping focus on the button - #4641 + var errorService = services.osmose; + if (errorService) { + d.newStatus = '/done'; + errorService.postUpdate(d, function(err, error) { + dispatch.call('change', error); + }); + } + }); + + buttonSection.select('.ignore-button') + .text(function(d) { + return t('QA.keepRight.ignore'); + }) + .on('click.ignore', function(d) { + this.blur(); // avoid keeping focus on the button - #4641 + var errorService = services.osmose; + if (errorService) { + d.newStatus = '/false'; + errorService.postUpdate(d, function(err, error) { + dispatch.call('change', error); + }); + } + }); + } + + osmoseEditor.error = function(val) { + if (!arguments.length) return _error; + _error = val; + return osmoseEditor; + }; + + + return utilRebind(osmoseEditor, dispatch, 'on'); +} \ No newline at end of file diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js new file mode 100644 index 0000000000..21995cbcb7 --- /dev/null +++ b/modules/ui/osmose_header.js @@ -0,0 +1,97 @@ +import { dataEn } from '../../data'; +import { t } from '../util/locale'; + + +export function uiOsmoseHeader() { + var _error; + + + function errorTitle(d) { + var unknown = t('inspector.unknown'); + + if (!d) return unknown; + var errorType = d.error_type; + var et = dataEn.QA.osmose.error_types[errorType]; + + if (et && et.title) { + return t('QA.osmose.error_types.' + errorType + '.title'); + } else { + return unknown; + } + } + + + function osmoseHeader(selection) { + var header = selection.selectAll('.error-header') + .data( + (_error ? [_error] : []), + function(d) { return d.id + '-' + (d.status || 0); } + ); + + header.exit() + .remove(); + + var headerEnter = header.enter() + .append('div') + .attr('class', 'error-header'); + + var iconEnter = headerEnter + .append('div') + .attr('class', 'error-header-icon') + .classed('new', function(d) { return d.id < 0; }); + + var svgEnter = iconEnter + .append('svg') + .attr('width', '20px') + .attr('height', '30px') + .attr('viewbox', '0 0 20 30') + .attr('class', function(d) { + return [ + 'preset-icon-28', + 'qa_error', + d.service, + 'error_id-' + d.id, + 'error_type-' + d.error_type, + 'category-' + d.category + ].join(' '); + }); + + svgEnter + .append('polygon') + .attr('fill', 'currentColor') + .attr('class', 'qa_error-fill') + .attr('points', '16,3 4,3 1,6 1,17 4,20 7,20 10,27 13,20 16,20 19,17.033 19,6'); + + svgEnter + .append('use') + .attr('class', 'icon-annotation') + .attr('width', '11px') + .attr('height', '11px') + .attr('transform', 'translate(4.5, 7)') + .attr('xlink:href', function(d) { + var picon = d.icon; + + if (!picon) { + return ''; + } else { + var isMaki = /^maki-/.test(picon); + return '#' + picon + (isMaki ? '-11' : ''); + } + }); + + headerEnter + .append('div') + .attr('class', 'error-header-label') + .text(errorTitle); + } + + + osmoseHeader.error = function(val) { + if (!arguments.length) return _error; + _error = val; + return osmoseHeader; + }; + + + return osmoseHeader; +} \ No newline at end of file diff --git a/modules/ui/sidebar.js b/modules/ui/sidebar.js index 74ceeb52f3..ebf9dd6a8f 100644 --- a/modules/ui/sidebar.js +++ b/modules/ui/sidebar.js @@ -16,6 +16,7 @@ import { uiFeatureList } from './feature_list'; import { uiInspector } from './inspector'; import { uiImproveOsmEditor } from './improveOSM_editor'; import { uiKeepRightEditor } from './keepRight_editor'; +import { uiOsmoseEditor } from './osmose_editor'; import { uiNoteEditor } from './note_editor'; import { textDirection } from '../util/locale'; @@ -26,6 +27,7 @@ export function uiSidebar(context) { var noteEditor = uiNoteEditor(context); var improveOsmEditor = uiImproveOsmEditor(context); var keepRightEditor = uiKeepRightEditor(context); + var osmoseEditor = uiOsmoseEditor(context); var _current; var _wasData = false; var _wasNote = false; @@ -147,8 +149,15 @@ export function uiSidebar(context) { datum = errService.getError(datum.id); } - // Temporary solution while only two services - var errEditor = (datum.service === 'keepRight') ? keepRightEditor : improveOsmEditor; + // Currently only three possible services + var errEditor; + if (datum.service === 'keepRight') { + errEditor = keepRightEditor; + } else if (datum.service === 'osmose') { + errEditor = osmoseEditor; + } else { + errEditor = improveOsmEditor; + } d3_selectAll('.qa_error.' + datum.service) .classed('hover', function(d) { return d.id === datum.id; }); @@ -357,4 +366,4 @@ export function uiSidebar(context) { sidebar.toggle = function() {}; return sidebar; -} +} \ No newline at end of file From af5b64a477b9afaae83ccbe808da9b271343c49f Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 7 Dec 2019 19:12:06 +0000 Subject: [PATCH 03/38] Allow translation with numerical token replacement Just escapes the special regex characters when creating a new RegExp so that numerical keys can be used (e.g. my use case of replacing an arbitrary number of tokens from an array). Otherwise it is interpreted as a repetition. --- modules/util/locale.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/util/locale.js b/modules/util/locale.js index 44710a5462..e003b759b6 100644 --- a/modules/util/locale.js +++ b/modules/util/locale.js @@ -45,7 +45,7 @@ export function t(s, o, loc) { if (rep !== undefined) { if (o) { for (var k in o) { - var variable = '{' + k + '}'; + var variable = '\\{' + k + '\\}'; var re = new RegExp(variable, 'g'); // check globally for variables rep = rep.replace(re, o[k]); } @@ -124,4 +124,4 @@ export function languageName(context, code, options) { } } return code; // if not found, use the code -} +} \ No newline at end of file From 947f9657db91aeca271f5bbeef2fc481beb025f0 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 7 Dec 2019 19:21:23 +0000 Subject: [PATCH 04/38] Port minor QoL tweaks to other QA services Use of MouseEnter and MouseLeave is nicer for SVGs since it won't fire multiple times (shouldn't happen hear anyway as only one element). Triggering redraw after forcing related elements visible causes them to render without having to pan manually. --- modules/ui/improveOSM_details.js | 7 ++++--- modules/ui/keepRight_details.js | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/modules/ui/improveOSM_details.js b/modules/ui/improveOSM_details.js index c7d33a0760..435c09c519 100644 --- a/modules/ui/improveOSM_details.js +++ b/modules/ui/improveOSM_details.js @@ -78,11 +78,11 @@ export function uiImproveOsmDetails(context) { // Add click handler link - .on('mouseover', function() { + .on('mouseenter', function() { context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) .classed('hover', true); }) - .on('mouseout', function() { + .on('mouseleave', function() { context.surface().selectAll('.hover') .classed('hover', false); }) @@ -122,6 +122,7 @@ export function uiImproveOsmDetails(context) { // Don't hide entities related to this error - #5880 context.features().forceVisible(relatedEntities); + context.map().pan([0,0]); // trigger a redraw } @@ -133,4 +134,4 @@ export function uiImproveOsmDetails(context) { return improveOsmDetails; -} +} \ No newline at end of file diff --git a/modules/ui/keepRight_details.js b/modules/ui/keepRight_details.js index c292bf0aea..966e75f0e1 100644 --- a/modules/ui/keepRight_details.js +++ b/modules/ui/keepRight_details.js @@ -80,11 +80,11 @@ export function uiKeepRightDetails(context) { // Add click handler link - .on('mouseover', function() { + .on('mouseenter', function() { context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) .classed('hover', true); }) - .on('mouseout', function() { + .on('mouseleave', function() { context.surface().selectAll('.hover') .classed('hover', false); }) @@ -124,6 +124,7 @@ export function uiKeepRightDetails(context) { // Don't hide entities related to this error - #5880 context.features().forceVisible(relatedEntities); + context.map().pan([0,0]); // trigger a redraw } @@ -135,4 +136,4 @@ export function uiKeepRightDetails(context) { return keepRightDetails; -} +} \ No newline at end of file From 1fa888069d1bb465c180542df9fef0a6f1c8d21f Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 7 Dec 2019 19:47:14 +0000 Subject: [PATCH 05/38] Fix resolution request for Osmose issues - Remove leftover code that caused undefied variable error - Change to a GET request --- modules/services/osmose.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 91aa5c2bdf..bded8e0eb0 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -178,15 +178,13 @@ export default { var that = this; - if (err) { return callback(err, d); } - // UI sets the status to either '/done' or '/false' var url = _osmoseUrlRoot + 'issue/' + d.identifier + d.newStatus; var controller = new AbortController(); _erCache.inflightPost[d.id] = controller; - fetch(url, { method: 'POST', signal: controller.signal }) + fetch(url, { signal: controller.signal }) .then(function() { delete _erCache.inflightPost[d.id]; From d9ba63aa3f71ccf8e10911afb203e91a4163b29b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 7 Dec 2019 20:12:53 +0000 Subject: [PATCH 06/38] Fix leftover errors and unused definitions --- modules/services/osmose.js | 13 ++++--------- modules/ui/osmose_details.js | 2 +- modules/ui/osmose_editor.js | 4 ++-- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index bded8e0eb0..af7aedd54d 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -3,9 +3,8 @@ import RBush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; -import { geoExtent, geoVecAdd, geoVecScale } from '../geo'; +import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; -import { t } from '../util/locale'; import { utilRebind, utilTiler, utilQsString } from '../util'; import { services } from '../../data/qa_errors.json'; @@ -50,10 +49,6 @@ function updateRtree(item, replace) { } } -function linkErrorObject(d) { - return '' + d + ''; -} - function linkEntity(d) { return '' + d + ''; } @@ -131,7 +126,7 @@ export default { data.issues.forEach(function(issue) { // Elements provided as string, separated by _ character var elems = issue.elems.split('_').map(function(i) { - return i.substring(0,1) + i.replace(/node|way|relation/, '') + return i.substring(0,1) + i.replace(/node|way|relation/, ''); }); var loc = [issue.lon, issue.lat]; // Item is the type of error, w/ class tells us the sub-type @@ -155,7 +150,7 @@ export default { // Variables used in the description d.replacements = elems.map(function(i) { - return linkEntity(i) + return linkEntity(i); }); _erCache.data[d.id] = d; @@ -192,7 +187,7 @@ export default { if (d.newStatus === '/done') { // No pretty identifier, so we just use coordinates var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); - _erCache.closed[key + ':' + closedID] = true; + _erCache.closed[d.error_type + ':' + closedID] = true; } if (callback) callback(null, d); }) diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 9c8dbb8749..9490eb25c1 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -6,7 +6,7 @@ import { import { dataEn } from '../../data'; import { modeSelect } from '../modes/select'; import { t } from '../util/locale'; -import { utilDisplayName, utilEntityOrMemberSelector, utilEntityRoot } from '../util'; +import { utilDisplayName, utilEntityOrMemberSelector } from '../util'; export function uiOsmoseDetails(context) { diff --git a/modules/ui/osmose_editor.js b/modules/ui/osmose_editor.js index 102144dbf9..38cbc42fae 100644 --- a/modules/ui/osmose_editor.js +++ b/modules/ui/osmose_editor.js @@ -129,7 +129,7 @@ export function uiOsmoseEditor(context) { .merge(buttonEnter); buttonSection.select('.close-button') - .text(function(d) { + .text(function() { return t('QA.keepRight.close'); }) .on('click.close', function(d) { @@ -144,7 +144,7 @@ export function uiOsmoseEditor(context) { }); buttonSection.select('.ignore-button') - .text(function(d) { + .text(function() { return t('QA.keepRight.ignore'); }) .on('click.ignore', function(d) { From 680fdb6b54f69c75052d539f366920402397f5ad Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 23 Dec 2019 18:32:26 +0000 Subject: [PATCH 07/38] Allow Osmose errors to use category for UI strings Some error types don't require unique strings and can share common strings among the category. This makes that possible as well as adding support for some other types of error for demonstrative purposes. --- css/65_data.css | 14 +++++++--- data/core.yaml | 35 ++++++++++++++++++++++++- data/qa_errors.json | 43 ++++++++++++++++++++++++++++-- dist/locales/en.json | 49 +++++++++++++++++++++++++++++++++-- modules/services/osmose.js | 10 +++---- modules/svg/osmose.js | 2 +- modules/ui/osmose_details.js | 31 ++++++++++------------ modules/ui/osmose_header.js | 19 +++++++++----- scripts/build_data.js | 3 ++- svg/fontawesome/far-clone.svg | 1 + 10 files changed, 166 insertions(+), 41 deletions(-) create mode 100644 svg/fontawesome/far-clone.svg diff --git a/css/65_data.css b/css/65_data.css index 39c1c1603d..9dfc509c0a 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -155,12 +155,18 @@ /* Osmose Errors ------------------------------------------------------- */ -.osmose { - color: #FFFFFF; + +.osmose.category-0 { + color: #BDBDBD; } -.osmose.category-structure { - color: #F9C700; +.osmose.category-1040, +.osmose.category-1050, +.osmose.category-1070, +.osmose.category-1080, +.osmose.category-1150, +.osmose.category-1280 { + color: #DCB000; } /* Custom Map Data (geojson, gpx, kml, vector tile) */ diff --git a/data/core.yaml b/data/core.yaml index b5b4c0af93..24507a75ae 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -830,9 +830,42 @@ en: osmose: title: Osmose error_types: + 0: + title: 'Building Intersection' + description: '{0} and {1} intersect.' + 0-3: + title: 'Suspiciously Small Building' + description: '{0} is a suspisciously small building.' + 0-4: + title: 'Gap Between Buildings' + description: 'A gap exists between {0} and {1} which looks suspiciously small.' + 1040-1: + title: 'Invalid Polygon' + description: '{0} intersects itself.' + 1050-1: + title: 'Reversed Roundabout' + description: '{0} looks like a roundabout mapped in the wrong direction for this location.' + 1050-1050: + title: 'Reversed Mini-Roundabout' + description: '{0} looks like a mini-roundabout with the incorrect "direction" tag for this location.' + 1070: + title: 'Highway intersecting water' + description: '{1} intersects with {0}, but there is no junction node, bridge, or tunnel.' 1070-1: title: 'Highway intersecting building' - description: '{1} intersects with {0}.' + 1070-8: + title: 'Highway intersecting highway' + 1070-10: + title: 'Waterway intersecting waterway' + 1080-1: + title: 'Orphan Nodes Cluster' + description: 'A culster of nodes with no tags is present.' + 1150: + title: 'Overlapping Similar Areas' + description: '{0} and {1} overlap.' + 1280-1: + title: 'Incorrect Speed Camera' + description: 'Speed camera should be on the highway or part of an "enforcement" relation.' improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/data/qa_errors.json b/data/qa_errors.json index aad4b3328a..50e930cf67 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,10 +34,49 @@ } }, "osmose": { + "items": ["0", "1040", "1050", "1070", "1080", "1150", "1280"], "errorTypes": { + "0-1": { + "icon": "maki-home" + }, + "0-2": { + "icon": "maki-home" + }, + "1040-1": { + "icon": "maki-square-stroked" + }, + "1050-1": { + "icon": "maki-circle-stroked" + }, + "1050-1050": { + "icon": "maki-circle-stroked" + }, + "1150-1": { + "icon": "far-clone" + }, + "1150-2": { + "icon": "far-clone" + }, + "1150-3": { + "icon": "far-clone" + }, "1070-1": { - "icon": "maki-home", - "category": "structure" + "icon": "maki-home" + }, + "1070-4": { + "icon": "maki-dam" + }, + "1070-5": { + "icon": "maki-dam" + }, + "1070-8": { + "icon": "maki-cross" + }, + "1070-10": { + "icon": "maki-cross" + }, + "1280-1": { + "icon": "maki-attraction" } } } diff --git a/dist/locales/en.json b/dist/locales/en.json index a38ff70e6a..df3cdc8189 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1034,9 +1034,54 @@ "osmose": { "title": "Osmose", "error_types": { + "0": { + "title": "Building Intersection", + "description": "{0} and {1} intersect." + }, + "1070": { + "title": "Highway intersecting water", + "description": "{1} intersects with {0}, but there is no junction node, bridge, or tunnel." + }, + "1150": { + "title": "Overlapping Similar Areas", + "description": "{0} and {1} overlap." + }, + "0-3": { + "title": "Suspiciously Small Building", + "description": "{0} is a suspisciously small building." + }, + "0-4": { + "title": "Gap Between Buildings", + "description": "A gap exists between {0} and {1} which looks suspiciously small." + }, + "1040-1": { + "title": "Invalid Polygon", + "description": "{0} intersects itself." + }, + "1050-1": { + "title": "Reversed Roundabout", + "description": "{0} looks like a roundabout mapped in the wrong direction for this location." + }, + "1050-1050": { + "title": "Reversed Mini-Roundabout", + "description": "{0} looks like a mini-roundabout with the incorrect \"direction\" tag for this location." + }, "1070-1": { - "title": "Highway intersecting building", - "description": "{1} intersects with {0}." + "title": "Highway intersecting building" + }, + "1070-8": { + "title": "Highway intersecting highway" + }, + "1070-10": { + "title": "Waterway intersecting waterway" + }, + "1080-1": { + "title": "Orphan Nodes Cluster", + "description": "A culster of nodes with no tags is present." + }, + "1280-1": { + "title": "Incorrect Speed Camera", + "description": "Speed camera should be on the highway or part of an \"enforcement\" relation." } } }, diff --git a/modules/services/osmose.js b/modules/services/osmose.js index af7aedd54d..28e2bfc323 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -94,7 +94,8 @@ export default { var options = { full: 'true', // Returns element IDs level: '1,2,3', - zoom: '19' + zoom: '19', + item: services.osmose.items.join() // only interested in certain errors }; // determine the needed tiles to cover the view @@ -110,7 +111,7 @@ export default { if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; var rect = tile.extent.rectangle(); // E, N, W, S - var params = Object.assign({}, options, { bbox: [rect[0], rect[1], rect[2], rect[3]].join() }); + var params = Object.assign({}, options, { bbox: rect.join() }); var url = _osmoseUrlRoot + 'issues?' + utilQsString(params); @@ -133,7 +134,7 @@ export default { var type = [issue.item, issue.classs].join('-'); // Filter out unsupported error types (some are too specific or advanced) - if (services.osmose.errorTypes[type]) { + if (type in services.osmose.errorTypes) { loc = preventCoincident(loc, true); var d = new qaError({ @@ -144,8 +145,7 @@ export default { // Extra details needed for this service identifier: issue.id, // this is used to post changes to the error elems: elems, - object_id: elems.length ? elems[0].substring(1) : '', - object_type: elems.length ? elems[0].substring(0,1) : '' + item: issue.item // category of the issue for styling }); // Variables used in the description diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index 1126087d68..bc25d96125 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -120,7 +120,7 @@ export function svgOsmose(projection, context, dispatch) { d.service, 'error_id-' + d.id, 'error_type-' + d.error_type, - 'category-' + d.category + 'category-' + d.item ].join(' '); }); diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 9490eb25c1..22353e7f4b 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -18,19 +18,18 @@ export function uiOsmoseDetails(context) { if (!d) return unknown; - if (d.desc) return d.desc; - - var errorType = d.error_type; - var et = dataEn.QA.osmose.error_types[errorType]; - - var detail; - if (et && et.description) { - detail = t('QA.osmose.error_types.' + errorType + '.description', d.replacements); - } else { - detail = unknown; + // Some errors fall back to their category for strings + var i, et; + var keys = [d.error_type, d.item]; + for (i = 0; i < 2; i++) { + et = dataEn.QA.osmose.error_types[keys[i]]; + + if (et && et.description) { + return t('QA.osmose.error_types.' + keys[i] + '.description', d.replacements); + } } - return detail; + return unknown; } @@ -64,14 +63,10 @@ export function uiOsmoseDetails(context) { .html(errorDetail); // If there are entity links in the error message.. - var relatedEntities = _error.elems; descriptionEnter.selectAll('.error_entity_link, .error_object_link') .each(function() { var link = d3_select(this); - var isObjectLink = link.classed('error_object_link'); - var entityID = isObjectLink ? - (_error.object_type + _error.object_id) - : this.textContent; + var entityID = this.textContent; var entity = context.hasEntity(entityID); // Add click handler @@ -107,7 +102,7 @@ export function uiOsmoseDetails(context) { if (entity) { var name = utilDisplayName(entity); // try to use common name - if (!name && !isObjectLink) { + if (!name) { var preset = context.presets().match(entity, context.graph()); name = preset && !preset.isFallback() && preset.name(); // fallback to preset name } @@ -119,7 +114,7 @@ export function uiOsmoseDetails(context) { }); // Don't hide entities related to this error - #5880 - context.features().forceVisible(relatedEntities); + context.features().forceVisible(_error.elems); context.map().pan([0,0]); // trigger a redraw } diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js index 21995cbcb7..b66d31980b 100644 --- a/modules/ui/osmose_header.js +++ b/modules/ui/osmose_header.js @@ -10,14 +10,19 @@ export function uiOsmoseHeader() { var unknown = t('inspector.unknown'); if (!d) return unknown; - var errorType = d.error_type; - var et = dataEn.QA.osmose.error_types[errorType]; - if (et && et.title) { - return t('QA.osmose.error_types.' + errorType + '.title'); - } else { - return unknown; + // Some errors fall back to their category for strings + var i, et; + var keys = [d.error_type, d.item]; + for (i = 0; i < 2; i++) { + et = dataEn.QA.osmose.error_types[keys[i]]; + + if (et && et.title) { + return t('QA.osmose.error_types.' + keys[i] + '.title'); + } } + + return unknown; } @@ -52,7 +57,7 @@ export function uiOsmoseHeader() { d.service, 'error_id-' + d.id, 'error_type-' + d.error_type, - 'category-' + d.category + 'category-' + d.item ].join(' '); }); diff --git a/scripts/build_data.js b/scripts/build_data.js index 1dfbee72f2..a5c7c2ef79 100644 --- a/scripts/build_data.js +++ b/scripts/build_data.js @@ -69,7 +69,8 @@ function buildData() { 'fas-lock': {}, 'fas-long-arrow-alt-right': {}, 'fas-th-list': {}, - 'fas-user-cog': {} + 'fas-user-cog': {}, + 'far-clone': {} }; // The Noun Project icons used diff --git a/svg/fontawesome/far-clone.svg b/svg/fontawesome/far-clone.svg new file mode 100644 index 0000000000..c3e65e21e7 --- /dev/null +++ b/svg/fontawesome/far-clone.svg @@ -0,0 +1 @@ + \ No newline at end of file From ebd206595a0acce03af4731242be2afe6bcf4274 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 28 Dec 2019 18:55:04 +0000 Subject: [PATCH 08/38] Use Osmose tiles API endpoint instead of issues - Simplifies our code a little bit - Quick to provide and gives us the `item` + `class` without requesting all other information (the two things needed to know the error type) - Removed handling of associated elements for now, need to implement fetching for that data on a per-issue basis when the UI loads - Set things up for the possibility of using the provided translation strings (need to determine if desirable) See: https://github.com/osm-fr/osmose-frontend/issues/193 --- data/qa_errors.json | 1 - modules/services/osmose.js | 44 ++++++++++++++++-------------------- modules/ui/osmose_details.js | 2 +- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/data/qa_errors.json b/data/qa_errors.json index 50e930cf67..77364cf8ca 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,7 +34,6 @@ } }, "osmose": { - "items": ["0", "1040", "1050", "1070", "1080", "1150", "1280"], "errorTypes": { "0-1": { "icon": "maki-home" diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 28e2bfc323..12060cbe96 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -14,7 +14,7 @@ var dispatch = d3_dispatch('loaded'); var _erCache; var _erZoom = 14; -var _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta/'; +var _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/'; function abortRequest(controller) { if (controller) { @@ -91,11 +91,8 @@ export default { }, loadErrors: function(projection) { - var options = { - full: 'true', // Returns element IDs - level: '1,2,3', - zoom: '19', - item: services.osmose.items.join() // only interested in certain errors + var params = { + level: '1,2,3' }; // determine the needed tiles to cover the view @@ -110,10 +107,9 @@ export default { tiles.forEach(function(tile) { if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; - var rect = tile.extent.rectangle(); // E, N, W, S - var params = Object.assign({}, options, { bbox: rect.join() }); - - var url = _osmoseUrlRoot + 'issues?' + utilQsString(params); + var lang = 'en'; // todo: may want to use provided translations + var path = [tile.xyz[2], tile.xyz[0], tile.xyz[1]].join('/'); + var url = _osmoseUrlRoot + lang + '/map/issues/' + path + '.json?' + utilQsString(params); var controller = new AbortController(); _erCache.inflightTile[tile.id] = controller; @@ -123,15 +119,12 @@ export default { delete _erCache.inflightTile[tile.id]; _erCache.loadedTile[tile.id] = true; - if (data.issues) { - data.issues.forEach(function(issue) { - // Elements provided as string, separated by _ character - var elems = issue.elems.split('_').map(function(i) { - return i.substring(0,1) + i.replace(/node|way|relation/, ''); - }); - var loc = [issue.lon, issue.lat]; + if (data.features) { + data.features.forEach(function(issue) { + var loc = issue.geometry.coordinates; // lon, lat + var props = issue.properties; // Item is the type of error, w/ class tells us the sub-type - var type = [issue.item, issue.classs].join('-'); + var type = [props.item, props.class].join('-'); // Filter out unsupported error types (some are too specific or advanced) if (type in services.osmose.errorTypes) { @@ -143,21 +136,22 @@ export default { service: 'osmose', error_type: type, // Extra details needed for this service - identifier: issue.id, // this is used to post changes to the error - elems: elems, - item: issue.item // category of the issue for styling + identifier: props.issue_id, // this is used to post changes to the error + item: props.item // category of the issue for styling }); // Variables used in the description - d.replacements = elems.map(function(i) { - return linkEntity(i); - }); + // d.replacements = elems.map(function(i) { + // return linkEntity(i); + // }); _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); } }); } + + dispatch.call('loaded'); }) .catch(function() { delete _erCache.inflightTile[tile.id]; @@ -166,6 +160,8 @@ export default { }); }, + // todo: need to fetch specific error details upon UI loading for element IDs + postUpdate: function(d, callback) { if (_erCache.inflightPost[d.id]) { return callback({ message: 'Error update already inflight', status: -2 }, d); diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 22353e7f4b..0d968e7f47 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -114,7 +114,7 @@ export function uiOsmoseDetails(context) { }); // Don't hide entities related to this error - #5880 - context.features().forceVisible(_error.elems); + // context.features().forceVisible(_error.elems); context.map().pan([0,0]); // trigger a redraw } From b24b041335dc1e5ccd5a9dd50313e4bc04812fcd Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 28 Dec 2019 21:45:45 +0000 Subject: [PATCH 09/38] Add Osmose issue detail fetching for UI Must fetch details per-issue to get the associated OSM elements for highlighting and use in the error description. Also fixes URL for error closure. --- data/core.yaml | 3 - data/qa_errors.json | 1 + dist/locales/en.json | 4 -- modules/services/osmose.js | 41 ++++++++++--- modules/ui/osmose_details.js | 113 ++++++++++++++++++----------------- 5 files changed, 93 insertions(+), 69 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 24507a75ae..a10db48ccf 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -857,9 +857,6 @@ en: title: 'Highway intersecting highway' 1070-10: title: 'Waterway intersecting waterway' - 1080-1: - title: 'Orphan Nodes Cluster' - description: 'A culster of nodes with no tags is present.' 1150: title: 'Overlapping Similar Areas' description: '{0} and {1} overlap.' diff --git a/data/qa_errors.json b/data/qa_errors.json index 77364cf8ca..5fe329ff8f 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,6 +34,7 @@ } }, "osmose": { + "items": ["0", "1040", "1050", "1070", "1150", "1280"], "errorTypes": { "0-1": { "icon": "maki-home" diff --git a/dist/locales/en.json b/dist/locales/en.json index df3cdc8189..d2204cd953 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1075,10 +1075,6 @@ "1070-10": { "title": "Waterway intersecting waterway" }, - "1080-1": { - "title": "Orphan Nodes Cluster", - "description": "A culster of nodes with no tags is present." - }, "1280-1": { "title": "Incorrect Speed Camera", "description": "Speed camera should be on the highway or part of an \"enforcement\" relation." diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 12060cbe96..7408480eb2 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -92,7 +92,8 @@ export default { loadErrors: function(projection) { var params = { - level: '1,2,3' + level: '1,2,3', + item: services.osmose.items.join() // only interested in certain errors }; // determine the needed tiles to cover the view @@ -140,11 +141,6 @@ export default { item: props.item // category of the issue for styling }); - // Variables used in the description - // d.replacements = elems.map(function(i) { - // return linkEntity(i); - // }); - _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); } @@ -160,7 +156,36 @@ export default { }); }, - // todo: need to fetch specific error details upon UI loading for element IDs + loadErrorDetail: function(d, callback) { + // Error details only need to be fetched once + if (d.elems !== undefined) { + if (callback) callback(null, d); + return; + } + + var url = _osmoseUrlRoot + 'en/api/0.3beta/issue/' + d.identifier; + + var that = this; + d3_json(url) + .then(function(data) { + // Associated elements used for highlighting + // Assign directly for immediate use in the callback + d.elems = data.elems.map(function(e) { + return e.type.substring(0,1) + e.id; + }); + + // Element links used in the error description + d.replacements = d.elems.map(function(i) { + return linkEntity(i); + }); + + that.replaceError(d); + if (callback) callback(null, d); + }) + .catch(function(err) { + if (callback) callback(err.message); + }); + }, postUpdate: function(d, callback) { if (_erCache.inflightPost[d.id]) { @@ -170,7 +195,7 @@ export default { var that = this; // UI sets the status to either '/done' or '/false' - var url = _osmoseUrlRoot + 'issue/' + d.identifier + d.newStatus; + var url = _osmoseUrlRoot + 'en/api/0.3beta/issue/' + d.identifier + d.newStatus; var controller = new AbortController(); _erCache.inflightPost[d.id] = controller; diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 0d968e7f47..756a5dd5a5 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -6,6 +6,7 @@ import { import { dataEn } from '../../data'; import { modeSelect } from '../modes/select'; import { t } from '../util/locale'; +import { services } from '../services'; import { utilDisplayName, utilEntityOrMemberSelector } from '../util'; @@ -57,65 +58,69 @@ export function uiOsmoseDetails(context) { .append('h4') .text(function() { return t('QA.keepRight.detail_description'); }); - descriptionEnter - .append('div') - .attr('class', 'error-details-description-text') - .html(errorDetail); - - // If there are entity links in the error message.. - descriptionEnter.selectAll('.error_entity_link, .error_object_link') - .each(function() { - var link = d3_select(this); - var entityID = this.textContent; - var entity = context.hasEntity(entityID); - - // Add click handler - link - .on('mouseenter', function() { - context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) - .classed('hover', true); - }) - .on('mouseleave', function() { - context.surface().selectAll('.hover') - .classed('hover', false); - }) - .on('click', function() { - d3_event.preventDefault(); - var osmlayer = context.layers().layer('osm'); - if (!osmlayer.enabled()) { - osmlayer.enabled(true); - } - - context.map().centerZoom(_error.loc, 20); - - if (entity) { - context.enter(modeSelect(context, [entityID])); - } else { - context.loadEntity(entityID, function() { + services.osmose.loadErrorDetail(_error, function(err, d) { + if (d.elems === undefined) { return; } + + descriptionEnter + .append('div') + .attr('class', 'error-details-description-text') + .html(errorDetail); + + // If there are entity links in the error message.. + descriptionEnter.selectAll('.error_entity_link, .error_object_link') + .each(function() { + var link = d3_select(this); + var entityID = this.textContent; + var entity = context.hasEntity(entityID); + + // Add click handler + link + .on('mouseenter', function() { + context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) + .classed('hover', true); + }) + .on('mouseleave', function() { + context.surface().selectAll('.hover') + .classed('hover', false); + }) + .on('click', function() { + d3_event.preventDefault(); + var osmlayer = context.layers().layer('osm'); + if (!osmlayer.enabled()) { + osmlayer.enabled(true); + } + + context.map().centerZoom(d.loc, 20); + + if (entity) { context.enter(modeSelect(context, [entityID])); - }); + } else { + context.loadEntity(entityID, function() { + context.enter(modeSelect(context, [entityID])); + }); + } + }); + + // Replace with friendly name if possible + // (The entity may not yet be loaded into the graph) + if (entity) { + var name = utilDisplayName(entity); // try to use common name + + if (!name) { + var preset = context.presets().match(entity, context.graph()); + name = preset && !preset.isFallback() && preset.name(); // fallback to preset name } - }); - // Replace with friendly name if possible - // (The entity may not yet be loaded into the graph) - if (entity) { - var name = utilDisplayName(entity); // try to use common name - - if (!name) { - var preset = context.presets().match(entity, context.graph()); - name = preset && !preset.isFallback() && preset.name(); // fallback to preset name - } - - if (name) { - this.innerText = name; + if (name) { + this.innerText = name; + } } - } - }); + }); - // Don't hide entities related to this error - #5880 - // context.features().forceVisible(_error.elems); - context.map().pan([0,0]); // trigger a redraw + // Don't hide entities related to this error - #5880 + context.features().forceVisible(d.elems); + context.map().pan([0,0]); // trigger a redraw + }); } From 5afdfc527b8bb6152a0eeb15f65f8cba51f20e65 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 29 Dec 2019 17:27:41 +0000 Subject: [PATCH 10/38] Add support for more Osmose error types - Missing parking access - Malformed opening hours tagging - Objects detected by Mapillary that aren't mapped - Traffic sign information detected by Mapillary that isn't mapped Mapillary errors add example code for special error type handling as they don't need to request further details and can use piecewise translation strings --- css/65_data.css | 10 +++++ data/core.yaml | 25 +++++++++++- data/qa_errors.json | 56 +++++++++++++++++++++----- dist/locales/en.json | 31 +++++++++++++- modules/services/osmose.js | 19 +++++++-- scripts/build_data.js | 3 +- svg/fontawesome/fas-weight-hanging.svg | 1 + 7 files changed, 129 insertions(+), 16 deletions(-) create mode 100644 svg/fontawesome/fas-weight-hanging.svg diff --git a/css/65_data.css b/css/65_data.css index 9dfc509c0a..7a885e8be4 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -169,6 +169,16 @@ color: #DCB000; } +.osmose.category-3161, +.osmose.category-3250 { + color: #2D9359; +} + +.osmose.category-8300, +.osmose.category-8360 { + color: #3DE736; +} + /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { pointer-events: none; diff --git a/data/core.yaml b/data/core.yaml index a10db48ccf..340b64a606 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -862,7 +862,30 @@ en: description: '{0} and {1} overlap.' 1280-1: title: 'Incorrect Speed Camera' - description: 'Speed camera should be on the highway or part of an "enforcement" relation.' + description: '{0} should be on the highway or part of an "enforcement" relation.' + 3161: + title: 'Missing Parking Access' + description: 'There is no highway leading to {0}.' + 3250-32501: + title: 'Invalid Opening Hours' + description: '{0} has an invalid value for the "opening_hours" tag.' + 8300: + title: 'Traffic Sign Suggestion' + description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' + parts: + 20: 'max height limit' + 21: 'max weight limit' + 32: 'roundabout' + 34: 'set of speed bumps' + 8360: + title: 'Missing Street Object' + description: 'Object detection by Mapillary suggests there may be an unmapped {0} nearby.' + parts: + 1: 'bench' + 2: 'type of bicycle parking' + 3: 'surveillance camera' + 4: 'fire hydrant' + 5: 'set of traffic signals' improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/data/qa_errors.json b/data/qa_errors.json index 5fe329ff8f..e0122a3711 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,7 +34,7 @@ } }, "osmose": { - "items": ["0", "1040", "1050", "1070", "1150", "1280"], + "items": ["0", "1040", "1050", "1070", "1150", "1280", "3161", "3250", "8300", "8360"], "errorTypes": { "0-1": { "icon": "maki-home" @@ -51,15 +51,6 @@ "1050-1050": { "icon": "maki-circle-stroked" }, - "1150-1": { - "icon": "far-clone" - }, - "1150-2": { - "icon": "far-clone" - }, - "1150-3": { - "icon": "far-clone" - }, "1070-1": { "icon": "maki-home" }, @@ -75,8 +66,53 @@ "1070-10": { "icon": "maki-cross" }, + "1150-1": { + "icon": "far-clone" + }, + "1150-2": { + "icon": "far-clone" + }, + "1150-3": { + "icon": "far-clone" + }, "1280-1": { "icon": "maki-attraction" + }, + "3161-1": { + "icon": "maki-parking" + }, + "3161-2": { + "icon": "maki-parking" + }, + "3250-32501": { + "icon": "maki-watch" + }, + "8300-20": { + "icon": "temaki-height_restrictor" + }, + "8300-21": { + "icon": "fas-weight-hanging" + }, + "8300-32": { + "icon": "maki-circle-stroked" + }, + "8300-34": { + "icon": "temaki-diamond" + }, + "8360-1": { + "icon": "temaki-bench" + }, + "8360-2": { + "icon": "maki-bicycle" + }, + "8360-3": { + "icon": "temaki-security_camera" + }, + "8360-4": { + "icon": "temaki-fire_hydrant" + }, + "8360-5": { + "icon": "temaki-traffic_signals" } } } diff --git a/dist/locales/en.json b/dist/locales/en.json index d2204cd953..b4a04fda98 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1046,6 +1046,31 @@ "title": "Overlapping Similar Areas", "description": "{0} and {1} overlap." }, + "3161": { + "title": "Missing Parking Access", + "description": "There is no highway leading to {0}." + }, + "8300": { + "title": "Traffic Sign Suggestion", + "description": "Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.", + "parts": { + "20": "max height limit", + "21": "max weight limit", + "32": "roundabout", + "34": "set of speed bumps" + } + }, + "8360": { + "title": "Missing Street Object", + "description": "Object detection by Mapillary suggests there may be an unmapped {0} nearby.", + "parts": { + "1": "bench", + "2": "type of bicycle parking", + "3": "surveillance camera", + "4": "fire hydrant", + "5": "set of traffic signals" + } + }, "0-3": { "title": "Suspiciously Small Building", "description": "{0} is a suspisciously small building." @@ -1077,7 +1102,11 @@ }, "1280-1": { "title": "Incorrect Speed Camera", - "description": "Speed camera should be on the highway or part of an \"enforcement\" relation." + "description": "{0} should be on the highway or part of an \"enforcement\" relation." + }, + "3250-32501": { + "title": "Invalid Opening Hours", + "description": "{0} has an invalid value for the \"opening_hours\" tag." } } }, diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 7408480eb2..8a495e982f 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -3,6 +3,7 @@ import RBush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; +import { dataEn } from '../../data'; import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -92,7 +93,6 @@ export default { loadErrors: function(projection) { var params = { - level: '1,2,3', item: services.osmose.items.join() // only interested in certain errors }; @@ -137,10 +137,23 @@ export default { service: 'osmose', error_type: type, // Extra details needed for this service - identifier: props.issue_id, // this is used to post changes to the error - item: props.item // category of the issue for styling + identifier: props.issue_id, // needed to query and update the error + item: props.item, // category of the issue for styling + class: props.class }); + // Special handling for some error types + // Setting elems here prevents UI error detail requests + var parts = dataEn.QA.osmose.error_types[d.item].parts; + switch (d.item) { + case 8300: + case 8360: + // todo: possible to add link to open mapillay photo overlay? + d.replacements = [parts[d.class]]; + d.elems = []; + break; + } + _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); } diff --git a/scripts/build_data.js b/scripts/build_data.js index a5c7c2ef79..e5ddff4bf1 100644 --- a/scripts/build_data.js +++ b/scripts/build_data.js @@ -70,7 +70,8 @@ function buildData() { 'fas-long-arrow-alt-right': {}, 'fas-th-list': {}, 'fas-user-cog': {}, - 'far-clone': {} + 'far-clone': {}, + 'fas-weight-hanging': {} }; // The Noun Project icons used diff --git a/svg/fontawesome/fas-weight-hanging.svg b/svg/fontawesome/fas-weight-hanging.svg new file mode 100644 index 0000000000..10ce728a47 --- /dev/null +++ b/svg/fontawesome/fas-weight-hanging.svg @@ -0,0 +1 @@ + \ No newline at end of file From d4723ec9e247e21dfdf63a128ecccb4d4dac73b2 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 29 Dec 2019 19:18:30 +0000 Subject: [PATCH 11/38] Read icons from qa_errors.json when building data Also adds support for the following error types: - Objects mapped as both node/area or way/area - Power lines that lead to nowhere - Extra nodes in power lines (which should only go from support to support to endpoint) - Power line supports with no power lines --- css/65_data.css | 8 ++++++++ data/core.yaml | 12 ++++++++++++ data/qa_errors.json | 23 ++++++++++++++++++++++- dist/locales/en.json | 16 ++++++++++++++++ scripts/build_data.js | 29 ++++++++++++++++++++++++----- svg/fontawesome/far-dot-circle.svg | 1 + 6 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 svg/fontawesome/far-dot-circle.svg diff --git a/css/65_data.css b/css/65_data.css index 7a885e8be4..416321186a 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -174,6 +174,14 @@ color: #2D9359; } +.osmose.category-4080 { + color: #F2F230; +} + +.osmose.category-7040 { + color: #9F16B4; +} + .osmose.category-8300, .osmose.category-8360 { color: #3DE736; diff --git a/data/core.yaml b/data/core.yaml index 340b64a606..8939c3b29f 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -869,6 +869,18 @@ en: 3250-32501: title: 'Invalid Opening Hours' description: '{0} has an invalid value for the "opening_hours" tag.' + 4080: + title: 'Object Mapped Twice' + description: '{0} and {1} both appear to represent the same object.' + 7040: + title: 'Unfinished Power Line' + description: 'A power line appears to be unfinished at {0}.' + 7040-1: + title: 'Lone Power Support' + description: '{0} is a power tower or pole with no connected power lines.' + 7040-4: + title: 'Extra Power Line Node' + description: 'Power lines only have nodes at connections and {0} is not tagged as a power line support.' 8300: title: 'Traffic Sign Suggestion' description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' diff --git a/data/qa_errors.json b/data/qa_errors.json index e0122a3711..54eed409b7 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,7 +34,7 @@ } }, "osmose": { - "items": ["0", "1040", "1050", "1070", "1150", "1280", "3161", "3250", "8300", "8360"], + "items": ["0", "1040", "1050", "1070", "1150", "1280", "3161", "3250", "4080", "7040", "8300", "8360"], "errorTypes": { "0-1": { "icon": "maki-home" @@ -87,6 +87,27 @@ "3250-32501": { "icon": "maki-watch" }, + "4080-1": { + "icon": "far-dot-circle" + }, + "4080-2": { + "icon": "far-dot-circle" + }, + "4080-3": { + "icon": "far-dot-circle" + }, + "7040-1": { + "icon": "temaki-power_tower" + }, + "7040-2": { + "icon": "temaki-power" + }, + "7040-4": { + "icon": "maki-marker" + }, + "7040-6": { + "icon": "temaki-power" + }, "8300-20": { "icon": "temaki-height_restrictor" }, diff --git a/dist/locales/en.json b/dist/locales/en.json index b4a04fda98..535db8a627 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1050,6 +1050,14 @@ "title": "Missing Parking Access", "description": "There is no highway leading to {0}." }, + "4080": { + "title": "Object Mapped Twice", + "description": "{0} and {1} both appear to represent the same object." + }, + "7040": { + "title": "Unfinished Power Line", + "description": "A power line appears to be unfinished at {0}." + }, "8300": { "title": "Traffic Sign Suggestion", "description": "Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.", @@ -1107,6 +1115,14 @@ "3250-32501": { "title": "Invalid Opening Hours", "description": "{0} has an invalid value for the \"opening_hours\" tag." + }, + "7040-1": { + "title": "Lone Power Support", + "description": "{0} is a power tower or pole with no connected power lines." + }, + "7040-4": { + "title": "Extra Power Line Node", + "description": "Power lines only have nodes at connections and {0} is not tagged as a power line support." } } }, diff --git a/scripts/build_data.js b/scripts/build_data.js index e5ddff4bf1..02f72b80fc 100644 --- a/scripts/build_data.js +++ b/scripts/build_data.js @@ -67,11 +67,8 @@ function buildData() { let faIcons = { 'fas-i-cursor': {}, 'fas-lock': {}, - 'fas-long-arrow-alt-right': {}, 'fas-th-list': {}, - 'fas-user-cog': {}, - 'far-clone': {}, - 'fas-weight-hanging': {} + 'fas-user-cog': {} }; // The Noun Project icons used @@ -92,7 +89,7 @@ function buildData() { 'dist/data/*', 'svg/fontawesome/*.svg', ]); - + readQAErrorIcons(faIcons, tnpIcons); let categories = generateCategories(tstrings, faIcons, tnpIcons); let fields = generateFields(tstrings, faIcons, tnpIcons, searchableFieldIDs); let presets = generatePresets(tstrings, faIcons, tnpIcons, searchableFieldIDs); @@ -174,6 +171,28 @@ function validate(file, instance, schema) { } +function readQAErrorIcons(faIcons, tnpIcons) { + const qa = read('data/qa_errors.json'); + + for (const service in qa.services) { + for (const error in qa.services[service].errorTypes) { + const icon = qa.services[service] + .errorTypes[error] + .icon; + + // fontawesome icon, remember for later + if (/^fa[srb]-/.test(icon)) { + faIcons[icon] = {}; + } + // noun project icon, remember for later + if (/^tnp-/.test(icon)) { + tnpIcons[icon] = {}; + } + } + } +} + + function generateCategories(tstrings, faIcons, tnpIcons) { let categories = {}; diff --git a/svg/fontawesome/far-dot-circle.svg b/svg/fontawesome/far-dot-circle.svg new file mode 100644 index 0000000000..9decb11459 --- /dev/null +++ b/svg/fontawesome/far-dot-circle.svg @@ -0,0 +1 @@ + \ No newline at end of file From a068cdfc5973e46249068a35fd1e52b95cbc7b78 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 30 Dec 2019 13:08:13 +0000 Subject: [PATCH 12/38] Tag changeset with Osmose issue closed counts This seems more useful than tagging with a list of joined coordinates which quickly becomes truncated due to tag value length. Adds a tag per issue type similar to iDs own validation. --- modules/services/osmose.js | 14 ++++++++------ modules/ui/commit.js | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 8a495e982f..b98c43df7a 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -219,9 +219,11 @@ export default { that.removeError(d); if (d.newStatus === '/done') { - // No pretty identifier, so we just use coordinates - var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); - _erCache.closed[d.error_type + ':' + closedID] = true; + // No error identifier, so we give a count of each category + if (!(d.item in _erCache.closed)) { + _erCache.closed[d.item] = 0; + } + _erCache.closed[d.item] += 1; } if (callback) callback(null, d); }) @@ -266,8 +268,8 @@ export default { updateRtree(encodeErrorRtree(error), false); // false = remove }, - // Used to populate `closed:osmose` changeset tag - getClosedIDs: function() { - return Object.keys(_erCache.closed).sort(); + // Used to populate `closed:osmose:*` changeset tags + getClosedCounts: function() { + return _erCache.closed; } }; \ No newline at end of file diff --git a/modules/ui/commit.js b/modules/ui/commit.js index 6f5e293d8f..b2a764a88f 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -150,9 +150,9 @@ export function uiCommit(context) { } } if (services.osmose) { - var osmoseClosed = services.osmose.getClosedIDs(); - if (osmoseClosed.length) { - tags['closed:osmose'] = osmoseClosed.join(';').substr(0, 255); + var osmoseClosed = services.osmose.getClosedCounts(); + for (var issueType in osmoseClosed) { + tags['closed:osmose:' + issueType] = osmoseClosed[issueType].toString().substr(0, 255); } } From 896ed762e08c71eb93e1e5d8697818f405dd4d1f Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 30 Dec 2019 16:20:54 +0000 Subject: [PATCH 13/38] Fix silent failure for some Osmose error types When I added special handling for mapillary errors the code was trying to access translation strings nested under the error category (which doesn't exist for all error types). This code is now moved into it's own function so that variable hoisting doesn't run it for non-applicable error types and prevent them from working. - Also adds support for error type 3040 (bad tag value) - Updated the Osmose sidebar UI title --- css/65_data.css | 1 + data/core.yaml | 7 +++++-- data/qa_errors.json | 5 ++++- dist/locales/en.json | 8 ++++++-- modules/services/osmose.js | 21 +++++++++++++++++---- svg/fontawesome/far-times-circle.svg | 1 + 6 files changed, 34 insertions(+), 9 deletions(-) create mode 100644 svg/fontawesome/far-times-circle.svg diff --git a/css/65_data.css b/css/65_data.css index 416321186a..146adde24b 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -169,6 +169,7 @@ color: #DCB000; } +.osmose.category-3040, .osmose.category-3161, .osmose.category-3250 { color: #2D9359; diff --git a/data/core.yaml b/data/core.yaml index 8939c3b29f..44f12d4d63 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -828,7 +828,7 @@ en: full_screen: Toggle Full Screen QA: osmose: - title: Osmose + title: Osmose Issue error_types: 0: title: 'Building Intersection' @@ -863,6 +863,9 @@ en: 1280-1: title: 'Incorrect Speed Camera' description: '{0} should be on the highway or part of an "enforcement" relation.' + 3040-3040: + title: 'Unusual Tagging' + description: '{0} has a non-conventional tag value: {1}.' 3161: title: 'Missing Parking Access' description: 'There is no highway leading to {0}.' @@ -880,7 +883,7 @@ en: description: '{0} is a power tower or pole with no connected power lines.' 7040-4: title: 'Extra Power Line Node' - description: 'Power lines only have nodes at connections and {0} is not tagged as a power line support.' + description: 'Power lines should only have nodes at supports and {0} is not tagged as a pole or tower.' 8300: title: 'Traffic Sign Suggestion' description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' diff --git a/data/qa_errors.json b/data/qa_errors.json index 54eed409b7..ddf8e35070 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,7 +34,7 @@ } }, "osmose": { - "items": ["0", "1040", "1050", "1070", "1150", "1280", "3161", "3250", "4080", "7040", "8300", "8360"], + "items": ["0", "1040", "1050", "1070", "1150", "1280", "3040", "3161", "3250", "4080", "7040", "8300", "8360"], "errorTypes": { "0-1": { "icon": "maki-home" @@ -78,6 +78,9 @@ "1280-1": { "icon": "maki-attraction" }, + "3040-3040": { + "icon": "far-times-circle" + }, "3161-1": { "icon": "maki-parking" }, diff --git a/dist/locales/en.json b/dist/locales/en.json index 535db8a627..60ff1331ee 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1032,7 +1032,7 @@ "full_screen": "Toggle Full Screen", "QA": { "osmose": { - "title": "Osmose", + "title": "Osmose Issue", "error_types": { "0": { "title": "Building Intersection", @@ -1112,6 +1112,10 @@ "title": "Incorrect Speed Camera", "description": "{0} should be on the highway or part of an \"enforcement\" relation." }, + "3040-3040": { + "title": "Unusual Tagging", + "description": "{0} has a non-conventional tag value: {1}." + }, "3250-32501": { "title": "Invalid Opening Hours", "description": "{0} has an invalid value for the \"opening_hours\" tag." @@ -1122,7 +1126,7 @@ }, "7040-4": { "title": "Extra Power Line Node", - "description": "Power lines only have nodes at connections and {0} is not tagged as a power line support." + "description": "Power lines should only have nodes at supports and {0} is not tagged as a pole or tower." } } }, diff --git a/modules/services/osmose.js b/modules/services/osmose.js index b98c43df7a..46f498ecf7 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -144,13 +144,10 @@ export default { // Special handling for some error types // Setting elems here prevents UI error detail requests - var parts = dataEn.QA.osmose.error_types[d.item].parts; switch (d.item) { case 8300: case 8360: - // todo: possible to add link to open mapillay photo overlay? - d.replacements = [parts[d.class]]; - d.elems = []; + mapillaryError(d); break; } @@ -167,6 +164,13 @@ export default { _erCache.loadedTile[tile.id] = true; }); }); + + function mapillaryError(d) { + // Parts only exists for these error types + var parts = dataEn.QA.osmose.error_types[d.item].parts; + d.replacements = [parts[d.class]]; + d.elems = []; + } }, loadErrorDetail: function(d, callback) { @@ -192,6 +196,15 @@ export default { return linkEntity(i); }); + // Special handling for some error types + switch (d.item) { + case 3040: + d.replacements.push(/Bad value for (.+)/i + .exec(data.subtitle)[1] + ); + break; + } + that.replaceError(d); if (callback) callback(null, d); }) diff --git a/svg/fontawesome/far-times-circle.svg b/svg/fontawesome/far-times-circle.svg new file mode 100644 index 0000000000..25b42d56e3 --- /dev/null +++ b/svg/fontawesome/far-times-circle.svg @@ -0,0 +1 @@ + \ No newline at end of file From 12206c68e6143ac01778b3b188b702ad619c90fd Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 30 Dec 2019 19:07:12 +0000 Subject: [PATCH 14/38] Convert Osmsoe service to ES6 syntax --- modules/services/osmose.js | 455 ++++++++++++++++++------------------ modules/ui/osmose_editor.js | 4 +- 2 files changed, 223 insertions(+), 236 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 46f498ecf7..d42994e5ac 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -9,280 +9,267 @@ import { qaError } from '../osm'; import { utilRebind, utilTiler, utilQsString } from '../util'; import { services } from '../../data/qa_errors.json'; -var tiler = utilTiler(); -var dispatch = d3_dispatch('loaded'); +const tiler = utilTiler(); +const dispatch = d3_dispatch('loaded'); +const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/'; +const _erZoom = 14; -var _erCache; -var _erZoom = 14; - -var _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/'; +// This gets reassigned if reset +let _erCache; function abortRequest(controller) { - if (controller) { - controller.abort(); - } + if (controller) { + controller.abort(); + } } function abortUnwantedRequests(cache, tiles) { - Object.keys(cache.inflightTile).forEach(function(k) { - var wanted = tiles.find(function(tile) { return k === tile.id; }); - if (!wanted) { - abortRequest(cache.inflightTile[k]); - delete cache.inflightTile[k]; - } - }); + Object.keys(cache.inflightTile).forEach(k => { + let wanted = tiles.find(tile => k === tile.id); + if (!wanted) { + abortRequest(cache.inflightTile[k]); + delete cache.inflightTile[k]; + } + }); } - function encodeErrorRtree(d) { - return { minX: d.loc[0], minY: d.loc[1], maxX: d.loc[0], maxY: d.loc[1], data: d }; + return { minX: d.loc[0], minY: d.loc[1], maxX: d.loc[0], maxY: d.loc[1], data: d }; } - // replace or remove error from rtree function updateRtree(item, replace) { - _erCache.rtree.remove(item, function isEql(a, b) { - return a.data.id === b.data.id; - }); + _erCache.rtree.remove(item, (a, b) => a.data.id === b.data.id); - if (replace) { - _erCache.rtree.insert(item); - } + if (replace) { + _erCache.rtree.insert(item); + } } function linkEntity(d) { - return '' + d + ''; + return `${d}`; } // Errors shouldn't obscure eachother -function preventCoincident(loc, bumpUp) { - var coincident = false; - do { - // first time, move marker up. after that, move marker right. - var delta = coincident ? [0.00001, 0] : (bumpUp ? [0, 0.00001] : [0, 0]); - loc = geoVecAdd(loc, delta); - var bbox = geoExtent(loc).bbox(); - coincident = _erCache.rtree.search(bbox).length; - } while (coincident); - - return loc; +function preventCoincident(loc) { + let coincident = false; + do { + // first time, move marker up. after that, move marker right. + let delta = coincident ? [0.00001, 0] : [0, 0.00001]; + loc = geoVecAdd(loc, delta); + let bbox = geoExtent(loc).bbox(); + coincident = _erCache.rtree.search(bbox).length; + } while (coincident); + + return loc; } export default { - init: function() { - if (!_erCache) { - this.reset(); - } - - this.event = utilRebind(this, dispatch, 'on'); - }, - - reset: function() { - if (_erCache) { - Object.values(_erCache.inflightTile).forEach(abortRequest); - } - _erCache = { - data: {}, - loadedTile: {}, - inflightTile: {}, - inflightPost: {}, - closed: {}, - rtree: new RBush() - }; - }, - - loadErrors: function(projection) { - var params = { - item: services.osmose.items.join() // only interested in certain errors - }; - - // determine the needed tiles to cover the view - var tiles = tiler - .zoomExtent([_erZoom, _erZoom]) - .getTiles(projection); - - // abort inflight requests that are no longer needed - abortUnwantedRequests(_erCache, tiles); - - // issue new requests.. - tiles.forEach(function(tile) { - if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; - - var lang = 'en'; // todo: may want to use provided translations - var path = [tile.xyz[2], tile.xyz[0], tile.xyz[1]].join('/'); - var url = _osmoseUrlRoot + lang + '/map/issues/' + path + '.json?' + utilQsString(params); - - var controller = new AbortController(); - _erCache.inflightTile[tile.id] = controller; - - d3_json(url, { signal: controller.signal }) - .then(function(data) { - delete _erCache.inflightTile[tile.id]; - _erCache.loadedTile[tile.id] = true; - - if (data.features) { - data.features.forEach(function(issue) { - var loc = issue.geometry.coordinates; // lon, lat - var props = issue.properties; - // Item is the type of error, w/ class tells us the sub-type - var type = [props.item, props.class].join('-'); - - // Filter out unsupported error types (some are too specific or advanced) - if (type in services.osmose.errorTypes) { - loc = preventCoincident(loc, true); - - var d = new qaError({ - // Info required for every error - loc: loc, - service: 'osmose', - error_type: type, - // Extra details needed for this service - identifier: props.issue_id, // needed to query and update the error - item: props.item, // category of the issue for styling - class: props.class - }); - - // Special handling for some error types - // Setting elems here prevents UI error detail requests - switch (d.item) { - case 8300: - case 8360: - mapillaryError(d); - break; - } - - _erCache.data[d.id] = d; - _erCache.rtree.insert(encodeErrorRtree(d)); - } - }); - } - - dispatch.call('loaded'); - }) - .catch(function() { - delete _erCache.inflightTile[tile.id]; - _erCache.loadedTile[tile.id] = true; - }); - }); - - function mapillaryError(d) { - // Parts only exists for these error types - var parts = dataEn.QA.osmose.error_types[d.item].parts; - d.replacements = [parts[d.class]]; - d.elems = []; - } - }, - - loadErrorDetail: function(d, callback) { - // Error details only need to be fetched once - if (d.elems !== undefined) { - if (callback) callback(null, d); - return; - } - - var url = _osmoseUrlRoot + 'en/api/0.3beta/issue/' + d.identifier; + init() { + if (!_erCache) { + this.reset(); + } - var that = this; - d3_json(url) - .then(function(data) { - // Associated elements used for highlighting - // Assign directly for immediate use in the callback - d.elems = data.elems.map(function(e) { - return e.type.substring(0,1) + e.id; - }); + this.event = utilRebind(this, dispatch, 'on'); + }, - // Element links used in the error description - d.replacements = d.elems.map(function(i) { - return linkEntity(i); + reset() { + if (_erCache) { + Object.values(_erCache.inflightTile).forEach(abortRequest); + } + _erCache = { + data: {}, + loadedTile: {}, + inflightTile: {}, + inflightPost: {}, + closed: {}, + rtree: new RBush() + }; + }, + + loadErrors(projection) { + let params = { + // Tiles return a maximum # of errors + // So we want to filter our request for only types iD supports + item: services.osmose.items.join() + }; + + // determine the needed tiles to cover the view + let tiles = tiler + .zoomExtent([_erZoom, _erZoom]) + .getTiles(projection); + + // abort inflight requests that are no longer needed + abortUnwantedRequests(_erCache, tiles); + + // issue new requests.. + tiles.forEach(tile => { + if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; + + let lang = 'en'; // todo: may want to use provided translations + let [ x, y, z ] = tile.xyz; + let url = _osmoseUrlRoot + `${lang}/map/issues/${z}/${x}/${y}.json?` + utilQsString(params); + + let controller = new AbortController(); + _erCache.inflightTile[tile.id] = controller; + + d3_json(url, { signal: controller.signal }) + .then(data => { + delete _erCache.inflightTile[tile.id]; + _erCache.loadedTile[tile.id] = true; + + if (data.features) { + data.features.forEach(issue => { + const { item, class: error_class, issue_id: identifier } = issue.properties; + // Item is the type of error, w/ class tells us the sub-type + const error_type = [item, error_class].join('-'); + + // Filter out unsupported error types (some are too specific or advanced) + if (error_type in services.osmose.errorTypes) { + let loc = issue.geometry.coordinates; // lon, lat + loc = preventCoincident(loc); + + let d = new qaError({ + // Info required for every error + loc, + service: 'osmose', + error_type, + // Extra details needed for this service + identifier, // needed to query and update the error + item // category of the issue for styling }); // Special handling for some error types + // Setting elems here prevents UI error detail requests switch (d.item) { - case 3040: - d.replacements.push(/Bad value for (.+)/i - .exec(data.subtitle)[1] - ); - break; + case 8300: + case 8360: { + // Parts only exists for these error types + let { parts } = dataEn.QA.osmose.error_types[d.item]; + d.replacements = [parts[error_class]]; + d.elems = []; + break; + } } - that.replaceError(d); - if (callback) callback(null, d); - }) - .catch(function(err) { - if (callback) callback(err.message); + _erCache.data[d.id] = d; + _erCache.rtree.insert(encodeErrorRtree(d)); + } }); - }, - - postUpdate: function(d, callback) { - if (_erCache.inflightPost[d.id]) { - return callback({ message: 'Error update already inflight', status: -2 }, d); - } + } - var that = this; - - // UI sets the status to either '/done' or '/false' - var url = _osmoseUrlRoot + 'en/api/0.3beta/issue/' + d.identifier + d.newStatus; - - var controller = new AbortController(); - _erCache.inflightPost[d.id] = controller; - - fetch(url, { signal: controller.signal }) - .then(function() { - delete _erCache.inflightPost[d.id]; + dispatch.call('loaded'); + }) + .catch(() => { + delete _erCache.inflightTile[tile.id]; + _erCache.loadedTile[tile.id] = true; + }); + }); + }, - that.removeError(d); - if (d.newStatus === '/done') { - // No error identifier, so we give a count of each category - if (!(d.item in _erCache.closed)) { - _erCache.closed[d.item] = 0; - } - _erCache.closed[d.item] += 1; - } - if (callback) callback(null, d); - }) - .catch(function(err) { - delete _erCache.inflightPost[d.id]; - if (callback) callback(err.message); - }); - }, + loadErrorDetail(d, callback) { + // Error details only need to be fetched once + if (d.elems !== undefined) { + if (callback) callback(null, d); + return; + } + let url = _osmoseUrlRoot + `en/api/0.3beta/issue/${d.identifier}`; - // get all cached errors covering the viewport - getErrors: function(projection) { - var viewport = projection.clipExtent(); - var min = [viewport[0][0], viewport[1][1]]; - var max = [viewport[1][0], viewport[0][1]]; - var bbox = geoExtent(projection.invert(min), projection.invert(max)).bbox(); + d3_json(url) + .then(data => { + // Associated elements used for highlighting + // Assign directly for immediate use in the callback + d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); - return _erCache.rtree.search(bbox).map(function(d) { - return d.data; - }); - }, + // Element links used in the error description + d.replacements = d.elems.map(linkEntity); - // get a single error from the cache - getError: function(id) { - return _erCache.data[id]; - }, + // Special handling for some error types + switch (d.item) { + case 3040: { + let [, key_value] = /Bad value for (.+)/i.exec(data.subtitle); + d.replacements.push(key_value); + break; + } + } - // replace a single error in the cache - replaceError: function(error) { - if (!(error instanceof qaError) || !error.id) return; + this.replaceError(d); + if (callback) callback(null, d); + }) + .catch(err => { + if (callback) callback(err.message); + }); + }, + + postUpdate(d, callback) { + if (_erCache.inflightPost[d.id]) { + return callback({ message: 'Error update already inflight', status: -2 }, d); + } - _erCache.data[error.id] = error; - updateRtree(encodeErrorRtree(error), true); // true = replace - return error; - }, + // UI sets the status to either 'done' or 'false' + let url = _osmoseUrlRoot + `en/api/0.3beta/issue/${d.identifier}/${d.newStatus}`; - // remove a single error from the cache - removeError: function(error) { - if (!(error instanceof qaError) || !error.id) return; + let controller = new AbortController(); + _erCache.inflightPost[d.id] = controller; - delete _erCache.data[error.id]; - updateRtree(encodeErrorRtree(error), false); // false = remove - }, + fetch(url, { signal: controller.signal }) + .then(() => { + delete _erCache.inflightPost[d.id]; - // Used to populate `closed:osmose:*` changeset tags - getClosedCounts: function() { - return _erCache.closed; - } -}; \ No newline at end of file + this.removeError(d); + if (d.newStatus === 'done') { + // No error identifier, so we give a count of each category + if (!(d.item in _erCache.closed)) { + _erCache.closed[d.item] = 0; + } + _erCache.closed[d.item] += 1; + } + if (callback) callback(null, d); + }) + .catch(err => { + delete _erCache.inflightPost[d.id]; + if (callback) callback(err.message); + }); + }, + + + // get all cached errors covering the viewport + getErrors(projection) { + let viewport = projection.clipExtent(); + let min = [viewport[0][0], viewport[1][1]]; + let max = [viewport[1][0], viewport[0][1]]; + let bbox = geoExtent(projection.invert(min), projection.invert(max)).bbox(); + + return _erCache.rtree.search(bbox).map(d => { + return d.data; + }); + }, + + // get a single error from the cache + getError(id) { + return _erCache.data[id]; + }, + + // replace a single error in the cache + replaceError(error) { + if (!(error instanceof qaError) || !error.id) return; + + _erCache.data[error.id] = error; + updateRtree(encodeErrorRtree(error), true); // true = replace + return error; + }, + + // remove a single error from the cache + removeError(error) { + if (!(error instanceof qaError) || !error.id) return; + + delete _erCache.data[error.id]; + updateRtree(encodeErrorRtree(error), false); // false = remove + }, + + // Used to populate `closed:osmose:*` changeset tags + getClosedCounts() { + return _erCache.closed; + } +}; diff --git a/modules/ui/osmose_editor.js b/modules/ui/osmose_editor.js index 38cbc42fae..4571eeb56a 100644 --- a/modules/ui/osmose_editor.js +++ b/modules/ui/osmose_editor.js @@ -136,7 +136,7 @@ export function uiOsmoseEditor(context) { this.blur(); // avoid keeping focus on the button - #4641 var errorService = services.osmose; if (errorService) { - d.newStatus = '/done'; + d.newStatus = 'done'; errorService.postUpdate(d, function(err, error) { dispatch.call('change', error); }); @@ -151,7 +151,7 @@ export function uiOsmoseEditor(context) { this.blur(); // avoid keeping focus on the button - #4641 var errorService = services.osmose; if (errorService) { - d.newStatus = '/false'; + d.newStatus = 'false'; errorService.postUpdate(d, function(err, error) { dispatch.call('change', error); }); From cc3039033f541ed25bfab492abb9934e1cbe02ac Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 30 Dec 2019 23:03:43 +0000 Subject: [PATCH 15/38] Support more subtypes of osmose error type 8300 --- data/core.yaml | 2 ++ data/qa_errors.json | 54 ++++++++++++++++++++++++++++++++++++++ dist/locales/en.json | 4 ++- modules/services/osmose.js | 13 ++++++--- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 44f12d4d63..027b6769bf 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -888,10 +888,12 @@ en: title: 'Traffic Sign Suggestion' description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' parts: + 1: 'speed limit' 20: 'max height limit' 21: 'max weight limit' 32: 'roundabout' 34: 'set of speed bumps' + 39: 'living street' 8360: title: 'Missing Street Object' description: 'Object detection by Mapillary suggests there may be an unmapped {0} nearby.' diff --git a/data/qa_errors.json b/data/qa_errors.json index ddf8e35070..5bfeff88aa 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -111,6 +111,57 @@ "7040-6": { "icon": "temaki-power" }, + "8300-1": { + "icon": "fas-tachometer-alt" + }, + "8300-2": { + "icon": "fas-tachometer-alt" + }, + "8300-3": { + "icon": "fas-tachometer-alt" + }, + "8300-4": { + "icon": "fas-tachometer-alt" + }, + "8300-5": { + "icon": "fas-tachometer-alt" + }, + "8300-6": { + "icon": "fas-tachometer-alt" + }, + "8300-7": { + "icon": "fas-tachometer-alt" + }, + "8300-8": { + "icon": "fas-tachometer-alt" + }, + "8300-9": { + "icon": "fas-tachometer-alt" + }, + "8300-10": { + "icon": "fas-tachometer-alt" + }, + "8300-11": { + "icon": "fas-tachometer-alt" + }, + "8300-12": { + "icon": "fas-tachometer-alt" + }, + "8300-13": { + "icon": "fas-tachometer-alt" + }, + "8300-14": { + "icon": "fas-tachometer-alt" + }, + "8300-15": { + "icon": "fas-tachometer-alt" + }, + "8300-16": { + "icon": "fas-tachometer-alt" + }, + "8300-17": { + "icon": "fas-tachometer-alt" + }, "8300-20": { "icon": "temaki-height_restrictor" }, @@ -123,6 +174,9 @@ "8300-34": { "icon": "temaki-diamond" }, + "8300-39": { + "icon": "temaki-pedestrian" + }, "8360-1": { "icon": "temaki-bench" }, diff --git a/dist/locales/en.json b/dist/locales/en.json index 60ff1331ee..e1434169fd 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1062,10 +1062,12 @@ "title": "Traffic Sign Suggestion", "description": "Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.", "parts": { + "1": "speed limit", "20": "max height limit", "21": "max weight limit", "32": "roundabout", - "34": "set of speed bumps" + "34": "set of speed bumps", + "39": "living street" } }, "8360": { diff --git a/modules/services/osmose.js b/modules/services/osmose.js index d42994e5ac..74648053ec 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -140,13 +140,20 @@ export default { }); // Special handling for some error types - // Setting elems here prevents UI error detail requests switch (d.item) { case 8300: case 8360: { // Parts only exists for these error types let { parts } = dataEn.QA.osmose.error_types[d.item]; - d.replacements = [parts[error_class]]; + let k = error_class; + + // First 17 classes are all speed limits + if (item === 8300 && error_class <= 17) { + k = 1; + } + + // Setting elems here prevents UI error detail requests + d.replacements = [parts[k]]; d.elems = []; break; } @@ -272,4 +279,4 @@ export default { getClosedCounts() { return _erCache.closed; } -}; +}; \ No newline at end of file From c13eb15691f49adca6ca34ffdef18175bf11b8e3 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 31 Dec 2019 18:16:36 +0000 Subject: [PATCH 16/38] Add support for more Osmose error types - Unconnected level crossings - Unconnected highway features/barries - Unexpected punctuation/symbols in tag values - Unbalanced parentheses/quotes in name values - Invalid date formats - Access tagging that permits all transport modes - Elements with a "name" tag and no main tagging - Relations missing a "type" value --- css/65_data.css | 14 ++++++- data/core.yaml | 32 ++++++++++++++- data/qa_errors.json | 61 +++++++++++++++++++++++++++- dist/locales/en.json | 42 ++++++++++++++++++- modules/services/osmose.js | 21 +++++++--- svg/fontawesome/fas-calendar-alt.svg | 1 + svg/fontawesome/fas-question.svg | 1 + svg/fontawesome/fas-shapes.svg | 1 + svg/fontawesome/fas-share-alt.svg | 1 + svg/fontawesome/fas-tint-slash.svg | 1 + 10 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 svg/fontawesome/fas-calendar-alt.svg create mode 100644 svg/fontawesome/fas-question.svg create mode 100644 svg/fontawesome/fas-shapes.svg create mode 100644 svg/fontawesome/fas-share-alt.svg create mode 100644 svg/fontawesome/fas-tint-slash.svg diff --git a/css/65_data.css b/css/65_data.css index 146adde24b..181d656faf 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -165,12 +165,19 @@ .osmose.category-1070, .osmose.category-1080, .osmose.category-1150, +.osmose.category-1190, .osmose.category-1280 { color: #DCB000; } +.osmose.category-2110 { + color: #2CDCDC; +} + .osmose.category-3040, +.osmose.category-3090, .osmose.category-3161, +.osmose.category-3220, .osmose.category-3250 { color: #2D9359; } @@ -179,7 +186,12 @@ color: #F2F230; } -.osmose.category-7040 { +.osmose.category-5070 { + color: #EC0000; +} + +.osmose.category-7040, +.osmose.category-7090 { color: #9F16B4; } diff --git a/data/core.yaml b/data/core.yaml index 027b6769bf..c4c0b24219 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -860,21 +860,45 @@ en: 1150: title: 'Overlapping Similar Areas' description: '{0} and {1} overlap.' + 1190: + title: 'Approximate Way' + description: '{0} looks like it could be mapped more accurately.' 1280-1: title: 'Incorrect Speed Camera' description: '{0} should be on the highway or part of an "enforcement" relation.' + 2110-21101: + title: 'Unknown Named Object' + description: '{0} has a name but no main tag.' + 2110-21102: + title: 'Missing Relation Type' + description: '{0} is missing a "type" tag.' 3040-3040: title: 'Unusual Tagging' - description: '{0} has a non-conventional tag value: {1}.' + description: '{0} has a non-conventional tag value: "{1}".' + 3090-3090: + title: 'Invalid Date' + description: '{0} is tagged with an invalid date value: "{1}".' 3161: title: 'Missing Parking Access' description: 'There is no highway leading to {0}.' + 3220: + title: 'All Access Allowed' + description: '{0} has an "access=yes" or "access=permissive" tag which permits use by all transport modes (ski, horse, hazmat, etc.).' 3250-32501: title: 'Invalid Opening Hours' description: '{0} has an invalid value for the "opening_hours" tag.' 4080: title: 'Object Mapped Twice' description: '{0} and {1} both appear to represent the same object.' + 5070-50703: + title: 'Unexpected Symbol' + description: '{0} has an unexpected symbol, {2}, in the "{1}" tag.' + 5070-50704: + title: 'Unbalanced Punctuation' + description: '{0} has an unbalanced {1} character in its name.' + 5070-50705: + title: 'Unexpected Punctuation' + description: '{0} has an unexpected {1} character in its name.' 7040: title: 'Unfinished Power Line' description: 'A power line appears to be unfinished at {0}.' @@ -884,6 +908,12 @@ en: 7040-4: title: 'Extra Power Line Node' description: 'Power lines should only have nodes at supports and {0} is not tagged as a pole or tower.' + 7090-1: + title: 'Disconnected Level Crossing' + description: '{0} is a level crossing at a non-junction node.' + 7090-3: + title: 'Disconnected Highway Feature' + description: '{0} is highway feature or barrier with no connected highway.' 8300: title: 'Traffic Sign Suggestion' description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' diff --git a/data/qa_errors.json b/data/qa_errors.json index 5bfeff88aa..08ecf6c736 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -34,7 +34,27 @@ } }, "osmose": { - "items": ["0", "1040", "1050", "1070", "1150", "1280", "3040", "3161", "3250", "4080", "7040", "8300", "8360"], + "items": [ + "0", + "1040", + "1050", + "1070", + "1150", + "1190", + "1280", + "2110", + "3040", + "3090", + "3161", + "3220", + "3250", + "4080", + "7040", + "7090", + "5070", + "8300", + "8360" + ], "errorTypes": { "0-1": { "icon": "maki-home" @@ -75,18 +95,42 @@ "1150-3": { "icon": "far-clone" }, + "1190-10": { + "icon": "fas-share-alt" + }, + "1190-20": { + "icon": "fas-share-alt" + }, + "1190-30": { + "icon": "fas-share-alt" + }, "1280-1": { "icon": "maki-attraction" }, + "2110-21101": { + "icon": "temaki-plaque" + }, + "2110-21102": { + "icon": "fas-shapes" + }, "3040-3040": { "icon": "far-times-circle" }, + "3090-3090": { + "icon": "fas-calendar-alt" + }, "3161-1": { "icon": "maki-parking" }, "3161-2": { "icon": "maki-parking" }, + "3220-32200": { + "icon": "maki-roadblock" + }, + "3220-32201": { + "icon": "maki-roadblock" + }, "3250-32501": { "icon": "maki-watch" }, @@ -99,6 +143,15 @@ "4080-3": { "icon": "far-dot-circle" }, + "5070-50703": { + "icon": "fas-tint-slash" + }, + "5070-50704": { + "icon": "fas-code" + }, + "5070-50705": { + "icon": "fas-question" + }, "7040-1": { "icon": "temaki-power_tower" }, @@ -111,6 +164,12 @@ "7040-6": { "icon": "temaki-power" }, + "7090-1": { + "icon": "maki-rail" + }, + "7090-3": { + "icon": "maki-circle" + }, "8300-1": { "icon": "fas-tachometer-alt" }, diff --git a/dist/locales/en.json b/dist/locales/en.json index e1434169fd..37d9a553c0 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1046,10 +1046,18 @@ "title": "Overlapping Similar Areas", "description": "{0} and {1} overlap." }, + "1190": { + "title": "Approximate Way", + "description": "{0} looks like it could be mapped more accurately." + }, "3161": { "title": "Missing Parking Access", "description": "There is no highway leading to {0}." }, + "3220": { + "title": "All Access Allowed", + "description": "{0} has an \"access=yes\" or \"access=permissive\" tag which permits use by all transport modes (ski, horse, hazmat, etc.)." + }, "4080": { "title": "Object Mapped Twice", "description": "{0} and {1} both appear to represent the same object." @@ -1114,14 +1122,38 @@ "title": "Incorrect Speed Camera", "description": "{0} should be on the highway or part of an \"enforcement\" relation." }, + "2110-21101": { + "title": "Unknown Named Object", + "description": "{0} has a name but no main tag." + }, + "2110-21102": { + "title": "Missing Relation Type", + "description": "{0} is missing a \"type\" tag." + }, "3040-3040": { "title": "Unusual Tagging", - "description": "{0} has a non-conventional tag value: {1}." + "description": "{0} has a non-conventional tag value: \"{1}\"." + }, + "3090-3090": { + "title": "Invalid Date", + "description": "{0} is tagged with an invalid date value: \"{1}\"." }, "3250-32501": { "title": "Invalid Opening Hours", "description": "{0} has an invalid value for the \"opening_hours\" tag." }, + "5070-50703": { + "title": "Unexpected Symbol", + "description": "{0} has an unexpected symbol, {2}, in the \"{1}\" tag." + }, + "5070-50704": { + "title": "Unbalanced Punctuation", + "description": "{0} has an unbalanced {1} character in its name." + }, + "5070-50705": { + "title": "Unexpected Punctuation", + "description": "{0} has an unexpected {1} character in its name." + }, "7040-1": { "title": "Lone Power Support", "description": "{0} is a power tower or pole with no connected power lines." @@ -1129,6 +1161,14 @@ "7040-4": { "title": "Extra Power Line Node", "description": "Power lines should only have nodes at supports and {0} is not tagged as a pole or tower." + }, + "7090-1": { + "title": "Disconnected Level Crossing", + "description": "{0} is a level crossing at a non-junction node." + }, + "7090-3": { + "title": "Disconnected Highway Feature", + "description": "{0} is highway feature or barrier with no connected highway." } } }, diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 74648053ec..6855e471eb 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -174,6 +174,7 @@ export default { }); }, + // todo: &item=1090%2C1110%2C1120%2C3010%2C3020%2C3032%2C3050%2C3060%2C3091%2C3092%2C3150%2C3180%2C3190%2C3200%2C3210%2C4010%2C4030%2C5010%2C5020%2C5030%2C5040%2C5050%2C5080%2C6030%2C9000%2C9001%2C9004%2C9005%2C9010 loadErrorDetail(d, callback) { // Error details only need to be fetched once if (d.elems !== undefined) { @@ -192,12 +193,20 @@ export default { // Element links used in the error description d.replacements = d.elems.map(linkEntity); - // Special handling for some error types - switch (d.item) { - case 3040: { - let [, key_value] = /Bad value for (.+)/i.exec(data.subtitle); - d.replacements.push(key_value); - break; + // Some error types have details in their subtitle + const special = { + '3040-3040': /Bad value for (.+)/i, + '3090-3090': /Incorrect date "(.+)"/i, + '5070-50703': /"(.+)"=".+" unexpected symbol char \(.+, (.+)\)/i, + '5070-50704': /Umbalanced (.+)/i, + '5070-50705': /Unexpected char (.+)/i + }; + if (d.error_type in special) { + let [, ...details] = special[d.error_type].exec(data.subtitle); + d.replacements.push(...details); + + if (d.error_type === '5070-50703') { + d.replacements[2] = String.fromCharCode(details[1]); } } diff --git a/svg/fontawesome/fas-calendar-alt.svg b/svg/fontawesome/fas-calendar-alt.svg new file mode 100644 index 0000000000..872f8a4ee0 --- /dev/null +++ b/svg/fontawesome/fas-calendar-alt.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/svg/fontawesome/fas-question.svg b/svg/fontawesome/fas-question.svg new file mode 100644 index 0000000000..2a3a5c6639 --- /dev/null +++ b/svg/fontawesome/fas-question.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/svg/fontawesome/fas-shapes.svg b/svg/fontawesome/fas-shapes.svg new file mode 100644 index 0000000000..46100668b2 --- /dev/null +++ b/svg/fontawesome/fas-shapes.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/svg/fontawesome/fas-share-alt.svg b/svg/fontawesome/fas-share-alt.svg new file mode 100644 index 0000000000..40bb2fae51 --- /dev/null +++ b/svg/fontawesome/fas-share-alt.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/svg/fontawesome/fas-tint-slash.svg b/svg/fontawesome/fas-tint-slash.svg new file mode 100644 index 0000000000..e424cff658 --- /dev/null +++ b/svg/fontawesome/fas-tint-slash.svg @@ -0,0 +1 @@ + \ No newline at end of file From a53f1c0f67577dfca4c04f9283010b0889352a01 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 31 Dec 2019 19:36:27 +0000 Subject: [PATCH 17/38] Add support for Osmose unnecessary tag error type --- css/65_data.css | 4 ++++ data/core.yaml | 6 ++++++ data/qa_errors.json | 9 ++++++++- dist/locales/en.json | 8 ++++++++ modules/services/osmose.js | 12 ++++++++++-- 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/css/65_data.css b/css/65_data.css index 181d656faf..ff62cb7b1d 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -200,6 +200,10 @@ color: #3DE736; } +.osmose.category-9010 { + color: #976432; +} + /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { pointer-events: none; diff --git a/data/core.yaml b/data/core.yaml index c4c0b24219..fbc6491c9b 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -933,6 +933,12 @@ en: 3: 'surveillance camera' 4: 'fire hydrant' 5: 'set of traffic signals' + 9010-9010001: + title: 'Unnecessary Tag' + description: '{0} is redundantly tagged with "{1}".' + 9010-9010003: + title: 'Descriptive Name' + description: '{0} appears to have a descriptive name: "{1}".' improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/data/qa_errors.json b/data/qa_errors.json index 08ecf6c736..230e565f4a 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -53,7 +53,8 @@ "7090", "5070", "8300", - "8360" + "8360", + "9010" ], "errorTypes": { "0-1": { @@ -250,6 +251,12 @@ }, "8360-5": { "icon": "temaki-traffic_signals" + }, + "9010-9010001": { + "icon": "make-waste-basket" + }, + "9010-9010003": { + "icon": "temaki-plaque" } } } diff --git a/dist/locales/en.json b/dist/locales/en.json index 37d9a553c0..d396d060b3 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1169,6 +1169,14 @@ "7090-3": { "title": "Disconnected Highway Feature", "description": "{0} is highway feature or barrier with no connected highway." + }, + "9010-9010001": { + "title": "Unnecessary Tag", + "description": "{0} is redundantly tagged with \"{1}\"." + }, + "9010-9010003": { + "title": "Descriptive Name", + "description": "{0} appears to have a descriptive name: \"{1}\"." } } }, diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 6855e471eb..71d9ba24b8 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -174,7 +174,7 @@ export default { }); }, - // todo: &item=1090%2C1110%2C1120%2C3010%2C3020%2C3032%2C3050%2C3060%2C3091%2C3092%2C3150%2C3180%2C3190%2C3200%2C3210%2C4010%2C4030%2C5010%2C5020%2C5030%2C5040%2C5050%2C5080%2C6030%2C9000%2C9001%2C9004%2C9005%2C9010 + // todo: &item=1090%2C1110%2C1120%2C3010%2C3020%2C3032%2C3050%2C3060%2C3091%2C3092%2C3150%2C3180%2C3190%2C3200%2C3210%2C4010%2C4030%2C5010%2C5020%2C5030%2C5040%2C5050%2C5080%2C6030%2C9000%2C9001 loadErrorDetail(d, callback) { // Error details only need to be fetched once if (d.elems !== undefined) { @@ -199,7 +199,8 @@ export default { '3090-3090': /Incorrect date "(.+)"/i, '5070-50703': /"(.+)"=".+" unexpected symbol char \(.+, (.+)\)/i, '5070-50704': /Umbalanced (.+)/i, - '5070-50705': /Unexpected char (.+)/i + '5070-50705': /Unexpected char (.+)/i, + '9010-9010003': /(.+)/ }; if (d.error_type in special) { let [, ...details] = special[d.error_type].exec(data.subtitle); @@ -208,6 +209,13 @@ export default { if (d.error_type === '5070-50703') { d.replacements[2] = String.fromCharCode(details[1]); } + } else if (d.error_type === '9010-9010001') { + // This error has a rare subtitle variant + let details = /(.+) is unnecessary/i.exec(data.subtitle); + if (details == null) { + details = /\. Remove (.+)/i.exec(data.subtitle); + } + d.replacements.push(details[1]); } this.replaceError(d); From 406264c37e63c07d35fe9a40947f84e3cd666f48 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 31 Dec 2019 20:16:15 +0000 Subject: [PATCH 18/38] Simplify structure of QA error data --- css/65_data.css | 42 ++--- data/qa_errors.json | 307 ++++++++------------------------ modules/osm/qa_error.js | 11 +- modules/services/osmose.js | 2 +- modules/svg/improveOSM.js | 5 +- modules/svg/osmose.js | 2 +- modules/ui/improveOSM_header.js | 5 +- modules/ui/osmose_header.js | 2 +- scripts/build_data.js | 5 +- 9 files changed, 108 insertions(+), 273 deletions(-) diff --git a/css/65_data.css b/css/65_data.css index ff62cb7b1d..503fca1a71 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -156,51 +156,51 @@ /* Osmose Errors ------------------------------------------------------- */ -.osmose.category-0 { +.osmose.item-0 { color: #BDBDBD; } -.osmose.category-1040, -.osmose.category-1050, -.osmose.category-1070, -.osmose.category-1080, -.osmose.category-1150, -.osmose.category-1190, -.osmose.category-1280 { +.osmose.item-1040, +.osmose.item-1050, +.osmose.item-1070, +.osmose.item-1080, +.osmose.item-1150, +.osmose.item-1190, +.osmose.item-1280 { color: #DCB000; } -.osmose.category-2110 { +.osmose.item-2110 { color: #2CDCDC; } -.osmose.category-3040, -.osmose.category-3090, -.osmose.category-3161, -.osmose.category-3220, -.osmose.category-3250 { +.osmose.item-3040, +.osmose.item-3090, +.osmose.item-3161, +.osmose.item-3220, +.osmose.item-3250 { color: #2D9359; } -.osmose.category-4080 { +.osmose.item-4080 { color: #F2F230; } -.osmose.category-5070 { +.osmose.item-5070 { color: #EC0000; } -.osmose.category-7040, -.osmose.category-7090 { +.osmose.item-7040, +.osmose.item-7090 { color: #9F16B4; } -.osmose.category-8300, -.osmose.category-8360 { +.osmose.item-8300, +.osmose.item-8360 { color: #3DE736; } -.osmose.category-9010 { +.osmose.item-9010 { color: #976432; } diff --git a/data/qa_errors.json b/data/qa_errors.json index 230e565f4a..374d553ea3 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -1,36 +1,13 @@ { "services": { "improveOSM": { - "errorTypes": { - "ow": { - "icon": "fas-long-arrow-alt-right", - "category": "routing" - }, - "mr-both": { - "icon": "maki-car", - "category": "geometry" - }, - "mr-parking": { - "icon": "maki-parking", - "category": "geometry" - }, - "mr-path": { - "icon": "maki-shoe", - "category": "geometry" - }, - "mr-road": { - "icon": "maki-car", - "category": "geometry" - }, - "tr": { - "icon": "temaki-junction", - "category": "routing" - } - } - }, - "keepRight": { - "errorTypes": { - + "errorIcons": { + "ow": "fas-long-arrow-alt-right", + "mr-both": "maki-car", + "mr-parking": "maki-parking", + "mr-path": "maki-shoe", + "mr-road": "maki-car", + "tr": "temaki-junction" } }, "osmose": { @@ -56,208 +33,74 @@ "8360", "9010" ], - "errorTypes": { - "0-1": { - "icon": "maki-home" - }, - "0-2": { - "icon": "maki-home" - }, - "1040-1": { - "icon": "maki-square-stroked" - }, - "1050-1": { - "icon": "maki-circle-stroked" - }, - "1050-1050": { - "icon": "maki-circle-stroked" - }, - "1070-1": { - "icon": "maki-home" - }, - "1070-4": { - "icon": "maki-dam" - }, - "1070-5": { - "icon": "maki-dam" - }, - "1070-8": { - "icon": "maki-cross" - }, - "1070-10": { - "icon": "maki-cross" - }, - "1150-1": { - "icon": "far-clone" - }, - "1150-2": { - "icon": "far-clone" - }, - "1150-3": { - "icon": "far-clone" - }, - "1190-10": { - "icon": "fas-share-alt" - }, - "1190-20": { - "icon": "fas-share-alt" - }, - "1190-30": { - "icon": "fas-share-alt" - }, - "1280-1": { - "icon": "maki-attraction" - }, - "2110-21101": { - "icon": "temaki-plaque" - }, - "2110-21102": { - "icon": "fas-shapes" - }, - "3040-3040": { - "icon": "far-times-circle" - }, - "3090-3090": { - "icon": "fas-calendar-alt" - }, - "3161-1": { - "icon": "maki-parking" - }, - "3161-2": { - "icon": "maki-parking" - }, - "3220-32200": { - "icon": "maki-roadblock" - }, - "3220-32201": { - "icon": "maki-roadblock" - }, - "3250-32501": { - "icon": "maki-watch" - }, - "4080-1": { - "icon": "far-dot-circle" - }, - "4080-2": { - "icon": "far-dot-circle" - }, - "4080-3": { - "icon": "far-dot-circle" - }, - "5070-50703": { - "icon": "fas-tint-slash" - }, - "5070-50704": { - "icon": "fas-code" - }, - "5070-50705": { - "icon": "fas-question" - }, - "7040-1": { - "icon": "temaki-power_tower" - }, - "7040-2": { - "icon": "temaki-power" - }, - "7040-4": { - "icon": "maki-marker" - }, - "7040-6": { - "icon": "temaki-power" - }, - "7090-1": { - "icon": "maki-rail" - }, - "7090-3": { - "icon": "maki-circle" - }, - "8300-1": { - "icon": "fas-tachometer-alt" - }, - "8300-2": { - "icon": "fas-tachometer-alt" - }, - "8300-3": { - "icon": "fas-tachometer-alt" - }, - "8300-4": { - "icon": "fas-tachometer-alt" - }, - "8300-5": { - "icon": "fas-tachometer-alt" - }, - "8300-6": { - "icon": "fas-tachometer-alt" - }, - "8300-7": { - "icon": "fas-tachometer-alt" - }, - "8300-8": { - "icon": "fas-tachometer-alt" - }, - "8300-9": { - "icon": "fas-tachometer-alt" - }, - "8300-10": { - "icon": "fas-tachometer-alt" - }, - "8300-11": { - "icon": "fas-tachometer-alt" - }, - "8300-12": { - "icon": "fas-tachometer-alt" - }, - "8300-13": { - "icon": "fas-tachometer-alt" - }, - "8300-14": { - "icon": "fas-tachometer-alt" - }, - "8300-15": { - "icon": "fas-tachometer-alt" - }, - "8300-16": { - "icon": "fas-tachometer-alt" - }, - "8300-17": { - "icon": "fas-tachometer-alt" - }, - "8300-20": { - "icon": "temaki-height_restrictor" - }, - "8300-21": { - "icon": "fas-weight-hanging" - }, - "8300-32": { - "icon": "maki-circle-stroked" - }, - "8300-34": { - "icon": "temaki-diamond" - }, - "8300-39": { - "icon": "temaki-pedestrian" - }, - "8360-1": { - "icon": "temaki-bench" - }, - "8360-2": { - "icon": "maki-bicycle" - }, - "8360-3": { - "icon": "temaki-security_camera" - }, - "8360-4": { - "icon": "temaki-fire_hydrant" - }, - "8360-5": { - "icon": "temaki-traffic_signals" - }, - "9010-9010001": { - "icon": "make-waste-basket" - }, - "9010-9010003": { - "icon": "temaki-plaque" - } + "errorIcons": { + "0-1": "maki-home", + "0-2": "maki-home", + "1040-1": "maki-square-stroked", + "1050-1": "maki-circle-stroked", + "1050-1050": "maki-circle-stroked", + "1070-1": "maki-home", + "1070-4": "maki-dam", + "1070-5": "maki-dam", + "1070-8": "maki-cross", + "1070-10": "maki-cross", + "1150-1": "far-clone", + "1150-2": "far-clone", + "1150-3": "far-clone", + "1190-10": "fas-share-alt", + "1190-20": "fas-share-alt", + "1190-30": "fas-share-alt", + "1280-1": "maki-attraction", + "2110-21101": "temaki-plaque", + "2110-21102": "fas-shapes", + "3040-3040": "far-times-circle", + "3090-3090": "fas-calendar-alt", + "3161-1": "maki-parking", + "3161-2": "maki-parking", + "3220-32200": "maki-roadblock", + "3220-32201": "maki-roadblock", + "3250-32501": "maki-watch", + "4080-1": "far-dot-circle", + "4080-2": "far-dot-circle", + "4080-3": "far-dot-circle", + "5070-50703": "fas-tint-slash", + "5070-50704": "fas-code", + "5070-50705": "fas-question", + "7040-1": "temaki-power_tower", + "7040-2": "temaki-power", + "7040-4": "maki-marker", + "7040-6": "temaki-power", + "7090-1": "maki-rail", + "7090-3": "maki-circle", + "8300-1": "fas-tachometer-alt", + "8300-2": "fas-tachometer-alt", + "8300-3": "fas-tachometer-alt", + "8300-4": "fas-tachometer-alt", + "8300-5": "fas-tachometer-alt", + "8300-6": "fas-tachometer-alt", + "8300-7": "fas-tachometer-alt", + "8300-8": "fas-tachometer-alt", + "8300-9": "fas-tachometer-alt", + "8300-10": "fas-tachometer-alt", + "8300-11": "fas-tachometer-alt", + "8300-12": "fas-tachometer-alt", + "8300-13": "fas-tachometer-alt", + "8300-14": "fas-tachometer-alt", + "8300-15": "fas-tachometer-alt", + "8300-16": "fas-tachometer-alt", + "8300-17": "fas-tachometer-alt", + "8300-20": "temaki-height_restrictor", + "8300-21": "fas-weight-hanging", + "8300-32": "maki-circle-stroked", + "8300-34": "temaki-diamond", + "8300-39": "temaki-pedestrian", + "8360-1": "temaki-bench", + "8360-2": "maki-bicycle", + "8360-3": "temaki-security_camera", + "8360-4": "temaki-fire_hydrant", + "8360-5": "temaki-traffic_signals", + "9010-9010001": "maki-waste-basket", + "9010-9010003": "temaki-plaque" } } } diff --git a/modules/osm/qa_error.js b/modules/osm/qa_error.js index 7e18de2371..d76893d015 100644 --- a/modules/osm/qa_error.js +++ b/modules/osm/qa_error.js @@ -44,13 +44,8 @@ Object.assign(qaError.prototype, { if (this.service && this.error_type) { var serviceInfo = services[this.service]; - if (serviceInfo) { - var errInfo = serviceInfo.errorTypes[this.error_type]; - - if (errInfo) { - this.icon = errInfo.icon; - this.category = errInfo.category; - } + if (serviceInfo && serviceInfo.errorIcons) { + this.icon = serviceInfo.errorIcons[this.error_type]; } } @@ -65,4 +60,4 @@ Object.assign(qaError.prototype, { update: function(attrs) { return qaError(this, attrs); // {v: 1 + (this.v || 0)} } -}); +}); \ No newline at end of file diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 71d9ba24b8..74cb2c4611 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -125,7 +125,7 @@ export default { const error_type = [item, error_class].join('-'); // Filter out unsupported error types (some are too specific or advanced) - if (error_type in services.osmose.errorTypes) { + if (error_type in services.osmose.errorIcons) { let loc = issue.geometry.coordinates; // lon, lat loc = preventCoincident(loc); diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index 1c55402996..9dee085535 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -119,8 +119,7 @@ export function svgImproveOSM(projection, context, dispatch) { 'qa_error', d.service, 'error_id-' + d.id, - 'error_type-' + d.error_type, - 'category-' + d.category + 'error_type-' + d.error_type ].join(' '); }); @@ -258,4 +257,4 @@ export function svgImproveOSM(projection, context, dispatch) { return drawImproveOSM; -} +} \ No newline at end of file diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index bc25d96125..77d54e921a 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -120,7 +120,7 @@ export function svgOsmose(projection, context, dispatch) { d.service, 'error_id-' + d.id, 'error_type-' + d.error_type, - 'category-' + d.item + 'item-' + d.item ].join(' '); }); diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index f9ae383279..bc0fb28b38 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -51,8 +51,7 @@ export function uiImproveOsmHeader() { 'qa_error', d.service, 'error_id-' + d.id, - 'error_type-' + d.error_type, - 'category-' + d.category + 'error_type-' + d.error_type ].join(' '); }); @@ -94,4 +93,4 @@ export function uiImproveOsmHeader() { return improveOsmHeader; -} +} \ No newline at end of file diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js index b66d31980b..c2f0ce1aae 100644 --- a/modules/ui/osmose_header.js +++ b/modules/ui/osmose_header.js @@ -57,7 +57,7 @@ export function uiOsmoseHeader() { d.service, 'error_id-' + d.id, 'error_type-' + d.error_type, - 'category-' + d.item + 'item-' + d.item ].join(' '); }); diff --git a/scripts/build_data.js b/scripts/build_data.js index 02f72b80fc..30e2f49797 100644 --- a/scripts/build_data.js +++ b/scripts/build_data.js @@ -175,10 +175,9 @@ function readQAErrorIcons(faIcons, tnpIcons) { const qa = read('data/qa_errors.json'); for (const service in qa.services) { - for (const error in qa.services[service].errorTypes) { + for (const error in qa.services[service].errorIcons) { const icon = qa.services[service] - .errorTypes[error] - .icon; + .errorIcons[error]; // fontawesome icon, remember for later if (/^fa[srb]-/.test(icon)) { From 9fca611ec1f5fa3af83eb7e603b0246501e2a718 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 31 Dec 2019 22:23:09 +0000 Subject: [PATCH 19/38] Add support for even more Osmose error types - Conflicting tags - Orthographic errors (excess whitespace and all capital names) - Deprecated tagging - Bad use of the "area" tag --- css/65_data.css | 4 ++++ data/core.yaml | 22 ++++++++++++++++++- data/qa_errors.json | 13 +++++++++++ dist/locales/en.json | 31 ++++++++++++++++++++++++++- modules/services/osmose.js | 10 ++++----- svg/fontawesome/fas-sort-alpha-up.svg | 1 + svg/fontawesome/fas-yin-yang.svg | 1 + 7 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 svg/fontawesome/fas-sort-alpha-up.svg create mode 100644 svg/fontawesome/fas-yin-yang.svg diff --git a/css/65_data.css b/css/65_data.css index 503fca1a71..21461c37ac 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -177,15 +177,19 @@ .osmose.item-3040, .osmose.item-3090, .osmose.item-3161, +.osmose.item-3200, .osmose.item-3220, .osmose.item-3250 { color: #2D9359; } +.osmose.item-4010, +.osmose.item-4030, .osmose.item-4080 { color: #F2F230; } +.osmose.item-5010, .osmose.item-5070 { color: #EC0000; } diff --git a/data/core.yaml b/data/core.yaml index fbc6491c9b..67a7f69128 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -868,7 +868,7 @@ en: description: '{0} should be on the highway or part of an "enforcement" relation.' 2110-21101: title: 'Unknown Named Object' - description: '{0} has a name but no main tag.' + description: '{0} has a name but no feature tag.' 2110-21102: title: 'Missing Relation Type' description: '{0} is missing a "type" tag.' @@ -881,15 +881,35 @@ en: 3161: title: 'Missing Parking Access' description: 'There is no highway leading to {0}.' + 3200: + title: 'Invalid Area Tagging' + 3200-32001: + description: '{0} is tagged "area=yes", but this feature is an area by default.' + 3200-32002: + description: '{0} has an "area" tag, but no feature tag.' + 3200-32003: + description: '{0} is tagged "area=no", but this feature already isn''t an area.' 3220: title: 'All Access Allowed' description: '{0} has an "access=yes" or "access=permissive" tag which permits use by all transport modes (ski, horse, hazmat, etc.).' 3250-32501: title: 'Invalid Opening Hours' description: '{0} has an invalid value for the "opening_hours" tag.' + 4010: + title: 'Deprecated Tagging' + description: '{0} uses tag "{1}" which is deprecated in favour of "{2}".' + 4030-900: + title: 'Conflicting Tags' + description: '{0} is tagged with both "{1}" and "{2}".' 4080: title: 'Object Mapped Twice' description: '{0} and {1} both appear to represent the same object.' + 5010: + title: 'Orthographic Error' + 5010-803: + description: 'The "name" of {0} is written in all capitals.' + 5010-903: + description: 'The "name" of {0} has excessive space characters.' 5070-50703: title: 'Unexpected Symbol' description: '{0} has an unexpected symbol, {2}, in the "{1}" tag.' diff --git a/data/qa_errors.json b/data/qa_errors.json index 374d553ea3..d73d988168 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -23,11 +23,16 @@ "3040", "3090", "3161", + "3200", "3220", "3250", + "4010", + "4030", "4080", + "5010", "7040", "7090", + "5010", "5070", "8300", "8360", @@ -57,12 +62,20 @@ "3090-3090": "fas-calendar-alt", "3161-1": "maki-parking", "3161-2": "maki-parking", + "3200-32001": "fas-vector-square", + "3200-32002": "fas-vector-square", + "3200-32003": "fas-vector-square", "3220-32200": "maki-roadblock", "3220-32201": "maki-roadblock", "3250-32501": "maki-watch", + "4010-4010": "maki-waste-basket", + "4010-40102": "maki-waste-basket", + "4030-900": "fas-yin-yang", "4080-1": "far-dot-circle", "4080-2": "far-dot-circle", "4080-3": "far-dot-circle", + "5010-803": "fas-sort-alpha-up", + "5010-903": "fas-rocket", "5070-50703": "fas-tint-slash", "5070-50704": "fas-code", "5070-50705": "fas-question", diff --git a/dist/locales/en.json b/dist/locales/en.json index d396d060b3..1d212bee12 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1054,14 +1054,24 @@ "title": "Missing Parking Access", "description": "There is no highway leading to {0}." }, + "3200": { + "title": "Invalid Area Tagging" + }, "3220": { "title": "All Access Allowed", "description": "{0} has an \"access=yes\" or \"access=permissive\" tag which permits use by all transport modes (ski, horse, hazmat, etc.)." }, + "4010": { + "title": "Deprecated Tagging", + "description": "{0} uses tag \"{1}\" which is deprecated in favour of \"{2}\"." + }, "4080": { "title": "Object Mapped Twice", "description": "{0} and {1} both appear to represent the same object." }, + "5010": { + "title": "Orthographic Error" + }, "7040": { "title": "Unfinished Power Line", "description": "A power line appears to be unfinished at {0}." @@ -1124,7 +1134,7 @@ }, "2110-21101": { "title": "Unknown Named Object", - "description": "{0} has a name but no main tag." + "description": "{0} has a name but no feature tag." }, "2110-21102": { "title": "Missing Relation Type", @@ -1138,10 +1148,29 @@ "title": "Invalid Date", "description": "{0} is tagged with an invalid date value: \"{1}\"." }, + "3200-32001": { + "description": "{0} is tagged \"area=yes\", but this feature is an area by default." + }, + "3200-32002": { + "description": "{0} has an \"area\" tag, but no feature tag." + }, + "3200-32003": { + "description": "{0} is tagged \"area=no\", but this feature already isn't an area." + }, "3250-32501": { "title": "Invalid Opening Hours", "description": "{0} has an invalid value for the \"opening_hours\" tag." }, + "4030-900": { + "title": "Conflicting Tags", + "description": "{0} is tagged with both \"{1}\" and \"{2}\"." + }, + "5010-803": { + "description": "The \"name\" of {0} is written in all capitals." + }, + "5010-903": { + "description": "The \"name\" of {0} has excessive space characters." + }, "5070-50703": { "title": "Unexpected Symbol", "description": "{0} has an unexpected symbol, {2}, in the \"{1}\" tag." diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 74cb2c4611..c0ccbe44c2 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -3,7 +3,7 @@ import RBush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; -import { dataEn } from '../../data'; +import { t } from '../util/locale'; import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -143,8 +143,6 @@ export default { switch (d.item) { case 8300: case 8360: { - // Parts only exists for these error types - let { parts } = dataEn.QA.osmose.error_types[d.item]; let k = error_class; // First 17 classes are all speed limits @@ -153,7 +151,7 @@ export default { } // Setting elems here prevents UI error detail requests - d.replacements = [parts[k]]; + d.replacements = [t(`QA.osmose.error_types.${d.item}.parts.${k}`)]; d.elems = []; break; } @@ -174,7 +172,6 @@ export default { }); }, - // todo: &item=1090%2C1110%2C1120%2C3010%2C3020%2C3032%2C3050%2C3060%2C3091%2C3092%2C3150%2C3180%2C3190%2C3200%2C3210%2C4010%2C4030%2C5010%2C5020%2C5030%2C5040%2C5050%2C5080%2C6030%2C9000%2C9001 loadErrorDetail(d, callback) { // Error details only need to be fetched once if (d.elems !== undefined) { @@ -197,6 +194,9 @@ export default { const special = { '3040-3040': /Bad value for (.+)/i, '3090-3090': /Incorrect date "(.+)"/i, + '4010-4010': /Tag (.+) is deprecated: (.+)/i, + '4010-40102': /Tag (.+) is deprecated: (.+)/i, + '4030-900': /Conflict between tags: (.+), (.+)/i, '5070-50703': /"(.+)"=".+" unexpected symbol char \(.+, (.+)\)/i, '5070-50704': /Umbalanced (.+)/i, '5070-50705': /Unexpected char (.+)/i, diff --git a/svg/fontawesome/fas-sort-alpha-up.svg b/svg/fontawesome/fas-sort-alpha-up.svg new file mode 100644 index 0000000000..8ebf95f8f3 --- /dev/null +++ b/svg/fontawesome/fas-sort-alpha-up.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/svg/fontawesome/fas-yin-yang.svg b/svg/fontawesome/fas-yin-yang.svg new file mode 100644 index 0000000000..5d70c62f43 --- /dev/null +++ b/svg/fontawesome/fas-yin-yang.svg @@ -0,0 +1 @@ + \ No newline at end of file From d2f9278fe283a86e392bfc42201ff2b903bc8812 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 1 Jan 2020 11:56:50 +0000 Subject: [PATCH 20/38] Port changeset tag improvement to ImproveOSM QA Also marked the `closed:` tags as readonly since these are filled in automatically. - Gives a count of each error type closed instead of making a list of joined coordinates which quickly fills the maximum value length. - This is more useful since a changeset already gives the area worked on while these counts give insight into the type of changes made. - Similar to iD own validation tags --- modules/services/improveOSM.js | 14 ++++++++------ modules/ui/commit.js | 19 ++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 71c5f98978..281052a08f 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -436,9 +436,11 @@ export default { } else { that.removeError(d); if (d.newStatus === 'SOLVED') { - // No pretty identifier, so we just use coordinates - var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); - _erCache.closed[key + ':' + closedID] = true; + // No error identifier, so we give a count of each category + if (!(d.error_key in _erCache.closed)) { + _erCache.closed[d.error_key] = 0; + } + _erCache.closed[d.error_key] += 1; } } if (callback) callback(null, d); @@ -486,7 +488,7 @@ export default { }, // Used to populate `closed:improveosm` changeset tag - getClosedIDs: function() { - return Object.keys(_erCache.closed).sort(); + getClosedCounts: function() { + return _erCache.closed; } -}; +}; \ No newline at end of file diff --git a/modules/ui/commit.js b/modules/ui/commit.js index b2a764a88f..dc2af6da2a 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -25,7 +25,11 @@ var readOnlyTags = [ /^host$/, /^locale$/, /^warnings:/, - /^resolved:/ + /^resolved:/, + /^closed:note$/, + /^closed:keepright$/, + /^closed:improveosm:/, + /^closed:osmose:/ ]; // treat most punctuation (except -, _, +, &) as hashtag delimiters - #4398 @@ -134,6 +138,7 @@ export function uiCommit(context) { // assign tags for closed issues and notes var osmClosed = osm.getClosedIDs(); + var issueType; if (osmClosed.length) { tags['closed:note'] = osmClosed.join(';').substr(0, tagCharLimit); } @@ -144,15 +149,15 @@ export function uiCommit(context) { } } if (services.improveOSM) { - var iOsmClosed = services.improveOSM.getClosedIDs(); - if (iOsmClosed.length) { - tags['closed:improveosm'] = iOsmClosed.join(';').substr(0, tagCharLimit); + var iOsmClosed = services.improveOSM.getClosedCounts(); + for (issueType in iOsmClosed) { + tags['closed:improveosm:' + issueType] = iOsmClosed[issueType].toString().substr(0, tagCharLimit); } } if (services.osmose) { var osmoseClosed = services.osmose.getClosedCounts(); - for (var issueType in osmoseClosed) { - tags['closed:osmose:' + issueType] = osmoseClosed[issueType].toString().substr(0, 255); + for (issueType in osmoseClosed) { + tags['closed:osmose:' + issueType] = osmoseClosed[issueType].toString().substr(0, tagCharLimit); } } @@ -591,4 +596,4 @@ export function uiCommit(context) { return utilRebind(commit, dispatch, 'on'); -} \ No newline at end of file +} From 6310349a5ac847ddca32e4d4720acf8d7971f8cd Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 1 Jan 2020 15:53:22 +0000 Subject: [PATCH 21/38] Use latest Osmose API tiles endpoint --- modules/services/osmose.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index c0ccbe44c2..bc86e74e8a 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -11,7 +11,7 @@ import { services } from '../../data/qa_errors.json'; const tiler = utilTiler(); const dispatch = d3_dispatch('loaded'); -const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/'; +const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta/'; const _erZoom = 14; // This gets reassigned if reset @@ -106,9 +106,8 @@ export default { tiles.forEach(tile => { if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; - let lang = 'en'; // todo: may want to use provided translations let [ x, y, z ] = tile.xyz; - let url = _osmoseUrlRoot + `${lang}/map/issues/${z}/${x}/${y}.json?` + utilQsString(params); + let url = _osmoseUrlRoot + `issues/${z}/${x}/${y}.json?` + utilQsString(params); let controller = new AbortController(); _erCache.inflightTile[tile.id] = controller; @@ -120,7 +119,7 @@ export default { if (data.features) { data.features.forEach(issue => { - const { item, class: error_class, issue_id: identifier } = issue.properties; + const { item, class: error_class, uuid: identifier } = issue.properties; // Item is the type of error, w/ class tells us the sub-type const error_type = [item, error_class].join('-'); @@ -179,7 +178,7 @@ export default { return; } - let url = _osmoseUrlRoot + `en/api/0.3beta/issue/${d.identifier}`; + let url = _osmoseUrlRoot + `issue/${d.identifier}`; d3_json(url) .then(data => { @@ -232,7 +231,7 @@ export default { } // UI sets the status to either 'done' or 'false' - let url = _osmoseUrlRoot + `en/api/0.3beta/issue/${d.identifier}/${d.newStatus}`; + let url = _osmoseUrlRoot + `issue/${d.identifier}/${d.newStatus}`; let controller = new AbortController(); _erCache.inflightPost[d.id] = controller; From 316aca091bf541de5374110200b6713b057d6b4b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 25 Jan 2020 16:19:25 +0000 Subject: [PATCH 22/38] Change icon for Osmose Unnecessary Tag issue --- data/qa_errors.json | 2 +- svg/fontawesome/fas-tags.svg | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 svg/fontawesome/fas-tags.svg diff --git a/data/qa_errors.json b/data/qa_errors.json index d73d988168..b41bbfe6c0 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -112,7 +112,7 @@ "8360-3": "temaki-security_camera", "8360-4": "temaki-fire_hydrant", "8360-5": "temaki-traffic_signals", - "9010-9010001": "maki-waste-basket", + "9010-9010001": "fas-tags", "9010-9010003": "temaki-plaque" } } diff --git a/svg/fontawesome/fas-tags.svg b/svg/fontawesome/fas-tags.svg new file mode 100644 index 0000000000..39e3969bea --- /dev/null +++ b/svg/fontawesome/fas-tags.svg @@ -0,0 +1 @@ + \ No newline at end of file From 7d76950d555ef156763c7681a60c76e769310446 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 25 Jan 2020 16:25:04 +0000 Subject: [PATCH 23/38] Use Osmose translation strings for issue titles This is just step one in the process while we wait for the API to supply further strings and filtering to reduce the amount of string data requested (since we don't need it all). Strings are requested when the layer is first enabled and cached for the current language (plus English as a backup). --- data/core.yaml | 42 --------------------- data/qa_errors.json | 50 ++++++++++++------------- dist/locales/en.json | 47 ----------------------- modules/services/osmose.js | 75 ++++++++++++++++++++++++++++++++++--- modules/svg/osmose.js | 3 +- modules/ui/osmose_header.js | 17 +++------ 6 files changed, 103 insertions(+), 131 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 67a7f69128..df409495b2 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -831,58 +831,35 @@ en: title: Osmose Issue error_types: 0: - title: 'Building Intersection' description: '{0} and {1} intersect.' 0-3: - title: 'Suspiciously Small Building' description: '{0} is a suspisciously small building.' 0-4: - title: 'Gap Between Buildings' description: 'A gap exists between {0} and {1} which looks suspiciously small.' 1040-1: - title: 'Invalid Polygon' description: '{0} intersects itself.' 1050-1: - title: 'Reversed Roundabout' description: '{0} looks like a roundabout mapped in the wrong direction for this location.' 1050-1050: - title: 'Reversed Mini-Roundabout' description: '{0} looks like a mini-roundabout with the incorrect "direction" tag for this location.' 1070: - title: 'Highway intersecting water' description: '{1} intersects with {0}, but there is no junction node, bridge, or tunnel.' - 1070-1: - title: 'Highway intersecting building' - 1070-8: - title: 'Highway intersecting highway' - 1070-10: - title: 'Waterway intersecting waterway' 1150: - title: 'Overlapping Similar Areas' description: '{0} and {1} overlap.' 1190: - title: 'Approximate Way' description: '{0} looks like it could be mapped more accurately.' 1280-1: - title: 'Incorrect Speed Camera' description: '{0} should be on the highway or part of an "enforcement" relation.' 2110-21101: - title: 'Unknown Named Object' description: '{0} has a name but no feature tag.' 2110-21102: - title: 'Missing Relation Type' description: '{0} is missing a "type" tag.' 3040-3040: - title: 'Unusual Tagging' description: '{0} has a non-conventional tag value: "{1}".' 3090-3090: - title: 'Invalid Date' description: '{0} is tagged with an invalid date value: "{1}".' 3161: - title: 'Missing Parking Access' description: 'There is no highway leading to {0}.' - 3200: - title: 'Invalid Area Tagging' 3200-32001: description: '{0} is tagged "area=yes", but this feature is an area by default.' 3200-32002: @@ -890,52 +867,36 @@ en: 3200-32003: description: '{0} is tagged "area=no", but this feature already isn''t an area.' 3220: - title: 'All Access Allowed' description: '{0} has an "access=yes" or "access=permissive" tag which permits use by all transport modes (ski, horse, hazmat, etc.).' 3250-32501: - title: 'Invalid Opening Hours' description: '{0} has an invalid value for the "opening_hours" tag.' 4010: - title: 'Deprecated Tagging' description: '{0} uses tag "{1}" which is deprecated in favour of "{2}".' 4030-900: - title: 'Conflicting Tags' description: '{0} is tagged with both "{1}" and "{2}".' 4080: - title: 'Object Mapped Twice' description: '{0} and {1} both appear to represent the same object.' - 5010: - title: 'Orthographic Error' 5010-803: description: 'The "name" of {0} is written in all capitals.' 5010-903: description: 'The "name" of {0} has excessive space characters.' 5070-50703: - title: 'Unexpected Symbol' description: '{0} has an unexpected symbol, {2}, in the "{1}" tag.' 5070-50704: - title: 'Unbalanced Punctuation' description: '{0} has an unbalanced {1} character in its name.' 5070-50705: - title: 'Unexpected Punctuation' description: '{0} has an unexpected {1} character in its name.' 7040: - title: 'Unfinished Power Line' description: 'A power line appears to be unfinished at {0}.' 7040-1: - title: 'Lone Power Support' description: '{0} is a power tower or pole with no connected power lines.' 7040-4: - title: 'Extra Power Line Node' description: 'Power lines should only have nodes at supports and {0} is not tagged as a pole or tower.' 7090-1: - title: 'Disconnected Level Crossing' description: '{0} is a level crossing at a non-junction node.' 7090-3: - title: 'Disconnected Highway Feature' description: '{0} is highway feature or barrier with no connected highway.' 8300: - title: 'Traffic Sign Suggestion' description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' parts: 1: 'speed limit' @@ -945,7 +906,6 @@ en: 34: 'set of speed bumps' 39: 'living street' 8360: - title: 'Missing Street Object' description: 'Object detection by Mapillary suggests there may be an unmapped {0} nearby.' parts: 1: 'bench' @@ -954,10 +914,8 @@ en: 4: 'fire hydrant' 5: 'set of traffic signals' 9010-9010001: - title: 'Unnecessary Tag' description: '{0} is redundantly tagged with "{1}".' 9010-9010003: - title: 'Descriptive Name' description: '{0} appears to have a descriptive name: "{1}".' improveOSM: title: ImproveOSM Detection diff --git a/data/qa_errors.json b/data/qa_errors.json index b41bbfe6c0..613c6cb5cb 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -12,31 +12,31 @@ }, "osmose": { "items": [ - "0", - "1040", - "1050", - "1070", - "1150", - "1190", - "1280", - "2110", - "3040", - "3090", - "3161", - "3200", - "3220", - "3250", - "4010", - "4030", - "4080", - "5010", - "7040", - "7090", - "5010", - "5070", - "8300", - "8360", - "9010" + 0, + 1040, + 1050, + 1070, + 1150, + 1190, + 1280, + 2110, + 3040, + 3090, + 3161, + 3200, + 3220, + 3250, + 4010, + 4030, + 4080, + 5010, + 7040, + 7090, + 5010, + 5070, + 8300, + 8360, + 9010 ], "errorIcons": { "0-1": "maki-home", diff --git a/dist/locales/en.json b/dist/locales/en.json index 1d212bee12..98dda7275e 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1035,49 +1035,33 @@ "title": "Osmose Issue", "error_types": { "0": { - "title": "Building Intersection", "description": "{0} and {1} intersect." }, "1070": { - "title": "Highway intersecting water", "description": "{1} intersects with {0}, but there is no junction node, bridge, or tunnel." }, "1150": { - "title": "Overlapping Similar Areas", "description": "{0} and {1} overlap." }, "1190": { - "title": "Approximate Way", "description": "{0} looks like it could be mapped more accurately." }, "3161": { - "title": "Missing Parking Access", "description": "There is no highway leading to {0}." }, - "3200": { - "title": "Invalid Area Tagging" - }, "3220": { - "title": "All Access Allowed", "description": "{0} has an \"access=yes\" or \"access=permissive\" tag which permits use by all transport modes (ski, horse, hazmat, etc.)." }, "4010": { - "title": "Deprecated Tagging", "description": "{0} uses tag \"{1}\" which is deprecated in favour of \"{2}\"." }, "4080": { - "title": "Object Mapped Twice", "description": "{0} and {1} both appear to represent the same object." }, - "5010": { - "title": "Orthographic Error" - }, "7040": { - "title": "Unfinished Power Line", "description": "A power line appears to be unfinished at {0}." }, "8300": { - "title": "Traffic Sign Suggestion", "description": "Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.", "parts": { "1": "speed limit", @@ -1089,7 +1073,6 @@ } }, "8360": { - "title": "Missing Street Object", "description": "Object detection by Mapillary suggests there may be an unmapped {0} nearby.", "parts": { "1": "bench", @@ -1100,52 +1083,33 @@ } }, "0-3": { - "title": "Suspiciously Small Building", "description": "{0} is a suspisciously small building." }, "0-4": { - "title": "Gap Between Buildings", "description": "A gap exists between {0} and {1} which looks suspiciously small." }, "1040-1": { - "title": "Invalid Polygon", "description": "{0} intersects itself." }, "1050-1": { - "title": "Reversed Roundabout", "description": "{0} looks like a roundabout mapped in the wrong direction for this location." }, "1050-1050": { - "title": "Reversed Mini-Roundabout", "description": "{0} looks like a mini-roundabout with the incorrect \"direction\" tag for this location." }, - "1070-1": { - "title": "Highway intersecting building" - }, - "1070-8": { - "title": "Highway intersecting highway" - }, - "1070-10": { - "title": "Waterway intersecting waterway" - }, "1280-1": { - "title": "Incorrect Speed Camera", "description": "{0} should be on the highway or part of an \"enforcement\" relation." }, "2110-21101": { - "title": "Unknown Named Object", "description": "{0} has a name but no feature tag." }, "2110-21102": { - "title": "Missing Relation Type", "description": "{0} is missing a \"type\" tag." }, "3040-3040": { - "title": "Unusual Tagging", "description": "{0} has a non-conventional tag value: \"{1}\"." }, "3090-3090": { - "title": "Invalid Date", "description": "{0} is tagged with an invalid date value: \"{1}\"." }, "3200-32001": { @@ -1158,11 +1122,9 @@ "description": "{0} is tagged \"area=no\", but this feature already isn't an area." }, "3250-32501": { - "title": "Invalid Opening Hours", "description": "{0} has an invalid value for the \"opening_hours\" tag." }, "4030-900": { - "title": "Conflicting Tags", "description": "{0} is tagged with both \"{1}\" and \"{2}\"." }, "5010-803": { @@ -1172,39 +1134,30 @@ "description": "The \"name\" of {0} has excessive space characters." }, "5070-50703": { - "title": "Unexpected Symbol", "description": "{0} has an unexpected symbol, {2}, in the \"{1}\" tag." }, "5070-50704": { - "title": "Unbalanced Punctuation", "description": "{0} has an unbalanced {1} character in its name." }, "5070-50705": { - "title": "Unexpected Punctuation", "description": "{0} has an unexpected {1} character in its name." }, "7040-1": { - "title": "Lone Power Support", "description": "{0} is a power tower or pole with no connected power lines." }, "7040-4": { - "title": "Extra Power Line Node", "description": "Power lines should only have nodes at supports and {0} is not tagged as a pole or tower." }, "7090-1": { - "title": "Disconnected Level Crossing", "description": "{0} is a level crossing at a non-junction node." }, "7090-3": { - "title": "Disconnected Highway Feature", "description": "{0} is highway feature or barrier with no connected highway." }, "9010-9010001": { - "title": "Unnecessary Tag", "description": "{0} is redundantly tagged with \"{1}\"." }, "9010-9010003": { - "title": "Descriptive Name", "description": "{0} appears to have a descriptive name: \"{1}\"." } } diff --git a/modules/services/osmose.js b/modules/services/osmose.js index bc86e74e8a..abc9ed70c4 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -3,16 +3,17 @@ import RBush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; -import { t } from '../util/locale'; +import { currentLocale, t } from '../util/locale'; import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; import { utilRebind, utilTiler, utilQsString } from '../util'; -import { services } from '../../data/qa_errors.json'; +import { services as qaServices } from '../../data/qa_errors.json'; const tiler = utilTiler(); const dispatch = d3_dispatch('loaded'); const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta/'; const _erZoom = 14; +const _stringCache = {}; // This gets reassigned if reset let _erCache; @@ -91,7 +92,7 @@ export default { let params = { // Tiles return a maximum # of errors // So we want to filter our request for only types iD supports - item: services.osmose.items.join() + item: qaServices.osmose.items.join() }; // determine the needed tiles to cover the view @@ -121,10 +122,10 @@ export default { data.features.forEach(issue => { const { item, class: error_class, uuid: identifier } = issue.properties; // Item is the type of error, w/ class tells us the sub-type - const error_type = [item, error_class].join('-'); + const error_type = `${item}-${error_class}`; // Filter out unsupported error types (some are too specific or advanced) - if (error_type in services.osmose.errorIcons) { + if (error_type in qaServices.osmose.errorIcons) { let loc = issue.geometry.coordinates; // lon, lat loc = preventCoincident(loc); @@ -225,6 +226,70 @@ export default { }); }, + loadStrings(callback, locale=currentLocale) { + if (locale in _stringCache) { + if (callback) callback(null, _stringCache[locale]); + return; + } + + const langs = { [locale]: true }; + + // Need English strings if not already fetched for fallback values + if (locale != 'en' && !('en' in _stringCache)) { + langs.en = true; + } + + // TODO: Currently all locales are served, in future a param will be available to request specifics + const url = _osmoseUrlRoot + 'items'; + + d3_json(url) + .then(data => { + for (let l in langs) { + _stringCache[l] = {}; + } + + for (let i = 0; i < data.categories.length; i++) { + let cat = data.categories[i]; + + for (let j = 0; j < cat.items.length; j++) { + let item = cat.items[j]; + + // Only need to cache strings for supported error types + // TODO: may be possible to request additional filter by `item` + if (qaServices.osmose.items.indexOf(item.item) !== -1) { + for (let k = 0; k < item.class.length; k++) { + let { class: cl, item: cat } = item.class[k]; + let issueType = `${cat}-${cl}`; + + for (let l in langs) { + _stringCache[l][issueType] = {}; + + let issueStrings = _stringCache[l][issueType]; + + // TODO: Only title is currently served, in future description and other strings will be too + let { title: {[l]: title} } = item.class[k]; + if (title) issueStrings.title = title; + } + } + } + } + } + + if (callback) callback(null, _stringCache[locale]); + }) + .catch(err => { + if (callback) callback(err.message); + }); + }, + + getStrings(issueType, locale=currentLocale) { + const l = (locale in _stringCache) ? _stringCache[locale][issueType] : {}; + const en = ('en' in _stringCache) ? _stringCache['en'][issueType] : {}; + + // Fallback to English if string is untranslated + return Object.assign({}, en, l); + }, + postUpdate(d, callback) { if (_erCache.inflightPost[d.id]) { return callback({ message: 'Error update already inflight', status: -2 }, d); diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index 77d54e921a..70b3c1aa1f 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -63,7 +63,8 @@ export function svgOsmose(projection, context, dispatch) { // Enable the layer. This shows the errors and transitions them to visible. function layerOn() { - editOn(); + // Strings supplied by Osmose fetched before showing layer for first time + getService().loadStrings(editOn); drawLayer .style('opacity', 0) diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js index c2f0ce1aae..36ce763b92 100644 --- a/modules/ui/osmose_header.js +++ b/modules/ui/osmose_header.js @@ -1,5 +1,5 @@ -import { dataEn } from '../../data'; -import { t } from '../util/locale'; +import { services } from '../services'; +import { currentLocale, t } from '../util/locale'; export function uiOsmoseHeader() { @@ -11,15 +11,10 @@ export function uiOsmoseHeader() { if (!d) return unknown; - // Some errors fall back to their category for strings - var i, et; - var keys = [d.error_type, d.item]; - for (i = 0; i < 2; i++) { - et = dataEn.QA.osmose.error_types[keys[i]]; - - if (et && et.title) { - return t('QA.osmose.error_types.' + keys[i] + '.title'); - } + // Issue titles supplied by Osmose + var s = services.osmose.getStrings(d.error_type); + if ('title' in s) { + return s.title; } return unknown; From 5d93d3cace766f78faef36daf1bdbfd78cad723b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sat, 25 Jan 2020 20:26:00 +0000 Subject: [PATCH 24/38] Prepare to use Osmose issue detail strings - Remove translation description strings, will be using supplied translations once the API is updated - Change the way elements and details are presented since they can't be easily integrated into the 3rd party description strings --- css/80_app.css | 6 +- data/core.yaml | 94 ++----------------------- dist/locales/en.json | 133 ++--------------------------------- modules/services/osmose.js | 92 ++++++++++++------------ modules/svg/osmose.js | 2 + modules/ui/osmose_details.js | 68 ++++++++++++------ modules/ui/osmose_header.js | 8 +-- 7 files changed, 112 insertions(+), 291 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index b5b29af136..784db9f465 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2753,6 +2753,10 @@ input.key-trap { [dir='rtl'] .error-details-description-text::first-letter { text-transform: none; /* #5877 */ } +.error-details-description +.error-details-subtitle { + padding-top: 10px; +} .note-save .new-comment-input, .error-save .new-comment-input { @@ -5590,4 +5594,4 @@ li.hide + li.version .badge .tooltip .popover-arrow { } [dir='rtl'] .list-item-photos.list-item-mapillary-map-features .request-data-link { float: left; -} +} \ No newline at end of file diff --git a/data/core.yaml b/data/core.yaml index df409495b2..98d36b3cce 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -829,94 +829,12 @@ en: QA: osmose: title: Osmose Issue - error_types: - 0: - description: '{0} and {1} intersect.' - 0-3: - description: '{0} is a suspisciously small building.' - 0-4: - description: 'A gap exists between {0} and {1} which looks suspiciously small.' - 1040-1: - description: '{0} intersects itself.' - 1050-1: - description: '{0} looks like a roundabout mapped in the wrong direction for this location.' - 1050-1050: - description: '{0} looks like a mini-roundabout with the incorrect "direction" tag for this location.' - 1070: - description: '{1} intersects with {0}, but there is no junction node, bridge, or tunnel.' - 1150: - description: '{0} and {1} overlap.' - 1190: - description: '{0} looks like it could be mapped more accurately.' - 1280-1: - description: '{0} should be on the highway or part of an "enforcement" relation.' - 2110-21101: - description: '{0} has a name but no feature tag.' - 2110-21102: - description: '{0} is missing a "type" tag.' - 3040-3040: - description: '{0} has a non-conventional tag value: "{1}".' - 3090-3090: - description: '{0} is tagged with an invalid date value: "{1}".' - 3161: - description: 'There is no highway leading to {0}.' - 3200-32001: - description: '{0} is tagged "area=yes", but this feature is an area by default.' - 3200-32002: - description: '{0} has an "area" tag, but no feature tag.' - 3200-32003: - description: '{0} is tagged "area=no", but this feature already isn''t an area.' - 3220: - description: '{0} has an "access=yes" or "access=permissive" tag which permits use by all transport modes (ski, horse, hazmat, etc.).' - 3250-32501: - description: '{0} has an invalid value for the "opening_hours" tag.' - 4010: - description: '{0} uses tag "{1}" which is deprecated in favour of "{2}".' - 4030-900: - description: '{0} is tagged with both "{1}" and "{2}".' - 4080: - description: '{0} and {1} both appear to represent the same object.' - 5010-803: - description: 'The "name" of {0} is written in all capitals.' - 5010-903: - description: 'The "name" of {0} has excessive space characters.' - 5070-50703: - description: '{0} has an unexpected symbol, {2}, in the "{1}" tag.' - 5070-50704: - description: '{0} has an unbalanced {1} character in its name.' - 5070-50705: - description: '{0} has an unexpected {1} character in its name.' - 7040: - description: 'A power line appears to be unfinished at {0}.' - 7040-1: - description: '{0} is a power tower or pole with no connected power lines.' - 7040-4: - description: 'Power lines should only have nodes at supports and {0} is not tagged as a pole or tower.' - 7090-1: - description: '{0} is a level crossing at a non-junction node.' - 7090-3: - description: '{0} is highway feature or barrier with no connected highway.' - 8300: - description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' - parts: - 1: 'speed limit' - 20: 'max height limit' - 21: 'max weight limit' - 32: 'roundabout' - 34: 'set of speed bumps' - 39: 'living street' - 8360: - description: 'Object detection by Mapillary suggests there may be an unmapped {0} nearby.' - parts: - 1: 'bench' - 2: 'type of bicycle parking' - 3: 'surveillance camera' - 4: 'fire hydrant' - 5: 'set of traffic signals' - 9010-9010001: - description: '{0} is redundantly tagged with "{1}".' - 9010-9010003: - description: '{0} appears to have a descriptive name: "{1}".' + elems_title: Issue Elements + details: + values: Issue Values + tags: Issue Tags + chars: Issue Characters + sug_tags: Suggested Tags improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/dist/locales/en.json b/dist/locales/en.json index 98dda7275e..819618a992 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1033,133 +1033,12 @@ "QA": { "osmose": { "title": "Osmose Issue", - "error_types": { - "0": { - "description": "{0} and {1} intersect." - }, - "1070": { - "description": "{1} intersects with {0}, but there is no junction node, bridge, or tunnel." - }, - "1150": { - "description": "{0} and {1} overlap." - }, - "1190": { - "description": "{0} looks like it could be mapped more accurately." - }, - "3161": { - "description": "There is no highway leading to {0}." - }, - "3220": { - "description": "{0} has an \"access=yes\" or \"access=permissive\" tag which permits use by all transport modes (ski, horse, hazmat, etc.)." - }, - "4010": { - "description": "{0} uses tag \"{1}\" which is deprecated in favour of \"{2}\"." - }, - "4080": { - "description": "{0} and {1} both appear to represent the same object." - }, - "7040": { - "description": "A power line appears to be unfinished at {0}." - }, - "8300": { - "description": "Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.", - "parts": { - "1": "speed limit", - "20": "max height limit", - "21": "max weight limit", - "32": "roundabout", - "34": "set of speed bumps", - "39": "living street" - } - }, - "8360": { - "description": "Object detection by Mapillary suggests there may be an unmapped {0} nearby.", - "parts": { - "1": "bench", - "2": "type of bicycle parking", - "3": "surveillance camera", - "4": "fire hydrant", - "5": "set of traffic signals" - } - }, - "0-3": { - "description": "{0} is a suspisciously small building." - }, - "0-4": { - "description": "A gap exists between {0} and {1} which looks suspiciously small." - }, - "1040-1": { - "description": "{0} intersects itself." - }, - "1050-1": { - "description": "{0} looks like a roundabout mapped in the wrong direction for this location." - }, - "1050-1050": { - "description": "{0} looks like a mini-roundabout with the incorrect \"direction\" tag for this location." - }, - "1280-1": { - "description": "{0} should be on the highway or part of an \"enforcement\" relation." - }, - "2110-21101": { - "description": "{0} has a name but no feature tag." - }, - "2110-21102": { - "description": "{0} is missing a \"type\" tag." - }, - "3040-3040": { - "description": "{0} has a non-conventional tag value: \"{1}\"." - }, - "3090-3090": { - "description": "{0} is tagged with an invalid date value: \"{1}\"." - }, - "3200-32001": { - "description": "{0} is tagged \"area=yes\", but this feature is an area by default." - }, - "3200-32002": { - "description": "{0} has an \"area\" tag, but no feature tag." - }, - "3200-32003": { - "description": "{0} is tagged \"area=no\", but this feature already isn't an area." - }, - "3250-32501": { - "description": "{0} has an invalid value for the \"opening_hours\" tag." - }, - "4030-900": { - "description": "{0} is tagged with both \"{1}\" and \"{2}\"." - }, - "5010-803": { - "description": "The \"name\" of {0} is written in all capitals." - }, - "5010-903": { - "description": "The \"name\" of {0} has excessive space characters." - }, - "5070-50703": { - "description": "{0} has an unexpected symbol, {2}, in the \"{1}\" tag." - }, - "5070-50704": { - "description": "{0} has an unbalanced {1} character in its name." - }, - "5070-50705": { - "description": "{0} has an unexpected {1} character in its name." - }, - "7040-1": { - "description": "{0} is a power tower or pole with no connected power lines." - }, - "7040-4": { - "description": "Power lines should only have nodes at supports and {0} is not tagged as a pole or tower." - }, - "7090-1": { - "description": "{0} is a level crossing at a non-junction node." - }, - "7090-3": { - "description": "{0} is highway feature or barrier with no connected highway." - }, - "9010-9010001": { - "description": "{0} is redundantly tagged with \"{1}\"." - }, - "9010-9010003": { - "description": "{0} appears to have a descriptive name: \"{1}\"." - } + "elems_title": "Issue Elements", + "details": { + "values": "Issue Values", + "tags": "Issue Tags", + "chars": "Issue Characters", + "sug_tags": "Suggested Tags" } }, "improveOSM": { diff --git a/modules/services/osmose.js b/modules/services/osmose.js index abc9ed70c4..f58d940977 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -3,7 +3,7 @@ import RBush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; -import { currentLocale, t } from '../util/locale'; +import { currentLocale } from '../util/locale'; import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -47,10 +47,6 @@ function updateRtree(item, replace) { } } -function linkEntity(d) { - return `${d}`; -} - // Errors shouldn't obscure eachother function preventCoincident(loc) { let coincident = false; @@ -139,22 +135,9 @@ export default { item // category of the issue for styling }); - // Special handling for some error types - switch (d.item) { - case 8300: - case 8360: { - let k = error_class; - - // First 17 classes are all speed limits - if (item === 8300 && error_class <= 17) { - k = 1; - } - - // Setting elems here prevents UI error detail requests - d.replacements = [t(`QA.osmose.error_types.${d.item}.parts.${k}`)]; - d.elems = []; - break; - } + // Setting elems here prevents UI error detail requests + if (d.item === 8300 || d.item === 8360) { + d.elems = []; } _erCache.data[d.id] = d; @@ -187,35 +170,50 @@ export default { // Assign directly for immediate use in the callback d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); - // Element links used in the error description - d.replacements = d.elems.map(linkEntity); - // Some error types have details in their subtitle const special = { - '3040-3040': /Bad value for (.+)/i, - '3090-3090': /Incorrect date "(.+)"/i, - '4010-4010': /Tag (.+) is deprecated: (.+)/i, - '4010-40102': /Tag (.+) is deprecated: (.+)/i, - '4030-900': /Conflict between tags: (.+), (.+)/i, - '5070-50703': /"(.+)"=".+" unexpected symbol char \(.+, (.+)\)/i, - '5070-50704': /Umbalanced (.+)/i, - '5070-50705': /Unexpected char (.+)/i, - '9010-9010003': /(.+)/ + tags: { + '3040-3040': /Bad tag value: "(.+)"/i, + '4010-4010': /Tag (.+) is deprecated/i, + '4010-40102': /Tag (.+) is deprecated/i, + '4030-900': /Conflict between tags: (.+), (.+)/i, + '5070-50703': /"(.+)"=".+" unexpected/i, + '9010-9010001': /(.+) is unnecessary/i + }, + values: { + '3090-3090': /Incorrect date "(.+)"/i, + '9010-9010003': /(.+)/ + }, + chars: { + '5070-50703': /unexpected symbol char \(.+, (.+)\)/i, + '5070-50704': /Umbalanced (.+)/i, + '5070-50705': /Unexpected char (.+)/i + }, + sug_tags: { + '4010-4010': /Tag .+ is deprecated: (.+)/i, + '4010-40102': /Tag .+ is deprecated: (.+)/i, + } }; - if (d.error_type in special) { - let [, ...details] = special[d.error_type].exec(data.subtitle); - d.replacements.push(...details); + for (let type in special) { + if (d.error_type in special[type]) { + // Destructuring doesn't work if no match returns null, hence [] + let [, ...details] = special[type][d.error_type].exec(data.subtitle) || []; + + if ( + d.error_type === '5070-50703' + && type === 'chars' + && details + ) { + details[0] = String.fromCharCode(details[0]); + } - if (d.error_type === '5070-50703') { - d.replacements[2] = String.fromCharCode(details[1]); - } - } else if (d.error_type === '9010-9010001') { - // This error has a rare subtitle variant - let details = /(.+) is unnecessary/i.exec(data.subtitle); - if (details == null) { - details = /\. Remove (.+)/i.exec(data.subtitle); + // This error has a rare subtitle variant + if (d.error_type === '9010-9010001' && !details) { + [, ...details] = /\. Remove (.+)/i.exec(data.subtitle) || []; + } + + if (details) d[type] = details; } - d.replacements.push(details[1]); } this.replaceError(d); @@ -235,7 +233,7 @@ export default { const langs = { [locale]: true }; // Need English strings if not already fetched for fallback values - if (locale != 'en' && !('en' in _stringCache)) { + if (locale !== 'en' && !('en' in _stringCache)) { langs.en = true; } @@ -284,7 +282,7 @@ export default { getStrings(issueType, locale=currentLocale) { const l = (locale in _stringCache) ? _stringCache[locale][issueType] : {}; - const en = ('en' in _stringCache) ? _stringCache['en'][issueType] : {}; + const en = ('en' in _stringCache) ? _stringCache.en[issueType] : {}; // Fallback to English if string is untranslated return Object.assign({}, en, l); diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index 70b3c1aa1f..409dfa9d3c 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -64,6 +64,8 @@ export function svgOsmose(projection, context, dispatch) { // Enable the layer. This shows the errors and transitions them to visible. function layerOn() { // Strings supplied by Osmose fetched before showing layer for first time + // TODO: If layer is toggled quickly multiple requests are sent + // TODO: No error handling in place getService().loadStrings(editOn); drawLayer diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 756a5dd5a5..2f876f00a2 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -3,7 +3,6 @@ import { select as d3_select } from 'd3-selection'; -import { dataEn } from '../../data'; import { modeSelect } from '../modes/select'; import { t } from '../util/locale'; import { services } from '../services'; @@ -14,23 +13,14 @@ export function uiOsmoseDetails(context) { var _error; - function errorDetail(d) { + function issueDetail(d) { var unknown = t('inspector.unknown'); if (!d) return unknown; - // Some errors fall back to their category for strings - var i, et; - var keys = [d.error_type, d.item]; - for (i = 0; i < 2; i++) { - et = dataEn.QA.osmose.error_types[keys[i]]; - - if (et && et.description) { - return t('QA.osmose.error_types.' + keys[i] + '.description', d.replacements); - } - } - - return unknown; + // Issue description supplied by Osmose + var s = services.osmose.getStrings(d.error_type); + return ('description' in s) ? s.description : unknown; } @@ -58,16 +48,30 @@ export function uiOsmoseDetails(context) { .append('h4') .text(function() { return t('QA.keepRight.detail_description'); }); - services.osmose.loadErrorDetail(_error, function(err, d) { - if (d.elems === undefined) { return; } + descriptionEnter + .append('div') + .attr('class', 'error-details-description-text') + .html(issueDetail); - descriptionEnter - .append('div') - .attr('class', 'error-details-description-text') - .html(errorDetail); + descriptionEnter + .append('h4') + .attr('class', 'error-details-subtitle') + .text(function() { return t('QA.osmose.elems_title'); }); + + var elementList = descriptionEnter + .append('ul') + .attr('class', 'error-details-elements'); - // If there are entity links in the error message.. - descriptionEnter.selectAll('.error_entity_link, .error_object_link') + services.osmose.loadErrorDetail(_error, function(err, d) { + if (d.elems === undefined) return; + + elementList.selectAll('.error_entity_link') + .data(d.elems) + .enter() + .append('li') + .append('a') + .attr('class', 'error_entity_link') + .text(function(d) { return d; }) .each(function() { var link = d3_select(this); var entityID = this.textContent; @@ -117,6 +121,26 @@ export function uiOsmoseDetails(context) { } }); + // Things like keys and values are dynamic details + const special = { tags: true, values: true, chars: true, sug_tags: true }; + for (let type in special) { + if (type in d) { + descriptionEnter + .append('h4') + .attr('class', 'error-details-subtitle') + .text(function() { return t(`QA.osmose.details.${type}`); }); + + descriptionEnter + .append('ul') + .attr('class', 'error-details-list') + .selectAll('li') + .data(d[type]) + .enter() + .append('li') + .html(function(d) { return d; }); + } + } + // Don't hide entities related to this error - #5880 context.features().forceVisible(d.elems); context.map().pan([0,0]); // trigger a redraw diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js index 36ce763b92..c7049591e1 100644 --- a/modules/ui/osmose_header.js +++ b/modules/ui/osmose_header.js @@ -1,5 +1,5 @@ import { services } from '../services'; -import { currentLocale, t } from '../util/locale'; +import { t } from '../util/locale'; export function uiOsmoseHeader() { @@ -13,11 +13,7 @@ export function uiOsmoseHeader() { // Issue titles supplied by Osmose var s = services.osmose.getStrings(d.error_type); - if ('title' in s) { - return s.title; - } - - return unknown; + return ('title' in s) ? s.title : unknown; } From 7a00c0a9746e0646e79424b27f4813c2ec9f38df Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 01:33:11 +0000 Subject: [PATCH 25/38] Use Osmose locale filter and detail strings --- modules/services/osmose.js | 49 ++++++++++++++++-------------------- modules/svg/osmose.js | 5 ++-- modules/ui/osmose_details.js | 29 ++++++++++----------- 3 files changed, 39 insertions(+), 44 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index f58d940977..58a777526b 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -230,21 +230,12 @@ export default { return; } - const langs = { [locale]: true }; - - // Need English strings if not already fetched for fallback values - if (locale !== 'en' && !('en' in _stringCache)) { - langs.en = true; - } - - // TODO: Currently all locales are served, in future a param will be available to request specifics - const url = _osmoseUrlRoot + 'items'; + // Osmose API falls back to English strings where untranslated or if locale doesn't exist + const url = _osmoseUrlRoot + 'items?' + utilQsString({ langs: locale }); d3_json(url) .then(data => { - for (let l in langs) { - _stringCache[l] = {}; - } + _stringCache[locale] = {}; for (let i = 0; i < data.categories.length; i++) { let cat = data.categories[i]; @@ -252,22 +243,27 @@ export default { for (let j = 0; j < cat.items.length; j++) { let item = cat.items[j]; + // TODO: Item has 'color' key with hex color code value, automatically style issue markers + // Only need to cache strings for supported error types - // TODO: may be possible to request additional filter by `item` + // TODO: Investigate making multiple requests with filter by `item` and `class` to reduce data further + // See endpoint: https://osmose.openstreetmap.fr/en/api/0.3beta/items/X/class/X?langs=X if (qaServices.osmose.items.indexOf(item.item) !== -1) { for (let k = 0; k < item.class.length; k++) { let { class: cl, item: cat } = item.class[k]; let issueType = `${cat}-${cl}`; - - for (let l in langs) { - _stringCache[l][issueType] = {}; - - let issueStrings = _stringCache[l][issueType]; - - // TODO: Only title is currently served, in future description and other strings will be too - let { title: {[l]: title} } = item.class[k]; - if (title) issueStrings.title = title; - } + let issueStrings = {}; + + // Value of root key will be null if no string exists + // If string exists, value is an object with key 'auto' for string + let { title, detail, trap, fix, example } = item.class[k]; + if (title) issueStrings.title = title.auto; + if (detail) issueStrings.detail = detail.auto; + if (trap) issueStrings.trap = trap.auto; + if (fix) issueStrings.fix = fix.auto; + if (example) issueStrings.example = example.auto; + + _stringCache[locale][issueType] = issueStrings; } } } @@ -281,11 +277,8 @@ export default { }, getStrings(issueType, locale=currentLocale) { - const l = (locale in _stringCache) ? _stringCache[locale][issueType] : {}; - const en = ('en' in _stringCache) ? _stringCache.en[issueType] : {}; - - // Fallback to English if string is untranslated - return Object.assign({}, en, l); + // No need to fallback to English, Osmose API handles this for us + return (locale in _stringCache) ? _stringCache[locale][issueType] : {}; }, postUpdate(d, callback) { diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index 409dfa9d3c..898386b091 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -64,8 +64,9 @@ export function svgOsmose(projection, context, dispatch) { // Enable the layer. This shows the errors and transitions them to visible. function layerOn() { // Strings supplied by Osmose fetched before showing layer for first time - // TODO: If layer is toggled quickly multiple requests are sent - // TODO: No error handling in place + // NOTE: Currently no way to change locale in iD at runtime, would need to re-call this method if that's ever implemented + // FIXME: If layer is toggled quickly multiple requests are sent + // FIXME: No error handling in place getService().loadStrings(editOn); drawLayer diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 2f876f00a2..788dd4d3e0 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -20,15 +20,15 @@ export function uiOsmoseDetails(context) { // Issue description supplied by Osmose var s = services.osmose.getStrings(d.error_type); - return ('description' in s) ? s.description : unknown; + return ('detail' in s) ? s.detail : unknown; } function osmoseDetails(selection) { var details = selection.selectAll('.error-details') .data( - (_error ? [_error] : []), - function(d) { return d.id + '-' + (d.status || 0); } + _error ? [_error] : [], + d => `${d.id}-${d.status || 0}` ); details.exit() @@ -39,30 +39,31 @@ export function uiOsmoseDetails(context) { .attr('class', 'error-details error-details-container'); - // description var descriptionEnter = detailsEnter .append('div') .attr('class', 'error-details-description'); + // Description descriptionEnter .append('h4') - .text(function() { return t('QA.keepRight.detail_description'); }); + .text(() => t('QA.keepRight.detail_description')); descriptionEnter .append('div') .attr('class', 'error-details-description-text') .html(issueDetail); + // Elements (populated later as data is requested) descriptionEnter .append('h4') .attr('class', 'error-details-subtitle') - .text(function() { return t('QA.osmose.elems_title'); }); + .text(() => t('QA.osmose.elems_title')); var elementList = descriptionEnter .append('ul') .attr('class', 'error-details-elements'); - services.osmose.loadErrorDetail(_error, function(err, d) { + services.osmose.loadErrorDetail(_error, (err, d) => { if (d.elems === undefined) return; elementList.selectAll('.error_entity_link') @@ -71,7 +72,7 @@ export function uiOsmoseDetails(context) { .append('li') .append('a') .attr('class', 'error_entity_link') - .text(function(d) { return d; }) + .text(d => d) .each(function() { var link = d3_select(this); var entityID = this.textContent; @@ -79,11 +80,11 @@ export function uiOsmoseDetails(context) { // Add click handler link - .on('mouseenter', function() { + .on('mouseenter', () => { context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) .classed('hover', true); }) - .on('mouseleave', function() { + .on('mouseleave', () => { context.surface().selectAll('.hover') .classed('hover', false); }) @@ -123,12 +124,12 @@ export function uiOsmoseDetails(context) { // Things like keys and values are dynamic details const special = { tags: true, values: true, chars: true, sug_tags: true }; - for (let type in special) { + for (const type in special) { if (type in d) { descriptionEnter .append('h4') .attr('class', 'error-details-subtitle') - .text(function() { return t(`QA.osmose.details.${type}`); }); + .text(() => t(`QA.osmose.details.${type}`)); descriptionEnter .append('ul') @@ -137,7 +138,7 @@ export function uiOsmoseDetails(context) { .data(d[type]) .enter() .append('li') - .html(function(d) { return d; }); + .html(d => d); } } @@ -148,7 +149,7 @@ export function uiOsmoseDetails(context) { } - osmoseDetails.error = function(val) { + osmoseDetails.error = (val) => { if (!arguments.length) return _error; _error = val; return osmoseDetails; From 010ca30999a2ebd88d199281ca9ddbbea9ebfe48 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 11:39:40 +0000 Subject: [PATCH 26/38] Show more Osmose strings in the UI Also only add sections where appropriate --- data/core.yaml | 2 ++ dist/locales/en.json | 2 ++ modules/services/osmose.js | 2 ++ modules/ui/osmose_details.js | 66 +++++++++++++++++++++++++----------- 4 files changed, 53 insertions(+), 19 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 98d36b3cce..92e5d3c739 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -830,6 +830,8 @@ en: osmose: title: Osmose Issue elems_title: Issue Elements + fix_title: Fix Guidelines + trap_title: Common Pitfalls details: values: Issue Values tags: Issue Tags diff --git a/dist/locales/en.json b/dist/locales/en.json index 819618a992..2e2d4e3149 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1034,6 +1034,8 @@ "osmose": { "title": "Osmose Issue", "elems_title": "Issue Elements", + "fix_title": "Fix Guidelines", + "trap_title": "Common Pitfalls", "details": { "values": "Issue Values", "tags": "Issue Tags", diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 58a777526b..6abd37fea3 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -258,6 +258,8 @@ export default { // If string exists, value is an object with key 'auto' for string let { title, detail, trap, fix, example } = item.class[k]; if (title) issueStrings.title = title.auto; + // TODO: Replace \[(.+?)\]\((.+?)\) pattern with \1 + // TODO: Replace `(.+?)` with some sort of code styling if (detail) issueStrings.detail = detail.auto; if (trap) issueStrings.trap = trap.auto; if (fix) issueStrings.fix = fix.auto; diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 788dd4d3e0..1aad58a7ef 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -10,17 +10,16 @@ import { utilDisplayName, utilEntityOrMemberSelector } from '../util'; export function uiOsmoseDetails(context) { - var _error; + let _error; + const unknown = t('inspector.unknown'); - function issueDetail(d) { - var unknown = t('inspector.unknown'); - + function issueString(d, type) { if (!d) return unknown; // Issue description supplied by Osmose var s = services.osmose.getStrings(d.error_type); - return ('detail' in s) ? s.detail : unknown; + return (type in s) ? s[type] : unknown; } @@ -51,22 +50,50 @@ export function uiOsmoseDetails(context) { descriptionEnter .append('div') .attr('class', 'error-details-description-text') - .html(issueDetail); + .html(d => issueString(d, 'detail')); // Elements (populated later as data is requested) - descriptionEnter - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.elems_title')); - - var elementList = descriptionEnter - .append('ul') - .attr('class', 'error-details-elements'); + const detailsDiv = descriptionEnter.append('div'); + + // Suggested Fix (musn't exist for every issue type) + if (issueString(_error, 'fix') !== unknown) { + descriptionEnter + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.fix_title')); + + descriptionEnter + .append('div') + .attr('class', 'error-details-description-text') + .html(d => issueString(d, 'fix')); + } + + // Common Pitfalls (musn't exist for every issue type) + if (issueString(_error, 'trap') !== unknown) { + descriptionEnter + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.trap_title')); + + descriptionEnter + .append('div') + .attr('class', 'error-details-description-text') + .html(d => issueString(d, 'trap')); + } services.osmose.loadErrorDetail(_error, (err, d) => { - if (d.elems === undefined) return; - - elementList.selectAll('.error_entity_link') + // No details to add if there are no associated issue elements + if (!d.elems || d.elems.length === 0) return; + + detailsDiv + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.elems_title')); + + detailsDiv + .append('ul') + .attr('class', 'error-details-elements') + .selectAll('.error_entity_link') .data(d.elems) .enter() .append('li') @@ -122,16 +149,17 @@ export function uiOsmoseDetails(context) { } }); + // TODO: Show the dynamic subtitle string directly once langs parameter is added // Things like keys and values are dynamic details const special = { tags: true, values: true, chars: true, sug_tags: true }; for (const type in special) { if (type in d) { - descriptionEnter + detailsDiv .append('h4') .attr('class', 'error-details-subtitle') .text(() => t(`QA.osmose.details.${type}`)); - descriptionEnter + detailsDiv .append('ul') .attr('class', 'error-details-list') .selectAll('li') From e3c6f58624c78da76e1afe3bd3f583700fd13f5f Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 12:16:39 +0000 Subject: [PATCH 27/38] Format markdown in Osmose strings appropriately --- css/80_app.css | 8 ++++++++ modules/services/osmose.js | 19 +++++++++++-------- modules/ui/osmose_details.js | 12 ++++++------ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 784db9f465..e1571247c5 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2757,6 +2757,14 @@ input.key-trap { .error-details-subtitle { padding-top: 10px; } +.error-details code { + padding: .2em .4em; + margin: 0; + font-size: 85%; + font-family: monospace; + background-color: rgba(27,31,35,.05); + border-radius: 3px; +} .note-save .new-comment-input, .error-save .new-comment-input { diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 6abd37fea3..f7a36350cb 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -230,6 +230,12 @@ export default { return; } + function format(string) { + // Some strings contain markdown syntax + string = string.replace(/\[((?:.|\n)+?)\]\((.+?)\)/g, '$1'); + return string.replace(/`(.+?)`/g, '$1'); + } + // Osmose API falls back to English strings where untranslated or if locale doesn't exist const url = _osmoseUrlRoot + 'items?' + utilQsString({ langs: locale }); @@ -246,7 +252,7 @@ export default { // TODO: Item has 'color' key with hex color code value, automatically style issue markers // Only need to cache strings for supported error types - // TODO: Investigate making multiple requests with filter by `item` and `class` to reduce data further + // TODO: Instead, make multiple requests with filter by `item` and `class` to reduce data further // See endpoint: https://osmose.openstreetmap.fr/en/api/0.3beta/items/X/class/X?langs=X if (qaServices.osmose.items.indexOf(item.item) !== -1) { for (let k = 0; k < item.class.length; k++) { @@ -256,14 +262,11 @@ export default { // Value of root key will be null if no string exists // If string exists, value is an object with key 'auto' for string - let { title, detail, trap, fix, example } = item.class[k]; + let { title, detail, trap, fix } = item.class[k]; if (title) issueStrings.title = title.auto; - // TODO: Replace \[(.+?)\]\((.+?)\) pattern with \1 - // TODO: Replace `(.+?)` with some sort of code styling - if (detail) issueStrings.detail = detail.auto; - if (trap) issueStrings.trap = trap.auto; - if (fix) issueStrings.fix = fix.auto; - if (example) issueStrings.example = example.auto; + if (detail) issueStrings.detail = format(detail.auto); + if (trap) issueStrings.trap = format(trap.auto); + if (fix) issueStrings.fix = format(fix.auto); _stringCache[locale][issueType] = issueStrings; } diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 1aad58a7ef..0b4938311d 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -48,7 +48,7 @@ export function uiOsmoseDetails(context) { .text(() => t('QA.keepRight.detail_description')); descriptionEnter - .append('div') + .append('p') .attr('class', 'error-details-description-text') .html(d => issueString(d, 'detail')); @@ -63,8 +63,8 @@ export function uiOsmoseDetails(context) { .text(() => t('QA.osmose.fix_title')); descriptionEnter - .append('div') - .attr('class', 'error-details-description-text') + .append('p') + .attr('class', 'error-details-fix-text') .html(d => issueString(d, 'fix')); } @@ -76,8 +76,8 @@ export function uiOsmoseDetails(context) { .text(() => t('QA.osmose.trap_title')); descriptionEnter - .append('div') - .attr('class', 'error-details-description-text') + .append('p') + .attr('class', 'error-details-trap-text') .html(d => issueString(d, 'trap')); } @@ -161,7 +161,7 @@ export function uiOsmoseDetails(context) { detailsDiv .append('ul') - .attr('class', 'error-details-list') + .attr('class', `error-details-${type}`) .selectAll('li') .data(d[type]) .enter() From 06ac02f89f3977c61b6aed458b0b8d6b31ce3004 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 14:52:50 +0000 Subject: [PATCH 28/38] Use Osmose dynamic subtitile string Providing a `langs` param that's currently not implemented so these will all be English until that is done. --- data/core.yaml | 8 ++--- dist/locales/en.json | 9 ++---- modules/services/osmose.js | 57 +++++------------------------------- modules/ui/osmose_details.js | 35 +++++++++------------- 4 files changed, 25 insertions(+), 84 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 92e5d3c739..e245869889 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -829,14 +829,10 @@ en: QA: osmose: title: Osmose Issue + detail_title: Issue Details elems_title: Issue Elements fix_title: Fix Guidelines - trap_title: Common Pitfalls - details: - values: Issue Values - tags: Issue Tags - chars: Issue Characters - sug_tags: Suggested Tags + trap_title: Common Mistakes improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/dist/locales/en.json b/dist/locales/en.json index 2e2d4e3149..907d91e51b 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1033,15 +1033,10 @@ "QA": { "osmose": { "title": "Osmose Issue", + "detail_title": "Issue Details", "elems_title": "Issue Elements", "fix_title": "Fix Guidelines", - "trap_title": "Common Pitfalls", - "details": { - "values": "Issue Values", - "tags": "Issue Tags", - "chars": "Issue Characters", - "sug_tags": "Suggested Tags" - } + "trap_title": "Common Mistakes" }, "improveOSM": { "title": "ImproveOSM Detection", diff --git a/modules/services/osmose.js b/modules/services/osmose.js index f7a36350cb..6321ff01fa 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -11,7 +11,7 @@ import { services as qaServices } from '../../data/qa_errors.json'; const tiler = utilTiler(); const dispatch = d3_dispatch('loaded'); -const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta/'; +const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta'; const _erZoom = 14; const _stringCache = {}; @@ -104,7 +104,7 @@ export default { if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; let [ x, y, z ] = tile.xyz; - let url = _osmoseUrlRoot + `issues/${z}/${x}/${y}.json?` + utilQsString(params); + let url = `${_osmoseUrlRoot}/issues/${z}/${x}/${y}.json?` + utilQsString(params); let controller = new AbortController(); _erCache.inflightTile[tile.id] = controller; @@ -162,7 +162,7 @@ export default { return; } - let url = _osmoseUrlRoot + `issue/${d.identifier}`; + let url = `${_osmoseUrlRoot}/issue/${d.identifier}?langs=${currentLocale}`; d3_json(url) .then(data => { @@ -170,51 +170,8 @@ export default { // Assign directly for immediate use in the callback d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); - // Some error types have details in their subtitle - const special = { - tags: { - '3040-3040': /Bad tag value: "(.+)"/i, - '4010-4010': /Tag (.+) is deprecated/i, - '4010-40102': /Tag (.+) is deprecated/i, - '4030-900': /Conflict between tags: (.+), (.+)/i, - '5070-50703': /"(.+)"=".+" unexpected/i, - '9010-9010001': /(.+) is unnecessary/i - }, - values: { - '3090-3090': /Incorrect date "(.+)"/i, - '9010-9010003': /(.+)/ - }, - chars: { - '5070-50703': /unexpected symbol char \(.+, (.+)\)/i, - '5070-50704': /Umbalanced (.+)/i, - '5070-50705': /Unexpected char (.+)/i - }, - sug_tags: { - '4010-4010': /Tag .+ is deprecated: (.+)/i, - '4010-40102': /Tag .+ is deprecated: (.+)/i, - } - }; - for (let type in special) { - if (d.error_type in special[type]) { - // Destructuring doesn't work if no match returns null, hence [] - let [, ...details] = special[type][d.error_type].exec(data.subtitle) || []; - - if ( - d.error_type === '5070-50703' - && type === 'chars' - && details - ) { - details[0] = String.fromCharCode(details[0]); - } - - // This error has a rare subtitle variant - if (d.error_type === '9010-9010001' && !details) { - [, ...details] = /\. Remove (.+)/i.exec(data.subtitle) || []; - } - - if (details) d[type] = details; - } - } + // Some issues have instance specific detail in a subtitle + d.detail = data.subtitle; this.replaceError(d); if (callback) callback(null, d); @@ -237,7 +194,7 @@ export default { } // Osmose API falls back to English strings where untranslated or if locale doesn't exist - const url = _osmoseUrlRoot + 'items?' + utilQsString({ langs: locale }); + const url = `${_osmoseUrlRoot}/items?langs=${locale}`; d3_json(url) .then(data => { @@ -292,7 +249,7 @@ export default { } // UI sets the status to either 'done' or 'false' - let url = _osmoseUrlRoot + `issue/${d.identifier}/${d.newStatus}`; + let url = `${_osmoseUrlRoot}/issue/${d.identifier}/${d.newStatus}`; let controller = new AbortController(); _erCache.inflightPost[d.id] = controller; diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 0b4938311d..0374bb22df 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -85,6 +85,20 @@ export function uiOsmoseDetails(context) { // No details to add if there are no associated issue elements if (!d.elems || d.elems.length === 0) return; + // Things like keys and values are dynamically added to a subtitle string + if (d.detail) { + detailsDiv + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.detail_title')); + + detailsDiv + .append('p') + .attr('class', 'error-details-detail') + .html(d => d.detail); + } + + // Create list of linked issue elements detailsDiv .append('h4') .attr('class', 'error-details-subtitle') @@ -149,27 +163,6 @@ export function uiOsmoseDetails(context) { } }); - // TODO: Show the dynamic subtitle string directly once langs parameter is added - // Things like keys and values are dynamic details - const special = { tags: true, values: true, chars: true, sug_tags: true }; - for (const type in special) { - if (type in d) { - detailsDiv - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t(`QA.osmose.details.${type}`)); - - detailsDiv - .append('ul') - .attr('class', `error-details-${type}`) - .selectAll('li') - .data(d[type]) - .enter() - .append('li') - .html(d => d); - } - } - // Don't hide entities related to this error - #5880 context.features().forceVisible(d.elems); context.map().pan([0,0]); // trigger a redraw From 2d2fa440c1ed3351ab41e2efde8e0789cb825f51 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 17:43:30 +0000 Subject: [PATCH 29/38] Use multiple specific Osmose string requests - Only requesting the data we need (Osmose has many more issue types than iD will ever display) --- data/qa_errors.json | 27 -------- modules/services/osmose.js | 123 +++++++++++++++++++++++-------------- 2 files changed, 77 insertions(+), 73 deletions(-) diff --git a/data/qa_errors.json b/data/qa_errors.json index 613c6cb5cb..0d6ed3d0f9 100644 --- a/data/qa_errors.json +++ b/data/qa_errors.json @@ -11,33 +11,6 @@ } }, "osmose": { - "items": [ - 0, - 1040, - 1050, - 1070, - 1150, - 1190, - 1280, - 2110, - 3040, - 3090, - 3161, - 3200, - 3220, - 3250, - 4010, - 4030, - 4080, - 5010, - 7040, - 7090, - 5010, - 5070, - 8300, - 8360, - 9010 - ], "errorIcons": { "0-1": "maki-home", "0-2": "maki-home", diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 6321ff01fa..d289e1d857 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -12,6 +12,10 @@ import { services as qaServices } from '../../data/qa_errors.json'; const tiler = utilTiler(); const dispatch = d3_dispatch('loaded'); const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta'; +const _osmoseItems = + qaServices.osmose.errorIcons.keys() + .map(s => s.split('-')[0]) + .reduce((unique, item) => unique.indexOf(item) !== -1 ? unique : [...unique, item], []); const _erZoom = 14; const _stringCache = {}; @@ -88,7 +92,7 @@ export default { let params = { // Tiles return a maximum # of errors // So we want to filter our request for only types iD supports - item: qaServices.osmose.items.join() + item: _osmoseItems }; // determine the needed tiles to cover the view @@ -182,60 +186,74 @@ export default { }, loadStrings(callback, locale=currentLocale) { - if (locale in _stringCache) { + const issueTypes = qaServices.osmose.errorIcons.keys(); + + if ( + locale in _stringCache + && _stringCache[locale].keys().length === issueTypes.length + ) { if (callback) callback(null, _stringCache[locale]); return; } - function format(string) { + // May be partially populated already if some requests were successful + if (!(locale in _stringCache)) { + _stringCache[locale] = {}; + } + + const format = string => { // Some strings contain markdown syntax string = string.replace(/\[((?:.|\n)+?)\]\((.+?)\)/g, '$1'); return string.replace(/`(.+?)`/g, '$1'); - } - - // Osmose API falls back to English strings where untranslated or if locale doesn't exist - const url = `${_osmoseUrlRoot}/items?langs=${locale}`; + }; - d3_json(url) - .then(data => { - _stringCache[locale] = {}; - - for (let i = 0; i < data.categories.length; i++) { - let cat = data.categories[i]; - - for (let j = 0; j < cat.items.length; j++) { - let item = cat.items[j]; - - // TODO: Item has 'color' key with hex color code value, automatically style issue markers - - // Only need to cache strings for supported error types - // TODO: Instead, make multiple requests with filter by `item` and `class` to reduce data further - // See endpoint: https://osmose.openstreetmap.fr/en/api/0.3beta/items/X/class/X?langs=X - if (qaServices.osmose.items.indexOf(item.item) !== -1) { - for (let k = 0; k < item.class.length; k++) { - let { class: cl, item: cat } = item.class[k]; - let issueType = `${cat}-${cl}`; - let issueStrings = {}; - - // Value of root key will be null if no string exists - // If string exists, value is an object with key 'auto' for string - let { title, detail, trap, fix } = item.class[k]; - if (title) issueStrings.title = title.auto; - if (detail) issueStrings.detail = format(detail.auto); - if (trap) issueStrings.trap = format(trap.auto); - if (fix) issueStrings.fix = format(fix.auto); - - _stringCache[locale][issueType] = issueStrings; - } - } - } + // Only need to cache strings for supported issue types + // Using multiple individual item + class requests to reduce fetched data size + const allRequests = issueTypes.map(issueType => { + // No need to request data we already have + if (issueType in _stringCache[locale]) return; + + const cacheData = data => { + // Bunch of nested single value arrays of objects + const [ cat = {items:[]} ] = data.categories; + const [ item = {class:[]} ] = cat.items; + const [ cl = null ] = item.class; + + // If null default value is reached, data wasn't as expected (or was empty) + if (!cl) { + /* eslint-disable no-console */ + console.log(`Osmose strings request (${issueType}) had unexpected data`); + /* eslint-enable no-console */ + return; } - if (callback) callback(null, _stringCache[locale]); - }) - .catch(err => { - if (callback) callback(err.message); - }); + // TODO: Item has 'color' key with hex color code value, automatically style issue markers + // const { item, color } = item; + + // Value of root key will be null if no string exists + // If string exists, value is an object with key 'auto' for string + const { title, detail, fix, trap } = cl; + + let issueStrings = {}; + if (title) issueStrings.title = title.auto; + if (detail) issueStrings.detail = format(detail.auto); + if (trap) issueStrings.trap = format(trap.auto); + if (fix) issueStrings.fix = format(fix.auto); + + _stringCache[locale][issueType] = issueStrings; + }; + + const [ item, cl ] = issueType.split('-'); + + // Osmose API falls back to English strings where untranslated or if locale doesn't exist + const url = `${_osmoseUrlRoot}/items/${item}/class/${cl}?langs=${locale}`; + + return jsonPromise(url, cacheData); + }); + + Promise.all(allRequests) + .then(() => { if (callback) callback(null, _stringCache[locale]); }) + .catch(err => { if (callback) callback(err); }); }, getStrings(issueType, locale=currentLocale) { @@ -313,4 +331,17 @@ export default { getClosedCounts() { return _erCache.closed; } -}; \ No newline at end of file +}; + +function jsonPromise(url, then) { + return new Promise((resolve, reject) => { + d3_json(url) + .then(data => { + then(data); + resolve(); + }) + .catch(err => { + reject(err); + }); + }); +} \ No newline at end of file From cf878ca4102ffaf440e050da662dbfd1d47e7cde Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 18:22:59 +0000 Subject: [PATCH 30/38] =?UTF-8?q?Use=20Object.keys()=20not=20Python=20{}.k?= =?UTF-8?q?eys()=20=F0=9F=A4=A6=E2=80=8D=E2=99=82=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/services/osmose.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index d289e1d857..b48e9e6acd 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -13,9 +13,9 @@ const tiler = utilTiler(); const dispatch = d3_dispatch('loaded'); const _osmoseUrlRoot = 'https://osmose.openstreetmap.fr/en/api/0.3beta'; const _osmoseItems = - qaServices.osmose.errorIcons.keys() - .map(s => s.split('-')[0]) - .reduce((unique, item) => unique.indexOf(item) !== -1 ? unique : [...unique, item], []); + Object.keys(qaServices.osmose.errorIcons) + .map(s => s.split('-')[0]) + .reduce((unique, item) => unique.indexOf(item) !== -1 ? unique : [...unique, item], []); const _erZoom = 14; const _stringCache = {}; @@ -186,11 +186,11 @@ export default { }, loadStrings(callback, locale=currentLocale) { - const issueTypes = qaServices.osmose.errorIcons.keys(); + const issueTypes = Object.keys(qaServices.osmose.errorIcons); if ( locale in _stringCache - && _stringCache[locale].keys().length === issueTypes.length + && Object.keys(_stringCache[locale]).length === issueTypes.length ) { if (callback) callback(null, _stringCache[locale]); return; From ecadafb3e462e1013cd27620ad77af7614988b8e Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 19:02:43 +0000 Subject: [PATCH 31/38] Use Osmose supplied hex colors --- css/65_data.css | 55 ------------------------------------- modules/services/osmose.js | 12 ++++++-- modules/svg/osmose.js | 4 +-- modules/ui/osmose_header.js | 4 +-- 4 files changed, 14 insertions(+), 61 deletions(-) diff --git a/css/65_data.css b/css/65_data.css index 21461c37ac..1687909e6d 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -153,61 +153,6 @@ color: #EC1C24; } -/* Osmose Errors -------------------------------------------------------- */ - -.osmose.item-0 { - color: #BDBDBD; -} - -.osmose.item-1040, -.osmose.item-1050, -.osmose.item-1070, -.osmose.item-1080, -.osmose.item-1150, -.osmose.item-1190, -.osmose.item-1280 { - color: #DCB000; -} - -.osmose.item-2110 { - color: #2CDCDC; -} - -.osmose.item-3040, -.osmose.item-3090, -.osmose.item-3161, -.osmose.item-3200, -.osmose.item-3220, -.osmose.item-3250 { - color: #2D9359; -} - -.osmose.item-4010, -.osmose.item-4030, -.osmose.item-4080 { - color: #F2F230; -} - -.osmose.item-5010, -.osmose.item-5070 { - color: #EC0000; -} - -.osmose.item-7040, -.osmose.item-7090 { - color: #9F16B4; -} - -.osmose.item-8300, -.osmose.item-8360 { - color: #3DE736; -} - -.osmose.item-9010 { - color: #976432; -} - /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { pointer-events: none; diff --git a/modules/services/osmose.js b/modules/services/osmose.js index b48e9e6acd..b7fc2d0ee0 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -18,6 +18,7 @@ const _osmoseItems = .reduce((unique, item) => unique.indexOf(item) !== -1 ? unique : [...unique, item], []); const _erZoom = 14; const _stringCache = {}; +const _colorCache = {}; // This gets reassigned if reset let _erCache; @@ -227,8 +228,11 @@ export default { return; } - // TODO: Item has 'color' key with hex color code value, automatically style issue markers - // const { item, color } = item; + // Cache served item colors to automatically style issue markers later + const { item: itemInt, color } = item; + if (color.test(/^#[A-Fa-f0-9]{6}|[A-Fa-f0-9]{3}/)) { + _colorCache[itemInt] = color; + } // Value of root key will be null if no string exists // If string exists, value is an object with key 'auto' for string @@ -261,6 +265,10 @@ export default { return (locale in _stringCache) ? _stringCache[locale][issueType] : {}; }, + getColor(itemType) { + return (itemType in _colorCache) ? _colorCache[itemType] : '#FFFFFF'; + }, + postUpdate(d, callback) { if (_erCache.inflightPost[d.id]) { return callback({ message: 'Error update already inflight', status: -2 }, d); diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index 898386b091..1fb565080e 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -142,7 +142,7 @@ export function svgOsmose(projection, context, dispatch) { markersEnter .append('polygon') - .attr('fill', 'currentColor') + .attr('fill', d => getService().getColor(d.item)) .call(markerPath, 'qa_error-fill'); markersEnter @@ -262,4 +262,4 @@ export function svgOsmose(projection, context, dispatch) { return drawOsmose; -} \ No newline at end of file +} diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js index c7049591e1..4016b4b711 100644 --- a/modules/ui/osmose_header.js +++ b/modules/ui/osmose_header.js @@ -54,7 +54,7 @@ export function uiOsmoseHeader() { svgEnter .append('polygon') - .attr('fill', 'currentColor') + .attr('fill', d => services.osmose.getColor(d.item)) .attr('class', 'qa_error-fill') .attr('points', '16,3 4,3 1,6 1,17 4,20 7,20 10,27 13,20 16,20 19,17.033 19,6'); @@ -90,4 +90,4 @@ export function uiOsmoseHeader() { return osmoseHeader; -} \ No newline at end of file +} From 1dd65cb93ed8ace48f0a1017cf8ecdd4909551e6 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 18:33:41 +0000 Subject: [PATCH 32/38] Correct Osmose UI element spacing --- css/80_app.css | 4 ++-- modules/ui/osmose_details.js | 43 ++++++++++++++++++++---------------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index e1571247c5..b39b66cf38 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2753,9 +2753,9 @@ input.key-trap { [dir='rtl'] .error-details-description-text::first-letter { text-transform: none; /* #5877 */ } -.error-details-description -.error-details-subtitle { +.error-details-subsection h4 { padding-top: 10px; + padding-bottom: 0px; } .error-details code { padding: .2em .4em; diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 0374bb22df..c91d51b086 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -53,31 +53,37 @@ export function uiOsmoseDetails(context) { .html(d => issueString(d, 'detail')); // Elements (populated later as data is requested) - const detailsDiv = descriptionEnter.append('div'); + const detailsDiv = descriptionEnter + .append('div') + .attr('class', 'error-details-subsection'); + + const elemsDiv = descriptionEnter + .append('div') + .attr('class', 'error-details-subsection'); // Suggested Fix (musn't exist for every issue type) if (issueString(_error, 'fix') !== unknown) { - descriptionEnter - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.fix_title')); + let div = descriptionEnter + .append('div') + .attr('class', 'error-details-subsection') - descriptionEnter - .append('p') - .attr('class', 'error-details-fix-text') + div.append('h4') + .text(() => t('QA.osmose.fix_title')) + + div.append('p') .html(d => issueString(d, 'fix')); } // Common Pitfalls (musn't exist for every issue type) if (issueString(_error, 'trap') !== unknown) { - descriptionEnter - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.trap_title')); + let div = descriptionEnter + .append('div') + .attr('class', 'error-details-subsection') + + div.append('h4') + .text(() => t('QA.osmose.trap_title')) - descriptionEnter - .append('p') - .attr('class', 'error-details-trap-text') + div.append('p') .html(d => issueString(d, 'trap')); } @@ -90,21 +96,20 @@ export function uiOsmoseDetails(context) { detailsDiv .append('h4') .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.detail_title')); + .text(() => t('QA.osmose.detail_title')) detailsDiv .append('p') - .attr('class', 'error-details-detail') .html(d => d.detail); } // Create list of linked issue elements - detailsDiv + elemsDiv .append('h4') .attr('class', 'error-details-subtitle') .text(() => t('QA.osmose.elems_title')); - detailsDiv + elemsDiv .append('ul') .attr('class', 'error-details-elements') .selectAll('.error_entity_link') From 377e99240f547920a34dd413862e17734626a539 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 3 Feb 2020 21:04:43 +0000 Subject: [PATCH 33/38] Add Osmose translations link to details footer Improves structure of the details UI and introduces the use of flexboxes for this. Does not break UI for other error services with shared classes. --- css/80_app.css | 15 +++++++++++---- data/core.yaml | 1 + dist/locales/en.json | 3 ++- modules/ui/osmose_details.js | 33 +++++++++++++++++++++++---------- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index b39b66cf38..323b262d02 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2734,18 +2734,19 @@ input.key-trap { padding-top: 20px; } -.error-details { - padding: 10px; -} .error-details-container { background: #ececec; padding: 10px; margin-top: 20px; border-radius: 4px; border: 1px solid #ccc; + display: flex; + flex-direction: column; } .error-details-description { margin-bottom: 10px; + display: flex; + flex-direction: column; } .error-details-description-text::first-letter { text-transform: capitalize; @@ -2755,7 +2756,7 @@ input.key-trap { } .error-details-subsection h4 { padding-top: 10px; - padding-bottom: 0px; + padding-bottom: 0; } .error-details code { padding: .2em .4em; @@ -2765,6 +2766,12 @@ input.key-trap { background-color: rgba(27,31,35,.05); border-radius: 3px; } +.error-details .translation-link { + margin-top: 5px; + display: flex; + flex-direction: row; + justify-content: flex-end; +} .note-save .new-comment-input, .error-save .new-comment-input { diff --git a/data/core.yaml b/data/core.yaml index e245869889..eb571850a0 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -833,6 +833,7 @@ en: elems_title: Issue Elements fix_title: Fix Guidelines trap_title: Common Mistakes + translation: Translations supplied by Osmose improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/dist/locales/en.json b/dist/locales/en.json index 907d91e51b..15822daa37 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1036,7 +1036,8 @@ "detail_title": "Issue Details", "elems_title": "Issue Elements", "fix_title": "Fix Guidelines", - "trap_title": "Common Mistakes" + "trap_title": "Common Mistakes", + "translation": "Translations supplied by Osmose" }, "improveOSM": { "title": "ImproveOSM Detection", diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index c91d51b086..7183810e52 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -40,7 +40,7 @@ export function uiOsmoseDetails(context) { var descriptionEnter = detailsEnter .append('div') - .attr('class', 'error-details-description'); + .attr('class', 'error-details-subsection'); // Description descriptionEnter @@ -53,22 +53,22 @@ export function uiOsmoseDetails(context) { .html(d => issueString(d, 'detail')); // Elements (populated later as data is requested) - const detailsDiv = descriptionEnter + const detailsDiv = detailsEnter .append('div') .attr('class', 'error-details-subsection'); - const elemsDiv = descriptionEnter + const elemsDiv = detailsEnter .append('div') .attr('class', 'error-details-subsection'); // Suggested Fix (musn't exist for every issue type) if (issueString(_error, 'fix') !== unknown) { - let div = descriptionEnter + let div = detailsEnter .append('div') - .attr('class', 'error-details-subsection') + .attr('class', 'error-details-subsection'); div.append('h4') - .text(() => t('QA.osmose.fix_title')) + .text(() => t('QA.osmose.fix_title')); div.append('p') .html(d => issueString(d, 'fix')); @@ -76,17 +76,30 @@ export function uiOsmoseDetails(context) { // Common Pitfalls (musn't exist for every issue type) if (issueString(_error, 'trap') !== unknown) { - let div = descriptionEnter + let div = detailsEnter .append('div') - .attr('class', 'error-details-subsection') + .attr('class', 'error-details-subsection'); div.append('h4') - .text(() => t('QA.osmose.trap_title')) + .text(() => t('QA.osmose.trap_title')); div.append('p') .html(d => issueString(d, 'trap')); } + detailsEnter + .append('div') + .attr('class', 'translation-link') + .append('a') + .attr('target', '_blank') + .attr('rel', 'noopener noreferrer') // security measure + .attr('href', 'https://www.transifex.com/openstreetmap-france/osmose') + .text(d => t('QA.osmose.translation')) + .append('svg') + .attr('class', 'icon inline') + .append('use') + .attr('href', '#iD-icon-out-link'); + services.osmose.loadErrorDetail(_error, (err, d) => { // No details to add if there are no associated issue elements if (!d.elems || d.elems.length === 0) return; @@ -96,7 +109,7 @@ export function uiOsmoseDetails(context) { detailsDiv .append('h4') .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.detail_title')) + .text(() => t('QA.osmose.detail_title')); detailsDiv .append('p') From e6cf53dfe89404cf84cf2da13956790bddee3e3e Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 4 Feb 2020 12:05:36 +0000 Subject: [PATCH 34/38] Fix use of test() method on color string --- modules/services/osmose.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index b7fc2d0ee0..4997ddb2fc 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -230,7 +230,7 @@ export default { // Cache served item colors to automatically style issue markers later const { item: itemInt, color } = item; - if (color.test(/^#[A-Fa-f0-9]{6}|[A-Fa-f0-9]{3}/)) { + if (/^#[A-Fa-f0-9]{6}|[A-Fa-f0-9]{3}/.test(color)) { _colorCache[itemInt] = color; } @@ -352,4 +352,4 @@ function jsonPromise(url, then) { reject(err); }); }); -} \ No newline at end of file +} From 02f088e5a9f3c009896adb2266d72eebe2d04650 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 4 Feb 2020 21:03:34 +0000 Subject: [PATCH 35/38] Update some QA translation strings --- data/core.yaml | 14 +++++++------- dist/locales/en.json | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index eb571850a0..69f809997a 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -625,13 +625,13 @@ en: tooltip: Note data from OpenStreetMap title: OpenStreetMap notes keepRight: - tooltip: Automatically detected map issues from keepright.at + tooltip: Data issues detected by keepright.at title: KeepRight Issues improveOSM: - tooltip: Missing data automatically detected by improveosm.org + tooltip: Missing data detected by improveosm.org title: ImproveOSM Issues osmose: - tooltip: Automatically detected map issues from osmose.openstreetmap.fr + tooltip: Data issues detected by osmose.openstreetmap.fr title: Osmose Issues custom: tooltip: "Drag and drop a data file onto the page, or click the button to setup" @@ -829,11 +829,11 @@ en: QA: osmose: title: Osmose Issue - detail_title: Issue Details - elems_title: Issue Elements + detail_title: Details + elems_title: Features fix_title: Fix Guidelines trap_title: Common Mistakes - translation: Translations supplied by Osmose + translation: Translations provided by Osmose improveOSM: title: ImproveOSM Detection geometry_types: @@ -1353,7 +1353,7 @@ en: title: Quality Assurance intro: "*Quality Assurance* (Q/A) tools can find improper tags, disconnected roads, and other issues with OpenStreetMap, which mappers can then fix. To view existing Q/A issues, click the {data} **Map data** panel to enable a specific Q/A layer." tools_h: "Tools" - tools: "The following tools are currently supported: [KeepRight](https://www.keepright.at/), [ImproveOSM](https://improveosm.org/en/) and [Osmose](https://osmose.openstreetmap.fr/). Expect iD to support more Q/A tools in the future." + tools: "The following tools are currently supported: [KeepRight](https://www.keepright.at/), [ImproveOSM](https://improveosm.org/en/) and [Osmose](https://osmose.openstreetmap.fr/)." issues_h: "Handling Issues" issues: "Handling Q/A issues is similar to handling notes. Click on a marker to view the issue details in the sidebar. Each tool has its own capabilities, but generally you can comment and/or close an issue." field: diff --git a/dist/locales/en.json b/dist/locales/en.json index 15822daa37..6440278628 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -775,15 +775,15 @@ "title": "OpenStreetMap notes" }, "keepRight": { - "tooltip": "Automatically detected map issues from keepright.at", + "tooltip": "Data issues detected by keepright.at", "title": "KeepRight Issues" }, "improveOSM": { - "tooltip": "Missing data automatically detected by improveosm.org", + "tooltip": "Missing data detected by improveosm.org", "title": "ImproveOSM Issues" }, "osmose": { - "tooltip": "Automatically detected map issues from osmose.openstreetmap.fr", + "tooltip": "Data issues detected by osmose.openstreetmap.fr", "title": "Osmose Issues" }, "custom": { @@ -1033,11 +1033,11 @@ "QA": { "osmose": { "title": "Osmose Issue", - "detail_title": "Issue Details", - "elems_title": "Issue Elements", + "detail_title": "Details", + "elems_title": "Features", "fix_title": "Fix Guidelines", "trap_title": "Common Mistakes", - "translation": "Translations supplied by Osmose" + "translation": "Translations provided by Osmose" }, "improveOSM": { "title": "ImproveOSM Detection", @@ -1665,7 +1665,7 @@ "title": "Quality Assurance", "intro": "*Quality Assurance* (Q/A) tools can find improper tags, disconnected roads, and other issues with OpenStreetMap, which mappers can then fix. To view existing Q/A issues, click the {data} **Map data** panel to enable a specific Q/A layer.", "tools_h": "Tools", - "tools": "The following tools are currently supported: [KeepRight](https://www.keepright.at/), [ImproveOSM](https://improveosm.org/en/) and [Osmose](https://osmose.openstreetmap.fr/). Expect iD to support more Q/A tools in the future.", + "tools": "The following tools are currently supported: [KeepRight](https://www.keepright.at/), [ImproveOSM](https://improveosm.org/en/) and [Osmose](https://osmose.openstreetmap.fr/).", "issues_h": "Handling Issues", "issues": "Handling Q/A issues is similar to handling notes. Click on a marker to view the issue details in the sidebar. Each tool has its own capabilities, but generally you can comment and/or close an issue." }, From 087867d5c8e7eb5d0da3f088b4fb45d07b91c78e Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 4 Feb 2020 21:05:40 +0000 Subject: [PATCH 36/38] Use Promises for Osmose issue detail requests Also ES6ify the details script --- modules/services/osmose.js | 30 ++-- modules/ui/osmose_details.js | 336 ++++++++++++++++++----------------- 2 files changed, 185 insertions(+), 181 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 4997ddb2fc..e34260e0e4 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -160,30 +160,26 @@ export default { }); }, - loadErrorDetail(d, callback) { + loadErrorDetail(d) { // Error details only need to be fetched once if (d.elems !== undefined) { - if (callback) callback(null, d); - return; + return Promise.resolve(d); } - let url = `${_osmoseUrlRoot}/issue/${d.identifier}?langs=${currentLocale}`; + const url = `${_osmoseUrlRoot}/issue/${d.identifier}?langs=${currentLocale}`; + const cacheDetails = data => { + // Associated elements used for highlighting + // Assign directly for immediate use in the callback + d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); - d3_json(url) - .then(data => { - // Associated elements used for highlighting - // Assign directly for immediate use in the callback - d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); + // Some issues have instance specific detail in a subtitle + d.detail = data.subtitle; - // Some issues have instance specific detail in a subtitle - d.detail = data.subtitle; + this.replaceError(d); + }; - this.replaceError(d); - if (callback) callback(null, d); - }) - .catch(err => { - if (callback) callback(err.message); - }); + return jsonPromise(url, cacheDetails) + .then(() => d); }, loadStrings(callback, locale=currentLocale) { diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 7183810e52..a27e08cb12 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -1,6 +1,6 @@ import { - event as d3_event, - select as d3_select + event as d3_event, + select as d3_select } from 'd3-selection'; import { modeSelect } from '../modes/select'; @@ -10,190 +10,198 @@ import { utilDisplayName, utilEntityOrMemberSelector } from '../util'; export function uiOsmoseDetails(context) { - let _error; - const unknown = t('inspector.unknown'); + let _error; + const unknown = t('inspector.unknown'); - function issueString(d, type) { - if (!d) return unknown; - - // Issue description supplied by Osmose - var s = services.osmose.getStrings(d.error_type); - return (type in s) ? s[type] : unknown; - } + function issueString(d, type) { + if (!d) return unknown; + // Issue description supplied by Osmose + const s = services.osmose.getStrings(d.error_type); + return (type in s) ? s[type] : unknown; + } - function osmoseDetails(selection) { - var details = selection.selectAll('.error-details') - .data( - _error ? [_error] : [], - d => `${d.id}-${d.status || 0}` - ); - details.exit() - .remove(); + function osmoseDetails(selection) { + const details = selection.selectAll('.error-details') + .data( + _error ? [_error] : [], + d => `${d.id}-${d.status || 0}` + ); - var detailsEnter = details.enter() - .append('div') - .attr('class', 'error-details error-details-container'); + details.exit() + .remove(); + const detailsEnter = details.enter() + .append('div') + .attr('class', 'error-details error-details-container'); - var descriptionEnter = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); - // Description - descriptionEnter - .append('h4') - .text(() => t('QA.keepRight.detail_description')); + // Description + const descriptionDiv = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - descriptionEnter - .append('p') - .attr('class', 'error-details-description-text') - .html(d => issueString(d, 'detail')); + descriptionDiv + .append('h4') + .text(() => t('QA.keepRight.detail_description')); - // Elements (populated later as data is requested) - const detailsDiv = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + descriptionDiv + .append('p') + .attr('class', 'error-details-description-text') + .html(d => issueString(d, 'detail')); - const elemsDiv = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + // Elements (populated later as data is requested) + const detailsDiv = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - // Suggested Fix (musn't exist for every issue type) - if (issueString(_error, 'fix') !== unknown) { - let div = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + const elemsDiv = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - div.append('h4') - .text(() => t('QA.osmose.fix_title')); + // Suggested Fix (musn't exist for every issue type) + if (issueString(_error, 'fix') !== unknown) { + let div = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - div.append('p') - .html(d => issueString(d, 'fix')); - } + div + .append('h4') + .text(() => t('QA.osmose.fix_title')); - // Common Pitfalls (musn't exist for every issue type) - if (issueString(_error, 'trap') !== unknown) { - let div = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + div + .append('p') + .html(d => issueString(d, 'fix')); + } - div.append('h4') - .text(() => t('QA.osmose.trap_title')); + // Common Pitfalls (musn't exist for every issue type) + if (issueString(_error, 'trap') !== unknown) { + let div = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - div.append('p') - .html(d => issueString(d, 'trap')); - } + div + .append('h4') + .text(() => t('QA.osmose.trap_title')); - detailsEnter - .append('div') - .attr('class', 'translation-link') - .append('a') - .attr('target', '_blank') - .attr('rel', 'noopener noreferrer') // security measure - .attr('href', 'https://www.transifex.com/openstreetmap-france/osmose') - .text(d => t('QA.osmose.translation')) - .append('svg') - .attr('class', 'icon inline') - .append('use') - .attr('href', '#iD-icon-out-link'); - - services.osmose.loadErrorDetail(_error, (err, d) => { - // No details to add if there are no associated issue elements - if (!d.elems || d.elems.length === 0) return; - - // Things like keys and values are dynamically added to a subtitle string - if (d.detail) { - detailsDiv - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.detail_title')); - - detailsDiv - .append('p') - .html(d => d.detail); - } - - // Create list of linked issue elements - elemsDiv - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.elems_title')); - - elemsDiv - .append('ul') - .attr('class', 'error-details-elements') - .selectAll('.error_entity_link') - .data(d.elems) - .enter() - .append('li') - .append('a') - .attr('class', 'error_entity_link') - .text(d => d) - .each(function() { - var link = d3_select(this); - var entityID = this.textContent; - var entity = context.hasEntity(entityID); - - // Add click handler - link - .on('mouseenter', () => { - context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) - .classed('hover', true); - }) - .on('mouseleave', () => { - context.surface().selectAll('.hover') - .classed('hover', false); - }) - .on('click', function() { - d3_event.preventDefault(); - var osmlayer = context.layers().layer('osm'); - if (!osmlayer.enabled()) { - osmlayer.enabled(true); - } - - context.map().centerZoom(d.loc, 20); - - if (entity) { - context.enter(modeSelect(context, [entityID])); - } else { - context.loadEntity(entityID, function() { - context.enter(modeSelect(context, [entityID])); - }); - } - }); - - // Replace with friendly name if possible - // (The entity may not yet be loaded into the graph) - if (entity) { - var name = utilDisplayName(entity); // try to use common name - - if (!name) { - var preset = context.presets().match(entity, context.graph()); - name = preset && !preset.isFallback() && preset.name(); // fallback to preset name - } - - if (name) { - this.innerText = name; - } - } - }); - - // Don't hide entities related to this error - #5880 - context.features().forceVisible(d.elems); - context.map().pan([0,0]); // trigger a redraw - }); + div + .append('p') + .html(d => issueString(d, 'trap')); } + detailsEnter + .append('div') + .attr('class', 'translation-link') + .append('a') + .attr('target', '_blank') + .attr('rel', 'noopener noreferrer') // security measure + .attr('href', 'https://www.transifex.com/openstreetmap-france/osmose') + .text(() => t('QA.osmose.translation')) + .append('svg') + .attr('class', 'icon inline') + .append('use') + .attr('href', '#iD-icon-out-link'); + + services.osmose.loadErrorDetail(_error) + .then(d => { + // No details to add if there are no associated issue elements + if (!d.elems || d.elems.length === 0) return; + + // TODO: Do nothing if UI has moved on by the time this resolves + + // Things like keys and values are dynamically added to a subtitle string + if (d.detail) { + detailsDiv + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.detail_title')); - osmoseDetails.error = (val) => { - if (!arguments.length) return _error; - _error = val; - return osmoseDetails; - }; + detailsDiv + .append('p') + .html(d => d.detail); + } + // Create list of linked issue elements + elemsDiv + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.elems_title')); + + elemsDiv + .append('ul') + .attr('class', 'error-details-elements') + .selectAll('.error_entity_link') + .data(d.elems) + .enter() + .append('li') + .append('a') + .attr('class', 'error_entity_link') + .text(d => d) + .each(function() { + const link = d3_select(this); + const entityID = this.textContent; + const entity = context.hasEntity(entityID); + + // Add click handler + link + .on('mouseenter', () => { + context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) + .classed('hover', true); + }) + .on('mouseleave', () => { + context.surface().selectAll('.hover') + .classed('hover', false); + }) + .on('click', () => { + d3_event.preventDefault(); + const osmlayer = context.layers().layer('osm'); + if (!osmlayer.enabled()) { + osmlayer.enabled(true); + } + + context.map().centerZoom(d.loc, 20); + + if (entity) { + context.enter(modeSelect(context, [entityID])); + } else { + context.loadEntity(entityID, () => { + context.enter(modeSelect(context, [entityID])); + }); + } + }); + // Replace with friendly name if possible + // (The entity may not yet be loaded into the graph) + if (entity) { + let name = utilDisplayName(entity); // try to use common name + + if (!name) { + const preset = context.presets().match(entity, context.graph()); + name = preset && !preset.isFallback() && preset.name(); // fallback to preset name + } + + if (name) { + this.innerText = name; + } + } + }); + + // Don't hide entities related to this error - #5880 + context.features().forceVisible(d.elems); + context.map().pan([0,0]); // trigger a redraw + }) + .catch(err => {}); // TODO: Handle failed json request gracefully in some way + } + + + osmoseDetails.error = val => { + if (!arguments.length) return _error; + _error = val; return osmoseDetails; -} \ No newline at end of file + }; + + + return osmoseDetails; +} From 3745c06d622482ad32cd9a9fae9874333b6a092a Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 4 Feb 2020 21:22:44 +0000 Subject: [PATCH 37/38] Hide Osmose issue description if no string --- modules/ui/osmose_details.js | 38 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index a27e08cb12..d3e1333853 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -11,15 +11,13 @@ import { utilDisplayName, utilEntityOrMemberSelector } from '../util'; export function uiOsmoseDetails(context) { let _error; - const unknown = t('inspector.unknown'); - function issueString(d, type) { - if (!d) return unknown; + if (!d) return ''; - // Issue description supplied by Osmose + // Issue strings are cached from Osmose API const s = services.osmose.getStrings(d.error_type); - return (type in s) ? s[type] : unknown; + return (type in s) ? s[type] : ''; } @@ -39,18 +37,20 @@ export function uiOsmoseDetails(context) { // Description - const descriptionDiv = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + if (issueString(_error, 'detail')) { + const div = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - descriptionDiv - .append('h4') - .text(() => t('QA.keepRight.detail_description')); + div + .append('h4') + .text(() => t('QA.keepRight.detail_description')); - descriptionDiv - .append('p') - .attr('class', 'error-details-description-text') - .html(d => issueString(d, 'detail')); + div + .append('p') + .attr('class', 'error-details-description-text') + .html(d => issueString(d, 'detail')); + } // Elements (populated later as data is requested) const detailsDiv = detailsEnter @@ -62,8 +62,8 @@ export function uiOsmoseDetails(context) { .attr('class', 'error-details-subsection'); // Suggested Fix (musn't exist for every issue type) - if (issueString(_error, 'fix') !== unknown) { - let div = detailsEnter + if (issueString(_error, 'fix')) { + const div = detailsEnter .append('div') .attr('class', 'error-details-subsection'); @@ -77,8 +77,8 @@ export function uiOsmoseDetails(context) { } // Common Pitfalls (musn't exist for every issue type) - if (issueString(_error, 'trap') !== unknown) { - let div = detailsEnter + if (issueString(_error, 'trap')) { + const div = detailsEnter .append('div') .attr('class', 'error-details-subsection'); From 03a097c847cf3746ead7661cc925b737fef6396a Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 4 Feb 2020 21:44:20 +0000 Subject: [PATCH 38/38] Move Osmose translation link below details box --- css/80_app.css | 2 +- modules/ui/osmose_details.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 323b262d02..67f1b31489 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2766,7 +2766,7 @@ input.key-trap { background-color: rgba(27,31,35,.05); border-radius: 3px; } -.error-details .translation-link { +.error-details + .translation-link { margin-top: 5px; display: flex; flex-direction: row; diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index d3e1333853..1740099007 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -91,7 +91,8 @@ export function uiOsmoseDetails(context) { .html(d => issueString(d, 'trap')); } - detailsEnter + // Translation link below details container + selection .append('div') .attr('class', 'translation-link') .append('a')