Develop #943
Develop #943
Conversation
[ISTE-262] - Saving meter reading after taxperiod validation
ISTE-374 Fix
Iste 228 updated
changed calculation logic for metered connection
Corrected Kibana Dashboard via Iframe and Added extra Columns in "Seach Department User" (ISTE-418)
WalkthroughThe changes encompass modifications across various files in the project, including updates to widget parameters for improved layout, adjustments to scrolling behavior, and enhancements in CSS for responsiveness. Additionally, new components and functionalities have been introduced, such as a dashboard and a utilities module, along with updates to existing components for better organization and user experience. The overall structure and logic have been refined to enhance maintainability and clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard
participant IFrameInterface
participant UtilitiesModule
User->>Dashboard: Navigate to Dashboard
Dashboard->>IFrameInterface: Load content based on parameters
IFrameInterface->>UtilitiesModule: Fetch utility data
UtilitiesModule-->>IFrameInterface: Return utility data
IFrameInterface-->>Dashboard: Render content
Dashboard-->>User: Display dashboard content
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (6)
Files skipped from review due to trivial changes (3)
Files skipped from review as they are similar to previous changes (1)
Additional comments not posted (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (10)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/Dashboard.js (3)
12-15: Consider using constants for class names.The component is using hardcoded class names for the wrapper and iframe elements. Consider defining them as constants at the top of the file for better maintainability and readability.
+const WRAPPER_CLASS_NAME = "custom-iframe-wrapper"; +const IFRAME_CLASS_NAME = "custom-iframe"; + const Dashboard = () => { // ... return ( <div className="dashboard"> <IFrameInterface - wrapperClassName="custom-iframe-wrapper" + wrapperClassName={WRAPPER_CLASS_NAME} - className="custom-iframe" + className={IFRAME_CLASS_NAME} // ... /> </div> ); };
4-23: Consider handling loading states and errors.The component is not handling any loading states or errors that might occur while retrieving the values from query parameters or global configs. Consider adding loading indicators and error handling to improve the user experience and maintainability of the component.
19-19: Remove commented out code.The
filtersprop is commented out. If it's not needed, consider removing it to keep the code clean and maintainable. If it's needed for future use, consider adding a TODO comment to explain its purpose.frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/WorkflowComponentTest/index.js (1)
8-26: Component looks good, but consider adding some enhancements.The
WorkflowCompTestcomponent is correctly defined and exported. It retrieves the required values from query parameters usingDigit.Hooks.useQueryParams()and passes them as props to theWorkflowTimelineandWorkflowActionscomponents.However, consider adding the following enhancements to improve the component:
- Handle loading states and errors when retrieving the query parameters.
- Handle edge cases where the required query parameters are missing.
- Use PropTypes to validate the props passed to the component.
- Use default props to provide default values for the props.
- Use styling or CSS modules to style the component.
- Add unit tests to test the component.
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/IFrameInterface/index.js (3)
21-27: LGTM, but consider a minor simplification.The MDMS data fetching and usage looks good.
Suggestion: The
selectfunction could be simplified by directly returning the first element of the array instead of assigning it to a variable:select: (data) => { - let formattedResponse = data?.["common-masters"]?.["uiCommonConstants"]?.[0] || {}; - return formattedResponse; + return data?.["common-masters"]?.["uiCommonConstants"]?.[0] || {}; },
29-142: LGTM, but consider improving error handling.The custom HTTP interceptors look good and correctly add the authorization headers to the requests.
Suggestion: Improve the error handling by propagating the errors to the parent component or showing an error message to the user. This will help in debugging and providing a better user experience.
247-272: LGTM, but consider improving the toast message.The rendering logic looks good and properly handles the loading and error states.
Suggestion: Make the toast message more informative and show it only in development mode. For example:
- {iframeLoaded && <Toast type={iframeLoaded ? "info" : ""} label={"Iframe Loaded"}></Toast>} + {process.env.NODE_ENV === "development" && iframeLoaded && <Toast type="info" label={`Iframe loaded: ${url}`}></Toast>}frontend/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/inbox.scss (2)
57-60: LGTM, but consider avoiding!importantif possible.The code changes are approved.
However, the use of
!importantis generally discouraged as it can make the CSS harder to maintain. Consider removing it if the styles can be applied without it.
62-65: LGTM, but consider avoiding!importantif possible.The code changes are approved.
However, the use of
!importantis generally discouraged as it can make the CSS harder to maintain. Consider removing it if the styles can be applied without it.municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/EstimationService.java (1)
230-232: Remove commented out code.The commented out code seems to be an old implementation that is no longer needed. The new implementation at line 216-218 sets the
waterChargeto the minimum charge upfront, so this check is redundant.Consider removing the commented out code to improve readability and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (24)
- frontend/mgramseva/lib/screeens/household_register/household_list.dart (1 hunks)
- frontend/mgramseva/lib/screeens/household_register/household_register.dart (2 hunks)
- frontend/mgramseva/lib/screeens/household_register/household_search.dart (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/iframe.scss (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/inbox.scss (2 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/custom-css/example/index.css (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/EmployeeModuleCard.js (3 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/components/inbox/DesktopInbox.js (2 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/Dashboard.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/SearchUser.js (2 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/index.js (3 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/configs/UICustomizations.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/README.md (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/package.json (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/Module.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/DynamicCreateComponent/index.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/DynamicSearchComponent/index.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/IFrameInterface/index.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/WorkflowComponentTest/index.js (1 hunks)
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/index.js (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/EstimationService.java (1 hunks)
- utilities/rollout-dashboard-cronjob/app.py (1 hunks)
Files skipped from review due to trivial changes (2)
- frontend/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js
- frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/package.json
Additional context used
LanguageTool
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/README.md
[grammar] ~70-~70: The singular determiner ‘this’ may not agree with the plural noun ‘versions’. Did you mean “these”?
Context: ...L193)_ ## List of Screens available in this versions were as follows 1. Search or ...(THIS_NNS)
[grammar] ~148-~148: This phrase is duplicated. You should probably use “DIGIT Frontend” only once.
Context: ...om/jagankumar-egov) ### Published from DIGIT Frontend DIGIT Frontend Repo (https://github.com/egovernments/D...(PHRASE_REPETITION)
Markdownlint
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/README.md
149-149: null
Bare URL used(MD034, no-bare-urls)
Biome
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/EmployeeModuleCard.js
[error] 76-76: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 72-72: 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)
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/IFrameInterface/index.js
[error] 181-181: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Ruff
utilities/rollout-dashboard-cronjob/app.py
821-821: SyntaxError: Expected a statement
821-821: SyntaxError: Expected a statement
821-822: SyntaxError: Expected a statement
822-822: SyntaxError: Unexpected indentation
Additional comments not posted (40)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/Dashboard.js (1)
4-23: LGTM!The component looks good overall. It correctly retrieves values from query parameters and global configs and passes them as props to
IFrameInterface.frontend/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/iframe.scss (2)
51-54: LGTM!The changes to the
leftandtopproperties of the.app-iframe-wrapperclass are approved. Using relative units (em) instead of fixed pixel values enhances the responsiveness of the layout by allowing the positioning to scale with the font size.
58-59: LGTM!The changes to the
widthandheightproperties of the.app-iframeclass are approved. Using relative units (em) andcalcinstead of fixed pixel values and percentages improves the layout and adaptability of the iframe within its container.frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/Module.js (4)
1-5: LGTM!The code changes are approved.
6-21: LGTM!The code changes are approved.
23-25: LGTM!The code changes are approved.
27-31: LGTM!The code changes are approved.
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/SearchUser.js (2)
12-12: LGTM!The code changes are approved.
41-41: LGTM!The code changes are approved.
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/index.js (1)
26-44: LGTM!The
Appcomponent looks good overall. Here are some observations:
- The component structure is clear and follows a good separation of concerns.
- The use of
react-routercomponents (Switch,Route) for setting up the routing is appropriate.- The
pathprop is used consistently across the routes.- The
commonPropsobject is a nice way to pass common props to the sub-components.- The
AppContainercomponent is used to wrap the routes, which is a good practice.- The
ProjectBreadCrumbcomponent is rendered inside theAppContainer, which is fine.- The sub-components are rendered based on the defined routes, which is the expected behavior.
The code changes are approved.
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/DynamicSearchComponent/index.js (1)
1-62: LGTM!The code changes in the
DynamicSearchComponentare approved. The component is well-structured, follows the recommended practices, and uses the appropriate hooks and utility functions to keep the code clean and maintainable.frontend/mgramseva/lib/screeens/household_register/household_list.dart (1)
76-77: LGTM!The changes made to the
heightandscrollPhysicsparameters of theBillsTablewidget are approved for the following reasons:
- Height Calculation:
- Capping the length of
tableDataat 10 ensures that the height does not exceed a certain threshold, improving the layout and usability of the interface when there are many entries.
- Scroll Physics:
- Changing the
scrollPhysicsfromNeverScrollableScrollPhysics()toBouncingScrollPhysics()allows for a more dynamic and responsive user experience.These changes enhance the functionality and user interaction of the household list display.
frontend/mgramseva/lib/screeens/household_register/household_search.dart (2)
71-71: LGTM!Setting
primarytofalseis appropriate here as theSingleChildScrollViewis used to scroll the tab buttons horizontally and it is not the primary scroll view in the widget tree.
76-76: LGTM!Adding a blank line before the
tabList.lengthexpression improves the code readability.frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/index.js (5)
6-6: LGTM!The import statement for the
Dashboardcomponent is correctly added.
28-35: LGTM!The
useEffecthook has been reformatted for improved readability without altering the functionality.
42-45: LGTM!The
Linkcomponent'stoprop has been reformatted for consistency without altering the functionality.
47-53: LGTM!The code changes add a new back button functionality to navigate to the previous page.
65-65: LGTM!The
PrivateRoutefor theDashboardcomponent is correctly added and allows navigation to the new route.frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/hrmscard.js (6)
20-22: LGTM!The changes to the
rolesobject improve code readability.
28-33: LGTM!The addition of the
categoryproperty to themoduleForSomeDIVAdminarray enhances the categorization of the displayed items.
39-44: LGTM!The addition of the
categoryproperty to themoduleForSomeSTATEUserarray enhances the categorization of the displayed items.
48-56: LGTM!The addition of the
categoryproperty to themoduleForDivisionUserarray enhances the categorization of the displayed items.
75-83: LGTM!The changes to the
linksarray in thepropsForModuleCardobject improve the user experience by:
- Providing more accurate labels based on the user's access level.
- Enhancing the categorization of the displayed items.
88-101: LGTM!The addition of the
categoryproperty to the objects in thelinksarray enhances the categorization of the displayed items.frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/README.md (2)
17-50: LGTM!The usage instructions are clear and the code examples are correct.
54-63: LGTM!The code example for using the InboxSearchComposer component is correct and provides a useful demonstration.
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/EmployeeModuleCard.js (4)
49-49: LGTM!The change in the
links-wrapperdiv's width from "80%" to "100%" is approved.
52-65: LGTM!The changes to group links by category using the
reducemethod are approved. The logic is implemented correctly and improves the organization of links.
66-103: LGTM!The changes to filter and render categories and their respective items are approved. The code correctly handles the conditional rendering of links based on their type and the special case for "Dashboard" links, enhancing the user experience.
Also, good job on addressing the static analysis hint by adding the
keyproperty to thedivelement.Tools
Biome
[error] 76-76: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 72-72: 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)
76-76: False positive static analysis hint - the code is correct.The code already uses an optional chaining operator (
?.) to safely access theincludesmethod onitem.link. The static analysis hint suggesting to change it to an optional chain is a false positive, as the code is already following the recommended approach.No changes are needed here.
Tools
Biome
[error] 76-76: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/inbox/DesktopInbox.js (2)
94-101: LGTM!The code segment correctly adds a new column configuration for "HR_USER_TYPE" to display the user type information. The
GetCellfunction is correctly used to render thedepartmentfield from the first assignment.
102-109: LGTM!The code segment correctly adds a new column configuration for "HR_USER_DESIGNATION" to display the user designation information. The
GetCellfunction is correctly used to render thedesignationfield from the first assignment.frontend/micro-ui/web/micro-ui-internals/packages/modules/payment/src/configs/UICustomizations.js (1)
180-184: LGTM!The sorting functionality is correctly implemented and enhances the presentation of the data in the UI. The sorting is performed in a case-insensitive manner, which is a good practice. The sorting is performed on the
divisionNameproperty, which is appropriate for the given context. The sorting is performed before theresultarray is returned, which is the correct control flow.The code changes are approved.
frontend/mgramseva/lib/screeens/household_register/household_register.dart (1)
77-79: LGTM, but verify the impact on user experience.The code changes are approved.
However, setting
primarytofalseon theCustomScrollViewmay affect how it interacts with other scrollable widgets in the hierarchy. This could potentially influence the user experience by changing the scroll behavior or focus.Ensure that this change is thoroughly tested to confirm that:
- The scroll behavior within the
CustomScrollViewitself remains as expected.- The interaction with other scrollable widgets in the screen is not adversely affected.
- The overall user experience of scrolling and navigation on this screen is not negatively impacted.
frontend/micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/IFrameInterface/index.js (1)
9-19: LGTM!The component props and state variables look good and appropriate for the functionality.
municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/EstimationService.java (3)
208-209: LGTM!The code changes are approved.
216-218: LGTM!The code changes are approved.
220-226: LGTM!The code changes are approved.
frontend/micro-ui/web/micro-ui-internals/packages/custom-css/example/index.css (1)
1887-1888: LGTM! The change may affect the UI layout.The width of elements with the
.employee .card-homeclass has been increased from270pxto300px. This will likely result in a more spacious appearance for the card elements, potentially affecting the overall design and spacing of the user interface where these elements are rendered.
| body: config?.body.filter((a) => !a.hideInEmployee), | ||
| }; | ||
| })} | ||
| onSubmit={() => {}} |
There was a problem hiding this comment.
Handle the form submission.
The onSubmit handler is an empty function, which means the form submission is not handled.
Implement the onSubmit handler to handle the form submission. For example:
onSubmit={(data) => {
// Call the API to create the project
// Show success/error message
// Navigate to the project details page
}}| const stateTenant = Digit.ULBService.getStateId(); | ||
| const { moduleName, masterName } = useParams(); |
There was a problem hiding this comment.
Use the URL params for moduleName and masterName.
The component is using hardcoded values for moduleName and masterName instead of getting them from the URL params.
Use the useParams hook from react-router-dom to get the moduleName and masterName from the URL params. For example:
import { useParams } from "react-router-dom";
const { moduleName, masterName } = useParams();| const { isLoading, data: configs } = Digit.Hooks.useCustomMDMS( | ||
| //change to data | ||
| stateTenant, | ||
| moduleName, | ||
| [ | ||
| { | ||
| name: masterName, | ||
| }, | ||
| ], | ||
| { | ||
| select: (data) => { | ||
| return data?.[moduleName]?.[masterName]?.[0]; | ||
| }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Handle loading and error states of the useCustomMDMS hook.
The component is not handling loading and error states of the useCustomMDMS hook.
Handle the loading and error states of the useCustomMDMS hook. For example:
const { isLoading, isError, data: configs } = Digit.Hooks.useCustomMDMS(
stateTenant,
moduleName,
[
{
name: masterName,
},
],
{
select: (data) => {
return data?.[moduleName]?.[masterName]?.[0];
},
}
);
if (isLoading) return <Loader />;
if (isError) return <div>Something went wrong</div>;| @@ -0,0 +1,152 @@ | |||
| <!-- TODO: update this --> | |||
There was a problem hiding this comment.
Reminder: Update the README.
The TODO comment indicates that the README needs to be updated.
Do you want me to help update the README or open a GitHub issue to track this task?
| In MDMS | ||
|
|
||
| _Add this configuration to enable this module [MDMS Enabling Utilities Module](https://github.com/egovernments/works-mdms-data/blob/48461ecaf944ea243e24e1c1f9a5e2179d8091ac/data/pg/tenant/citymodule.json#L193)_ | ||
|
|
||
| ## List of Screens available in this versions were as follows |
There was a problem hiding this comment.
Fix the grammar in the list introduction phrase.
The MDMS configuration instructions are clear and the link to the MDMS file is helpful.
However, please fix the grammar in this phrase:
List of Screens available in this versions were as follows
"this versions" should be corrected to "these versions".
-List of Screens available in this versions were as follows
+List of Screens available in these versions were as followsCommittable 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.
| In MDMS | |
| _Add this configuration to enable this module [MDMS Enabling Utilities Module](https://github.com/egovernments/works-mdms-data/blob/48461ecaf944ea243e24e1c1f9a5e2179d8091ac/data/pg/tenant/citymodule.json#L193)_ | |
| ## List of Screens available in this versions were as follows | |
| In MDMS | |
| _Add this configuration to enable this module [MDMS Enabling Utilities Module](https://github.com/egovernments/works-mdms-data/blob/48461ecaf944ea243e24e1c1f9a5e2179d8091ac/data/pg/tenant/citymodule.json#L193)_ | |
| ## List of Screens available in these versions were as follows |
Tools
LanguageTool
[grammar] ~70-~70: The singular determiner ‘this’ may not agree with the plural noun ‘versions’. Did you mean “these”?
Context: ...L193)_ ## List of Screens available in this versions were as follows 1. Search or ...(THIS_NNS)
| ### Published from DIGIT Frontend | ||
| DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/master) |
There was a problem hiding this comment.
Fix the phrase repetition and bare URL.
Please address the following issues in the "Published from DIGIT Frontend" section:
- Fix the phrase repetition in "Published from DIGIT Frontend DIGIT Frontend Repo". You can remove the second "DIGIT Frontend".
-### Published from DIGIT Frontend DIGIT Frontend Repo
+### Published from DIGIT Frontend Repo- Fix the bare URL by wrapping it in angle brackets or using Markdown link syntax.
-DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/master)
+DIGIT Frontend Repo (<https://github.com/egovernments/Digit-Frontend/tree/master>)or
-DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/master)
+[DIGIT Frontend Repo](https://github.com/egovernments/Digit-Frontend/tree/master)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.
| ### Published from DIGIT Frontend | |
| DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/master) | |
| ### Published from DIGIT Frontend Repo | |
| [DIGIT Frontend Repo](https://github.com/egovernments/Digit-Frontend/tree/master) |
Tools
LanguageTool
[grammar] ~148-~148: This phrase is duplicated. You should probably use “DIGIT Frontend” only once.
Context: ...om/jagankumar-egov) ### Published from DIGIT Frontend DIGIT Frontend Repo (https://github.com/egovernments/D...(PHRASE_REPETITION)
Markdownlint
149-149: null
Bare URL used(MD034, no-bare-urls)
| useEffect(() => { | ||
| const pageObject = data?.[moduleName]?.["iframe-routes"]?.[pageName] || {}; | ||
| if (pageObject?.Authorization) { | ||
| if (pageObject?.SendInvalidAuthorization) { | ||
| setSendAuth("invalid"); | ||
| } else { | ||
| setSendAuth(true); | ||
| } | ||
| } else { | ||
| setSendAuth(false); | ||
| } | ||
|
|
||
| const isOrign = pageObject?.["isOrigin"] || false; | ||
| const domain = isOrign | ||
| ? process.env.NODE_ENV === "development" | ||
| ? "https://mgramseva-dwss.punjab.gov.in" | ||
| : document.location.origin | ||
| : pageObject?.["domain"]; | ||
| //checking if overwrite time is true then update the url as per filter time else return the url | ||
|
|
||
| const contextPath = pageObject?.["routePath"] | ||
| ? pageObject?.["overwriteTimeFilter"] && filters?.range?.startDate && filters?.range?.endDate | ||
| ? pageObject["routePath"] | ||
| .replace("from:now-15m", `from:'${new Date(filters?.range?.startDate).toISOString()}'`) | ||
| .replace("to:now", `to:'${new Date(filters?.range?.endDate).toISOString()}'`) | ||
| : pageObject["routePath"] | ||
| : ""; | ||
| const title = pageObject?.["title"] || ""; | ||
| let url = `${domain}${contextPath}`; | ||
|
|
||
| if (pageObject?.authToken && pageObject?.authToken?.enable) { | ||
| const authKey = pageObject?.authToken?.key || "auth-token"; | ||
| if (pageObject?.authToken?.customFun && Digit.Utils.createFunction(pageObject?.authToken?.customFun)) { | ||
| const customFun = Digit.Utils.createFunction(pageObject?.authToken?.customFun); | ||
| url = customFun(url, Digit.UserService.getUser()?.access_token, pageObject?.authToken); | ||
| } else { | ||
| url = `${url}&${authKey}=${Digit.UserService.getUser()?.access_token || ""}`; | ||
| } | ||
| } | ||
| setUrl(url); | ||
|
|
||
| setTitle(title); | ||
| }, [data, moduleName, pageName, location, filters]); |
There was a problem hiding this comment.
Suggest simplifying the URL construction logic.
The iframe URL construction logic seems quite complex and could be prone to errors. Here are a few suggestions to improve it:
- Use the optional chaining operator to simplify nested property accesses. For example:
- const pageObject = data?.[moduleName]?.["iframe-routes"]?.[pageName] || {};
+ const pageObject = data?.[moduleName]?.["iframe-routes"]?.[pageName] ?? {};- Extract the time filter overwrite logic into a separate function for better readability. For example:
const overwriteTimeFilter = (routePath, filters) => {
if (filters?.range?.startDate && filters?.range?.endDate) {
return routePath
.replace("from:now-15m", `from:'${new Date(filters.range.startDate).toISOString()}'`)
.replace("to:now", `to:'${new Date(filters.range.endDate).toISOString()}'`);
}
return routePath;
};Then use it like:
const contextPath = pageObject?.["routePath"]
- ? pageObject?.["overwriteTimeFilter"] && filters?.range?.startDate && filters?.range?.endDate
- ? pageObject["routePath"]
- .replace("from:now-15m", `from:'${new Date(filters?.range?.startDate).toISOString()}'`)
- .replace("to:now", `to:'${new Date(filters?.range?.endDate).toISOString()}'`)
- : pageObject["routePath"]
+ ? overwriteTimeFilter(pageObject["routePath"], filters)
: "";- Use the optional chaining operator and nullish coalescing operator consistently throughout the URL construction logic to handle potential
undefinedvalues.
Tools
Biome
[error] 181-181: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| try { | ||
| const basePath = data?.[moduleName]?.["iframe-routes"]?.[pageName]?.["base-kibana-path"] || "/kibana/"; | ||
| const response = await axios.post( | ||
| `${window.location.origin}${basePath}internal/security/login`, | ||
| { | ||
| providerType: "anonymous", | ||
| providerName: "anonymous1", | ||
| currentURL: `${window.location.origin}${basePath}login`, | ||
| }, | ||
| { | ||
| headers: { | ||
| Accept: "*/*", | ||
| "Accept-Language": "en-US,en;q=0.9", | ||
| "Cache-Control": "no-cache", | ||
| Connection: "keep-alive", | ||
| "Content-Type": "application/json", | ||
| DNT: "1", | ||
| Origin: window.location.origin, | ||
| Pragma: "no-cache", | ||
| Referer: `${window.location.origin}${basePath}login?next=%2Fkibana%2F`, | ||
| "Sec-Fetch-Dest": "empty", | ||
| "Sec-Fetch-Mode": "cors", | ||
| "Sec-Fetch-Site": "same-origin", | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36", | ||
| "kbn-build-number": "68312", | ||
| "kbn-version": "8.11.3", | ||
| "sec-ch-ua": '"Not/A)Brand";v="8", "Chromium";v="126", "Google Chrome";v="126"', | ||
| "sec-ch-ua-mobile": "?0", | ||
| "sec-ch-ua-platform": '"Windows"', | ||
| "x-elastic-internal-origin": "Kibana", | ||
| "x-kbn-context": `{"type":"application","name":"security_login","url":"${basePath}login"}`, | ||
| }, | ||
| } | ||
| ); | ||
| const setCookie = response.headers["set-cookie"]; | ||
| if (setCookie) { | ||
| console.log("axios cookie set", setCookie); | ||
| document.cookie = setCookie; | ||
| } | ||
| } catch (error) { | ||
| console.log("axios resp err", error.message); | ||
| console.error("Error fetching data:", error); | ||
| } finally { | ||
| setIsAxiosLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchData(); | ||
| }, []); |
There was a problem hiding this comment.
Suggest improving the Kibana login request.
The Kibana login request looks mostly fine, but there are a few potential issues to address:
- Use
URLconstructor to build the base path instead of string concatenation to avoid potential URL errors. For example:
- const basePath = data?.[moduleName]?.["iframe-routes"]?.[pageName]?.["base-kibana-path"] || "/kibana/";
+ const basePath = new URL(data?.[moduleName]?.["iframe-routes"]?.[pageName]?.["base-kibana-path"] ?? "/kibana/", window.location.origin).href;- Improve the error handling by showing user feedback in case of an error. For example:
} catch (error) {
console.log("axios resp err", error.message);
console.error("Error fetching data:", error);
+ Toast.error("Failed to login to Kibana. Please try again later.");
} finally {
setIsAxiosLoading(false);
}- Move the
setIsAxiosLoading(false)to thetryblock to avoid setting it tofalsein case of an error. For example:
try {
// ...
if (setCookie) {
console.log("axios cookie set", setCookie);
document.cookie = setCookie;
}
+ setIsAxiosLoading(false);
} catch (error) {
// ...
- } finally {
- setIsAxiosLoading(false);
}Committable suggestion was skipped due to low confidence.
| else: | ||
| return ['Last seven days', 'Last 15 days', 'currentMonth-Till date', 'Consolidated (As on date)', 'FY to date'] |
There was a problem hiding this comment.
Fix the syntax errors in the else clause.
The else clause is missing a colon (:) at the end, and the indentation of the return statement is incorrect.
Apply this diff to fix the syntax errors:
- else:
- return ['Last seven days', 'Last 15 days', 'currentMonth-Till date', 'Consolidated (As on date)', 'FY to date']
+ else:
+ return ['Last seven days', 'Last 15 days', 'currentMonth-Till date', 'Consolidated (As on date)', 'FY to date']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.
| else: | |
| return ['Last seven days', 'Last 15 days', 'currentMonth-Till date', 'Consolidated (As on date)', 'FY to date'] | |
| else: | |
| return ['Last seven days', 'Last 15 days', 'currentMonth-Till date', 'Consolidated (As on date)', 'FY to date'] |
Tools
Ruff
821-821: SyntaxError: Expected a statement
821-821: SyntaxError: Expected a statement
821-822: SyntaxError: Expected a statement
822-822: SyntaxError: Unexpected indentation
ISTE-391: Dropdown Menu Enhancement and Role Display Issue
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation