adding translations to excel extractor and extractor plugin#738
adding translations to excel extractor and extractor plugin#738elisadinsmore merged 6 commits intomainfrom
Conversation
|
Related Platform PR: https://github.com/FlatFilers/Platform/pull/9074 |
WalkthroughThe pull request introduces standardized error and logging message identifiers across three files: 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)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 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 (4)
plugins/xlsx-extractor/src/parser.ts (2)
42-44: Convert debug log message to use translation key.The error message has been converted to use a translation key, but the debug log message is still using a hardcoded string. For consistency, consider converting the debug message to use a translation key as well.
if (options?.debug) { - console.log( - 'File is too large to parse. Try converting this file to CSV.' - ) + console.log('files.errors.fileTooLargeDebug') }
Line range hint
133-147: Standardize debug logging messages.For consistency with the translation approach, consider converting these debug messages to use translation keys:
No rows found in '${sheetName}'Detected header:if (debug) { - console.log(`No rows found in '${sheetName}'`) + console.log('files.extraction.noRowsFound', { sheetName }) } ... if (debug) { - console.log('Detected header:', slicedHeader) + console.log('files.extraction.detectedHeader', { header: slicedHeader }) }utils/extractor/src/index.ts (2)
104-104: Consider using a constant for duplicate translation key.The translation key 'files.extraction.addingRecords' is used in multiple places. Consider defining it as a constant to maintain consistency and make updates easier.
+const TRANSLATION_KEYS = { + ADDING_RECORDS: 'files.extraction.addingRecords' +} as const; ... -await tick(10, 'files.extraction.addingRecords') +await tick(10, TRANSLATION_KEYS.ADDING_RECORDS) ... -'files.extraction.addingRecords' +TRANSLATION_KEYS.ADDING_RECORDSAlso applies to: 126-126
Line range hint
147-148: Convert debug log message to use translation key.For consistency with other changes, consider converting the debug log message to use a translation key.
if (debug) { - console.log(`Extractor error: ${e.message}`) + console.log('files.extraction.error', { message: e.message }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/xlsx-extractor/src/parser.ts(2 hunks)utils/extractor/src/index.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Setup
🔇 Additional comments (1)
utils/extractor/src/index.ts (1)
63-63: LGTM! Translation keys follow consistent patterns.The translation keys follow consistent patterns:
- Extraction-related keys:
files.extraction.*- UI-related keys:
files.uploadFile.popovers.*Also applies to: 78-78, 86-86, 101-101, 141-141, 143-143, 154-154
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
flatfilers/sandbox/src/index.ts (1)
8-8: Ensure adequate testing of the newly integrated pluginInvoking
listener.use(ExcelExtractor())seamlessly incorporates Excel file extraction into the pipeline. However, consider adding or extending integration tests to confirm that Excel files are extracted as expected and that no regressions occur with the existing JSONExtractor or export-delimited-zip plugin.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.json
📒 Files selected for processing (1)
flatfilers/sandbox/src/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Nullify Dependencies
🔇 Additional comments (1)
flatfilers/sandbox/src/index.ts (1)
5-5: Good addition of the ExcelExtractor importThis import is essential for providing Excel file extraction functionality. Please ensure the dependency version in your package.json is aligned with your expected usage and that any related peer dependencies are satisfied.
3 participants
Please explain how to summarize this PR for the Changelog:
Added translation keys for the excel extractor and extractor util
Tell code reviewer how and what to test:
When this is run with the accompanying Platform PR, the excel extraction process should be unchanged and the correct messages should be seen instead of translation keys.