From de429da0a5bff762e6ea97281277df8593db066b Mon Sep 17 00:00:00 2001 From: kaulfield23 Date: Sun, 1 Sep 2024 13:40:52 +0200 Subject: [PATCH 1/2] Tweak predictProblems logic, add test for it --- .../utils/problems/predictProblems.spec.ts | 34 +++++++++++++++++++ .../import/utils/problems/predictProblems.ts | 29 +++++++++------- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/features/import/utils/problems/predictProblems.spec.ts b/src/features/import/utils/problems/predictProblems.spec.ts index bcf79e791d..9464e6dedf 100644 --- a/src/features/import/utils/problems/predictProblems.spec.ts +++ b/src/features/import/utils/problems/predictProblems.spec.ts @@ -446,6 +446,40 @@ describe('predictProblem()', () => { ]); }); + it('returns problem when either the first name or last name value is missing', () => { + const sheet = makeFullSheet({ + columns: [ + { + field: 'first_name', + kind: ColumnKind.FIELD, + selected: true, + }, + { + field: 'last_name', + kind: ColumnKind.FIELD, + selected: true, + }, + { + field: 'email', + kind: ColumnKind.FIELD, + selected: true, + }, + ], + rows: [ + { data: ['Clara', 'Zetkin', 'zetkin@example.com'] }, + { data: ['John', '', 'john@example.com'] }, + ], + }); + + const problems = predictProblems(sheet, 'SE', []); + expect(problems).toEqual([ + { + indices: [1], + kind: ImportProblemKind.MISSING_ID_AND_NAME, + }, + ]); + }); + it('returns no problems when date column is configured and all cells have a value', () => { const sheet = makeFullSheet({ columns: [ diff --git a/src/features/import/utils/problems/predictProblems.ts b/src/features/import/utils/problems/predictProblems.ts index 96bec2c9a0..506d16f63a 100644 --- a/src/features/import/utils/problems/predictProblems.ts +++ b/src/features/import/utils/problems/predictProblems.ts @@ -46,18 +46,6 @@ export function predictProblems( (col) => col.kind == ColumnKind.FIELD && col.field == 'last_name' ); - if (!sheetHasId) { - if (sheetHasFirstName && sheetHasLastName) { - problems.push({ - kind: ImportProblemKind.UNCONFIGURED_ID, - }); - } else { - problems.push({ - kind: ImportProblemKind.UNCONFIGURED_ID_AND_NAME, - }); - } - } - function accumulateFieldProblem(field: string, row: number) { const existing = problemByField[field]; if (existing) { @@ -75,6 +63,7 @@ export function predictProblems( customFields.forEach((field) => { customFieldsBySlug[field.slug] = field; }); + let isMissingValue = false; sheet.rows.forEach((row, index) => { const rowIndex = sheet.firstRowIsHeaders ? index - 1 : index; @@ -133,11 +122,13 @@ export function predictProblems( } } hadImpact = true; + } else { + isMissingValue = true; } } }); - if (sheetHasId && sheetHasFirstName && sheetHasLastName) { + if (sheetHasFirstName && sheetHasLastName) { if (!rowHasId && (!rowHasFirstName || !rowHasLastName)) { if (!rowProblemByKind[ImportProblemKind.MISSING_ID_AND_NAME]) { rowProblemByKind[ImportProblemKind.MISSING_ID_AND_NAME] = { @@ -153,6 +144,18 @@ export function predictProblems( } }); + if (!sheetHasId && !isMissingValue) { + if (sheetHasFirstName && sheetHasLastName) { + problems.push({ + kind: ImportProblemKind.UNCONFIGURED_ID, + }); + } else { + problems.push({ + kind: ImportProblemKind.UNCONFIGURED_ID_AND_NAME, + }); + } + } + if (!hadImpact) { problems.push({ kind: ImportProblemKind.NO_IMPACT, From 5a51a43d62e5ddcd7dd859b7bda73319d39f90ce Mon Sep 17 00:00:00 2001 From: kaulfield23 Date: Sun, 22 Sep 2024 11:28:22 +0200 Subject: [PATCH 2/2] Add more condition to ensure missingValueInName to be true only for names --- src/features/import/utils/problems/predictProblems.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/features/import/utils/problems/predictProblems.ts b/src/features/import/utils/problems/predictProblems.ts index 506d16f63a..7c645acc69 100644 --- a/src/features/import/utils/problems/predictProblems.ts +++ b/src/features/import/utils/problems/predictProblems.ts @@ -63,7 +63,7 @@ export function predictProblems( customFields.forEach((field) => { customFieldsBySlug[field.slug] = field; }); - let isMissingValue = false; + let missingValueInName = false; sheet.rows.forEach((row, index) => { const rowIndex = sheet.firstRowIsHeaders ? index - 1 : index; @@ -123,7 +123,12 @@ export function predictProblems( } hadImpact = true; } else { - isMissingValue = true; + if ( + column.kind == ColumnKind.FIELD && + (column.field == 'first_name' || column.field == 'last_name') + ) { + missingValueInName = true; + } } } }); @@ -144,7 +149,7 @@ export function predictProblems( } }); - if (!sheetHasId && !isMissingValue) { + if (!sheetHasId && !missingValueInName) { if (sheetHasFirstName && sheetHasLastName) { problems.push({ kind: ImportProblemKind.UNCONFIGURED_ID,