diff --git a/wherehows-web/app/components/dataset-compliance.js b/wherehows-web/app/components/dataset-compliance.js index 85fe7808c0..e34cdf8ccf 100644 --- a/wherehows-web/app/components/dataset-compliance.js +++ b/wherehows-web/app/components/dataset-compliance.js @@ -6,7 +6,8 @@ import { fieldIdentifierTypes, idLogicalTypes, nonIdFieldLogicalTypes, - defaultFieldDataTypeClassification + defaultFieldDataTypeClassification, + compliancePolicyStrings } from 'wherehows-web/constants'; import { isPolicyExpectedShape } from 'wherehows-web/utils/datasets/functions'; @@ -23,25 +24,14 @@ const { String: { htmlSafe } } = Ember; -// TODO: DSS-6671 Extract to constants module -const missingTypes = 'Looks like some fields may contain privacy data but do not have a specified `Field Format`?'; -const successUpdating = 'Your changes have been successfully saved!'; -const failedUpdating = 'Oops! We are having trouble updating this dataset at the moment.'; +const { complianceDataException, missingTypes, successUpdating, failedUpdating, helpText } = compliancePolicyStrings; + const hiddenTrackingFieldsMsg = htmlSafe( '

Some fields in this dataset have been hidden from the table(s) below. ' + "These are tracking fields for which we've been able to predetermine the compliance classification.

" + '

For example: header.memberId, requestHeader. ' + 'Hopefully, this saves you some scrolling!

