From b892b8c1569bbf547b5f38e5cd7164ed39003fbd Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 2 Dec 2015 11:44:00 -0500 Subject: [PATCH] Adjust to new taginfo wiki_pages API call (closes #2844) Also decouple tag_reference.js from taginfo.js service (no need to have special callback semantics just to retry the call w/o value) --- js/id/services/taginfo.js | 19 ++++--------------- js/id/ui/tag_reference.js | 39 +++++++++++++++++++------------------- test/spec/taginfo.js | 40 ++------------------------------------- 3 files changed, 26 insertions(+), 72 deletions(-) diff --git a/js/id/services/taginfo.js b/js/id/services/taginfo.js index a4f28c85d..750aeb767 100644 --- a/js/id/services/taginfo.js +++ b/js/id/services/taginfo.js @@ -113,21 +113,10 @@ iD.taginfo = function() { if (parameters.value) path = 'tag/wiki_pages?'; else if (parameters.rtype) path = 'relation/wiki_pages?'; - var decoratedCallback; - if (parameters.value) { - decoratedCallback = function(err, data) { - // The third argument to callback is the softfail flag, to - // make the callback function not show a message to the end - // user when no docs are found but just return false. - var docsFound = callback(err, data, true); - if (!docsFound) { - taginfo.docs(_.omit(parameters, 'value'), callback); - } - }; - } - - request(endpoint + path + - iD.util.qsString(parameters), debounce, decoratedCallback || callback); + request(endpoint + path + iD.util.qsString(parameters), debounce, function(err, d) { + if (err) return callback(err); + callback(null, d.data); + }); }; taginfo.endpoint = function(_) { diff --git a/js/id/ui/tag_reference.js b/js/id/ui/tag_reference.js index 05723ee8a..2d6860049 100644 --- a/js/id/ui/tag_reference.js +++ b/js/id/ui/tag_reference.js @@ -5,11 +5,11 @@ iD.ui.TagReference = function(tag, context) { loaded, showing; - function findLocal(docs) { + function findLocal(data) { var locale = iD.detect().locale.toLowerCase(), localized; - localized = _.find(docs, function(d) { + localized = _.find(data, function(d) { return d.lang.toLowerCase() === locale; }); if (localized) return localized; @@ -18,34 +18,37 @@ iD.ui.TagReference = function(tag, context) { // 'en' if the language is 'en-US' if (locale.indexOf('-') !== -1) { var first = locale.split('-')[0]; - localized = _.find(docs, function(d) { + localized = _.find(data, function(d) { return d.lang.toLowerCase() === first; }); if (localized) return localized; } // finally fall back to english - return _.find(docs, function(d) { + return _.find(data, function(d) { return d.lang.toLowerCase() === 'en'; }); } - function load() { + function load(param) { button.classed('tag-reference-loading', true); - context.taginfo().docs(tag, function(err, docs, softfail) { - if (!err && docs) { - docs = findLocal(docs); + context.taginfo().docs(param, function show(err, data) { + var docs; + if (!err && data) { + docs = findLocal(data); } body.html(''); if (!docs || !docs.description) { - if (!softfail) { + if (param.hasOwnProperty('value')) { + load(_.omit(param, 'value')); // retry with key only + } else { body.append('p').text(t('inspector.no_documentation_key')); - show(); + done(); } - return false; + return; } if (docs.image && docs.image.thumb_url_prefix) { @@ -53,10 +56,10 @@ iD.ui.TagReference = function(tag, context) { .append('img') .attr('class', 'wiki-image') .attr('src', docs.image.thumb_url_prefix + '100' + docs.image.thumb_url_suffix) - .on('load', function() { show(); }) - .on('error', function() { d3.select(this).remove(); show(); }); + .on('load', function() { done(); }) + .on('error', function() { d3.select(this).remove(); done(); }); } else { - show(); + done(); } body @@ -70,12 +73,10 @@ iD.ui.TagReference = function(tag, context) { .call(iD.svg.Icon('#icon-out-link', 'inline')) .append('span') .text(t('inspector.reference')); - - return true; }); } - function show() { + function done() { loaded = true; button.classed('tag-reference-loading', false); @@ -114,10 +115,10 @@ iD.ui.TagReference = function(tag, context) { if (showing) { hide(); } else if (loaded) { - show(); + done(); } else { if (context.taginfo()) { - load(); + load(tag); } } }); diff --git a/test/spec/taginfo.js b/test/spec/taginfo.js index dc1ca000d..dca47cca4 100644 --- a/test/spec/taginfo.js +++ b/test/spec/taginfo.js @@ -96,7 +96,7 @@ describe("iD.taginfo", function() { server.respondWith("GET", new RegExp("https://taginfo.openstreetmap.org/api/4/tag/wiki_page"), [200, { "Content-Type": "application/json" }, - '[{"on_way":false,"lang":"en","on_area":true,"image":"File:Car park2.jpg"}]']); + '{"data":[{"on_way":false,"lang":"en","on_area":true,"image":"File:Car park2.jpg"}]}']); server.respond(); expect(query(server.requests[0].url)).to.eql( @@ -104,42 +104,6 @@ describe("iD.taginfo", function() { expect(callback).to.have.been.calledWith(null, [{"on_way":false,"lang":"en","on_area":true,"image":"File:Car park2.jpg"}]); }); - - it("falls back to key if the callback function returns false", function() { - var callback = sinon.spy(function(err, docs, softfail) { - if (!err && docs) { - docs = (docs[0] && docs[0].lang === "en") ? docs[0] : false; - } - - if (!docs || !docs.description) { - if (!softfail) { - return null; - } - return false; - } - return true; - }); - - taginfo.docs({key: "amenity", value: "some-non-existing-value"}, callback); - - server.respondWith("GET", new RegExp("https://taginfo\\.openstreetmap\\.org/api/4/tag/wiki_page"), - [200, { "Content-Type": "application/json" }, '[]']); - server.respond(); - - server.respondWith("GET", new RegExp("https://taginfo\\.openstreetmap\\.org/api/4/tag/wiki_page"), - [200, { "Content-Type": "application/json" }, - '[{"on_way":false,"lang":"en","on_area":true,"image":"File:Car park2.jpg"}]']); - server.respond(); - - expect(query(server.requests[0].url)).to.eql( - {key: "amenity", value: "some-non-existing-value"}); - expect(query(server.requests[1].url)).to.eql( - {key: "amenity"}); - expect(callback.firstCall).calledWith(null, [], true); - expect(callback.firstCall.returnValue).to.eql(false); - expect(callback.secondCall.calledWith(null, - [{"on_way":false,"lang":"en","on_area":true,"image":"File:Car park2.jpg"}])); - expect(callback.secondCall.returnValue).to.eql(null); - }); }); + });