Feat/file uploader message #99
Conversation
WalkthroughThis pull request introduces updates to the web project's dependency management and file upload functionality. The changes include a minor version bump for the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)
🪧 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
🧹 Nitpick comments (6)
web/src/utils/index.ts (4)
10-16: Add JSDoc documentation for the Role type.Consider adding documentation to explain the purpose and usage of this type, especially the units for
maxSize(bytes) and expected format forallowedMimeTypes.+/** + * Represents a role with its associated file upload restrictions. + * @property {string} name - The name of the role + * @property {Object} restriction - The upload restrictions for this role + * @property {number} restriction.maxSize - Maximum allowed file size in bytes + * @property {string[]} restriction.allowedMimeTypes - Array of allowed MIME types (e.g., "application/pdf", "image/*") + */ type Role = { name: string; restriction: { maxSize: number; allowedMimeTypes: string[]; }; };
18-20: Enhance type safety and add documentation.The function could benefit from improved type safety and documentation:
- Add return type annotation
- Add JSDoc documentation
- Consider using type guard for the find operation
+/** + * Generates a formatted message describing file upload restrictions for a given role. + * @param {Roles} role - The role to check restrictions for + * @param {Role[]} [roleRestrictions] - Array of role-based restrictions + * @returns {string | undefined} A formatted message with allowed file types and max size, or undefined if no restrictions found + */ -export const getFileUploaderMsg = (role: Roles, roleRestrictions?: Role[]) => { +export const getFileUploaderMsg = (role: Roles, roleRestrictions?: Role[]): string | undefined => { if (!roleRestrictions) return; - const restrictions = roleRestrictions.find((supportedRoles) => Roles[supportedRoles.name] === role); + const restrictions = roleRestrictions.find((supportedRole): supportedRole is Role => + Roles[supportedRole.name] === role + );
24-32: Improve MIME type processing and extract size conversion.Consider these improvements:
- Extract size conversion to a reusable utility
- Add validation for MIME type format
- Handle edge cases in type splitting
+const bytesToMB = (bytes: number): string => (bytes / (1024 * 1024)).toFixed(2); + +const formatMimeType = (type: string): string => { + if (!type.includes('/')) return type; + const [prefix, suffix] = type.split('/'); + if (!suffix || !prefix) return type; + return suffix === '*' ? prefix : suffix; +}; + export const getFileUploaderMsg = (role: Roles, roleRestrictions?: Role[]): string | undefined => { // ... previous code ... return `Allowed file types: [ ${restrictions.restriction.allowedMimeTypes - .map((type) => { - const [prefix, suffix] = type.split("/"); - if (!suffix) return prefix ?? null; - - return suffix === "*" ? prefix : suffix; - }) + .map(formatMimeType) .join(", ")} ], Max allowed size: ${ - (restrictions.restriction.maxSize / (1024 * 1024)).toFixed(2) + bytesToMB(restrictions.restriction.maxSize) } MB.`; };
Line range hint
80-82: Add error handling for missing role restrictions.Consider handling cases where role restrictions are undefined or invalid:
- Add fallback message
- Log warning for missing restrictions
+ const getRoleMessage = () => { + const restrictionsMessage = getFileUploaderMsg(Roles.Policy, roleRestrictions); + if (!restrictionsMessage) { + console.warn('No role restrictions found for Policy role'); + return 'Please upload a PDF file.'; + } + return restrictionsMessage; + }; + const fileFootMessage = "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) ?? ""); + "the relevant parts of the file content.\n" + + getRoleMessage();Also applies to: 94-98
web/src/pages/NewTransaction/Terms/Deliverable/index.tsx (2)
12-12: Consider using a more specific import path.The import path
src/utilscould be more specific to avoid potential naming conflicts and improve maintainability.-import { getFileUploaderMsg } from "src/utils"; +import { getFileUploaderMsg } from "src/utils/fileUploader";
94-98: Consider explicit variant values for better maintainability.Instead of falling back to undefined for mobile, consider explicitly setting the variant for both desktop and mobile views.
- variant={isDesktop ? "info" : undefined} + variant={isDesktop ? "info" : "compact"}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
web/package.json(1 hunks)web/src/pages/NewTransaction/Terms/Deliverable/index.tsx(5 hunks)web/src/utils/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/package.json
🔇 Additional comments (5)
web/src/utils/index.ts (4)
1-1: LGTM!The import statement is correctly added, and the package version bump is appropriate.
11-13: LGTM!The imports are correctly organized and necessary for the new functionality.
Line range hint
38-43: LGTM!The CSS updates improve responsiveness and text formatting:
- Dynamic margin using responsiveSize
- Proper text wrapping for the file uploader message
Line range hint
65-65: LGTM!Hooks are correctly placed at the top level of the component.
Also applies to: 67-67
web/src/pages/NewTransaction/Terms/Deliverable/index.tsx (1)
38-43: LGTM! Styling improvements enhance readability.The responsive margin and text alignment changes improve the component's visual presentation across different screen sizes.
✅ Deploy Preview for kleros-escrow-v2 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
597435f
PR-Codex overview
This PR updates the
@kleros/kleros-appdependency, adds a newgetFileUploaderMsgfunction for file upload restrictions, and modifies theDeliverablecomponent to utilize this function and adjust the file uploader's message and layout.Detailed summary
@kleros/kleros-appfrom^2.0.1to^2.0.2.Roletype andgetFileUploaderMsgfunction insrc/utils/index.ts.Deliverablecomponent to usegetFileUploaderMsg.StyledFileUploader.Summary by CodeRabbit
Release Notes
New Features
Dependency Updates
@kleros/kleros-apppackage to version 2.0.2.Improvements