feat(web): file-restrictions-message #1847
Conversation
WalkthroughThis pull request introduces role-based file upload restrictions across multiple components. The changes enhance file upload functionality by adding dynamic messaging and role-specific upload constraints. A new utility function, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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 (
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![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 (3)
web/src/utils/index.ts (1)
24-31: Improve readability and add validation for edge cases.The function logic is correct but could be improved for better maintainability and robustness.
Consider breaking down the complex string manipulation and adding validation:
export const getFileUploaderMsg = (role: Roles, roleRestrictions?: Role[]) => { if (!roleRestrictions) return; const restrictions = roleRestrictions.find((supportedRoles) => Roles[supportedRoles.name] === role); if (!restrictions) return; + + const { allowedMimeTypes, maxSize } = restrictions.restriction; + if (!allowedMimeTypes.length) return; + + const fileTypes = allowedMimeTypes + .map((type) => type.split("/")?.[1]) + .filter(Boolean) + .join(", "); + const maxSizeMB = (maxSize / (1024 * 1024)).toFixed(2); - return `Allowed file types: [${restrictions.restriction.allowedMimeTypes.map((type) => type.split("/")?.[1] ?? null).join(", ")}], Max allowed size: ${(restrictions.restriction.maxSize / (1024 * 1024)).toFixed(2)} MB.`; + return `Allowed file types: [${fileTypes}], Max allowed size: ${maxSizeMB} MB.`; };web/src/pages/Resolver/Policy/index.tsx (1)
88-89: Consider separating the message template from the restrictions.The message template is hardcoded with the restrictions message, which could make it harder to maintain or localize.
Consider extracting the template:
+const POLICY_UPLOAD_MESSAGE = "You can attach additional information as a PDF file. Important: the above description must reference the relevant parts of the file content."; + <StyledFileUploader callback={handleFileUpload} variant={isDesktop ? "info" : undefined} - msg={`You can attach additional information as a PDF file. Important: the above description must reference the relevant parts of the file content.\n${getFileUploaderMsg(Roles.Policy, roleRestrictions)}`} + msg={`${POLICY_UPLOAD_MESSAGE}\n${getFileUploaderMsg(Roles.Policy, roleRestrictions)}`} />web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (1)
102-106: Consider making the variant prop consistent with Policy component.The FileUploader's variant is fixed to "info" while in Policy it's conditional based on desktop view.
Consider using the same desktop-aware approach:
+const isDesktop = useIsDesktop(); + <StyledFileUploader callback={(file: File) => setFile(file)} msg={getFileUploaderMsg(Roles.Evidence, roleRestrictions)} - variant="info" + variant={isDesktop ? "info" : undefined} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx(5 hunks)web/src/pages/Resolver/Policy/index.tsx(3 hunks)web/src/utils/index.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: dependency-review
- GitHub Check: contracts-testing
🔇 Additional comments (3)
web/src/utils/index.ts (1)
16-22: LGTM! Well-structured type definition.The
Roletype clearly defines the structure for file restrictions with appropriate typing.web/src/pages/Resolver/Policy/index.tsx (1)
51-54: LGTM! Proper styling for multiline text.The styling ensures proper alignment and wrapping of the file restrictions message.
web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (1)
Line range hint
128-132: LGTM! Proper error handling with user feedback.The error handling provides both console logging for debugging and user-friendly toast messages.
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Harman-singh-waraich
dismissed stale reviews from kemuru and coderabbitai[bot]
via
2735f07
|
Code Climate has analyzed commit 2735f07 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/src/pages/Resolver/Policy/index.tsx (1)
88-89: Optimize the message formatting.Since
white-space: pre-lineis already set in the styles, the explicit\nin the template literal is redundant.- msg={`You can attach additional information here. Important: the above description must reference the relevant parts of the file content.\n${getFileUploaderMsg(Roles.Policy, roleRestrictions)}`} + msg={`You can attach additional information here. Important: the above description must reference the relevant parts of the file content.${getFileUploaderMsg(Roles.Policy, roleRestrictions)}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/pages/Resolver/Policy/index.tsx(3 hunks)web/src/utils/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/utils/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: dependency-review
- GitHub Check: Mend Security Check
🔇 Additional comments (4)
web/src/pages/Resolver/Policy/index.tsx (4)
4-4: LGTM! Import changes are well-organized.The new imports align with the PR objectives and follow a logical grouping pattern.
Also applies to: 8-11
44-44: Verify the significant margin increase.The margin-bottom has been increased significantly from 72 to 150. Please confirm if this large increase is intentional and necessary for accommodating the new file restriction messages.
The new small text styles look good and will properly handle multi-line messages.
Also applies to: 51-54
59-60: LGTM! Role-based restrictions are properly integrated.The addition of
roleRestrictionsand responsive desktop handling improves the component's functionality.
59-59: Verify consistent role-based message handling across components.The implementation looks good, but let's verify that the role-based message handling is consistent with the
SubmitEvidenceModalcomponent mentioned in the PR description.Also applies to: 88-89
✅ Verification successful
Role-based message handling is consistent across components ✅
Both
SubmitEvidenceModalandPolicycomponents follow the same pattern:
- Use
getFileUploaderMsgwith appropriate role enums- Pass
roleRestrictionsfrom context- Display messages in file upload components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar implementations in other components rg -l "getFileUploaderMsg" "web/src/pages" rg "roleRestrictions.*Roles\." "web/src/pages"Length of output: 192
Script:
#!/bin/bash # Get the context around getFileUploaderMsg usage in both files echo "=== SubmitEvidenceModal.tsx ===" rg -B 2 -A 2 "getFileUploaderMsg" "web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx" echo -e "\n=== Policy/index.tsx ===" rg -B 2 -A 2 "getFileUploaderMsg" "web/src/pages/Resolver/Policy/index.tsx" # Check for roleRestrictions usage echo -e "\n=== Role Restrictions Usage ===" ast-grep --pattern 'const { $$$, roleRestrictions, $$$ } = $_'Length of output: 1310
PR-Codex overview
This PR introduces a new utility function to generate file uploader messages based on role restrictions and updates components to utilize this function, enhancing user feedback regarding file upload requirements.
Detailed summary
getFileUploaderMsgfunction inweb/src/utils/index.tsto generate messages based on role restrictions.SubmitEvidenceModalto usegetFileUploaderMsgfor the file uploader message.Policycomponent to include file uploader message usinggetFileUploaderMsg.StyledFileUploadercomponents for better layout.Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes