From 5430fe2d9b4ea943df64c9304afddb547039d6e4 Mon Sep 17 00:00:00 2001 From: simonyang08 Date: Sun, 6 Sep 2026 00:13:56 +0800 Subject: [PATCH] fix(import): exclude rowHeader and hidden columns from xlsx/array import mapping When importing data via table.import("xlsx") (or any array-of-arrays importer), Import.structureArrayToColumns was mapping imported cells to columns by raw index from this.table.getColumns(). That array includes the rowHeader column at index 0 and any hidden (visible:false) columns that the Export side strips out, so an export->import round-trip shifted all values by one slot when rowHeader was configured and silently dropped/wrote into hidden fields otherwise. Filter the column list to skip isRowHeader and !visible columns before index-based mapping so it mirrors what Export.columnVisCheck excludes. Regression tests added for both the rowHeader and hidden-column cases. Fixes https://github.com/tabulator-tables/tabulator/issues/4726 Signed-off-by: simonyang08 --- src/js/modules/Import/Import.js | 20 ++++--- test/unit/modules/Import.spec.js | 93 ++++++++++++++++++++++++++++++-- 2 files changed, 101 insertions(+), 12 deletions(-) diff --git a/src/js/modules/Import/Import.js b/src/js/modules/Import/Import.js index cc3763ec3..369431cc3 100644 --- a/src/js/modules/Import/Import.js +++ b/src/js/modules/Import/Import.js @@ -239,32 +239,38 @@ export default class Import extends Module{ structureArrayToColumns(parsedData){ var data = [], firstRow = this.transformHeader(parsedData[0]), - columns = this.table.getColumns(); - + allColumns = this.table.getColumns(), + //exclude rowHeader and hidden columns so the column-position-based mapping + //matches what was actually written to the export + //(see https://github.com/tabulator-tables/tabulator/issues/4726) + columns = allColumns.filter(function(column){ + return column && !column.isRowHeader && column.visible; + }); + //remove first row if it is the column names if(columns[0] && firstRow[0]){ if(columns[0].getDefinition().title === firstRow[0]){ parsedData.shift(); } } - + //convert row arrays to objects parsedData.forEach((rowData) => { var row = {}; rowData = this.transformData(rowData); - + rowData.forEach((value, index) => { var column = columns[index]; - + if(column){ row[column.getField()] = value; } }); - + data.push(row); }); - + return data; } diff --git a/test/unit/modules/Import.spec.js b/test/unit/modules/Import.spec.js index 607c52614..ed3634568 100644 --- a/test/unit/modules/Import.spec.js +++ b/test/unit/modules/Import.spec.js @@ -37,7 +37,8 @@ describe("Import module", () => { const mockColumnField = "columnField"; const mockColumn = { getField: jest.fn().mockReturnValue(mockColumnField), - getDefinition: jest.fn().mockReturnValue({ title: "Column" }) + getDefinition: jest.fn().mockReturnValue({ title: "Column" }), + visible: true, }; // Create mock module manager @@ -375,18 +376,100 @@ describe("Import module", () => { return [{ name: "Imported" }]; }); jest.spyOn(importMod, 'structureData').mockImplementation(data => data); - + // Set importFormat mockTable.options.importFormat = "csv"; - + // Trigger data load const result = await importMod.loadData("some csv data"); - + // Verify functions were called expect(importMod.lookupImporter).toHaveBeenCalled(); expect(importMod.structureData).toHaveBeenCalledWith([{ name: "Imported" }]); - + // Verify result expect(result).toEqual([{ name: "Imported" }]); }); + + // Regression: https://github.com/tabulator-tables/tabulator/issues/4726 + // rowHeader must not consume an import column slot. + it("should skip rowHeader column when mapping imported xlsx/array data to columns", () => { + jest.spyOn(importMod, 'transformHeader').mockImplementation(headers => headers); + jest.spyOn(importMod, 'transformData').mockImplementation(data => data); + + // columns: [rowHeader, name, rating] - rowHeader must be ignored + const rowHeaderCol = { + getField: jest.fn().mockReturnValue(undefined), + getDefinition: jest.fn().mockReturnValue({ title: "" }), + isRowHeader: true, + visible: true, + }; + const nameCol = { + getField: jest.fn().mockReturnValue("name"), + getDefinition: jest.fn().mockReturnValue({ title: "Name" }), + visible: true, + }; + const ratingCol = { + getField: jest.fn().mockReturnValue("rating"), + getDefinition: jest.fn().mockReturnValue({ title: "Rating" }), + visible: true, + }; + + mockTable.getColumns.mockReturnValue([rowHeaderCol, nameCol, ratingCol]); + + // Simulate a 2-column export: the rowHeader column is not written + // to the file, so only the two data columns appear in the array data. + const arrayData = [ + ["AB", 50], + ["CD", 55], + ]; + + const result = importMod.structureArrayToColumns(arrayData); + + // name should hold the first column ("AB", "CD"), rating the second (50, 55) + expect(result).toEqual([ + { name: "AB", rating: 50 }, + { name: "CD", rating: 55 }, + ]); + }); + + // Regression: https://github.com/tabulator-tables/tabulator/issues/4726 + // Hidden columns (visible:false) also break the index-based mapping on import. + it("should skip hidden columns when mapping imported xlsx/array data to columns", () => { + jest.spyOn(importMod, 'transformHeader').mockImplementation(headers => headers); + jest.spyOn(importMod, 'transformData').mockImplementation(data => data); + + // columns: [name, hiddenProgress, rating] + const nameCol = { + getField: jest.fn().mockReturnValue("name"), + getDefinition: jest.fn().mockReturnValue({ title: "Name" }), + visible: true, + }; + const hiddenCol = { + getField: jest.fn().mockReturnValue("progress"), + getDefinition: jest.fn().mockReturnValue({ title: "Progress" }), + visible: false, + }; + const ratingCol = { + getField: jest.fn().mockReturnValue("rating"), + getDefinition: jest.fn().mockReturnValue({ title: "Rating" }), + visible: true, + }; + + mockTable.getColumns.mockReturnValue([nameCol, hiddenCol, ratingCol]); + + // Hidden column was excluded on export, so import has only 2 columns + const arrayData = [ + ["AB", 50], + ["CD", 55], + ]; + + const result = importMod.structureArrayToColumns(arrayData); + + // hidden column must not absorb index 1; rating should receive it + expect(result).toEqual([ + { name: "AB", rating: 50 }, + { name: "CD", rating: 55 }, + ]); + }); });