Skip to content

Commit

Permalink
better handling of empty headers and dupes
Browse files Browse the repository at this point in the history
  • Loading branch information
theoephraim committed Feb 4, 2020
1 parent 87812c3 commit 30ec42c
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 9 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = {
'prefer-destructuring': 0,
'no-param-reassign': 0, // sometimes it's just much easier
'lines-between-class-members': 0, // grouping related one-liners can be nice
'no-continue': 0,
},
overrides: [
{ // extra jest related rules for tests
Expand Down
2 changes: 1 addition & 1 deletion docs/classes/google-spreadsheet.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Param|Type|Required|Description
---|---|---|---
`props`|Object|-|Object of all sheet properties
`props.sheetId`|Number<br>_positive int_|-|Sheet ID, cannot be chagned after setting<br>_easiest to just let google handle it_
`props.headers`|[String]|-|Sets the contents of the first row, to be used in row-based interactions
`props.headerValues`|[String]|-|Sets the contents of the first row, to be used in row-based interactions
`props.[more]`|...|-|_See [GoogleSpreadsheetWorksheet](classes/google-spreadsheet-worksheet#basic-document-properties) for more props_


Expand Down
6 changes: 3 additions & 3 deletions lib/GoogleSpreadsheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,9 @@ class GoogleSpreadsheet {
const newSheetId = response.properties.sheetId;
const newSheet = this.sheetsById[newSheetId];

if (properties.headers) {
await newSheet.setHeaderRow(properties.headers);
}
// allow it to work with `.headers` but `.headerValues` is the real prop
if (properties.headers) { await newSheet.setHeaderRow(properties.headers); }
if (properties.headerValues) { await newSheet.setHeaderRow(properties.headerValues); }

return newSheet;
}
Expand Down
1 change: 1 addition & 0 deletions lib/GoogleSpreadsheetRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class GoogleSpreadsheetRow {

for (let i = 0; i < this._sheet.headerValues.length; i++) {
const propName = this._sheet.headerValues[i];
if (!propName) continue; // skip empty header
Object.defineProperty(this, propName, {
get: () => this._rawData[i],
set: (newVal) => { this._rawData[i] = newVal; },
Expand Down
21 changes: 18 additions & 3 deletions lib/GoogleSpreadsheetWorksheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ const GoogleSpreadsheetCell = require('./GoogleSpreadsheetCell');

const { getFieldMask, columnToLetter, letterToColumn } = require('./utils');

function checkForDuplicateHeaders(headers) {
// check for duplicate headers
const checkForDupes = _.groupBy(headers); // { c1: ['c1'], c2: ['c2', 'c2' ]}
_.each(checkForDupes, (grouped, header) => {
if (!header) return; // empty columns are skipped, so multiple is ok
if (grouped.length > 1) {
throw new Error(`Duplicate header detected: "${header}". Please make sure all non-empty headers are unique`);
}
});
}

class GoogleSpreadsheetWorksheet {
constructor(parentSpreadsheet, { properties, data }) {
this._spreadsheet = parentSpreadsheet; // the parent GoogleSpreadsheet instance
Expand Down Expand Up @@ -39,7 +50,7 @@ class GoogleSpreadsheetWorksheet {

resetLocalCache(dataOnly) {
if (!dataOnly) this._rawProperties = null;
this.headers = null;
this.headerValues = null;
this._cells = [];
}

Expand Down Expand Up @@ -266,16 +277,20 @@ class GoogleSpreadsheetWorksheet {


// ROW BASED FUNCTIONS ///////////////////////////////////////////////////////////////////////////

async loadHeaderRow() {
const rows = await this.getCellsInRange(`A1:${this.lastColumnLetter}1`);
this.headerValues = rows[0];
this.headerValues = _.map(rows[0], (header) => header.trim());
checkForDuplicateHeaders(this.headerValues);
}

async setHeaderRow(headerValues) {
if (!headerValues) return;
if (headerValues.length > this.columnCount) {
throw new Error(`Sheet is not large enough to fit ${headerValues.length} columns. Resize the sheet first.`);
}
const trimmedHeaderValues = _.map(headerValues, (h) => h.trim());
checkForDuplicateHeaders(trimmedHeaderValues);

const response = await this._spreadsheet.axios.request({
method: 'put',
Expand All @@ -287,7 +302,7 @@ class GoogleSpreadsheetWorksheet {
data: {
range: `${this.a1SheetName}!A1`,
majorDimension: 'ROWS',
values: [headerValues],
values: [trimmedHeaderValues],
},
});
this.headerValues = response.data.updatedData.values[0];
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"author": "Theo Ephraim <theozero@gmail.com> (https://theoephraim.com)",
"name": "google-spreadsheet",
"description": "Google Sheets API (v4) -- simple interface to read/write data and manage sheets",
"version": "3.0.2",
"version": "3.0.3",
"license": "Unlicense",
"keywords": [
"google spreadsheets",
Expand Down
2 changes: 1 addition & 1 deletion test/manage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ describe('Managing doc info and sheets', () => {
const copiedSheet = docs.public.sheetsByIndex.splice(-1)[0];
expect(copiedSheet.title).toBe(`Copy of ${sheet.title}`);
await copiedSheet.loadHeaderRow();
expect(copiedSheet.headers).toEqual(sheet.headers);
expect(copiedSheet.headerValues).toEqual(sheet.headerValues);
await copiedSheet.delete();
});
});
Expand Down
34 changes: 34 additions & 0 deletions test/rows.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,38 @@ describe('Row-based operations', () => {
);
});
});

describe('header validation and cleanup', () => {
beforeAll(async () => {
sheet.loadCells('A1:E1');
});

it('allows empty headers', async () => {
await sheet.setHeaderRow(['', 'col1', '', 'col2']);
rows = await sheet.getRows();
expect(rows[0]['']).toBeUndefined();
expect(rows[0].col1).not.toBeUndefined();
});

it('trims each header', async () => {
await sheet.setHeaderRow([' col1 ', ' something with spaces ']);
rows = await sheet.getRows();
expect(rows[0].col1).not.toBeUndefined();
expect(rows[0]['something with spaces']).not.toBeUndefined();
});

it('throws an error if setting duplicate headers', async () => {
await expect(sheet.setHeaderRow(['col1', 'col1'])).rejects.toThrow();
});

it('throws an error if duplicate headers already exist', async () => {
await sheet.loadCells('A1:C1');
sheet.getCellByA1('A1').value = 'col1';
sheet.getCellByA1('B1').value = 'col1';
sheet.getCellByA1('C1').value = 'col2';
await sheet.saveUpdatedCells();
sheet.resetLocalCache(true); // forget the header values
await expect(sheet.getRows()).rejects.toThrow();
});
});
});

0 comments on commit 30ec42c

Please sign in to comment.