-
Notifications
You must be signed in to change notification settings - Fork 20
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
Develop #990
Develop #990
Conversation
added comment
Iste 454 & 416 : Added hyperlink for adding localisation and Solved numeric problem issue
updated createdtime code
Created date issue
Added changes to allow Advance in electricity bill from pspcl
WalkthroughThe changes in this pull request involve multiple components across different modules, primarily focusing on the 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 (
|
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.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (5)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/hrmscard.js (1)
Line range hint 35-49
: Consider extracting workbench URLs to configuration constants.
While the implementation is correct, the hardcoded workbench URLs (/workbench-ui/employee/workbench/...
) should be moved to a configuration file or constants. This would make it easier to maintain and update these URLs across the application if the paths change in the future.
Example implementation:
// config.js
export const WORKBENCH_URLS = {
VILLAGE_MASTER: '/workbench-ui/employee/workbench/mdms-search-v2',
LOCALISATION: '/workbench-ui/employee/workbench/localisation-search'
};
// hrmscard.js
import { WORKBENCH_URLS } from './config';
// ...
{
label: t("WORK_BENCH_URL_VILLAGE_MASTER_DATA"),
link: `${window?.location?.origin}${WORKBENCH_URLS.VILLAGE_MASTER}?moduleName=tenant&masterName=tenants`,
category: t("HR_EDIT_MASTER"),
}
municipal-services/echallan-services/src/main/java/org/egov/echallan/validator/ChallanValidator.java (1)
Line range hint 1-180
: Improve code maintainability and documentation.
The validator contains several areas that could be improved:
- There are multiple commented-out validation blocks without explanation
- Validation patterns and error message formats are inconsistent
- The method is quite long and handles multiple concerns
Consider the following improvements:
- Remove or document commented code with TODO tickets
- Extract validation groups into separate methods
- Standardize error message format
- Add documentation explaining the validation rules and business logic
Example refactor for better organization:
@Component
@Slf4j
public class ChallanValidator {
private void validateTaxHeadCodes(List<String> current, List<String> required, Map<String, String> errorMap) {
if (!current.isEmpty() && !required.isEmpty()) {
if (!current.stream().anyMatch(required::contains)) {
errorMap.put(
ErrorCodes.INVALID_TAXHEAD_CODE,
"At least one mandatory tax head code must be present"
);
}
}
}
private void validateAmounts(List<Amount> amounts, Map<String, String> errorMap) {
// Extract amount validation logic here
}
// Additional extracted methods...
}
Would you like me to propose a complete refactoring plan?
frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/configs/UICustomizations.js (1)
185-193
: Enhance consumer code validation
The current validation only checks the length of the consumer code. Consider adding more robust validation:
- Check for valid characters
- Add specific format validation if applicable
Example enhancement:
if (!consumerCode) return false;
- if (consumerCode.length < 10 || consumerCode.length > 25) {
+ const validFormat = /^[A-Z0-9-_]+$/i; // adjust regex based on your requirements
+ if (consumerCode.length < 10 || consumerCode.length > 25 || !validFormat.test(consumerCode)) {
return { warning: true, label: "ES_COMMON_ENTER_VALID_CONSUMER_CODE" };
}
frontend/micro-ui/web/src/Customisations/UICustomizations.js (1)
Line range hint 96-122
: Improve error handling in updatePayload for works.purchase.
The function should validate the existence of required fields before accessing them to prevent potential runtime errors.
Apply this diff to add validation:
if (businessService === businessServiceMap?.["works.purchase"]) {
+ if (!applicationDetails?.additionalDetails?.projectId || !applicationDetails?.billDate || !applicationDetails?.referenceId) {
+ throw new Error("Missing required fields for works.purchase payload");
+ }
const workflow = {
// ... existing workflow code ...
};
business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (1)
232-239
: Remove commented-out code.
This block of commented code should be removed as it's no longer needed and the new implementation properly handles audit details. Keeping commented-out code reduces readability and can lead to confusion about which implementation is current.
-// for (Demand demand : demands) {
-// AuditDetails currAuditDetails = demand.getAuditDetails();
-// if (currAuditDetails != null) {
-// auditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
-// auditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
-// }
-// demand.setAuditDetails(auditDetail);
-// }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (3 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/hrmscard.js (3 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/Response.js (5 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/configs/UICustomizations.js (6 hunks)
- frontend/micro-ui/web/src/Customisations/UICustomizations.js (17 hunks)
- municipal-services/echallan-services/src/main/java/org/egov/echallan/validator/ChallanValidator.java (1 hunks)
🧰 Additional context used
🪛 Biome
frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/configs/UICustomizations.js
[error] 119-138: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
frontend/micro-ui/web/src/Customisations/UICustomizations.js
[error] 205-205: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 219-219: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 242-242: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 243-243: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 269-269: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 668-693: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🔇 Additional comments (8)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/Response.js (3)
25-25
: LGTM! Proper use of optional chaining.
The formatting improvement and safe property access using optional chaining is well implemented.
92-94
: LGTM! Proper loading state handling.
The loading state condition correctly handles both the initial loading and mutation states.
108-110
: 🛠️ Refactor suggestion
Consider using environment variables for context path.
Direct access to window.contextPath
could be unsafe. Consider using environment variables or a configuration service.
- <Link to={`${props.parentRoute.includes("employee") ? `/${window?.contextPath}/employee` : `/${window?.contextPath}/citizen`}`}>
+ // Add to your environment config
+ const BASE_CONTEXT_PATH = process.env.REACT_APP_CONTEXT_PATH || '';
+ // In component:
+ <Link to={`${props.parentRoute.includes("employee") ? `/${BASE_CONTEXT_PATH}/employee` : `/${BASE_CONTEXT_PATH}/citizen`}`}>
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/hrmscard.js (2)
97-101
: LGTM! Formatting improvements enhance readability.
The reformatting of the state reports link maintains functionality while improving code consistency.
Line range hint 35-49
: Verify workbench routes and translations.
Please ensure that:
- The new workbench routes (
/mdms-search-v2
and/localisation-search
) are properly configured in the workbench UI. - The translation keys are defined in the localization files.
Also applies to: 97-101
municipal-services/echallan-services/src/main/java/org/egov/echallan/validator/ChallanValidator.java (1)
94-97
:
Review the business impact of relaxed tax head validation.
The validation logic has been significantly relaxed from requiring all mandatory tax head codes to requiring just one. This change could lead to incomplete challans being generated.
Consider the following concerns:
- Is this relaxation intentional and approved by the business team?
- Could this lead to revenue leakage or billing issues?
- The error message is inconsistent with the new validation logic, as it still suggests all mandatory codes are required.
Let's check how tax head codes are used elsewhere:
If this change is intentional, consider updating the code as follows:
- if (!(currentTaxHeadCodes.stream().anyMatch(requiredTaxHeadCodes::contains)))
- errorMap.put("INVALID_TAXHEAD_CODE_DETAILS",
- "Mandatory taxhead codes details are not present in request for provided business service");
+ if (!currentTaxHeadCodes.stream().anyMatch(requiredTaxHeadCodes::contains)) {
+ errorMap.put("INVALID_TAXHEAD_CODE_DETAILS",
+ "At least one mandatory tax head code must be present in the request for the provided business service");
+ }
frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/configs/UICustomizations.js (1)
4-4
: LGTM: Import added for internationalization support
The addition of useTranslation import is appropriate for the i18n implementation throughout the file.
business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (1)
Line range hint 253-275
: LGTM! Improved audit details handling.
The new implementation correctly handles audit details by:
- Creating separate audit detail objects for each demand
- Properly preserving creation information while updating modification details
- Consistently applying the same pattern to both demands and demand details
This approach prevents shared state issues and maintains proper audit history.
|
||
const onError = (error, variables) => { | ||
setErrorInfo(error?.response?.data?.Errors[0]?.code || 'ERROR'); | ||
setErrorInfo(error?.response?.data?.Errors[0]?.code || "ERROR"); |
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.
🛠️ Refactor suggestion
Standardize error handling approach.
The error message extraction is inconsistent between the onError
function and DisplayText
function. Consider creating a utility function for consistent error handling.
+ const getErrorMessage = (error) => {
+ return error?.response?.data?.Errors?.[0]?.code || "ERROR";
+ };
const onError = (error, variables) => {
- setErrorInfo(error?.response?.data?.Errors[0]?.code || "ERROR");
+ setErrorInfo(getErrorMessage(error));
setMutationHappened(true);
};
const DisplayText = (action, isSuccess, isEmployee, t) => {
if (!isSuccess) {
- return mutation?.error?.response?.data?.Errors[0].code || "ERROR";
+ return getErrorMessage(mutation?.error);
}
// ...
};
Also applies to: 86-86
useEffect(() => { | ||
if (!window.location.href.includes("/hrms/create") && sessionFormData && Object.keys(sessionFormData) != 0) { | ||
clearSessionFormData(); | ||
clearSessionFormData(); | ||
} | ||
},[location]); | ||
}, [location]); |
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.
Fix useEffect implementation issues.
The cleanup effect has several issues that should be addressed:
- Use strict equality (
!==
) instead of loose equality (!=
) - The
location
dependency isn't defined (should be from react-router) - Missing cleanup function
Here's the suggested fix:
- useEffect(() => {
- if (!window.location.href.includes("/hrms/create") && sessionFormData && Object.keys(sessionFormData) != 0) {
- clearSessionFormData();
- }
- }, [location]);
+ import { useLocation } from 'react-router-dom';
+ // Add at the component start:
+ const location = useLocation();
+
+ useEffect(() => {
+ if (!window.location.href.includes("/hrms/create") && sessionFormData && Object.keys(sessionFormData) !== 0) {
+ clearSessionFormData();
+ }
+ return () => {
+ // Cleanup if needed
+ };
+ }, [location, sessionFormData, clearSessionFormData]);
Committable suggestion was skipped due to low confidence.
const result = data?.MdmsRes?.tenant?.tenants | ||
?.filter((row) => row?.divisionCode && row?.divisionName) | ||
?.map((row) => { | ||
return { | ||
...row, | ||
updatedCode: `${row?.divisionName} - ${t(row?.code)}`, | ||
}; | ||
}); |
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.
🛠️ Refactor suggestion
Consider memoizing the transformed data
The data transformation and sorting operation could be expensive for large datasets. Consider memoizing the result using useMemo if this component re-renders frequently.
const memoizedResult = useMemo(() => {
const result = data?.MdmsRes?.tenant?.tenants
?.filter((row) => row?.divisionCode && row?.divisionName)
?.map((row) => ({
...row,
updatedCode: `${row?.divisionName} - ${t(row?.code)}`,
}))
?.sort((a, b) => {
const nameA = (a.divisionName || "").toLowerCase().trim();
const nameB = (b?.divisionName || "").toLowerCase().trim();
return nameA.localeCompare(nameB);
});
return result;
}, [data?.MdmsRes?.tenant?.tenants, t]);
if (key === "OP_BILL_TOTAL_AMT") { | ||
return <span>{`₹ ${value}`}</span>; | ||
} | ||
|
||
if(key === "OP_CONS_CODE") { | ||
return <span className="link"> | ||
if (key === "OP_CONS_CODE") { | ||
return ( | ||
<span className="link"> | ||
<Link | ||
to={`/${window.contextPath}/citizen/payment/open-view?tenantId=${row.tenantId}&businessService=${row.businessService}&consumerCode=${row.consumerCode}`} | ||
> | ||
{String(value ? (column.translate ? t(column.prefix ? `${column.prefix}${value}` : value) : value) : t("ES_COMMON_NA"))} | ||
</Link> | ||
</span> | ||
</span> | ||
); | ||
} | ||
}, |
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.
Fix unreachable code after switch statement
The code block after the switch statement is unreachable due to the default case that always returns. This means the conditions for 'OP_BILL_DATE', 'OP_BILL_TOTAL_AMT', and 'OP_CONS_CODE' will never be executed.
Move these conditions into the switch statement:
switch (key) {
case "OP_CONS_CODE":
return (
<span className="link">
<Link
to={`/${window.contextPath}/citizen/payment/open-view?tenantId=${row.tenantId}&businessService=WS&consumerCode=${row.connectionNo}`}
>
{String(value ? (column.translate ? t(column.prefix ? `${column.prefix}${value}` : value) : value) : t("ES_COMMON_NA"))}
</Link>
</span>
);
+ case "OP_BILL_DATE":
+ return Digit.DateUtils.ConvertEpochToDate(value);
+ case "OP_BILL_TOTAL_AMT":
+ return <span>{`₹ ${value}`}</span>;
default:
return <span>{t("ES_COMMON_DEFAULT_NA")}</span>;
}
- if (key === "OP_BILL_DATE") {
- return Digit.DateUtils.ConvertEpochToDate(value);
- }
- if (key === "OP_BILL_TOTAL_AMT") {
- return <span>{`₹ ${value}`}</span>;
- }
- if (key === "OP_CONS_CODE") {
- return (
- <span className="link">
- <Link
- to={`/${window.contextPath}/citizen/payment/open-view?tenantId=${row.tenantId}&businessService=${row.businessService}&consumerCode=${row.consumerCode}`}
- >
- {String(value ? (column.translate ? t(column.prefix ? `${column.prefix}${value}` : value) : value) : t("ES_COMMON_NA"))}
- </Link>
- </span>
- );
- }
Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 119-138: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
let { consumerCode, businessService, tenantId } = data?.state?.searchForm || {}; | ||
businessService = businessService?.code; | ||
tenantId = tenantId?.[0]?.code; |
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.
Add null checks for nested property destructuring
The current destructuring could throw errors if data.state is undefined.
Consider adding null checks:
- let { consumerCode, businessService, tenantId } = data?.state?.searchForm || {};
- businessService = businessService?.code;
- tenantId = tenantId?.[0]?.code;
+ let { consumerCode, businessService, tenantId } = data?.state?.searchForm ?? {};
+ businessService = businessService?.code ?? undefined;
+ tenantId = tenantId?.[0]?.code ?? undefined;
📝 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.
let { consumerCode, businessService, tenantId } = data?.state?.searchForm || {}; | |
businessService = businessService?.code; | |
tenantId = tenantId?.[0]?.code; | |
let { consumerCode, businessService, tenantId } = data?.state?.searchForm ?? {}; | |
businessService = businessService?.code ?? undefined; | |
tenantId = tenantId?.[0]?.code ?? undefined; |
if (key === "ATM_AMOUNT_IN_RS") { | ||
return ( | ||
<span> | ||
{value | ||
? Digit.Utils.dss.formatterWithoutRound(value, "number") | ||
: t("ES_COMMON_NA")} | ||
</span> | ||
); |
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.
🛠️ Refactor suggestion
Improve amount formatting.
The current implementation might not handle negative amounts or zero values appropriately.
Consider adding more robust amount formatting:
if (key === "ATM_AMOUNT_IN_RS") {
return (
<span>
- {value
- ? Digit.Utils.dss.formatterWithoutRound(value, "number")
- : t("ES_COMMON_NA")}
+ {typeof value === 'number'
+ ? Digit.Utils.dss.formatterWithoutRound(Math.abs(value), "number")
+ : t("ES_COMMON_NA")}
</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.
if (key === "ATM_AMOUNT_IN_RS") { | |
return ( | |
<span> | |
{value | |
? Digit.Utils.dss.formatterWithoutRound(value, "number") | |
: t("ES_COMMON_NA")} | |
</span> | |
); | |
if (key === "ATM_AMOUNT_IN_RS") { | |
return ( | |
<span> | |
{typeof value === 'number' | |
? Digit.Utils.dss.formatterWithoutRound(Math.abs(value), "number") | |
: t("ES_COMMON_NA")} | |
</span> | |
); |
const result = data?.MdmsRes?.tenant?.tenants | ||
?.filter((row) => row?.divisionCode && row?.divisionName) | ||
?.map((row) => { | ||
return { | ||
...row, | ||
updatedCode: `${row?.divisionName} - ${t(row?.code)}`, | ||
}; | ||
}); |
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.
🛠️ Refactor suggestion
Improve MDMS data transformation.
The current implementation might not handle edge cases where division details are missing.
Consider adding more robust data transformation:
select: (data) => {
const result = data?.MdmsRes?.tenant?.tenants
- ?.filter((row) => row?.divisionCode && row?.divisionName)
+ ?.filter((row) => Boolean(row?.divisionCode && row?.divisionName))
?.map((row) => ({
...row,
- updatedCode: `${row?.divisionName} - ${t(row?.code)}`,
+ updatedCode: `${row.divisionName} - ${t(row.code || '')}`.trim(),
}));
- return result;
+ return result || [];
},
📝 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 result = data?.MdmsRes?.tenant?.tenants | |
?.filter((row) => row?.divisionCode && row?.divisionName) | |
?.map((row) => { | |
return { | |
...row, | |
updatedCode: `${row?.divisionName} - ${t(row?.code)}`, | |
}; | |
}); | |
const result = data?.MdmsRes?.tenant?.tenants | |
?.filter((row) => Boolean(row?.divisionCode && row?.divisionName)) | |
?.map((row) => ({ | |
...row, | |
updatedCode: `${row.divisionName} - ${t(row.code || '')}`.trim(), | |
})); | |
return result || []; |
if (key === "OP_BILL_DATE") { | ||
return Digit.DateUtils.ConvertEpochToDate(value); | ||
} | ||
|
||
if(key === "OP_BILL_TOTAL_AMT"){ | ||
return <span>{`₹ ${value}`}</span> | ||
if (key === "OP_BILL_TOTAL_AMT") { | ||
return <span>{`₹ ${value}`}</span>; | ||
} | ||
|
||
if(key === "OP_CONS_CODE") { | ||
return <span className="link"> | ||
if (key === "OP_CONS_CODE") { | ||
return ( | ||
<span className="link"> | ||
<Link | ||
to={`/${window.contextPath}/citizen/payment/open-view?tenantId=${row.tenantId}&businessService=${row.businessService}&consumerCode=${row.consumerCode}`} | ||
> | ||
{String(value ? (column.translate ? t(column.prefix ? `${column.prefix}${value}` : value) : value) : t("ES_COMMON_NA"))} | ||
{String( | ||
value | ||
? column.translate | ||
? t(column.prefix ? `${column.prefix}${value}` : value) | ||
: value | ||
: t("ES_COMMON_NA") | ||
)} | ||
</Link> | ||
</span> | ||
</span> | ||
); | ||
} | ||
}, |
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.
Fix unreachable code in additionalCustomizations.
There's unreachable code after the switch statement due to duplicate handling of "OP_CONS_CODE" and other cases.
Remove the duplicate code block:
switch (key) {
// ... existing cases ...
default:
return <span>{t("ES_COMMON_DEFAULT_NA")}</span>;
}
- if (key === "OP_BILL_DATE") {
- return Digit.DateUtils.ConvertEpochToDate(value);
- }
- // ... remove all code until line 693 ...
Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 668-693: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
if (!consumerCode) return false; | ||
if (consumerCode.length < 10 || consumerCode.length > 25) { | ||
return { warning: true, label: "ES_COMMON_ENTER_VALID_CONSUMER_CODE" }; | ||
} | ||
// if ((createdFrom === "" && createdTo !== "") || (createdFrom !== "" && createdTo === "")) | ||
// return { warning: true, label: "ES_COMMON_ENTER_DATE_RANGE" }; | ||
|
||
return false; | ||
} | ||
} | ||
}, |
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.
🛠️ Refactor suggestion
Enhance consumer code validation.
The validation logic for consumer code could be more robust by checking for valid characters.
Consider adding character validation:
if (!consumerCode) return false;
- if (consumerCode.length < 10 || consumerCode.length > 25) {
+ if (consumerCode.length < 10 || consumerCode.length > 25 || !/^[a-zA-Z0-9-_\/]+$/.test(consumerCode)) {
return { warning: true, label: "ES_COMMON_ENTER_VALID_CONSUMER_CODE" };
}
📝 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.
if (!consumerCode) return false; | |
if (consumerCode.length < 10 || consumerCode.length > 25) { | |
return { warning: true, label: "ES_COMMON_ENTER_VALID_CONSUMER_CODE" }; | |
} | |
// if ((createdFrom === "" && createdTo !== "") || (createdFrom !== "" && createdTo === "")) | |
// return { warning: true, label: "ES_COMMON_ENTER_DATE_RANGE" }; | |
return false; | |
} | |
} | |
}, | |
if (!consumerCode) return false; | |
if (consumerCode.length < 10 || consumerCode.length > 25 || !/^[a-zA-Z0-9-_\/]+$/.test(consumerCode)) { | |
return { warning: true, label: "ES_COMMON_ENTER_VALID_CONSUMER_CODE" }; | |
} | |
// if ((createdFrom === "" && createdTo !== "") || (createdFrom !== "" && createdTo === "")) | |
// return { warning: true, label: "ES_COMMON_ENTER_DATE_RANGE" }; | |
return false; | |
}, |
data.body.inbox.processSearchCriteria.tenantId = | ||
Digit.ULBService.getCurrentTenantId(); | ||
|
||
const musterRollNumber = data?.body?.inbox?.moduleSearchCriteria?.musterRollNumber?.trim(); | ||
if(musterRollNumber) data.body.inbox.moduleSearchCriteria.musterRollNumber = musterRollNumber | ||
const musterRollNumber = | ||
data?.body?.inbox?.moduleSearchCriteria?.musterRollNumber?.trim(); | ||
if (musterRollNumber) | ||
data.body.inbox.moduleSearchCriteria.musterRollNumber = | ||
musterRollNumber; | ||
|
||
const attendanceRegisterName = data?.body?.inbox?.moduleSearchCriteria?.attendanceRegisterName?.trim(); | ||
if(attendanceRegisterName) data.body.inbox.moduleSearchCriteria.attendanceRegisterName = attendanceRegisterName | ||
const attendanceRegisterName = | ||
data?.body?.inbox?.moduleSearchCriteria?.attendanceRegisterName?.trim(); | ||
if (attendanceRegisterName) | ||
data.body.inbox.moduleSearchCriteria.attendanceRegisterName = | ||
attendanceRegisterName; |
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.
Improve string validation in preProcess.
The current implementation uses optional chaining with trim() which could throw if the value is null/undefined.
Apply this diff to add safer string validation:
- const musterRollNumber = data?.body?.inbox?.moduleSearchCriteria?.musterRollNumber?.trim();
+ const musterRollNumber = data?.body?.inbox?.moduleSearchCriteria?.musterRollNumber;
+ const trimmedMusterRollNumber = typeof musterRollNumber === 'string' ? musterRollNumber.trim() : undefined;
- if (musterRollNumber)
+ if (trimmedMusterRollNumber)
data.body.inbox.moduleSearchCriteria.musterRollNumber = trimmedMusterRollNumber;
Committable suggestion was skipped due to low confidence.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style