From cd9388c6b8f293acfc11c41c532e323ebe59e4ac Mon Sep 17 00:00:00 2001 From: missinglink Date: Tue, 9 Apr 2019 18:25:20 +0200 Subject: [PATCH 1/2] feat(categories): enable filter for autocomplete, show categories in geojson even when categories param is blank --- query/autocomplete.js | 6 ++ query/search.js | 2 +- query/search_original.js | 2 +- sanitizer/_categories.js | 24 ++----- .../autocomplete_with_category_filtering.js | 66 +++++++++++++++++++ test/unit/query/autocomplete.js | 19 ++++++ test/unit/sanitizer/_categories.js | 40 +++++------ 7 files changed, 120 insertions(+), 39 deletions(-) create mode 100644 test/unit/fixture/autocomplete_with_category_filtering.js diff --git a/query/autocomplete.js b/query/autocomplete.js index 228689823..65fe222fd 100644 --- a/query/autocomplete.js +++ b/query/autocomplete.js @@ -54,6 +54,7 @@ query.filter( peliasQuery.view.sources ); query.filter( peliasQuery.view.layers ); query.filter( peliasQuery.view.boundary_rect ); query.filter( peliasQuery.view.boundary_country ); +query.filter( peliasQuery.view.categories ); query.filter( peliasQuery.view.boundary_gid ); // -------------------------------- @@ -135,6 +136,11 @@ function generateQuery( clean ){ }); } + // categories + if (clean.categories && clean.categories.length) { + vs.var('input:categories', clean.categories); + } + // run the address parser if( clean.parsed_text ){ textParser( clean, vs ); diff --git a/query/search.js b/query/search.js index 5bd7811db..65a5aaa30 100644 --- a/query/search.js +++ b/query/search.js @@ -48,7 +48,7 @@ function generateQuery( clean ){ } // categories - if (clean.categories) { + if (clean.categories && clean.categories.length) { vs.var('input:categories', clean.categories); } diff --git a/query/search_original.js b/query/search_original.js index 99b98081c..b250a7f40 100644 --- a/query/search_original.js +++ b/query/search_original.js @@ -75,7 +75,7 @@ function generateQuery( clean ){ } // categories - if (clean.categories) { + if (clean.categories && clean.categories.length) { vs.var('input:categories', clean.categories); } diff --git a/sanitizer/_categories.js b/sanitizer/_categories.js index b964d1e7d..97e9dccb2 100644 --- a/sanitizer/_categories.js +++ b/sanitizer/_categories.js @@ -1,29 +1,22 @@ var check = require('check-types'); var categoryTaxonomy = require('pelias-categories'); -var ERRORS = { - empty: 'Categories parameter cannot be left blank. See documentation of service for valid options.', - invalid: 'Invalid categories parameter value(s). See documentation of service for valid options.' +var WARNINGS = { + empty: 'Categories parameter left blank, showing results from all categories.' }; // validate inputs, convert types and apply defaults -function _sanitize( raw, clean, categories ) { - +function _sanitize (raw, clean, categories) { categories = categories || categoryTaxonomy; // error & warning messages - var messages = {errors: [], warnings: []}; + var messages = { errors: [], warnings: [] }; // it's not a required parameter, so if it's not provided just move on if (!raw.hasOwnProperty('categories')) { return messages; } - if (!check.nonEmptyString(raw.categories)) { - messages.errors.push(ERRORS.empty); - return messages; - } - // if categories string has been set // map input categories to valid format try { @@ -39,17 +32,14 @@ function _sanitize( raw, clean, categories ) { }); } catch (err) { // remove everything from the list if there was any error - delete clean.categories; - } - - if (check.undefined(clean.categories) || check.emptyArray(clean.categories)) { - messages.errors.push(ERRORS.invalid); + messages.warnings.push(WARNINGS.empty); + clean.categories = []; } return messages; } -function _expected() { +function _expected () { return [{ name: 'categories' }]; } // export function diff --git a/test/unit/fixture/autocomplete_with_category_filtering.js b/test/unit/fixture/autocomplete_with_category_filtering.js new file mode 100644 index 000000000..577e32d95 --- /dev/null +++ b/test/unit/fixture/autocomplete_with_category_filtering.js @@ -0,0 +1,66 @@ +module.exports = { + 'query': { + 'bool': { + 'must': [{ + 'constant_score': { + 'query': { + 'match': { + 'name.default': { + 'analyzer': 'peliasQueryPartialToken', + 'cutoff_frequency': 0.01, + 'boost': 100, + 'query': 'test', + 'type': 'phrase', + 'operator': 'and', + 'slop': 3 + } + } + } + } + }], + 'should': [{ + 'function_score': { + 'query': { + 'match_all': {} + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + }, { + 'function_score': { + 'query': { + 'match_all': {} + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 3 + }] + } + }], + 'filter': [{ + 'terms': { + 'category': ['retail', 'food'] + } + }] + } + }, + 'sort': ['_score'], + 'size': 20, + 'track_scores': true +}; diff --git a/test/unit/query/autocomplete.js b/test/unit/query/autocomplete.js index 46384fdbf..baab33f28 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -235,6 +235,25 @@ module.exports.tests.query = function(test, common) { t.end(); }); + test('valid categories filter', function (t) { + var clean = { + text: 'test', + tokens: ['test'], + tokens_complete: [], + tokens_incomplete: ['test'], + categories: ['retail', 'food'] + }; + + var query = generate(clean); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/autocomplete_with_category_filtering'); + + t.deepEqual(compiled.type, 'autocomplete', 'query type set'); + t.deepEqual(compiled.body, expected, 'valid autocomplete query with category filtering'); + t.end(); + }); + test('single character street address', function(t) { var query = generate({ text: 'k road, laird', diff --git a/test/unit/sanitizer/_categories.js b/test/unit/sanitizer/_categories.js index 0c7dbe599..20ddb2beb 100644 --- a/test/unit/sanitizer/_categories.js +++ b/test/unit/sanitizer/_categories.js @@ -25,14 +25,14 @@ module.exports.tests.no_categories = function(test, common) { clean: { } }; - var expected_error = 'Categories parameter cannot be left blank. See documentation of service for valid options.'; + var expected_warning = 'Categories parameter left blank, showing results from all categories.'; var messages = sanitizer.sanitize(req.query, req.clean); - t.equal(req.clean.categories, undefined, 'no categories should be defined'); - t.deepEqual(messages.errors.length, 1, 'error returned'); - t.deepEqual(messages.errors[0], expected_error, 'error returned'); - t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.deepEqual(req.clean.categories, [], 'empty categories array should be defined'); + t.deepEqual(messages.errors, [], 'no errors returned'); + t.deepEqual(messages.warnings.length, 1, 'warning returned'); + t.deepEqual(messages.warnings[0], expected_warning, 'warning returned'); t.end(); }); @@ -44,14 +44,14 @@ module.exports.tests.no_categories = function(test, common) { clean: { } }; - var expected_error = 'Invalid categories parameter value(s). See documentation of service for valid options.'; + var expected_warning = 'Categories parameter left blank, showing results from all categories.'; var messages = sanitizer.sanitize(req.query, req.clean); - t.equal(req.clean.categories, undefined, 'no categories should be defined'); - t.deepEqual(messages.errors.length, 1, 'error returned'); - t.deepEqual(messages.errors[0], expected_error, 'error returned'); - t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.deepEqual(req.clean.categories, [], 'empty categories array should be defined'); + t.deepEqual(messages.errors, [], 'no errors returned'); + t.deepEqual(messages.warnings.length, 1, 'warning returned'); + t.deepEqual(messages.warnings[0], expected_warning, 'warning returned'); t.end(); }); }; @@ -124,16 +124,16 @@ module.exports.tests.invalid_categories = function(test, common) { clean: { } }; var expected_messages = { - errors: [ - 'Invalid categories parameter value(s). See documentation of service for valid options.' - ], - warnings: [] + errors: [], + warnings: [ + 'Categories parameter left blank, showing results from all categories.' + ] }; var messages = sanitizer.sanitize(req.query, req.clean, validCategories); t.deepEqual(messages, expected_messages, 'error with message returned'); - t.equal(req.clean.categories, undefined, 'clean.categories should remain empty'); + t.deepEqual(req.clean.categories, [], 'empty categories array should be defined'); t.end(); }); @@ -145,16 +145,16 @@ module.exports.tests.invalid_categories = function(test, common) { clean: { } }; var expected_messages = { - errors: [ - 'Invalid categories parameter value(s). See documentation of service for valid options.' - ], - warnings: [] + errors: [], + warnings: [ + 'Categories parameter left blank, showing results from all categories.' + ] }; var messages = sanitizer.sanitize(req.query, req.clean, validCategories); t.deepEqual(messages, expected_messages, 'error with message returned'); - t.equal(req.clean.categories, undefined, 'clean.categories should remain empty'); + t.deepEqual(req.clean.categories, [], 'empty categories array should be defined'); t.end(); }); From 6ee479c0a413327e449fbaa2280af2d00a49caf2 Mon Sep 17 00:00:00 2001 From: missinglink Date: Wed, 10 Apr 2019 11:02:15 +0200 Subject: [PATCH 2/2] feat(categories): gracefully handle a mixture of valid and invalid categories --- sanitizer/_categories.js | 27 +++++++++++++-------------- test/unit/sanitizer/_categories.js | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/sanitizer/_categories.js b/sanitizer/_categories.js index 97e9dccb2..a7671d374 100644 --- a/sanitizer/_categories.js +++ b/sanitizer/_categories.js @@ -19,21 +19,20 @@ function _sanitize (raw, clean, categories) { // if categories string has been set // map input categories to valid format - try { - clean.categories = raw.categories.split(',') - .map(function (cat) { - return cat.toLowerCase().trim(); // lowercase inputs - }) - .filter(function (cat) { - if (check.nonEmptyString(cat) && categories.isValidCategory(cat)) { - return true; - } - throw new Error('Empty string value'); - }); - } catch (err) { - // remove everything from the list if there was any error + clean.categories = raw.categories.split(',') + .map(cat => { + return cat.toLowerCase().trim(); // lowercase inputs + }) + .filter(cat => { + if (check.nonEmptyString(cat) && categories.isValidCategory(cat)) { + return true; + } + return false; + }); + + if( !clean.categories.length ){ + // display a warning that the input was empty messages.warnings.push(WARNINGS.empty); - clean.categories = []; } return messages; diff --git a/test/unit/sanitizer/_categories.js b/test/unit/sanitizer/_categories.js index 20ddb2beb..655a7b0e0 100644 --- a/test/unit/sanitizer/_categories.js +++ b/test/unit/sanitizer/_categories.js @@ -54,6 +54,21 @@ module.exports.tests.no_categories = function(test, common) { t.deepEqual(messages.warnings[0], expected_warning, 'warning returned'); t.end(); }); + + test('categories is mix of valid categories and empty strings', function (t) { + var req = { + query: { + categories: ',food,' + }, + clean: {} + }; + + var messages = sanitizer.sanitize(req.query, req.clean); + t.deepEqual(req.clean.categories, ['food'], 'junk trimmed'); + t.deepEqual(messages.errors, [], 'no errors returned'); + t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.end(); + }); }; module.exports.tests.valid_categories = function(test, common) {