-
Notifications
You must be signed in to change notification settings - Fork 11
fix(export-workbook): export all schema fields even when values missing #871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(export-workbook): export all schema fields even when values missing #871
Conversation
… record values are missing The export plugin was iterating over Object.keys(record.values) which only included fields that were present in the record values. This caused columns defined in the sheet schema but missing from record values to be omitted from the export. Changed to iterate over sheet.config.fields instead, ensuring all schema fields are exported. Missing values are now exported as empty cells. Fixes: IA-294777 Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…exported Updated implementation to export the union of: 1. All schema fields (fixes missing columns issue) 2. Additional fields present in record values (preserves allowAdditionalFields) This ensures schema columns always appear while maintaining backward compatibility for users with additional fields. Co-Authored-By: [email protected] <[email protected]>
|
📝 Documentation updates detected! New suggestion: Add changelog entry for export-workbook schema fields fix |
Optimized performance by computing schemaKeys and schemaFieldKeys once per sheet instead of once per record. This reduces O(n*m) overhead to O(n) where n is number of records and m is number of fields. Co-Authored-By: [email protected] <[email protected]>
CI E2E Test Failures AnalysisThe E2E test failures appear to be unrelated flaky tests rather than issues caused by this PR. Evidence: Test Results Across 3 CI Runs:
Why These Are Unrelated:
Recommendation:These E2E failures appear to be environmental issues (network timeouts, API availability) rather than functional regressions. The export-workbook plugin changes are isolated and all relevant unit tests pass. Would appreciate maintainer confirmation if these E2E flakes are a known issue in CI. |
|
Closing this PR to investigate the root cause further. After examining the customer's Excel file, I found that the columns (Employee ID, Status, End date) ARE present in the export but have empty cells. This suggests the issue is not about missing columns, but rather about data not being populated. The customer reports that data IS visible in the review screen but NOT in the exported file, which points to a potential data layer mismatch between what the UI displays and what the export plugin accesses. Will investigate further and provide findings before proposing any code changes. |
Summary
Fixes IA-294777 where columns defined in the sheet schema but missing from record values were completely omitted from Excel exports.
Root Cause: The export plugin iterated over
Object.keys(record.values), which only included fields present in the record values object. If a field was defined in the schema but the API didn't return it in record values (e.g., null/empty values), that column would be missing from the export entirely.Solution: Changed to iterate over the union of:
allowAdditionalFieldsfunctionalityMissing values are now exported as empty cells rather than omitting the column entirely.
Performance Optimization: Moved schema keys computation outside the record loop to avoid O(n*m) overhead where n = records, m = fields.
Please explain how to summarize this PR for the Changelog:
Fixed issue where columns defined in sheet schema but missing from record values were omitted from Excel exports. Export now includes all schema fields even when values are null/empty, while preserving additional fields from
allowAdditionalFields.Tell code reviewer how and what to test:
Manual Testing:
allowAdditionalFields, verify additional columns still appear after schema columnsKey Review Points:
Setand filters keys once per sheet (moved outside record loop). For large datasets (10k+ records), verify this doesn't cause issues.columnNameTransformerproduces identical labels for different keys,json_to_sheetmay collapse columns. Review if deduplication is needed.allowAdditionalFieldsenabledexcludeFieldsoptionPotential Issues:
Link to Devin run: https://app.devin.ai/sessions/69f9079121c84b029c7a3b02d21eaf40
Requested by: [email protected] (@cnharrison)