' ); -const helpText = { - classification: - 'The default value is taken from go/dht and should be good enough in most cases. ' + - 'You can optionally override it if required by house security.', - subjectOwner: - 'Member ID (Subject Owner): Choose this option if the member specified here is not the owner of this ' + - 'record, e.g. the profile ID viewed, or recipient ID of a message. See wiki for more explanation. ' + - 'Mixed ID: Choose this option if the field is a URN containing a mixture of member, organization, or group ID.' -}; /** * Takes a string, returns a formatted string. Niche , single use case @@ -85,10 +75,10 @@ const policyFieldClassificationKey = 'complianceInfo.fieldClassification'; const policyComplianceEntitiesKey = 'complianceInfo.complianceEntities'; /** * Duplicate check using every to short-circuit iteration - * @param {Array} names = [] the list to check for dupes + * @param {Array} list = [] the list to check for dupes * @return {Boolean} true is unique, false otherwise */ -const fieldNamesAreUnique = (names = []) => names.every((name, index) => names.indexOf(name) === index); +const listIsUnique = (list = []) => new Set(list).size === list.length; assert('`fieldIdentifierTypes` contains an object with a key `none`', typeof fieldIdentifierTypes.none === 'object'); const fieldIdentifierTypeKeysBarNone = Object.keys(fieldIdentifierTypes).filter(k => k !== 'none'); @@ -146,8 +136,17 @@ export default Component.extend({ fieldIdentifierOptions, hiddenTrackingFields: hiddenTrackingFieldsMsg, classNames: ['compliance-container'], - isEditing: false, classNameBindings: ['isEditing:compliance-container--edit-mode'], + /** + * Flag indicating that the component is in edit mode + * @type {String} + */ + isEditing: false, + /** + * Flag indicating that the component is currently saving / attempting to save the privacy policy + * @type {String} + */ + isSaving: false, didReceiveAttrs() { this._super(...arguments); @@ -164,7 +163,8 @@ export default Component.extend({ validateAttrs() { const fieldNames = getWithDefault(this, 'schemaFieldNamesMappedToDataTypes', []).mapBy('fieldName'); - if (fieldNamesAreUnique(fieldNames.sort())) { + // identifier field names from the column api should be unique + if (listIsUnique(fieldNames.sort())) { return set(this, '_hasBadData', false); } @@ -196,18 +196,20 @@ export default Component.extend({ * tracking header. * Used to indicate to viewer that these fields are hidden. */ - containsHiddenTrackingFields: computed('complianceDataFieldsSansHiddenTracking.length', function() { - // If their is a diff in complianceDataFields and complianceDataFieldsSansHiddenTracking, + containsHiddenTrackingFields: computed('truncatedSchemaFields.length', function() { + // If their is a diff in complianceDataFields and truncatedSchemaFields, // then we have hidden tracking fields - return get(this, 'complianceDataFieldsSansHiddenTracking.length') !== get(this, 'complianceDataFields.length'); + return get(this, 'truncatedSchemaFields.length') !== get(this, 'complianceDataFields.length'); }), /** * @type {Array.} Filters the mapped compliance data fields without `kafka type` * tracking headers */ - complianceDataFieldsSansHiddenTracking: computed('complianceDataFields.[]', function() { - return get(this, 'complianceDataFields').filter(({ identifierField }) => !isTrackingHeaderField(identifierField)); + truncatedSchemaFields: computed('schemaFieldNamesMappedToDataTypes', function() { + return getWithDefault(this, 'schemaFieldNamesMappedToDataTypes', []).filter( + ({ fieldName }) => !isTrackingHeaderField(fieldName) + ); }), /** @@ -222,6 +224,28 @@ export default Component.extend({ .every(({ value }) => [true, false].includes(value)); }), + /** + * Determines if the save feature is allowed for the current dataset, otherwise e.g. interface should be disabled + * @type {Ember.computed} + */ + isSavingDisabled: computed('isDatasetFullyClassified', 'isSaving', function() { + const { isDatasetFullyClassified, isSaving } = getProperties(this, ['isDatasetFullyClassified', 'isSaving']); + + return !isDatasetFullyClassified || isSaving; + }), + + /** + * Checks to ensure the the number of fields added to compliance entities is less than or equal + * to what is available on the dataset schema + * @return {boolean} + */ + isSchemaFieldLengthGreaterThanComplianceEntities() { + const { length: columnFieldsLength } = getWithDefault(this, 'schemaFieldNamesMappedToDataTypes', []); + const { length: complianceListLength } = get(this, policyComplianceEntitiesKey); + + return columnFieldsLength >= complianceListLength; + }, + /** * Computed property that is dependent on all the keys in the datasetClassification map * Returns a new map of datasetClassificationKey: String-> Object. @@ -250,7 +274,7 @@ export default Component.extend({ `${policyComplianceEntitiesKey}.@each.identifierType`, `${policyComplianceEntitiesKey}.[]`, policyFieldClassificationKey, - 'schemaFieldNamesMappedToDataTypes', + 'truncatedSchemaFields', function() { /** * Retrieves an attribute on the `policyComplianceEntitiesKey` where the identifierField is the same as the @@ -261,7 +285,17 @@ export default Component.extend({ */ const getAttributeOnField = (attribute, fieldName) => { const complianceEntities = get(this, policyComplianceEntitiesKey) || []; - const sourceField = complianceEntities.find(({ identifierField }) => identifierField === fieldName); + // const sourceField = complianceEntities.find(({ identifierField }) => identifierField === fieldName); + let sourceField; + + // For long records: >500 elements, the find operation is consistently less performant than a for-loop: + // trading elegance for efficiency here + for (let i = 0; i < complianceEntities.length; i++) { + if (complianceEntities[i]['identifierField'] === fieldName) { + sourceField = complianceEntities[i]; + break; + } + } return sourceField ? sourceField[attribute] : null; }; @@ -277,11 +311,7 @@ export default Component.extend({ // Set default or if already in policy, retrieve current values from // privacyCompliancePolicy.complianceEntities - return getWithDefault( - this, - 'schemaFieldNamesMappedToDataTypes', - [] - ).map(({ fieldName: identifierField, dataType }) => { + return getWithDefault(this, 'truncatedSchemaFields', []).map(({ fieldName: identifierField, dataType }) => { const [identifierType, isSubject, logicalType] = getAttributesOnField( ['identifierType', 'isSubject', 'logicalType'], identifierField @@ -372,7 +402,7 @@ export default Component.extend({ * @param { Boolean} [isSaving = false] optional flag indicating when the user intends to persist / save */ whenRequestCompletes(request, { successMessage, isSaving = false } = {}) { - Promise.resolve(request) + return Promise.resolve(request) .then(({ status = 'error' }) => { return status === 'ok' ? setProperties(this, { @@ -421,10 +451,11 @@ export default Component.extend({ */ confirmUnformattedFields() { // Current list of compliance entities on policy - const complianceList = get(this, policyComplianceEntitiesKey); - // All candidate fields that can be on policy - const datasetFields = get(this, 'complianceDataFieldsSansHiddenTracking'); - // Fields that do not have a logicalType and not identifierType or identifierType is `fieldIdentifierTypes.none` + const complianceEntities = get(this, policyComplianceEntitiesKey); + // All candidate fields that can be on policy, excluding tracking type fields + const datasetFields = get(this, 'complianceDataFields'); + const fieldsCurrentlyInComplianceList = complianceEntities.mapBy('identifierField'); + // Fields that do not have a logicalType, and no identifierType or identifierType is `fieldIdentifierTypes.none` const unformattedFields = datasetFields.filter( ({ identifierType, logicalType }) => !logicalType && (fieldIdentifierTypes.none.value === identifierType || !identifierType) @@ -433,6 +464,17 @@ export default Component.extend({ // If there are unformatted fields, require confirmation from user if (!isEmpty(unformattedFields)) { + // Ensure that the unformatted fields to be added to the entities are not already present + // in the previous compliance entities list + const unformattedComplianceEntities = unformattedFields + .filter(({ identifierField }) => !fieldsCurrentlyInComplianceList.includes(identifierField)) + .map(({ identifierField }) => ({ + identifierField, + identifierType: fieldIdentifierTypes.none.value, + isSubject: null, + logicalType: void 0 + })); + isConfirmed = confirm( `There are ${unformattedFields.length} non-ID fields that have no field format specified. ` + `Are you sure they don't contain any of the following PII?\n\n` + @@ -441,21 +483,43 @@ export default Component.extend({ // If the user confirms that this is ok, apply the unformatted fields on the current compliance list // to be saved - isConfirmed && - complianceList.setObjects([ - ...complianceList, - ...unformattedFields.map(({ identifierField }) => ({ - identifierField, - identifierType: fieldIdentifierTypes.none.value, - isSubject: null, - logicalType: void 0 - })) - ]); + isConfirmed && complianceEntities.setObjects([...complianceEntities, ...unformattedComplianceEntities]); } return isConfirmed; }, + /** + * Ensures the fields in the updated list of compliance entities meet the criteria + * checked in the function. If criteria is not met, an the returned promise is settled + * in a rejected state, otherwise fulfilled + * @method + * @return {any | Promise} + */ + validateFields() { + const complianceEntities = get(this, policyComplianceEntitiesKey); + const idFieldsHaveValidLogicalType = this.checkEachEntityByLogicalType( + complianceEntities.filter(({ identifierType }) => fieldIdentifierTypeIds.includes(identifierType)), + [...genericLogicalTypes, ...idLogicalTypes] + ); + const fieldIdentifiersAreUnique = listIsUnique(complianceEntities.mapBy('identifierField')); + const schemaFieldLengthGreaterThanComplianceEntities = this.isSchemaFieldLengthGreaterThanComplianceEntities(); + + // + if (!fieldIdentifiersAreUnique || !schemaFieldLengthGreaterThanComplianceEntities) { + return Promise.reject(new Error(complianceDataException)); + } + + if (!idFieldsHaveValidLogicalType) { + return Promise.reject( + setProperties(this, { + _message: missingTypes, + _alertType: 'danger' + }) + ); + } + }, + actions: { /** * Handle the user intent to place this compliance component in edit mode @@ -634,26 +698,31 @@ export default Component.extend({ /** * If all validity checks are passed, invoke onSave action on controller */ - saveCompliance() { - const complianceList = get(this, policyComplianceEntitiesKey); - const idFieldsHaveValidLogicalType = this.checkEachEntityByLogicalType( - complianceList.filter(({ identifierType }) => fieldIdentifierTypeIds.includes(identifierType)), - [...genericLogicalTypes, ...idLogicalTypes] - ); + async saveCompliance() { + const setSaveFlag = (flag = false) => set(this, 'isSaving', flag); + // If fields are confirmed as unique we can proceed with saving compliance entities const saveConfirmed = this.confirmUnformattedFields(); // If user provides confirmation for unformatted fields or there are none, - // then check if id type fields have a specified logical type - // otherwise we should inform the user of missing field formats for id fields, which are not allowed + // then validate fields against expectations + // otherwise inform user of validation exception if (saveConfirmed) { - if (idFieldsHaveValidLogicalType) { - return this.whenRequestCompletes(get(this, 'onSave')(), { isSaving: true }); - } + try { + const isSaving = true; + const onSave = get(this, 'onSave'); + setSaveFlag(isSaving); + await this.validateFields(); - setProperties(this, { - _message: missingTypes, - _alertType: 'danger' - }); + return await this.whenRequestCompletes(onSave(), { isSaving }); + } catch (e) { + // Flag this dataset's data as problematic + if (e instanceof Error && e.message === complianceDataException) { + set(this, '_hasBadData', true); + window.scrollTo(0, 0); + } + } finally { + setSaveFlag(); + } } }, diff --git a/wherehows-web/app/constants/dataset-compliance.js b/wherehows-web/app/constants/dataset-compliance.js new file mode 100644 index 0000000000..925c02b268 --- /dev/null +++ b/wherehows-web/app/constants/dataset-compliance.js @@ -0,0 +1,17 @@ +/** + * Defines a map of values for the compliance policy on a dataset + * @type {Object} + */ +const compliancePolicyStrings = { + complianceDataException: 'Unexpected discrepancy in compliance data', + missingTypes: 'Looks like some fields may contain privacy data but do not have a specified `Field Format`?', + successUpdating: 'Your changes have been successfully saved!', + failedUpdating: 'Oops! We are having trouble updating this dataset at the moment.', + helpText: { + classification: + 'The default value is taken from go/dht and should be good enough in most cases. ' + + 'You can optionally override it if required by house security.' + } +}; + +export { compliancePolicyStrings }; diff --git a/wherehows-web/app/constants/index.js b/wherehows-web/app/constants/index.js index f115174e3d..aa18bdccdc 100644 --- a/wherehows-web/app/constants/index.js +++ b/wherehows-web/app/constants/index.js @@ -1,2 +1,3 @@ export * from 'wherehows-web/constants/metadata-acquisition'; export * from 'wherehows-web/constants/dataset-classification'; +export * from 'wherehows-web/constants/dataset-compliance'; diff --git a/wherehows-web/app/styles/components/notifications/_action-notifications.scss b/wherehows-web/app/styles/components/notifications/_action-notifications.scss index 8d7e99f768..fbdc312bc5 100644 --- a/wherehows-web/app/styles/components/notifications/_action-notifications.scss +++ b/wherehows-web/app/styles/components/notifications/_action-notifications.scss @@ -1,7 +1,11 @@ .post-action-notification { margin-bottom: 0; - display: inline-block; border-radius: 2px; padding: 5px; vertical-align: top; } + +/// Defines styles for a notification bar in an action bar +.action-footer-notification { + display: inline-block; +} diff --git a/wherehows-web/app/templates/components/dataset-compliance.hbs b/wherehows-web/app/templates/components/dataset-compliance.hbs index 9a03036282..81f4f54aea 100644 --- a/wherehows-web/app/templates/components/dataset-compliance.hbs +++ b/wherehows-web/app/templates/components/dataset-compliance.hbs @@ -15,11 +15,12 @@
{{#if isEditing}}
- @@ -32,7 +33,7 @@ {{#if _message}} - {{#if isEditing}} - {{json-upload receiveJsonFile=(action "onComplianceJsonUpload") class="secondary-actions__action"}} + {{#if (not _hasBadData)}} + {{json-upload receiveJsonFile=(action "onComplianceJsonUpload") class="secondary-actions__action"}} + {{/if}} {{else}}