Self code review

src/config/index.ts

@@ -196,12 +196,8 @@ export class Config {
 
   public static getUniqueProperties(contactTypeName: string): ContactProperty[] {
     const contactMatch = config.contact_types.find(c => c.name === contactTypeName);
-    if (!contactMatch) {
-      return [];
-    }
-
-    return contactMatch.place_properties
-      .filter(prop => prop.unique);
+    const uniqueProperties = contactMatch?.place_properties.filter(prop => prop.unique);
+    return uniqueProperties || [];
   }
 
   // TODO: Joi? Chai?

src/services/place.ts

@@ -1,15 +1,15 @@
-import Contact from './contact';
+import _ from 'lodash';
 import { v4 as uuidv4 } from 'uuid';
 
 import { Config, ContactProperty, ContactType } from '../config';
+import Contact from './contact';
+import { ContactPropertyValue, HierarchyPropertyValue } from '../property-value';
 import { IPropertyValue, RemotePlacePropertyValue } from '../property-value';
 import { PlacePayload } from '../lib/cht-api';
 // can't use package.json because of rootDir in ts
-import { version as appVersion } from '../package.json';
-import RemotePlaceResolver from '../lib/remote-place-resolver';
-import { HierarchyPropertyValue, ContactPropertyValue } from '../property-value';
 import { RemotePlace } from '../lib/remote-place-cache';
-import _ from 'lodash';
+import RemotePlaceResolver from '../lib/remote-place-resolver';
+import { version as appVersion } from '../package.json';
 
 export type FormattedPropertyCollection = {
   [key: string]: IPropertyValue;
@@ -181,22 +181,14 @@ export default class Place {
   }
 
   public asRemotePlace() : RemotePlace {
-    function getUniqueKeys(properties: FormattedPropertyCollection, place_properties: ContactProperty[]): FormattedPropertyCollection {
-      const uniquePropertyNames = place_properties
-        .filter(prop => prop.unique)
-        .map(prop => prop.property_name);
-
-      return _.pick(properties, uniquePropertyNames);
-    }
-
     const nameProperty = Config.getPropertyWithName(this.type.place_properties, 'name');
     return {
       id: this.id,
       name: new RemotePlacePropertyValue(this.name, nameProperty),
       placeType: this.type.name,
       type: this.isCreated ? 'remote' : 'local',
-      uniquePlaceValues: getUniqueKeys(this.properties, this.type.place_properties),
-      uniqueContactValues: getUniqueKeys(this.contact.properties, this.type.contact_properties),
+      uniquePlaceValues: this.getUniqueKeys(this.properties, this.type.place_properties),
+      uniqueContactValues: this.getUniqueKeys(this.contact.properties, this.type.contact_properties),
       stagedPlace: this,
       lineage: this.buildLineage(),
     };
@@ -295,4 +287,12 @@ export default class Place {
 
     return lineage;
   }
+
+  private getUniqueKeys(properties: FormattedPropertyCollection, place_properties: ContactProperty[]): FormattedPropertyCollection {
+    const uniquePropertyNames = place_properties
+      .filter(prop => prop.unique)
+      .map(prop => prop.property_name);
+
+    return _.pick(properties, uniquePropertyNames);
+  }
 }

src/warnings/index.ts

@@ -35,6 +35,20 @@ export default class WarningSystem {
     });
   }
 
+  private static createClassifiers(contactType: ContactType): IWarningClassifier[] {
+    const createUniquePropertyClassifiers = (properties: ContactProperty[], propertyType: 'place' | 'contact') => properties
+      .filter(prop => prop.unique)
+      .map(prop => new UniquePropertyClassifier(propertyType, prop));
+
+    const classifiers = [
+      ...createUniquePropertyClassifiers(contactType.place_properties, 'place'),
+      ...createUniquePropertyClassifiers(contactType.contact_properties, 'contact'),
+      new RedundantReplaceClassifier(contactType),
+    ];
+
+    return classifiers;
+  }
+
   private static runClassifiers(warningClassifiers: IWarningClassifier[], remotePlaces: RemotePlace[], localPlaces: Place[]): Warning[] {
     const warnings: Warning[] = [];
     const knownWarnings = new Set<string>();
@@ -78,8 +92,8 @@ export default class WarningSystem {
     }
 
     const implicatedLocalPlacesWithoutBase = implicatedPlaces
-      .filter(remotePlace => remotePlace.stagedPlace)
-      .map(remotePlace => remotePlace.stagedPlace) as Place[];
+      .map(remotePlace => remotePlace.stagedPlace)
+      .filter(Boolean) as Place[];
     const implicatedLocalPlaces = [basePlace.stagedPlace, ...implicatedLocalPlacesWithoutBase];
 
     const implicatedRemotePlaces = implicatedPlaces.filter(remotePlace => !remotePlace.stagedPlace);
@@ -89,18 +103,4 @@ export default class WarningSystem {
       uniqueKey: classifier.uniqueKey(place),
     }));
   }
-
-  private static createClassifiers(contactType: ContactType): IWarningClassifier[] {
-    const createUniquePropertyClassifiers = (properties: ContactProperty[], propertyType: 'place' | 'contact') => properties
-      .filter(prop => prop.unique)
-      .map(prop => new UniquePropertyClassifier(propertyType, prop));
-
-    const classifiers = [
-      ...createUniquePropertyClassifiers(contactType.place_properties, 'place'),
-      ...createUniquePropertyClassifiers(contactType.contact_properties, 'contact'),
-      new RedundantReplaceClassifier(contactType),
-    ];
-
-    return classifiers;
-  }
 }

src/warnings/redundant-replace-classifier.ts

@@ -19,7 +19,7 @@ export default class RedundantReplaceClassifier implements IWarningClassifier {
     const confirmedDuplicates = placesToCompare
       .filter(place => place.type !== 'invalid' && 
         place.stagedPlace && 
-        place.stagedPlace?.resolvedHierarchy[0]?.id === replacementDetails.id);
+        place.stagedPlace.resolvedHierarchy?.[0]?.id === replacementDetails.id);
 
     if (confirmedDuplicates.length) {
       return confirmedDuplicates;