diff --git a/wherehows-web/app/components/dataset-compliance-row.js b/wherehows-web/app/components/dataset-compliance-row.js index fd216ef86f..1727b5e909 100644 --- a/wherehows-web/app/components/dataset-compliance-row.js +++ b/wherehows-web/app/components/dataset-compliance-row.js @@ -2,6 +2,7 @@ import Ember from 'ember'; import DatasetTableRow from 'wherehows-web/components/dataset-table-row'; import { fieldIdentifierTypeIds, + fieldIdentifierOptions, defaultFieldDataTypeClassification, isMixedId, isCustomId, @@ -16,8 +17,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 +83,24 @@ 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 (hasEnumerableKeys(get(this, 'prediction'))) { + /** + * Current value on policy prior to the suggested value + * @type {string} + */ + const value = get(this, `field.${fieldProp}`); - if (suggested !== 'undefined' && current) { - return current; + /** + * Convenience function to get `label` attribute on the display properties object + * @param {Array<{value: string, label: string}>} valueObjects + */ + const getLabel = (valueObjects = []) => + Array.isArray(valueObjects) && (valueObjects.findBy('value', value) || {}).label; + + return { + identifierType: getLabel(fieldIdentifierOptions), + logicalType: getLabel(get(this, 'fieldFormats')) + }[fieldProp]; } }, @@ -181,16 +195,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 diff --git a/wherehows-web/app/components/dataset-compliance.js b/wherehows-web/app/components/dataset-compliance.js index 097b00f01b..8e1cf1ee16 100644 --- a/wherehows-web/app/components/dataset-compliance.js +++ b/wherehows-web/app/components/dataset-compliance.js @@ -4,6 +4,7 @@ import { classifiers, datasetClassifiers, fieldIdentifierTypes, + fieldIdentifierOptions, fieldIdentifierTypeIds, idLogicalTypes, nonIdFieldLogicalTypes, @@ -12,8 +13,7 @@ import { logicalTypesForIds, logicalTypesForGeneric, hasPredefinedFieldFormat, - getDefaultLogicalType, - lastSeenSuggestionInterval + getDefaultLogicalType } from 'wherehows-web/constants'; import { isPolicyExpectedShape, @@ -85,8 +85,6 @@ const datasetClassifiersKeys = Object.keys(datasetClassifiers); const policyComplianceEntitiesKey = 'complianceInfo.complianceEntities'; assert('`fieldIdentifierTypes` contains an object with a key `none`', typeof fieldIdentifierTypes.none === 'object'); -const fieldIdentifierTypeKeysBarNone = Object.keys(fieldIdentifierTypes).filter(k => k !== 'none'); -const fieldDisplayKeys = ['none', '_', ...fieldIdentifierTypeKeysBarNone]; /** * Returns a list of changeSet fields that requires user attention @@ -94,23 +92,6 @@ const fieldDisplayKeys = ['none', '_', ...fieldIdentifierTypeKeysBarNone]; */ const changeSetFieldsRequiringReview = arrayFilter(fieldChangeSetRequiresReview); -/** - * A list of field identifier types mapped to label, value options for select display - * @type {any[]|Array.<{value: String, label: String}>} - */ -const fieldIdentifierOptions = fieldDisplayKeys.map(fieldIdentifierType => { - const divider = '──────────'; - const { value = fieldIdentifierType, displayAs: label = divider } = fieldIdentifierTypes[fieldIdentifierType] || {}; - - // Adds a divider for a value of _ - // Visually this separates ID from none fieldIdentifierTypes - return { - value, - label, - isDisabled: fieldIdentifierType === '_' - }; -}); - export default Component.extend({ sortColumnWithName: 'identifierField', filterBy: 'identifierField', @@ -322,27 +303,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 +388,10 @@ export default Component.extend({ * * @param {Array} columnFieldProps * @param {Array} 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 +417,7 @@ export default Component.extend({ identifierField, dataType, ...currentPrivacyAttrs, + policyModificationTime, privacyPolicyExists: hasEnumerableKeys(currentPrivacyAttrs), isDirty: false } @@ -479,7 +441,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 +490,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 +501,8 @@ export default Component.extend({ ...identifierFieldToSuggestion, [fieldName]: { identifierTypePrediction, - logicalTypePrediction + logicalTypePrediction, + suggestionsModificationTime } }), identifierFieldToSuggestion diff --git a/wherehows-web/app/constants/dataset-compliance.ts b/wherehows-web/app/constants/dataset-compliance.ts index 554b8e3d23..230c3f2851 100644 --- a/wherehows-web/app/constants/dataset-compliance.ts +++ b/wherehows-web/app/constants/dataset-compliance.ts @@ -1,3 +1,4 @@ +import { fieldIdentifierTypes } from 'wherehows-web/constants/metadata-acquisition'; /** * Defines a map of values for the compliance policy on a dataset * @type {object} @@ -17,4 +18,37 @@ const compliancePolicyStrings = { } }; -export { compliancePolicyStrings }; +/** + * List of identifier type keys without the none object value + * @type {Array} + */ +const fieldIdentifierTypeKeysBarNone: Array = Object.keys(fieldIdentifierTypes).filter(k => k !== 'none'); + +/** + * Keys for the field display options + * @type {Array} + */ +const fieldDisplayKeys: Array = ['none', '_', ...fieldIdentifierTypeKeysBarNone]; + +/** + * A list of field identifier types mapped to label, value options for select display + * @type {Array<{value: string, label: string, isDisabled: boolean}>} + */ +const fieldIdentifierOptions: Array<{ + value: string; + label: string; + isDisabled: boolean; +}> = fieldDisplayKeys.map(fieldIdentifierType => { + const divider = '──────────'; + const { value = fieldIdentifierType, displayAs: label = divider } = fieldIdentifierTypes[fieldIdentifierType] || {}; + + // Adds a divider for a value of _ + // Visually this separates ID from none fieldIdentifierTypes + return { + value, + label, + isDisabled: fieldIdentifierType === '_' + }; +}); + +export { compliancePolicyStrings, fieldIdentifierOptions }; diff --git a/wherehows-web/app/templates/datasets/dataset-compliance/-dataset-compliance-entities.hbs b/wherehows-web/app/templates/datasets/dataset-compliance/-dataset-compliance-entities.hbs index c4e7ed1066..34661c9969 100644 --- a/wherehows-web/app/templates/datasets/dataset-compliance/-dataset-compliance-entities.hbs +++ b/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') diff --git a/wherehows-web/app/utils/datasets/compliance-policy.js b/wherehows-web/app/utils/datasets/compliance-policy.js index 37d0950814..32281c8d3e 100644 --- a/wherehows-web/app/utils/datasets/compliance-policy.js +++ b/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 }; diff --git a/wherehows-web/tests/helpers/datasets/compliance-policy/recent-suggestions-constants.ts b/wherehows-web/tests/helpers/datasets/compliance-policy/recent-suggestions-constants.ts new file mode 100644 index 0000000000..3fe5b9c2d0 --- /dev/null +++ b/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 }; diff --git a/wherehows-web/tests/unit/utils/datasets/compliance-policy-test.js b/wherehows-web/tests/unit/utils/datasets/compliance-policy-test.js index 1a713bc29e..26bed59f4b 100644 --- a/wherehows-web/tests/unit/utils/datasets/compliance-policy-test.js +++ b/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}`); + }); +});