[TASK-7268] fix: add attachment to requests #558
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request primarily involve modifications to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/components/Global/FileUploadInput/index.tsx (3)
Line range hint
20-27: Add memory cleanup and improve type safety
- The URL.createObjectURL() call creates a memory leak as the URL is not revoked
- The event type should be properly typed instead of 'any'
Consider this implementation:
- const handleFileChange = (e: any) => { + const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files[0] if (file) { + if (attachmentOptions.fileUrl) { + URL.revokeObjectURL(attachmentOptions.fileUrl) + } const url = URL.createObjectURL(file) setAttachmentOptions({ message: attachmentOptions.message, fileUrl: url, rawFile: file }) } } + + useEffect(() => { + return () => { + if (attachmentOptions.fileUrl) { + URL.revokeObjectURL(attachmentOptions.fileUrl) + } + } + }, [])
Line range hint
29-41: Optimize file type detectionThe current implementation fetches the file to determine its type, which is inefficient as we already have the raw file.
Consider this optimization:
useEffect(() => { - if (attachmentOptions.fileUrl) { - fetch(attachmentOptions.fileUrl) - .then((response) => response.blob()) - .then((blob) => { - setFileType(blob.type) - }) - .catch((error) => { - console.log('Error fetching the blob from URL:', error) - setFileType('') // Reset or handle the error state - }) + if (attachmentOptions.rawFile) { + setFileType(attachmentOptions.rawFile.type) } - }, [attachmentOptions.fileUrl]) + }, [attachmentOptions.rawFile])
Line range hint
47-49: Add file size validationThere's currently no validation for file size, which could lead to issues with large files.
Consider adding size validation:
<input type="file" accept="image/*,application/pdf" + data-max-size="5242880" onChange={handleFileChange} className="hidden" id="file-input" />And update the handler:
const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0] if (!file) return const maxSize = parseInt(e.target.dataset.maxSize || '5242880', 10) // 5MB default if (file.size > maxSize) { alert('File is too large. Maximum size is 5MB.') return } if (attachmentOptions.fileUrl) { URL.revokeObjectURL(attachmentOptions.fileUrl) } const url = URL.createObjectURL(file) setAttachmentOptions({ message: attachmentOptions.message, fileUrl: url, rawFile: file }) }src/components/Request/Create/Views/Initial.view.tsx (1)
Line range hint
169-179: Reduce code duplication in handleOnNext callsThe same handleOnNext call with identical parameters is duplicated in the component.
Consider extracting the parameters to a function:
const getHandleOnNextParams = () => ({ recipientAddress, tokenAddress: selectedTokenAddress, chainId: selectedChainID, userBalances: balances, tokenValue, tokenData: selectedTokenData, attachmentOptions, })Then use it in both places:
- handleOnNext({ - recipientAddress, - tokenAddress: selectedTokenAddress, - chainId: selectedChainID, - userBalances: balances, - tokenValue, - tokenData: selectedTokenData, - attachmentOptions, - }) + handleOnNext(getHandleOnNextParams())Also applies to: 199-209
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Global/FileUploadInput/index.tsx(1 hunks)src/components/Request/Create/Views/Initial.view.tsx(4 hunks)
🔇 Additional comments (1)
src/components/Global/FileUploadInput/index.tsx (1)
Line range hint 4-14: LGTM on interface export and structure
The interface is well-structured with clear type definitions for all properties.
| attachmentOptions, | ||
| }: { | ||
| recipientAddress: string | undefined | ||
| tokenAddress: string | ||
| chainId: string | ||
| userBalances: IUserBalance[] | ||
| tokenValue: string | undefined | ||
| tokenData: Pick<IToken, 'chainId' | 'address' | 'decimals' | 'symbol'> | undefined | ||
| attachmentOptions: IFileUploadInputProps['attachmentOptions'] |
There was a problem hiding this comment.
Add validation for attachment data
The attachmentOptions parameter is added but there's no validation before passing to the SDK.
Consider adding validation:
const validateAttachment = (
attachmentOptions: IFileUploadInputProps['attachmentOptions']
): { isValid: boolean; error?: string } => {
if (!attachmentOptions) return { isValid: true }
if (attachmentOptions.rawFile && attachmentOptions.rawFile.size > 5242880) {
return { isValid: false, error: 'Attachment file size exceeds 5MB limit' }
}
if (attachmentOptions.message && attachmentOptions.message.length > 140) {
return { isValid: false, error: 'Reference message exceeds 140 characters' }
}
return { isValid: true }
}Then use it in handleOnNext:
try {
+ const { isValid, error } = validateAttachment(attachmentOptions)
+ if (!isValid) {
+ setErrorState({
+ showError: true,
+ errorMessage: error || 'Invalid attachment',
+ })
+ return
+ }
let inputValue = tokenValue
Summary by CodeRabbit
New Features
Bug Fixes