feat: export delimited zip #680
Conversation
![flatfile-nullify[bot]](https://avatars.githubusercontent.com/u/31830338?s=60&v=4){kind=small}
Nullify Code Vulnerabilities8 findings found in this pull request
You can find a list of all findings here |
carlbrugger
force-pushed
the
feat/export-delimited-zip
branch
from
657a489
to
f7bb4ae
Compare
WalkthroughThe pull request introduces several new files and configurations for the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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: 4
🧹 Outside diff range and nitpick comments (4)
export/delimited-zip/README.MD (4)
5-6: Add context for the event type trigger.Consider adding a brief explanation of when the
job:readyevent is triggered and how it relates to the export process. This would help developers better understand the plugin's lifecycle.**Event Type:** -`job:ready` +`job:ready` - Triggered when an export job is initiated through the Flatfile platform
17-27: Enhance parameter documentation with examples and constraints.Consider adding:
- Example values for each parameter
- Validation constraints (e.g., allowed delimiters)
- Common use cases for different file extensions
#### `delimiter` - `string` -The delimiter to use in the exported files. Default: ',' +The delimiter to use in the exported files. Default: ','. Common values: ',', '|', ';', '\t' #### `fileExtension` - `string` -The file extension for the exported files. Default: 'csv' +The file extension for the exported files. Default: 'csv'. Supported values: 'csv', 'tsv', 'txt'🧰 Tools
🪛 Markdownlint
17-17: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
31-38: Add context for API endpoint usage.Consider documenting how each API endpoint is used in the export process to help developers understand the plugin's workflow.
## API Calls -api.jobs.ack -api.jobs.complete -api.jobs.fail -api.sheets.list -api.records.get -api.files.upload +- `api.jobs.ack` - Acknowledges the export job has started +- `api.jobs.complete` - Marks the export job as successfully completed +- `api.jobs.fail` - Handles export job failures +- `api.sheets.list` - Retrieves all sheets from the workbook for export +- `api.records.get` - Fetches records from each sheet +- `api.files.upload` - Uploads the generated ZIP file
49-61: Expand usage documentation with advanced examples.Consider adding:
- Error handling examples
- Advanced configuration scenarios
- Event handling examples for monitoring export progress
// Add examples like: listener.use(exportDelimitedZip({ job: 'export-delimited-zip', delimiter: ',', fileExtension: 'csv', debug: true, // Add error handling onError: (error) => { console.error('Export failed:', error); }, // Add progress tracking onProgress: (progress) => { console.log(`Export progress: ${progress}%`); } }))🧰 Tools
🪛 Markdownlint
49-49: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
export/delimited-zip/package.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.json
📒 Files selected for processing (7)
export/delimited-zip/README.MD(1 hunks)export/delimited-zip/jest.config.js(1 hunks)export/delimited-zip/src/export.delimited.zip.plugin.ts(1 hunks)export/delimited-zip/src/index.ts(1 hunks)export/delimited-zip/tsup.config.mjs(1 hunks)export/pivot-table/jest.config.js(1 hunks)flatfilers/sandbox/src/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- export/delimited-zip/jest.config.js
- export/delimited-zip/src/index.ts
- export/delimited-zip/tsup.config.mjs
- export/pivot-table/jest.config.js
🧰 Additional context used
🪛 Markdownlint
export/delimited-zip/README.MD
17-17: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
44-44: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
49-49: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (7)
flatfilers/sandbox/src/index.ts (5)
2-3: Import Statements: Verify the Necessity and Order
The import statements include exportDelimitedZip and configureSpace from their respective packages. Ensure that these imports are necessary and properly used in the code below. Additionally, consider organizing the import statements for better readability if the project coding standards require it.
5-5: Async Function Declaration
The default exported function is now declared as async. If there are no await expressions inside this function, the async keyword might be unnecessary. Please verify whether await is used within this function or any of its callbacks.
If await is not used, consider removing the async keyword:
-export default async function (listener: FlatfileListener) {
+export default function (listener: FlatfileListener) {6-13: Middleware Usage of exportDelimitedZip
The exportDelimitedZip middleware is configured with specific options. Verify that:
- The
jobname'export-delimited-zip'matches the operation defined in the action configuration. - The
delimiter,fileExtension, anddebugoptions are correctly set according to the desired export format. - The middleware is correctly integrated into the listener.
15-112: Sheet and Action Configurations
The sheets Sales and Sales 2 are configured with fields and actions. Consider the following:
- Field Definitions: Ensure that all required fields are included and that their
key,type, andlabelproperties are correctly defined. - Action Operations: Confirm that the operations
'generateExampleRecords'and'export-external-api'are implemented elsewhere in the codebase. - Action Modes: Both actions are set to
'foreground'. Verify that this is the intended mode of operation.
You might want to check if the actions' operation values correspond to actual listener implementations. If they are not yet implemented, consider adding placeholders or implementations to prevent runtime errors.
105-109: Association of Workbook Action with Middleware
The workbook action is configured with the operation 'export-delimited-zip'. Ensure that this action correctly triggers the exportDelimitedZip middleware configured earlier.
Confirm that the job name in the middleware configuration matches the operation name in the action:
listener.use(
exportDelimitedZip({
- job: 'export-delimited-zip',
+ job: 'export-delimited-zip', // Should match the action's operation name
delimiter: '\t',
fileExtension: 'tsv',
debug: true,
})
)This alignment is crucial for the middleware to respond to the correct action.
export/delimited-zip/src/export.delimited.zip.plugin.ts (2)
190-192: 🛠️ Refactor suggestion
Include Original Error Message for Improved Debugging
The error thrown provides a generic message, which may hinder debugging efforts. Including the original error message can help identify the root cause more efficiently.
Modify the error handling to incorporate the original error message:
throw new Error(
- `This job failed probably because it couldn't write to the ${options.fileExtension.toUpperCase()} files, compress them into a ZIP file, or upload it.`
+ `Job failed due to an error: ${error.message}. This may be caused by issues writing to the ${options.fileExtension.toUpperCase()} files, compressing them into a ZIP file, or uploading it.`
)Likely invalid or redundant comment.
149-162:
Use Consistent Sheet References During Cleanup
In the cleanup section, you're iterating over workbook.sheets, but earlier you retrieved sheets using api.sheets.list, stored in the sheets variable. There may be a discrepancy if workbook.sheets is undefined or doesn't match the sheets list.
Update the loop to use the sheets variable for consistency:
- for (const sheet of workbook.sheets) {
+ for (const sheet of sheets) {Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
.changeset/lemon-bears-drive.md (1)
9-11: Enhance the changeset description with more details.The current description could be more comprehensive. Consider adding:
- Plugin configuration options and parameters
- API calls for managing the export job lifecycle
- Integration instructions
- Any breaking changes or dependencies
Here's a suggested structure:
This release introduces the @flatfile/plugin-export-delimited-zip plugin, which: - Exports Workbook Sheets as CSV files in a ZIP format - Provides configuration options for customizing the export process - Includes API calls for managing the export job lifecycle - Integrates at the Workbook level via [integration details] Additional updates include documentation improvements for [specific areas]. Configuration options: [List key configuration parameters] Usage: [Basic usage example]export/delimited-zip/README.MD (3)
1-12: Enhance the info card section readability.Consider these minor improvements to make the documentation more consistent and prominent:
<!-- START_INFOCARD --> The `@flatfile/plugin-export-delimited-zip` plugin exports data from Flatfile sheets to delimited files (e.g., CSV) and compresses them into a ZIP file. This plugin provides an efficient way to download and package your data from Flatfile workbooks. -**Event Type:** -`job:ready` +**Event Type:** `job:ready` <!-- END_INFOCARD --> - -> ⚠️ **Important:** When embedding Flatfile, this plugin should be deployed in a server-side listener. [Learn more](/docs/orchestration/listeners#listener-types)
31-39: Enhance API calls documentation.Consider adding brief descriptions for each API call to explain their role in the export process. This would help developers understand the plugin's workflow better.
## API Calls -api.jobs.ack -api.jobs.complete -api.jobs.fail -api.sheets.list -api.records.get -api.files.upload +- `api.jobs.ack` - Acknowledges the export job has started +- `api.jobs.complete` - Marks the export job as successfully completed +- `api.jobs.fail` - Handles export job failures +- `api.sheets.list` - Retrieves the list of sheets to be exported +- `api.records.get` - Fetches the records from each sheet +- `api.files.upload` - Uploads the generated ZIP file
50-61: Enhance the code example with comments.The example would be more helpful with additional comments explaining the configuration options.
**listener.ts** ```typescript import type { FlatfileListener } from '@flatfile/listener' import { exportDelimitedZip } from '@flatfile/plugin-export-delimited-zip' export default function (listener: FlatfileListener) { listener.use(exportDelimitedZip({ + // The job name that triggers this export job: 'export-delimited-zip', + // Character used to separate values in the output file delimiter: ',', + // Output file extension (e.g., 'csv', 'tsv') fileExtension: 'csv', })) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.json
📒 Files selected for processing (5)
.changeset/lemon-bears-drive.md(1 hunks)bundlers/rollup-config/index.mjs(1 hunks)export/delimited-zip/README.MD(1 hunks)plugins/record-hook/rollup.config.mjs(0 hunks)plugins/rollout/README.md(1 hunks)
💤 Files with no reviewable changes (1)
- plugins/record-hook/rollup.config.mjs
🧰 Additional context used
🪛 Markdownlint
export/delimited-zip/README.MD
17-17: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
44-44: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
49-49: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (5)
.changeset/lemon-bears-drive.md (1)
1-7: Please clarify the version bumps for other packages.
While the bump for @flatfile/plugin-export-delimited-zip is clear, could you explain why the following packages require version bumps?
- @flatfile/bundler-config-rollup
- @flatfile/plugin-record-hook
- @flatfile/plugin-export-pivot-table
- @flatfile/plugin-rollout
✅ Verification successful
Let me try another verification strategy to check for changes in these packages by looking at the full git diff and package dependencies.
Let me check for any changes in the source files of these packages by looking at the git diff.
Let me try another approach to check for changes by looking at the full git diff and any new files.
Based on the findings, I can now generate a final response:
Version bumps appear to be justified by new package and dependency updates
The version bumps are warranted for the following reasons:
@flatfile/plugin-export-delimited-zip: New plugin being introduced with initial implementation@flatfile/plugin-export-pivot-table: Has existing implementation with CHANGELOG indicating updates@flatfile/plugin-record-hook: Core dependency used by multiple packages including the new delimited-zip plugin@flatfile/plugin-rollout: Has existing implementation with CHANGELOG@flatfile/bundler-config-rollup: Build tooling dependency that may require updates to support new plugins
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related changes in other packages
# Expected: Find changes that justify version bumps
echo "Checking for changes in related packages..."
for pkg in "bundler-config-rollup" "plugin-record-hook" "plugin-export-pivot-table" "plugin-rollout"; do
echo "Changes in @flatfile/$pkg:"
git diff --name-only | grep -i "$pkg" || echo "No changes found"
done
Length of output: 1270
Script:
#!/bin/bash
# Get the full git diff to see all changes
git diff
# Also check package dependencies
echo "Checking package.json files..."
fd package.json --exec cat {} \;
Length of output: 116424
Script:
#!/bin/bash
# Get the git diff for the specific packages mentioned in the changeset
git diff HEAD -- \
"export/delimited-zip" \
"plugins/record-hook" \
"export/pivot-table" \
"plugins/rollout" \
"bundlers/rollup"
Length of output: 110
Script:
#!/bin/bash
# Check for any new files in these directories
echo "Checking for new files..."
fd . "export/delimited-zip" "plugins/record-hook" "export/pivot-table" "plugins/rollout" "bundlers/rollup"
# Get the full git status to see changes
echo -e "\nChecking git status..."
git status
Length of output: 1879
plugins/rollout/README.md (1)
17-17: LGTM! Clear and important deployment guidance.
The added note effectively communicates the deployment requirement and enhances the documentation.
export/delimited-zip/README.MD (1)
1-61: Documentation structure and content look good!
The README provides a clear and comprehensive guide for using the export-delimited-zip plugin. The structure is logical, and the content covers all essential aspects of the plugin's functionality.
🧰 Tools
🪛 Markdownlint
17-17: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
44-44: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
49-49: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
bundlers/rollup-config/index.mjs (2)
44-46: LGTM: Added validation for UMD configuration
Good addition of validation to ensure UMD builds have the required name configuration. The error message is clear and helpful.
38-41:
The default values for build types have been significantly changed:
includeNode: true → falseincludeBrowser: true → falseincludeDefinition: true → falseincludeUmd: false → true
This change will affect any projects that rely on the default configuration. Ensure this breaking change is properly documented and communicated to users.
…lers/flatfile-plugins into feat/export-delimited-zip
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
export/delimited-zip/src/export.delimited.zip.plugin.ts (2)
61-61: Reconsider limiting sheet names to 31 characters.Limiting sheet names to 31 characters might truncate important information. Consider increasing the limit or handling longer names gracefully.
123-124: Consider increasing thepageSizefor record processing.The
pageSizeis set to 5, which may lead to a high number of API calls for large datasets. Increasing thepageSizecan improve performance by reducing the number of calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
.changeset/lemon-bears-drive.md(1 hunks)export/delimited-zip/src/export.delimited.zip.plugin.ts(1 hunks)plugins/dedupe/README.md(1 hunks)plugins/delimiter-extractor/README.md(1 hunks)plugins/export-workbook/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- plugins/dedupe/README.md
- plugins/delimiter-extractor/README.md
- plugins/export-workbook/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/lemon-bears-drive.md
🧰 Additional context used
🪛 Biome
export/delimited-zip/src/export.delimited.zip.plugin.ts
[error] 41-41: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 41-41: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (1)
export/delimited-zip/src/export.delimited.zip.plugin.ts (1)
93-94: Potential issue with accessing undefined record values.
Please explain how to summarize this PR for the Changelog:
Tell code reviewer how and what to test: