-
Notifications
You must be signed in to change notification settings - Fork 1
Add validation to document uploader #683
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
Conversation
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.
Hello @german664, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team,
Gemini here, providing a summary of this pull request authored by @german664. The primary goal of this PR, as indicated by the title "Add validation", is to implement client-side validation for file uploads within the dispute management flow, specifically for the "Defend Dispute" section. This includes enforcing constraints on allowed file types and sizes, and ensuring that necessary documents are uploaded before the user can proceed.
The changes involve modifying the DefendDisputeFileUpload
component to incorporate validation logic and pass down necessary props to its child components (DefendDocumentUpload
and SelectAndUploadOptionalDoc
). A new constants file is introduced to define the allowed file types and their respective maximum sizes. The generic FileInput
and Dropzone
components were also updated to support more flexible maxFileSize
definitions (allowing different sizes per file type) and to provide more detailed context for large file errors in the translated messages. Additionally, new translation keys for the validation errors have been added, and there are minor unrelated CSS adjustments in the DisputesData
component styles.
Highlights
- File Upload Validation: Implemented client-side validation for file uploads in the Defend Dispute flow, checking for allowed file types and maximum sizes.
- File Type and Size Limits: Defined specific allowed file types (PDF, JPEG, JPG, TIFF) and maximum size limits per type (PDF: 2MB, Images: 10MB) in a new constants file.
- Submit Button Logic: The submit button in the Defend Dispute file upload step is now disabled until all required documents and at least one optional document have been successfully uploaded.
- FileInput Component Enhancement: Updated the generic
FileInput
andDropzone
components to support definingmaxFileSize
as a function (allowing type-specific limits) and to pass more context (file type and size limit) for large file validation error messages. - Translations: Added new translation keys for file upload validation error messages, including specific messages for large files with dynamic size and type information.
- CSS Adjustments: Made minor adjustments to the CSS for skeleton loading states in the
DisputesData
component, primarily reordering properties and adding stylelint comments.
Changelog
Click here to see the changelog
- src/components/external/DisputeManagement/components/DefendDisputeFlow/DefendDisputeFileUpload.tsx
- Added a
mapError
function to translate validation error types to translation keys (lines 29-40). - Removed the old
actionButtons
memo hook (lines 61-74). - Added
areRequiredDocsUploaded
memo hook to check if required and at least one optional document are uploaded (lines 111-119). - Re-added the
actionButtons
memo hook, disabling the submit button based onareRequiredDocsUploaded
state (lines 121-134). - Passed the
mapError
prop toDefendDocumentUpload
(line 215) andSelectAndUploadOptionalDoc
components (lines 222, 238).
- Added a
- src/components/external/DisputeManagement/components/DefendDisputeFlow/DefendDocumentUpload.tsx
- Imported validation-related types and constants (lines 8-10).
- Added
mapError
prop to the component's interface (line 16). - Passed
allowedFileTypes
,maxFileSize
(using constants),mapError
, and anonDelete
handler to theFileInput
component (lines 52-59).
- src/components/external/DisputeManagement/components/DefendDisputeFlow/SelectAndUploadOptionalDoc.tsx
- Imported validation-related types and constants (lines 13-15).
- Added
mapError
prop to the component's interface (line 26, 36). - Passed
maxFileSize
(using constants),allowedFileTypes
,mapError
, and anonDelete
handler to theFileInput
component (lines 107-114).
- src/components/external/DisputeManagement/components/DefendDisputeFlow/constants.ts
- Added a new file defining
UPLOAD_DOCUMENT_MAX_SIZE
,ALLOWED_FILE_TYPES
, andDOCUMENT_MAX_SIZE
constants for file validation (lines 1-13).
- Added a new file defining
- src/components/external/DisputeManagement/components/DisputesData/DisputeData.scss
- Reordered the
@include skeleton.adyen-pe-skeleton-animation();
mixin and added stylelint comments in.adyen-pe-dispute-data__status-box-status
(lines 140-147),.adyen-pe-dispute-data__status-box-amount
(lines 149-156), and.adyen-pe-dispute-data__properties-skeleton-element
(lines 191-196).
- Reordered the
- src/components/internal/FormFields/FileInput/FileInput.tsx
- Added
onDelete
prop to the component interface (line 11). - Called the
onDelete
callback within thedeleteFile
function (line 36).
- Added
- src/components/internal/FormFields/FileInput/components/Dropzone.tsx
- Imported
getHumanReadableFileSize
utility (line 8). - Added
largeFileErrorContext
state to store details for large file errors (line 44). - Cleared
largeFileErrorContext
on file change (line 72). - Updated
updateInputValidationError
to be memoized withinputRef
dependency (lines 90-108). - Modified file size validation to handle
maxFileSize
as a function and setlargeFileErrorContext
on large file errors (lines 125-130). - Updated the error message display to use values from
largeFileErrorContext
for the large file translation (lines 200-205).
- Imported
- src/components/internal/FormFields/FileInput/types.ts
- Updated the type of
maxFileSize
inBaseFileInputProps
to accept a number or a function returning a number (line 11). - Added an optional
onDelete
prop toFileInputProps
(line 22).
- Updated the type of
- src/translations/en-US.json
- Added new translation keys:
disputes.error.weCouldNotLoadTheDisputesOverviewComponent
(line 546),disputes.inputError.fileIsOverSizeLimitForTypeChooseASmallerFileAndTryAgain
(line 553), anddisputes.inputError.uploadAtLeastOneSupportingDocumentToContinue
(line 554).
- Added new translation keys:
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Files upload with care,
Size and type checked right there,
Errors, now more clear.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces important validation logic for file uploads within the dispute management flow, enhancing data integrity and user experience by providing clear feedback on file requirements. The changes include checks for file types, maximum file sizes (configurable per type), and ensuring required documents are uploaded.
The implementation is generally solid, with good use of context for managing dispute flow state and payload. The introduction of constants for allowed file types and sizes is a good practice. The error mapping to translation keys is also well-handled.
I've identified a couple of areas that might need further attention to ensure robustness and cover all edge cases, detailed in the comments below.
Summary of Findings
- Missing default case in error mapping: The
mapError
function inDefendDisputeFileUpload.tsx
lacks a default case in its switch statement, potentially leading to undefined behavior if new error types are introduced without updating the function. - Potential logic issue in document upload validation: The
areRequiredDocsUploaded
logic inDefendDisputeFileUpload.tsx
might incorrectly prevent submission if theoneOrMoreDocuments
list is empty, even when all strictly required documents are uploaded. This depends on the specific business requirements for when the 'one or more' category is empty. - Constant definition style (not commented due to review settings): In
src/components/external/DisputeManagement/components/DefendDisputeFlow/constants.ts
, the keys forDOCUMENT_MAX_SIZE
(e.g.,['application/pdf']
) can be simplified to string literals (e.g.,'application/pdf'
). This is a minor stylistic improvement and was not commented on directly due to severity settings.
Merge Readiness
The pull request introduces valuable validation features. However, there are a couple of medium-severity concerns regarding error handling and validation logic that should be addressed to ensure the robustness and correctness of the implementation. I recommend addressing these points before merging. As an AI, I am not authorized to approve pull requests; please ensure further review and approval by team members.
Summary
This pull request introduces validation logic for file uploads within the dispute defend flow, providing clear feedback on file requirements. The changes include checks for file types, maximum file sizes (configurable per type), and ensuring required documents are uploaded.
Fixed issue: CXP-3649