fix: trim leading/trailing whitespace from Excel header cells on upload
Header matching in searchDataInExcel() was case-insensitive but not whitespace-tolerant, so a header like " SOME_CHAR" (an easy defect to pick up via copy-paste from another spreadsheet/system) was reported as a missing column and aborted the whole upload. - trim() the cell value before matching, alongside the existing toLowerCase() normalization - add spreadsheet-util.spec.ts (no prior spec file existed) covering the trimmed match and a regression guard that a genuinely different header is still correctly reported missing - add a Cypress case + fixture in excel.cy.ts covering the same defect end to end
This commit is contained in:
@@ -386,6 +386,19 @@ context('excel tests: ', function () {
|
||||
})
|
||||
})
|
||||
|
||||
it('24 | Uploads Excel with leading whitespace in header row (should still succeed)', (done) => {
|
||||
openTableFromTree(libraryToOpenIncludes, 'mpe_x_test')
|
||||
|
||||
// Every header except the first has a leading space, matching a
|
||||
// real-world defect from copy-pasting between spreadsheets/systems.
|
||||
// Header matching must tolerate
|
||||
// this rather than reporting the columns missing and aborting.
|
||||
attachExcelFile('leading_whitespace_header_excel.xlsx', () => {
|
||||
submitExcel()
|
||||
rejectExcel(done)
|
||||
})
|
||||
})
|
||||
|
||||
// Large files break Cypress
|
||||
|
||||
// it ('? | Uploads Excel with size of 5MB', (done) => {
|
||||
|
||||
Binary file not shown.
@@ -0,0 +1,78 @@
|
||||
import { BehaviorSubject } from 'rxjs'
|
||||
import * as XLSX from '@sheet/crypto'
|
||||
import { SpreadsheetUtil } from './spreadsheet-util'
|
||||
import { LicenceState } from 'src/app/models/LicenceState'
|
||||
import { ParseParams } from 'src/app/models/ParseParams.interface'
|
||||
import { SearchDataExcelResult } from 'src/app/models/SearchDataExcelResult.interface'
|
||||
|
||||
describe('SpreadsheetUtil - header matching', () => {
|
||||
// licenceState is unused by searchDataInExcel, so an empty stub is fine.
|
||||
const buildSpreadsheetUtil = () =>
|
||||
new SpreadsheetUtil({
|
||||
licenceState: new BehaviorSubject<LicenceState>({} as LicenceState)
|
||||
})
|
||||
|
||||
const buildWorkbook = (headerRow: string[], dataRow: any[]) => {
|
||||
const ws = XLSX.utils.aoa_to_sheet([headerRow, dataRow])
|
||||
const wb = XLSX.utils.book_new()
|
||||
|
||||
XLSX.utils.book_append_sheet(wb, ws, 'Sheet1')
|
||||
|
||||
return wb
|
||||
}
|
||||
|
||||
// searchDataInExcel only reads headerArray/headerPks off ParseParams; the
|
||||
// other required fields (dcValidator, etc.) are irrelevant here, so a
|
||||
// minimal cast object stands in for a full ParseParams.
|
||||
const buildParseParams = (): ParseParams =>
|
||||
({
|
||||
headerArray: ['PRIMARY_KEY_FIELD', 'SOME_CHAR', 'SOME_NUM'],
|
||||
headerPks: ['PRIMARY_KEY_FIELD']
|
||||
}) as any as ParseParams
|
||||
|
||||
// searchDataInExcel is private; there's no existing precedent in this
|
||||
// codebase for testing private methods, so this casts through `any` to
|
||||
// reach it directly rather than exercising it via the public
|
||||
// parseSpreadsheetFile (which would need a real File/FileReader).
|
||||
const searchDataInExcel = (
|
||||
spreadsheetUtil: SpreadsheetUtil,
|
||||
wb: XLSX.WorkBook,
|
||||
parseParams: ParseParams
|
||||
): SearchDataExcelResult =>
|
||||
(spreadsheetUtil as any).searchDataInExcel(wb, parseParams)
|
||||
|
||||
it('matches headers with leading whitespace against the expected clean header names', () => {
|
||||
const wb = buildWorkbook(
|
||||
['PRIMARY_KEY_FIELD', ' SOME_CHAR', ' SOME_NUM'],
|
||||
[1, 'a value', 42]
|
||||
)
|
||||
|
||||
const result = searchDataInExcel(
|
||||
buildSpreadsheetUtil(),
|
||||
wb,
|
||||
buildParseParams()
|
||||
)
|
||||
|
||||
expect(result.missing).toBeUndefined()
|
||||
expect(result.found).toBeDefined()
|
||||
expect(result.found?.headers).toContain('some_char')
|
||||
expect(result.found?.headers).toContain('some_num')
|
||||
})
|
||||
|
||||
it('still reports a genuinely different header as missing (no over-broadening)', () => {
|
||||
const wb = buildWorkbook(
|
||||
['PRIMARY_KEY_FIELD', ' SOME_CHARX', ' SOME_NUM'],
|
||||
[1, 'a value', 42]
|
||||
)
|
||||
|
||||
const result = searchDataInExcel(
|
||||
buildSpreadsheetUtil(),
|
||||
wb,
|
||||
buildParseParams()
|
||||
)
|
||||
|
||||
expect(result.found).toBeUndefined()
|
||||
expect(result.missing).toBeDefined()
|
||||
expect(result.missing?.[0].missingHeaders).toContain('SOME_CHAR')
|
||||
})
|
||||
})
|
||||
@@ -608,7 +608,11 @@ export class SpreadsheetUtil {
|
||||
|
||||
// If the cell does not have `v` property we ignore it, those are metadata properties
|
||||
if (cellValue && typeof cellValue === 'string') {
|
||||
const potentialHeader = cellValue.toLowerCase()
|
||||
// .trim(): header cells can pick up incidental leading/trailing
|
||||
// whitespace from copy-pasting between spreadsheets/systems — treat
|
||||
// that the same as the existing case-insensitive matching below,
|
||||
// not as a genuinely different (missing) column.
|
||||
const potentialHeader = cellValue.trim().toLowerCase()
|
||||
const headerIndex = csvArrayHeadersLower.indexOf(potentialHeader)
|
||||
|
||||
if (headerIndex > -1) {
|
||||
|
||||
Reference in New Issue
Block a user