Skip to content

Commit

Permalink
Load: Show warning or error if record id is mapped #679
Browse files Browse the repository at this point in the history
Give user a nice indicator that the load will most likely fail. We could have made this slightly better by not auto-mapping the record id for upsert, but there could be some edge-cases so the easy path was taken to show an error.

Mapping errors are considered "soft" as the uer can still continue if they so choose to.

resolves #679
  • Loading branch information
paustint committed Jan 3, 2024
1 parent 6be78a9 commit a9821e5
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ export const LoadRecords: FunctionComponent<LoadRecordsProps> = ({ featureFlags

useEffect(() => {
if (mappableFields && inputFileHeader) {
setFieldMapping(autoMapFields(inputFileHeader, mappableFields, binaryAttachmentBodyField));
setFieldMapping(autoMapFields(inputFileHeader, mappableFields, binaryAttachmentBodyField, loadType, externalId));
}
}, [mappableFields, inputFileHeader, loadType, setFieldMapping, binaryAttachmentBodyField]);
}, [mappableFields, inputFileHeader, loadType, setFieldMapping, binaryAttachmentBodyField, externalId]);

useEffect(() => {
setExternalIdFields(fields.filter((field) => field.name === 'Id' || field.externalId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,9 @@ export const LoadRecordsFieldMappingRow: FunctionComponent<LoadRecordsFieldMappi
comboboxProps={{
hideLabel: true,
label: 'Salesforce Fields',
hasError: fieldMappingItem.isDuplicateMappedField,
errorMessage: 'Each Salesforce field should only be mapped once',
errorMessageId: `${csvField}-${fieldMappingItem.targetField}-duplicate-field-error`,
hasError: !!fieldMappingItem.fieldErrorMsg,
errorMessage: fieldMappingItem.fieldErrorMsg,
errorMessageId: `${csvField}-${fieldMappingItem.targetField}-mapping-error`,
}}
items={fieldListItems}
selectedItemId={fieldMappingItem.targetField}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export interface FieldMappingItemBase {
fieldMetadata: Maybe<FieldWithRelatedEntities>;
relatedFieldMetadata?: FieldRelatedEntity;
isDuplicateMappedField?: boolean;
fieldErrorMsg?: string;
lookupOptionUseFirstMatch: NonExtIdLookupOption;
lookupOptionNullIfNoMatch: boolean;
isBinaryBodyField: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
import { LoadSavedMappingItem } from '../load-records.state';
import {
autoMapFields,
checkFieldsForMappingError,
checkForDuplicateFieldMappings,
initStaticFieldMappingItem,
loadFieldMappingFromSavedMapping,
Expand Down Expand Up @@ -134,7 +135,7 @@ export const LoadRecordsFieldMapping = memo<LoadRecordsFieldMappingProps>(
}
}
setWarningMessage(null);
}, [externalId, fieldMapping, loadType]);
}, [externalId, fieldMapping, isCustomMetadataObject, loadType]);

useNonInitialEffect(() => {
let tempVisibleHeaders = inputHeader;
Expand All @@ -158,7 +159,9 @@ export const LoadRecordsFieldMapping = memo<LoadRecordsFieldMappingProps>(
*
*/
function handleFieldMappingChange(csvField: string, fieldMappingItem: FieldMappingItem) {
setFieldMapping((fieldMapping) => checkForDuplicateFieldMappings({ ...fieldMapping, [csvField]: fieldMappingItem }));
setFieldMapping((fieldMapping) =>
checkFieldsForMappingError({ ...fieldMapping, [csvField]: fieldMappingItem }, loadType, externalId)
);
}

function handleAction(id: DropDownAction) {
Expand All @@ -170,7 +173,7 @@ export const LoadRecordsFieldMapping = memo<LoadRecordsFieldMappingProps>(
break;
case MAPPING_RESET:
setStaticRowHeaders([]);
setFieldMapping(autoMapFields(inputHeader, fields, binaryAttachmentBodyField));
setFieldMapping(autoMapFields(inputHeader, fields, binaryAttachmentBodyField, loadType, externalId));
setFilter(FILTER_ALL);
trackEvent(ANALYTICS_KEYS.load_MappingAutomationChanged, { action: id });
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,13 @@ export function getMaxBatchSize(apiMode: ApiMode): number {
* @param inputHeader
* @param fields
*/
export function autoMapFields(inputHeader: string[], fields: FieldWithRelatedEntities[], binaryBodyField: Maybe<string>): FieldMapping {
export function autoMapFields(
inputHeader: string[],
fields: FieldWithRelatedEntities[],
binaryBodyField: Maybe<string>,
loadType: InsertUpdateUpsertDelete,
externalId?: Maybe<string>
): FieldMapping {
const output: FieldMapping = {};
const fieldVariations: MapOf<FieldWithRelatedEntities> = {};
const fieldLabelVariations: MapOf<FieldWithRelatedEntities> = {};
Expand Down Expand Up @@ -244,7 +250,7 @@ export function autoMapFields(inputHeader: string[], fields: FieldWithRelatedEnt
}
});

return checkForDuplicateFieldMappings(output);
return checkFieldsForMappingError(output, loadType, externalId);
}

export function resetFieldMapping(inputHeader: string[]): FieldMapping {
Expand Down Expand Up @@ -332,6 +338,16 @@ export function loadFieldMappingFromSavedMapping(
return newMapping;
}

export function checkFieldsForMappingError(
fieldMapping: FieldMapping,
loadType: InsertUpdateUpsertDelete,
externalId?: Maybe<string>
): FieldMapping {
fieldMapping = checkForDuplicateFieldMappings(fieldMapping);
fieldMapping = checkForExternalIdFieldMappingsError(fieldMapping, loadType, externalId);
return fieldMapping;
}

export function checkForDuplicateFieldMappings(fieldMapping: FieldMapping): FieldMapping {
fieldMapping = { ...fieldMapping };
const mappedFieldFrequency = Object.values(fieldMapping)
Expand All @@ -345,9 +361,11 @@ export function checkForDuplicateFieldMappings(fieldMapping: FieldMapping): Fiel
}, {});
Object.keys(fieldMapping).forEach((key) => {
if (fieldMapping[key].targetField) {
const isDuplicateMappedField = (mappedFieldFrequency[fieldMapping[key].targetField || ''] || 0) > 1;
fieldMapping[key] = {
...fieldMapping[key],
isDuplicateMappedField: (mappedFieldFrequency[fieldMapping[key].targetField || ''] || 0) > 1,
isDuplicateMappedField,
fieldErrorMsg: isDuplicateMappedField ? 'Each Salesforce field should only be mapped once' : undefined,
};
} else {
fieldMapping[key] = { ...fieldMapping[key], isDuplicateMappedField: false };
Expand All @@ -356,6 +374,26 @@ export function checkForDuplicateFieldMappings(fieldMapping: FieldMapping): Fiel
return fieldMapping;
}

export function checkForExternalIdFieldMappingsError(
fieldMapping: FieldMapping,
loadType: InsertUpdateUpsertDelete,
externalId?: Maybe<string>
): FieldMapping {
if (loadType !== 'UPSERT' || !externalId || externalId === 'Id') {
return fieldMapping;
}
fieldMapping = { ...fieldMapping };
Object.keys(fieldMapping).forEach((key) => {
if (fieldMapping[key].targetField === 'Id' && !fieldMapping[key].fieldErrorMsg) {
fieldMapping[key] = {
...fieldMapping[key],
fieldErrorMsg: 'Including a Record Id in an upsert will cause the load to fail',
};
}
});
return fieldMapping;
}

function getExternalIdFieldsForSobjectsQuery(sobjects: string[]) {
const soql = composeQuery({
fields: [
Expand Down

0 comments on commit a9821e5

Please sign in to comment.