move empire include/exclude logic to eojsonify_place_details

helper/geojsonify.js

@@ -28,16 +28,6 @@ function geojsonifyPlaces( params, docs ){
   const geojson             = GeoJSON.parse( geodata, { Point: ['lat', 'lng'] });
   const geojsonExtentPoints = GeoJSON.parse( extentPoints, { Point: ['lat', 'lng'] });
 
-  // iterate all features removing empire* properties if there is a country_gid property
-  // per: https://github.com/pelias/placeholder/issues/54#issuecomment-333921012
-  geojson.features.forEach(feature => {
-    if (_.has(feature, 'properties.country_gid')) {
-      delete feature.properties.empire_gid;
-      delete feature.properties.empire;
-      delete feature.properties.empire_a;
-    }
-  });
-
   // to insert the bbox property at the top level of each feature, it must be done separately after
   // initial geojson construction is finished
   addBBoxPerFeature(geojson);

helper/geojsonify_place_details.js

@@ -46,6 +46,9 @@ const DETAILS_PROPS = [
   { name: 'continent',         type: 'string' },
   { name: 'continent_gid',     type: 'string' },
   { name: 'continent_a',       type: 'string' },
+  { name: 'empire',            type: 'string',    condition: _.negate(hasCountry) },
+  { name: 'empire_gid',        type: 'string',    condition: _.negate(hasCountry) },
+  { name: 'empire_a',          type: 'string',    condition: _.negate(hasCountry) },
   { name: 'ocean',             type: 'string' },
   { name: 'ocean_gid',         type: 'string' },
   { name: 'ocean_a',           type: 'string' },
@@ -57,6 +60,11 @@ const DETAILS_PROPS = [
   { name: 'category',          type: 'array',     condition: checkCategoryParam }
 ];
 
+// returns true IFF source a country_gid property
+function hasCountry(params, source) {
+  return source.hasOwnProperty('country_gid');
+}
+
 function checkCategoryParam(params) {
   return _.isObject(params) && params.hasOwnProperty('categories');
 }
@@ -72,7 +80,7 @@ function checkCategoryParam(params) {
 function collectProperties( params, source ) {
   return DETAILS_PROPS.reduce((result, prop) => {
     // if condition isn't met, don't set the property
-    if (_.isFunction(prop.condition) && !prop.condition(params)) {
+    if (_.isFunction(prop.condition) && !prop.condition(params, source)) {
       return result;
     }
 

test/unit/helper/geojsonify.js

@@ -636,104 +636,6 @@ module.exports.tests.non_optimal_conditions = (test, common) => {
 
   });
 
-  test('empire properties should be removed when there is a country_gid property', t => {
-    const input = [
-      {
-        _id: 'id 1',
-        source: 'source 1',
-        source_id: 'source_id 1',
-        layer: 'layer 1',
-        name: {
-          default: 'name 1',
-        },
-        center_point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
-      },
-      {
-        _id: 'id 2',
-        source: 'source 2',
-        source_id: 'source_id 2',
-        layer: 'layer 2',
-        name: {
-          default: 'name 2',
-        },
-        center_point: {
-          lat: 13.131313,
-          lon: 31.313131
-        }
-      }
-    ];
-
-    const geojsonify = proxyquire('../../../helper/geojsonify', {
-      './geojsonify_place_details': (params, source, dst) => {
-        if (source._id === 'id 1') {
-          return {
-            country_gid: 'country gid',
-            empire_gid: 'empire gid',
-            empire: 'empire',
-            empire_a: 'empire abbr'
-          };
-        } else if (source._id === 'id 2') {
-          return {
-            empire_gid: 'empire gid',
-            empire: 'empire',
-            empire_a: 'empire abbr'
-          };
-        }
-
-      }
-    });
-
-    const actual = geojsonify({}, input);
-
-    const expected = {
-      type: 'FeatureCollection',
-      features: [
-        {
-          type: 'Feature',
-          geometry: {
-            type: 'Point',
-            coordinates: [ 21.212121, 12.121212 ]
-          },
-          properties: {
-            id: 'id 1',
-            gid: 'source 1:layer 1:id 1',
-            layer: 'layer 1',
-            source: 'source 1',
-            source_id: 'source_id 1',
-            name: 'name 1',
-            country_gid: 'country gid',
-          }
-        },
-        {
-          type: 'Feature',
-          geometry: {
-            type: 'Point',
-            coordinates: [ 31.313131, 13.131313 ]
-          },
-          properties: {
-            id: 'id 2',
-            gid: 'source 2:layer 2:id 2',
-            layer: 'layer 2',
-            source: 'source 2',
-            source_id: 'source_id 2',
-            name: 'name 2',
-            empire_gid: 'empire gid',
-            empire: 'empire',
-            empire_a: 'empire abbr'
-          }
-        }
-      ],
-      bbox: [21.212121, 12.121212, 31.313131, 13.131313]
-    };
-
-    t.deepEquals(actual, expected);
-    t.end();
-
-  });
-
 };
 
 module.exports.all = (tape, common) => {

test/unit/helper/geojsonify_place_details.js

@@ -532,6 +532,52 @@ module.exports.tests.geojsonify_place_details = (test, common) => {
 
 };
 
+module.exports.tests.empire_specific = (test, common) => {
+  test('empire* properties should not be output when country_gid property is available', t => {
+    const source = {
+      country_gid: 'country_gid value',
+      empire: 'empire value',
+      empire_gid: 'empire_gid value',
+      empire_a: 'empire_a value',
+      label: 'label value'
+    };
+
+    const expected = {
+      country_gid: 'country_gid value',
+      label: 'label value'
+    };
+
+    const actual = geojsonify({}, source);
+
+    t.deepEqual(actual, expected);
+    t.end();
+
+  });
+
+  test('empire* properties should be output when country_gid property is not available', t => {
+    const source = {
+      empire: 'empire value',
+      empire_gid: 'empire_gid value',
+      empire_a: 'empire_a value',
+      label: 'label value'
+    };
+
+    const expected = {
+      empire: 'empire value',
+      empire_gid: 'empire_gid value',
+      empire_a: 'empire_a value',
+      label: 'label value'
+    };
+
+    const actual = geojsonify({}, source);
+
+    t.deepEqual(actual, expected);
+    t.end();
+
+  });
+
+};
+
 module.exports.all = (tape, common) => {
 
   function test(name, testFunction) {