-
Notifications
You must be signed in to change notification settings - Fork 198
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
feat: added csv document rendering #2958
Conversation
|
WalkthroughThis pull request introduces comprehensive support for CSV file handling across multiple components in the backoffice application. The changes include adding new dependencies like Changes
Sequence DiagramsequenceDiagram
participant User
participant DocumentComponent
participant isCsvUtil
participant CsvConverter
participant PDFGenerator
User->>DocumentComponent: Upload CSV file
DocumentComponent->>isCsvUtil: Check if file is CSV
isCsvUtil-->>DocumentComponent: Return true
DocumentComponent->>CsvConverter: Convert CSV to base64
CsvConverter->>PDFGenerator: Generate PDF
PDFGenerator-->>DocumentComponent: Return PDF representation
DocumentComponent->>User: Display PDF
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (8)
apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/DocumentField/DocumentField.tsx (2)
247-247
: Add CSV MIME type validation and documentation.While the property is correctly passed through, consider adding validation for CSV MIME types and documenting the accepted format patterns.
Add validation and documentation:
+// Valid formats: '.csv' or 'text/csv' +const validateFileFormats = (formats?: string) => { + if (!formats) return formats; + const csvTypes = ['.csv', 'text/csv']; + const formatList = formats.split(',').map(f => f.trim()); + return formatList.some(f => csvTypes.includes(f)) ? formats : `${formats},.csv`; +}; <FileUploaderField uploadFile={fileUploader} disabled={elementState.isLoading || (state.isRevision && warnings.length ? false : warningsRef.current?.length ? false : restProps.disabled)} fileId={fileId} fileRepository={collectionFlowFileStorage} onBlur={onBlur as () => void} testId={definition.name} onChange={handleChange} - acceptFileFormats={definition.options.acceptFileFormats} + acceptFileFormats={validateFileFormats(definition.options.acceptFileFormats)} />
Line range hint
132-165
: Enhance error handling for CSV files.The error handling in the file upload logic should be enhanced to handle CSV-specific scenarios.
Add CSV-specific error handling:
try { toggleElementLoading(); const uploadResult = await uploadFile({ file }); setFieldError(null); + // Validate CSV file format if applicable + if (file.type === 'text/csv' || file.name.endsWith('.csv')) { + try { + // Basic CSV validation - check if file is readable + const text = await file.text(); + const lines = text.split('\n'); + if (lines.length < 2) { + throw new Error('CSV file appears to be empty or invalid'); + } + } catch (csvError) { + setFieldError({ + fieldId: document?.id, + message: 'Invalid CSV file format', + type: 'warning', + }); + return; + } + } return { fileId: uploadResult.id }; } catch (error) {apps/backoffice-v2/src/common/utils/is-csv/is-csv.ts (1)
1-2
: Consider supporting additional CSV MIME types.While the current implementation covers the standard 'text/csv' and common 'application/csv' types, consider adding support for additional MIME types that are frequently used for CSV files:
- 'application/vnd.ms-excel'
- 'text/x-csv'
- 'text/comma-separated-values'
export const isCsv = <T extends { fileType: string }>(document: T) => - document?.fileType === 'text/csv' || document?.fileType === 'application/csv'; + [ + 'text/csv', + 'application/csv', + 'application/vnd.ms-excel', + 'text/x-csv', + 'text/comma-separated-values', + ].includes(document?.fileType);apps/backoffice-v2/src/common/utils/convert-csv-to-pdf-base64-string/convert-csv-to-pdf-base64-string.ts (1)
5-7
: Improve type safety for jsPDF plugin.The current typing for
autoTable
is too permissive. Consider using the types from@types/jspdf-autotable
.+import 'jspdf-autotable'; +import { UserOptions } from 'jspdf-autotable'; interface jsPDFWithPlugin extends jsPDF { - autoTable: any; + autoTable: (options: UserOptions) => void; }apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx (1)
34-34
: Consider consolidating document type checks.The repeated
!isPdf(image) && !isCsv(image)
checks suggest an opportunity for a helper function.Consider creating a utility function like:
+const isEditableImage = (image: { fileType: string }) => !isPdf(image) && !isCsv(image);
Then use it throughout:
-'hover:cursor-move': !isPdf(image) && !isCsv(image), +'hover:cursor-move': isEditableImage(image), -display: !isPdf(image) && !isCsv(image) ? 'block' : 'flex', +display: isEditableImage(image) ? 'block' : 'flex',Also applies to: 45-45
apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx (1)
Line range hint
35-44
: Consider consolidating document type checks.Similar to the ImageEditor component, the repeated document type check could be extracted into a utility function.
Consider creating a utility function:
+const isDocumentType = (image?: { fileType: string }) => isPdf(image) || isCsv(image);
Then use it:
-if (isPdf(selectedImage) || isCsv(selectedImage)) { +if (isDocumentType(selectedImage)) {apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.Toolbar.tsx (1)
Line range hint
59-72
: Consider consolidating document type checks.Similar to previous components, the document type check could be extracted into a shared utility function.
Consider using the same utility function suggested earlier:
+const isEditableImage = (image: { fileType: string }) => !isPdf(image) && !isCsv(image);
Then use it:
-{!isPdf(image) && !isCsv(image) && !isLoading && ( +{isEditableImage(image) && !isLoading && (apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx (1)
104-118
: Consider adding visual feedback for filtered CSV files.While the logic to filter out CSV files is correct, silently excluding them from the document list might confuse users. Consider:
- Adding a message when CSV files are present but filtered
- Showing CSV files with a different visual treatment instead of completely hiding them
This would improve user experience by making the system behavior more transparent.
: documents?.map(document => { const { imageUrl, title, fileType, fileName, id } = document; - return !isCsv(document) ? ( + return ( <ImageViewer.Item id={id} key={keyFactory(id, title, fileName, fileType, imageUrl)} src={imageUrl} fileType={fileType} fileName={fileName} alt={title} caption={title} + disabled={isCsv(document)} + overlay={isCsv(document) ? "CSV files are handled separately" : undefined} /> - ) : null; + ); })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/backoffice-v2/package.json
(2 hunks)apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx
(3 hunks)apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx
(3 hunks)apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.ZoomModal.tsx
(2 hunks)apps/backoffice-v2/src/common/constants.ts
(0 hunks)apps/backoffice-v2/src/common/utils/convert-csv-to-pdf-base64-string/convert-csv-to-pdf-base64-string.ts
(1 hunks)apps/backoffice-v2/src/common/utils/is-csv/is-csv.ts
(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.Toolbar.tsx
(2 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
(2 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/hooks/useDocuments/helpers.ts
(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/hooks/useDocuments/useDocumentsLogic.tsx
(1 hunks)apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/DocumentField/DocumentField.tsx
(2 hunks)services/workflows-service/prisma/data-migrations
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/backoffice-v2/src/common/constants.ts
✅ Files skipped from review due to trivial changes (1)
- services/workflows-service/prisma/data-migrations
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: build (windows-latest)
- GitHub Check: test_linux
🔇 Additional comments (10)
apps/kyb-app/src/components/organisms/UIRenderer/elements/JSONForm/components/DocumentField/DocumentField.tsx (1)
25-25
: LGTM! Verify usage ofacceptFileFormats
.The new optional property is well-typed and follows naming conventions. Let's verify its usage across the codebase to ensure consistent implementation.
✅ Verification successful
Implementation of
acceptFileFormats
is verified and correct.The property is:
- Consistently typed as optional string across components
- Properly passed from DocumentField to FileUploaderField
- Correctly applied to file input's accept attribute
- Covered by unit tests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of acceptFileFormats across the codebase # Find all TypeScript/JavaScript files that might use DocumentFieldParams echo "Files referencing DocumentFieldParams:" rg -l "DocumentFieldParams" --type ts --type tsx # Find all instances where acceptFileFormats is used echo -e "\nUsage of acceptFileFormats:" rg "acceptFileFormats" --type ts --type tsx -C 2Length of output: 311
Script:
#!/bin/bash # Search for DocumentFieldParams interface and its usage echo "=== Files containing DocumentFieldParams ===" rg -p "DocumentFieldParams" --glob "*.{ts,tsx}" -C 2 echo -e "\n=== Usage of acceptFileFormats ===" rg -p "acceptFileFormats" --glob "*.{ts,tsx}" -C 2Length of output: 3008
apps/backoffice-v2/src/common/utils/convert-csv-to-pdf-base64-string/convert-csv-to-pdf-base64-string.ts (1)
9-36
:⚠️ Potential issueAdd input validation and error handling.
The current implementation has several security and reliability concerns:
- No validation of input size
- Unsafe base64 decoding
- No handling of malformed CSV data
- No validation of empty data
Consider this safer implementation:
-export const convertCsvToPdfBase64String = (csvBase64: string) => { +const MAX_CSV_SIZE = 10 * 1024 * 1024; // 10MB limit + +export const convertCsvToPdfBase64String = async (csvBase64: string) => { + if (!csvBase64) { + throw new Error('No CSV data provided'); + } + // Extract base64 data from data URI const base64Data = csvBase64.split(',')[1] || csvBase64; + // Check size before decoding + const sizeInBytes = (base64Data.length * 3) / 4; + if (sizeInBytes > MAX_CSV_SIZE) { + throw new Error('CSV file too large'); + } + // Decode base64 to string - const csvString = atob(base64Data); + let csvString: string; + try { + csvString = atob(base64Data); + } catch (error) { + throw new Error('Invalid base64 data'); + } // Parse CSV string to array using PapaParse - const { data } = Papa.parse(csvString, { + const { data, errors } = Papa.parse(csvString, { header: true, skipEmptyLines: true, }); + if (errors.length > 0) { + throw new Error(`CSV parsing failed: ${errors[0].message}`); + } + + if (!Array.isArray(data) || data.length === 0) { + throw new Error('No valid data found in CSV'); + } + // Create new PDF document const doc = new jsPDF() as jsPDFWithPlugin; + const headers = Object.keys(data[0] as object); + if (headers.length === 0) { + throw new Error('No columns found in CSV'); + } + // Add table to PDF using autoTable doc.autoTable({ - head: [Object.keys(data[0] as object)], // Column headers + head: [headers], body: data.map(row => Object.values(row as object)), // Row data startY: 10, margin: { top: 10 }, styles: { fontSize: 8 }, headStyles: { fillColor: [66, 66, 66] }, }); - return doc.output('datauristring'); + try { + return doc.output('datauristring'); + } catch (error) { + throw new Error('Failed to generate PDF'); + } };Let's verify the CSV parsing capabilities with different file formats:
apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx (2)
3-3
: LGTM: Import added for CSV handling.The
isCsv
utility import aligns with the new CSV document rendering feature.
51-54
: LGTM: ReactCrop handling for CSV files.The disabled state and className conditions correctly handle CSV files similar to PDFs, preventing crop operations on document types that shouldn't support it.
apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx (2)
1-1
: LGTM: Import added for CSV handling.The
isCsv
utility import aligns with the new CSV document rendering feature.
64-65
: LGTM: Added displayName for debugging.Adding the displayName property improves component identification in React DevTools.
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.Toolbar.tsx (1)
7-7
: LGTM: Import added for CSV handling.The
isCsv
utility import aligns with the new CSV document rendering feature.apps/backoffice-v2/src/pages/Entity/components/Case/hooks/useDocuments/useDocumentsLogic.tsx (1)
14-16
: Verify CSV to PDF conversion performance impact.The conversion of CSV documents to PDF format is memoized, but we should ensure this doesn't impact performance with large CSV files.
Let's check for potential performance bottlenecks:
✅ Verification successful
CSV to PDF conversion implementation is performance-optimized ✅
The conversion implementation is lightweight and well-optimized:
- Uses efficient native browser APIs for base64 processing
- Properly memoized to prevent unnecessary conversions
- Only processes CSV documents, leaving others unchanged
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CSV file size limits or handling rg -A 5 "convertCsvToPdfBase64String|convertCsvDocumentsToPdf" # Look for any performance-related comments or tests rg -l "performance|benchmark" --type ts --type tsxLength of output: 3789
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx (1)
8-8
: LGTM! Import of CSV utility aligns with feature requirements.The addition of the
isCsv
utility supports the new CSV document handling feature.apps/backoffice-v2/package.json (1)
119-120
: Verify security of new dependencies and consider version pinning.The new dependencies for CSV handling look appropriate, but:
- Consider pinning exact versions to prevent unexpected updates
- Need to verify these versions for security vulnerabilities
Run the following script to check for known vulnerabilities:
Also applies to: 128-128, 178-178
✅ Verification successful
Dependencies are secure, but consider version pinning
The current versions of jspdf (2.5.2) and jspdf-autotable (3.8.4) are free from known vulnerabilities. However, consider pinning exact versions by removing the caret (^) prefix to prevent unexpected updates:
"jspdf": "2.5.2", "jspdf-autotable": "3.8.4",🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for security advisories in new dependencies # Check for vulnerabilities in new packages for pkg in "jspdf" "jspdf-autotable" "papaparse"; do echo "Checking $pkg..." gh api graphql -f query='{ securityVulnerabilities(first: 5, ecosystem: NPM, package: "'$pkg'") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' doneLength of output: 2482
Updates
Features
Bug Fixes
acceptedFileFormats
in KYB.Screenshots
Document on List Page
Zoom
Document Page
Summary by CodeRabbit
Release Notes
New Features
Dependencies
jspdf
andjspdf-autotable
libraries@types/papaparse
as a development dependencyImprovements
Configuration