Features added to export-workbook plugin#762
Conversation
WalkthroughThis pull request introduces a major version update for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as FlatfileEvent Caller
participant Plugin as exportRecords Function
participant Utils as createXLSXSheetOptions
participant Logger as Logging System
Client->>Plugin: Call exportRecords(event, options, tick)
Plugin->>Utils: Convert ExportSheetOptions to JSON2SheetOpts
Utils-->>Plugin: Return JSON2SheetOpts configuration
Plugin->>Logger: Log operation using PACKAGE_NAME
Plugin->>Client: Report progress via tick callback
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/export-workbook/src/utils.ts (1)
62-87: Well-implemented utility function with proper JSDocThe implementation correctly transforms
ExportSheetOptionstoJSON2SheetOptsformat with clean handling of different input types for theoriginproperty and proper mapping ofskipColumnHeaderstoskipHeader.Consider adding additional type guards for the origin property to handle unexpected object structures:
if (sheetOptions?.origin) { if (typeof sheetOptions.origin === 'number') { options.origin = sheetOptions.origin - } else { + } else if ('column' in sheetOptions.origin && 'row' in sheetOptions.origin) { options.origin = { c: sheetOptions.origin.column, r: sheetOptions.origin.row, } + } }plugins/export-workbook/src/plugin.ts (1)
102-102: Type assertion toComments
Though this approach works, consider avoiding type assertions for clarity. A typed definition can prevent accidental type mismatches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fluffy-pillows-call.md(1 hunks)plugins/export-workbook/src/index.ts(1 hunks)plugins/export-workbook/src/options.ts(1 hunks)plugins/export-workbook/src/plugin.ts(11 hunks)plugins/export-workbook/src/utils.spec.ts(1 hunks)plugins/export-workbook/src/utils.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/fluffy-pillows-call.md
🔇 Additional comments (24)
plugins/export-workbook/src/utils.spec.ts (1)
1-23: Well-structured test suite for the new utility functionThis test suite efficiently tests the
createXLSXSheetOptionsfunction using parameterized testing withit.each. The test cases cover important scenarios including null/undefined inputs and various combinations of options. The approach ensures that the function transformsExportSheetOptionscorrectly intoJSON2SheetOptsformat.plugins/export-workbook/src/index.ts (1)
5-6: Good restructuring of importsMoving the
PluginOptionsinterface to a dedicated options file improves code organization, especially as the interface now includes additional properties for new features.plugins/export-workbook/src/options.ts (3)
3-23: Well-defined interfaces for sheet configurationThe
SheetAddressandExportSheetOptionsinterfaces are clearly defined with comprehensive JSDoc comments. They provide a clean structure for specifying sheet origin coordinates and column header options.
25-25: Good function type definition for column name transformationThe
ColumnNameTransformerCallbacktype provides a clear contract for implementing column name transformations, with appropriate parameters for the column name and sheet slug.
27-50: Comprehensive plugin options interface with complete documentationThe
PluginOptionsinterface includes all necessary properties with thorough JSDoc comments. The new propertiessheetOptionsandcolumnNameTransformeralign well with the PR objectives, providing flexible customization for exports.plugins/export-workbook/src/plugin.ts (19)
11-15: Imports for utility functions look good.
No issues detected.
16-16: ImportingComments
This import aligns with the usage for adding comments to.xlsxcells.
17-17: ImportingPluginOptions
Shifting the interface to a dedicated file is a neat approach for organization.
21-21: Use ofLOG_NAME
Defining a constant for log naming improves consistency.
28-28: Updated JSDoc to referencetick
This accurately reflects the function’s parameters.
52-52: Logging sheets found
Helpful debug log for capturing the sheets in the workbook.
60-60: Logging skipped sheets
Clear communication in debug logs.
65-69: Potential column name collisions incolumnNameTransformer
This is a useful feature; however, consider verifying that two or more columns do not accidentally transform to the same name. This could lead to overwriting.Would you like a script to scan for potential collisions, or is this guaranteed never to occur in your implementation?
86-86: Reducing record columns
This appears correct for eliminating excluded fields and constructing the final row object.
95-95: Initialize comments array
Representing cell comments as an empty array is consistent.
112-113: Applying column name transformer to cell values
Renaming each column before populating the worksheet is properly handled here.
152-154: Creating XLSX sheet with custom options
IntegratingcreateXLSXSheetOptionsimproves maintainability by centralizing sheet configuration logic.
185-185: Logging empty data scenario
Clear error message if no data is present.
198-198: Logging file generation
Clear statement confirming the file is saved.
201-201: Logging failure to write the file
Adequate error handling case.
225-225: Refined log message parameters
Ensures consistent usage ofLOG_NAME.
230-230: Logging file upload failure
Another important failure mode captured.
235-235: Logging completion
Provides helpful feedback that the process has finished.
266-266: Error logging
Capturing full error details can be valuable for debugging, but ensure no sensitive data is included in the logged message.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/export-workbook/src/utils.ts (1)
62-90: Well-implemented utility function with clear responsibility.This utility function cleanly converts between the plugin's option format and the XLSX library's format, with proper handling of different origin types.
One minor suggestion for improved robustness:
if (sheetOptions?.origin) { if (typeof sheetOptions.origin === 'number') { options.origin = sheetOptions.origin - } else if ( - 'column' in sheetOptions.origin && - 'row' in sheetOptions.origin - ) { + } else if ( + typeof sheetOptions.origin === 'object' && + 'column' in sheetOptions.origin && + 'row' in sheetOptions.origin + ) { options.origin = { c: sheetOptions.origin.column, r: sheetOptions.origin.row, } } }This additional check would ensure the origin is actually an object before checking for properties.
plugins/export-workbook/src/plugin.ts (1)
137-146: Optimization opportunity for empty cell generation.The current implementation creates empty cells for all fields when there are no results. Consider optimizing this by creating the empty object only once.
results = [ [ - Object.fromEntries( - sheet.config.fields.map((field) => [ - columnNameTransformer(field.key), - emptyCell, - ]) - ), + sheet.config.fields.reduce((acc, field) => { + acc[columnNameTransformer(field.key)] = emptyCell; + return acc; + }, {}) ], ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/export-workbook/src/plugin.ts(10 hunks)plugins/export-workbook/src/utils.ts(2 hunks)
🔇 Additional comments (6)
plugins/export-workbook/src/utils.ts (1)
1-2: Clean import organization.The imports are well organized, bringing in both the internal type definitions and the XLSX library types needed for the new functionality.
plugins/export-workbook/src/plugin.ts (5)
11-16: Clean import restructuring.The imports have been well-organized, with the
PluginOptionscorrectly moved to a separate file and utility functions properly imported.
20-20: Good practice using a constant for logging.Using a constant for the log name ensures consistency throughout the codebase and makes maintenance easier.
64-68: Well-implemented column name transformer feature.The implementation of the column name transformer is clean and follows functional programming principles. The optional chaining and default implementation are handled elegantly.
110-112: Correctly applied column name transformer to cell formatting.The column name transformer is properly applied to the cell keys.
150-153:Details
❓ Verification inconclusive
Clean integration of the new sheet options feature.
The integration of the
createXLSXSheetOptionsutility function is well done, properly handling the optional sheet options based on the sheet slug.Consider adding a check for when
sheet.config.slugis undefined:const worksheet = XLSX.utils.json_to_sheet( rows, - createXLSXSheetOptions(options.sheetOptions?.[sheet.config.slug]) + createXLSXSheetOptions(sheet.config.slug ? options.sheetOptions?.[sheet.config.slug] : undefined) )
🏁 Script executed:
#!/bin/bash # Check if there are any sheets without a slug in the codebase rg -i "sheet\s+.*without.*slug|missing.*slug" --type ts rg "config\s*:\s*\{\s*slug\s*:\s*undefined\s*" --type tsLength of output: 116
Optional Slug Check for Sheet Options
The integration of the
createXLSXSheetOptionsutility is solid. However, please verify that there is no risk ofsheet.config.slugbeing undefined. If there is any chance it might be, consider modifying the call to safeguard against it:const worksheet = XLSX.utils.json_to_sheet( rows, - createXLSXSheetOptions(options.sheetOptions?.[sheet.config.slug]) + createXLSXSheetOptions(sheet.config.slug ? options.sheetOptions?.[sheet.config.slug] : undefined) )Please manually verify that no sheets can have an undefined slug or confirm that the defensive check is unnecessary.
carlbrugger
left a comment
carlbrugger
left a comment
There was a problem hiding this comment.
Thanks for the PR! 🙌🏼
Just a few nits. Also, the please add the new options to the README.md along explanations as to what the options do. Without playing around, it is unclear that the origin pads the sheet with empty cells/rows. It might be helpful to add a usage example.
carlbrugger
left a comment
carlbrugger
left a comment
There was a problem hiding this comment.
Almost there! Let add the import type and it will be good to go.
Co-authored-by: Carl Brugger <cebrugg@gmail.com>
Apologies. I asked for a re-review before making that change. Should be good. |
Co-authored-by: Carl Brugger <cebrugg@gmail.com>
@carlbrugger thanks for the thoughtful review! |
|
@carlbrugger I do not have the option to merge this PR. How can it be merged? |
|
I gotcha |
2 participants
Please explain how to summarize this PR for the Changelog:
New options were added to the
export-workbookplugin options:columnNameTransformeris a callback function enabling users to transform column names before exporting workbook. The callback takes a column name and slug name and returns a string.sheetOptionsis map of slug name toSheetOptionsobject with these props:originthe starting point of the sheet (can either be a row number or full column, row coordinate)skipColumnHeadersoptionally not include column headers. I know its not relevant for our use case, but could be worth while for someone.Tell code reviewer how and what to test:
This was tested in the plugin sandbox.
A second sheet was added:
export-workbookwas installed like so:Here are the results of an export:
The companies export starts the data at row 10 and column 2, as specified by
originThe contacts export transforms the column names to upper case.
