From 41691c6ed32b1abf846b5a39cf5631d1a9488a20 Mon Sep 17 00:00:00 2001 From: missinglink Date: Tue, 17 Aug 2021 14:20:45 +0200 Subject: [PATCH] feat(change-language-same-source): only change name.default when translation is same source and source_id --- middleware/changeLanguage.js | 48 ++++++----- test/unit/middleware/changeLanguage.js | 111 +++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 19 deletions(-) diff --git a/middleware/changeLanguage.js b/middleware/changeLanguage.js index 90d45b99e..8bd81437d 100644 --- a/middleware/changeLanguage.js +++ b/middleware/changeLanguage.js @@ -80,10 +80,10 @@ function updateDocs( req, res, translations ){ var match = attr.match(/^(.*)_id$/); if( !match ){ continue; } - // adminKey is the property name without the '_id' - // eg. for 'country_id', adminKey would be 'country'. - var adminKey = match[1]; - var adminValues = doc.parent[adminKey]; + // adminLayer is the property name without the '_id' + // eg. for 'country_id', adminLayer would be 'country'. + var adminLayer = match[1]; + var adminValues = _.get(doc.parent, adminLayer); // skip invalid/empty arrays if( !Array.isArray( adminValues ) || !adminValues.length ){ continue; } @@ -92,29 +92,39 @@ function updateDocs( req, res, translations ){ for( var i in adminValues ){ // find the corresponding key from the '_id' Array - var id = doc.parent[attr][i]; - if( !id ){ continue; } - - // id not found in translation service response - if( !_.has(translations, id)){ - logger.debug( `[language] [debug] failed to find translations for ${id}` ); + let adminID = _.get(doc.parent[attr], i); + if (!adminID) { continue; } + + // find the corresponding key from the '_source' Array + // dataset the parent property was sourced from, else 'whosonfirst'. + // the language service *at time of writing* only returns translations + // from whosonfirst, so we skip translating fields from other sources. + // see: https://github.com/pelias/schema/pull/459 + // see: https://github.com/pelias/model/pull/128 + let adminSource = _.get(doc.parent, `${match}_source[${i}]`, 'whosonfirst'); + if (adminSource !== 'whosonfirst') { continue; } + + // adminID not found in translation service response + if( !_.has(translations, adminID)){ + logger.debug( `[language] [debug] failed to find translations for ${adminID}` ); continue; } // requested language is not available - if (_.isEmpty(_.get(translations[id].names, requestLanguage, [] ))) { - logger.debug( `[language] [debug] missing translation ${requestLanguage} ${id}` ); + if (_.isEmpty(_.get(translations[adminID].names, requestLanguage, [] ))) { + logger.debug( `[language] [debug] missing translation ${requestLanguage} ${adminID}` ); continue; } // translate 'parent.*' property - adminValues[i] = translations[id].names[ requestLanguage ][0]; - - // if the record is an admin record we also translate - // the 'name.default' property. - if( adminKey === doc.layer ){ - doc.name.default = translations[id].names[ requestLanguage ][0]; - } + adminValues[i] = translations[adminID].names[ requestLanguage ][0]; + + // if the record is an admin record we also translate the 'name.default' property. + // note: ensure the records have the same 'layer' 'source' and 'source_id'. + if (adminLayer !== doc.layer){ continue; } + if (adminSource !== doc.source){ continue; } + if (_.toString(adminID) !== _.toString(doc.source_id)){ continue; } + doc.name.default = translations[adminID].names[requestLanguage][0]; } } }); diff --git a/test/unit/middleware/changeLanguage.js b/test/unit/middleware/changeLanguage.js index 66c1fda31..aa79fd70a 100644 --- a/test/unit/middleware/changeLanguage.js +++ b/test/unit/middleware/changeLanguage.js @@ -148,6 +148,8 @@ module.exports.tests.success_conditions = (test, common) => { default: 'original name for 1st result' }, layer: 'layer1', + source: 'whosonfirst', + source_id: '1', parent: { layer1_id: ['1'], layer1: ['original name for layer1'], @@ -165,6 +167,8 @@ module.exports.tests.success_conditions = (test, common) => { default: 'original name for 2nd result' }, layer: 'layer10', + source: 'whosonfirst', + source_id: '2', parent: { layer3_id: ['3', '4'], layer3: ['original name 1 for layer3', 'original name 2 for layer3'], @@ -190,6 +194,7 @@ module.exports.tests.success_conditions = (test, common) => { }, // note that this is address! layer: 'address', + source: 'whosonfirst', parent: { layer1_id: ['1'], layer1: ['original name for layer1'] @@ -203,6 +208,7 @@ module.exports.tests.success_conditions = (test, common) => { }, // note that this is address! layer: 'address', + source: 'whosonfirst', parent: { layer1_id: ['1'], layer1: ['original name for layer1'] @@ -225,6 +231,8 @@ module.exports.tests.success_conditions = (test, common) => { default: 'replacement name for layer1' }, layer: 'layer1', + source: 'whosonfirst', + source_id: '1', parent: { layer1_id: ['1'], layer1: ['replacement name for layer1'], @@ -239,6 +247,8 @@ module.exports.tests.success_conditions = (test, common) => { default: 'original name for 2nd result' }, layer: 'layer10', + source: 'whosonfirst', + source_id: '2', parent: { layer3_id: ['3', '4'], layer3: ['replacement name 1 for layer3', 'replacement name 2 for layer3'], @@ -258,6 +268,7 @@ module.exports.tests.success_conditions = (test, common) => { 'requested language': 'translated name' }, layer: 'address', + source: 'whosonfirst', parent: { layer1_id: ['1'], layer1: ['replacement name for layer1'] @@ -269,6 +280,7 @@ module.exports.tests.success_conditions = (test, common) => { 'random language': 'translated name' }, layer: 'address', + source: 'whosonfirst', parent: { layer1_id: ['1'], layer1: ['replacement name for layer1'] @@ -280,7 +292,106 @@ module.exports.tests.success_conditions = (test, common) => { t.end(); }); + }); + + test('admin fields from non-whosonfirst source should be skipped', t => { + + const service = (req, res, callback) => { + callback(null, { '1': { names: { 'requested language': ['replacement name for layer1'] } } }); + }; + + const logger = require('pelias-mock-logger')(); + const controller = proxyquire('../../../middleware/changeLanguage', { + 'pelias-logger': logger + })(service, () => true); + + const req = { + clean: { + lang: { + iso6393: 'requested language', + iso6391: 'requested language', + defaulted: false + } + } + }; + + const res = { + data: [{ + name: { + default: 'original default name 1' + }, + layer: 'layer1', + source: 'whosonfirst', + source_id: '1', + parent: { + layer1_id: ['1'], + layer1: ['original name for layer1'] + } + }, { + name: { + default: 'original default name 2' + }, + layer: 'layer1', + source: 'foo', + source_id: '1', + parent: { + layer1_id: ['1'], + layer1: ['original name for layer1'] + } + }, { + name: { + default: 'original default name 3' + }, + layer: 'layer1', + source: 'whosonfirst', + source_id: '2', + parent: { + layer1_id: ['1'], + layer1: ['original name for layer1'] + } + }] + }; + + controller(req, res, () => { + t.deepEquals(res, { + data: [{ + name: { + default: 'replacement name for layer1' + }, + layer: 'layer1', + source: 'whosonfirst', + source_id: '1', + parent: { + layer1_id: ['1'], + layer1: ['replacement name for layer1'] + } + }, { + name: { + default: 'original default name 2' + }, + layer: 'layer1', + source: 'foo', + source_id: '1', + parent: { + layer1_id: ['1'], + layer1: ['replacement name for layer1'] + } + },{ + name: { + default: 'original default name 3' + }, + layer: 'layer1', + source: 'whosonfirst', + source_id: '2', + parent: { + layer1_id: ['1'], + layer1: ['replacement name for layer1'] + } + }] + }); + t.end(); + }); }); };