From 03bb9162870e562fc6481b55b1c09bb13eb36086 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 28 Apr 2016 16:59:12 -0400 Subject: [PATCH] Adjust taginfo values filtering, update tests --- js/id/services/taginfo.js | 20 +++++++++------- test/spec/services/taginfo.js | 45 +++++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/js/id/services/taginfo.js b/js/id/services/taginfo.js index 0af481616..76148c014 100644 --- a/js/id/services/taginfo.js +++ b/js/id/services/taginfo.js @@ -34,11 +34,10 @@ iD.services.taginfo = function() { return _.omit(parameters, 'geometry', 'debounce'); } - function filterKeys(parameters) { - var pop_field = 'count_all'; - if (parameters.filter) pop_field = 'count_' + parameters.filter; + function filterKeys(type) { + var count_type = type ? 'count_' + type : 'count_all'; return function(d) { - return parseFloat(d[pop_field]) > 2500 || d.in_wiki; + return parseFloat(d[count_type]) > 2500 || d.in_wiki; }; } @@ -50,8 +49,8 @@ iD.services.taginfo = function() { function filterValues() { return function(d) { - return d.value.match(/;/g) === null && // exclude values with ';' - (parseFloat(d.fraction) > 0.0 || d.in_wiki); + if (d.value.match(/[A-Z*;,]/) !== null) return false; // exclude some punctuation, uppercase letters + return parseFloat(d.fraction) > 0.0 || d.in_wiki; }; } @@ -106,7 +105,8 @@ iD.services.taginfo = function() { page: 1 }, parameters)), debounce, function(err, d) { if (err) return callback(err); - callback(null, d.data.filter(filterKeys(parameters)).sort(sortKeys).map(valKey)); + var f = filterKeys(parameters.filter); + callback(null, d.data.filter(f).sort(sortKeys).map(valKey)); }); }; @@ -121,7 +121,8 @@ iD.services.taginfo = function() { page: 1 }, parameters)), debounce, function(err, d) { if (err) return callback(err); - callback(null, d.data.filter(filterMultikeys(parameters)).map(valKey)); + var f = filterMultikeys(); + callback(null, d.data.filter(f).map(valKey)); }); }; @@ -136,7 +137,8 @@ iD.services.taginfo = function() { page: 1 }, parameters)), debounce, function(err, d) { if (err) return callback(err); - callback(null, d.data.filter(filterValues()).map(valKeyDescription), parameters); + var f = filterValues(); + callback(null, d.data.filter(f).map(valKeyDescription)); }); }; diff --git a/test/spec/services/taginfo.js b/test/spec/services/taginfo.js index b595e13c7..55c938424 100644 --- a/test/spec/services/taginfo.js +++ b/test/spec/services/taginfo.js @@ -29,7 +29,7 @@ describe("iD.services.taginfo", function() { expect(callback).to.have.been.calledWith(null, [{"title":"amenity", "value":"amenity"}]); }); - it("filters only popular keys", function() { + it("includes popular keys", function() { var callback = sinon.spy(); taginfo.keys({query: "amen"}, callback); @@ -42,7 +42,7 @@ describe("iD.services.taginfo", function() { expect(callback).to.have.been.calledWith(null, [{"title":"amenity", "value":"amenity"}]); }); - it("filters only popular keys with an entity type filter", function() { + it("includes popular keys with an entity type filter", function() { var callback = sinon.spy(); taginfo.keys({query: "amen", filter: "nodes"}, callback); @@ -55,6 +55,22 @@ describe("iD.services.taginfo", function() { expect(callback).to.have.been.calledWith(null, [{"title":"amenity", "value":"amenity"}]); }); + it("includes unpopular keys with a wiki page", function() { + var callback = sinon.spy(); + taginfo.keys({query: "amen"}, callback); + + server.respondWith("GET", new RegExp("https://taginfo.openstreetmap.org/api/4/keys/all"), + [200, { "Content-Type": "application/json" }, + '{"data":[{"count_all":5190337,"key":"amenity","count_all_fraction":1.0, "count_nodes_fraction":1.0},\ + {"count_all":1,"key":"amenityother","count_all_fraction":0.0, "count_nodes_fraction":0.0, "in_wiki": true}]}']); + server.respond(); + + expect(callback).to.have.been.calledWith(null, [ + {"title":"amenity", "value":"amenity"} + {"title":"amenityother", "value":"amenityother"} + ]); + }); + it("sorts keys with ':' below keys without ':'", function() { var callback = sinon.spy(); taginfo.keys({query: "ref"}, callback); @@ -113,7 +129,7 @@ describe("iD.services.taginfo", function() { expect(callback).to.have.been.calledWith(null, [{"value":"parking","title":"A place for parking cars"}]); }); - it("filters popular values", function() { + it("includes popular values", function() { var callback = sinon.spy(); taginfo.values({key: "amenity", query: "par"}, callback); @@ -126,14 +142,33 @@ describe("iD.services.taginfo", function() { expect(callback).to.have.been.calledWith(null, [{"value":"parking","title":"A place for parking cars"}]); }); - it("excludes values with semicolons", function() { + it("includes unpopular values with a wiki page", function() { var callback = sinon.spy(); taginfo.values({key: "amenity", query: "par"}, callback); server.respondWith("GET", new RegExp("https://taginfo.openstreetmap.org/api/4/key/values"), [200, { "Content-Type": "application/json" }, '{"data":[{"value":"parking","description":"A place for parking cars", "fraction":1.0},\ - {"value":"parking;partying","description":"A place for parking cars *and* partying", "fraction":1.0}]}']); + {"value":"party","description":"A place for partying", "fraction":0.0, "in_wiki": true}]}']); + server.respond(); + + expect(callback).to.have.been.calledWith(null, [ + {"value":"parking","title":"A place for parking cars"}, + {"value":"party","title":"A place for partying"} + ]); + }); + + it("excludes values with capital letters and some punctuation", function() { + var callback = sinon.spy(); + taginfo.values({key: "amenity", query: "par"}, callback); + + server.respondWith("GET", new RegExp("https://taginfo.openstreetmap.org/api/4/key/values"), + [200, { "Content-Type": "application/json" }, + '{"data":[{"value":"parking","description":"A place for parking cars", "fraction":0.2},\ + {"value":"PArking","description":"A common mispelling", "fraction":0.2},\ + {"value":"parking;partying","description":"A place for parking cars *and* partying", "fraction":0.2},\ + {"value":"parking, partying","description":"A place for parking cars *and* partying", "fraction":0.2},\ + {"value":"*","description":"", "fraction":0.2}]}']); server.respond(); expect(callback).to.have.been.calledWith(null, [{"value":"parking","title":"A place for parking cars"}]);