Skip to content

Conversation

@apoddubn
Copy link
Contributor

@apoddubn apoddubn commented Jul 26, 2024

Please explain how to summarize this PR for the Changelog:

Fixes an issue where the presence of empty columns would shift the extracted cell values into other columns. While we remove null values for the purpose of header detection, we need to use the original data when constructing the actual headers.

Tell code reviewer how and what to test:

An xlsx file with an empty column between filled columns should have the filled columns extracted correctly.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 26, 2024

Walkthrough

The recent updates to the convertSheet function in parser.ts enhance how column headers are determined based on the headerSelectionEnabled flag. By introducing the headerRowValues variable, the function now directly retrieves values from the header row when header selection is disabled. This adjustment improves data extraction accuracy and ensures that the correct values are processed downstream, particularly in scenarios with empty columns.

Changes

Files Change Summary
plugins/xlsx-extractor/src/parser.ts Modified convertSheet to utilize headerRowValues for column headers when headerSelectionEnabled is false, enhancing data accuracy.
.changeset/short-apples-flash.md Introduced a patch for the @flatfile/plugin-xlsx-extractor to improve handling of empty columns during record extraction, increasing robustness.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Parser
    participant DataStore

    User->>Parser: Request to convert sheet
    Parser->>DataStore: Fetch rows
    alt headerSelectionEnabled is true
        Parser->>Parser: Use selected headers
    else headerSelectionEnabled is false
        Parser->>Parser: Use headerRowValues from rows
    end
    Parser->>User: Return converted data
Loading

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 011dade and b155f9a.

Files selected for processing (1)
  • .changeset/short-apples-flash.md (1 hunks)
Additional comments not posted (5)
.changeset/short-apples-flash.md (5)

1-1: LGTM!

The front matter block starts correctly.


2-2: LGTM!

The package and release type are specified correctly.


3-3: LGTM!

The front matter block ends correctly.


4-4: LGTM!

The empty line is appropriate for readability.


5-5: LGTM!

The description of the fix is clear and concise.

@bangarang
Copy link
Collaborator

Just needs a changeset!

@apoddubn apoddubn enabled auto-merge (squash) July 26, 2024 22:03
@apoddubn apoddubn merged commit 611b3e3 into main Jul 26, 2024
@apoddubn apoddubn deleted the fix-null-header-extraction branch July 26, 2024 22:05
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.

3 participants