From 515d4aeb941e5b57d084df95a2efcb8a8ceb230c Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 1 Mar 2016 10:34:06 -0500 Subject: [PATCH 01/10] extracted `getSchema` helper method --- helper/labelGenerator.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index ffc5bf3f..bbe688fe 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -4,15 +4,10 @@ var _ = require('lodash'), schemas = require('./labelSchema.json'); module.exports = function( record ){ + var schema = getSchema(record.country_a); var labelParts = [ record.name.default ]; - var schema = schemas.default; - - if (record.country_a && record.country_a.length && schemas[record.country_a]) { - schema = schemas[record.country_a]; - } - var buildOutput = function(parts, schemaArr, record) { for (var i=0; i Date: Tue, 1 Mar 2016 10:47:38 -0500 Subject: [PATCH 02/10] broke country-specific label tests out to own file for easier testing --- helper/labelGenerator.js | 6 +- test/unit/helper/labelGenerator_GBR.js | 70 ++++++ test/unit/helper/labelGenerator_SGP.js | 51 +++++ test/unit/helper/labelGenerator_SWE.js | 53 +++++ test/unit/helper/labelGenerator_USA.js | 115 ++++++++++ ...Generator.js => labelGenerator_default.js} | 201 ------------------ test/unit/run.js | 6 +- 7 files changed, 296 insertions(+), 206 deletions(-) create mode 100644 test/unit/helper/labelGenerator_GBR.js create mode 100644 test/unit/helper/labelGenerator_SGP.js create mode 100644 test/unit/helper/labelGenerator_SWE.js create mode 100644 test/unit/helper/labelGenerator_USA.js rename test/unit/helper/{labelGenerator.js => labelGenerator_default.js} (55%) diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index bbe688fe..34e353cd 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -23,10 +23,8 @@ module.exports = function( record ){ labelParts = buildOutput(labelParts, schema[key], record); } - // de-dupe outputs - labelParts = _.uniq( labelParts ); - - return labelParts.join(', ').trim(); + // de-dupe, join, trim + return _.uniq( labelParts ).join(', ').trim(); }; function getSchema(country_a) { diff --git a/test/unit/helper/labelGenerator_GBR.js b/test/unit/helper/labelGenerator_GBR.js new file mode 100644 index 00000000..38ab3f91 --- /dev/null +++ b/test/unit/helper/labelGenerator_GBR.js @@ -0,0 +1,70 @@ + +var generator = require('../../../helper/labelGenerator'); + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('interface', function(t) { + t.equal(typeof generator, 'function', 'valid function'); + t.end(); + }); +}; + +// GBR street address +module.exports.tests.one_main_street_uk = function(test, common) { + test('one main street uk', function(t) { + var doc = { + 'name': { 'default': '1 Main St' }, + 'housenumber': '1', + 'street': 'Main St', + 'postalcode': 'BT77 0BG', + 'country_a': 'GBR', + 'country': 'United Kingdom', + 'region': 'Dungannon' + }; + t.equal(generator(doc),'1 Main St, Dungannon, United Kingdom'); + t.end(); + }); +}; + +// GBR venue +module.exports.tests.hackney_city_farm = function(test, common) { + test('hackney city farm', function(t) { + var doc = { + 'name': { 'default': 'Hackney City Farm' }, + 'country_a': 'GBR', + 'country': 'United Kingdom', + 'region': 'Hackney', + 'county': 'Greater London', + 'locality': 'London', + 'neighbourhood': 'Haggerston' + }; + t.equal(generator(doc),'Hackney City Farm, Haggerston, Greater London'); + t.end(); + }); +}; + +// GBR country +module.exports.tests.wales = function(test, common) { + test('wales', function(t) { + var doc = { + 'name': { 'default': 'Wales' }, + 'country_a': 'GBR', + 'country': 'United Kingdom', + 'region': 'Wales' + }; + t.equal(generator(doc),'Wales, United Kingdom'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('label generator (GBR): ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/helper/labelGenerator_SGP.js b/test/unit/helper/labelGenerator_SGP.js new file mode 100644 index 00000000..92aca117 --- /dev/null +++ b/test/unit/helper/labelGenerator_SGP.js @@ -0,0 +1,51 @@ + +var generator = require('../../../helper/labelGenerator'); + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('interface', function(t) { + t.equal(typeof generator, 'function', 'valid function'); + t.end(); + }); +}; + +// SGP region +module.exports.tests.north_west_singapore = function(test, common) { + test('north west singapore', function(t) { + var doc = { + 'name': { 'default': 'North West' }, + 'country_a': 'SGP', + 'country': 'Singapore', + 'region': 'North West' + }; + t.equal(generator(doc),'North West, Singapore'); + t.end(); + }); +}; + +// SGP venue +module.exports.tests.singapore_mcdonalds = function(test, common) { + test('singapore_mcdonalds', function(t) { + var doc = { + 'name': { 'default': 'McDonald\'s' }, + 'country_a': 'SGP', + 'country': 'Singapore', + 'region': 'Central Singapore', + 'locality': 'Singapore' + }; + t.equal(generator(doc),'McDonald\'s, Central Singapore, Singapore'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('label generator: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/helper/labelGenerator_SWE.js b/test/unit/helper/labelGenerator_SWE.js new file mode 100644 index 00000000..594ea0e5 --- /dev/null +++ b/test/unit/helper/labelGenerator_SWE.js @@ -0,0 +1,53 @@ + +var generator = require('../../../helper/labelGenerator'); + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('interface', function(t) { + t.equal(typeof generator, 'function', 'valid function'); + t.end(); + }); +}; + +// SWE city +module.exports.tests.skane1 = function(test, common) { + test('skåne 1', function(t) { + var doc = { + 'name': { 'default': 'Malmö' }, + 'country_a': 'SWE', + 'country': 'Sweden', + 'region': 'Skåne', + 'county': 'Malmö' + }; + t.equal(generator(doc),'Malmö, Skåne, Sweden'); + t.end(); + }); +}; + +// SWE city +module.exports.tests.skane2 = function(test, common) { + test('skåne 2', function(t) { + var doc = { + 'name': { 'default': 'Malmö' }, + 'country_a': 'SWE', + 'country': 'Sweden', + 'region': 'Skåne', + 'county': 'Malmö', + 'locality': 'Malmö' + }; + t.equal(generator(doc),'Malmö, Skåne, Sweden'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('label generator: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js new file mode 100644 index 00000000..93715839 --- /dev/null +++ b/test/unit/helper/labelGenerator_USA.js @@ -0,0 +1,115 @@ + +var generator = require('../../../helper/labelGenerator'); + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('interface', function(t) { + t.equal(typeof generator, 'function', 'valid function'); + t.end(); + }); +}; + +// major USA city +module.exports.tests.san_francisco = function(test, common) { + test('san francisco', function(t) { + var doc = { + 'name': { 'default': 'San Francisco' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'California', + 'region_a': 'CA', + 'county': 'San Francisco County', + 'locality': 'San Francisco' + }; + t.equal(generator(doc),'San Francisco, San Francisco County, CA'); + t.end(); + }); +}; + +// USA venue +module.exports.tests.nyc_office = function(test, common) { + test('30 West 26th Street', function(t) { + var doc = { + 'name': { 'default': '30 West 26th Street' }, + 'housenumber': '30', + 'street': 'West 26th Street', + 'postalcode': '10010', + 'country_a': 'USA', + 'country': 'United States', + 'region': 'New York', + 'region_a': 'NY', + 'county': 'New York County', + 'localadmin': 'Manhattan', + 'locality': 'New York', + 'neighbourhood': 'Flatiron District' + }; + t.equal(generator(doc),'30 West 26th Street, Manhattan, NY'); + t.end(); + }); +}; + +// USA NYC eatery +module.exports.tests.nyc_bakery = function(test, common) { + test('New York Bakery', function(t) { + var doc = { + 'name': { 'default': 'New York Bakery' }, + 'housenumber': '51 W', + 'street': '29th', + 'country_a': 'USA', + 'country': 'United States', + 'region': 'New York', + 'region_a': 'NY', + 'county': 'New York County', + 'localadmin': 'Manhattan', + 'locality': 'New York', + 'neighbourhood': 'Koreatown' + }; + t.equal(generator(doc),'New York Bakery, Manhattan, NY'); + t.end(); + }); +}; + +// USA SFC building +module.exports.tests.ferry_building = function(test, common) { + test('Ferry Building', function(t) { + var doc = { + 'name': { 'default': 'Ferry Building' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'California', + 'region_a': 'CA', + 'county': 'San Francisco County', + 'locality': 'San Francisco', + 'neighbourhood': 'Financial District' + }; + t.equal(generator(doc),'Ferry Building, San Francisco, CA'); + t.end(); + }); +}; + +// USA state +module.exports.tests.california = function(test, common) { + test('california', function(t) { + var doc = { + 'name': { 'default': 'California' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'California', + 'region_a': 'CA' + }; + t.equal(generator(doc),'California, CA'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('label generator: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/helper/labelGenerator.js b/test/unit/helper/labelGenerator_default.js similarity index 55% rename from test/unit/helper/labelGenerator.js rename to test/unit/helper/labelGenerator_default.js index 9a382397..dc8ad02d 100644 --- a/test/unit/helper/labelGenerator.js +++ b/test/unit/helper/labelGenerator_default.js @@ -10,84 +10,6 @@ module.exports.tests.interface = function(test, common) { }); }; -// major USA city -module.exports.tests.san_francisco = function(test, common) { - test('san francisco', function(t) { - var doc = { - 'name': { 'default': 'San Francisco' }, - 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'county': 'San Francisco County', - 'locality': 'San Francisco' - }; - t.equal(generator(doc),'San Francisco, San Francisco County, CA'); - t.end(); - }); -}; - -// USA venue -module.exports.tests.nyc_office = function(test, common) { - test('30 West 26th Street', function(t) { - var doc = { - 'name': { 'default': '30 West 26th Street' }, - 'housenumber': '30', - 'street': 'West 26th Street', - 'postalcode': '10010', - 'country_a': 'USA', - 'country': 'United States', - 'region': 'New York', - 'region_a': 'NY', - 'county': 'New York County', - 'localadmin': 'Manhattan', - 'locality': 'New York', - 'neighbourhood': 'Flatiron District' - }; - t.equal(generator(doc),'30 West 26th Street, Manhattan, NY'); - t.end(); - }); -}; - -// USA NYC eatery -module.exports.tests.nyc_bakery = function(test, common) { - test('New York Bakery', function(t) { - var doc = { - 'name': { 'default': 'New York Bakery' }, - 'housenumber': '51 W', - 'street': '29th', - 'country_a': 'USA', - 'country': 'United States', - 'region': 'New York', - 'region_a': 'NY', - 'county': 'New York County', - 'localadmin': 'Manhattan', - 'locality': 'New York', - 'neighbourhood': 'Koreatown' - }; - t.equal(generator(doc),'New York Bakery, Manhattan, NY'); - t.end(); - }); -}; - -// USA SFC building -module.exports.tests.ferry_building = function(test, common) { - test('Ferry Building', function(t) { - var doc = { - 'name': { 'default': 'Ferry Building' }, - 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'county': 'San Francisco County', - 'locality': 'San Francisco', - 'neighbourhood': 'Financial District' - }; - t.equal(generator(doc),'Ferry Building, San Francisco, CA'); - t.end(); - }); -}; - // AUS state module.exports.tests.new_south_wales = function(test, common) { test('new south wales', function(t) { @@ -102,21 +24,6 @@ module.exports.tests.new_south_wales = function(test, common) { }); }; -// USA state -module.exports.tests.california = function(test, common) { - test('california', function(t) { - var doc = { - 'name': { 'default': 'California' }, - 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA' - }; - t.equal(generator(doc),'California, CA'); - t.end(); - }); -}; - // IND state module.exports.tests.west_bengal = function(test, common) { test('west bengal', function(t) { @@ -194,20 +101,6 @@ module.exports.tests.wellington_victoria = function(test, common) { }); }; -// SGP region -module.exports.tests.north_west_singapore = function(test, common) { - test('north west singapore', function(t) { - var doc = { - 'name': { 'default': 'North West' }, - 'country_a': 'SGP', - 'country': 'Singapore', - 'region': 'North West' - }; - t.equal(generator(doc),'North West, Singapore'); - t.end(); - }); -}; - // IRQ region module.exports.tests.arbil = function(test, common) { test('arbil', function(t) { @@ -236,71 +129,6 @@ module.exports.tests.madrid = function(test, common) { }); }; -// SWE city -module.exports.tests.skane1 = function(test, common) { - test('skåne 1', function(t) { - var doc = { - 'name': { 'default': 'Malmö' }, - 'country_a': 'SWE', - 'country': 'Sweden', - 'region': 'Skåne', - 'county': 'Malmö' - }; - t.equal(generator(doc),'Malmö, Skåne, Sweden'); - t.end(); - }); -}; - -// SWE city -module.exports.tests.skane2 = function(test, common) { - test('skåne 2', function(t) { - var doc = { - 'name': { 'default': 'Malmö' }, - 'country_a': 'SWE', - 'country': 'Sweden', - 'region': 'Skåne', - 'county': 'Malmö', - 'locality': 'Malmö' - }; - t.equal(generator(doc),'Malmö, Skåne, Sweden'); - t.end(); - }); -}; - -// GBR street address -module.exports.tests.one_main_street_uk = function(test, common) { - test('one main street uk', function(t) { - var doc = { - 'name': { 'default': '1 Main St' }, - 'housenumber': '1', - 'street': 'Main St', - 'postalcode': 'BT77 0BG', - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'region': 'Dungannon' - }; - t.equal(generator(doc),'1 Main St, Dungannon, United Kingdom'); - t.end(); - }); -}; - -// GBR venue -module.exports.tests.hackney_city_farm = function(test, common) { - test('hackney city farm', function(t) { - var doc = { - 'name': { 'default': 'Hackney City Farm' }, - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'region': 'Hackney', - 'county': 'Greater London', - 'locality': 'London', - 'neighbourhood': 'Haggerston' - }; - t.equal(generator(doc),'Hackney City Farm, Haggerston, Greater London'); - t.end(); - }); -}; - // DEU street address module.exports.tests.one_grolmanstrasse = function(test, common) { test('one grolmanstrasse', function(t) { @@ -334,20 +162,6 @@ module.exports.tests.new_zealand = function(test, common) { }); }; -// GBR country -module.exports.tests.wales = function(test, common) { - test('wales', function(t) { - var doc = { - 'name': { 'default': 'Wales' }, - 'country_a': 'GBR', - 'country': 'United Kingdom', - 'region': 'Wales' - }; - t.equal(generator(doc),'Wales, United Kingdom'); - t.end(); - }); -}; - // IRL country module.exports.tests.republic_of_ireland = function(test, common) { test('northern ireland', function(t) { @@ -362,21 +176,6 @@ module.exports.tests.republic_of_ireland = function(test, common) { }); }; -// SGP venue -module.exports.tests.singapore_mcdonalds = function(test, common) { - test('singapore_mcdonalds', function(t) { - var doc = { - 'name': { 'default': 'McDonald\'s' }, - 'country_a': 'SGP', - 'country': 'Singapore', - 'region': 'Central Singapore', - 'locality': 'Singapore' - }; - t.equal(generator(doc),'McDonald\'s, Central Singapore, Singapore'); - t.end(); - }); -}; - // THA province module.exports.tests.krabi_province = function(test, common) { test('Krabi Provence', function(t) { diff --git a/test/unit/run.js b/test/unit/run.js index 0510cda6..c600e0ad 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -7,7 +7,11 @@ var tests = [ require('./controller/place'), require('./controller/search'), require('./helper/geojsonify'), - require('./helper/labelGenerator'), + require('./helper/labelGenerator_default'), + require('./helper/labelGenerator_GBR'), + require('./helper/labelGenerator_SGP'), + require('./helper/labelGenerator_SWE'), + require('./helper/labelGenerator_USA'), require('./helper/labelSchema'), require('./helper/text_parser'), require('./helper/type_mapping'), From 3e11e6687c1d9b2fa2334f9b60e9c555294ba0cd Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 1 Mar 2016 10:56:51 -0500 Subject: [PATCH 03/10] added `country_a` to label for all USA results --- helper/labelSchema.json | 3 ++- test/unit/helper/geojsonify.js | 2 +- test/unit/helper/labelGenerator_USA.js | 10 +++++----- test/unit/helper/labelSchema.js | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/helper/labelSchema.json b/helper/labelSchema.json index 4b555790..2c2859ea 100644 --- a/helper/labelSchema.json +++ b/helper/labelSchema.json @@ -1,7 +1,8 @@ { "USA": { "local": ["localadmin", "locality", "neighbourhood", "county"], - "regional": ["region_a", "region", "country"] + "regional": ["region_a", "region"], + "country": ["country_a"] }, "GBR": { "local": ["neighbourhood", "county", "localadmin", "locality", "region"], diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index a66421de..3c969302 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -199,7 +199,7 @@ module.exports.tests.search = function(test, common) { 'gid': 'osm:venue:34633854', 'layer': 'venue', 'source': 'osm', - 'label': 'Empire State Building, Manhattan, NY', + 'label': 'Empire State Building, Manhattan, NY, USA', 'name': 'Empire State Building', 'country_a': 'USA', 'country': 'United States', diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js index 93715839..5512661d 100644 --- a/test/unit/helper/labelGenerator_USA.js +++ b/test/unit/helper/labelGenerator_USA.js @@ -22,7 +22,7 @@ module.exports.tests.san_francisco = function(test, common) { 'county': 'San Francisco County', 'locality': 'San Francisco' }; - t.equal(generator(doc),'San Francisco, San Francisco County, CA'); + t.equal(generator(doc),'San Francisco, San Francisco County, CA, USA'); t.end(); }); }; @@ -44,7 +44,7 @@ module.exports.tests.nyc_office = function(test, common) { 'locality': 'New York', 'neighbourhood': 'Flatiron District' }; - t.equal(generator(doc),'30 West 26th Street, Manhattan, NY'); + t.equal(generator(doc),'30 West 26th Street, Manhattan, NY, USA'); t.end(); }); }; @@ -65,7 +65,7 @@ module.exports.tests.nyc_bakery = function(test, common) { 'locality': 'New York', 'neighbourhood': 'Koreatown' }; - t.equal(generator(doc),'New York Bakery, Manhattan, NY'); + t.equal(generator(doc),'New York Bakery, Manhattan, NY, USA'); t.end(); }); }; @@ -83,7 +83,7 @@ module.exports.tests.ferry_building = function(test, common) { 'locality': 'San Francisco', 'neighbourhood': 'Financial District' }; - t.equal(generator(doc),'Ferry Building, San Francisco, CA'); + t.equal(generator(doc),'Ferry Building, San Francisco, CA, USA'); t.end(); }); }; @@ -98,7 +98,7 @@ module.exports.tests.california = function(test, common) { 'region': 'California', 'region_a': 'CA' }; - t.equal(generator(doc),'California, CA'); + t.equal(generator(doc),'California, CA, USA'); t.end(); }); }; diff --git a/test/unit/helper/labelSchema.js b/test/unit/helper/labelSchema.js index dcc3cdfa..442b7f70 100644 --- a/test/unit/helper/labelSchema.js +++ b/test/unit/helper/labelSchema.js @@ -13,7 +13,7 @@ module.exports.tests.interface = function(test, common) { }; module.exports.tests.valid = function(test, common) { - var valid_keys = ['localadmin', 'locality', 'neighbourhood', 'county', 'region_a', 'region', 'country']; + var valid_keys = ['localadmin', 'locality', 'neighbourhood', 'county', 'region_a', 'region', 'country', 'country_a']; var default_schema = { local: ['localadmin', 'locality', 'neighbourhood', 'county', 'region'], regional: ['country'] From 5456632cdfee211e89674376d23662155161263c Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 1 Mar 2016 13:47:03 -0500 Subject: [PATCH 04/10] Added flag to not include `default.name` in label when source=`geonames` or `whosonfirst` and layer=`region` This will stop, say, a search for "New Mexico" resulting in the label "New Mexico, NM, USA" --- helper/labelGenerator.js | 11 ++++++- test/unit/helper/labelGenerator_USA.js | 44 +++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index 34e353cd..3e4a4237 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -6,7 +6,7 @@ var _ = require('lodash'), module.exports = function( record ){ var schema = getSchema(record.country_a); - var labelParts = [ record.name.default ]; + var labelParts = getInitialLabel(record); var buildOutput = function(parts, schemaArr, record) { for (var i=0; i Date: Tue, 1 Mar 2016 15:57:58 -0500 Subject: [PATCH 05/10] added tests for USA local and regional fallbacks for label generation --- test/unit/helper/labelGenerator_USA.js | 181 ++++++++++++++++++------- 1 file changed, 130 insertions(+), 51 deletions(-) diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js index 5a7ac532..8d994481 100644 --- a/test/unit/helper/labelGenerator_USA.js +++ b/test/unit/helper/labelGenerator_USA.js @@ -10,6 +10,136 @@ module.exports.tests.interface = function(test, common) { }); }; +module.exports.tests.localadmin = function(test, common) { + test('localadmin should trump locality, neighbourhood, and county', function(t) { + var doc = { + 'name': { 'default': 'Default Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'county': 'County Name', + 'localadmin': 'LocalAdmin Name', + 'locality': 'Locality Name', + 'neighbourhood': 'Neighbourhood Name' + }; + t.equal(generator(doc),'Default Name, LocalAdmin Name, Region Abbr, USA'); + t.end(); + }); +}; + +module.exports.tests.locality = function(test, common) { + test('locality should trump neighbourhood and county when localadmin not available', function(t) { + var doc = { + 'name': { 'default': 'Default Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'county': 'County Name', + 'locality': 'Locality Name', + 'neighbourhood': 'Neighbourhood Name' + }; + t.equal(generator(doc),'Default Name, Locality Name, Region Abbr, USA'); + t.end(); + }); +}; + +module.exports.tests.neighbourhood = function(test, common) { + test('neighbourhood should trump county when neither localadmin nor locality', function(t) { + var doc = { + 'name': { 'default': 'Default Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'county': 'County Name', + 'neighbourhood': 'Neighbourhood Name' + }; + t.equal(generator(doc),'Default Name, Neighbourhood Name, Region Abbr, USA'); + t.end(); + }); +}; + +module.exports.tests.county = function(test, common) { + test('county should be used when localadmin, locality, and neighbourhood are not available', function(t) { + var doc = { + 'name': { 'default': 'Default Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'county': 'County Name' + }; + t.equal(generator(doc),'Default Name, County Name, Region Abbr, USA'); + t.end(); + }); +}; + +module.exports.tests.region = function(test, common) { + test('region should be used when region_a is not available', function(t) { + var doc = { + 'name': { 'default': 'Default Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name' + }; + t.equal(generator(doc),'Default Name, Region Name, USA'); + t.end(); + }); +}; + +// USA geonames state +module.exports.tests.region_geonames = function(test, common) { + test('default name should not be prepended when source=geonames and layer=region', function(t) { + var doc = { + 'name': { 'default': 'Region Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'source': 'geonames', + 'layer': 'region' + }; + t.equal(generator(doc),'Region Abbr, USA'); + t.end(); + }); +}; + +// USA whosonfirst state +module.exports.tests.region_whosonfirst = function(test, common) { + test('default name should not be prepended when source=whosonfirst and layer=region', function(t) { + var doc = { + 'name': { 'default': 'California' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'source': 'whosonfirst', + 'layer': 'region' + }; + t.equal(generator(doc),'Region Abbr, USA'); + t.end(); + }); +}; + +// USA non-geonames/whosonfirst state +module.exports.tests.region_other_source = function(test, common) { + test('default name should not be prepended when source=whosonfirst and layer=region', function(t) { + var doc = { + 'name': { 'default': 'Region Name' }, + 'country_a': 'USA', + 'country': 'United States', + 'region': 'Region Name', + 'region_a': 'Region Abbr', + 'source': 'not geonames or whosonfirst', + 'layer': 'region' + }; + t.equal(generator(doc),'Region Name, Region Abbr, USA'); + t.end(); + }); +}; + // major USA city module.exports.tests.san_francisco = function(test, common) { test('san francisco', function(t) { @@ -88,57 +218,6 @@ module.exports.tests.ferry_building = function(test, common) { }); }; -// USA geonames state -module.exports.tests.california_geonames = function(test, common) { - test('default name should not be prepended when source=geonames and layer=region', function(t) { - var doc = { - 'name': { 'default': 'California' }, - 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'source': 'geonames', - 'layer': 'region' - }; - t.equal(generator(doc),'CA, USA'); - t.end(); - }); -}; - -// USA whosonfirst state -module.exports.tests.california_whosonfirst = function(test, common) { - test('default name should not be prepended when source=whosonfirst and layer=region', function(t) { - var doc = { - 'name': { 'default': 'California' }, - 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'source': 'whosonfirst', - 'layer': 'region' - }; - t.equal(generator(doc),'CA, USA'); - t.end(); - }); -}; - -// USA non-geonames/whosonfirst state -module.exports.tests.california_other_source = function(test, common) { - test('default name should not be prepended when source=whosonfirst and layer=region', function(t) { - var doc = { - 'name': { 'default': 'California' }, - 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'source': 'not geonames or whosonfirst', - 'layer': 'region' - }; - t.equal(generator(doc),'California, CA, USA'); - t.end(); - }); -}; - module.exports.all = function (tape, common) { function test(name, testFunction) { From 7b5b9dee61b378ec1e99637ead4956e1eff02d3b Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 1 Mar 2016 17:04:43 -0500 Subject: [PATCH 06/10] added comments illustrating the subtleties of label generation --- helper/labelGenerator.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index 3e4a4237..c9eff405 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -24,7 +24,28 @@ module.exports = function( record ){ } // de-dupe, join, trim + // NOTE: while it may seem odd to call `uniq` on the list of label parts, + // the effect is quite subtle. Take, for instance, a result for "Lancaster, PA" + // the pseudo-object is: + // { + // 'name': 'Lancaster', + // 'locality': 'Lancaster', + // 'region_a': 'PA', + // 'country_a': 'USA' + // } + // + // the code up to this point generates the label: + // `Lancaster, Lancaster, PA, USA` + // + // then the `unique` call reduces this to: + // `Lancaster, PA, USA` + // + // this code doesn't have the same effect in the case of a venue or address + // where the `name` field would contain the address or name of a point-of-interest + // + // Also see https://github.com/pelias/api/issues/429 for other ways that this is bad return _.uniq( labelParts ).join(', ').trim(); + }; function getSchema(country_a) { From 08f414b3d57f92d6d931b593f7560873edb0efe1 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 1 Mar 2016 20:31:51 -0500 Subject: [PATCH 07/10] fixed encoding error that came from master --- test/unit/helper/labelGenerator_SWE.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/helper/labelGenerator_SWE.js b/test/unit/helper/labelGenerator_SWE.js index 594ea0e5..edefc93a 100644 --- a/test/unit/helper/labelGenerator_SWE.js +++ b/test/unit/helper/labelGenerator_SWE.js @@ -34,7 +34,7 @@ module.exports.tests.skane2 = function(test, common) { 'country': 'Sweden', 'region': 'Skåne', 'county': 'Malmö', - 'locality': 'Malmö' + 'locality': 'Malmö' }; t.equal(generator(doc),'Malmö, Skåne, Sweden'); t.end(); From 431fe44f05a0d4d6a059ea1d1a6fc36c3e937b83 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 2 Mar 2016 10:05:09 -0500 Subject: [PATCH 08/10] converted labelSchema to .js with module.exports this is in preparation to introduce functions into the schema instead of strings --- helper/labelGenerator.js | 5 +++-- helper/labelSchema.js | 23 +++++++++++++++++++++++ helper/labelSchema.json | 23 ----------------------- test/unit/helper/labelSchema.js | 2 +- 4 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 helper/labelSchema.js delete mode 100644 helper/labelSchema.json diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index c9eff405..31a04414 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -1,7 +1,7 @@ var _ = require('lodash'), check = require('check-types'), - schemas = require('./labelSchema.json'); + schemas = require('./labelSchema'); module.exports = function( record ){ var schema = getSchema(record.country_a); @@ -23,7 +23,6 @@ module.exports = function( record ){ labelParts = buildOutput(labelParts, schema[key], record); } - // de-dupe, join, trim // NOTE: while it may seem odd to call `uniq` on the list of label parts, // the effect is quite subtle. Take, for instance, a result for "Lancaster, PA" // the pseudo-object is: @@ -44,6 +43,8 @@ module.exports = function( record ){ // where the `name` field would contain the address or name of a point-of-interest // // Also see https://github.com/pelias/api/issues/429 for other ways that this is bad + // + // de-dupe, join, trim return _.uniq( labelParts ).join(', ').trim(); }; diff --git a/helper/labelSchema.js b/helper/labelSchema.js new file mode 100644 index 00000000..d37ffc15 --- /dev/null +++ b/helper/labelSchema.js @@ -0,0 +1,23 @@ +module.exports = { + 'USA': { + 'local': ['localadmin', 'locality', 'neighbourhood', 'county'], + 'regional': ['region_a', 'region'], + 'country': ['country_a'] + }, + 'GBR': { + 'local': ['neighbourhood', 'county', 'localadmin', 'locality', 'region'], + 'regional': ['county','country','region'] + }, + 'SGP': { + 'local': ['neighbourhood', 'region', 'county', 'localadmin', 'locality'], + 'regional': ['county','country','region'] + }, + 'SWE': { + 'local': ['neighbourhood', 'region', 'county', 'localadmin', 'locality'], + 'regional': ['country'] + }, + 'default': { + 'local': ['localadmin', 'locality', 'neighbourhood', 'county', 'region'], + 'regional': ['country'] + } +}; diff --git a/helper/labelSchema.json b/helper/labelSchema.json deleted file mode 100644 index 2c2859ea..00000000 --- a/helper/labelSchema.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "USA": { - "local": ["localadmin", "locality", "neighbourhood", "county"], - "regional": ["region_a", "region"], - "country": ["country_a"] - }, - "GBR": { - "local": ["neighbourhood", "county", "localadmin", "locality", "region"], - "regional": ["county","country","region"] - }, - "SGP": { - "local": ["neighbourhood", "region", "county", "localadmin", "locality"], - "regional": ["county","country","region"] - }, - "SWE": { - "local": ["neighbourhood", "region", "county", "localadmin", "locality"], - "regional": ["country"] - }, - "default": { - "local": ["localadmin", "locality", "neighbourhood", "county", "region"], - "regional": ["country"] - } -} diff --git a/test/unit/helper/labelSchema.js b/test/unit/helper/labelSchema.js index 442b7f70..cb2308cd 100644 --- a/test/unit/helper/labelSchema.js +++ b/test/unit/helper/labelSchema.js @@ -1,5 +1,5 @@ -var schemas = require('../../../helper/labelSchema.json'); +var schemas = require('../../../helper/labelSchema'); var alpha3 = require('../mock/alpha3.json'); module.exports.tests = {}; From 0f6059c05a8a443376c8a39bca030e289733f304 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 3 Mar 2016 10:04:28 -0500 Subject: [PATCH 09/10] Refactored to use a function for looking up value for a label The reason for this switch is that special case logic is needed to determine the US state name to display. All other fields can be looked up by name. The burden of fallback functionality is now in `helper/labelSchema` so the burden of unit testing is now in `test/unit/helper/labelSchema.js` --- helper/labelGenerator.js | 17 +- helper/labelSchema.js | 65 ++- test/unit/helper/labelGenerator_USA.js | 13 +- test/unit/helper/labelSchema.js | 661 +++++++++++++++++++++++-- 4 files changed, 695 insertions(+), 61 deletions(-) diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index 31a04414..e772ceb9 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -1,6 +1,5 @@ var _ = require('lodash'), - check = require('check-types'), schemas = require('./labelSchema'); module.exports = function( record ){ @@ -8,19 +7,11 @@ module.exports = function( record ){ var labelParts = getInitialLabel(record); - var buildOutput = function(parts, schemaArr, record) { - for (var i=0; i Date: Thu, 3 Mar 2016 12:55:47 -0500 Subject: [PATCH 10/10] added tests for `default` schema --- test/unit/helper/labelSchema.js | 129 ++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/test/unit/helper/labelSchema.js b/test/unit/helper/labelSchema.js index 3d079b73..edbf2b77 100644 --- a/test/unit/helper/labelSchema.js +++ b/test/unit/helper/labelSchema.js @@ -645,6 +645,135 @@ module.exports.tests.swe = function(test, common) { }; +module.exports.tests.default = function(test, common) { + test('default.local should use localadmin value over locality, neighbourhood, county, region', function(t) { + var record = { + neighbourhood: 'neighbourhood value', + county: 'county value', + localadmin: 'localadmin value', + locality: 'locality value', + region: 'region value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.local; + + t.deepEqual(f(record, labelParts), ['initial value', 'localadmin value']); + t.end(); + + }); + + test('default.local should use locality value over neighbourhood, county, region', function(t) { + var record = { + neighbourhood: 'neighbourhood value', + county: 'county value', + locality: 'locality value', + region: 'region value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.local; + + t.deepEqual(f(record, labelParts), ['initial value', 'locality value']); + t.end(); + + }); + + test('default.local should use neighbourhood value over county, region', function(t) { + var record = { + neighbourhood: 'neighbourhood value', + county: 'county value', + region: 'region value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.local; + + t.deepEqual(f(record, labelParts), ['initial value', 'neighbourhood value']); + t.end(); + + }); + + test('default.local should use county value over region', function(t) { + var record = { + county: 'county value', + region: 'region value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.local; + + t.deepEqual(f(record, labelParts), ['initial value', 'county value']); + t.end(); + + }); + + test('default.local should use region value when nothing else is available', function(t) { + var record = { + region: 'region value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.local; + + t.deepEqual(f(record, labelParts), ['initial value', 'region value']); + t.end(); + + }); + + test('default.local should not append anything when none of neighbourhood, region, county, localadmin, ' + + 'locality are available', function(t) { + var record = {}; + + var labelParts = ['initial value']; + + var f = schemas.default.local; + + t.deepEqual(f(record, labelParts), ['initial value']); + t.end(); + + }); + + test('default.regional should use country over region, region_a, or country_a', function(t) { + var record = { + region: 'region value', + region_a: 'region_a value', + country: 'country value', + country_a: 'country_a value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.regional; + + t.deepEqual(f(record, labelParts), ['initial value', 'country value']); + t.end(); + + }); + + test('default.regional should not append any value if country is not available', function(t) { + var record = { + region: 'region value', + region_a: 'region_a value', + country_a: 'country_a value' + }; + + var labelParts = ['initial value']; + + var f = schemas.default.regional; + + t.deepEqual(f(record, labelParts), ['initial value']); + t.end(); + + }); + +}; + module.exports.all = function (tape, common) { function test(name, testFunction) {