diff --git a/.changeset/fix-empty-row-save.md b/.changeset/fix-empty-row-save.md new file mode 100644 index 0000000..52eac11 --- /dev/null +++ b/.changeset/fix-empty-row-save.md @@ -0,0 +1,5 @@ +--- +"google-spreadsheet": patch +--- + +Fix crash when saving a row with all empty values, and ensure empty cells always return `''` instead of `undefined` diff --git a/src/lib/GoogleSpreadsheetRow.ts b/src/lib/GoogleSpreadsheetRow.ts index 48fb400..c49de07 100644 --- a/src/lib/GoogleSpreadsheetRow.ts +++ b/src/lib/GoogleSpreadsheetRow.ts @@ -13,7 +13,15 @@ export class GoogleSpreadsheetRow = Record = Record(); - this._rawData = data.updatedData.values[0]; + this._rawData = data.updatedData.values?.[0] || []; + this._padRawData(); } /** delete this row */ diff --git a/src/test/cells.test.ts b/src/test/cells.test.ts index 804cce4..d651e14 100644 --- a/src/test/cells.test.ts +++ b/src/test/cells.test.ts @@ -274,6 +274,109 @@ describe('Cell-based operations', () => { }); }); + describe('merge and unmerge operations', () => { + beforeEach(async () => { + sheet.resetLocalCache(true); + }); + + it('merges all cells (MERGE_ALL)', async () => { + // set up some values first + await sheet.loadCells('A2:B3'); + sheet.getCell(1, 0).value = 'top-left'; + sheet.getCell(1, 1).value = 'top-right'; + sheet.getCell(2, 0).value = 'bot-left'; + sheet.getCell(2, 1).value = 'bot-right'; + await sheet.saveUpdatedCells(); + + await sheet.mergeCells({ + startRowIndex: 1, + endRowIndex: 3, + startColumnIndex: 0, + endColumnIndex: 2, + }); + + await sheet.loadCells('A2:B3'); + expect(sheet.getCell(1, 0).value).toBe('top-left'); // top-left kept + expect(sheet.getCell(1, 1).value).toBeNull(); // merged away + expect(sheet.getCell(2, 0).value).toBeNull(); + expect(sheet.getCell(2, 1).value).toBeNull(); + }); + + it('merges cells in column direction (MERGE_COLUMNS)', async () => { + await sheet.loadCells('C2:D3'); + sheet.getCell(1, 2).value = 'C2'; + sheet.getCell(1, 3).value = 'D2'; + sheet.getCell(2, 2).value = 'C3'; + sheet.getCell(2, 3).value = 'D3'; + await sheet.saveUpdatedCells(); + + await sheet.mergeCells({ + startRowIndex: 1, + endRowIndex: 3, + startColumnIndex: 2, + endColumnIndex: 4, + }, 'MERGE_COLUMNS'); + + await sheet.loadCells('C2:D3'); + expect(sheet.getCell(1, 2).value).toBe('C2'); // top of each column kept + expect(sheet.getCell(1, 3).value).toBe('D2'); + expect(sheet.getCell(2, 2).value).toBeNull(); // merged away + expect(sheet.getCell(2, 3).value).toBeNull(); + }); + + it('merges cells in row direction (MERGE_ROWS)', async () => { + await sheet.loadCells('E2:F3'); + sheet.getCell(1, 4).value = 'E2'; + sheet.getCell(1, 5).value = 'F2'; + sheet.getCell(2, 4).value = 'E3'; + sheet.getCell(2, 5).value = 'F3'; + await sheet.saveUpdatedCells(); + + await sheet.mergeCells({ + startRowIndex: 1, + endRowIndex: 3, + startColumnIndex: 4, + endColumnIndex: 6, + }, 'MERGE_ROWS'); + + await sheet.loadCells('E2:F3'); + expect(sheet.getCell(1, 4).value).toBe('E2'); // left of each row kept + expect(sheet.getCell(1, 5).value).toBeNull(); // merged away + expect(sheet.getCell(2, 4).value).toBe('E3'); + expect(sheet.getCell(2, 5).value).toBeNull(); + }); + + it('can unmerge cells and write to previously merged cells', async () => { + await sheet.loadCells('G2:H2'); + sheet.getCell(1, 6).value = 'G2'; + sheet.getCell(1, 7).value = 'H2'; + await sheet.saveUpdatedCells(); + + // merge + await sheet.mergeCells({ + startRowIndex: 1, + endRowIndex: 2, + startColumnIndex: 6, + endColumnIndex: 8, + }); + await sheet.loadCells('G2:H2'); + expect(sheet.getCell(1, 6).value).toBe('G2'); + expect(sheet.getCell(1, 7).value).toBeNull(); + + // unmerge + await sheet.unmergeCells({ + startRowIndex: 1, + endRowIndex: 2, + startColumnIndex: 6, + endColumnIndex: 8, + }); + await sheet.loadCells('G2:H2'); + sheet.getCell(1, 7).value = 'restored'; + await sheet.saveUpdatedCells(); + expect(sheet.getCell(1, 7).value).toBe('restored'); + }); + }); + describe.todo('cell formatting', () => { // TODO: add tests! // - set the background color twice, conflicts b/w backgroundColor and backgroundColorStyle diff --git a/src/test/data-operations.test.ts b/src/test/data-operations.test.ts index d86b035..ee3a7b6 100644 --- a/src/test/data-operations.test.ts +++ b/src/test/data-operations.test.ts @@ -47,10 +47,10 @@ describe('Data operations - rows, columns, and ranges', () => { // header row expect(rows[0].get('a')).toEqual('a1'); expect(rows[0].get('b')).toEqual('b1'); - expect(rows[1].get('a')).toBeUndefined(); - expect(rows[1].get('b')).toBeUndefined(); - expect(rows[2].get('a')).toBeUndefined(); - expect(rows[2].get('b')).toBeUndefined(); + expect(rows[1].get('a')).toEqual(''); + expect(rows[1].get('b')).toEqual(''); + expect(rows[2].get('a')).toEqual(''); + expect(rows[2].get('b')).toEqual(''); expect(rows[3].get('a')).toEqual('a2'); expect(rows[3].get('b')).toEqual('b2'); }); diff --git a/src/test/rows.test.ts b/src/test/rows.test.ts index 3369714..81a485b 100644 --- a/src/test/rows.test.ts +++ b/src/test/rows.test.ts @@ -215,6 +215,56 @@ describe('Row-based operations', () => { expect(row.get('col3')).toEqual('3'); // it evaluates the formula and formats as a string }); + it('can save a row with trailing empty values', async () => { + rows = await sheet.getRows(); + row = rows[0]; + row.set('numbers', 'hello'); + row.set('letters', ''); + row.set('col1', ''); + row.set('col2', ''); + row.set('col3', ''); + await row.save(); + expect(row.get('numbers')).toBe('hello'); + // trailing empty values should be empty strings, not undefined + expect(row.get('letters')).toBe(''); + expect(row.get('col1')).toBe(''); + expect(row.get('col2')).toBe(''); + expect(row.get('col3')).toBe(''); + }); + + it('empty trailing values persist correctly after re-fetching', async () => { + rows = await sheet.getRows(); + row = rows[0]; + expect(row.get('numbers')).toBe('hello'); + expect(row.get('letters')).toBe(''); + expect(row.get('col3')).toBe(''); + }); + + it('can clear all values in a row by saving empty strings', async () => { + rows = await sheet.getRows(); + row = rows[0]; + const nonEmptyHeaders = HEADERS.filter((h) => h); + _.each(nonEmptyHeaders, (header) => row.set(header, '')); + await row.save(); + _.each(nonEmptyHeaders, (header) => { + expect(row.get(header)).toBe(''); + }); + // toObject should also return empty strings, not undefined + const obj = row.toObject(); + _.each(nonEmptyHeaders, (header) => { + expect(obj[header]).toBe(''); + }); + }); + + it('all-empty row persists correctly after re-fetching', async () => { + rows = await sheet.getRows(); + row = rows[0]; + const nonEmptyHeaders = HEADERS.filter((h) => h); + _.each(nonEmptyHeaders, (header) => { + expect(row.get(header)).toBe(''); + }); + }); + describe('encoding and odd characters', () => { _.each( { @@ -234,83 +284,6 @@ describe('Row-based operations', () => { }); }); - // TODO: Move to cells.test.js because mergeCells and unmergeCells are really cell operations - // but they were implemented using the existing data we have here in the rows tests - // so we'll leave them here for now - describe('merge and unmerge operations', () => { - beforeAll(async () => { - await sheet.loadCells('A1:H2'); - }); - - const range = { - startColumnIndex: 0, - endColumnIndex: 2, - }; - - it('merges all cells', async () => { - await sheet.mergeCells({ - startRowIndex: 2, - endRowIndex: 4, - ...range, - }); - const mergedRows = await sheet.getRows(); - expect(mergedRows[1].get('numbers')).toBe('2'); - expect(mergedRows[1].get('letters')).toBe(undefined); - expect(mergedRows[2].get('numbers')).toBe(undefined); - expect(mergedRows[2].get('letters')).toBe(undefined); - }); - - it('merges all cells in column direction', async () => { - await sheet.mergeCells({ - startRowIndex: 4, - endRowIndex: 6, - ...range, - }, 'MERGE_COLUMNS'); - const mergedRows = await sheet.getRows(); - expect(mergedRows[3].get('numbers')).toBe('4'); - expect(mergedRows[3].get('letters')).toBe('E'); - expect(mergedRows[4].get('numbers')).toBe(undefined); - expect(mergedRows[4].get('letters')).toBe(undefined); - }); - - it('merges all cells in row direction', async () => { - await sheet.mergeCells({ - startRowIndex: 6, - endRowIndex: 8, - ...range, - }, 'MERGE_ROWS'); - const mergedRows = await sheet.getRows(); - expect(mergedRows[5].get('numbers')).toBe('6'); - expect(mergedRows[5].get('letters')).toBe(undefined); - expect(mergedRows[6].get('numbers')).toBe('7'); - expect(mergedRows[6].get('letters')).toBe(undefined); - }); - - it('unmerges cells', async () => { - await sheet.mergeCells({ - startRowIndex: 8, - endRowIndex: 9, - ...range, - }); - const mergedRows = await sheet.getRows(); - expect(mergedRows[7].get('numbers')).toBe('8'); - expect(mergedRows[7].get('letters')).toBe(undefined); - mergedRows[7].set('letters', 'Z'); - await mergedRows[7].save(); - expect(mergedRows[7].get('numbers')).toBe('8'); - expect(mergedRows[7].get('letters')).toBe(undefined); - await sheet.unmergeCells({ - startRowIndex: 8, - endRowIndex: 9, - ...range, - }); - mergedRows[7].set('letters', 'Z'); - await mergedRows[7].save(); - expect(mergedRows[7].get('numbers')).toBe('8'); - expect(mergedRows[7].get('letters')).toBe('Z'); - }); - }); - describe('header validation and cleanup', () => { let rows: GoogleSpreadsheetRow[]; beforeAll(async () => {