HCMPRE 154 #1186
HCMPRE 154 #1186
Conversation
nabeelmd-eGov
commented
Jul 30, 2024
nabeelmd-eGov
commented
- date validation fix
- Update date logic
|
Warning Rate limit exceeded@nabeelmd-eGov has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 49 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWalkthroughThe recent updates enhance the micro UI framework by improving date validation and handling logic, alongside version upgrades for the CSS library. Key modifications include the introduction of a new validation helper function for date entries, refined error handling, and adjustments to the date processing to ensure better compatibility. These changes aim to elevate the reliability and user experience of date-related features within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Form
participant DateValidation
participant Backend
User->>Form: Submit Date
Form->>DateValidation: Validate Dates
DateValidation-->>Form: Validation Result
alt Valid
Form->>Backend: Send Data
Backend-->>Form: Response
else Invalid
Form-->>User: Show Error Message
end
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 as PR comments)
Additionally, you can add 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: 9
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
micro-ui/web/micro-ui-internals/example/package.jsonis excluded by!**/*.jsonmicro-ui/web/package.jsonis excluded by!**/*.jsonmicro-ui/web/workbench/package.jsonis excluded by!**/*.json
Files selected for processing (7)
- micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryWithDate.js (7 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/DateAndCycleUpdate.js (9 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateDatesWithBoundaries.js (2 hunks)
- micro-ui/web/public/index.html (1 hunks)
Files skipped from review due to trivial changes (2)
- micro-ui/web/micro-ui-internals/example/public/index.html
- micro-ui/web/public/index.html
Additional context used
Path-based instructions (4)
micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateDatesWithBoundaries.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryWithDate.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/DateAndCycleUpdate.js (1)
Pattern
**/*.js: check
Biome
micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryWithDate.js
[error] 150-150: 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] 176-176: 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)
micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/DateAndCycleUpdate.js
[error] 198-198: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 220-220: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 253-253: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 281-281: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 258-258: 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] 285-285: 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)
Additional comments not posted (10)
micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryWithDate.js (3)
31-33: LGTM!The added check for undefined date values improves robustness.
50-52: LGTM!The added check for undefined date values improves robustness.
63-63: LGTM!The correction in the type dispatched for the cycle end date ensures proper state management.
micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js (1)
65-65: LGTM!The correction in the case label from
"CYCYLE_END_DATE"to"CYCLE_END_DATE"ensures proper action recognition and processing.micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/DateAndCycleUpdate.js (5)
33-33: Ensure type safety for date conversion.The updated logic ensures that only valid date strings are processed. This enhances robustness against unexpected data types.
49-49: Corrected typo in action type.The typo in the case label has been corrected from
"CYCYLE_END_DATE"to"CYCLE_END_DATE", ensuring the correct action type is processed.
119-128: Prevent potential errors with conditional date setting.The checks for the existence of
startDateandendDateprevent potential errors when rendering components that depend on these values.
144-144: Clarify intent withundefinedcheck.The check for
undefineddates clarifies the intent and ensures correct handling of cases where the date might be explicitly set tonullorundefined.
163-163: Clarify intent withundefinedcheck.The check for
undefineddates clarifies the intent and ensures correct handling of cases where the date might be explicitly set tonullorundefined.micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1)
553-555: Enhance responsiveness withwidth: unset.The new CSS rule allows the element to inherit its width from its parent container, enhancing responsiveness.
| const checkValid = (data) => { | ||
| if (DateWithBoundary) { | ||
| const temp = data?.dateWithBoundary; | ||
| const allCycleDateValid = temp | ||
| .map((i) => i.additionalDetails.projectType.cycles.every((j) => j.startDate && j.endDate)) | ||
| .every((k) => k === true); | ||
| const allDateValid = temp.every((i) => i.startDate && i.endDate); | ||
|
|
||
| if (allCycleDateValid && allDateValid) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } else if (!DateWithBoundary) { | ||
| const cycleDateValid = data?.dateAndCycle?.additionalDetails?.projectType?.cycles?.every((item) => item?.startDate && item?.endDate); | ||
| if (data?.dateAndCycle?.startDate && data?.dateAndCycle?.endDate && cycleDateValid) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Simplify the checkValid function.
The function can be simplified by reducing redundancy and improving readability.
const checkValid = (data) => {
if (DateWithBoundary) {
const temp = data?.dateWithBoundary;
return temp.every((i) => i.startDate && i.endDate && i.additionalDetails.projectType.cycles.every((j) => j.startDate && j.endDate));
} else {
const cycleDateValid = data?.dateAndCycle?.additionalDetails?.projectType?.cycles?.every((item) => item?.startDate && item?.endDate);
return data?.dateAndCycle?.startDate && data?.dateAndCycle?.endDate && cycleDateValid;
}
};| if (DateWithBoundary) { | ||
| if (!formData?.dateWithBoundary || !Array.isArray(formData?.dateWithBoundary) || formData?.dateWithBoundary?.length === 0) { | ||
| setShowToast({ isError: true, label: "SELECT_BOUNDARY_LEVEL_ERROR" }); | ||
| return; | ||
| } | ||
| } else if (!DateWithBoundary) { | ||
| if (!formData?.dateAndCycle) { | ||
| setShowToast({ isError: true, label: "SELECT_DATE_CHANGE_MANDATORY_ERROR" }); | ||
| return; | ||
| } | ||
| } | ||
| let isValid = checkValid(formData); | ||
| if (!isValid) { | ||
| setShowToast({ isError: true, label: "MANDATORY_FIELDS_MISSING" }); |
There was a problem hiding this comment.
Enhance readability and optimize early returns in onSubmit.
The function can be improved by restructuring the conditional checks and optimizing early returns.
const onSubmit = async (formData) => {
if (DateWithBoundary) {
if (!formData?.dateWithBoundary || !Array.isArray(formData?.dateWithBoundary) || formData?.dateWithBoundary?.length === 0) {
setShowToast({ isError: true, label: "SELECT_BOUNDARY_LEVEL_ERROR" });
return;
}
} else {
if (!formData?.dateAndCycle) {
setShowToast({ isError: true, label: "SELECT_DATE_CHANGE_MANDATORY_ERROR" });
return;
}
}
const isValid = checkValid(formData);
if (!isValid) {
setShowToast({ isError: true, label: "MANDATORY_FIELDS_MISSING" });
return;
}
setShowPopUp(formData);
};| // updating the endDate by +1 sec and -1 sec so that backend logic for ancestor update work | ||
| const payload = formData?.dateWithBoundary?.map((item) => { | ||
| let itemEndDate = item?.endDate; | ||
| let endDate = new Date(item?.endDate); | ||
| let endSecond = endDate?.getSeconds(); | ||
| if (endSecond < 59) { | ||
| return { | ||
| ...item, | ||
| endDate: itemEndDate + 1000, | ||
| }; | ||
| } else { | ||
| return { | ||
| ...item, | ||
| endDate: itemEndDate - 1000, | ||
| }; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Enhance readability and consistency in onConfirm.
The function can be improved by restructuring the conditional checks and ensuring consistent date handling.
const onConfirm = async (formData) => {
setShowPopUp(null);
try {
if (DateWithBoundary) {
const payload = formData?.dateWithBoundary?.map((item) => {
const endDate = new Date(item?.endDate);
const endSecond = endDate?.getSeconds();
return {
...item,
endDate: endSecond < 59 ? item?.endDate + 1000 : item?.endDate - 1000,
};
});
await Digit.Hooks.campaign.useProjectUpdateWithBoundary({ formData: payload });
history.push(`/${window.contextPath}/employee/campaign/response?isSuccess=${true}`, {
message: t("ES_CAMPAIGN_DATE_CHANGE_WITH_BOUNDARY_SUCCESS"),
actionLabel: t("HCM_DATE_CHANGE_SUCCESS_RESPONSE_ACTION"),
actionLink: `/${window.contextPath}/employee/campaign/my-campaign`,
});
} else {
await Digit.CustomService.getResponse({
url: "/health-project/v1/_update",
body: { Projects: [formData?.dateAndCycle] },
});
history.push(`/${window.contextPath}/employee/campaign/response?isSuccess=${true}`, {
message: t("ES_CAMPAIGN_DATE_CHANGE_SUCCESS"),
actionLabel: t("HCM_DATE_CHANGE_SUCCESS_RESPONSE_ACTION"),
actionLink: `/${window.contextPath}/employee/campaign/my-campaign`,
});
}
} catch (error) {
setShowToast({ isError: true, label: error?.response?.data?.Errors?.[0]?.message || error });
}
};| withoutLabel={true} | ||
| type="date" | ||
| value={item?.startDate} | ||
| nonEditable={today >= item?.startDate ? true : false} | ||
| nonEditable={item?.startDate && item?.startDate?.length > 0 && today >= item?.startDate ? true : false} |
There was a problem hiding this comment.
Simplify the nonEditable property check.
The use of boolean literals in the conditional expression is unnecessary. Simplify the code by directly assigning the result.
- nonEditable={item?.startDate && item?.startDate?.length > 0 && today >= item?.startDate ? true : false}
+ nonEditable={item?.startDate && item?.startDate?.length > 0 && today >= item?.startDate}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.
| nonEditable={item?.startDate && item?.startDate?.length > 0 && today >= item?.startDate ? true : false} | |
| nonEditable={item?.startDate && item?.startDate?.length > 0 && today >= item?.startDate} |
Tools
Biome
[error] 253-253: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
| populators={{ | ||
| validation: { | ||
| min: | ||
| index > 0 && !isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index)?.endDate)?.getTime()) |
There was a problem hiding this comment.
Replace isNaN with Number.isNaN.
The use of isNaN is unsafe as it attempts a type coercion. Use Number.isNaN instead.
- index > 0 && !isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index)?.endDate)?.getTime())
+ index > 0 && !Number.isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index)?.endDate)?.getTime())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.
| index > 0 && !isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index)?.endDate)?.getTime()) | |
| index > 0 && !Number.isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index)?.endDate)?.getTime()) |
Tools
Biome
[error] 258-258: 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)
| } | ||
| populators={{ | ||
| validation: { | ||
| min: !isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index + 1)?.startDate)?.getTime()) |
There was a problem hiding this comment.
Replace isNaN with Number.isNaN.
The use of isNaN is unsafe as it attempts a type coercion. Use Number.isNaN instead.
- min: !isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index + 1)?.startDate)?.getTime())
+ min: !Number.isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index + 1)?.startDate)?.getTime())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.
| min: !isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index + 1)?.startDate)?.getTime()) | |
| min: !Number.isNaN(new Date(cycleDates?.find((j) => j.cycleIndex == index + 1)?.startDate)?.getTime()) |
Tools
Biome
[error] 285-285: 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)
| required={true} | ||
| withoutLabel={true} | ||
| type="date" | ||
| value={startDate} | ||
| nonEditable={today >= startDate ? true : false} | ||
| nonEditable={startDate && startDate?.length > 0 && today >= startDate ? true : false} |
There was a problem hiding this comment.
Simplify the nonEditable property check.
The use of boolean literals in the conditional expression is unnecessary. Simplify the code by directly assigning the result.
- nonEditable={startDate && startDate?.length > 0 && today >= startDate ? true : false}
+ nonEditable={startDate && startDate?.length > 0 && today >= startDate}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.
| required={true} | |
| withoutLabel={true} | |
| type="date" | |
| value={startDate} | |
| nonEditable={today >= startDate ? true : false} | |
| nonEditable={startDate && startDate?.length > 0 && today >= startDate ? true : false} | |
| required={true} | |
| withoutLabel={true} | |
| type="date" | |
| value={startDate} | |
| nonEditable={startDate && startDate?.length > 0 && today >= startDate} |
Tools
Biome
[error] 198-198: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
| withoutLabel={true} | ||
| type="date" | ||
| value={endDate} | ||
| nonEditable={today >= endDate ? true : false} | ||
| nonEditable={endDate && endDate?.length > 0 && today >= endDate ? true : false} |
There was a problem hiding this comment.
Simplify the nonEditable property check.
The use of boolean literals in the conditional expression is unnecessary. Simplify the code by directly assigning the result.
- nonEditable={endDate && endDate?.length > 0 && today >= endDate ? true : false}
+ nonEditable={endDate && endDate?.length > 0 && today >= endDate}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.
| nonEditable={endDate && endDate?.length > 0 && today >= endDate ? true : false} | |
| nonEditable={endDate && endDate?.length > 0 && today >= endDate} |
Tools
Biome
[error] 220-220: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
| withoutLabel={true} | ||
| type="date" | ||
| value={item?.endDate} | ||
| nonEditable={today >= item?.endDate ? true : false} | ||
| nonEditable={item?.endDate && item?.endDate?.length > 0 && today >= item?.endDate ? true : false} |
There was a problem hiding this comment.
Simplify the nonEditable property check.
The use of boolean literals in the conditional expression is unnecessary. Simplify the code by directly assigning the result.
- nonEditable={item?.endDate && item?.endDate?.length > 0 && today >= item?.endDate ? true : false}
+ nonEditable={item?.endDate && item?.endDate?.length > 0 && today >= item?.endDate}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.
| nonEditable={item?.endDate && item?.endDate?.length > 0 && today >= item?.endDate ? true : false} | |
| nonEditable={item?.endDate && item?.endDate?.length > 0 && today >= item?.endDate} |
Tools
Biome
[error] 281-281: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
* date validation fix * Update date logic * FIX --------- Co-authored-by: nabeelmd-eGov <[email protected]>
