From 659a291e8982e48868398d780379e4dee9bbd8d0 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Tue, 17 Jul 2018 16:43:00 -0400 Subject: [PATCH 01/27] add core overwrite for external preset source ref #remote-presets --- modules/core/context.js | 2 +- modules/presets/index.js | 14 +++++++++---- test/spec/presets/index.js | 42 +++++++++++++++++++++++++++++++++++++- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/modules/core/context.js b/modules/core/context.js index 4d758066e..00e9103d0 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -306,7 +306,7 @@ export function coreContext() { /* Presets */ var presets; context.presets = function() { return presets; }; - + context.overwritePresets = function(newPresets) { presets.overwrite(newPresets); }; /* Map */ var map; diff --git a/modules/presets/index.js b/modules/presets/index.js index 789c3cb9c..6edeb6f55 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -118,10 +118,7 @@ export function presetIndex() { return areaKeys; }; - - all.init = function() { - var d = data.presets; - + all.build = function (d) { all.collection = []; _recent.collection = []; _fields = {}; @@ -172,6 +169,15 @@ export function presetIndex() { } } + }; + + all.overwrite = function (d) { + all.build(d); + return all; + }; + + all.init = function() { + all.build(data.presets); return all; }; diff --git a/test/spec/presets/index.js b/test/spec/presets/index.js index 9178a4519..248c4e132 100644 --- a/test/spec/presets/index.js +++ b/test/spec/presets/index.js @@ -156,7 +156,47 @@ describe('iD.presetIndex', function() { }); }); - + describe('#overwrite', function() { + it('overwrites iD presets with provided list of presets', function() { + var testPresets = { + presets: { + 'amenity/fuel/shell': { + tags: { 'amenity': 'fuel' }, + geometry: ['point','area'], + suggestion: true + }, + 'highway/foo': { + tags: { 'highway': 'foo' }, + geometry: ['area'] + }, + 'leisure/track': { + tags: { 'leisure': 'track' }, + geometry: ['line', 'area'] + }, + 'natural': { + tags: { 'natural': '*' }, + geometry: ['point', 'vertex', 'area'] + }, + 'natural/peak': { + tags: { 'natural': 'peak' }, + geometry: ['point', 'vertex'] + }, + 'natural/tree_row': { + tags: { 'natural': 'tree_row' }, + geometry: ['line'] + }, + 'natural/wood': { + tags: { 'natural': 'wood' }, + geometry: ['point', 'area'] + } + } + }; + var currentPresets = iD.Context().presets(); + var overwrittenPresets = iD.Context().overwritePresets(testPresets); + expect(currentPresets).to.not.eql(overwrittenPresets); + }); + }); + describe('expected matches', function() { it('prefers building to multipolygon', function() { From 57ea2401775dbafbe7674d91ffea147270a70a0e Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Wed, 18 Jul 2018 10:50:54 -0400 Subject: [PATCH 02/27] make uiModes options strict to match only those within defaults ref #remote-presets --- modules/core/context.js | 9 ++++++-- modules/presets/index.js | 47 ++++++++++++++++++++++++++++++---------- modules/ui/modes.js | 19 +++++++++++++--- modules/util/index.js | 1 + modules/util/util.js | 4 ++++ 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/modules/core/context.js b/modules/core/context.js index 00e9103d0..984c6ce0a 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -40,7 +40,8 @@ import { utilDetect } from '../util/detect'; import { utilCallWhenIdle, - utilRebind + utilRebind, + utilExternalPresets } from '../util'; @@ -493,7 +494,11 @@ export function coreContext() { background.init(); features.init(); - presets.init(); + if (utilExternalPresets()) { + presets.fromExternal(); + } else { + presets.init(); + } areaKeys = presets.areaKeys(); diff --git a/modules/presets/index.js b/modules/presets/index.js index 6edeb6f55..e41e42dc6 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -8,6 +8,13 @@ import { presetCategory } from './category'; import { presetCollection } from './collection'; import { presetField } from './field'; import { presetPreset } from './preset'; +import { utilStringQs } from '../util'; + +import { json as d3_json } from 'd3-request'; +import { + select as d3_select, + selectAll as d3_selectAll +} from 'd3-selection'; export { presetCategory }; export { presetCollection }; @@ -20,7 +27,7 @@ export function presetIndex() { // loading new data and returning defaults var all = presetCollection([]); - var _defaults = { area: all, line: all, point: all, vertex: all, relation: all }; + var _defaults = {}; var _fields = {}; var _universal = []; var _recent = presetCollection([]); @@ -119,7 +126,7 @@ export function presetIndex() { }; all.build = function (d) { - all.collection = []; + all.collection = []; _recent.collection = []; _fields = {}; _universal = []; @@ -148,13 +155,17 @@ export function presetIndex() { if (d.defaults) { var getItem = _bind(all.item, all); - _defaults = { - area: presetCollection(d.defaults.area.map(getItem)), - line: presetCollection(d.defaults.line.map(getItem)), - point: presetCollection(d.defaults.point.map(getItem)), - vertex: presetCollection(d.defaults.vertex.map(getItem)), - relation: presetCollection(d.defaults.relation.map(getItem)) - }; + _defaults = {}; + _forEach(Object.keys(d.defaults), function (k) { + _defaults[k] = presetCollection(d.defaults[k].map(getItem)); + }); + // _defaults = { + // area: presetCollection(d.defaults.area.map(getItem)), + // line: presetCollection(d.defaults.line.map(getItem)), + // point: presetCollection(d.defaults.point.map(getItem)), + // vertex: presetCollection(d.defaults.vertex.map(getItem)), + // relation: presetCollection(d.defaults.relation.map(getItem)) + // }; } for (var i = 0; i < all.collection.length; i++) { @@ -171,13 +182,22 @@ export function presetIndex() { }; + all.fromExternal = function(callback) { + var presetsUrl = utilStringQs(window.location.hash)['presets']; + d3_json(presetsUrl, function(err, presets) { + if (err) all.init(); + all.overwrite(presets); + }); + return all; + }; + all.overwrite = function (d) { all.build(d); return all; }; all.init = function() { - all.build(data.presets); + all.build(data.presets); return all; }; @@ -192,7 +212,12 @@ export function presetIndex() { all.defaults = function(geometry, n) { var rec = _recent.matchGeometry(geometry).collection.slice(0, 4); var def = _uniq(rec.concat(_defaults[geometry].collection)).slice(0, n - 1); - return presetCollection(_uniq(rec.concat(def).concat(all.item(geometry)))); + var fin = _uniq(rec.concat(def).concat(all.item(geometry))).filter(i => i !== undefined); + return presetCollection(fin); + }; + + all.defaultTypes = function() { + return Object.keys(_defaults); }; all.choose = function(preset) { diff --git a/modules/ui/modes.js b/modules/ui/modes.js index eb86e30c3..d68fa5b72 100644 --- a/modules/ui/modes.js +++ b/modules/ui/modes.js @@ -1,6 +1,9 @@ import _debounce from 'lodash-es/debounce'; -import { select as d3_select } from 'd3-selection'; +import { + select as d3_select, + selectAll as d3_selectAll +} from 'd3-selection'; import { d3keybinding as d3_keybinding } from '../lib/d3.keybinding.js'; import { @@ -28,6 +31,12 @@ export function uiModes(context) { return context.editable() && mode && mode.id !== 'save'; } + function difference(mode) { + var match = mode.attr('class'); + var defaultIndex = context.presets().defaultTypes().findIndex(function(d) { return new RegExp(d).test(match); }); + return defaultIndex === -1; + } + return function(selection) { var buttons = selection.selectAll('button.add-button') @@ -111,8 +120,12 @@ export function uiModes(context) { function update() { - selection.selectAll('button.add-button') - .property('disabled', !editable()); + var modes = selection.selectAll('button.add-button'); + modes.property('disabled', !editable()); + d3_selectAll('button.add-button').each(function (d) { + var mode = d3_select(this); + mode.property('disabled', difference(mode)); + }); } }; } diff --git a/modules/util/index.js b/modules/util/index.js index f64856de8..9725ab41f 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -7,6 +7,7 @@ export { utilDisplayType } from './util'; export { utilEditDistance } from './util'; export { utilEntitySelector } from './util'; export { utilEntityOrMemberSelector } from './util'; +export { utilExternalPresets } from './util'; export { utilFastMouse } from './util'; export { utilFunctor } from './util'; export { utilGetAllNodes } from './util'; diff --git a/modules/util/util.js b/modules/util/util.js index cf226f5ee..b86f2be39 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -266,3 +266,7 @@ export function utilNoAuto(selection) { .attr('autocapitalize', 'off') .attr('spellcheck', isText ? 'true' : 'false'); } + +export function utilExternalPresets() { + return utilStringQs(window.location.hash).hasOwnProperty('presets'); +} From 873b451befda1f3854d1cfe805b0901d6c321092 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Thu, 19 Jul 2018 15:39:20 -0400 Subject: [PATCH 03/27] move data presets overwritting to the build function ref #remote-presets' --- modules/presets/index.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/presets/index.js b/modules/presets/index.js index e41e42dc6..75553e7c5 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -125,7 +125,8 @@ export function presetIndex() { return areaKeys; }; - all.build = function (d) { + all.build = function () { + var d = data.presets; all.collection = []; _recent.collection = []; _fields = {}; @@ -155,7 +156,6 @@ export function presetIndex() { if (d.defaults) { var getItem = _bind(all.item, all); - _defaults = {}; _forEach(Object.keys(d.defaults), function (k) { _defaults[k] = presetCollection(d.defaults[k].map(getItem)); }); @@ -190,14 +190,15 @@ export function presetIndex() { }); return all; }; - + all.overwrite = function (d) { - all.build(d); + data.presets = d; + all.build(); return all; }; all.init = function() { - all.build(data.presets); + all.build(); return all; }; From db78df55c8bd49098e3b118ecdc2fa068b7264b1 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Tue, 31 Jul 2018 13:41:24 -0400 Subject: [PATCH 04/27] get validaiton objects on context ref #remote-presets --- modules/core/context.js | 16 ++++++++++++++-- modules/presets/index.js | 2 +- modules/ui/entity_editor.js | 2 +- modules/util/index.js | 1 + modules/util/util.js | 4 ++++ modules/validations/index.js | 1 + modules/validations/validation_collection.js | 16 ++++++++++++++++ 7 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 modules/validations/validation_collection.js diff --git a/modules/core/context.js b/modules/core/context.js index 984c6ce0a..58ed89731 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -41,9 +41,11 @@ import { utilDetect } from '../util/detect'; import { utilCallWhenIdle, utilRebind, - utilExternalPresets + utilExternalPresets, + utilExternalValidationRules } from '../util'; +import { validationCollection } from '../validations'; export var areaKeys = {}; @@ -494,11 +496,21 @@ export function coreContext() { background.init(); features.init(); + + // get external data if directed by query parameters if (utilExternalPresets()) { presets.fromExternal(); } else { presets.init(); - } + } + + if (utilExternalValidationRules()) { + var validations = validationCollection(); + validations.init(function(rules) { + context.validationRules = function() { return rules; }; + }); + } + areaKeys = presets.areaKeys(); diff --git a/modules/presets/index.js b/modules/presets/index.js index 75553e7c5..a21a0dee3 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -182,7 +182,7 @@ export function presetIndex() { }; - all.fromExternal = function(callback) { + all.fromExternal = function() { var presetsUrl = utilStringQs(window.location.hash)['presets']; d3_json(presetsUrl, function(err, presets) { if (err) all.init(); diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index 7d6c8c1c7..465892a28 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -38,11 +38,11 @@ export function uiEntityEditor(context) { var rawTagEditor = uiRawTagEditor(context) .on('change', changeTags); - function entityEditor(selection) { var entity = context.entity(_entityID); var tags = _clone(entity.tags); + console.log(context.validationRules()); // Header var header = selection.selectAll('.header') .data([0]); diff --git a/modules/util/index.js b/modules/util/index.js index 9725ab41f..efd132175 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -8,6 +8,7 @@ export { utilEditDistance } from './util'; export { utilEntitySelector } from './util'; export { utilEntityOrMemberSelector } from './util'; export { utilExternalPresets } from './util'; +export { utilExternalValidationRules } from './util'; export { utilFastMouse } from './util'; export { utilFunctor } from './util'; export { utilGetAllNodes } from './util'; diff --git a/modules/util/util.js b/modules/util/util.js index b86f2be39..60e1436df 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -270,3 +270,7 @@ export function utilNoAuto(selection) { export function utilExternalPresets() { return utilStringQs(window.location.hash).hasOwnProperty('presets'); } + +export function utilExternalValidationRules() { + return utilStringQs(window.location.hash).hasOwnProperty('validations'); +} \ No newline at end of file diff --git a/modules/validations/index.js b/modules/validations/index.js index 4f99fa485..402bbffe0 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -1,3 +1,4 @@ +export { validationCollection } from './validation_collection'; export { validationDeprecatedTag } from './deprecated_tag'; export { validationDisconnectedHighway } from './disconnected_highway'; export { validationManyDeletions } from './many_deletions'; diff --git a/modules/validations/validation_collection.js b/modules/validations/validation_collection.js new file mode 100644 index 000000000..f7dadab31 --- /dev/null +++ b/modules/validations/validation_collection.js @@ -0,0 +1,16 @@ +import { utilStringQs } from '../util'; +import { text as d3_text } from 'd3-request'; +import mapcssParse from 'mapcss-parse/source/index'; + +export function validationCollection() { + var validations = {}; + validations.init = function (callback) { + var validationsUrl = utilStringQs(window.location.hash)['validations']; + d3_text(validationsUrl, function(err, mapcss) { + if (err) return; + callback(mapcssParse(mapcss)); + }); + }; + + return validations; +} \ No newline at end of file From 2bf5eaf6e7233e3082477d31ca1f59e278da609c Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Tue, 31 Jul 2018 14:22:34 -0400 Subject: [PATCH 05/27] initial plumbing for iD validation errors ref #remote-presets --- modules/core/context.js | 37 ++++++++++++-------- modules/core/history.js | 10 +++++- modules/ui/entity_editor.js | 1 - modules/util/index.js | 1 + modules/util/mapcss_rule.js | 1 + modules/validations/index.js | 2 +- modules/validations/mapcss_checks.js | 20 +++++++++++ modules/validations/validation_collection.js | 16 --------- 8 files changed, 55 insertions(+), 33 deletions(-) create mode 100644 modules/util/mapcss_rule.js create mode 100644 modules/validations/mapcss_checks.js delete mode 100644 modules/validations/validation_collection.js diff --git a/modules/core/context.js b/modules/core/context.js index 58ed89731..6c6d5ac81 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -7,9 +7,16 @@ import _isObject from 'lodash-es/isObject'; import _isString from 'lodash-es/isString'; import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { json as d3_json } from 'd3-request'; + +import { + json as d3_json, + text as d3_text +} from 'd3-request'; + import { select as d3_select } from 'd3-selection'; +import mapcssParse from 'mapcss-parse/source/index'; + import { t, currentLocale, @@ -24,6 +31,7 @@ import { dataEn } from '../../data'; + import { geoRawMercator } from '../geo/raw_mercator'; import { modeSelect } from '../modes/select'; import { presetIndex } from '../presets'; @@ -40,13 +48,13 @@ import { utilDetect } from '../util/detect'; import { utilCallWhenIdle, - utilRebind, utilExternalPresets, - utilExternalValidationRules + utilExternalValidationRules, + utilMapCSSRule, + utilRebind, + utilStringQs } from '../util'; -import { validationCollection } from '../validations'; - export var areaKeys = {}; export function setAreaKeys(value) { @@ -448,6 +456,15 @@ export function coreContext() { locale = locale.split('-')[0]; } + if (utilExternalValidationRules()) { + var validationsUrl = utilStringQs(window.location.hash)['validations']; + d3_text(validationsUrl, function (err, mapcss) { + if (err) return; + var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcsS) }); + context.validationRules = function() { return validations; }; + }); + } + history = coreHistory(context); context.graph = history.graph; context.changes = history.changes; @@ -497,20 +514,12 @@ export function coreContext() { background.init(); features.init(); - // get external data if directed by query parameters if (utilExternalPresets()) { presets.fromExternal(); } else { presets.init(); } - - if (utilExternalValidationRules()) { - var validations = validationCollection(); - validations.init(function(rules) { - context.validationRules = function() { return rules; }; - }); - } - + areaKeys = presets.areaKeys(); diff --git a/modules/core/history.js b/modules/core/history.js index 57e3a2260..e8d47a02d 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -282,7 +282,15 @@ export function coreHistory(context) { validate: function(changes) { return _flatten( - _map(Validations, function(fn) { return fn()(changes, _stack[_index].graph); }) + _map(Validations, function(fn) { + var warnings; + if (fn === Validations.validationMapCSSChecks) { + warnings = fn()(changes, _stack[_index].graph, context.validationRules()); + } else { + warnings = fn()(changes, _stack[_index].graph); + } + return warnings; + }) ); }, diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index 465892a28..2cf181067 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -42,7 +42,6 @@ export function uiEntityEditor(context) { var entity = context.entity(_entityID); var tags = _clone(entity.tags); - console.log(context.validationRules()); // Header var header = selection.selectAll('.header') .data([0]); diff --git a/modules/util/index.js b/modules/util/index.js index efd132175..fb05fe83c 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -15,6 +15,7 @@ export { utilGetAllNodes } from './util'; export { utilGetPrototypeOf } from './util'; export { utilGetSetValue } from './get_set_value'; export { utilIdleWorker} from './idle_worker'; +export { utilMapCSSRule } from './mapcss_rule'; export { utilNoAuto } from './util'; export { utilPrefixCSSProperty } from './util'; export { utilPrefixDOMProperty } from './util'; diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js new file mode 100644 index 000000000..712c75474 --- /dev/null +++ b/modules/util/mapcss_rule.js @@ -0,0 +1 @@ +export function mapcssRule() {} \ No newline at end of file diff --git a/modules/validations/index.js b/modules/validations/index.js index 402bbffe0..112b38e6c 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -1,7 +1,7 @@ -export { validationCollection } from './validation_collection'; export { validationDeprecatedTag } from './deprecated_tag'; export { validationDisconnectedHighway } from './disconnected_highway'; export { validationManyDeletions } from './many_deletions'; +export { validationMapCSSChecks } from './mapcss_checks'; export { validationMissingTag } from './missing_tag'; export { validationOldMultipolygon } from './old_multipolygon'; export { validationTagSuggestsArea } from './tag_suggests_area'; diff --git a/modules/validations/mapcss_checks.js b/modules/validations/mapcss_checks.js new file mode 100644 index 000000000..21d57ba1d --- /dev/null +++ b/modules/validations/mapcss_checks.js @@ -0,0 +1,20 @@ +// import { t } from '../util/locale'; + +export function validationMapCSSChecks() { + var validation = function(changes, graph, rules) { + var warnings = []; + var createdModified = ['created', 'modified']; + for (var i = 0; i < createdModified.length; i++) { + var entities = changes[createdModified[i]]; + for (var j = 0; j < entities.length; j++) { + var entity = entities[i]; + for (var k = 0; k < rules.length; k++) { + var rule = rules[k]; + rules.findWarnings(entity, rules); + } + } + } + return warnings; + }; + return validation; +} diff --git a/modules/validations/validation_collection.js b/modules/validations/validation_collection.js deleted file mode 100644 index f7dadab31..000000000 --- a/modules/validations/validation_collection.js +++ /dev/null @@ -1,16 +0,0 @@ -import { utilStringQs } from '../util'; -import { text as d3_text } from 'd3-request'; -import mapcssParse from 'mapcss-parse/source/index'; - -export function validationCollection() { - var validations = {}; - validations.init = function (callback) { - var validationsUrl = utilStringQs(window.location.hash)['validations']; - d3_text(validationsUrl, function(err, mapcss) { - if (err) return; - callback(mapcssParse(mapcss)); - }); - }; - - return validations; -} \ No newline at end of file From 5b1dee3779bd91880e75a47a23440c58f0fc6376 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Thu, 2 Aug 2018 12:47:47 -0400 Subject: [PATCH 06/27] add tests for mapcss checks ref #remote-presets --- modules/core/context.js | 19 +- modules/core/history.js | 2 +- modules/presets/index.js | 4 +- modules/util/mapcss_rule.js | 86 ++++++++- modules/validations/mapcss_checks.js | 4 +- test/spec/util/mapcss_rule.js | 275 +++++++++++++++++++++++++++ 6 files changed, 373 insertions(+), 17 deletions(-) create mode 100644 test/spec/util/mapcss_rule.js diff --git a/modules/core/context.js b/modules/core/context.js index 6c6d5ac81..a69b5f882 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -5,6 +5,7 @@ import _find from 'lodash-es/find'; import _forOwn from 'lodash-es/forOwn'; import _isObject from 'lodash-es/isObject'; import _isString from 'lodash-es/isString'; +import _map from 'lodash-es/map'; import { dispatch as d3_dispatch } from 'd3-dispatch'; @@ -15,8 +16,6 @@ import { import { select as d3_select } from 'd3-selection'; -import mapcssParse from 'mapcss-parse/source/index'; - import { t, currentLocale, @@ -456,14 +455,14 @@ export function coreContext() { locale = locale.split('-')[0]; } - if (utilExternalValidationRules()) { - var validationsUrl = utilStringQs(window.location.hash)['validations']; - d3_text(validationsUrl, function (err, mapcss) { - if (err) return; - var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcsS) }); - context.validationRules = function() { return validations; }; - }); - } + // if (utilExternalValidationRules()) { + // var validationsUrl = utilStringQs(window.location.hash).validations; + // d3_text(validationsUrl, function (err, mapcss) { + // if (err) return; + // var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig); }); + // context.validationRules = function() { return validations; }; + // }); + // } history = coreHistory(context); context.graph = history.graph; diff --git a/modules/core/history.js b/modules/core/history.js index e8d47a02d..9e45f17cb 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -284,7 +284,7 @@ export function coreHistory(context) { return _flatten( _map(Validations, function(fn) { var warnings; - if (fn === Validations.validationMapCSSChecks) { + if (fn === Validations.validationMapCSSChecks && context.hasOwnProperty('validationRules')) { warnings = fn()(changes, _stack[_index].graph, context.validationRules()); } else { warnings = fn()(changes, _stack[_index].graph); diff --git a/modules/presets/index.js b/modules/presets/index.js index a21a0dee3..04c61cfba 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -183,7 +183,7 @@ export function presetIndex() { }; all.fromExternal = function() { - var presetsUrl = utilStringQs(window.location.hash)['presets']; + var presetsUrl = utilStringQs(window.location.hash).presets; d3_json(presetsUrl, function(err, presets) { if (err) all.init(); all.overwrite(presets); @@ -213,7 +213,7 @@ export function presetIndex() { all.defaults = function(geometry, n) { var rec = _recent.matchGeometry(geometry).collection.slice(0, 4); var def = _uniq(rec.concat(_defaults[geometry].collection)).slice(0, n - 1); - var fin = _uniq(rec.concat(def).concat(all.item(geometry))).filter(i => i !== undefined); + var fin = _uniq(rec.concat(def).concat(all.item(geometry))).filter(function(d) { return d !== undefined; }); return presetCollection(fin); }; diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js index 712c75474..5429a9673 100644 --- a/modules/util/mapcss_rule.js +++ b/modules/util/mapcss_rule.js @@ -1 +1,85 @@ -export function mapcssRule() {} \ No newline at end of file +import _isMatch from 'lodash-es/isMatch'; + +export function utilMapCSSRule(selector) { + var ruleChecks = { + equals: function (tags) { + return _isMatch(tags, selector.equals); + }, + notEquals: function (tags) { + return !_isMatch(tags, selector.notEquals); + }, + absence: function(tags) { + return Object.keys(tags).indexOf(selector.absence) === -1; + }, + presence: function(tags) { + return Object.keys(tags).indexOf(selector.presence) > -1; + }, + greaterThan: function(tags) { + var key = Object.keys(selector.greaterThan)[0]; + var value = selector.greaterThan[key]; + return tags[key] > value; + }, + greaterThanEqual: function(tags) { + var key = Object.keys(selector.greaterThanEqual)[0]; + var value = selector.greaterThanEqual[key]; + return tags[key] >= value; + }, + lessThan: function(tags) { + var key = Object.keys(selector.lessThan)[0]; + var value = selector.lessThan[key]; + return tags[key] < value; + }, + lessThanEqual: function(tags) { + var key = Object.keys(selector.lessThanEqual)[0]; + var value = selector.lessThanEqual[key]; + return tags[key] <= value; + }, + positiveRegex: function(tags) { + var tagKey = Object.keys(selector.positiveRegex)[0]; + var expression = selector.positiveRegex[tagKey].join('|'); + var regex = new RegExp(expression); + return regex.test(tags[tagKey]); + }, + negativeRegex: function(tags) { + var tagKey = Object.keys(selector.negativeRegex)[0]; + var expression = selector.negativeRegex[tagKey].join('|'); + var regex = new RegExp(expression); + return !regex.test(tags[tagKey]); + } + }; + + var rule = { + ruleChecks: ruleChecks, + type: Object.keys(selector).indexOf('error') > -1 ? 'error' : 'warning', + buildChecks: function() { + return Object.keys(selector) + .filter(function(key) { return key !== 'geometry' && key !== 'error' && key !== 'warning'; }) + .map(function(key) { return ruleChecks[key]; }); + + }, + matches: function(entity) { + return this.buildChecks().every(function(check) { return check(entity.tags); }); + + }, + geometryMatches: function(entity, graph) { + if (entity.type === 'node' || entity.type === 'relation') { + return selector.geometry === entity.type; + } else if (entity.type === 'way') { + return selector.geometry === entity.geometry(graph); + } + + }, + findWarnings: function (entity, graph, warnings) { + if (this.geometryMatches(entity, graph) && this.matches(entity)) { + warnings.push({ + id: 'mapcss_' + rule.type, + message: selector[rule.type], + entity: entity + }); + } + + } + }; + + return rule; +} diff --git a/modules/validations/mapcss_checks.js b/modules/validations/mapcss_checks.js index 21d57ba1d..f7c508ae8 100644 --- a/modules/validations/mapcss_checks.js +++ b/modules/validations/mapcss_checks.js @@ -1,5 +1,3 @@ -// import { t } from '../util/locale'; - export function validationMapCSSChecks() { var validation = function(changes, graph, rules) { var warnings = []; @@ -10,7 +8,7 @@ export function validationMapCSSChecks() { var entity = entities[i]; for (var k = 0; k < rules.length; k++) { var rule = rules[k]; - rules.findWarnings(entity, rules); + rule.findWarnings(entity, graph, warnings); } } } diff --git a/test/spec/util/mapcss_rule.js b/test/spec/util/mapcss_rule.js new file mode 100644 index 000000000..5be819a16 --- /dev/null +++ b/test/spec/util/mapcss_rule.js @@ -0,0 +1,275 @@ +describe('iD.utilMapCSSRule', function() { + var entities = [ + iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}), + iD.Entity({ type: 'node', tags: { man_made: 'water_tap' }}), + iD.Entity({ type: 'node', tags: { amenity: 'marketplace', height: 0 }}), + iD.Entity({ type: 'node', tags: { amenity: 'school', height: 5, width: 3 }}), + iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}), + iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}), + ]; + var selectors = [ + { + 'geometry':'node', + 'equals':{'amenity':'marketplace'}, + 'absence':'name', + 'warning':'throwWarning: "[amenity=marketplace]: MapRules preset \'Market\': must be coupled with name";' + }, + { + 'geometry':'node', + 'equals':{'man_made':'water_tap'}, + 'absence':'name', + 'warning':'throwWarning: "[amenity=drinking_water][man_made=water_tap]: MapRules preset \'Water Tap\': must be coupled with name";' + }, + { + 'geometry':'node', + 'equals':{'amenity':'marketplace'}, + 'presence':'height', + 'lessThanEqual': { 'height': 0 }, + 'warning':'throwWarning: "[amenity=marketplace]: height must be greater than 0";' + }, + { + 'geometry': 'node', + 'equals': {'amenity': 'school'}, + 'greaterThan': { 'height': 0 }, + 'greaterThanEqual': { 'width': 1 }, + 'lessThanEqual': { 'width': 10 }, + 'lessThan': { 'height': 10 }, + 'warning': 'this is the warning!' + }, + { + 'geometry': 'node', + 'presence': 'amenity', + 'positiveRegex': { amenity: ['^school$', '^healthcare$'] }, + 'error': 'amenity cannot be healthcare or school!' + }, + { + 'geometry': 'node', + 'presence': 'amenity', + 'negativeRegex': { amenity: ['^school$', '^healthcare$'] }, + 'error': 'amenity must be healtcare or school!' + } + ]; + var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s); }); + it ('turns selector object in mapcssRule', function () { + var ruleKeys = ['ruleChecks', 'type','buildChecks','matches','geometryMatches','findWarnings']; + rules.forEach(function(rule) { + expect(Object.keys(rule)).to.eql(ruleKeys); + }); + }); + describe('#type', function() { + it('is either error or warning', function() { + selectors.forEach(function(s) { + expect(['error', 'warning'].indexOf(iD.utilMapCSSRule(s).type)).to.be.greaterThan(-1); + }); + }); + }); + describe('#geometryMatches', function() { + it('determines if entity and rule geometries match', function() { + var node = iD.Entity({ type: 'node'}); + var way = iD.Entity({ type: 'way'}); + var graph = iD.Graph([node, way]); + rules.forEach(function(rule) { + expect(rule.geometryMatches(node, graph)).to.be.true; + expect(rule.geometryMatches(way, graph)).to.be.false; + }); + }); + }); + describe('#buildsChecks', function() { + it('builds array of MapCSS rule check functions to run entities against', function() { + rules.forEach(function(rule) { + expect(rule.buildChecks().every(function(fn) { return fn instanceof Function; })).to.be.true; + }); + }); + }); + describe('#matches', function() { + it('determines if an entity matches the MapCSS rule checks', function() { + var node = iD.Entity({ type: 'node', tags: { power: 'tower' }}); + rules.forEach(function(rule, i) { + expect(rule.matches(entities[i])).to.be.true; + expect(rule.matches(node)).to.be.false; + }); + }); + }); + describe('#ruleChecks', function() { + describe('equals', function() { + it('is true when entity.tags intersects selector.equals', function() { + var pseudoSelector = { equals: {'amenity': 'school'} }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var school = iD.Entity({ type: 'node', tags: { amenity: 'school' }}); + expect(pseudoRule.ruleChecks.equals(school.tags)).to.be.true; + }); + it('is false when entity.tags intersects selector.equals', function() { + var pseudoSelector = { equals: { 'man_made': 'water_tap'} }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); + expect(pseudoRule.ruleChecks.equals(school.tags)).to.be.false; + }); + }); + describe('notEquals', function() { + it('is true when entity.tags does not intersect selector.notEquals', function() { + var pseudoSelector = { notEquals: { 'man_made': 'water_tap'} }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); + expect(pseudoRule.ruleChecks.notEquals(school.tags)).to.be.true; + }); + it('is false when entity.tags does not intersect selector.notEquals', function() { + var pseudoSelector = { notEquals: { 'amenity': 'school'} }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); + expect(pseudoRule.ruleChecks.notEquals(school.tags)).to.be.false; + }); + }); + describe('presence', function() { + it('is true when entity.tags\' key s include selector.presence', function() { + var pseudoSelector = { presence: 'name' }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var kHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace', name: 'Kensington Square' }}); + expect(pseudoRule.ruleChecks.presence(kHouse.tags)).to.be.true; + }); + it('is false when entity tags\' keys do not include selector.presence', function() { + var pseudoSelector = { presence: 'name' }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var notKHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}); + expect(pseudoRule.ruleChecks.presence(notKHouse.tags)).to.be.false; + }); + }); + describe('absence', function() { + it('is true when entity.tags\' keys do not include selector.absence', function() { + var pseudoSelector = { absence: 'name' }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var notKHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}); + expect(pseudoRule.ruleChecks.absence(notKHouse.tags)).to.be.true; + }); + it('is false when entity.tags\' keys include selector.absence', function() { + var pseudoSelector = { absence: 'name' }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var kHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace', name: 'Kensington Square' }}); + expect(pseudoRule.ruleChecks.presence(kHouse.tags)).to.be.false; + }); + }); + describe('greaterThan', function() { + it('is true when entity.tags\' equivalent value is greater than selector.greaterThan', function() { + var pseudoSelector = { greaterThan: { height: 10 }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var tallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9000 }}); + expect(pseudoRule.ruleChecks.greaterThan(tallSchool.tags)).to.be.true; + }); + it('is false when entity.tags\' equivalent value is less than or equal to selector.greaterThan', function() { + var pseudoSelector = { greaterThan: { height: 10 }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9 }}); + expect(pseudoRule.ruleChecks.greaterThan(smallSchool.tags)).to.be.false; + }); + }); + describe('greaterThanEqual', function() { + it('is true when entity.tags\' equivalent value is greater than or equal to selector.greaterThanEqual', function() { + var pseudoSelector = { greaterThanEqual: { height: 10 } }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var okHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 10 }}); + expect(pseudoRule.ruleChecks.greaterThanEqual(okHeightSchool.tags)).to.be.true; + }); + it('is false when entity.tags\' equivalent value is less than to selector.greaterThanEqual', function() { + var pseudoSelector = { greaterThanEqual: { height: 10 }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9 }}); + expect(pseudoRule.ruleChecks.greaterThanEqual(smallSchool.tags)).to.be.false; + }); + }); + describe('lessThan', function() { + it('is true when entity.tags\' equivalent value is less than to selector.lessThan', function() { + var pseudoSelector = { lessThan: { height: 10 } }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 3 }}); + expect(pseudoRule.ruleChecks.lessThan(smallSchool.tags)).to.be.tru; + }); + it('is false when entity.tags\' equivalent value is greater than or equal to selector.lessThan', function() { + var pseudoSelector = { lessThan: { height: 10 } }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var notOkHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 10 }}); + expect(pseudoRule.ruleChecks.lessThan(notOkHeightSchool.tags)).to.be.false; + }); + }); + describe('lessThanEqual', function() { + it('is true when entity.tags\' equivalent value is less than or equal to to selector.lessThan', function() { + var pseudoSelector = { lessThanEqual: { height: 10 } }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var okHeightSchool = iD.Entity({ type: 'node', tags: { 'amenity': 'school', 'height': 10 }}); + expect(pseudoRule.ruleChecks.lessThanEqual(okHeightSchool.tags)).to.be.true; + }); + it('is false when entity.tags\' equivalent value is greater than to selector.lessThan', function() { + var pseudoSelector = { lessThanEqual: { height: 10 } }; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var notOkHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 11 }}); + expect(pseudoRule.ruleChecks.lessThanEqual(notOkHeightSchool.tags)).to.be.false; + }); + }); + describe('positiveRegex', function() { + it('is true when entity.tags\' equivalent value matches regular expression built from selector.positiveRegex', function() { + var pseudoSelector = { positiveRegex: { amenity: ['^school$', '^healthcare$'] }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var okAmenities = [ + iD.Entity({ type: 'node', tags: { amenity: 'school' }}), + iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}) + ]; + okAmenities.forEach(function(amenity) { + expect(pseudoRule.ruleChecks.positiveRegex(amenity.tags)).to.be.true; + }); + }); + it('is false when entity.tags\' equivalent value does not match regular expression built from selector.positiveRegex', function() { + var pseudoSelector = { positiveRegex: { amenity: ['^school$', '^healthcare$'] }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var notOkAmenities = [ + iD.Entity({ type: 'node', tags: { amenity: 'parking' }}), + iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}) + ]; + notOkAmenities.forEach(function(amenity) { + expect(pseudoRule.ruleChecks.positiveRegex(amenity.tags)).to.be.false; + }); + }); + }); + describe('negativeRegex', function() { + it('is true when entity.tags\' equivalent value does not match regular exprsesion built from selector.negativeRegex', function() { + var pseudoSelector = { negativeRegex: { amenity: ['^school$', '^healthcare$'] }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var notOkAmenities = [ + iD.Entity({ type: 'node', tags: { amenity: 'parking' }}), + iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}) + ]; + notOkAmenities.forEach(function(amenity) { + expect(pseudoRule.ruleChecks.negativeRegex(amenity.tags)).to.be.true; + }); + }); + it('is false when entity.tags\' equivalent value matches regular expression built from selector.negativeRegex', function() { + var pseudoSelector = { negativeRegex: { amenity: ['^school$', '^healthcare$'] }}; + var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var okAmenities = [ + iD.Entity({ type: 'node', tags: { amenity: 'school' }}), + iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}) + ]; + okAmenities.forEach(function(amenity) { + expect(pseudoRule.ruleChecks.negativeRegex(amenity.tags)).to.be.false; + }); + }); + }); + }); + describe('#findWarnings', function() { + it('adds found warnings to warnings array', function() { + var graph = iD.Graph([entities]); + var warnings = []; + + rules.forEach(function(rule) { + entities.forEach(function(entity) { + rule.findWarnings(entity, graph, warnings); + }); + }); + + warnings.forEach(function(warning) { + // console.log(warning); + // expect(warning.message).to.not.be.null; + // expect(['mapcss_warning', 'mapcss_error'].indexOf(warning.id)).to.be.greaterThan(-1); + // expect(warning.entity).to.be.instanceOf(iD.Entity); + }); + }); + }); +}); + From b93444de75a61994634b72677fc3ee043b1b0e96 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Thu, 2 Aug 2018 18:03:13 -0400 Subject: [PATCH 07/27] working on isArea-ish equivalent for mapcss selector objects ref #remote-presets --- css/80_app.css | 4 + modules/core/context.js | 18 +-- modules/core/history.js | 18 +-- modules/presets/index.js | 2 +- modules/ui/commit_warnings.js | 158 +++++++++++++++------------ modules/util/mapcss_rule.js | 46 +++++++- modules/validations/mapcss_checks.js | 4 +- test/spec/util/mapcss_rule.js | 6 - 8 files changed, 158 insertions(+), 98 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index aeba94aea..fa948cefa 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -3909,6 +3909,10 @@ svg.mouseclick use.right { background: #ffb; } +.mode-save .error-section { + background: #ffa5a5; +} + .mode-save .warning-section .changeset-list button { border-left: 1px solid #ccc; } diff --git a/modules/core/context.js b/modules/core/context.js index a69b5f882..5fd428894 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -7,6 +7,8 @@ import _isObject from 'lodash-es/isObject'; import _isString from 'lodash-es/isString'; import _map from 'lodash-es/map'; +import mapcssParse from 'mapcss-parse'; + import { dispatch as d3_dispatch } from 'd3-dispatch'; import { @@ -455,14 +457,14 @@ export function coreContext() { locale = locale.split('-')[0]; } - // if (utilExternalValidationRules()) { - // var validationsUrl = utilStringQs(window.location.hash).validations; - // d3_text(validationsUrl, function (err, mapcss) { - // if (err) return; - // var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig); }); - // context.validationRules = function() { return validations; }; - // }); - // } + if (utilExternalValidationRules()) { + var validationsUrl = utilStringQs(window.location.hash).validations; + d3_text(validationsUrl, function (err, mapcss) { + if (err) return; + var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context.presets().areaKeys()); }); + context.validationRules = function() { return validations; }; + }); + } history = coreHistory(context); context.graph = history.graph; diff --git a/modules/core/history.js b/modules/core/history.js index 9e45f17cb..cd7dbffbe 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -9,6 +9,7 @@ import _isEmpty from 'lodash-es/isEmpty'; import _forEach from 'lodash-es/forEach'; import _map from 'lodash-es/map'; import _omit from 'lodash-es/omit'; +import _pickBy from 'lodash-es/pickBy'; import _reject from 'lodash-es/reject'; import _values from 'lodash-es/values'; import _without from 'lodash-es/without'; @@ -281,15 +282,16 @@ export function coreHistory(context) { validate: function(changes) { + // strip mapcss checks if no mapcss to check against. + var validationsToRun = _pickBy(Validations, function(validation) { + return validation === Validations.validationMapCSSChecks && context.hasOwnProperty('validationRules') || + validation !== Validations.validationMapCSSChecks; + }); return _flatten( - _map(Validations, function(fn) { - var warnings; - if (fn === Validations.validationMapCSSChecks && context.hasOwnProperty('validationRules')) { - warnings = fn()(changes, _stack[_index].graph, context.validationRules()); - } else { - warnings = fn()(changes, _stack[_index].graph); - } - return warnings; + _map(validationsToRun, function(fn, name) { + return name === 'validationMapCSSChecks' ? + fn()(changes, _stack[_index].graph, context.validationRules()) : + fn()(changes, _stack[_index].graph); }) ); }, diff --git a/modules/presets/index.js b/modules/presets/index.js index 04c61cfba..ec47773dd 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -121,7 +121,7 @@ export function presetIndex() { areaKeys[key][value] = true; } }); - + console.log(areaKeys); return areaKeys; }; diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 0c83cfbf5..74174772a 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -3,91 +3,107 @@ import { modeSelect } from '../modes'; import { svgIcon } from '../svg'; import { tooltip } from '../util/tooltip'; import { utilEntityOrMemberSelector } from '../util'; - +import _reduce from 'lodash-es/reduce'; +import _forEach from 'lodash-es/forEach'; export function uiCommitWarnings(context) { function commitWarnings(selection) { var changes = context.history().changes(); - var warnings = context.history().validate(changes); + var validations = context.history().validate(changes); - var container = selection.selectAll('.warning-section') - .data(warnings.length ? [0] : []); - - container.exit() - .remove(); - - var containerEnter = container.enter() - .append('div') - .attr('class', 'modal-section warning-section fillL2'); - - containerEnter - .append('h3') - .text(t('commit.warnings')); - - containerEnter - .append('ul') - .attr('class', 'changeset-list'); - - container = containerEnter - .merge(container); - - - var items = container.select('ul').selectAll('li') - .data(warnings); - - items.exit() - .remove(); - - var itemsEnter = items.enter() - .append('li') - .attr('class', 'warning-item'); - - itemsEnter - .call(svgIcon('#iD-icon-alert', 'pre-text')); - - itemsEnter - .append('strong') - .text(function(d) { return d.message; }); - - itemsEnter.filter(function(d) { return d.tooltip; }) - .call(tooltip() - .title(function(d) { return d.tooltip; }) - .placement('top') - ); - - items = itemsEnter - .merge(items); - - items - .on('mouseover', mouseover) - .on('mouseout', mouseout) - .on('click', warningClick); - - - function mouseover(d) { - if (d.entity) { - context.surface().selectAll( - utilEntityOrMemberSelector([d.entity.id], context.graph()) - ).classed('hover', true); + validations = _reduce(validations, function(validations, val) { + var type = val.id === 'mapcss_error' ? 'error' : 'warning'; + if (validations.hasOwnProperty(type)) { + validations[type].push(val); + } else { + validations[type] = [val]; } - } + return validations; + }, {}); + + _forEach(validations, function(instances, type) { + + var section = type + '-section'; + var instanceItem = type + '-item'; + + var container = selection.selectAll('.' + section) + .data(instances.length ? [0] : []); + + container.exit() + .remove(); + + var containerEnter = container.enter() + .append('div') + .attr('class', 'modal-section ' + section + ' fillL2'); + + containerEnter + .append('h3') + .text(type === 'warning' ? t('commit.warnings') : t('commit.errors')); + + containerEnter + .append('ul') + .attr('class', 'changeset-list'); + + container = containerEnter + .merge(container); - function mouseout() { - context.surface().selectAll('.hover') - .classed('hover', false); - } + var items = container.select('ul').selectAll('li') + .data(instances); + + items.exit() + .remove(); + + var itemsEnter = items.enter() + .append('li') + .attr('class', instanceItem); + + itemsEnter + .call(svgIcon('#iD-icon-alert', 'pre-text')); + + itemsEnter + .append('strong') + .text(function(d) { return d.message; }); + + itemsEnter.filter(function(d) { return d.tooltip; }) + .call(tooltip() + .title(function(d) { return d.tooltip; }) + .placement('top') + ); + + items = itemsEnter + .merge(items); + + items + .on('mouseover', mouseover) + .on('mouseout', mouseout) + .on('click', warningClick); - function warningClick(d) { - if (d.entity) { - context.map().zoomTo(d.entity); - context.enter(modeSelect(context, [d.entity.id])); + function mouseover(d) { + if (d.entity) { + context.surface().selectAll( + utilEntityOrMemberSelector([d.entity.id], context.graph()) + ).classed('hover', true); + } } - } + + function mouseout() { + context.surface().selectAll('.hover') + .classed('hover', false); + } + + + function warningClick(d) { + if (d.entity) { + context.map().zoomTo(d.entity); + context.enter(modeSelect(context, [d.entity.id])); + } + } + }); } diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js index 5429a9673..ce5b25b71 100644 --- a/modules/util/mapcss_rule.js +++ b/modules/util/mapcss_rule.js @@ -1,6 +1,7 @@ import _isMatch from 'lodash-es/isMatch'; +import _reduce from 'lodash-es/reduce'; -export function utilMapCSSRule(selector) { +export function utilMapCSSRule(selector, areaKeys) { var ruleChecks = { equals: function (tags) { return _isMatch(tags, selector.equals); @@ -60,12 +61,53 @@ export function utilMapCSSRule(selector) { matches: function(entity) { return this.buildChecks().every(function(check) { return check(entity.tags); }); + }, + // borrowed from Way#isArea() + inferGeometry() { + // keys is a map of osm key + all found values across the selector map. + var selectorKeys = _reduce(Object.keys(selector), function(expectedTags, key) { + var values; + if(/regex/gi.test(key)) { + Object.keys(p[key]).forEach(function(regexKey) { + values = p[key][rKey].map(function(val) { return val.replace(/\$|\^/g, '') }) + if (expectedTags.hasOwnProperty(regexKey)) { + values = values.concat(expectedTags[regexKey]) + + } + expectedTags[regexKey] = values + }) + } + if (key === 'equals') { + var equalsKey = Object.keys(p[key])[0] + values = [p[key][equalsKey]] + if (expectedTags.hasOwnProperty(equalsKey)) { + values = values.concat(expectedTags[equalsKey]) + } + expectedTags[equalsKey] = values + } + return expectedTags + }, {}) + + if (Object.keys(keys).indexOf('area') > -1) { + inferredGeometry = 'area'; + return; + } + + for (var key in Object.keys(keys)) { + if (key in areaKeys) { + if (keys[key] !== 'absence' && key in ) { + + } + } + + } + }, geometryMatches: function(entity, graph) { if (entity.type === 'node' || entity.type === 'relation') { return selector.geometry === entity.type; } else if (entity.type === 'way') { - return selector.geometry === entity.geometry(graph); + return this.inferGeometry() === entity.geometry(graph); } }, diff --git a/modules/validations/mapcss_checks.js b/modules/validations/mapcss_checks.js index f7c508ae8..e287d2bd8 100644 --- a/modules/validations/mapcss_checks.js +++ b/modules/validations/mapcss_checks.js @@ -1,11 +1,11 @@ export function validationMapCSSChecks() { - var validation = function(changes, graph, rules) { + var validation = function(changes, graph, rules, areaKeys) { var warnings = []; var createdModified = ['created', 'modified']; for (var i = 0; i < createdModified.length; i++) { var entities = changes[createdModified[i]]; for (var j = 0; j < entities.length; j++) { - var entity = entities[i]; + var entity = entities[j]; for (var k = 0; k < rules.length; k++) { var rule = rules[k]; rule.findWarnings(entity, graph, warnings); diff --git a/test/spec/util/mapcss_rule.js b/test/spec/util/mapcss_rule.js index 5be819a16..40a65f6fa 100644 --- a/test/spec/util/mapcss_rule.js +++ b/test/spec/util/mapcss_rule.js @@ -41,12 +41,6 @@ describe('iD.utilMapCSSRule', function() { 'presence': 'amenity', 'positiveRegex': { amenity: ['^school$', '^healthcare$'] }, 'error': 'amenity cannot be healthcare or school!' - }, - { - 'geometry': 'node', - 'presence': 'amenity', - 'negativeRegex': { amenity: ['^school$', '^healthcare$'] }, - 'error': 'amenity must be healtcare or school!' } ]; var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s); }); From f5c21ddaafe9f34c84da8d4546410a570d5c40b0 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Wed, 8 Aug 2018 08:50:45 -0400 Subject: [PATCH 08/27] some geometry inferring!!! ref #remote-presets --- modules/core/context.js | 18 +++++----- modules/presets/index.js | 1 - modules/util/mapcss_rule.js | 70 ++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/modules/core/context.js b/modules/core/context.js index 5fd428894..d4e65f4b3 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -7,7 +7,7 @@ import _isObject from 'lodash-es/isObject'; import _isString from 'lodash-es/isString'; import _map from 'lodash-es/map'; -import mapcssParse from 'mapcss-parse'; +// import mapcssParse from 'mapcss-parse'; import { dispatch as d3_dispatch } from 'd3-dispatch'; @@ -457,14 +457,14 @@ export function coreContext() { locale = locale.split('-')[0]; } - if (utilExternalValidationRules()) { - var validationsUrl = utilStringQs(window.location.hash).validations; - d3_text(validationsUrl, function (err, mapcss) { - if (err) return; - var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context.presets().areaKeys()); }); - context.validationRules = function() { return validations; }; - }); - } + // if (utilExternalValidationRules()) { + // var validationsUrl = utilStringQs(window.location.hash).validations; + // d3_text(validationsUrl, function (err, mapcss) { + // if (err) return; + // var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context.presets().areaKeys()); }); + // context.validationRules = function() { return validations; }; + // }); + // } history = coreHistory(context); context.graph = history.graph; diff --git a/modules/presets/index.js b/modules/presets/index.js index ec47773dd..522882c48 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -121,7 +121,6 @@ export function presetIndex() { areaKeys[key][value] = true; } }); - console.log(areaKeys); return areaKeys; }; diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js index ce5b25b71..c413b895d 100644 --- a/modules/util/mapcss_rule.js +++ b/modules/util/mapcss_rule.js @@ -1,5 +1,5 @@ import _isMatch from 'lodash-es/isMatch'; -import _reduce from 'lodash-es/reduce'; +import _intersection from 'lodash-es/intersection'; export function utilMapCSSRule(selector, areaKeys) { var ruleChecks = { @@ -63,53 +63,53 @@ export function utilMapCSSRule(selector, areaKeys) { }, // borrowed from Way#isArea() - inferGeometry() { - // keys is a map of osm key + all found values across the selector map. - var selectorKeys = _reduce(Object.keys(selector), function(expectedTags, key) { - var values; - if(/regex/gi.test(key)) { - Object.keys(p[key]).forEach(function(regexKey) { - values = p[key][rKey].map(function(val) { return val.replace(/\$|\^/g, '') }) - if (expectedTags.hasOwnProperty(regexKey)) { - values = values.concat(expectedTags[regexKey]) - - } - expectedTags[regexKey] = values - }) - } - if (key === 'equals') { - var equalsKey = Object.keys(p[key])[0] - values = [p[key][equalsKey]] - if (expectedTags.hasOwnProperty(equalsKey)) { - values = values.concat(expectedTags[equalsKey]) - } - expectedTags[equalsKey] = values + inferGeometry: function (tagMap, areaKeys) { + var lineKeys = { + highway: { + rest_area: true, + services: true + }, + railway: { + roundhouse: true, + station: true, + traverser: true, + turntable: true, + wash: true } - return expectedTags - }, {}) + }; + var isAreaKeyBlackList = function(key) { + return _intersection(tagMap[key], Object.keys(areaKeys[key])).length > 0; + }; + var isLineKeysWhiteList = function(key) { + return _intersection(tagMap[key], Object.keys(lineKeys[key])).length > 0; + }; - if (Object.keys(keys).indexOf('area') > -1) { - inferredGeometry = 'area'; - return; + if (tagMap.hasOwnProperty('area')) { + if (tagMap.area.indexOf('yes') > -1) { + return 'area'; + } + if (tagMap.area.indexOf('no') > -1) { + return 'line'; + } } - for (var key in Object.keys(keys)) { - if (key in areaKeys) { - if (keys[key] !== 'absence' && key in ) { - - } + for (var key in tagMap) { + if (key in areaKeys && !isAreaKeyBlackList(key)) { + return 'area'; + } + if (key in lineKeys && isLineKeysWhiteList(key)) { + return 'area'; } - } + return 'line'; }, geometryMatches: function(entity, graph) { if (entity.type === 'node' || entity.type === 'relation') { return selector.geometry === entity.type; } else if (entity.type === 'way') { - return this.inferGeometry() === entity.geometry(graph); + return 'area' === entity.geometry(graph); } - }, findWarnings: function (entity, graph, warnings) { if (this.geometryMatches(entity, graph) && this.matches(entity)) { From 3a0f8ddd09b5c4752dccf9a6651736e7b4cfba5a Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Wed, 8 Aug 2018 13:21:35 -0400 Subject: [PATCH 09/27] infer geometries ref #remote-presets --- data/core.yaml | 1 + modules/core/context.js | 45 +++++++++---------- modules/presets/index.js | 1 + modules/ui/commit_warnings.js | 3 +- modules/util/mapcss_rule.js | 8 ++-- package.json | 1 + test/spec/util/mapcss_rule.js | 81 +++++++++++++++++++++++++++++++++-- 7 files changed, 108 insertions(+), 32 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index b317df32a..476f165cf 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -297,6 +297,7 @@ en: cancel: Cancel changes: "{count} Changes" download_changes: Download osmChange file + errors: Errors warnings: Warnings modified: Modified deleted: Deleted diff --git a/modules/core/context.js b/modules/core/context.js index d4e65f4b3..626545886 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -7,7 +7,7 @@ import _isObject from 'lodash-es/isObject'; import _isString from 'lodash-es/isString'; import _map from 'lodash-es/map'; -// import mapcssParse from 'mapcss-parse'; +import { parse as parseMapCSS } from 'mapcss-parse'; import { dispatch as d3_dispatch } from 'd3-dispatch'; @@ -314,12 +314,6 @@ export function coreContext() { return features.hasHiddenConnections(entity, graph); }; - - /* Presets */ - var presets; - context.presets = function() { return presets; }; - context.overwritePresets = function(newPresets) { presets.overwrite(newPresets); }; - /* Map */ var map; context.map = function() { return map; }; @@ -457,14 +451,24 @@ export function coreContext() { locale = locale.split('-')[0]; } - // if (utilExternalValidationRules()) { - // var validationsUrl = utilStringQs(window.location.hash).validations; - // d3_text(validationsUrl, function (err, mapcss) { - // if (err) return; - // var validations = _map(mapcssParse(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context.presets().areaKeys()); }); - // context.validationRules = function() { return validations; }; - // }); - // } + /* Presets */ + var presets; + presets = presetIndex(); + if (utilExternalPresets()) { + presets.fromExternal(); + } else { + presets.init(); + } + context.presets = function() { return presets; }; + + if (utilExternalValidationRules()) { + var validationsUrl = utilStringQs(window.location.hash).validations; + d3_text(validationsUrl, function (err, mapcss) { + if (err) return; + var validations = _map(parseMapCSS(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context); }); + context.validationRules = function() { return validations; }; + }); + } history = coreHistory(context); context.graph = history.graph; @@ -494,7 +498,6 @@ export function coreContext() { connection = services.osm; background = rendererBackground(context); features = rendererFeatures(context); - presets = presetIndex(); map = rendererMap(context); context.mouse = map.mouse; @@ -512,17 +515,9 @@ export function coreContext() { } }); + areaKeys = presets.areaKeys(); background.init(); features.init(); - - if (utilExternalPresets()) { - presets.fromExternal(); - } else { - presets.init(); - } - - areaKeys = presets.areaKeys(); - return utilRebind(context, dispatch, 'on'); } diff --git a/modules/presets/index.js b/modules/presets/index.js index 522882c48..aa1924032 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -186,6 +186,7 @@ export function presetIndex() { d3_json(presetsUrl, function(err, presets) { if (err) all.init(); all.overwrite(presets); + all.areaKeys(); }); return all; }; diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 74174772a..2317fc9f5 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -5,6 +5,7 @@ import { tooltip } from '../util/tooltip'; import { utilEntityOrMemberSelector } from '../util'; import _reduce from 'lodash-es/reduce'; import _forEach from 'lodash-es/forEach'; +import _uniqBy from 'lodash-es/uniqBy'; export function uiCommitWarnings(context) { @@ -24,7 +25,7 @@ export function uiCommitWarnings(context) { }, {}); _forEach(validations, function(instances, type) { - + instances = _uniqBy(instances, function(val) { return val.id + '_' + val.message.replace(/\s+/g,''); }); var section = type + '-section'; var instanceItem = type + '-item'; diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js index c413b895d..d2b3259de 100644 --- a/modules/util/mapcss_rule.js +++ b/modules/util/mapcss_rule.js @@ -1,7 +1,8 @@ import _isMatch from 'lodash-es/isMatch'; import _intersection from 'lodash-es/intersection'; +import { tagMap } from 'mapcss-parse'; -export function utilMapCSSRule(selector, areaKeys) { +export function utilMapCSSRule(selector, context) { var ruleChecks = { equals: function (tags) { return _isMatch(tags, selector.equals); @@ -63,7 +64,8 @@ export function utilMapCSSRule(selector, areaKeys) { }, // borrowed from Way#isArea() - inferGeometry: function (tagMap, areaKeys) { + inferGeometry: function (tagMap, context) { + var areaKeys = context.presets().areaKeys(); var lineKeys = { highway: { rest_area: true, @@ -108,7 +110,7 @@ export function utilMapCSSRule(selector, areaKeys) { if (entity.type === 'node' || entity.type === 'relation') { return selector.geometry === entity.type; } else if (entity.type === 'way') { - return 'area' === entity.geometry(graph); + return this.inferGeometry(tagMap(selector), context) === entity.geometry(graph); } }, findWarnings: function (entity, graph, warnings) { diff --git a/package.json b/package.json index 3f2f448fc..52c1ac628 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "@mapbox/vector-tile": "^1.3.1", "diacritics": "1.3.0", "lodash-es": "4.17.10", + "mapcss-parse": "github:DigitalGlobe/mapcss-parse#12", "marked": "0.4.0", "node-diff3": "1.0.0", "osm-auth": "1.0.2", diff --git a/test/spec/util/mapcss_rule.js b/test/spec/util/mapcss_rule.js index 40a65f6fa..6c25c9e77 100644 --- a/test/spec/util/mapcss_rule.js +++ b/test/spec/util/mapcss_rule.js @@ -45,7 +45,7 @@ describe('iD.utilMapCSSRule', function() { ]; var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s); }); it ('turns selector object in mapcssRule', function () { - var ruleKeys = ['ruleChecks', 'type','buildChecks','matches','geometryMatches','findWarnings']; + var ruleKeys = ['ruleChecks', 'type','buildChecks','matches', 'inferGeometry', 'geometryMatches','findWarnings']; rules.forEach(function(rule) { expect(Object.keys(rule)).to.eql(ruleKeys); }); @@ -246,6 +246,81 @@ describe('iD.utilMapCSSRule', function() { }); }); }); + describe('#inferGeometry', function() { + var amenityDerivedArea = { + selector: { + 'geometry': 'closedway', + 'presence': 'amenity', + 'positiveRegex': { amenity: ['^school$', '^healthcare$'] }, + 'error': 'amenity cannot be healthcare or school!' + }, + tagMap: { + amenity: [ 'school', 'healthcare' ] + } + }; + + var areaDerivedArea = { + selector: { + 'geometry': 'closedway', + 'equals': { area: 'yes' }, + }, + tagMap: { + amenity: [ 'school', 'healthcare' ], + area: [ 'yes' ] + } + }; + + var badAreaDerivedLine = { + selector: { + 'geometry': 'closedway', + 'equals': { 'area': 'no' } + }, + tagMap: { + area: ['no'] + } + }; + + var roundHouseRailwayDerivedArea = { + selector: { + 'geometry': 'closedway', + 'equals': { 'railway': 'roundhouse' } + }, + tagMap: { + railway: ['roundhouse'] + } + }; + + var justClosedWayDerivedLine = { + selector: { + 'geometry': 'closedway' + }, + tagMap: {} + }; + + var areaKeys = iD.Context().presets().areaKeys(); + var rule, geom; + + rule = iD.utilMapCSSRule(amenityDerivedArea.selector); + geom = rule.inferGeometry(amenityDerivedArea.tagMap, areaKeys); + expect(geom).to.be.eql('area'); + + rule = iD.utilMapCSSRule(areaDerivedArea.selector); + geom = rule.inferGeometry(areaDerivedArea.tagMap, areaKeys); + expect(geom).to.be.eql('area'); + + rule = iD.utilMapCSSRule(badAreaDerivedLine.selector); + geom = rule.inferGeometry(badAreaDerivedLine.tagMap); + expect(geom).to.be.eql('line'); + + rule = iD.utilMapCSSRule(roundHouseRailwayDerivedArea.selector); + geom = rule.inferGeometry(roundHouseRailwayDerivedArea.tagMap, areaKeys); + expect(geom).to.be.eql('area'); + + rule = iD.utilMapCSSRule(justClosedWayDerivedLine.selector); + geom = rule.inferGeometry(justClosedWayDerivedLine.tagMap); + expect(geom).to.be.eql('line'); + + }); describe('#findWarnings', function() { it('adds found warnings to warnings array', function() { var graph = iD.Graph([entities]); @@ -257,12 +332,12 @@ describe('iD.utilMapCSSRule', function() { }); }); - warnings.forEach(function(warning) { + // warnings.forEach(function(warning) { // console.log(warning); // expect(warning.message).to.not.be.null; // expect(['mapcss_warning', 'mapcss_error'].indexOf(warning.id)).to.be.greaterThan(-1); // expect(warning.entity).to.be.instanceOf(iD.Entity); - }); + // }); }); }); }); From f168f854960803c7aee9732c4adcdd24cabb129f Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Fri, 10 Aug 2018 09:16:59 -0400 Subject: [PATCH 10/27] passing mapcss test w/out parser, use that on sever ref #remote-presets --- dist/locales/en.json | 33 ++++- modules/core/context.js | 4 +- modules/util/mapcss_rule.js | 46 ++++++- modules/validations/mapcss_checks.js | 2 +- package.json | 1 - test/index.html | 5 +- test/spec/presets/index.js | 2 +- test/spec/spec_helpers.js | 1 - test/spec/util/mapcss_rule.js | 172 ++++++++++++++------------- 9 files changed, 165 insertions(+), 101 deletions(-) diff --git a/dist/locales/en.json b/dist/locales/en.json index e4b2ccb4d..aab4fc36b 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -376,6 +376,7 @@ "cancel": "Cancel", "changes": "{count} Changes", "download_changes": "Download osmChange file", + "errors": "Errors", "warnings": "Warnings", "modified": "Modified", "deleted": "Deleted", @@ -6895,6 +6896,27 @@ }, "name": "Hike & Bike" }, + "kelkkareitit": { + "attribution": { + "text": "© Kelkkareitit.fi" + }, + "description": "Kelkkareitit.fi snowmobile trails from OSM (Nordic coverage)", + "name": "Nordic snowmobile overlay" + }, + "lantmateriet-orto1960": { + "attribution": { + "text": "© Lantmäteriet, CC0" + }, + "description": "Mosaic of Swedish orthophotos from the period 1949-1970.", + "name": "Lantmäteriet Historic Orthophoto 1960" + }, + "lantmateriet-orto1975": { + "attribution": { + "text": "© Lantmäteriet, CC0" + }, + "description": "Mosaic of Swedish orthophotos from the period 1970-1980. To be expanded.", + "name": "Lantmäteriet Historic Orthophoto 1975" + }, "mapbox_locator_overlay": { "attribution": { "text": "Terms & Feedback" @@ -6931,8 +6953,8 @@ "attribution": { "text": "© Lantmäteriet" }, - "description": "Scan of ´Economic maps´ ca 1950-1980", - "name": "Lantmäteriet Economic Map (historic)" + "description": "Scan of ´Economic maps´ ca. 1950-1980", + "name": "Lantmäteriet Economic Map ca 1950-1980" }, "qa_no_address": { "attribution": { @@ -6946,6 +6968,13 @@ }, "name": "skobbler" }, + "skoterleder": { + "attribution": { + "text": "© Skoterleder.org" + }, + "description": "Snowmobile trails", + "name": "Snowmobile map Sweden" + }, "stamen-terrain-background": { "attribution": { "text": "Map tiles by Stamen Design, under CC BY 3.0. Data by OpenStreetMap, under ODbL" diff --git a/modules/core/context.js b/modules/core/context.js index 626545886..25a72c2ae 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -7,8 +7,6 @@ import _isObject from 'lodash-es/isObject'; import _isString from 'lodash-es/isString'; import _map from 'lodash-es/map'; -import { parse as parseMapCSS } from 'mapcss-parse'; - import { dispatch as d3_dispatch } from 'd3-dispatch'; import { @@ -465,7 +463,7 @@ export function coreContext() { var validationsUrl = utilStringQs(window.location.hash).validations; d3_text(validationsUrl, function (err, mapcss) { if (err) return; - var validations = _map(parseMapCSS(mapcss), function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context); }); + var validations = _map(mapcss, function(mapcss) { return utilMapCSSRule(mapcss, context.presets().areaKeys()); }); context.validationRules = function() { return validations; }; }); } diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js index d2b3259de..7187bdb90 100644 --- a/modules/util/mapcss_rule.js +++ b/modules/util/mapcss_rule.js @@ -1,8 +1,8 @@ import _isMatch from 'lodash-es/isMatch'; import _intersection from 'lodash-es/intersection'; -import { tagMap } from 'mapcss-parse'; +import _reduce from 'lodash-es/reduce'; -export function utilMapCSSRule(selector, context) { +export function utilMapCSSRule(selector, areaKeys) { var ruleChecks = { equals: function (tags) { return _isMatch(tags, selector.equals); @@ -59,13 +59,49 @@ export function utilMapCSSRule(selector, context) { .map(function(key) { return ruleChecks[key]; }); }, + buildTagMap: function() { + var selectorKeys = Object.keys(selector); + var tagMap = _reduce(selectorKeys, function (expectedTags, key) { + var values; + if (/regex/gi.test(key)) { + Object.keys(selector[key]).forEach(function(regexKey) { + values = selector[key][regexKey].map(function(val) { + return val.replace(/\$|\^/g, ''); + }); + + if (expectedTags.hasOwnProperty(regexKey)) { + values = values.concat(expectedTags[regexKey]); + } + + expectedTags[regexKey] = values; + }); + } + if (/(greater|less)Than(Equal)?|equals|presence/g.test(key)) { + var tagKey = /presence/.test(key) ? selector[key] : Object.keys(selector[key])[0]; + + values = (key === 'equals') ? [selector[key][tagKey]] : []; + + if (expectedTags.hasOwnProperty(tagKey)) { + values = (key === 'equals') ? values.concat(expectedTags[tagKey]) : []; + } + + expectedTags[tagKey] = values; + } + return expectedTags; + }, {}); + return tagMap; + }, matches: function(entity) { return this.buildChecks().every(function(check) { return check(entity.tags); }); }, + areaKeys: function() { + return areaKeys; + }, // borrowed from Way#isArea() - inferGeometry: function (tagMap, context) { - var areaKeys = context.presets().areaKeys(); + inferGeometry: function () { + var tagMap = this.buildTagMap(); + var areaKeys = this.areaKeys(); var lineKeys = { highway: { rest_area: true, @@ -110,7 +146,7 @@ export function utilMapCSSRule(selector, context) { if (entity.type === 'node' || entity.type === 'relation') { return selector.geometry === entity.type; } else if (entity.type === 'way') { - return this.inferGeometry(tagMap(selector), context) === entity.geometry(graph); + return this.inferGeometry(areaKeys) === entity.geometry(graph); } }, findWarnings: function (entity, graph, warnings) { diff --git a/modules/validations/mapcss_checks.js b/modules/validations/mapcss_checks.js index e287d2bd8..5f0d753d4 100644 --- a/modules/validations/mapcss_checks.js +++ b/modules/validations/mapcss_checks.js @@ -1,5 +1,5 @@ export function validationMapCSSChecks() { - var validation = function(changes, graph, rules, areaKeys) { + var validation = function(changes, graph, rules) { var warnings = []; var createdModified = ['created', 'modified']; for (var i = 0; i < createdModified.length; i++) { diff --git a/package.json b/package.json index 52c1ac628..3f2f448fc 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,6 @@ "@mapbox/vector-tile": "^1.3.1", "diacritics": "1.3.0", "lodash-es": "4.17.10", - "mapcss-parse": "github:DigitalGlobe/mapcss-parse#12", "marked": "0.4.0", "node-diff3": "1.0.0", "osm-auth": "1.0.2", diff --git a/test/index.html b/test/index.html index b094f4b32..f2d81848d 100644 --- a/test/index.html +++ b/test/index.html @@ -24,7 +24,6 @@ - @@ -89,7 +88,6 @@ - @@ -116,7 +114,6 @@ - @@ -138,7 +135,7 @@ - + diff --git a/test/spec/presets/index.js b/test/spec/presets/index.js index 248c4e132..742a0a6b7 100644 --- a/test/spec/presets/index.js +++ b/test/spec/presets/index.js @@ -192,7 +192,7 @@ describe('iD.presetIndex', function() { } }; var currentPresets = iD.Context().presets(); - var overwrittenPresets = iD.Context().overwritePresets(testPresets); + var overwrittenPresets = iD.Context().presets().overwrite(testPresets); expect(currentPresets).to.not.eql(overwrittenPresets); }); }); diff --git a/test/spec/spec_helpers.js b/test/spec/spec_helpers.js index f8b0b13f4..262159477 100644 --- a/test/spec/spec_helpers.js +++ b/test/spec/spec_helpers.js @@ -1,6 +1,5 @@ /* globals chai:false */ /* eslint no-extend-native:off */ - iD.debug = true; // disable things that use the network diff --git a/test/spec/util/mapcss_rule.js b/test/spec/util/mapcss_rule.js index 6c25c9e77..0052dc05d 100644 --- a/test/spec/util/mapcss_rule.js +++ b/test/spec/util/mapcss_rule.js @@ -43,9 +43,13 @@ describe('iD.utilMapCSSRule', function() { 'error': 'amenity cannot be healthcare or school!' } ]; - var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s); }); + var areaKeys = iD.Context().presets().areaKeys(); + var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s, areaKeys); }); it ('turns selector object in mapcssRule', function () { - var ruleKeys = ['ruleChecks', 'type','buildChecks','matches', 'inferGeometry', 'geometryMatches','findWarnings']; + var ruleKeys = [ + 'ruleChecks', 'type','buildChecks', 'buildTagMap', 'matches', + 'areaKeys', 'inferGeometry', 'geometryMatches','findWarnings' + ]; rules.forEach(function(rule) { expect(Object.keys(rule)).to.eql(ruleKeys); }); @@ -53,7 +57,7 @@ describe('iD.utilMapCSSRule', function() { describe('#type', function() { it('is either error or warning', function() { selectors.forEach(function(s) { - expect(['error', 'warning'].indexOf(iD.utilMapCSSRule(s).type)).to.be.greaterThan(-1); + expect(['error', 'warning'].indexOf(iD.utilMapCSSRule(s, areaKeys).type)).to.be.greaterThan(-1); }); }); }); @@ -75,6 +79,28 @@ describe('iD.utilMapCSSRule', function() { }); }); }); + describe('#buildTagMap', function() { + it('builds tag map from selector config', function () { + var selector = { + 'geometry':'node', + 'equals':{'amenity':'marketplace'}, + 'positiveRegex': { 'marketplace:type': ['open', 'indoor', 'mall']}, + 'greaterThan': { 'width': 10, 'area': 300 }, + 'presence': 'opening_hours', + 'absence':'name', + 'warning':'throwWarning: "[amenity=marketplace]: MapRules preset \'Market\': must be coupled with name";' + }; + var tagMap = { + 'amenity':['marketplace'], + 'marketplace:type':['open','indoor','mall'], + 'width':[], + 'opening_hours':[] + }; + + var rule = iD.utilMapCSSRule(selector, areaKeys); + expect(rule.buildTagMap()).to.be.eql(tagMap); + }); + }); describe('#matches', function() { it('determines if an entity matches the MapCSS rule checks', function() { var node = iD.Entity({ type: 'node', tags: { power: 'tower' }}); @@ -84,17 +110,23 @@ describe('iD.utilMapCSSRule', function() { }); }); }); + describe('areaKeys', function() { + it('returns areaKeys used to construct rule', function() { + var rule = iD.utilMapCSSRule(selectors[0], areaKeys); + expect(rule.areaKeys()).to.eql(areaKeys); + }); + }); describe('#ruleChecks', function() { describe('equals', function() { it('is true when entity.tags intersects selector.equals', function() { var pseudoSelector = { equals: {'amenity': 'school'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var school = iD.Entity({ type: 'node', tags: { amenity: 'school' }}); expect(pseudoRule.ruleChecks.equals(school.tags)).to.be.true; }); it('is false when entity.tags intersects selector.equals', function() { var pseudoSelector = { equals: { 'man_made': 'water_tap'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); expect(pseudoRule.ruleChecks.equals(school.tags)).to.be.false; }); @@ -102,13 +134,13 @@ describe('iD.utilMapCSSRule', function() { describe('notEquals', function() { it('is true when entity.tags does not intersect selector.notEquals', function() { var pseudoSelector = { notEquals: { 'man_made': 'water_tap'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); expect(pseudoRule.ruleChecks.notEquals(school.tags)).to.be.true; }); it('is false when entity.tags does not intersect selector.notEquals', function() { var pseudoSelector = { notEquals: { 'amenity': 'school'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); expect(pseudoRule.ruleChecks.notEquals(school.tags)).to.be.false; }); @@ -116,13 +148,13 @@ describe('iD.utilMapCSSRule', function() { describe('presence', function() { it('is true when entity.tags\' key s include selector.presence', function() { var pseudoSelector = { presence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var kHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace', name: 'Kensington Square' }}); expect(pseudoRule.ruleChecks.presence(kHouse.tags)).to.be.true; }); it('is false when entity tags\' keys do not include selector.presence', function() { var pseudoSelector = { presence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var notKHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}); expect(pseudoRule.ruleChecks.presence(notKHouse.tags)).to.be.false; }); @@ -130,13 +162,13 @@ describe('iD.utilMapCSSRule', function() { describe('absence', function() { it('is true when entity.tags\' keys do not include selector.absence', function() { var pseudoSelector = { absence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var notKHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}); expect(pseudoRule.ruleChecks.absence(notKHouse.tags)).to.be.true; }); it('is false when entity.tags\' keys include selector.absence', function() { var pseudoSelector = { absence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var kHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace', name: 'Kensington Square' }}); expect(pseudoRule.ruleChecks.presence(kHouse.tags)).to.be.false; }); @@ -144,13 +176,13 @@ describe('iD.utilMapCSSRule', function() { describe('greaterThan', function() { it('is true when entity.tags\' equivalent value is greater than selector.greaterThan', function() { var pseudoSelector = { greaterThan: { height: 10 }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var tallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9000 }}); expect(pseudoRule.ruleChecks.greaterThan(tallSchool.tags)).to.be.true; }); it('is false when entity.tags\' equivalent value is less than or equal to selector.greaterThan', function() { var pseudoSelector = { greaterThan: { height: 10 }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9 }}); expect(pseudoRule.ruleChecks.greaterThan(smallSchool.tags)).to.be.false; }); @@ -158,13 +190,13 @@ describe('iD.utilMapCSSRule', function() { describe('greaterThanEqual', function() { it('is true when entity.tags\' equivalent value is greater than or equal to selector.greaterThanEqual', function() { var pseudoSelector = { greaterThanEqual: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var okHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 10 }}); expect(pseudoRule.ruleChecks.greaterThanEqual(okHeightSchool.tags)).to.be.true; }); it('is false when entity.tags\' equivalent value is less than to selector.greaterThanEqual', function() { var pseudoSelector = { greaterThanEqual: { height: 10 }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9 }}); expect(pseudoRule.ruleChecks.greaterThanEqual(smallSchool.tags)).to.be.false; }); @@ -172,13 +204,13 @@ describe('iD.utilMapCSSRule', function() { describe('lessThan', function() { it('is true when entity.tags\' equivalent value is less than to selector.lessThan', function() { var pseudoSelector = { lessThan: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 3 }}); expect(pseudoRule.ruleChecks.lessThan(smallSchool.tags)).to.be.tru; }); it('is false when entity.tags\' equivalent value is greater than or equal to selector.lessThan', function() { var pseudoSelector = { lessThan: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var notOkHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 10 }}); expect(pseudoRule.ruleChecks.lessThan(notOkHeightSchool.tags)).to.be.false; }); @@ -186,13 +218,13 @@ describe('iD.utilMapCSSRule', function() { describe('lessThanEqual', function() { it('is true when entity.tags\' equivalent value is less than or equal to to selector.lessThan', function() { var pseudoSelector = { lessThanEqual: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var okHeightSchool = iD.Entity({ type: 'node', tags: { 'amenity': 'school', 'height': 10 }}); expect(pseudoRule.ruleChecks.lessThanEqual(okHeightSchool.tags)).to.be.true; }); it('is false when entity.tags\' equivalent value is greater than to selector.lessThan', function() { var pseudoSelector = { lessThanEqual: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var notOkHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 11 }}); expect(pseudoRule.ruleChecks.lessThanEqual(notOkHeightSchool.tags)).to.be.false; }); @@ -200,7 +232,7 @@ describe('iD.utilMapCSSRule', function() { describe('positiveRegex', function() { it('is true when entity.tags\' equivalent value matches regular expression built from selector.positiveRegex', function() { var pseudoSelector = { positiveRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var okAmenities = [ iD.Entity({ type: 'node', tags: { amenity: 'school' }}), iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}) @@ -211,7 +243,7 @@ describe('iD.utilMapCSSRule', function() { }); it('is false when entity.tags\' equivalent value does not match regular expression built from selector.positiveRegex', function() { var pseudoSelector = { positiveRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var notOkAmenities = [ iD.Entity({ type: 'node', tags: { amenity: 'parking' }}), iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}) @@ -224,7 +256,7 @@ describe('iD.utilMapCSSRule', function() { describe('negativeRegex', function() { it('is true when entity.tags\' equivalent value does not match regular exprsesion built from selector.negativeRegex', function() { var pseudoSelector = { negativeRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var notOkAmenities = [ iD.Entity({ type: 'node', tags: { amenity: 'parking' }}), iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}) @@ -235,7 +267,7 @@ describe('iD.utilMapCSSRule', function() { }); it('is false when entity.tags\' equivalent value matches regular expression built from selector.negativeRegex', function() { var pseudoSelector = { negativeRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector); + var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); var okAmenities = [ iD.Entity({ type: 'node', tags: { amenity: 'school' }}), iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}) @@ -247,79 +279,54 @@ describe('iD.utilMapCSSRule', function() { }); }); describe('#inferGeometry', function() { - var amenityDerivedArea = { - selector: { + it ('infers selector geometry from its tags', function() { + var amenityDerivedArea = { 'geometry': 'closedway', 'presence': 'amenity', 'positiveRegex': { amenity: ['^school$', '^healthcare$'] }, 'error': 'amenity cannot be healthcare or school!' - }, - tagMap: { - amenity: [ 'school', 'healthcare' ] - } - }; - - var areaDerivedArea = { - selector: { + }; + + var areaDerivedArea = { 'geometry': 'closedway', 'equals': { area: 'yes' }, - }, - tagMap: { - amenity: [ 'school', 'healthcare' ], - area: [ 'yes' ] - } - }; + }; - var badAreaDerivedLine = { - selector: { + var badAreaDerivedLine = { 'geometry': 'closedway', 'equals': { 'area': 'no' } - }, - tagMap: { - area: ['no'] - } - }; + }; - var roundHouseRailwayDerivedArea = { - selector: { + var roundHouseRailwayDerivedArea = { 'geometry': 'closedway', 'equals': { 'railway': 'roundhouse' } - }, - tagMap: { - railway: ['roundhouse'] - } - }; + }; - var justClosedWayDerivedLine = { - selector: { + var justClosedWayDerivedLine = { 'geometry': 'closedway' - }, - tagMap: {} - }; + }; - var areaKeys = iD.Context().presets().areaKeys(); - var rule, geom; - - rule = iD.utilMapCSSRule(amenityDerivedArea.selector); - geom = rule.inferGeometry(amenityDerivedArea.tagMap, areaKeys); - expect(geom).to.be.eql('area'); + var rule, geom; + rule = iD.utilMapCSSRule(amenityDerivedArea, areaKeys); + geom = rule.inferGeometry(); + expect(geom).to.be.eql('area'); - rule = iD.utilMapCSSRule(areaDerivedArea.selector); - geom = rule.inferGeometry(areaDerivedArea.tagMap, areaKeys); - expect(geom).to.be.eql('area'); + rule = iD.utilMapCSSRule(areaDerivedArea, areaKeys); + geom = rule.inferGeometry(); + expect(geom).to.be.eql('area'); - rule = iD.utilMapCSSRule(badAreaDerivedLine.selector); - geom = rule.inferGeometry(badAreaDerivedLine.tagMap); - expect(geom).to.be.eql('line'); + rule = iD.utilMapCSSRule(badAreaDerivedLine, areaKeys); + geom = rule.inferGeometry(); + expect(geom).to.be.eql('line'); - rule = iD.utilMapCSSRule(roundHouseRailwayDerivedArea.selector); - geom = rule.inferGeometry(roundHouseRailwayDerivedArea.tagMap, areaKeys); - expect(geom).to.be.eql('area'); - - rule = iD.utilMapCSSRule(justClosedWayDerivedLine.selector); - geom = rule.inferGeometry(justClosedWayDerivedLine.tagMap); - expect(geom).to.be.eql('line'); + rule = iD.utilMapCSSRule(roundHouseRailwayDerivedArea, areaKeys); + geom = rule.inferGeometry(); + expect(geom).to.be.eql('area'); + rule = iD.utilMapCSSRule(justClosedWayDerivedLine, areaKeys); + geom = rule.inferGeometry(); + expect(geom).to.be.eql('line'); + }); }); describe('#findWarnings', function() { it('adds found warnings to warnings array', function() { @@ -332,12 +339,11 @@ describe('iD.utilMapCSSRule', function() { }); }); - // warnings.forEach(function(warning) { - // console.log(warning); - // expect(warning.message).to.not.be.null; - // expect(['mapcss_warning', 'mapcss_error'].indexOf(warning.id)).to.be.greaterThan(-1); - // expect(warning.entity).to.be.instanceOf(iD.Entity); - // }); + warnings.forEach(function(warning) { + expect(warning.message).to.not.be.null; + expect(['mapcss_warning', 'mapcss_error'].indexOf(warning.id)).to.be.greaterThan(-1); + expect(warning.entity).to.be.instanceOf(iD.Entity); + }); }); }); }); From 8f9a2818256a0da1303f847b63e1c36cd21fa059 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Tue, 14 Aug 2018 12:49:41 -0400 Subject: [PATCH 11/27] additional test tweaks ref #remote-presets --- modules/core/context.js | 13 +++++-------- modules/util/mapcss_rule.js | 7 +++++-- test/spec/util/mapcss_rule.js | 10 ++++++++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/modules/core/context.js b/modules/core/context.js index 25a72c2ae..57e7178b8 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -8,12 +8,7 @@ import _isString from 'lodash-es/isString'; import _map from 'lodash-es/map'; import { dispatch as d3_dispatch } from 'd3-dispatch'; - -import { - json as d3_json, - text as d3_text -} from 'd3-request'; - +import { json as d3_json } from 'd3-request'; import { select as d3_select } from 'd3-selection'; import { @@ -461,9 +456,11 @@ export function coreContext() { if (utilExternalValidationRules()) { var validationsUrl = utilStringQs(window.location.hash).validations; - d3_text(validationsUrl, function (err, mapcss) { + d3_json(validationsUrl, function (err, mapcssConfigs) { if (err) return; - var validations = _map(mapcss, function(mapcss) { return utilMapCSSRule(mapcss, context.presets().areaKeys()); }); + var validations = _map(mapcssConfigs, function(mapcssConfig) { + return utilMapCSSRule(mapcssConfig, context.presets().areaKeys()); + }); context.validationRules = function() { return validations; }; }); } diff --git a/modules/util/mapcss_rule.js b/modules/util/mapcss_rule.js index 7187bdb90..ab0b68c3b 100644 --- a/modules/util/mapcss_rule.js +++ b/modules/util/mapcss_rule.js @@ -59,8 +59,11 @@ export function utilMapCSSRule(selector, areaKeys) { .map(function(key) { return ruleChecks[key]; }); }, + selector: function() { + return selector; + }, buildTagMap: function() { - var selectorKeys = Object.keys(selector); + var selector = this.selector(), selectorKeys = Object.keys(selector); var tagMap = _reduce(selectorKeys, function (expectedTags, key) { var values; if (/regex/gi.test(key)) { @@ -146,7 +149,7 @@ export function utilMapCSSRule(selector, areaKeys) { if (entity.type === 'node' || entity.type === 'relation') { return selector.geometry === entity.type; } else if (entity.type === 'way') { - return this.inferGeometry(areaKeys) === entity.geometry(graph); + return this.inferGeometry() === entity.geometry(graph); } }, findWarnings: function (entity, graph, warnings) { diff --git a/test/spec/util/mapcss_rule.js b/test/spec/util/mapcss_rule.js index 0052dc05d..d6168b750 100644 --- a/test/spec/util/mapcss_rule.js +++ b/test/spec/util/mapcss_rule.js @@ -47,7 +47,7 @@ describe('iD.utilMapCSSRule', function() { var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s, areaKeys); }); it ('turns selector object in mapcssRule', function () { var ruleKeys = [ - 'ruleChecks', 'type','buildChecks', 'buildTagMap', 'matches', + 'ruleChecks', 'type','buildChecks', 'selector', 'buildTagMap', 'matches', 'areaKeys', 'inferGeometry', 'geometryMatches','findWarnings' ]; rules.forEach(function(rule) { @@ -110,7 +110,13 @@ describe('iD.utilMapCSSRule', function() { }); }); }); - describe('areaKeys', function() { + describe('#selector', function() { + it('returns selector used to construct rule', function() { + var rule = iD.utilMapCSSRule(selectors[1], areaKeys); + expect(rule.selector()).to.eql(selectors[1]); + }); + }); + describe('#areaKeys', function() { it('returns areaKeys used to construct rule', function() { var rule = iD.utilMapCSSRule(selectors[0], areaKeys); expect(rule.areaKeys()).to.eql(areaKeys); From 155983197f2d5472a069e6d33bd448a47eeff54b Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Sun, 23 Sep 2018 12:26:14 -0400 Subject: [PATCH 12/27] small variable change --- modules/core/context.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/core/context.js b/modules/core/context.js index 57e7178b8..e56a81183 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -456,8 +456,9 @@ export function coreContext() { if (utilExternalValidationRules()) { var validationsUrl = utilStringQs(window.location.hash).validations; - d3_json(validationsUrl, function (err, mapcssConfigs) { + d3_json(validationsUrl, function (err, mapcss) { if (err) return; + var mapcssConfigs = mapcss.rules; var validations = _map(mapcssConfigs, function(mapcssConfig) { return utilMapCSSRule(mapcssConfig, context.presets().areaKeys()); }); From 622b7b9e0cd0c353797daf404169f162346fb050 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Mon, 1 Oct 2018 21:45:21 -0400 Subject: [PATCH 13/27] move validations to its own class ref #remote-presets --- modules/core/context.js | 5 +- modules/services/maprules.js | 75 +++-- test/index.html | 4 +- test/spec/services/maprules.js | 574 +++++++++++++++++++++++++++++++++ test/spec/util/mapcss_rule.js | 356 -------------------- 5 files changed, 624 insertions(+), 390 deletions(-) create mode 100644 test/spec/services/maprules.js delete mode 100644 test/spec/util/mapcss_rule.js diff --git a/modules/core/context.js b/modules/core/context.js index d6fbd9cb8..2d4166b79 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -464,10 +464,9 @@ export function coreContext() { var validationsUrl = utilStringQs(window.location.hash).validations; d3_json(validationsUrl, function (err, mapcss) { if (err) return; - services.maprules.init(); - var areaKeys = context.presets().areaKeys(); + services.maprules.init(context.presets().areaKeys()); _each(mapcss, function(mapcssSelector) { - return services.maprules.addRule(mapcssSelector, areaKeys); + return services.maprules.addRule(mapcssSelector); }); context.validationRules = true; }); diff --git a/modules/services/maprules.js b/modules/services/maprules.js index fd49f6fa4..b55f39017 100644 --- a/modules/services/maprules.js +++ b/modules/services/maprules.js @@ -3,9 +3,6 @@ import _intersection from 'lodash-es/intersection'; import _reduce from 'lodash-es/reduce'; import _every from 'lodash-es/every'; -var ruleChecks, - validationRules; - var buildRuleChecks = function() { return { equals: function (equals) { @@ -81,17 +78,35 @@ var buildRuleChecks = function() { }; }; +var buildLineKeys = function() { + return { + highway: { + rest_area: true, + services: true + }, + railway: { + roundhouse: true, + station: true, + traverser: true, + turntable: true, + wash: true + } + }; +}; export default { - init: function() { - ruleChecks = buildRuleChecks(); - validationRules = []; + init: function(areaKeys) { + this._ruleChecks = buildRuleChecks(); + this._validationRules = []; + this._areaKeys = areaKeys; + this._lineKeys = buildLineKeys(); }, // list of rules only relevant to tag checks... filterRuleChecks: function(selector) { + var _ruleChecks = this._ruleChecks; return _reduce(Object.keys(selector), function(rules, key) { if (['geometry', 'error', 'warning'].indexOf(key) === -1) { - rules.push(ruleChecks[key](selector[key])); + rules.push(_ruleChecks[key](selector[key])); } return rules; }, []); @@ -123,8 +138,14 @@ export default { } else if (/(greater|less)Than(Equal)?|presence/g.test(key)) { var tagKey = /presence/.test(key) ? selector[key] : Object.keys(selector[key])[0]; - expectedTags[tagKey] = []; + + values = [selector[key][tagKey]]; + if (expectedTags.hasOwnProperty(tagKey)) { + values = values.concat(expectedTags[tagKey]); + } + + expectedTags[tagKey] = values; } return expectedTags; @@ -133,25 +154,15 @@ export default { return tagMap; }, // inspired by osmWay#isArea() - inferGeometry: function(tagMap, areaKeys) { - var lineKeys = { - highway: { - rest_area: true, - services: true - }, - railway: { - roundhouse: true, - station: true, - traverser: true, - turntable: true, - wash: true - } - }; + inferGeometry: function(tagMap) { + var _lineKeys = this._lineKeys; + var _areaKeys = this._areaKeys; + var isAreaKeyBlackList = function(key) { - return _intersection(tagMap[key], Object.keys(areaKeys[key])).length > 0; + return _intersection(tagMap[key], Object.keys(_areaKeys[key])).length > 0; }; var isLineKeysWhiteList = function(key) { - return _intersection(tagMap[key], Object.keys(lineKeys[key])).length > 0; + return _intersection(tagMap[key], Object.keys(_lineKeys[key])).length > 0; }; if (tagMap.hasOwnProperty('area')) { @@ -164,10 +175,10 @@ export default { } for (var key in tagMap) { - if (key in areaKeys && !isAreaKeyBlackList(key)) { + if (key in _areaKeys && !isAreaKeyBlackList(key)) { return 'area'; } - if (key in lineKeys && isLineKeysWhiteList(key)) { + if (key in _lineKeys && isLineKeysWhiteList(key)) { return 'area'; } } @@ -175,7 +186,8 @@ export default { return 'line'; }, // adds from mapcss-parse selector check... - addRule: function(selector, areaKeys) { + addRule: function(selector) { + var _areaKeys = this._areaKeys; var rule = { // checks relevant to mapcss-selector checks: this.filterRuleChecks(selector), @@ -186,7 +198,7 @@ export default { }); }, // borrowed from Way#isArea() - inferredGeometry: this.inferGeometry(this.buildTagMap(selector), areaKeys), + inferredGeometry: this.inferGeometry(this.buildTagMap(selector), this._areaKeys), geometryMatches: function(entity, graph) { if (entity.type === 'node' || entity.type === 'relation') { return selector.geometry === entity.type; @@ -206,8 +218,11 @@ export default { } } }; - validationRules.push(rule); + this._validationRules.push(rule); }, + clearRules: function() { this._validationRules = []; }, // returns validationRules... - validationRules: function() { return validationRules; } + validationRules: function() { return this._validationRules; }, + // returns ruleChecks + ruleChecks: function() { return this._ruleChecks; } }; diff --git a/test/index.html b/test/index.html index d8d5ef4d6..caf87bc2e 100644 --- a/test/index.html +++ b/test/index.html @@ -112,7 +112,9 @@ - + --> + + diff --git a/test/spec/services/maprules.js b/test/spec/services/maprules.js new file mode 100644 index 000000000..34b741b90 --- /dev/null +++ b/test/spec/services/maprules.js @@ -0,0 +1,574 @@ +describe('maprules', function() { + var _ruleChecks, validationRules; + before(function() { + var areaKeys = iD.Context().presets().areaKeys(); + iD.serviceMapRules.init(areaKeys); + _ruleChecks = iD.serviceMapRules.ruleChecks(); + }); + + describe('#filterRuleChecks', function() { + it('returns shortlist of mapcss checks relevant to provided selector', function() { + var selector = { + geometry: 'closedway', + equals: {amenity: 'marketplace'}, + absence: 'name', + error: '\'Marketplace\' preset must be coupled with name' + }; + var filteredChecks = iD.serviceMapRules.filterRuleChecks(selector); + var equalsCheck = filteredChecks[0]; + var absenceCheck = filteredChecks[1]; + var entityTags = {amenity: 'marketplace'}; + + expect(filteredChecks.length).eql(2); + expect(equalsCheck(entityTags)).to.be.true; + expect(absenceCheck(entityTags)).to.be.true; + }); + }); + + describe('#buildTagMap', function() { + it('builds a map of tag keys/values found in mapcss selector', function() { + [ + { + t: { + equals: { + man_made: 'tower', + 'tower:type': 'communication' + } + }, + r: { + man_made: ['tower'], + 'tower:type': ['communication'] + } + }, + { + t: { + equals: { + building: 'yes', + amenity: 'school' + }, + positiveRegex: { + opening_hours: [ + '24/7', + 'sunrise_sundown' + ] + }, + negativeRegex: { + source: [ + 'missing_maps', + 'american_red_cross' + ] + }, + greaterThanEqual: { floors: 2 }, + lessThanEqual: { floors: 4 } + + }, + r: { + building: ['yes'], + amenity: ['school'], + opening_hours: ['24/7', 'sunrise_sundown'], + source: ['missing_maps', 'american_red_cross'], + floors: [4, 2] + } + }, + { + t: { + equals: { highway: 'yes' }, + greaterThan: { lanes: 1 }, + lessThan: { lanes: 4 } + }, + r: { + highway: ['yes'], + lanes: [4, 1] + } + } + ].forEach(function(test) { + expect(iD.serviceMapRules.buildTagMap(test.t)).to.eql(test.r); + }); + }); + }); + + describe('#inferGeometry', function() { + it('infers geometry using selector keys', function() { + + var amenityDerivedArea = { + geometry: 'closedway', + presence: 'amenity', + positiveRegex: { amenity: ['^school$', '^healthcare$'] }, + error: 'amenity cannot be healthcare or school!' + }; + + var areaDerivedArea = { + geometry: 'closedway', + equals: { area: 'yes' }, + }; + + var badAreaDerivedLine = { + geometry: 'closedway', + equals: { 'area': 'no' } + }; + + var roundHouseRailwayDerivedArea = { + geometry: 'closedway', + equals: { 'railway': 'roundhouse' } + }; + + var justClosedWayDerivedLine = { + geometry: 'closedway' + }; + + var tagMap, geom; + tagMap = iD.serviceMapRules.buildTagMap(amenityDerivedArea); + geom = iD.serviceMapRules.inferGeometry(tagMap); + expect(geom).to.be.eql('area'); + + tagMap = iD.serviceMapRules.buildTagMap(areaDerivedArea); + geom = iD.serviceMapRules.inferGeometry(tagMap); + expect(geom).to.be.eql('area'); + + tagMap = iD.serviceMapRules.buildTagMap(badAreaDerivedLine); + geom = iD.serviceMapRules.inferGeometry(tagMap); + expect(geom).to.be.eql('line'); + + tagMap = iD.serviceMapRules.buildTagMap(roundHouseRailwayDerivedArea); + geom = iD.serviceMapRules.inferGeometry(tagMap); + expect(geom).to.be.eql('area'); + + tagMap = iD.serviceMapRules.buildTagMap(justClosedWayDerivedLine); + geom = iD.serviceMapRules.inferGeometry(tagMap); + expect(geom).to.be.eql('line'); + }); + }); + + describe('#addRule', function() { + it ('builds a rule from provided selector and adds it to _validationRules', function () { + var selector = { + geometry:'node', + equals: {amenity:'marketplace'}, + absence:'name', + warning:'\'Marketplace\' preset must be coupled with name' + }; + expect(iD.serviceMapRules.validationRules()).to.be.empty; + iD.serviceMapRules.addRule(selector); + expect(iD.serviceMapRules.validationRules().length).to.eql(1); + }); + }); + describe('#clearRules', function() { + it ('clears _validationRules array', function() { + expect(iD.serviceMapRules.validationRules().length).to.eql(1); + iD.serviceMapRules.clearRules(); + expect(iD.serviceMapRules.validationRules()).to.be.empty; + }); + }); + + describe('#validationRules', function() { + it('returns _validationRules array', function() { + var selector = { + geometry: 'closedway', + equals: {amenity: 'marketplace'}, + absence: 'name', + error: '\'Marketplace\' preset must be coupled with name' + }; + iD.serviceMapRules.addRule(selector); + var rules = iD.serviceMapRules.validationRules(); + expect(rules).instanceof(Array); + expect(rules.length).to.eql(1); + }); + }); + + describe('_ruleChecks', function () { + describe('#equals', function() { + it('is true when two tag maps intersect', function() { + var a = { amenity: 'school'}; + var b = { amenity: 'school' }; + expect(_ruleChecks.equals(a)(b)).to.be.true; + }); + it('is false when two tag maps intersect', function() { + var a = { man_made: 'water_tap'}; + var b = { amenity: 'school'}; + expect(_ruleChecks.equals(a)(b)).to.be.false; + }); + }); + describe('#notEquals', function() { + it('is true when two tag maps do not intersect', function() { + var a = { man_made: 'water_tap'}; + var b = { amenity: 'school' }; + expect(_ruleChecks.notEquals(a)(b)).to.be.true; + }); + it('is not true when two tag maps intersect', function() { + var a = { amenity: 'school' }; + var b = { amenity: 'school', opening_hours: '9-5' }; + expect(_ruleChecks.notEquals(a)(b)).to.be.false; + }); + }); + describe('absence', function() { + it('is true when tag map keys does not include key in question', function() { + var key = 'amenity'; + var map = { building: 'yes' }; + expect(_ruleChecks.absence(key)(map)).to.be.true; + }); + it('is false when tag map keys does include key in question', function() { + var key = 'amenity'; + var map = { amenity: 'school' }; + expect(_ruleChecks.absence(key)(map)).to.be.false; + }); + }); + describe('presence', function() { + it('is true when tag map keys includes key in question', function() { + var key = 'amenity'; + var map = { amenity: 'school'}; + expect(_ruleChecks.presence(key)(map)).to.be.true; + }); + it('is false when tag map keys do not include key in question', function() { + var key = 'amenity'; + var map = { building: 'yes'}; + expect(_ruleChecks.presence(key)(map)).to.be.false; + }); + }); + describe('greaterThan', function() { + it ('is true when a tag value is greater than the selector value', function() { + var selectorTags = { lanes: 5 }; + var tags = { lanes : 6 }; + expect(_ruleChecks.greaterThan(selectorTags)(tags)).to.be.true; + }); + it ('is false when a tag value is less than or equal to the selector value', function() { + var selectorTags = { lanes: 5 }; + [4, 5].forEach(function(val) { + expect(_ruleChecks.greaterThan(selectorTags)({ lanes: val })).to.be.false; + }); + }); + }); + describe('greaterThanEqual', function() { + it ('is true when a tag value is greater than or equal to the selector value', function() { + var selectorTags = { lanes: 5 }; + [5, 6].forEach(function(val) { + expect(_ruleChecks.greaterThanEqual(selectorTags)({ lanes: val })).to.be.true; + }); + }); + it ('is false when a tag value is less than the selector value', function () { + var selectorTags = { lanes: 5 }; + var tags = { lanes: 4 }; + expect(_ruleChecks.greaterThanEqual(selectorTags)(tags)).to.be.false; + }); + }); + describe('lessThan', function() { + it ('is true when a tag value is less than the selector value', function() { + var selectorTags = { lanes: 5 }; + var tags = { lanes: 4 }; + expect(_ruleChecks.lessThan(selectorTags)(tags)).to.be.true; + }); + it ('is false when a tag value is greater than or equal to the selector value', function() { + var selectorTags = { lanes: 5 }; + [6, 7].forEach(function(val) { + expect(_ruleChecks.lessThan(selectorTags)({ lanes: val })).to.be.false; + }); + }); + }); + describe('lessThanEqual', function() { + it ('is true when a tag value is less than or equal to the selector value', function() { + var selectorTags = { lanes: 5 }; + [4, 5].forEach(function(val) { + expect(_ruleChecks.lessThanEqual(selectorTags)({ lanes: val })).to.be.true; + }); + }); + it ('is false when a tag value is greater than the selector value', function() { + var selectorTags = { lanes: 5 }; + var tags = { lanes: 6 }; + expect(_ruleChecks.lessThanEqual(selectorTags)(tags)).to.be.false; + }); + }); + describe('positiveRegex', function() { + var positiveRegex = { amenity: ['^hospital$','^clinic$']}; + it ('is true when tag value matches positiveRegex', function() { + var tags = { amenity: 'hospital' }; + expect(_ruleChecks.positiveRegex(positiveRegex)(tags)).to.be.true; + }); + it ('is false when tag value does not match negative regex', function() { + var tags = { amenity: 'school' }; + expect(_ruleChecks.positiveRegex(positiveRegex)(tags)).to.be.false; + }); + }); + describe('negativeRegex', function() { + var negativeRegex = { bicycle: [ 'use_path', 'designated' ] }; + it ('is true when tag value does not match negativeRegex', function() { + var tags = { bicycle: 'yes' }; + expect(_ruleChecks.negativeRegex(negativeRegex)(tags)).to.be.true; + }); + it ('is false when tag value matches negativeRegex', function() { + var tags = { bicycle: 'designated' }; + expect(_ruleChecks.negativeRegex(negativeRegex)(tags)).to.be.false; + }); + }); + }); + describe('rule', function() { + var selectors, entities; + before(function() { + selectors = [ + { + geometry:'node', + equals: {amenity:'marketplace'}, + absence:'name', + error:'\'Marketplace\' preset must be coupled with name' + }, + { + geometry: 'closedway', + notEquals: { building: 'yes', amenity: 'clinic' }, + error: '\'Clinic\' preset must be coupled with building=yes' + }, + { + geometry:'node', + equals: {man_made: 'tower', 'tower:type': 'communication'}, + presence: 'height', + error:'\'Communication Tower\' preset must not be coupled with height' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + lessThanEqual: { height: 6 }, + error: '\'Tower\' preset height must be greater than 6' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + greaterThanEqual: { height: 9 }, + error: '\'Tower\' preset height must be less than 9' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + lessThan: { height: 6 }, + error: '\'Tower\' preset height must be greater than or equal to 6' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + greaterThan: { height: 9 }, + error: '\'Tower\' preset height must be greater less than or equal to 9' + }, + { + geometry: 'closedway', + equals: { amenity: 'clinic' }, + negativeRegex: { emergency: ['yes', 'no'] }, + error: '\'Clinic\' preset\'s emergency tag must be equal to \'yes\' or \'no\'' + }, + { + geometry: 'way', + equals: { highway: 'residential' }, + positiveRegex: { structure: ['bridge', 'tunnel'] }, + error: '\'suburban road\' structure tag cannot be \'bridge\' or \'tunnel\'' + } + ]; + entities = [ + iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}), + iD.Way({ tags: { building: 'house', amenity: 'clinic' }, nodes: [ 'a', 'b', 'c', 'a' ]}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', 'tower:type': 'communication', height: 5 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 6 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 9 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 5 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 10 }}), + iD.Way({ tags: { amenity: 'clinic', emergency: 'definitely' }, nodes: [ 'd', 'e', 'f', 'd' ]}), + iD.Way({ tags: { highway: 'residential', structure: 'bridge' }}), + ]; + + iD.serviceMapRules.clearRules(); + selectors.forEach(function(selector) { iD.serviceMapRules.addRule(selector); }); + validationRules = iD.serviceMapRules.validationRules(); + }); + describe('#matches', function() { + var selectors, entities; + before(function() { + selectors = [ + { + geometry:'node', + equals: {amenity:'marketplace'}, + absence:'name', + error:'\'Marketplace\' preset must be coupled with name' + }, + { + geometry: 'closedway', + notEquals: { building: 'yes', amenity: 'clinic' }, + error: '\'Clinic\' preset must be coupled with building=yes' + }, + { + geometry:'node', + equals: {man_made: 'tower', 'tower:type': 'communication'}, + presence: 'height', + error:'\'Communication Tower\' preset must not be coupled with height' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + lessThanEqual: { height: 6 }, + error: '\'Tower\' preset height must be greater than 6' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + greaterThanEqual: { height: 9 }, + error: '\'Tower\' preset height must be less than 9' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + lessThan: { height: 6 }, + error: '\'Tower\' preset height must be greater than or equal to 6' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + greaterThan: { height: 9 }, + error: '\'Tower\' preset height must be greater less than or equal to 9' + }, + { + geometry: 'closedway', + equals: { amenity: 'clinic' }, + negativeRegex: { emergency: ['yes', 'no'] }, + error: '\'Clinic\' preset\'s emergency tag must be equal to \'yes\' or \'no\'' + }, + { + geometry: 'way', + equals: { highway: 'residential' }, + positiveRegex: { structure: ['bridge', 'tunnel'] }, + error: '\'suburban road\' structure tag cannot be \'bridge\' or \'tunnel\'' + } + ]; + entities = [ + iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}), + iD.Way({ tags: { building: 'house', amenity: 'clinic' }, nodes: [ 'a', 'b', 'c', 'a' ]}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', 'tower:type': 'communication', height: 5 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 6 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 9 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 5 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 10 }}), + iD.Way({ tags: { amenity: 'clinic', emergency: 'definitely' }, nodes: [ 'd', 'e', 'f', 'd' ]}), + iD.Way({ tags: { highway: 'residential', structure: 'bridge' }}), + ]; + + iD.serviceMapRules.clearRules(); + selectors.forEach(function(selector) { iD.serviceMapRules.addRule(selector); }); + validationRules = iD.serviceMapRules.validationRules(); + }); + it('is true when each rule check is \'true\'', function() { + validationRules.forEach(function(rule, i) { + expect(rule.matches(entities[i])).to.be.true; + }); + }); + it ('is true when at least one rule check is \'false\'', function() { + var selector = { + geometry: 'way', + equals: { highway: 'residential' }, + positiveRegex: { structure: ['embarkment', 'bridge'] }, + error: '\'suburban road\' structure tag cannot be \'bridge\' or \'tunnel\'' + }; + var entity = iD.Way({ tags: { highway: 'residential', structure: 'tunnel' }}); + iD.serviceMapRules.clearRules(); + iD.serviceMapRules.addRule(selector); + var rule = iD.serviceMapRules.validationRules()[0]; + + expect(rule.matches(entity)).to.be.false; + }); + }); + describe('#findWarnings', function() { + var selectors, entities, _graph; + + before(function() { + selectors = [ + { + geometry:'node', + equals: {amenity:'marketplace'}, + absence:'name', + error:'\'Marketplace\' preset must be coupled with name' + }, + { + geometry: 'closedway', + notEquals: { building: 'yes', amenity: 'clinic' }, + error: '\'Clinic\' preset must be coupled with building=yes' + }, + { + geometry:'node', + equals: {man_made: 'tower', 'tower:type': 'communication'}, + presence: 'height', + error:'\'Communication Tower\' preset must not be coupled with height' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + lessThanEqual: { height: 6 }, + error: '\'Tower\' preset height must be greater than 6' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + greaterThanEqual: { height: 9 }, + error: '\'Tower\' preset height must be less than 9' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + lessThan: { height: 6 }, + error: '\'Tower\' preset height must be greater than or equal to 6' + }, + { + geometry: 'node', + equals: { man_made: 'tower' }, + greaterThan: { height: 9 }, + error: '\'Tower\' preset height must be greater less than or equal to 9' + }, + { + geometry: 'closedway', + equals: { amenity: 'clinic' }, + negativeRegex: { emergency: ['yes', 'no'] }, + error: '\'Clinic\' preset\'s emergency tag must be equal to \'yes\' or \'no\'' + }, + { + geometry: 'way', + equals: { highway: 'residential' }, + positiveRegex: { structure: ['bridge', 'tunnel'] }, + error: '\'suburban road\' structure tag cannot be \'bridge\' or \'tunnel\'' + } + ]; + entities = [ + iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}), + iD.Way({ tags: { building: 'house', amenity: 'clinic' }, nodes: [ 'a', 'b', 'c', 'a' ]}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', 'tower:type': 'communication', height: 5 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 6 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 9 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 5 }}), + iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 10 }}), + iD.Way({ tags: { amenity: 'clinic', emergency: 'definitely' }, nodes: [ 'd', 'e', 'f', 'd' ]}), + iD.Way({ tags: { highway: 'residential', structure: 'bridge' }}), + ]; + + var wayNodes = [ + iD.osmNode({ id: 'a' }), + iD.osmNode({ id: 'b' }), + iD.osmNode({ id: 'c' }), + iD.osmNode({ id: 'd' }), + iD.osmNode({ id: 'e' }), + iD.osmNode({ id: 'f' }), + ]; + _graph = iD.Graph(entities.concat(wayNodes)); + iD.serviceMapRules.clearRules(); + selectors.forEach(function(selector) { iD.serviceMapRules.addRule(selector); }); + validationRules = iD.serviceMapRules.validationRules(); + }); + it('finds warnings', function() { + validationRules.forEach(function(rule, i) { + var warnings = []; + var entity = entities[i]; + var selector = selectors[i]; + + rule.findWarnings(entity, _graph, warnings); + + var warning = warnings[0]; + var type = Object.keys(selector).indexOf('error') ? 'error' : 'warning'; + + expect(warnings.length).to.eql(1); + expect(warning.entity).to.eql(entity); + expect(warning.message).to.eql(selector[type]); + expect('mapcss_' + type).to.eql(warning.id); + }); + }); + }); + }); +}); + diff --git a/test/spec/util/mapcss_rule.js b/test/spec/util/mapcss_rule.js deleted file mode 100644 index d6168b750..000000000 --- a/test/spec/util/mapcss_rule.js +++ /dev/null @@ -1,356 +0,0 @@ -describe('iD.utilMapCSSRule', function() { - var entities = [ - iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}), - iD.Entity({ type: 'node', tags: { man_made: 'water_tap' }}), - iD.Entity({ type: 'node', tags: { amenity: 'marketplace', height: 0 }}), - iD.Entity({ type: 'node', tags: { amenity: 'school', height: 5, width: 3 }}), - iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}), - iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}), - ]; - var selectors = [ - { - 'geometry':'node', - 'equals':{'amenity':'marketplace'}, - 'absence':'name', - 'warning':'throwWarning: "[amenity=marketplace]: MapRules preset \'Market\': must be coupled with name";' - }, - { - 'geometry':'node', - 'equals':{'man_made':'water_tap'}, - 'absence':'name', - 'warning':'throwWarning: "[amenity=drinking_water][man_made=water_tap]: MapRules preset \'Water Tap\': must be coupled with name";' - }, - { - 'geometry':'node', - 'equals':{'amenity':'marketplace'}, - 'presence':'height', - 'lessThanEqual': { 'height': 0 }, - 'warning':'throwWarning: "[amenity=marketplace]: height must be greater than 0";' - }, - { - 'geometry': 'node', - 'equals': {'amenity': 'school'}, - 'greaterThan': { 'height': 0 }, - 'greaterThanEqual': { 'width': 1 }, - 'lessThanEqual': { 'width': 10 }, - 'lessThan': { 'height': 10 }, - 'warning': 'this is the warning!' - }, - { - 'geometry': 'node', - 'presence': 'amenity', - 'positiveRegex': { amenity: ['^school$', '^healthcare$'] }, - 'error': 'amenity cannot be healthcare or school!' - } - ]; - var areaKeys = iD.Context().presets().areaKeys(); - var rules = selectors.map(function(s) { return iD.utilMapCSSRule(s, areaKeys); }); - it ('turns selector object in mapcssRule', function () { - var ruleKeys = [ - 'ruleChecks', 'type','buildChecks', 'selector', 'buildTagMap', 'matches', - 'areaKeys', 'inferGeometry', 'geometryMatches','findWarnings' - ]; - rules.forEach(function(rule) { - expect(Object.keys(rule)).to.eql(ruleKeys); - }); - }); - describe('#type', function() { - it('is either error or warning', function() { - selectors.forEach(function(s) { - expect(['error', 'warning'].indexOf(iD.utilMapCSSRule(s, areaKeys).type)).to.be.greaterThan(-1); - }); - }); - }); - describe('#geometryMatches', function() { - it('determines if entity and rule geometries match', function() { - var node = iD.Entity({ type: 'node'}); - var way = iD.Entity({ type: 'way'}); - var graph = iD.Graph([node, way]); - rules.forEach(function(rule) { - expect(rule.geometryMatches(node, graph)).to.be.true; - expect(rule.geometryMatches(way, graph)).to.be.false; - }); - }); - }); - describe('#buildsChecks', function() { - it('builds array of MapCSS rule check functions to run entities against', function() { - rules.forEach(function(rule) { - expect(rule.buildChecks().every(function(fn) { return fn instanceof Function; })).to.be.true; - }); - }); - }); - describe('#buildTagMap', function() { - it('builds tag map from selector config', function () { - var selector = { - 'geometry':'node', - 'equals':{'amenity':'marketplace'}, - 'positiveRegex': { 'marketplace:type': ['open', 'indoor', 'mall']}, - 'greaterThan': { 'width': 10, 'area': 300 }, - 'presence': 'opening_hours', - 'absence':'name', - 'warning':'throwWarning: "[amenity=marketplace]: MapRules preset \'Market\': must be coupled with name";' - }; - var tagMap = { - 'amenity':['marketplace'], - 'marketplace:type':['open','indoor','mall'], - 'width':[], - 'opening_hours':[] - }; - - var rule = iD.utilMapCSSRule(selector, areaKeys); - expect(rule.buildTagMap()).to.be.eql(tagMap); - }); - }); - describe('#matches', function() { - it('determines if an entity matches the MapCSS rule checks', function() { - var node = iD.Entity({ type: 'node', tags: { power: 'tower' }}); - rules.forEach(function(rule, i) { - expect(rule.matches(entities[i])).to.be.true; - expect(rule.matches(node)).to.be.false; - }); - }); - }); - describe('#selector', function() { - it('returns selector used to construct rule', function() { - var rule = iD.utilMapCSSRule(selectors[1], areaKeys); - expect(rule.selector()).to.eql(selectors[1]); - }); - }); - describe('#areaKeys', function() { - it('returns areaKeys used to construct rule', function() { - var rule = iD.utilMapCSSRule(selectors[0], areaKeys); - expect(rule.areaKeys()).to.eql(areaKeys); - }); - }); - describe('#ruleChecks', function() { - describe('equals', function() { - it('is true when entity.tags intersects selector.equals', function() { - var pseudoSelector = { equals: {'amenity': 'school'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var school = iD.Entity({ type: 'node', tags: { amenity: 'school' }}); - expect(pseudoRule.ruleChecks.equals(school.tags)).to.be.true; - }); - it('is false when entity.tags intersects selector.equals', function() { - var pseudoSelector = { equals: { 'man_made': 'water_tap'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); - expect(pseudoRule.ruleChecks.equals(school.tags)).to.be.false; - }); - }); - describe('notEquals', function() { - it('is true when entity.tags does not intersect selector.notEquals', function() { - var pseudoSelector = { notEquals: { 'man_made': 'water_tap'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); - expect(pseudoRule.ruleChecks.notEquals(school.tags)).to.be.true; - }); - it('is false when entity.tags does not intersect selector.notEquals', function() { - var pseudoSelector = { notEquals: { 'amenity': 'school'} }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var school = iD.Entity({ type: 'node', tags: { amenity: 'school' } } ); - expect(pseudoRule.ruleChecks.notEquals(school.tags)).to.be.false; - }); - }); - describe('presence', function() { - it('is true when entity.tags\' key s include selector.presence', function() { - var pseudoSelector = { presence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var kHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace', name: 'Kensington Square' }}); - expect(pseudoRule.ruleChecks.presence(kHouse.tags)).to.be.true; - }); - it('is false when entity tags\' keys do not include selector.presence', function() { - var pseudoSelector = { presence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var notKHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}); - expect(pseudoRule.ruleChecks.presence(notKHouse.tags)).to.be.false; - }); - }); - describe('absence', function() { - it('is true when entity.tags\' keys do not include selector.absence', function() { - var pseudoSelector = { absence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var notKHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}); - expect(pseudoRule.ruleChecks.absence(notKHouse.tags)).to.be.true; - }); - it('is false when entity.tags\' keys include selector.absence', function() { - var pseudoSelector = { absence: 'name' }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var kHouse = iD.Entity({ type: 'node', tags: { amenity: 'marketplace', name: 'Kensington Square' }}); - expect(pseudoRule.ruleChecks.presence(kHouse.tags)).to.be.false; - }); - }); - describe('greaterThan', function() { - it('is true when entity.tags\' equivalent value is greater than selector.greaterThan', function() { - var pseudoSelector = { greaterThan: { height: 10 }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var tallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9000 }}); - expect(pseudoRule.ruleChecks.greaterThan(tallSchool.tags)).to.be.true; - }); - it('is false when entity.tags\' equivalent value is less than or equal to selector.greaterThan', function() { - var pseudoSelector = { greaterThan: { height: 10 }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9 }}); - expect(pseudoRule.ruleChecks.greaterThan(smallSchool.tags)).to.be.false; - }); - }); - describe('greaterThanEqual', function() { - it('is true when entity.tags\' equivalent value is greater than or equal to selector.greaterThanEqual', function() { - var pseudoSelector = { greaterThanEqual: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var okHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 10 }}); - expect(pseudoRule.ruleChecks.greaterThanEqual(okHeightSchool.tags)).to.be.true; - }); - it('is false when entity.tags\' equivalent value is less than to selector.greaterThanEqual', function() { - var pseudoSelector = { greaterThanEqual: { height: 10 }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 9 }}); - expect(pseudoRule.ruleChecks.greaterThanEqual(smallSchool.tags)).to.be.false; - }); - }); - describe('lessThan', function() { - it('is true when entity.tags\' equivalent value is less than to selector.lessThan', function() { - var pseudoSelector = { lessThan: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var smallSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 3 }}); - expect(pseudoRule.ruleChecks.lessThan(smallSchool.tags)).to.be.tru; - }); - it('is false when entity.tags\' equivalent value is greater than or equal to selector.lessThan', function() { - var pseudoSelector = { lessThan: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var notOkHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 10 }}); - expect(pseudoRule.ruleChecks.lessThan(notOkHeightSchool.tags)).to.be.false; - }); - }); - describe('lessThanEqual', function() { - it('is true when entity.tags\' equivalent value is less than or equal to to selector.lessThan', function() { - var pseudoSelector = { lessThanEqual: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var okHeightSchool = iD.Entity({ type: 'node', tags: { 'amenity': 'school', 'height': 10 }}); - expect(pseudoRule.ruleChecks.lessThanEqual(okHeightSchool.tags)).to.be.true; - }); - it('is false when entity.tags\' equivalent value is greater than to selector.lessThan', function() { - var pseudoSelector = { lessThanEqual: { height: 10 } }; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var notOkHeightSchool = iD.Entity({ type: 'node', tags: { amenity: 'school', height: 11 }}); - expect(pseudoRule.ruleChecks.lessThanEqual(notOkHeightSchool.tags)).to.be.false; - }); - }); - describe('positiveRegex', function() { - it('is true when entity.tags\' equivalent value matches regular expression built from selector.positiveRegex', function() { - var pseudoSelector = { positiveRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var okAmenities = [ - iD.Entity({ type: 'node', tags: { amenity: 'school' }}), - iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}) - ]; - okAmenities.forEach(function(amenity) { - expect(pseudoRule.ruleChecks.positiveRegex(amenity.tags)).to.be.true; - }); - }); - it('is false when entity.tags\' equivalent value does not match regular expression built from selector.positiveRegex', function() { - var pseudoSelector = { positiveRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var notOkAmenities = [ - iD.Entity({ type: 'node', tags: { amenity: 'parking' }}), - iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}) - ]; - notOkAmenities.forEach(function(amenity) { - expect(pseudoRule.ruleChecks.positiveRegex(amenity.tags)).to.be.false; - }); - }); - }); - describe('negativeRegex', function() { - it('is true when entity.tags\' equivalent value does not match regular exprsesion built from selector.negativeRegex', function() { - var pseudoSelector = { negativeRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var notOkAmenities = [ - iD.Entity({ type: 'node', tags: { amenity: 'parking' }}), - iD.Entity({ type: 'node', tags: { amenity: 'place_of_worship' }}) - ]; - notOkAmenities.forEach(function(amenity) { - expect(pseudoRule.ruleChecks.negativeRegex(amenity.tags)).to.be.true; - }); - }); - it('is false when entity.tags\' equivalent value matches regular expression built from selector.negativeRegex', function() { - var pseudoSelector = { negativeRegex: { amenity: ['^school$', '^healthcare$'] }}; - var pseudoRule = iD.utilMapCSSRule(pseudoSelector, areaKeys); - var okAmenities = [ - iD.Entity({ type: 'node', tags: { amenity: 'school' }}), - iD.Entity({ type: 'node', tags: { amenity: 'healthcare' }}) - ]; - okAmenities.forEach(function(amenity) { - expect(pseudoRule.ruleChecks.negativeRegex(amenity.tags)).to.be.false; - }); - }); - }); - }); - describe('#inferGeometry', function() { - it ('infers selector geometry from its tags', function() { - var amenityDerivedArea = { - 'geometry': 'closedway', - 'presence': 'amenity', - 'positiveRegex': { amenity: ['^school$', '^healthcare$'] }, - 'error': 'amenity cannot be healthcare or school!' - }; - - var areaDerivedArea = { - 'geometry': 'closedway', - 'equals': { area: 'yes' }, - }; - - var badAreaDerivedLine = { - 'geometry': 'closedway', - 'equals': { 'area': 'no' } - }; - - var roundHouseRailwayDerivedArea = { - 'geometry': 'closedway', - 'equals': { 'railway': 'roundhouse' } - }; - - var justClosedWayDerivedLine = { - 'geometry': 'closedway' - }; - - var rule, geom; - rule = iD.utilMapCSSRule(amenityDerivedArea, areaKeys); - geom = rule.inferGeometry(); - expect(geom).to.be.eql('area'); - - rule = iD.utilMapCSSRule(areaDerivedArea, areaKeys); - geom = rule.inferGeometry(); - expect(geom).to.be.eql('area'); - - rule = iD.utilMapCSSRule(badAreaDerivedLine, areaKeys); - geom = rule.inferGeometry(); - expect(geom).to.be.eql('line'); - - rule = iD.utilMapCSSRule(roundHouseRailwayDerivedArea, areaKeys); - geom = rule.inferGeometry(); - expect(geom).to.be.eql('area'); - - rule = iD.utilMapCSSRule(justClosedWayDerivedLine, areaKeys); - geom = rule.inferGeometry(); - expect(geom).to.be.eql('line'); - }); - }); - describe('#findWarnings', function() { - it('adds found warnings to warnings array', function() { - var graph = iD.Graph([entities]); - var warnings = []; - - rules.forEach(function(rule) { - entities.forEach(function(entity) { - rule.findWarnings(entity, graph, warnings); - }); - }); - - warnings.forEach(function(warning) { - expect(warning.message).to.not.be.null; - expect(['mapcss_warning', 'mapcss_error'].indexOf(warning.id)).to.be.greaterThan(-1); - expect(warning.entity).to.be.instanceOf(iD.Entity); - }); - }); - }); -}); - From f3e645919125aa1a0a97ec5ce768355c9d9c1d52 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Thu, 13 Dec 2018 11:16:24 -0500 Subject: [PATCH 14/27] working on preset visibility --- dist/locales/en.json | 14 ++++++++++++++ modules/presets/index.js | 24 ++++++------------------ modules/presets/preset.js | 9 ++++++++- modules/ui/preset_list.js | 9 ++++++--- test/index.html | 10 +++++----- test/spec/presets/index.js | 6 ++++++ test/spec/presets/preset.js | 9 +++++++++ 7 files changed, 54 insertions(+), 27 deletions(-) diff --git a/dist/locales/en.json b/dist/locales/en.json index 52bafa357..6945836cb 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -7182,6 +7182,13 @@ "description": "Japan GSI Standard Map. Widely covered.", "name": "Japan GSI Standard Map" }, + "helsingborg-orto": { + "attribution": { + "text": "© Helsingborg municipality" + }, + "description": "Orthophotos from the municipality of Helsingborg 2016, public domain", + "name": "Helsingborg Orthophoto" + }, "hike_n_bike": { "attribution": { "text": "© OpenStreetMap contributors" @@ -7273,6 +7280,13 @@ }, "name": "Stamen Terrain" }, + "stockholm-orto": { + "attribution": { + "text": "© Stockholm municipality, CC0" + }, + "description": "Orthophotos from the municipality of Stockholm 2015, CC0 license", + "name": "Stockholm Orthophoto" + }, "tf-cycle": { "attribution": { "text": "Maps © Thunderforest, Data © OpenStreetMap contributors" diff --git a/modules/presets/index.js b/modules/presets/index.js index aa1924032..b038890a9 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -124,14 +124,7 @@ export function presetIndex() { return areaKeys; }; - all.build = function () { - var d = data.presets; - all.collection = []; - _recent.collection = []; - _fields = {}; - _universal = []; - _index = { point: {}, vertex: {}, line: {}, area: {}, relation: {} }; - + all.build = function (d, visible) { if (d.fields) { _forEach(d.fields, function(d, id) { _fields[id] = presetField(id, d); @@ -143,7 +136,7 @@ export function presetIndex() { if (d.presets) { _forEach(d.presets, function(d, id) { - all.collection.push(presetPreset(id, d, _fields)); + all.collection.push(presetPreset(id, d, _fields, visible)); }); } @@ -185,20 +178,15 @@ export function presetIndex() { var presetsUrl = utilStringQs(window.location.hash).presets; d3_json(presetsUrl, function(err, presets) { if (err) all.init(); - all.overwrite(presets); + all.build(presets, true); + all.build(data.presets, false); all.areaKeys(); }); return all; }; - all.overwrite = function (d) { - data.presets = d; - all.build(); - return all; - }; - all.init = function() { - all.build(); + all.build(data.presets, true); return all; }; @@ -213,7 +201,7 @@ export function presetIndex() { all.defaults = function(geometry, n) { var rec = _recent.matchGeometry(geometry).collection.slice(0, 4); var def = _uniq(rec.concat(_defaults[geometry].collection)).slice(0, n - 1); - var fin = _uniq(rec.concat(def).concat(all.item(geometry))).filter(function(d) { return d !== undefined; }); + var fin = _uniq(rec.concat(def).concat(all.item(geometry))).filter(function(d) { return d.visible(); }); return presetCollection(fin); }; diff --git a/modules/presets/preset.js b/modules/presets/preset.js index f4e4db27d..f3734177a 100644 --- a/modules/presets/preset.js +++ b/modules/presets/preset.js @@ -6,13 +6,14 @@ import { t } from '../util/locale'; import { areaKeys } from '../core/context'; -export function presetPreset(id, preset, fields) { +export function presetPreset(id, preset, fields, visible) { preset = _clone(preset); preset.id = id; preset.fields = (preset.fields || []).map(getFields); preset.geometry = (preset.geometry || []); + visible = visible || false; function getFields(f) { return fields[f]; @@ -71,6 +72,12 @@ export function presetPreset(id, preset, fields) { return tagCount === 0 || (tagCount === 1 && preset.tags.hasOwnProperty('area')); }; + preset.visible = function(_) { + if (!arguments.length) return visible; + visible = _; + return visible; + }; + var reference = preset.reference || {}; preset.reference = function(geometry) { diff --git a/modules/ui/preset_list.js b/modules/ui/preset_list.js index 3360aeb8c..eb92d944e 100644 --- a/modules/ui/preset_list.js +++ b/modules/ui/preset_list.js @@ -137,9 +137,12 @@ export function uiPresetList(context) { function drawList(list, presets) { - var collection = presets.collection.map(function(preset) { - return preset.members ? CategoryItem(preset) : PresetItem(preset); - }); + var collection = presets.collection.reduce(function(collection, preset) { + if (preset.visible()) { + collection.push(preset.members ? CategoryItem(preset) : PresetItem(preset)); + } + return collection; + }, []); var items = list.selectAll('.preset-list-item') .data(collection, function(d) { return d.preset.id; }); diff --git a/test/index.html b/test/index.html index caf87bc2e..fa6fc48e1 100644 --- a/test/index.html +++ b/test/index.html @@ -30,7 +30,7 @@ - + - + + @@ -145,7 +145,7 @@ - + --> diff --git a/test/spec/presets/index.js b/test/spec/presets/index.js index 742a0a6b7..51a2613ed 100644 --- a/test/spec/presets/index.js +++ b/test/spec/presets/index.js @@ -196,6 +196,12 @@ describe('iD.presetIndex', function() { expect(currentPresets).to.not.eql(overwrittenPresets); }); }); + + describe('#build', function () { + it('builds presets from provided', function() { + + }); + }); describe('expected matches', function() { diff --git a/test/spec/presets/preset.js b/test/spec/presets/preset.js index 1feff1082..8bed21ff5 100644 --- a/test/spec/presets/preset.js +++ b/test/spec/presets/preset.js @@ -149,4 +149,13 @@ describe('iD.presetPreset', function() { expect(preset.removeTags({a: 'b'}, 'area')).to.eql({a: 'b'}); }); }); + + describe('#visible', function() { + it('sets/gets visibility of preset', function() { + var preset = iD.presetPreset('test', {}, false); + expect(preset.visible()).to.be.false; + preset.visible(true); + expect(preset.visible()).to.be.true; + }); + }); }); From c897331cd1435200ba8658f53723f2bd10a4b830 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Thu, 13 Dec 2018 13:30:38 -0500 Subject: [PATCH 15/27] presets test w/build --- modules/core/context.js | 9 +++-- modules/presets/index.js | 75 +++++++++++++++++------------------ modules/util/index.js | 2 +- test/index.html | 32 +++++++-------- test/spec/presets/index.js | 81 +++++++++++++++++++------------------- 5 files changed, 98 insertions(+), 101 deletions(-) diff --git a/modules/core/context.js b/modules/core/context.js index 6f41a0a3a..c0041b7c9 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -21,7 +21,8 @@ import { rendererBackground, rendererFeatures, rendererMap } from '../renderer'; import { services } from '../services'; import { uiInit } from '../ui/init'; import { utilDetect } from '../util/detect'; -import { utilCallWhenIdle, utilKeybinding, utilRebind } from '../util'; +import { utilCallWhenIdle, utilKeybinding, utilRebind, utilStringQs } from '../util'; + export var areaKeys = {}; @@ -441,16 +442,16 @@ export function coreContext() { } /* Presets */ - var presets; presets = presetIndex(); - if (utilExternalPresets()) { + if (utilStringQs(window.location.hash).presets) { presets.fromExternal(); } else { presets.init(); } + context.presets = function() { return presets; }; - if (utilExternalValidationRules()) { + if (utilStringQs(window.location.hash).validations) { var validationsUrl = utilStringQs(window.location.hash).validations; d3_json(validationsUrl, function (err, mapcss) { if (err) return; diff --git a/modules/presets/index.js b/modules/presets/index.js index 0152fdb5c..7a71c5d63 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -3,18 +3,14 @@ import _forEach from 'lodash-es/forEach'; import _reject from 'lodash-es/reject'; import _uniq from 'lodash-es/uniq'; +import { json as d3_json } from 'd3-request'; + import { data } from '../../data/index'; import { presetCategory } from './category'; import { presetCollection } from './collection'; import { presetField } from './field'; import { presetPreset } from './preset'; -import { utilStringQs } from '../util'; - -import { json as d3_json } from 'd3-request'; -import { - select as d3_select, - selectAll as d3_selectAll -} from 'd3-selection'; +import { utilQsString } from '../util'; export { presetCategory }; export { presetCollection }; @@ -27,7 +23,7 @@ export function presetIndex() { // loading new data and returning defaults var all = presetCollection([]); - var _defaults = {}; + var _defaults = { area: all, line: all, point: all, vertex: all, relation: all }; var _fields = {}; var _universal = []; var _recent = presetCollection([]); @@ -123,10 +119,11 @@ export function presetIndex() { areaKeys[key][value] = true; } }); + return areaKeys; }; - all.build = function (d, visible) { + all.build = function(d, visibility) { if (d.fields) { _forEach(d.fields, function(d, id) { _fields[id] = presetField(id, d); @@ -142,7 +139,7 @@ export function presetIndex() { if (d.presets) { _forEach(d.presets, function(d, id) { - all.collection.push(presetPreset(id, d, _fields, visible)); + all.collection.push(presetPreset(id, d, _fields, visibility)); }); } @@ -154,16 +151,13 @@ export function presetIndex() { if (d.defaults) { var getItem = _bind(all.item, all); - _forEach(Object.keys(d.defaults), function (k) { - _defaults[k] = presetCollection(d.defaults[k].map(getItem)); - }); - // _defaults = { - // area: presetCollection(d.defaults.area.map(getItem)), - // line: presetCollection(d.defaults.line.map(getItem)), - // point: presetCollection(d.defaults.point.map(getItem)), - // vertex: presetCollection(d.defaults.vertex.map(getItem)), - // relation: presetCollection(d.defaults.relation.map(getItem)) - // }; + _defaults = { + area: presetCollection(d.defaults.area.map(getItem)), + line: presetCollection(d.defaults.line.map(getItem)), + point: presetCollection(d.defaults.point.map(getItem)), + vertex: presetCollection(d.defaults.vertex.map(getItem)), + relation: presetCollection(d.defaults.relation.map(getItem)) + }; } for (var i = 0; i < all.collection.length; i++) { @@ -177,22 +171,30 @@ export function presetIndex() { } } } - - }; - - all.fromExternal = function() { - var presetsUrl = utilStringQs(window.location.hash).presets; - d3_json(presetsUrl, function(err, presets) { - if (err) all.init(); - all.build(presets, true); - all.build(data.presets, false); - all.areaKeys(); - }); - return all; }; all.init = function() { - all.build(data.presets, true); + all.collection = []; + _recent.collection = []; + _fields = {}; + _universal = []; + _index = { point: {}, vertex: {}, line: {}, area: {}, relation: {} }; + + all.build(data.presets); + + return all; + }; + + all.fromExternal = function() { + var external = utilQsString(window.location.hash).presets; + d3_json(external, function(err, presets) { + if (err) { + all.init(); + } else { + all.build(data.presets, false); // make default presets hidden to begin + all.build(presets, true); // make the external visible + } + }); return all; }; @@ -207,12 +209,7 @@ export function presetIndex() { all.defaults = function(geometry, n) { var rec = _recent.matchGeometry(geometry).collection.slice(0, 4); var def = _uniq(rec.concat(_defaults[geometry].collection)).slice(0, n - 1); - var fin = _uniq(rec.concat(def).concat(all.item(geometry))).filter(function(d) { return d.visible(); }); - return presetCollection(fin); - }; - - all.defaultTypes = function() { - return Object.keys(_defaults); + return presetCollection(_uniq(rec.concat(def).concat(all.item(geometry)))); }; all.choose = function(preset) { diff --git a/modules/util/index.js b/modules/util/index.js index a8d1ad6e0..0517ce874 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -27,7 +27,7 @@ export { utilRebind } from './rebind'; export { utilSetTransform } from './util'; export { utilSessionMutex } from './session_mutex'; export { utilStringQs } from './util'; -export { utilSuggestNames } from './suggest_names'; +// export { utilSuggestNames } from './suggest_names'; export { utilTagText } from './util'; export { utilTiler } from './tiler'; export { utilTriggerEvent } from './trigger_event'; diff --git a/test/index.html b/test/index.html index 4600849c5..9b324a0c7 100644 --- a/test/index.html +++ b/test/index.html @@ -62,29 +62,29 @@ - + --> - + - + - + - - - - + + + - - - --> - + + + - + - + - + - + - + + + + - - + - + + - + - + From d8de9e785eb196cc0724f3f5d168b3958852d8c7 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Mon, 17 Dec 2018 14:56:33 -0500 Subject: [PATCH 22/27] do not add found presets to recent --- modules/ui/entity_editor.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index de0181383..68f41dea6 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -218,8 +218,6 @@ export function uiEntityEditor(context) { // A "weak" preset doesn't set any tags. (e.g. "Address") // Don't replace a weak preset with a fallback preset (e.g. "Point") if (!(weakPreset && match.isFallback())) { - match.visible(true); - context.presets().choose(match); entityEditor.preset(match); } entityEditor.modified(_base !== graph); From 8f33a85a1243debe7a6a3473da7ff3d08e417715 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Mon, 17 Dec 2018 17:16:10 -0500 Subject: [PATCH 23/27] make collection.index function to handle failing tests --- modules/core/history.js | 1 - modules/presets/collection.js | 6 ++++++ modules/presets/index.js | 5 ++--- modules/services/maprules.js | 1 - test/spec/presets/collection.js | 9 +++++++++ test/spec/services/maprules.js | 2 +- 6 files changed, 18 insertions(+), 6 deletions(-) diff --git a/modules/core/history.js b/modules/core/history.js index d6194a561..db837d45c 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -9,7 +9,6 @@ import _isEmpty from 'lodash-es/isEmpty'; import _forEach from 'lodash-es/forEach'; import _map from 'lodash-es/map'; import _omit from 'lodash-es/omit'; -import _reduce from 'lodash-es/reduce'; import _reject from 'lodash-es/reject'; import _values from 'lodash-es/values'; import _without from 'lodash-es/without'; diff --git a/modules/presets/collection.js b/modules/presets/collection.js index f16960173..cddd64f3a 100644 --- a/modules/presets/collection.js +++ b/modules/presets/collection.js @@ -1,5 +1,6 @@ import _filter from 'lodash-es/filter'; import _find from 'lodash-es/find'; +import _findIndex from 'lodash-es/findIndex'; import _some from 'lodash-es/some'; import _uniq from 'lodash-es/uniq'; import _values from 'lodash-es/values'; @@ -23,6 +24,11 @@ export function presetCollection(collection) { }); }, + index: function(id) { + return _findIndex(this.collection, function(d) { + return d.id === id; + }); + }, matchGeometry: function(geometry) { return presetCollection(this.collection.filter(function(d) { diff --git a/modules/presets/index.js b/modules/presets/index.js index b99309e78..bade492ed 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -10,7 +10,6 @@ import { presetCategory } from './category'; import { presetCollection } from './collection'; import { presetField } from './field'; import { presetPreset } from './preset'; -import { utilQsString } from '../util'; export { presetCategory }; export { presetCollection }; @@ -138,7 +137,7 @@ export function presetIndex() { if (d.presets) { _forEach(d.presets, function(d, id) { - var existing = all.collection.findIndex(function(p) { return p.id === id; }); + var existing = all.index(id); if (existing !== -1) { all.collection[existing] = presetPreset(id, d, _fields, visible); } else { @@ -149,7 +148,7 @@ export function presetIndex() { if (d.categories) { _forEach(d.categories, function(d, id) { - var existing = all.collection.findIndex(function(p) { return p.id === id; }); + var existing = all.index(id); if (existing !== -1) { all.collection[existing] = presetCategory(id, d, all); } else { diff --git a/modules/services/maprules.js b/modules/services/maprules.js index b55f39017..65c863f53 100644 --- a/modules/services/maprules.js +++ b/modules/services/maprules.js @@ -187,7 +187,6 @@ export default { }, // adds from mapcss-parse selector check... addRule: function(selector) { - var _areaKeys = this._areaKeys; var rule = { // checks relevant to mapcss-selector checks: this.filterRuleChecks(selector), diff --git a/test/spec/presets/collection.js b/test/spec/presets/collection.js index 5b1b559b9..64db4bcb8 100644 --- a/test/spec/presets/collection.js +++ b/test/spec/presets/collection.js @@ -84,6 +84,15 @@ describe('iD.presetCollection', function() { }); }); + describe('#index', function() { + it('returns preset position in the collection', function() { + expect(c.index('point')).to.equal(0); + }); + it('return -1 when given id for preset not in the collection', function() { + expect(c.index('foobar')).to.equal(-1); + }); + }); + describe('#matchGeometry', function() { it('returns a new collection only containing presets matching a geometry', function() { expect(c.matchGeometry('area').collection).to.include.members( diff --git a/test/spec/services/maprules.js b/test/spec/services/maprules.js index 34b741b90..51d62f0cf 100644 --- a/test/spec/services/maprules.js +++ b/test/spec/services/maprules.js @@ -300,7 +300,7 @@ describe('maprules', function() { }); }); describe('rule', function() { - var selectors, entities; + var selectors; before(function() { selectors = [ { From 64f2d913e85c46f2a7b3166393836747a83e0c94 Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Mon, 17 Dec 2018 17:39:47 -0500 Subject: [PATCH 24/27] remove unused entities from test --- modules/services/maprules.js | 2 +- test/spec/services/maprules.js | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/modules/services/maprules.js b/modules/services/maprules.js index 65c863f53..5944d8ae1 100644 --- a/modules/services/maprules.js +++ b/modules/services/maprules.js @@ -210,7 +210,7 @@ export default { if (this.geometryMatches(entity, graph) && this.matches(entity)) { var type = Object.keys(selector).indexOf('error') > -1 ? 'error' : 'warning'; warnings.push({ - id: 'mapcss_' + type, + severity: type, message: selector[type], entity: entity }); diff --git a/test/spec/services/maprules.js b/test/spec/services/maprules.js index 51d62f0cf..d4c535f40 100644 --- a/test/spec/services/maprules.js +++ b/test/spec/services/maprules.js @@ -357,17 +357,6 @@ describe('maprules', function() { error: '\'suburban road\' structure tag cannot be \'bridge\' or \'tunnel\'' } ]; - entities = [ - iD.Entity({ type: 'node', tags: { amenity: 'marketplace' }}), - iD.Way({ tags: { building: 'house', amenity: 'clinic' }, nodes: [ 'a', 'b', 'c', 'a' ]}), - iD.Entity({ type: 'node', tags: { man_made: 'tower', 'tower:type': 'communication', height: 5 }}), - iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 6 }}), - iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 9 }}), - iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 5 }}), - iD.Entity({ type: 'node', tags: { man_made: 'tower', height: 10 }}), - iD.Way({ tags: { amenity: 'clinic', emergency: 'definitely' }, nodes: [ 'd', 'e', 'f', 'd' ]}), - iD.Way({ tags: { highway: 'residential', structure: 'bridge' }}), - ]; iD.serviceMapRules.clearRules(); selectors.forEach(function(selector) { iD.serviceMapRules.addRule(selector); }); From 6ba8bacb7562aaa067bfdf9a29f17809dcaaaf4f Mon Sep 17 00:00:00 2001 From: Max Grossman Date: Tue, 18 Dec 2018 10:27:15 -0500 Subject: [PATCH 25/27] use severity in test --- modules/ui/commit_warnings.js | 8 ++++---- test/spec/services/maprules.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 2317fc9f5..8b517c9c3 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -15,11 +15,11 @@ export function uiCommitWarnings(context) { var validations = context.history().validate(changes); validations = _reduce(validations, function(validations, val) { - var type = val.id === 'mapcss_error' ? 'error' : 'warning'; - if (validations.hasOwnProperty(type)) { - validations[type].push(val); + var severity = val.severity; + if (validations.hasOwnProperty(severity)) { + validations[severity].push(val); } else { - validations[type] = [val]; + validations[severity] = [val]; } return validations; }, {}); diff --git a/test/spec/services/maprules.js b/test/spec/services/maprules.js index d4c535f40..cf17f7266 100644 --- a/test/spec/services/maprules.js +++ b/test/spec/services/maprules.js @@ -554,7 +554,7 @@ describe('maprules', function() { expect(warnings.length).to.eql(1); expect(warning.entity).to.eql(entity); expect(warning.message).to.eql(selector[type]); - expect('mapcss_' + type).to.eql(warning.id); + expect(type).to.eql(warning.severity); }); }); }); From 5a2049b4212efa152599a33f73b64d2772bac191 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 18 Dec 2018 15:38:53 -0500 Subject: [PATCH 26/27] Drop extra whitespace, make sure service exists before using it --- dist/locales/en.json | 48 +++------------------------- modules/core/context.js | 6 ++-- modules/presets/index.js | 6 ++-- modules/services/maprules.js | 32 +++++++++---------- modules/validations/mapcss_checks.js | 6 ++-- test/index.html | 3 +- test/spec/services/maprules.js | 40 +++++++++++++---------- 7 files changed, 55 insertions(+), 86 deletions(-) diff --git a/dist/locales/en.json b/dist/locales/en.json index 660ef793a..ea503fedb 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1,9 +1,13 @@ { "en": { "icons": { + "download": "download", "information": "info", "remove": "remove", - "undo": "undo" + "undo": "undo", + "zoom_to": "zoom to", + "copy": "copy", + "open_wikidata": "open on wikidata.org" }, "modes": { "add_area": { @@ -7682,41 +7686,6 @@ "description": "Orthophotos from the municipality of Helsingborg 2016, public domain", "name": "Helsingborg Orthophoto" }, - "eufar-balaton": { - "attribution": { - "text": "EUFAR Balaton ortofotó 2010" - }, - "description": "1940 geo-tagged photography from Balaton Limnological Institute.", - "name": "EUFAR Balaton orthophotos" - }, - "finds.jp_KBN_2500": { - "attribution": { - "text": "GSI KIBAN 2500" - }, - "description": "GSI Kiban 2500 via finds.jp. Good for tracing, but a bit older.", - "name": "Japan GSI KIBAN 2500" - }, - "gsi.go.jp": { - "attribution": { - "text": "GSI Japan" - }, - "description": "Japan GSI ortho Imagery. Usually better than bing, but a bit older.", - "name": "Japan GSI ortho Imagery" - }, - "gsi.go.jp_std_map": { - "attribution": { - "text": "GSI Japan" - }, - "description": "Japan GSI Standard Map. Widely covered.", - "name": "Japan GSI Standard Map" - }, - "helsingborg-orto": { - "attribution": { - "text": "© Helsingborg municipality" - }, - "description": "Orthophotos from the municipality of Helsingborg 2016, public domain", - "name": "Helsingborg Orthophoto" - }, "kalmar-orto-2014": { "attribution": { "text": "© Kalmar municipality" @@ -7844,13 +7813,6 @@ "description": "Orthophotos from the municipality of Stockholm 2015, CC0 license", "name": "Stockholm Orthophoto" }, - "stockholm-orto": { - "attribution": { - "text": "© Stockholm municipality, CC0" - }, - "description": "Orthophotos from the municipality of Stockholm 2015, CC0 license", - "name": "Stockholm Orthophoto" - }, "tf-cycle": { "attribution": { "text": "Maps © Thunderforest, Data © OpenStreetMap contributors" diff --git a/modules/core/context.js b/modules/core/context.js index c99d4eb08..53385070d 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -470,14 +470,14 @@ export function coreContext() { background = rendererBackground(context); features = rendererFeatures(context); presets = presetIndex(); - - if (utilStringQs(window.location.hash).validations) { + + if (services.maprules && utilStringQs(window.location.hash).validations) { var validations = utilStringQs(window.location.hash).validations; d3_json(validations, function (err, mapcss) { if (err) return; services.maprules.init(context.presets().areaKeys()); _each(mapcss, function(mapcssSelector) { - return services.maprules.addRule(mapcssSelector); + return services.maprules.addRule(mapcssSelector); }); context.validationRules = true; }); diff --git a/modules/presets/index.js b/modules/presets/index.js index bade492ed..ddeeb3ba9 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -173,7 +173,7 @@ export function presetIndex() { var geometry = preset.geometry; for (var j = 0; j < geometry.length; j++) { - var g = _index[geometry[j]]; + var g = _index[geometry[j]]; for (var k in preset.tags) { (g[k] = g[k] || []).push(preset); } @@ -183,7 +183,7 @@ export function presetIndex() { }; all.init = function() { - all.collection = []; + all.collection = []; _recent.collection = []; _fields = {}; _universal = []; @@ -192,7 +192,7 @@ export function presetIndex() { return all.build(data.presets, true); }; - + all.reset = function() { all.collection = []; _defaults = { area: all, line: all, point: all, vertex: all, relation: all }; diff --git a/modules/services/maprules.js b/modules/services/maprules.js index 5944d8ae1..9e2f44a4f 100644 --- a/modules/services/maprules.js +++ b/modules/services/maprules.js @@ -16,7 +16,7 @@ var buildRuleChecks = function() { }; }, absence: function(absence) { - return function(tags) { + return function(tags) { return Object.keys(tags).indexOf(absence) === -1; }; }, @@ -44,7 +44,7 @@ var buildRuleChecks = function() { lessThan: function(lessThan) { var key = Object.keys(lessThan)[0]; var value = lessThan[key]; - + return function(tags) { return tags[key] < value; }; @@ -52,9 +52,9 @@ var buildRuleChecks = function() { lessThanEqual: function(lessThanEqual) { var key = Object.keys(lessThanEqual)[0]; var value = lessThanEqual[key]; - + return function(tags) { - return tags[key] <= value; + return tags[key] <= value; }; }, positiveRegex: function(positiveRegex) { @@ -70,7 +70,7 @@ var buildRuleChecks = function() { var tagKey = Object.keys(negativeRegex)[0]; var expression = negativeRegex[tagKey].join('|'); var regex = new RegExp(expression); - + return function(tags) { return !regex.test(tags[tagKey]); }; @@ -128,23 +128,23 @@ export default { if (isRegex || isEqual) { Object.keys(selector[key]).forEach(function(selectorKey) { values = isEqual ? [selector[key][selectorKey]] : getRegexValues(selector[key][selectorKey]); - + if (expectedTags.hasOwnProperty(selectorKey)) { values = values.concat(expectedTags[selectorKey]); } - + expectedTags[selectorKey] = values; }); - + } else if (/(greater|less)Than(Equal)?|presence/g.test(key)) { var tagKey = /presence/.test(key) ? selector[key] : Object.keys(selector[key])[0]; - + values = [selector[key][tagKey]]; if (expectedTags.hasOwnProperty(tagKey)) { values = values.concat(expectedTags[tagKey]); } - + expectedTags[tagKey] = values; } @@ -173,7 +173,7 @@ export default { return 'line'; } } - + for (var key in tagMap) { if (key in _areaKeys && !isAreaKeyBlackList(key)) { return 'area'; @@ -192,15 +192,15 @@ export default { checks: this.filterRuleChecks(selector), // true if all conditions for a tag error are true.. matches: function(entity) { - return _every(this.checks, function(check) { - return check(entity.tags); + return _every(this.checks, function(check) { + return check(entity.tags); }); }, // borrowed from Way#isArea() - inferredGeometry: this.inferGeometry(this.buildTagMap(selector), this._areaKeys), + inferredGeometry: this.inferGeometry(this.buildTagMap(selector), this._areaKeys), geometryMatches: function(entity, graph) { - if (entity.type === 'node' || entity.type === 'relation') { - return selector.geometry === entity.type; + if (entity.type === 'node' || entity.type === 'relation') { + return selector.geometry === entity.type; } else if (entity.type === 'way') { return this.inferredGeometry === entity.geometry(graph); } diff --git a/modules/validations/mapcss_checks.js b/modules/validations/mapcss_checks.js index 86255a917..13fa6fe2b 100644 --- a/modules/validations/mapcss_checks.js +++ b/modules/validations/mapcss_checks.js @@ -1,8 +1,10 @@ -import { serviceMapRules } from '../services'; +import { services } from '../services'; export function validationMapCSSChecks() { var validation = function(changes, graph) { - var rules = serviceMapRules.validationRules(); + if (!services.maprules) return []; + + var rules = services.maprules.validationRules(); var warnings = []; var createdModified = ['created', 'modified']; diff --git a/test/index.html b/test/index.html index 04ebd669f..b455624b3 100644 --- a/test/index.html +++ b/test/index.html @@ -106,14 +106,13 @@ + - - diff --git a/test/spec/services/maprules.js b/test/spec/services/maprules.js index cf17f7266..188588d0e 100644 --- a/test/spec/services/maprules.js +++ b/test/spec/services/maprules.js @@ -1,11 +1,17 @@ describe('maprules', function() { - var _ruleChecks, validationRules; + var _ruleChecks, validationRules; + before(function() { + iD.services.maprules = iD.serviceMapRules; var areaKeys = iD.Context().presets().areaKeys(); iD.serviceMapRules.init(areaKeys); _ruleChecks = iD.serviceMapRules.ruleChecks(); }); + after(function() { + delete iD.services.maprules; + }); + describe('#filterRuleChecks', function() { it('returns shortlist of mapcss checks relevant to provided selector', function() { var selector = { @@ -30,9 +36,9 @@ describe('maprules', function() { [ { t: { - equals: { - man_made: 'tower', - 'tower:type': 'communication' + equals: { + man_made: 'tower', + 'tower:type': 'communication' } }, r: { @@ -42,9 +48,9 @@ describe('maprules', function() { }, { t: { - equals: { - building: 'yes', - amenity: 'school' + equals: { + building: 'yes', + amenity: 'school' }, positiveRegex: { opening_hours: [ @@ -96,7 +102,7 @@ describe('maprules', function() { positiveRegex: { amenity: ['^school$', '^healthcare$'] }, error: 'amenity cannot be healthcare or school!' }; - + var areaDerivedArea = { geometry: 'closedway', equals: { area: 'yes' }, @@ -141,7 +147,7 @@ describe('maprules', function() { describe('#addRule', function() { it ('builds a rule from provided selector and adds it to _validationRules', function () { - var selector = { + var selector = { geometry:'node', equals: {amenity:'marketplace'}, absence:'name', @@ -303,7 +309,7 @@ describe('maprules', function() { var selectors; before(function() { selectors = [ - { + { geometry:'node', equals: {amenity:'marketplace'}, absence:'name', @@ -361,12 +367,12 @@ describe('maprules', function() { iD.serviceMapRules.clearRules(); selectors.forEach(function(selector) { iD.serviceMapRules.addRule(selector); }); validationRules = iD.serviceMapRules.validationRules(); - }); + }); describe('#matches', function() { var selectors, entities; before(function() { selectors = [ - { + { geometry:'node', equals: {amenity:'marketplace'}, absence:'name', @@ -435,7 +441,7 @@ describe('maprules', function() { iD.serviceMapRules.clearRules(); selectors.forEach(function(selector) { iD.serviceMapRules.addRule(selector); }); validationRules = iD.serviceMapRules.validationRules(); - }); + }); it('is true when each rule check is \'true\'', function() { validationRules.forEach(function(rule, i) { expect(rule.matches(entities[i])).to.be.true; @@ -461,7 +467,7 @@ describe('maprules', function() { before(function() { selectors = [ - { + { geometry:'node', equals: {amenity:'marketplace'}, absence:'name', @@ -545,12 +551,12 @@ describe('maprules', function() { var warnings = []; var entity = entities[i]; var selector = selectors[i]; - + rule.findWarnings(entity, _graph, warnings); - + var warning = warnings[0]; var type = Object.keys(selector).indexOf('error') ? 'error' : 'warning'; - + expect(warnings.length).to.eql(1); expect(warning.entity).to.eql(entity); expect(warning.message).to.eql(selector[type]); From 6e1dd344471b8a9f9f96fae3a703c5edab42fccf Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Tue, 18 Dec 2018 15:55:50 -0500 Subject: [PATCH 27/27] Fixed an issue where a new container would be added in the Map Data pane every time a section was re-expanded --- modules/ui/map_data.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/ui/map_data.js b/modules/ui/map_data.js index 3fa8cfb8f..e12737025 100644 --- a/modules/ui/map_data.js +++ b/modules/ui/map_data.js @@ -466,7 +466,7 @@ export function uiMapData(context) { function renderDataLayers(selection) { - var container = selection.selectAll('data-layer-container') + var container = selection.selectAll('.data-layer-container') .data([0]); _dataLayerContainer = container.enter() @@ -477,7 +477,7 @@ export function uiMapData(context) { function renderFillList(selection) { - var container = selection.selectAll('layer-fill-list') + var container = selection.selectAll('.layer-fill-list') .data([0]); _fillList = container.enter() @@ -488,7 +488,7 @@ export function uiMapData(context) { function renderFeatureList(selection) { - var container = selection.selectAll('layer-feature-list') + var container = selection.selectAll('.layer-feature-list') .data([0]); _featureList = container.enter()