From 20ed8b50c98cf6ef01a6da07415227c69e87a9cf Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 3 Feb 2019 12:54:05 +0000 Subject: [PATCH] Add generic QA error I've converted the improveOSM errors to use this new generic QA error structure which should allow for more general code to be used in behaviour and UI. Sidebar preview is currently broken, but will be fixed shortly. --- css/65_data.css | 8 ++-- data/qa_errors.json | 39 ++++++++++++++++ modules/behavior/hover.js | 6 +-- modules/behavior/select.js | 10 ++--- modules/osm/index.js | 4 +- modules/osm/qa_error.js | 65 +++++++++++++++++++++++++++ modules/services/improveOSM.js | 76 +++++++++++++++----------------- modules/svg/improveOSM.js | 5 ++- modules/ui/improveOSM_details.js | 4 +- modules/ui/improveOSM_header.js | 13 ++++-- 10 files changed, 168 insertions(+), 62 deletions(-) create mode 100644 data/qa_errors.json create mode 100644 modules/osm/qa_error.js diff --git a/css/65_data.css b/css/65_data.css index 2686d940d..47d1c03a2 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -131,7 +131,7 @@ /* ImproveOSM Errors ------------------------------------------------------- */ -.iOSM.error_type-ow- { /* missing one way */ +.iOSM.error_type-ow { /* missing one way */ color: #1E90FF; } @@ -148,10 +148,11 @@ color: #FFA500; } -.iOSM.error_type-tr- { /* missing turn restriction */ +.iOSM.error_type-tr { /* missing turn restriction */ color: #EC1C24; } + /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { pointer-events: none; @@ -210,5 +211,4 @@ stroke: #000; stroke-width: 5px; stroke-miterlimit: 1; -} - +} \ No newline at end of file diff --git a/data/qa_errors.json b/data/qa_errors.json new file mode 100644 index 000000000..689b5da78 --- /dev/null +++ b/data/qa_errors.json @@ -0,0 +1,39 @@ +{ + "services": { + "improveOSM": { + "shortName": "iOSM", + "errorTypes": { + "ow": { + "icon": "", + "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": { + "shortName": "kr", + "errorTypes": { + + } + } + } +} \ No newline at end of file diff --git a/modules/behavior/hover.js b/modules/behavior/hover.js index 9645b83e2..04031afc7 100644 --- a/modules/behavior/hover.js +++ b/modules/behavior/hover.js @@ -5,7 +5,7 @@ import { select as d3_select } from 'd3-selection'; -import { osmEntity, osmNote, krError, iOsmError } from '../osm'; +import { osmEntity, osmNote, krError, qaError } from '../osm'; import { utilKeybinding, utilRebind } from '../util'; @@ -113,7 +113,7 @@ export function behaviorHover(context) { selector = '.data' + datum.__featurehash__; } else if ( - datum instanceof iOsmError || + datum instanceof qaError || datum instanceof krError ) { entity = datum; @@ -187,4 +187,4 @@ export function behaviorHover(context) { return utilRebind(behavior, dispatch, 'on'); -} +} \ No newline at end of file diff --git a/modules/behavior/select.js b/modules/behavior/select.js index 8043648d3..e87318b6c 100644 --- a/modules/behavior/select.js +++ b/modules/behavior/select.js @@ -19,8 +19,8 @@ import { import { osmEntity, osmNote, - iOsmError, - krError + krError, + qaError } from '../osm'; @@ -171,11 +171,11 @@ export function behaviorSelect(context) { context .selectedNoteID(datum.id) .enter(modeSelectNote(context, datum.id)); - } else if (datum instanceof iOsmError & !isMultiselect) { // clicked an improveOSM error + } else if (datum instanceof krError & !isMultiselect) { // clicked a krError error context .selectedErrorID(datum.id) .enter(modeSelectError(context, datum.id, datum.source)); - } else if (datum instanceof krError & !isMultiselect) { // clicked a krError error + } else if (datum instanceof qaError & !isMultiselect) { // clicked an external QA error context .selectedErrorID(datum.id) .enter(modeSelectError(context, datum.id, datum.source)); @@ -241,4 +241,4 @@ export function behaviorSelect(context) { return behavior; -} +} \ No newline at end of file diff --git a/modules/osm/index.js b/modules/osm/index.js index d97cb8feb..dd1397daa 100644 --- a/modules/osm/index.js +++ b/modules/osm/index.js @@ -1,11 +1,11 @@ export { osmChangeset } from './changeset'; export { osmEntity } from './entity'; export { krError } from './keepRight'; -export { iOsmError } from './improveOSM'; export { osmNode } from './node'; export { osmNote } from './note'; export { osmRelation } from './relation'; export { osmWay } from './way'; +export { qaError } from './qa_error'; export { osmIntersection, @@ -27,4 +27,4 @@ export { osmOneWayTags, osmPavedTags, osmIsInterestingTag -} from './tags'; +} from './tags'; \ No newline at end of file diff --git a/modules/osm/qa_error.js b/modules/osm/qa_error.js new file mode 100644 index 000000000..b00d6f5b6 --- /dev/null +++ b/modules/osm/qa_error.js @@ -0,0 +1,65 @@ +import _extend from 'lodash-es/extend'; +import { services } from '../../data/qa_errors.json'; + +export function qaError() { + if (!(this instanceof qaError)) { + return (new qaError()).initialize(arguments); + } else if (arguments.length) { + this.initialize(arguments); + } +} + +// Generic handling for services without nice IDs +qaError.id = function() { + return qaError.id.next--; +}; + +qaError.id.next = -1; + +_extend(qaError.prototype, { + type: 'qaError', + + // All errors need a position + loc: [0, 0], + + // These should be passed in, used to retrieve from qa_errors.json + service: '', + error_type: '', + + initialize: function(sources) { + for (var i = 0; i < sources.length; ++i) { + var source = sources[i]; + for (var prop in source) { + if (Object.prototype.hasOwnProperty.call(source, prop)) { + if (source[prop] === undefined) { + delete this[prop]; + } else { + this[prop] = source[prop]; + } + } + } + } + + if (this.service) { + this.source = services[this.service].shortName; + + if (this.error_type) { + var template = services[this.service].errorTypes[this.error_type]; + + this.icon = template.icon; + this.category = template.category; + } + } + + // All errors must have an ID for selection + if (!this.id) { + this.id = qaError.id() + ''; // as string + } + + return this; + }, + + update: function(attrs) { + return qaError(this, attrs); // {v: 1 + (this.v || 0)} + } +}); \ No newline at end of file diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index e170f930d..5623a09db 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -9,7 +9,7 @@ import { json as d3_json } from 'd3-request'; import { request as d3_request } from 'd3-request'; import { geoExtent, geoVecAdd } from '../geo'; -import { iOsmError } from '../osm'; +import { qaError } from '../osm'; import { services } from './index'; import { t } from '../util/locale'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -210,12 +210,13 @@ export default { // One-ways can land on same segment in opposite direction loc = preventCoincident(loc, false); - var d = new iOsmError({ + var d = new qaError({ + // Info required for every error loc: loc, - comments: null, - error_subtype: '', + service: 'improveOSM', error_type: k, - icon: '', //TODO: Find suitable icon + // Extra details needed for this service + error_key: k, identifier: { // this is used to post changes to the error wayId: feature.wayId, fromNodeId: feature.fromNodeId, @@ -243,27 +244,20 @@ export default { // Tiles at high zoom == missing roads if (data.tiles) { data.tiles.forEach(function(feature) { - // Average of recorded points should land on the missing geometry - var loc = pointAverage(feature.points); + var geoType = feature.type.toLowerCase(); + // Average of recorded points should land on the missing geometry // Missing geometry could happen to land on another error + var loc = pointAverage(feature.points); loc = preventCoincident(loc, false); - - var geoType = feature.type.toLowerCase(); - var geoIcons = { - road: 'maki-car', - parking: 'maki-parking', - both: 'maki-car', - path: 'maki-shoe' - }; - - var d = new iOsmError({ + var d = new qaError({ + // Info required for every error loc: loc, - comments: null, - error_subtype: geoType, - error_type: k, - icon: geoIcons[geoType], + service: 'improveOSM', + error_type: k + '-' + geoType, + // Extra details needed for this service + error_key: k, identifier: { x: feature.x, y: feature.y }, status: feature.status }); @@ -281,10 +275,9 @@ export default { // Entities at high zoom == turn restrictions if (data.entities) { data.entities.forEach(function(feature) { - var loc = feature.point; - // Turn restrictions could be missing at same junction // We also want to bump the error up so node is accessible + var loc = feature.point; loc = preventCoincident([loc.lon, loc.lat], true); // Elements are presented in a strange way @@ -293,24 +286,25 @@ export default { var via_node = ids[3]; var to_way = ids[2].split(':')[1]; - // Travel direction along from_way clarifies the turn restriction - var p1 = feature.segments[0].points[0]; - var p2 = feature.segments[0].points[1]; - - var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); - - var d = new iOsmError({ + var d = new qaError({ + // Info required for every error loc: loc, - comments: null, - error_subtype: '', + service: 'improveOSM', error_type: k, - icon: 'temaki-junction', + // Extra details needed for this service + error_key: k, identifier: feature.id, object_id: via_node, object_type: 'node', status: feature.status }); + // Travel direction along from_way clarifies the turn restriction + var p1 = feature.segments[0].points[0]; + var p2 = feature.segments[0].points[1]; + + var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); + // Variables used in the description d.replacements = { num_passed: feature.numberOfPasses, @@ -351,18 +345,18 @@ export default { function sendPayload(err, user) { if (err) { return callback(err, d); } - var type = d.error_type; - var url = _impOsmUrls[type] + '/comment'; + var key = d.error_key; + var url = _impOsmUrls[key] + '/comment'; var payload = { username: user.display_name }; // Each error type has different data for identification - if (type === 'ow') { + if (key === 'ow') { payload.roadSegments = [ d.identifier ]; - } else if (type === 'mr') { + } else if (key === 'mr') { payload.tiles = [ d.identifier ]; - } else if (type === 'tr') { + } else if (key === 'tr') { payload.targetIds = [ d.identifier ]; } @@ -390,7 +384,7 @@ export default { // No pretty identifier, so we just use coordinates if (d.newStatus === 'SOLVED') { var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); - _erCache.closed[d.error_type + ':' + closedID] = true; + _erCache.closed[key + ':' + closedID] = true; } return callback(err, d); @@ -417,7 +411,7 @@ export default { // replace a single error in the cache replaceError: function(error) { - if (!(error instanceof iOsmError) || !error.id) return; + if (!(error instanceof qaError) || !error.id) return; _erCache.data[error.id] = error; updateRtree(encodeErrorRtree(error), true); // true = replace @@ -426,7 +420,7 @@ export default { // remove a single error from the cache removeError: function(error) { - if (!(error instanceof iOsmError) || !error.id) return; + if (!(error instanceof qaError) || !error.id) return; delete _erCache.data[error.id]; updateRtree(encodeErrorRtree(error), false); // false = remove diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index b9881ff29..f10be4823 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -119,7 +119,8 @@ export function svgImproveOSM(projection, context, dispatch) { 'qa_error', d.source, 'error_id-' + d.id, - 'error_type-' + d.error_type + '-' + d.error_subtype + 'error_type-' + d.error_type, + 'category-' + d.category ].join(' '); }); @@ -259,4 +260,4 @@ export function svgImproveOSM(projection, context, dispatch) { return drawImproveOSM; -} +} \ No newline at end of file diff --git a/modules/ui/improveOSM_details.js b/modules/ui/improveOSM_details.js index be513ccb0..075ccd0cb 100644 --- a/modules/ui/improveOSM_details.js +++ b/modules/ui/improveOSM_details.js @@ -17,7 +17,7 @@ export function uiImproveOsmDetails(context) { var unknown = t('inspector.unknown'); if (!d) return unknown; - var errorType = d.error_type; + var errorType = d.error_key; var et = dataEn.QA.improveOSM.error_types[errorType]; var detail; @@ -124,4 +124,4 @@ export function uiImproveOsmDetails(context) { return improveOsmDetails; -} +} \ No newline at end of file diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index aa393b22b..381118a81 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -10,7 +10,7 @@ export function uiImproveOsmHeader() { var unknown = t('inspector.unknown'); if (!d) return unknown; - var errorType = d.error_type; + var errorType = d.error_key; var et = dataEn.QA.improveOSM.error_types[errorType]; if (et && et.title) { @@ -46,7 +46,14 @@ export function uiImproveOsmHeader() { .attr('height', '30px') .attr('viewbox', '0 0 20 30') .attr('class', function(d) { - return 'preset-icon-28 qa_error ' + d.source + ' error_id-' + d.id + ' error_type-' + d.error_type + '-' + d.error_subtype; + return [ + 'preset-icon-28', + 'qa_error', + d.source, + 'error_id-' + d.id, + 'error_type-' + d.error_type, + 'category-' + d.category + ].join(' '); }); svgEnter @@ -86,4 +93,4 @@ export function uiImproveOsmHeader() { return improveOsmHeader; -} +} \ No newline at end of file