Skip to content

Comments

fix: bug where importDataRecords would overwrite columnDefs#330

Merged
joaquimds merged 1 commit intomainfrom
fix/import-data-records-overwrite-columndefs
Feb 19, 2026
Merged

fix: bug where importDataRecords would overwrite columnDefs#330
joaquimds merged 1 commit intomainfrom
fix/import-data-records-overwrite-columndefs

Conversation

@joaquimds
Copy link
Member

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in the importDataRecords job where column definitions were being overwritten on each batch instead of being accumulated across all batches.

Changes:

  • Fixed columnDefs accumulator initialization to start with existing columns and persist outside the batch loop
  • Improved Google Sheets row counting accuracy by expanding the column range from A:A to A:Z

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/server/jobs/importDataRecords.ts Moved columnDefsAccumulator initialization outside the loop and updateDataSource call after all batches complete, fixing the overwrite bug
src/server/adaptors/googlesheets.ts Changed getRecordCount range from A:A to A:Z to count rows with data in any of the first 26 columns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

async getRecordCount(): Promise<number | null> {
try {
const url = `https://sheets.googleapis.com/v4/spreadsheets/${this.spreadsheetId}/values/${encodeURIComponent(this.sheetName)}!A:A`;
const url = `https://sheets.googleapis.com/v4/spreadsheets/${this.spreadsheetId}/values/${encodeURIComponent(this.sheetName)}!A:Z`;
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing from A:A to A:Z improves accuracy by counting rows with data in any of the first 26 columns, rather than only rows with data in column A. However, this still won't count rows that only have data beyond column Z. For better consistency with fetchAll() (which fetches all columns), consider using a larger range like A:ZZ or document this 26-column limitation.

Suggested change
const url = `https://sheets.googleapis.com/v4/spreadsheets/${this.spreadsheetId}/values/${encodeURIComponent(this.sheetName)}!A:Z`;
const url = `https://sheets.googleapis.com/v4/spreadsheets/${this.spreadsheetId}/values/${encodeURIComponent(this.sheetName)}!A:ZZ`;

Copilot uses AI. Check for mistakes.
@joaquimds joaquimds force-pushed the fix/import-data-records-overwrite-columndefs branch from d8be06a to fee3d55 Compare February 19, 2026 15:28
@joaquimds joaquimds merged commit 770de27 into main Feb 19, 2026
1 check passed
@joaquimds joaquimds deleted the fix/import-data-records-overwrite-columndefs branch February 19, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant