Develop #837
Develop #837
Conversation
Hrms chnages prebuiild
HRMS- v2 changes
updated query params
![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
Outside diff range and nitpick comments (7)
frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/components/OpenView.js (7)
72-76: Omit unnecessaryelseclause.The
elseclause can be omitted because previous branches break early.- } else if (payments?.length > 5) { - return payments.slice(0, 5); - } else { - return payments; - } + } + if (payments?.length > 5) { + return payments.slice(0, 5); + } + return payments;Tools
Biome
[error] 72-76: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 74-76: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
137-141: Move variable declaration to the root of the enclosing function.Using
varcan lead to unexpected behavior due to its function-scoped nature. Replace it withletorconstfor block-scoped variables.- var newForm = $("<form>", { + let newForm = $("<form>", {Tools
Biome
[error] 137-141: This var should be declared at the root of the enclosing function.
The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.(lint/correctness/noInnerDeclarations)
194-194: Fix unsafe usage of optional chaining.The optional chaining usage is unsafe and can throw a TypeError if it short-circuits with 'undefined'.
- if (error.response?.data?.Errors?.[0]) { + if (error?.response?.data?.Errors?.[0]) {Tools
Biome
[error] 194-194: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
323-324: Add 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.
- <Card style={{ maxWidth: "95vw", paddingLeft: "1.5rem", marginTop: "2rem" }}> + <Card key={payment?.paymentDetails?.[0]?.receiptNumber || payment?.transactionNumber} style={{ maxWidth: "95vw", paddingLeft: "1.5rem", marginTop: "2rem" }}>Tools
Biome
[error] 323-324: 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)
137-141: Move variable declaration to the root of the enclosing function.Using
varcan lead to unexpected behavior due to its function-scoped nature. Replace it withletorconstfor block-scoped variables.- var newForm = $("<form>", { + let newForm = $("<form>", {Tools
Biome
[error] 137-141: This var should be declared at the root of the enclosing function.
The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.(lint/correctness/noInnerDeclarations)
194-194: Fix unsafe usage of optional chaining.The optional chaining usage is unsafe and can throw a TypeError if it short-circuits with 'undefined'.
- if (error.response?.data?.Errors?.[0]) { + if (error?.response?.data?.Errors?.[0]) {Tools
Biome
[error] 194-194: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
72-76: Omit unnecessaryelseclause.The
elseclause can be omitted because previous branches break early.- } else if (payments?.length > 5) { - return payments.slice(0, 5); - } else { - return payments; - } + } + if (payments?.length > 5) { + return payments.slice(0, 5); + } + return payments;Tools
Biome
[error] 72-76: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 74-76: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- frontend/micro-ui/web/micro-ui-internals/example/devpackage.json (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/example/package.json (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/package.json (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/components/OpenView.js (1 hunks)
- frontend/micro-ui/web/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- frontend/micro-ui/web/micro-ui-internals/example/devpackage.json
- frontend/micro-ui/web/micro-ui-internals/example/package.json
- frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/package.json
- frontend/micro-ui/web/package.json
Additional context used
Biome
frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/components/OpenView.js
[error] 72-76: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 74-76: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 137-141: This var should be declared at the root of the enclosing function.
The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.(lint/correctness/noInnerDeclarations)
[error] 194-194: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 320-349: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 323-324: 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 not posted (3)
frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/components/OpenView.js (3)
7-15: LGTM!The
anonymizeHalfStringfunction is correct and efficient.
39-56: LGTM!The
requestCriteriaForConnectionSearchfunction is correct and efficient.
20-35: LGTM!The
requestCriteriafunction is correct and efficient.
| name: bill?.payerName, | ||
| mobileNumber: bill?.mobileNumber, | ||
| tenantId: bill?.tenantId, | ||
| emailId: "[email protected]", |
There was a problem hiding this comment.
Potential PII leakage.
The email address should not be hardcoded.
- emailId: "[email protected]",
+ emailId: bill?.emailId,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.
| name: bill?.payerName, | |
| mobileNumber: bill?.mobileNumber, | |
| tenantId: bill?.tenantId, | |
| emailId: "[email protected]", | |
| name: bill?.payerName, | |
| mobileNumber: bill?.mobileNumber, | |
| tenantId: bill?.tenantId, | |
| emailId: bill?.emailId, |
…mseva into payment-module-develop-dependent-drop
…ndent-drop Search Users Screen (dependent dropdowns)
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (9)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/SearchUser.js (2)
1-6: Optimize import statements.The
useEffectanduseStatehooks should be imported fromreactin a single import statement to follow best practices.-import React,{useState,useEffect} from 'react' +import React, { useState, useEffect } from 'react'
36-47: Ensure proper accessibility and semantic HTML.Consider adding appropriate ARIA roles and properties to improve accessibility.
<div className="inbox-search-component-wrapper" role="search"> <div className={`sections-parent search`} role="region"> <Header>{t("HR_SU")}</Header> <SearchUserForm uniqueTenants={uniqueTenants} setUniqueTenants={setUniqueTenants} roles={roles} setUniqueRoles={setUniqueRoles} /> </div> <div style={{marginTop:"1.5rem"}} role="region"> <SearchUserResults isLoading={isLoading} data={data} /> </div> </div>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/SearchUserResults.js (3)
1-6: Optimize import statements.The
useTranslationhook should be imported fromreact-i18nextin a single import statement to follow best practices.import React from 'react' import { useTranslation } from "react-i18next"; import { Table, Loader, Card } from "@egovernments/digit-ui-react-components"; import { Link } from "react-router-dom";
17-63: Simplify conditional rendering in cell components.The
GetCellandGetSlaCellfunctions can be simplified for better readability.const GetCell = (value) => <span className="cell-text">{t(value)}</span>; const GetSlaCell = (value) => ( <span className={value === "INACTIVE" ? "sla-cell-error" : "sla-cell-success"}> {t(value) || ""} </span> );
67-113: Ensure proper accessibility and semantic HTML.Consider adding appropriate ARIA roles and properties to improve accessibility.
<div role="table"> {isLoading ? ( <Loader /> ) : data?.length === 0 ? ( <Card style={{ marginTop: 20 }}> {t("COMMON_TABLE_NO_RECORD_FOUND") .split("\\n") .map((text, index) => ( <p key={index} style={{ textAlign: "center" }}> {text} </p> ))} </Card> ) : ( <Table t={t} data={data} columns={columns} getCellProps={(cellInfo) => ({ style: { maxWidth: cellInfo.column.Header === t("HR_EMP_ID_LABEL") ? "150px" : "", padding: "20px 18px", fontSize: "16px", minWidth: "150px", }, })} autoSort manualPagination={false} /> )} </div>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/MultiSelectDropdown.js (2)
1-4: Optimize import statements.The
useEffect,useReducer,useRef, anduseStatehooks should be imported fromreactin a single import statement to follow best practices.import React, { useEffect, useReducer, useRef, useState } from "react"; import { ArrowDown, CheckSvg } from "@egovernments/digit-ui-react-components"; import { useTranslation } from "react-i18next";
84-108: Simplify conditional rendering in menu item components.The
checkedattribute in theinputelement can be simplified for better readability.const MenuItem = ({ option, index }) => ( <div key={index} className={`${option.isDisabled ? "disabled" : ""}`} style={isOBPSMultiple ? (index % 2 !== 0 ? { background: "#EEEEEE" } : {}) : {}}> <input type="checkbox" value={option[optionsKey]} checked={!!alreadyQueuedSelectedState.find((selectedOption) => selectedOption[optionsKey] === option[optionsKey])} onChange={(e) => isPropsNeeded ? onSelectToAddToQueue(e, option, props) : isOBPSMultiple ? onSelectToAddToQueue(e, option, BlockNumber) : onSelectToAddToQueue(e, option)} style={{ minWidth: "24px", width: "100%" }} disabled={option.isDisabled || false} /> <div className="custom-checkbox"> <CheckSvg style={{ innerWidth: "24px", width: "24px" }} fill={option.isDisabled ? "#505050" : "#F47738"} /> </div> <p className="label" style={index === optionIndex ? { opacity: 1, backgroundColor: "rgba(238, 238, 238, var(--bg-opacity))" } : { }}>{t(option[optionsKey] && typeof option[optionsKey] === "string" && option[optionsKey])}</p> </div> );Tools
Biome
[error] 89-89: 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)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/SearchUserForm.js (2)
1-16: Optimize import statements.The
useState,useMemo, anduseEffecthooks should be imported fromreactin a single import statement to follow best practices.import { Loader, Header, Dropdown, LabelFieldPair, CardLabel, LinkLabel, SubmitBar, Toast } from "@egovernments/digit-ui-react-components"; import React, { useState, useMemo, useEffect } from "react"; import { useTranslation } from "react-i18next"; import { Controller, useForm, useWatch } from "react-hook-form"; import MultiSelectDropdown from "./MultiSelectDropdown";Tools
Biome
[error] 10-10: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
370-441: Ensure proper accessibility and semantic HTML.Consider adding appropriate ARIA roles and properties to improve accessibility.
<div className={"search-wrapper"} role="search"> <form onSubmit={handleSubmit(onSubmit)}> <div> <p className="search-instruction-header">{t("HR_SU_HINT")}</p> <div className={`search-field-wrapper search`}> {renderHierarchyFields} <LabelFieldPair> <CardLabel </blockquote></details> </blockquote></details> <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 54c7cab39a8829fd37fd10bbd6d6398f10e631d9 and ca89982714605fb765ea0fce9433a837ef25fff9. </details> <details> <summary>Files selected for processing (10)</summary> * frontend/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/css/package.json (1 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/MultiSelectDropdown.js (1 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/SearchUserForm.js (1 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/SearchUserResults.js (1 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/hrmscard.js (4 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/SearchUser.js (1 hunks) * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/index.js (4 hunks) * frontend/micro-ui/web/package.json (1 hunks) * frontend/micro-ui/web/public/index.html (1 hunks) </details> <details> <summary>Files skipped from review as they are similar to previous changes (6)</summary> * frontend/micro-ui/web/micro-ui-internals/example/public/index.html * frontend/micro-ui/web/micro-ui-internals/packages/css/package.json * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/hrmscard.js * frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/index.js * frontend/micro-ui/web/package.json * frontend/micro-ui/web/public/index.html </details> <details> <summary>Additional context used</summary> <details> <summary>Biome</summary><blockquote> <details> <summary>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/MultiSelectDropdown.js</summary><blockquote> [error] 17-17: 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] 89-89: 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] 57-57: Use the rest parameters instead of arguments. arguments does not have Array.prototype methods and can be inconvenient to use. (lint/style/noArguments) --- [error] 58-58: Use the rest parameters instead of arguments. arguments does not have Array.prototype methods and can be inconvenient to use. (lint/style/noArguments) --- [error] 58-58: Use the rest parameters instead of arguments. arguments does not have Array.prototype methods and can be inconvenient to use. (lint/style/noArguments) </blockquote></details> <details> <summary>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/SearchUserForm.js</summary><blockquote> [error] 10-10: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) --- [error] 61-61: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) --- [error] 328-328: 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) </blockquote></details> </blockquote></details> </details> <details> <summary>Additional comments not posted (3)</summary><blockquote> <details> <summary>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/SearchUser.js (1)</summary><blockquote> `49-49`: **LGTM!** The export statement is correct and follows best practices. </blockquote></details> <details> <summary>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/SearchUserResults.js (1)</summary><blockquote> `122-122`: **LGTM!** The export statement is correct and follows best practices. </blockquote></details> <details> <summary>frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/MultiSelectDropdown.js (1)</summary><blockquote> `141-141`: **LGTM!** The export statement is correct and follows best practices. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| const requestCriteriaForEmployeeSearch = { | ||
| url: "/egov-hrms/employees/_searchListOfEmployee", | ||
| params: {}, | ||
| body: { | ||
| criteria:{ | ||
| tenantIds:uniqueTenants, | ||
| roles:roles, | ||
| type:"EMPLOYEE" | ||
| } | ||
| }, | ||
| config: { | ||
| enabled: !!uniqueTenants && !!roles, | ||
| select: (data) => { | ||
| return data?.Employees | ||
| }, | ||
|
|
||
| }, | ||
| changeQueryName:{uniqueTenants,roles} | ||
| }; | ||
|
|
||
| const { isLoading, data, revalidate, isFetching, error } = Digit.Hooks.useCustomAPIHook(requestCriteriaForEmployeeSearch); |
There was a problem hiding this comment.
Ensure proper error handling for API requests.
The current implementation does not handle errors from the API request. Consider adding error handling logic to manage potential failures gracefully.
const { isLoading, data, revalidate, isFetching, error } = Digit.Hooks.useCustomAPIHook(requestCriteriaForEmployeeSearch);
if (error) {
// Handle error appropriately
}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 requestCriteriaForEmployeeSearch = { | |
| url: "/egov-hrms/employees/_searchListOfEmployee", | |
| params: {}, | |
| body: { | |
| criteria:{ | |
| tenantIds:uniqueTenants, | |
| roles:roles, | |
| type:"EMPLOYEE" | |
| } | |
| }, | |
| config: { | |
| enabled: !!uniqueTenants && !!roles, | |
| select: (data) => { | |
| return data?.Employees | |
| }, | |
| }, | |
| changeQueryName:{uniqueTenants,roles} | |
| }; | |
| const { isLoading, data, revalidate, isFetching, error } = Digit.Hooks.useCustomAPIHook(requestCriteriaForEmployeeSearch); | |
| const requestCriteriaForEmployeeSearch = { | |
| url: "/egov-hrms/employees/_searchListOfEmployee", | |
| params: {}, | |
| body: { | |
| criteria:{ | |
| tenantIds:uniqueTenants, | |
| roles:roles, | |
| type:"EMPLOYEE" | |
| } | |
| }, | |
| config: { | |
| enabled: !!uniqueTenants && !!roles, | |
| select: (data) => { | |
| return data?.Employees | |
| }, | |
| }, | |
| changeQueryName:{uniqueTenants,roles} | |
| }; | |
| const { isLoading, data, revalidate, isFetching, error } = Digit.Hooks.useCustomAPIHook(requestCriteriaForEmployeeSearch); | |
| if (error) { | |
| // Handle error appropriately | |
| } |
| const isChecked = arguments[0].target.checked | ||
| isChecked ? dispatch({type: "ADD_TO_SELECTED_EVENT_QUEUE", payload: arguments }) : dispatch({type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", payload: arguments }) |
There was a problem hiding this comment.
Use the rest parameters instead of arguments.
The arguments object does not have Array.prototype methods and can be inconvenient to use.
function onSelectToAddToQueue(...props) {
const isChecked = props[0].target.checked;
isChecked ? dispatch({type: "ADD_TO_SELECTED_EVENT_QUEUE", payload: props }) : dispatch({type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", payload: props });
}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 isChecked = arguments[0].target.checked | |
| isChecked ? dispatch({type: "ADD_TO_SELECTED_EVENT_QUEUE", payload: arguments }) : dispatch({type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", payload: arguments }) | |
| function onSelectToAddToQueue(...props) { | |
| const isChecked = props[0].target.checked; | |
| isChecked ? dispatch({type: "ADD_TO_SELECTED_EVENT_QUEUE", payload: props }) : dispatch({type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", payload: props }); | |
| } |
Tools
Biome
[error] 57-57: Use the rest parameters instead of arguments.
arguments does not have Array.prototype methods and can be inconvenient to use.
(lint/style/noArguments)
[error] 58-58: Use the rest parameters instead of arguments.
arguments does not have Array.prototype methods and can be inconvenient to use.
(lint/style/noArguments)
[error] 58-58: Use the rest parameters instead of arguments.
arguments does not have Array.prototype methods and can be inconvenient to use.
(lint/style/noArguments)
| function reducer(state, action){ | ||
| switch(action.type){ | ||
| case "ADD_TO_SELECTED_EVENT_QUEUE": | ||
| return [...state, {[optionsKey]: action.payload?.[1]?.[optionsKey], propsData: action.payload} ] | ||
| case "REMOVE_FROM_SELECTED_EVENT_QUEUE": | ||
| const newState = state.filter( e => e?.[optionsKey] !== action.payload?.[1]?.[optionsKey]) | ||
| onSelect(newState.map((e) => e.propsData), props); | ||
| return newState | ||
| case "REPLACE_COMPLETE_STATE": | ||
| return action.payload | ||
| default: | ||
| return state | ||
| } | ||
| } |
There was a problem hiding this comment.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration const newState is defined in a switch clause, which can lead to erroneous access by other switch clauses.
function reducer(state, action) {
switch (action.type) {
case "ADD_TO_SELECTED_EVENT_QUEUE":
return [...state, {[optionsKey]: action.payload?.[1]?.[optionsKey], propsData: action.payload} ];
case "REMOVE_FROM_SELECTED_EVENT_QUEUE": {
const newState = state.filter(e => e?.[optionsKey] !== action.payload?.[1]?.[optionsKey]);
onSelect(newState.map((e) => e.propsData), props);
return newState;
}
case "REPLACE_COMPLETE_STATE":
return action.payload;
default:
return state;
}
}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.
| function reducer(state, action){ | |
| switch(action.type){ | |
| case "ADD_TO_SELECTED_EVENT_QUEUE": | |
| return [...state, {[optionsKey]: action.payload?.[1]?.[optionsKey], propsData: action.payload} ] | |
| case "REMOVE_FROM_SELECTED_EVENT_QUEUE": | |
| const newState = state.filter( e => e?.[optionsKey] !== action.payload?.[1]?.[optionsKey]) | |
| onSelect(newState.map((e) => e.propsData), props); | |
| return newState | |
| case "REPLACE_COMPLETE_STATE": | |
| return action.payload | |
| default: | |
| return state | |
| } | |
| } | |
| function reducer(state, action){ | |
| switch(action.type){ | |
| case "ADD_TO_SELECTED_EVENT_QUEUE": | |
| return [...state, {[optionsKey]: action.payload?.[1]?.[optionsKey], propsData: action.payload} ]; | |
| case "REMOVE_FROM_SELECTED_EVENT_QUEUE": { | |
| const newState = state.filter( e => e?.[optionsKey] !== action.payload?.[1]?.[optionsKey]); | |
| onSelect(newState.map((e) => e.propsData), props); | |
| return newState; | |
| } | |
| case "REPLACE_COMPLETE_STATE": | |
| return action.payload; | |
| default: | |
| return state; | |
| } | |
| } |
Tools
Biome
[error] 17-17: 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)
| return data.map((item) => { | ||
| const filteredItem = {}; | ||
| keys.forEach((key) => { | ||
| if (item.hasOwnProperty(key)) { |
There was a problem hiding this comment.
Avoid using Object.prototype method directly.
It's recommended to use Object.hasOwn() instead of Object.hasOwnProperty().
if (Object.hasOwn(item, key)) {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 (item.hasOwnProperty(key)) { | |
| if (Object.hasOwn(item, key)) { |
Tools
Biome
[error] 10-10: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
| if (!currentLevel[value]) { | ||
| // Clone the item and delete the options property from it | ||
| const clonedItem = { ...item }; | ||
| delete clonedItem.options; |
There was a problem hiding this comment.
Avoid the delete operator which can impact performance.
Use an undefined assignment instead of the delete operator.
delete clonedItem.options;
clonedItem.options = 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.
| delete clonedItem.options; | |
| clonedItem.options = undefined; |
Tools
Biome
[error] 61-61: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
|
|
||
| const renderHierarchyFields = useMemo(() => { | ||
| return hierarchy.map(({ level, optionsKey, isMandatory, ...rest }, idx) => ( | ||
| <LabelFieldPair> |
There was a problem hiding this comment.
Add a key property to the element in the iterable.
The order of the items may change, and having a key can help React identify which item was moved.
<LabelFieldPair key={idx}>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.
| <LabelFieldPair> | |
| <LabelFieldPair key={idx}> |
Tools
Biome
[error] 328-328: 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)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- frontend/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- frontend/micro-ui/web/package.json (1 hunks)
- frontend/micro-ui/web/public/index.html (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- frontend/micro-ui/web/micro-ui-internals/example/public/index.html
- frontend/micro-ui/web/package.json
- frontend/micro-ui/web/public/index.html
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- frontend/micro-ui/web/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/micro-ui/web/package.json
indexersearchIssue: Added ssl certificate and auth while calling Indexer
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- municipal-services/property-services/src/main/java/org/egov/pt/config/PropertyConfiguration.java (1 hunks)
- municipal-services/property-services/src/main/java/org/egov/pt/repository/ElasticSearchRepository.java (4 hunks)
- municipal-services/property-services/src/main/resources/application.properties (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/config/WSConfiguration.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/ElasticSearchRepository.java (4 hunks)
- municipal-services/ws-services/src/main/resources/application.properties (2 hunks)
Files skipped from review due to trivial changes (1)
- municipal-services/property-services/src/main/java/org/egov/pt/config/PropertyConfiguration.java
Additional context used
Gitleaks
municipal-services/property-services/src/main/resources/application.properties
202-203: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
municipal-services/ws-services/src/main/resources/application.properties
208-208: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (15)
municipal-services/property-services/src/main/java/org/egov/pt/repository/ElasticSearchRepository.java (6)
18-22: Imports for SSL and Base64 encoding are necessary and correctly used.The imports for
javax.net.ssl.*,java.security.cert.*, andjava.util.Base64are necessary for the new methods added for SSL configuration and encoding credentials.
36-39: Constructor modification is appropriate.The
RestTemplateparameter has been removed from the constructor and is now instantiated within the class, which simplifies the constructor.
58-64: Ensure proper error handling for external calls.The
fuzzySearchPropertiesmethod adds an Authorization header using thegetESEncodedCredentialsmethod. Ensure that the credentials are securely handled and consider logging errors without exposing sensitive information.
89-94: Ensure secure handling of credentials.The
getESEncodedCredentialsmethod correctly encodes the credentials using Base64. Ensure that the credentials are securely stored and managed.
95-121: Ensure secure SSL configuration.The
trustSelfSignedSSLmethod configures SSL to trust self-signed certificates, which is necessary for development environments. Ensure this is not used in production to avoid security risks.
123-127: Ensure correct configuration of RestTemplate.The
restTemplatemethod returns aRestTemplateinstance with SSL settings configured. Ensure that the SSL configuration is appropriate for the environment.municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/ElasticSearchRepository.java (6)
3-6: Imports for SSL and Base64 encoding are necessary and correctly used.The imports for
javax.net.ssl.*,java.security.cert.*, andjava.util.Base64are necessary for the new methods added for SSL configuration and encoding credentials.
45-48: Constructor modification is appropriate.The
RestTemplateparameter has been removed from the constructor and is now instantiated within the class, which simplifies the constructor.
67-73: Ensure proper error handling for external calls.The
fuzzySearchPropertiesmethod adds an Authorization header using thegetESEncodedCredentialsmethod. Ensure that the credentials are securely handled and consider logging errors without exposing sensitive information.
98-103: Ensure secure handling of credentials.The
getESEncodedCredentialsmethod correctly encodes the credentials using Base64. Ensure that the credentials are securely stored and managed.
104-130: Ensure secure SSL configuration.The
trustSelfSignedSSLmethod configures SSL to trust self-signed certificates, which is necessary for development environments. Ensure this is not used in production to avoid security risks.
132-136: Ensure correct configuration of RestTemplate.The
restTemplatemethod returns aRestTemplateinstance with SSL settings configured. Ensure that the SSL configuration is appropriate for the environment.municipal-services/ws-services/src/main/resources/application.properties (3)
157-157: LGTM!The addition of the new property
egov.collection.hostfollows the existing pattern and is formatted correctly.
206-206: LGTM!The addition of the comment
#ES-configis clear and correctly formatted.
207-208: Security Risk: Avoid storing sensitive information in plain text.Storing Elasticsearch credentials (username and password) in plain text within the properties file is a security risk. It's recommended to use environment variables or a secrets management service to store such sensitive information.
- egov.indexer.es.username=elastic - egov.indexer.es.password=8fwbD6HbJh6HU0oddsHm8TEI + egov.indexer.es.username=${ES_USERNAME} + egov.indexer.es.password=${ES_PASSWORD}Ensure that the environment variables
ES_USERNAMEandES_PASSWORDare set correctly in the deployment environment.Tools
Gitleaks
208-208: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| egov.indexer.es.username=elastic | ||
| egov.indexer.es.password=8fwbD6HbJh6HU0oddsHm8TEI | ||
|
|
There was a problem hiding this comment.
Detected hardcoded credentials.
The Elasticsearch username and password are hardcoded in the application.properties file. This is a security risk.
Consider using environment variables or a secrets management service to handle these credentials securely.
- egov.indexer.es.username=elastic
- egov.indexer.es.password=8fwbD6HbJh6HU0oddsHm8TEI
+ egov.indexer.es.username=${ES_USERNAME}
+ egov.indexer.es.password=${ES_PASSWORD}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.
| egov.indexer.es.username=elastic | |
| egov.indexer.es.password=8fwbD6HbJh6HU0oddsHm8TEI | |
| egov.indexer.es.username=${ES_USERNAME} | |
| egov.indexer.es.password=${ES_PASSWORD} |
Tools
Gitleaks
202-203: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| @Value("${egov.indexer.es.username}") | ||
| private String esUsername; | ||
|
|
||
| @Value("${egov.indexer.es.password}") | ||
| private String esPassword; | ||
|
|
There was a problem hiding this comment.
Ensure secure handling of credentials.
The Elasticsearch username and password are hardcoded in the configuration file. This is a security risk.
Consider using environment variables or a secrets management service to handle these credentials securely.
- @Value("${egov.indexer.es.username}")
- private String esUsername;
- @Value("${egov.indexer.es.password}")
- private String esPassword;
+ @Value("${ES_USERNAME}")
+ private String esUsername;
+ @Value("${ES_PASSWORD}")
+ private String esPassword;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.
| @Value("${egov.indexer.es.username}") | |
| private String esUsername; | |
| @Value("${egov.indexer.es.password}") | |
| private String esPassword; | |
| @Value("${ES_USERNAME}") | |
| private String esUsername; | |
| @Value("${ES_PASSWORD}") | |
| private String esPassword; |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (5)
municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/rowmapper/DemandNotGeneratedRowMapper.java (1)
1-1: Add a class-level Javadoc comment.A class-level Javadoc comment would improve the readability and maintainability of the code.
/** * RowMapper implementation to map the ResultSet data to ConsumersDemandNotGenerated objects. */municipal-services/ws-services/src/main/java/org/egov/waterconnection/service/WaterService.java (1)
56-56: Add a Javadoc comment for the new method.A Javadoc comment would improve the readability and maintainability of the code.
/** * Fetches consumers with demand not generated based on previous meter readings. * * @param previousMeterReading The previous meter reading. * @param tenantId The tenant ID. * @param requestInfo The request information. * @return A response containing the consumers with demand not generated. */municipal-services/ws-services/src/main/java/org/egov/waterconnection/web/controller/WaterController.java (1)
238-245: Add a Javadoc comment for the new endpoint.A Javadoc comment would improve the readability and maintainability of the code.
/** * Endpoint to fetch consumers with demand not generated based on previous meter readings. * * @param requestInfoWrapper The request information wrapper. * @param previousMeterReading The previous meter reading. * @param tenantId The tenant ID. * @return A response containing the consumers with demand not generated. */municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/WaterDaoImpl.java (2)
52-53: Add a Javadoc comment for the new dependency.A Javadoc comment would improve the readability and maintainability of the code.
/** * RowMapper to map the result set data to ConsumersDemandNotGenerated objects. */
672-684: Add a Javadoc comment for the new method.A Javadoc comment would improve the readability and maintainability of the code.
/** * Fetches consumers by previous meter reading. * * @param previousMeterReading The previous meter reading. * @param tenantId The tenant ID. * @return A list of consumers with demand not generated. */
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/WaterDaoImpl.java (2 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/builder/WsQueryBuilder.java (4 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/rowmapper/DemandNotGeneratedRowMapper.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/service/WaterService.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/service/WaterServiceImpl.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/web/controller/WaterController.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/web/models/ConsumersDemandNotGenerated.java (1 hunks)
Files skipped from review due to trivial changes (1)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/web/models/ConsumersDemandNotGenerated.java
Files skipped from review as they are similar to previous changes (1)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/builder/WsQueryBuilder.java
Additional comments not posted (2)
municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/rowmapper/DemandNotGeneratedRowMapper.java (1)
20-22: Ensure all necessary fields are mapped.Currently, only the
consumerCodefield is being set. Ensure that all necessary fields ofConsumersDemandNotGeneratedare being mapped from theResultSet.municipal-services/ws-services/src/main/java/org/egov/waterconnection/web/controller/WaterController.java (1)
239-239: Use@RequestParamfor all parameters.The
previousMeterReadingandtenantIdparameters should use@RequestParam.- public ResponseEntity<WaterConnectionResponse> getConsumersWithDemandNotGenerated(@Valid @RequestBody RequestInfoWrapper requestInfoWrapper,@RequestParam(value="previousMeterReading") String previousMeterReading,@RequestParam (value="tenantId") String tenantId) + public ResponseEntity<WaterConnectionResponse> getConsumersWithDemandNotGenerated(@Valid @RequestBody RequestInfoWrapper requestInfoWrapper, @RequestParam(value="previousMeterReading") String previousMeterReading, @RequestParam(value="tenantId") String tenantId)Likely invalid or redundant comment.
| @Override | ||
| public WaterConnectionResponse getConsumersWithDemandNotGenerated(String previousMeterReading, String tenantId ,RequestInfo requestInfo) | ||
| { | ||
| Long previousReadingEpoch; | ||
| try { | ||
| previousReadingEpoch = Long.parseLong(previousMeterReading); | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException("Invalid format for previousMeterReading. Expected a timestamp in milliseconds.", e); | ||
| } | ||
|
|
||
| List<ConsumersDemandNotGenerated> list=waterDaoImpl.getConsumersByPreviousMeterReading(previousReadingEpoch,tenantId); | ||
| Set<String> connectionNo=new HashSet<>(); | ||
| for(ConsumersDemandNotGenerated connection:list) | ||
| { | ||
| connectionNo.add(connection.getConsumerCode()); | ||
| } | ||
| SearchCriteria criteria=SearchCriteria.builder().connectionNoSet(connectionNo).tenantId(tenantId).build(); | ||
| return search(criteria,requestInfo); | ||
| } |
There was a problem hiding this comment.
Improve exception handling and add null checks.
- The exception handling for
NumberFormatExceptionis good, but consider logging the error for better traceability. - Add null checks for
listandconnectionNoto prevent potentialNullPointerException. - Ensure the
searchmethod can handle emptyconnectionNogracefully.
@Override
public WaterConnectionResponse getConsumersWithDemandNotGenerated(String previousMeterReading, String tenantId, RequestInfo requestInfo) {
Long previousReadingEpoch;
try {
previousReadingEpoch = Long.parseLong(previousMeterReading);
} catch (NumberFormatException e) {
// Log the error for better traceability
log.error("Invalid format for previousMeterReading: {}", previousMeterReading, e);
throw new IllegalArgumentException("Invalid format for previousMeterReading. Expected a timestamp in milliseconds.", e);
}
List<ConsumersDemandNotGenerated> list = waterDaoImpl.getConsumersByPreviousMeterReading(previousReadingEpoch, tenantId);
if (list == null || list.isEmpty()) {
// Handle empty list scenario
return new WaterConnectionResponse();
}
Set<String> connectionNo = new HashSet<>();
for (ConsumersDemandNotGenerated connection : list) {
connectionNo.add(connection.getConsumerCode());
}
if (connectionNo.isEmpty()) {
// Handle empty connectionNo scenario
return new WaterConnectionResponse();
}
SearchCriteria criteria = SearchCriteria.builder().connectionNoSet(connectionNo).tenantId(tenantId).build();
return search(criteria, requestInfo);
}| StringBuilder query=new StringBuilder(wsQueryBuilder.DEMAND_NOT_GENERATED_QUERY); | ||
|
|
||
| List<Object> preparedStatement=new ArrayList<>(); | ||
| preparedStatement.add(tenantId); | ||
| preparedStatement.add(previousMeterReading); | ||
| preparedStatement.add(tenantId); | ||
|
|
||
| log.info("Query for consumer demand not generated "+ query +" prepared statement "+ preparedStatement); | ||
| List<ConsumersDemandNotGenerated> consumersDemandNotGeneratedList=jdbcTemplate.query(query.toString(),preparedStatement.toArray(),demandNotGeneratedRowMapper); |
There was a problem hiding this comment.
Use parameterized logging.
Use parameterized logging instead of string concatenation for better performance and readability.
- log.info("Query for consumer demand not generated "+ query +" prepared statement "+ preparedStatement);
+ log.info("Query for consumer demand not generated {} prepared statement {}", query, preparedStatement);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.
| StringBuilder query=new StringBuilder(wsQueryBuilder.DEMAND_NOT_GENERATED_QUERY); | |
| List<Object> preparedStatement=new ArrayList<>(); | |
| preparedStatement.add(tenantId); | |
| preparedStatement.add(previousMeterReading); | |
| preparedStatement.add(tenantId); | |
| log.info("Query for consumer demand not generated "+ query +" prepared statement "+ preparedStatement); | |
| List<ConsumersDemandNotGenerated> consumersDemandNotGeneratedList=jdbcTemplate.query(query.toString(),preparedStatement.toArray(),demandNotGeneratedRowMapper); | |
| StringBuilder query=new StringBuilder(wsQueryBuilder.DEMAND_NOT_GENERATED_QUERY); | |
| List<Object> preparedStatement=new ArrayList<>(); | |
| preparedStatement.add(tenantId); | |
| preparedStatement.add(previousMeterReading); | |
| preparedStatement.add(tenantId); | |
| log.info("Query for consumer demand not generated {} prepared statement {}", query, preparedStatement); | |
| List<ConsumersDemandNotGenerated> consumersDemandNotGeneratedList=jdbcTemplate.query(query.toString(),preparedStatement.toArray(),demandNotGeneratedRowMapper); |
addded user call in plainsearch
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- business-services/egov-hrms/src/main/java/org/egov/hrms/service/EmployeeService.java (3 hunks)
- business-services/egov-hrms/src/main/java/org/egov/hrms/web/controller/EmployeeController.java (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- business-services/egov-hrms/src/main/java/org/egov/hrms/service/EmployeeService.java
- business-services/egov-hrms/src/main/java/org/egov/hrms/web/controller/EmployeeController.java
Summary by CodeRabbit
New Features
Updates
Bug Fixes