feat(dedupe): improved handling of cases where "name", "parent" or "address_parts" propertiees are not set

helper/diffPlaces.js

@@ -27,12 +27,18 @@ function isParentHierarchyDifferent(item1, item2){
   let parent1 = _.get(item1, 'parent');
   let parent2 = _.get(item2, 'parent');
 
+  // check if these are plain 'ol javascript objects
+  let isPojo1 = _.isPlainObject(parent1);
+  let isPojo2 = _.isPlainObject(parent2);
+
   // if neither object has parent info, we consider them the same
-  if( !parent1 && !parent2 ){ return false; }
+  if( !isPojo1 && !isPojo2 ){ return false; }
 
-  // both have parent info
+  // if only one has parent info, we consider them the same
-  if( _.isPlainObject(parent1) && _.isPlainObject(parent2) ){
+  // note: this really shouldn't happen as at least on parent should exist
+  if( !isPojo1 || !isPojo2 ){ return false; }
 
+  // else both have parent info
   // iterate over all the placetypes, comparing between items
   return placeTypes.some( placeType => {
 
@@ -44,10 +50,6 @@ function isParentHierarchyDifferent(item1, item2){
   });
 }
 
-  // if one has parent info and the other doesn't, we consider them different
-  return true;
-}
-
 /**
  * Compare the name properties if they exist.
  * Returns false if the objects are the same, else true.
@@ -56,12 +58,18 @@ function isNameDifferent(item1, item2){
   let names1 = _.get(item1, 'name');
   let names2 = _.get(item2, 'name');
 
+  // check if these are plain 'ol javascript objects
+  let isPojo1 = _.isPlainObject(names1);
+  let isPojo2 = _.isPlainObject(names2);
+
   // if neither object has name info, we consider them the same
-  if( !names1 && !names2 ){ return false; }
+  if( !isPojo1 && !isPojo2 ){ return false; }
 
-  // if both have name info
+  // if only one has name info, we consider them the same
-  if( _.isPlainObject(names1) && _.isPlainObject(names2) ){
+  // note: this really shouldn't happen as name is a mandatory field
+  if( !isPojo1 || !isPojo2 ){ return false; }
 
+  // else both have name info
   // iterate over all the languages in item1, comparing between items
   return Object.keys(names1).some( lang => {
 
@@ -75,10 +83,6 @@ function isNameDifferent(item1, item2){
   });
 }
 
-  // if one has name info and the other doesn't, we consider them different
-  return true;
-}
-
 /**
  * Compare the address_parts properties if they exist.
  * Returns false if the objects are the same, else true.
@@ -87,12 +91,17 @@ function isAddressDifferent(item1, item2){
   let address1 = _.get(item1, 'address_parts');
   let address2 = _.get(item2, 'address_parts');
 
+  // check if these are plain 'ol javascript objects
+  let isPojo1 = _.isPlainObject(address1);
+  let isPojo2 = _.isPlainObject(address2);
+
   // if neither object has address info, we consider them the same
-  if( !address1 && !address2 ){ return false; }
+  if( !isPojo1 && !isPojo2 ){ return false; }
 
-  // if both have address info
+  // if only one has address info, we consider them the same
-  if( _.isPlainObject(address1) && _.isPlainObject(address2) ){
+  if( !isPojo1 || !isPojo2 ){ return false; }
 
+  // else both have address info
   if( isPropertyDifferent(address1, address2, 'number') ){ return true; }
   if( isPropertyDifferent(address1, address2, 'street') ){ return true; }
 
@@ -105,10 +114,6 @@ function isAddressDifferent(item1, item2){
   return false;
 }
 
-  // one has address and the other doesn't, different!
-  return true;
-}
-
 /**
  * Compare the two records and return true if they differ and false if same.
  */

test/unit/middleware/dedupe.js

@@ -1,7 +1,7 @@
 var data = require('../fixture/dedupe_elasticsearch_results');
 var nonAsciiData = require('../fixture/dedupe_elasticsearch_nonascii_results');
 var customLayerData = require('../fixture/dedupe_elasticsearch_custom_layer_results');
-var onlyPostalcodeDiffers = require('../fixture/dedupe_only_postalcode_differs');
+var onlyPostalcodeDiffersData = require('../fixture/dedupe_only_postalcode_differs');
 var dedupe = require('../../../middleware/dedupe')();
 
 module.exports.tests = {};
@@ -84,9 +84,9 @@ module.exports.tests.dedupe = function(test, common) {
       }
     };
     var res = {
-      data: onlyPostalcodeDiffers
+      data: onlyPostalcodeDiffersData
     };
-    var expected = onlyPostalcodeDiffers[1]; // non-canonical record
+    var expected = onlyPostalcodeDiffersData[1]; // record with postcode
 
     dedupe(req, res, function () {
       t.equal(res.data.length, 1, 'only one result displayed');