removes hasRecentSuggestions computed property in favour of diffing suggestion and policy timestamp prior to augmenting compliancepolicy changeSet with suggestion. updates logic to get field value: identifierType/logicalType prior to suggested value. issues: needs to update the previous field value with user friendly label/displayAs value

wherehows-web/app/components/dataset-compliance-row.js

@@ -16,8 +16,9 @@ import {
   highConfidenceSuggestions,
   accumulateFieldSuggestions
 } from 'wherehows-web/utils/datasets/compliance-suggestions';
+import { hasEnumerableKeys } from 'wherehows-web/utils/object';
 
-const { computed, get, getProperties } = Ember;
+const { computed, get, getProperties, getWithDefault } = Ember;
 
 /**
  * Extracts the suggestions for identifierType, logicalType suggestions, and confidence from a list of predictions
@@ -81,12 +82,9 @@ export default DatasetTableRow.extend({
    * @return {string | void} the current value if a suggestion & current value exists or undefined
    */
   getCurrentValueBeforeSuggestion(fieldProp) {
-    const prediction = get(this, 'prediction') || {};
-    const suggested = prediction[fieldProp];
-    const { label: current } = get(this, fieldProp) || {};
-
-    if (suggested !== 'undefined' && current) {
-      return current;
+    if (hasEnumerableKeys(get(this, 'prediction'))) {
+      // TODO: translate to label: logicalType or displayAs: identifierType
+      return get(this, `field.${fieldProp}`);
     }
   },
 
@@ -181,16 +179,17 @@ export default DatasetTableRow.extend({
 
   /**
    * Extracts the field suggestions into a cached computed property, if a suggestion exists
-   * @type {Ember.computed}
+   * @type {Ember.ComputedProperty}
+   * @return {void | {identifierType: string, logicalType: string, confidence: number}}
    */
-  prediction: computed('field.suggestion', 'field.suggestionAuthority', 'hasRecentSuggestions', function() {
-    const { field = {}, hasRecentSuggestions } = getProperties(this, 'field', 'hasRecentSuggestions');
+  prediction: computed('field.suggestion', 'field.suggestionAuthority', function() {
+    const field = getWithDefault(this, 'field', {});
     // If a suggestionAuthority property exists on the field, then the user has already either accepted or ignored
     // the suggestion for this field. It's value should not be take into account on re-renders
     // this line takes that into account and substitutes an empty suggestion
     const { suggestion } = field.hasOwnProperty('suggestionAuthority') ? {} : field;
 
-    if (suggestion && hasRecentSuggestions) {
+    if (suggestion) {
       const { identifierTypePrediction, logicalTypePrediction } = suggestion;
       // The order of the array supplied to getFieldSuggestions is importance to it's order of operations
       // the last element in the array takes highest precedence: think Object.assign

wherehows-web/app/components/dataset-compliance.js

@@ -322,27 +322,6 @@ export default Component.extend({
     return getWithDefault(this, 'complianceInfo.modifiedTime', 0);
   }),
 
-  /**
-   * Checks that suggested values postdate the last save date or that suggestions exist
-   * @type {boolean}
-   */
-  hasRecentSuggestions: computed('policyModificationTimeInEpoch', 'complianceSuggestion', function() {
-    const { policyModificationTimeInEpoch, complianceSuggestion = {} } = getProperties(this, [
-      'policyModificationTimeInEpoch',
-      'complianceSuggestion'
-    ]);
-    const { lastModified: suggestionsLastModified, complianceSuggestions = [] } = complianceSuggestion;
-
-    // If modification dates exist, check that the suggestions are considered 'unseen' since the last time the policy was saved
-    // and we have at least 1 suggestion, otherwise check that the count of suggestions is at least 1
-    if (policyModificationTimeInEpoch && suggestionsLastModified) {
-      const suggestionIsUnseen = suggestionsLastModified - policyModificationTimeInEpoch >= lastSeenSuggestionInterval;
-      return complianceSuggestions.length && suggestionIsUnseen;
-    }
-
-    return !!complianceSuggestions.length;
-  }),
-
   /**
    * @type {Boolean} cached boolean flag indicating that fields do contain a `kafka type`
    *    tracking header.
@@ -428,9 +407,10 @@ export default Component.extend({
    *
    * @param {Array<object>} columnFieldProps
    * @param {Array<object>} complianceEntities
+   * @param {policyModificationTime}
    * @return {object}
    */
-  mapColumnIdFieldsToCurrentPrivacyPolicy(columnFieldProps, complianceEntities) {
+  mapColumnIdFieldsToCurrentPrivacyPolicy(columnFieldProps, complianceEntities, { policyModificationTime }) {
     const getKeysOnField = (keys = [], fieldName, source = []) => {
       const sourceField = source.find(({ identifierField }) => identifierField === fieldName) || {};
       let ret = {};
@@ -456,6 +436,7 @@ export default Component.extend({
           identifierField,
           dataType,
           ...currentPrivacyAttrs,
+          policyModificationTime,
           privacyPolicyExists: hasEnumerableKeys(currentPrivacyAttrs),
           isDirty: false
         }
@@ -479,7 +460,9 @@ export default Component.extend({
       // Dataset fields that currently have a compliance policy
       const currentComplianceEntities = get(this, policyComplianceEntitiesKey) || [];
 
-      return this.mapColumnIdFieldsToCurrentPrivacyPolicy(columnFieldProps, currentComplianceEntities);
+      return this.mapColumnIdFieldsToCurrentPrivacyPolicy(columnFieldProps, currentComplianceEntities, {
+        policyModificationTime: getWithDefault(this, 'complianceInfo.modifiedTime', 0)
+      });
     }
   ),
 
@@ -526,7 +509,9 @@ export default Component.extend({
    */
   identifierFieldToSuggestion: computed('complianceSuggestion', function() {
     const identifierFieldToSuggestion = {};
-    const complianceSuggestions = getWithDefault(this, 'complianceSuggestion.complianceSuggestions', []);
+    const complianceSuggestion = get(this, 'complianceSuggestion') || {};
+    const { lastModified: suggestionsModificationTime, complianceSuggestions = [] } = complianceSuggestion;
+
     // If the compliance suggestions array contains suggestions the create reduced map,
     // otherwise, ignore
     if (complianceSuggestions.length) {
@@ -535,7 +520,8 @@ export default Component.extend({
           ...identifierFieldToSuggestion,
           [fieldName]: {
             identifierTypePrediction,
-            logicalTypePrediction
+            logicalTypePrediction,
+            suggestionsModificationTime
           }
         }),
         identifierFieldToSuggestion

wherehows-web/app/templates/datasets/dataset-compliance/-dataset-compliance-entities.hbs

@@ -111,8 +111,7 @@
         {{#body.row
           field=field
           isNewComplianceInfo=isNewComplianceInfo
-          class=(if (and hasRecentSuggestions field.suggestion) "dataset-compliance-fields__has-suggestions")
-          hasRecentSuggestions=hasRecentSuggestions
+          class=(if field.suggestion "dataset-compliance-fields__has-suggestions")
           onFieldLogicalTypeChange=(action 'onFieldLogicalTypeChange')
           onFieldClassificationChange=(action 'onFieldClassificationChange')
           onSuggestionIntent=(action 'onFieldSuggestionIntentChange')

wherehows-web/app/utils/datasets/compliance-policy.js

@@ -1,5 +1,6 @@
 import Ember from 'ember';
 import { datasetClassifiers } from 'wherehows-web/constants/dataset-classification';
+import { lastSeenSuggestionInterval } from 'wherehows-web/constants/metadata-acquisition';
 
 const { assert, Logger: { warn } } = Ember;
 
@@ -110,6 +111,16 @@ const isPolicyExpectedShape = (candidatePolicy = {}) => {
   return false;
 };
 
+/**
+ * Checks if the compliance suggestion has a date that is equal or exceeds the policy mod time by at least the
+ * ms time in lastSeenSuggestionInterval
+ * @param {number} [policyModificationTime = 0] timestamp for the policy modification date
+ * @param {number} suggestionModificationTime timestamp for the suggestion modification date
+ * @return {boolean}
+ */
+const isRecentSuggestion = (policyModificationTime = 0, suggestionModificationTime) =>
+  !!suggestionModificationTime && suggestionModificationTime - policyModificationTime >= lastSeenSuggestionInterval;
+
 /**
  * Checks if a compliance policy changeSet field requires user attention: if a suggestion
  * is available  but the user has not indicated intent or a policy for the field does not currently exist remotely
@@ -143,6 +154,7 @@ const mergeMappedColumnFieldsWithSuggestions = (mappedColumnFields = {}, fieldSu
       identifierType,
       logicalType,
       securityClassification,
+      policyModificationTime,
       privacyPolicyExists,
       isDirty
     } = mappedColumnFields[fieldName];
@@ -158,8 +170,9 @@ const mergeMappedColumnFieldsWithSuggestions = (mappedColumnFields = {}, fieldSu
       classification: securityClassification
     };
 
-    // If a suggestion exists for this field add the suggestion attribute to the field properties
-    if (suggestion) {
+    // If a suggestion exists for this field add the suggestion attribute to the field properties / changeSet
+    // Check if suggestion isRecent before augmenting, otherwise, suggestion will not be considered on changeSet
+    if (suggestion && isRecentSuggestion(policyModificationTime, suggestion.suggestionsModificationTime)) {
       return { ...field, suggestion };
     }
 
@@ -170,5 +183,6 @@ export {
   createInitialComplianceInfo,
   isPolicyExpectedShape,
   fieldChangeSetRequiresReview,
-  mergeMappedColumnFieldsWithSuggestions
+  mergeMappedColumnFieldsWithSuggestions,
+  isRecentSuggestion
 };

wherehows-web/tests/helpers/datasets/compliance-policy/recent-suggestions-constants.ts

@@ -0,0 +1,80 @@
+interface IMockTimeStamp {
+  policyModificationTime: number;
+  suggestionModificationTime: number;
+  __assertMsg__: string;
+  __isRecent__: boolean;
+}
+
+const sevenDays: number = 7 * 24 * 60 * 60 * 1000;
+
+const isRecent = ({ policyModificationTime, suggestionModificationTime }: IMockTimeStamp) =>
+  !!suggestionModificationTime && suggestionModificationTime - policyModificationTime >= sevenDays;
+
+const mockTimeStamps = [
+  {
+    policyModificationTime: new Date('2017-10-08 06:58:53.0').getTime(),
+    suggestionModificationTime: new Date('2017-10-15 06:58:53.0').getTime(),
+    __assertMsg__: 'Suggestion time is exactly 7 days after policy modification date',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: new Date('2017-10-08 06:58:53.0').getTime(),
+    suggestionModificationTime: new Date('2017-10-15 06:57:53.0').getTime(),
+    __assertMsg__: 'Suggestion time is slightly less than 7 days after policy modification date',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: new Date('2017-10-08 06:58:53.0').getTime(),
+    suggestionModificationTime: new Date('2017-10-14 06:58:53.0').getTime(),
+    __assertMsg__: 'Suggestion time is 6 days after policy modification date',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: new Date('2017-10-08 06:58:53.0').getTime(),
+    suggestionModificationTime: new Date('2016-10-15 06:58:53.0').getTime(),
+    __assertMsg__: 'Suggestion time is before policy modification date',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: new Date('2017-10-08 06:58:53.0').getTime(),
+    suggestionModificationTime: new Date('2017-11-15 06:58:53.0').getTime(),
+    __assertMsg__: 'Suggestion time is greater than minimum interval since policy modification date',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: 0,
+    suggestionModificationTime: new Date('2017-11-15 06:58:53.0').getTime(),
+    __assertMsg__: 'Policy modification time does not exist',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: new Date('2017-11-15 06:58:53.0').getTime(),
+    suggestionModificationTime: 0,
+    __assertMsg__: 'Suggestion modification time does not exist',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  },
+  {
+    policyModificationTime: 0,
+    suggestionModificationTime: 0,
+    __assertMsg__: 'Suggestion and policy modification time does not exist',
+    get __isRecent__(): boolean {
+      return isRecent(this);
+    }
+  }
+];
+
+export { mockTimeStamps };

wherehows-web/tests/unit/utils/datasets/compliance-policy-test.js

@@ -1,9 +1,11 @@
 import {
   createInitialComplianceInfo,
-  fieldChangeSetRequiresReview
+  fieldChangeSetRequiresReview,
+  isRecentSuggestion
 } from 'wherehows-web/utils/datasets/compliance-policy';
 
 import { mockFieldChangeSets } from 'wherehows-web/tests/helpers/datasets/compliance-policy/field-changeset-constants';
+import { mockTimeStamps } from 'wherehows-web/tests/helpers/datasets/compliance-policy/recent-suggestions-constants';
 import { module, test } from 'qunit';
 
 module('Unit | Utility | datasets/compliance policy');
@@ -32,10 +34,24 @@ test('Compliance utility function fieldChangeSetRequiresReview exists', function
   assert.ok(typeof fieldChangeSetRequiresReview() === 'boolean', 'fieldChangeSetRequiresReview returns a boolean');
 });
 
-test('Compliance utility function fieldChangeSetRequiresReview exists', function(assert) {
+test('fieldChangeSetRequiresReview correctly determines if a fieldChangeSet requires review', function(assert) {
   assert.expect(mockFieldChangeSets.length);
 
   mockFieldChangeSets.forEach(changeSet =>
     assert.ok(fieldChangeSetRequiresReview(changeSet) === changeSet.__requiresReview__, changeSet.__msg__)
   );
 });
+
+test('isRecentSuggestion exists', function(assert) {
+  assert.expect(1);
+  assert.ok(typeof isRecentSuggestion === 'function', 'isRecentSuggestion is a function');
+});
+
+test('isRecentSuggestion correctly determines if a suggestion is recent or not', function(assert) {
+  assert.expect(mockTimeStamps.length);
+
+  mockTimeStamps.forEach(({ policyModificationTime, suggestionModificationTime, __isRecent__, __assertMsg__ }) => {
+    const recent = isRecentSuggestion(policyModificationTime, suggestionModificationTime);
+    assert.ok(recent === __isRecent__, `${__assertMsg__} isRecent? ${recent}`);
+  });
+});