register #2113
register #2113
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of changes to the campaign management module, focusing on attendance tracking and user management. The modifications include adding new components like Changes
Possibly related PRs
Suggested Reviewers
Poem
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: 15
🔭 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js (1)
Line range hint
28-82: Add breadcrumb configuration for the attendance route.The
CampaignBreadCrumbcomponent needs to be updated to include the attendance path in its configuration.Add the following configuration to the
crumbsarray:const crumbs = [ // ... existing crumbs + { + path: pathVar === "attendance" ? "" : `/${window?.contextPath}/employee/campaign/attendance`, + content: t("ATTENDANCE"), + show: pathVar === "attendance" ? true : false, + }, // ... remaining crumbs ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
health/micro-ui/web/micro-ui-internals/example/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/package.jsonis excluded by!**/*.json
📒 Files selected for processing (12)
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/ProgressBar.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/tableCustomStyle.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchAttendanceWithUserDetails.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useAttendanceWithUserDetails.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Attendance.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/ProgressBar.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useAttendanceWithUserDetails.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchAttendanceWithUserDetails.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/tableCustomStyle.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Attendance.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
[error] 477-477: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 826-826: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Attendance.js
[error] 138-138: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 139-139: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 140-140: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 141-141: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 311-311: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 312-316: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 320-327: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (8)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js (1)
22-22: LGTM! Clean integration of the attendance hook.The
useAttendanceWithUserDetailshook is properly imported and integrated into the campaign object, following the established pattern.Also applies to: 52-52
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1)
100-102: Verify the proxy endpoints configuration.The new health-related endpoints are properly added. However, ensure that:
- The backend services for these endpoints are deployed and accessible.
- The endpoints are configured with the correct permissions.
✅ Verification successful
Endpoints are properly integrated and used in the codebase
The health-related endpoints are well-integrated:
- Used in campaign-manager module components with proper versioning (v1)
- Include proper tenant-based access control
- Follow consistent API patterns alongside other established services
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the configuration and accessibility of the new endpoints # Check if the endpoints are referenced in the codebase echo "Checking endpoint usage in the codebase..." rg -l "health-attendance|health-individual" --type js # Check if there are any environment-specific configurations echo "Checking for environment-specific configurations..." fd -e json -e yaml -e yml -e env | xargs rg "health-attendance|health-individual"Length of output: 650
Script:
#!/bin/bash # Search for backend configurations and API implementations # Check for backend service configurations echo "Checking for backend service configurations..." rg -A 5 "census-service|health-attendance|health-individual" --type js # Check for auth/permission configurations echo "Checking for auth configurations..." rg -l "auth.*health|permission.*health" --type js # Check for API implementations echo "Checking for API implementations..." ast-grep --pattern 'fetch($_, $$$)' | rg "health-attendance|health-individual"Length of output: 17042
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/tableCustomStyle.js (3)
25-25: LGTM! Improved table styling.The border styling changes enhance visual separation between rows and cells while maintaining a clean look by removing vertical lines.
Also applies to: 30-30, 43-44, 57-58
Line range hint
72-172: LGTM! Well-implemented column freezing functionality.The
getTableCustomStylefunction provides a flexible solution for freezing columns with proper z-index management and shadow effects for better visual hierarchy.
3-3: Verify table scrolling behavior.The removal of overflow styles could affect table scrolling behavior, especially for tables with many columns or on smaller screens.
Also applies to: 6-6, 9-9, 12-12
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js (1)
19-19: LGTM! Route properly configured.The Attendance component is correctly imported and the route is properly added following the established pattern.
Also applies to: 158-159
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.js (1)
9-38: Well-structured popup component implementation!The SupervisorPopup component is well-implemented with:
- Clean separation of concerns
- Proper event handling for closing
- Consistent styling approach using flexbox
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js (1)
60-62: LGTM - Clean component registration!The new components are properly imported and registered following the existing pattern.
Also applies to: 181-183
| const staffResponse = await Digit.CustomService.getResponse({ | ||
| url: "/health-individual/v1/_search", | ||
| method: "POST", | ||
| params: { | ||
| limit: 1000, | ||
| offset: 0, | ||
| tenantId, | ||
| }, | ||
| body: { | ||
| Individual: { | ||
| id: Array.from(staffUserIds) | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Prevent incomplete data retrieval due to fixed limit in API calls
The current implementation uses a fixed limit of 1000 when fetching staff and individual details. If the number of staffUserIds or individualIds exceeds 1000, some data may not be retrieved, leading to incomplete results.
To ensure all relevant data is fetched, consider setting the limit based on the size of the IDs or implement pagination to handle larger datasets. Apply the following changes:
For fetching staff details:
params: {
- limit: 1000,
+ limit: staffUserIds.size || 1000,
offset: 0,
tenantId,
},For fetching individual details:
params: {
- limit: 1000,
+ limit: individualIds.size || 1000,
offset: 0,
tenantId,
},Alternatively, if the API supports it, you can remove the limit and offset parameters to fetch all matching records.
Also applies to: 79-92
| const userDetails = staffResponse?.Individual.map(user => { | ||
| // Find the register that contains this staff member | ||
| const registerWithStaff = registerResponse.attendanceRegister.find( | ||
| register => register.staff?.[0]?.userId === user.id | ||
| ); | ||
|
|
||
| return { | ||
| id: user.id, | ||
| name: user?.name?.givenName, | ||
| dob: user.dateOfBirth, | ||
| email: user?.email, | ||
| mobileNumber: user?.mobileNumber, | ||
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider renaming variables for consistency and clarity
The variable userDetails holds details about staff members. Renaming it to staffUserDetails would improve code readability and make the purpose of the variable clearer.
Apply this diff to rename the variable:
- const userDetails = staffResponse?.Individual.map(user => {
+ const staffUserDetails = staffResponse?.Individual.map(user => {
// Find the register that contains this staff member
const registerWithStaff = registerResponse.attendanceRegister.find(
register => register.staff?.[0]?.userId === user.id
);
return {
id: user.id,
name: user?.name?.givenName,
dob: user.dateOfBirth,
email: user?.email,
mobileNumber: user?.mobileNumber,
registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here
};
});Remember to update all subsequent references to userDetails accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const userDetails = staffResponse?.Individual.map(user => { | |
| // Find the register that contains this staff member | |
| const registerWithStaff = registerResponse.attendanceRegister.find( | |
| register => register.staff?.[0]?.userId === user.id | |
| ); | |
| return { | |
| id: user.id, | |
| name: user?.name?.givenName, | |
| dob: user.dateOfBirth, | |
| email: user?.email, | |
| mobileNumber: user?.mobileNumber, | |
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | |
| }; | |
| }); | |
| const staffUserDetails = staffResponse?.Individual.map(user => { | |
| // Find the register that contains this staff member | |
| const registerWithStaff = registerResponse.attendanceRegister.find( | |
| register => register.staff?.[0]?.userId === user.id | |
| ); | |
| return { | |
| id: user.id, | |
| name: user?.name?.givenName, | |
| dob: user.dateOfBirth, | |
| email: user?.email, | |
| mobileNumber: user?.mobileNumber, | |
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | |
| }; | |
| }); |
| import React from 'react'; | ||
|
|
||
| const ProgressBar = ({ amount, total }) => { | ||
| const percentage = Math.min((amount / total) * 100, 100); |
There was a problem hiding this comment.
Handle division by zero when calculating percentage
The total value is used as a divisor without checking if it's zero. If total is zero, this will result in Infinity or NaN, causing rendering issues.
Apply this diff to safely handle the case when total is zero:
- const percentage = Math.min((amount / total) * 100, 100);
+ const percentage = total > 0 ? Math.min((amount / total) * 100, 100) : 0;This ensures that when total is zero, the percentage will default to 0%.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const percentage = Math.min((amount / total) * 100, 100); | |
| const percentage = total > 0 ? Math.min((amount / total) * 100, 100) : 0; |
| import searchAttendanceWithUserDetails from "../hooks/services/searchAttendanceWithUserDetails"; | ||
|
|
||
| const useAttendanceWithUserDetails = ({ tenantId, limit, offset, config = {} }) => { | ||
| console.log("hook eorking"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debugging console.log statement and correct typo
There's a console.log statement on line 6 with a typo ("hook eorking"). Debugging statements should be removed from production code to prevent unnecessary console output.
Apply this diff to remove the debugging statement:
- console.log("hook eorking");| .progress-bar-container { | ||
| position: relative; | ||
| width: 20rem; | ||
| max-width: 20rem; | ||
| height: 20px; | ||
| background-color: #f3f3f3; | ||
| border-radius: 10px; | ||
| overflow: hidden; | ||
| border: 1px solid #d6d5d4; | ||
| } | ||
|
|
||
| /* The fill of the progress bar */ | ||
| .progress-bar-fill { | ||
| height: 100%; | ||
| background-color: #2BD27F; /* Use your PRIMARY_COLOR */ | ||
| transition: width 0.4s ease-in-out; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| /* Text inside the progress bar */ | ||
| .progress-bar-text { | ||
| position: absolute; | ||
| width: 100%; | ||
| text-align: center; | ||
| font-size: 14px; | ||
| color: #fff; | ||
| font-weight: bold; | ||
| pointer-events: none; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance progress bar accessibility and maintainability.
The progress bar implementation could be improved in several ways:
- Use design tokens for colors and dimensions
- Add hover/focus states for better accessibility
- Consider adding ARIA attributes for screen readers
.progress-bar-container {
position: relative;
- width: 20rem;
- max-width: 20rem;
+ width: var(--progress-bar-width, 20rem);
+ max-width: var(--progress-bar-max-width, 20rem);
height: 20px;
- background-color: #f3f3f3;
+ background-color: var(--progress-bar-bg, #f3f3f3);
border-radius: 10px;
overflow: hidden;
- border: 1px solid #d6d5d4;
+ border: 1px solid var(--border-color, #d6d5d4);
+ &:hover {
+ border-color: var(--border-color-hover, #a8a29e);
+ }
+ &:focus-within {
+ outline: 2px solid var(--focus-color, #0B4B66);
+ outline-offset: 2px;
+ }
}
.progress-bar-fill {
height: 100%;
- background-color: #2BD27F; /* Use your PRIMARY_COLOR */
+ background-color: var(--progress-success, #2BD27F);
transition: width 0.4s ease-in-out;
display: flex;
align-items: center;
justify-content: center;
}
.progress-bar-text {
position: absolute;
width: 100%;
text-align: center;
- font-size: 14px;
+ font-size: var(--text-sm, 14px);
- color: #fff;
+ color: var(--text-white, #fff);
font-weight: bold;
pointer-events: none;
+ user-select: none;
}Add the following HTML attributes to the progress bar container for better accessibility:
<div
class="progress-bar-container"
role="progressbar"
aria-valuenow="75"
aria-valuemin="0"
aria-valuemax="100"
>
<!-- progress bar content -->
</div>| {/* Progress Bar Section */} | ||
| <div style={{ display: "flex", justifyContent: "space-between", alignItems: "center", marginTop: "1rem" }}> | ||
| <div style={{ display: "flex", alignItems: "center", gap: "1rem" }}> | ||
| <span style={{ fontWeight: 600 }}>{t("REGISTER_ASSIGNED_USERS")}</span> | ||
| <ProgressBar amount={40} total={100} /> | ||
| <span style={{ fontWeight: 600 }}>{`${40}%`}</span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded progress values
The progress bar uses hardcoded values (40, 100) instead of calculating actual progress based on assigned users.
- <ProgressBar amount={40} total={100} />
- <span style={{ fontWeight: 600 }}>{`${40}%`}</span>
+ <ProgressBar
+ amount={staffAttendeeIds.length}
+ total={data?.Individual?.length || 0}
+ />
+ <span style={{ fontWeight: 600 }}>
+ {`${Math.round((staffAttendeeIds.length / (data?.Individual?.length || 1)) * 100)}%`}
+ </span>Committable suggestion skipped: line range outside the PR's diff.
| const numberA = parseInt(rowA.number, 10); | ||
| const numberB = parseInt(rowB.number, 10); | ||
| if (isNaN(numberA)) return 1; // Treat invalid numbers as larger | ||
| if (isNaN(numberB)) return -1; | ||
|
|
||
| if (numberA < numberB) return -1; | ||
| if (numberA > numberB) return 1; | ||
| return 0; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use safer number parsing methods
Replace global parseInt and isNaN with their Number counterparts for better type safety.
- const numberA = parseInt(rowA.number, 10);
- const numberB = parseInt(rowB.number, 10);
- if (isNaN(numberA)) return 1;
- if (isNaN(numberB)) return -1;
+ const numberA = Number.parseInt(rowA.number, 10);
+ const numberB = Number.parseInt(rowB.number, 10);
+ if (Number.isNaN(numberA)) return 1;
+ if (Number.isNaN(numberB)) return -1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const numberA = parseInt(rowA.number, 10); | |
| const numberB = parseInt(rowB.number, 10); | |
| if (isNaN(numberA)) return 1; // Treat invalid numbers as larger | |
| if (isNaN(numberB)) return -1; | |
| if (numberA < numberB) return -1; | |
| if (numberA > numberB) return 1; | |
| return 0; | |
| }, | |
| const numberA = Number.parseInt(rowA.number, 10); | |
| const numberB = Number.parseInt(rowB.number, 10); | |
| if (Number.isNaN(numberA)) return 1; // Treat invalid numbers as larger | |
| if (Number.isNaN(numberB)) return -1; | |
| if (numberA < numberB) return -1; | |
| if (numberA > numberB) return 1; | |
| return 0; | |
| }, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 139-139: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 140-140: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 141-141: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
| <span style={{ fontWeight: 600 }}>{t("REGISTER_ASSIGNED_USERS")}</span> | ||
| <ProgressBar amount={40} total={100} /> | ||
| <span style={{ fontWeight: 600 }}>{`${40}%`}</span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded progress values
Similar to IndividualUserTable, the progress values are hardcoded instead of reflecting actual data.
- <ProgressBar amount={40} total={100} />
- <span style={{ fontWeight: 600 }}>{`${40}%`}</span>
+ <ProgressBar
+ amount={userDetails?.totalAssigned || 0}
+ total={userDetails?.totalUsers || 0}
+ />
+ <span style={{ fontWeight: 600 }}>
+ {`${Math.round(((userDetails?.totalAssigned || 0) / (userDetails?.totalUsers || 1)) * 100)}%`}
+ </span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span style={{ fontWeight: 600 }}>{t("REGISTER_ASSIGNED_USERS")}</span> | |
| <ProgressBar amount={40} total={100} /> | |
| <span style={{ fontWeight: 600 }}>{`${40}%`}</span> | |
| </div> | |
| <span style={{ fontWeight: 600 }}>{t("REGISTER_ASSIGNED_USERS")}</span> | |
| <ProgressBar | |
| amount={userDetails?.totalAssigned || 0} | |
| total={userDetails?.totalUsers || 0} | |
| /> | |
| <span style={{ fontWeight: 600 }}> | |
| {`${Math.round(((userDetails?.totalAssigned || 0) / (userDetails?.totalUsers || 1)) * 100)}%`} | |
| </span> | |
| </div> |
| children={[ | ||
| <IndividualUserTable | ||
| tenantId={tenantId} | ||
| staffAttendeeIds={curAttendeesList} | ||
| supervisorName={curSupervisor} | ||
| /> | ||
| ]} | ||
| onOverlayClick={() => setAssignUsersPopup(false)} | ||
| footerChildren={[ | ||
| <Button | ||
| type="button" | ||
| size="large" | ||
| variation="secondary" | ||
| label={t("CLOSE")} | ||
| onClick={() => setAssignUsersPopup(false)} | ||
| style={{ minWidth: "200px" }} | ||
| /> |
There was a problem hiding this comment.
Add missing key props to elements in iterables
React requires key props for elements in arrays to optimize rendering and prevent potential issues.
children={[
<IndividualUserTable
+ key="individual-table"
tenantId={tenantId}
staffAttendeeIds={curAttendeesList}
supervisorName={curSupervisor}
/>
]}
footerChildren={[
<Button
+ key="close-button"
type="button"
size="large"
variation="secondary"
label={t("CLOSE")}
onClick={() => setAssignUsersPopup(false)}
style={{ minWidth: "200px" }}
/>
]}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| children={[ | |
| <IndividualUserTable | |
| tenantId={tenantId} | |
| staffAttendeeIds={curAttendeesList} | |
| supervisorName={curSupervisor} | |
| /> | |
| ]} | |
| onOverlayClick={() => setAssignUsersPopup(false)} | |
| footerChildren={[ | |
| <Button | |
| type="button" | |
| size="large" | |
| variation="secondary" | |
| label={t("CLOSE")} | |
| onClick={() => setAssignUsersPopup(false)} | |
| style={{ minWidth: "200px" }} | |
| /> | |
| children={[ | |
| <IndividualUserTable | |
| key="individual-table" | |
| tenantId={tenantId} | |
| staffAttendeeIds={curAttendeesList} | |
| supervisorName={curSupervisor} | |
| /> | |
| ]} | |
| onOverlayClick={() => setAssignUsersPopup(false)} | |
| footerChildren={[ | |
| <Button | |
| key="close-button" | |
| type="button" | |
| size="large" | |
| variation="secondary" | |
| label={t("CLOSE")} | |
| onClick={() => setAssignUsersPopup(false)} | |
| style={{ minWidth: "200px" }} | |
| /> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 311-311: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 312-316: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 320-327: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| case "ACTION_LABEL_CONFIGURE_REGISTER": | ||
| window.history.pushState( | ||
| { | ||
| name: row?.campaignName, | ||
| data: row, | ||
| projectId: row?.projectId, | ||
| }, | ||
| "", | ||
| `/${window.contextPath}/employee/campaign/attendance` | ||
| ); | ||
| const navEvent2 = new PopStateEvent("popstate"); | ||
| window.dispatchEvent(navEvent2); | ||
| break; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reduce code duplication in register configuration
The register configuration code is duplicated between MyCampaignConfigOngoing and MyCampaignConfigUpcoming. Consider extracting this into a shared function.
+ const handleRegisterConfiguration = (row) => {
+ window.history.pushState(
+ {
+ name: row?.campaignName,
+ data: row,
+ projectId: row?.projectId,
+ },
+ "",
+ `/${window.contextPath}/employee/campaign/attendance`
+ );
+ const navEvent = new PopStateEvent("popstate");
+ window.dispatchEvent(navEvent);
+ };
case "ACTION_LABEL_CONFIGURE_REGISTER":
- window.history.pushState(...)
- const navEvent2 = new PopStateEvent("popstate");
- window.dispatchEvent(navEvent2);
+ handleRegisterConfiguration(row);
break;Also applies to: 816-828
🧰 Tools
🪛 Biome (1.9.4)
[error] 477-477: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit
New Features
UI/UX Improvements
Technical Enhancements