-
Notifications
You must be signed in to change notification settings - Fork 11
Finance module merge #359
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
base: master
Are you sure you want to change the base?
Finance module merge #359
Changes from all commits
da3c0de
53521fa
8f128f8
d84114e
89d3e05
f93f673
6fe67a7
3050f47
3d5946e
b6f88c7
c871cc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| FROM egovio/flyway:4.1.2 | ||
| FROM egovio/flyway:10.7.1 | ||
|
|
||
| COPY ./migration/main /flyway/sql | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||||||
| # digit-ui-module-finance | ||||||||
|
|
||||||||
| ## Install | ||||||||
|
|
||||||||
| ```bash | ||||||||
| npm install --save @egovernments/digit-ui-module-finance | ||||||||
| ``` | ||||||||
|
|
||||||||
| ## Limitation | ||||||||
|
|
||||||||
| ```bash | ||||||||
| This Package is more specific to Urban | ||||||||
| ``` | ||||||||
|
|
||||||||
| ## Usage | ||||||||
|
|
||||||||
| After adding the dependency make sure you have this dependency in | ||||||||
|
|
||||||||
| ```bash | ||||||||
| frontend/micro-ui/web/package.json | ||||||||
| ``` | ||||||||
|
|
||||||||
| ## Changelog | ||||||||
|
|
||||||||
| ### Summary for Version [0.0.1] - 2025-06-13 | ||||||||
|
|
||||||||
| Created finance module with submenu card and iframe pages | ||||||||
|
|
||||||||
| ### Contributors | ||||||||
|
|
||||||||
| [kamalesh-egov] | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add missing trailing newline. The file should end with a single newline character as flagged by the linter. [kamalesh-egov]
+📝 Committable suggestion
Suggested change
🧰 Tools🪛 markdownlint-cli2 (0.17.2)31-31: Files should end with a single newline character (MD047, single-trailing-newline) 🤖 Prompt for AI Agents |
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| { | ||
| "name": "@egovernments/digit-ui-module-finance", | ||
| "version": "0.0.1-sandbox", | ||
| "description": "FINANCE", | ||
| "license": "MIT", | ||
| "main": "dist/index.js", | ||
| "module": "dist/index.modern.js", | ||
| "source": "src/Module.js", | ||
| "files": [ | ||
| "dist" | ||
| ], | ||
| "scripts": { | ||
| "start": "microbundle-crl watch --no-compress --format modern,cjs", | ||
| "build": "microbundle-crl --compress --no-sourcemap --format cjs", | ||
| "prepublish": "yarn build" | ||
| }, | ||
| "dependencies": { | ||
| "@egovernments/digit-ui-react-components": "^1.7.0-beta.2", | ||
| "react": "17.0.2", | ||
| "react-dom": "17.0.2", | ||
| "react-hook-form": "6.15.8", | ||
| "react-i18next": "11.16.2", | ||
| "react-query": "3.6.1", | ||
| "react-router-dom": "5.3.0" | ||
|
Comment on lines
+19
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify React version consistency across the monorepo. The React version is pinned to 17.0.2. Ensure this aligns with other packages in the monorepo to avoid version conflicts. 🏁 Script executed: #!/bin/bash
# Description: Check React versions across the monorepo for consistency
# Expected: All packages should use the same React version to avoid conflicts
echo "Checking React versions across packages:"
fd -t f "package.json" . --exec grep -l '"react":' {} \; | head -10 | xargs grep '"react":'Length of output: 1387 Align React versions across the monorepo A discrepancy was found in one package:
Please update this entry to 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { CollectionIcon, Header } from "@egovernments/digit-ui-react-components"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useTranslation } from "react-i18next"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import LandingPageSubMenuCard from "./components/LandingPageSubMenuCard"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const buildMenuDict = (items) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const root = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| items?.forEach(item => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const parts = item.path.split('.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let current = root; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parts?.forEach((part, idx) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!current[part]) current[part] = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (idx === parts.length - 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // attach leaf data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| current[part] = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...current[part], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _meta: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label: item.displayName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| link: item.navigationURL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| current = current[part]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return root; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add defensive programming for buildMenuDict. The function could be more robust with better null/undefined checks and error handling. const buildMenuDict = (items) => {
const root = {};
+ if (!Array.isArray(items)) return root;
+
items?.forEach(item => {
+ if (!item?.path) return;
const parts = item.path.split('.');
let current = root;
parts?.forEach((part, idx) => {
+ if (!part) return;
if (!current[part]) current[part] = {};
if (idx === parts.length - 1) {
// attach leaf data
current[part] = {
...current[part],
_meta: {
label: item.displayName,
link: item.navigationURL,
}
};
}
current = current[part];
});
});
return root;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const FinanceCard = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { t } = useTranslation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const userRoles = Digit.SessionStorage.get("User")?.info?.roles; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const allowedRoles = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EMPLOYEE_FINANCE", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_VOUCHER_CREATOR", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_MASTER_ADMIN", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_REPORT_VIEW", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_BILL_CREATOR", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_ADMINISTRATOR", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_BILL_APPROVER", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "SYS_INTEGRATOR_FINANCE", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_PAYMENT_CREATOR", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_VOUCHER_APPROVER", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "WORKS_FINANCIAL_APPROVER", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "EGF_PAYMENT_APPROVER" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isFinanceEmployee = userRoles.find((role) => allowedRoles.includes(role.code)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+31
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add defensive programming for role checking. The role checking logic assumes const userRoles = Digit.SessionStorage.get("User")?.info?.roles;
const allowedRoles = [
"EMPLOYEE_FINANCE",
"EGF_VOUCHER_CREATOR",
"EGF_MASTER_ADMIN",
"EGF_REPORT_VIEW",
"EGF_BILL_CREATOR",
"EGF_ADMINISTRATOR",
"EGF_BILL_APPROVER",
"SYS_INTEGRATOR_FINANCE",
"EGF_PAYMENT_CREATOR",
"EGF_VOUCHER_APPROVER",
"WORKS_FINANCIAL_APPROVER",
"EGF_PAYMENT_APPROVER"
];
- const isFinanceEmployee = userRoles.find((role) => allowedRoles.includes(role.code));
+ const isFinanceEmployee = Array.isArray(userRoles) && userRoles.find((role) => role?.code && allowedRoles.includes(role.code));📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { isLoading, data } = Digit.Hooks.useAccessControl(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const menuDict = buildMenuDict(data?.actions)?.Finance; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!isFinanceEmployee || !menuDict) return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Header styles={{ marginLeft: "24px", paddingTop: "10px", fontSize: "32px" }}>{t("ACTION_TEST_FINANCE")}</Header> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="moduleCardWrapper gridModuleWrapper"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {Object.entries(menuDict).map(([key, value]) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <LandingPageSubMenuCard key={key} t={t} Icon={<CollectionIcon />} moduleName={key} menuDict={value} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default FinanceCard; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React, { useEffect } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useRouteMatch } from "react-router-dom"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useTranslation } from "react-i18next"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import EmployeeApp from "./pages/employee"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import FinanceCard from "./FinanceHomeCard"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const FinanceModule = ({ stateCode, userType, tenants }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tenantId = Digit.SessionStorage.get("CITIZEN.COMMON.HOME.CITY")?.code || Digit.ULBService.getCurrentTenantId(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider defensive coding for tenantId retrieval. The current implementation may fail if the session storage data structure changes. Consider adding null checks or default values. - const tenantId = Digit.SessionStorage.get("CITIZEN.COMMON.HOME.CITY")?.code || Digit.ULBService.getCurrentTenantId();
+ const tenantId = Digit.SessionStorage.get("CITIZEN.COMMON.HOME.CITY")?.code ||
+ Digit.ULBService.getCurrentTenantId() ||
+ stateCode;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const moduleCode = ["finance"]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const language = Digit.StoreData.getCurrentLanguage(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { isLoading, data: store } = Digit.Services.useStore({ stateCode, moduleCode, language }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Remove unused store data. The - const { isLoading, data: store } = Digit.Services.useStore({ stateCode, moduleCode, language });
+ // Remove if not needed, or implement the intended store usage
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { path, url } = useRouteMatch(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Digit.SessionStorage.set("FINANCE_TENANTS", tenants); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Move side effects to useEffect. Setting session storage directly in the render function can cause unnecessary re-renders and side effects. Move this to a useEffect hook. + useEffect(() => {
+ Digit.SessionStorage.set("FINANCE_TENANTS", tenants);
+ }, [tenants]);
+
- Digit.SessionStorage.set("FINANCE_TENANTS", tenants);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (userType === "employee") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return <EmployeeApp path={path} url={url} userType={"employee"} />; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add PropTypes validation. The component lacks prop validation which could lead to runtime errors. Add PropTypes to ensure type safety. Add PropTypes import and validation: import React, { useEffect } from "react";
import { useRouteMatch } from "react-router-dom";
import { useTranslation } from "react-i18next";
+import PropTypes from "prop-types";
import EmployeeApp from "./pages/employee";
import FinanceCard from "./FinanceHomeCard";
export const FinanceModule = ({ stateCode, userType, tenants }) => {
// ... component implementation
};
+FinanceModule.propTypes = {
+ stateCode: PropTypes.string.isRequired,
+ userType: PropTypes.string.isRequired,
+ tenants: PropTypes.array
+};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const componentsToRegister = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FinanceModule, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FinanceCard, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const initFinanceComponents = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Object.entries(componentsToRegister).forEach(([key, value]) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Digit.ComponentRegistryService.setComponent(key, value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,78 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { fromPairs } from "lodash"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Remove unused import. The -import { fromPairs } from "lodash";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React, { Component } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class EGFFinance extends Component { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(props) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| super(props); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.onFrameLoad = this.onFrameLoad.bind(this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.resetIframe = this.resetIframe.bind(this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onFrameLoad() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document.getElementById("erp_iframe").style.display = "block"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| render() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let auth_token = Digit.UserService.getUser()?.access_token, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| locale = localStorage.getItem("locale"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| menuUrl = this.props.location, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loc = window.location, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subdomainurl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| domainurl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finEnv, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname = loc.hostname, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| winheight = window.innerHeight - 200, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Move inline height calculation to proper styling. Avoid inline calculations in render method and use proper CSS styling. - winheight = window.innerHeight - 200,Consider moving this to a computed property or CSS-in-JS solution for better maintainability.
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| erp_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tenantId = Digit.ULBService.getCurrentTenantId(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //Reading domain name from the request url | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| domainurl = hostname.substring(hostname.indexOf(".") + 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Reading environment name (ex: dev, qa, uat, fin-uat etc) from the globalconfigs if exists else reading from the .env file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finEnv = this.globalConfigExists() ? window.globalConfigs.getConfig("FIN_ENV") : process.env.REACT_APP_FIN_ENV; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Preparing finance subdomain url using the above environment name and the domain url | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subdomainurl = !!(finEnv) ? "-" + finEnv + "." + domainurl : "." + domainurl; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| erp_url = loc.protocol + "//" + tenantId.split(".")[1] + subdomainurl + menuUrl; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate environment configuration and URL construction. The URL construction logic could be vulnerable to injection attacks if environment variables are not properly validated. Add input validation: + // Validate finEnv to prevent injection
+ if (finEnv && !/^[a-zA-Z0-9-]+$/.test(finEnv)) {
+ console.error('Invalid FIN_ENV value detected');
+ return null;
+ }
+
+ // Validate tenantId format
+ if (!tenantId || !tenantId.includes('.')) {
+ console.error('Invalid tenantId format');
+ return null;
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <iframe name="erp_iframe" id="erp_iframe" height={winheight} width="100%"/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add accessibility title to iframe. The iframe is missing a title attribute, which is required for screen reader accessibility. - <iframe name="erp_iframe" id="erp_iframe" height={winheight} width="100%"/>
+ <iframe
+ name="erp_iframe"
+ id="erp_iframe"
+ height={winheight}
+ width="100%"
+ title="Finance ERP System"
+ />🧰 Tools🪛 Biome (2.1.2)[error] 36-36: Provide a title attribute when using iframe elements. Screen readers rely on the title set on an iframe to describe the content being displayed. (lint/a11y/useIframeTitle) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <form action={erp_url} id="erp_form" method="post" target="erp_iframe"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <input readOnly hidden="true" name="auth_token" value={auth_token} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <input readOnly hidden="true" name="tenantId" value={tenantId} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <input readOnly hidden="true" name="locale" value={locale} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <input readOnly hidden="true" name="formPage" value="true" /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </form> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| componentDidMount() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.addEventListener("message", this.onMessage, false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.addEventListener("loacaleChangeEvent", this.resetIframe, false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo in event listener. There's a typo in the event name that will prevent the locale change listener from working. - window.addEventListener("loacaleChangeEvent", this.resetIframe, false);
+ window.addEventListener("localeChangeEvent", this.resetIframe, false);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // document.getElementById("erp_iframe").addEventListener("load", this.onFrameLoad); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document.forms["erp_form"].submit(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+46
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add componentWillUnmount to clean up event listeners. Event listeners should be removed when the component unmounts to prevent memory leaks. Add this method: + componentWillUnmount() {
+ window.removeEventListener("message", this.onMessage, false);
+ window.removeEventListener("localeChangeEvent", this.resetIframe, false);
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| componentDidUpdate() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let isSecure = window.location.protocol === "https"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let localeCookie = "locale=" + localStorage.getItem("locale") + ";path=/;domain=." + this.getSubdomain(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isSecure) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| localeCookie += ";secure"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.document.cookie = localeCookie; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document.forms["erp_form"].submit(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+52
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize componentDidUpdate to prevent unnecessary form submissions. The form is resubmitted on every component update, which could cause performance issues and iframe flickering. - componentDidUpdate() {
+ componentDidUpdate(prevProps) {
+ const currentLocale = localStorage.getItem("locale");
+ const localeChanged = prevProps.location !== this.props.location;
+
+ if (!localeChanged) return;
+
let isSecure = window.location.protocol === "https";
- let localeCookie = "locale=" + localStorage.getItem("locale") + ";path=/;domain=." + this.getSubdomain();
+ let localeCookie = "locale=" + currentLocale + ";path=/;domain=." + this.getSubdomain();
if (isSecure) {
localeCookie += ";secure";
}
window.document.cookie = localeCookie;
document.forms["erp_form"].submit();
- }
+ }
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onMessage = (event) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (event.data != "close") return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // document.getElementById('erp_iframe').style.display='none'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.props.history.push("/inbox"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+61
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Improve message handler consistency and safety. Several improvements needed for the message handler: - onMessage = (event) => {
- if (event.data != "close") return;
+ onMessage(event) {
+ // Validate message origin for security
+ if (!this.validateMessageOrigin(event.origin)) return;
+
+ if (event.data !== "close") return;
// document.getElementById('erp_iframe').style.display='none';
this.props.history.push("/inbox");
- };
+ }Add origin validation method: + validateMessageOrigin(origin) {
+ const allowedOrigins = process.env.REACT_APP_ALLOWED_ORIGINS?.split(',') || [];
+ return allowedOrigins.includes(origin);
+ }Also bind this method in the constructor for consistency. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resetIframe() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.forceUpdate(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid using forceUpdate in favor of proper state management. Using + constructor(props) {
+ super(props);
+ this.state = { iframeKey: 0 };
+ this.onFrameLoad = this.onFrameLoad.bind(this);
+ this.resetIframe = this.resetIframe.bind(this);
+ }
+
resetIframe() {
- this.forceUpdate();
+ this.setState(prevState => ({ iframeKey: prevState.iframeKey + 1 }));
}Then use the key in the iframe: - <iframe name="erp_iframe" id="erp_iframe" height={winheight} width="100%"/>
+ <iframe
+ key={this.state.iframeKey}
+ name="erp_iframe"
+ id="erp_iframe"
+ height={winheight}
+ width="100%"
+ />
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getSubdomain() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let hostname = window.location.hostname; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return hostname.substring(hostname.indexOf(".") + 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+69
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Extract duplicate domain parsing logic. The subdomain extraction logic is duplicated between Consider consolidating the domain parsing logic and using 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| globalConfigExists() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return typeof window.globalConfigs !== "undefined" && typeof window.globalConfigs.getConfig === "function"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default EGFFinance; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,80 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React, { useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import PropTypes from 'prop-types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useHistory } from 'react-router-dom'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ArrowForward, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ArrowVectorDown, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@egovernments/digit-ui-react-components"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const RenderMenuDict = ({ t, tree, level = 0 }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const history = useHistory(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [openKeys, setOpenKeys] = useState({}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const toggle = (key) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setOpenKeys(prev => ({ ...prev, [key]: !prev[key] })); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const indentPx = level * 16; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const buttonBaseStyle = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| display: 'flex', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| justifyContent: 'space-between', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alignItems: 'center', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| padding: '8px 4px', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cursor: 'pointer', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fontSize: '16px', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| color: "#F47738", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Object.entries(tree).sort().map(([key, node], idx) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const meta = node._meta || {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const hasChildren = Object.keys(node).some(childKey => childKey !== '_meta'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isOpen = openKeys[key]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={level + '-' + idx}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| style={{ ...buttonBaseStyle, paddingLeft: `${indentPx + 8}px` }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick={() => hasChildren ? toggle(key) : meta.link && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| meta.link.includes(`${window.contextPath}/`) ? history.push(meta.link) : window.location.href = meta.link | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider security implications of external navigation. Direct assignment to +const isValidURL = (url) => {
+ try {
+ const urlObj = new URL(url);
+ return ['http:', 'https:'].includes(urlObj.protocol);
+ } catch {
+ return false;
+ }
+};
onClick={() => hasChildren ? toggle(key) : meta.link && (
- meta.link.includes(`${window.contextPath}/`) ? history.push(meta.link) : window.location.href = meta.link
+ meta.link.includes(`${window.contextPath}/`) ? history.push(meta.link) :
+ isValidURL(meta.link) ? window.location.href = meta.link : console.warn('Invalid URL blocked:', meta.link)
)}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <p>{t(meta.label || key)}</p> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {hasChildren && (isOpen ? <ArrowVectorDown styles={{width: "18px", height: "18px"}} /> : <div className='primary-label-btn'><ArrowForward /></div>)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix accessibility violations. The interactive span element lacks keyboard accessibility and proper semantic meaning. Use a button element with proper accessibility attributes. - <span
- style={{ ...buttonBaseStyle, paddingLeft: `${indentPx + 8}px` }}
- onClick={() => hasChildren ? toggle(key) : meta.link && (
- meta.link.includes(`${window.contextPath}/`) ? history.push(meta.link) : window.location.href = meta.link
- )}
- >
+ <button
+ style={{ ...buttonBaseStyle, paddingLeft: `${indentPx + 8}px`, border: 'none', background: 'none' }}
+ onClick={() => hasChildren ? toggle(key) : meta.link && (
+ meta.link.includes(`${window.contextPath}/`) ? history.push(meta.link) : window.location.href = meta.link
+ )}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ hasChildren ? toggle(key) : meta.link && (
+ meta.link.includes(`${window.contextPath}/`) ? history.push(meta.link) : window.location.href = meta.link
+ );
+ }
+ }}
+ aria-expanded={hasChildren ? isOpen : undefined}
+ role={hasChildren ? "button" : "link"}
+ >
<p>{t(meta.label || key)}</p>
{hasChildren && (isOpen ? <ArrowVectorDown styles={{width: "18px", height: "18px"}} /> : <div className='primary-label-btn'><ArrowForward /></div>)}
- </span>
+ </button>📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (2.1.2)[error] 35-40: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation. (lint/a11y/useKeyWithClickEvents) [error] 35-40: Static Elements should not be interactive. To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value. (lint/a11y/noStaticElementInteractions) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {hasChildren && isOpen && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <RenderMenuDict t={t} tree={Object.fromEntries( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Object.entries(node).filter(([k]) => k !== '_meta') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} level={level + 1} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const LandingPageSubMenuCard = ({ t, Icon, moduleName, menuDict = {}}) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className='employeeCard customEmployeeCard card-home home-action-cards'> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="complaint-links-container"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="header" > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="text removeHeight">{t(moduleName)}</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="logo removeBorderRadiusLogo">{Icon}</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="body" style={{ margin: "0px", padding: "0px" }}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="links-wrapper" style={{ width: "90%" }}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <RenderMenuDict t={t} tree={menuDict} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LandingPageSubMenuCard.propTypes = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| icon: PropTypes.node, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| moduleName: PropTypes.string.isRequired, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| menuDict: PropTypes.object | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+74
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix PropTypes to match component interface. The PropTypes don't match the actual props. The component receives LandingPageSubMenuCard.propTypes = {
- icon: PropTypes.node,
+ t: PropTypes.func.isRequired,
+ Icon: PropTypes.node,
moduleName: PropTypes.string.isRequired,
menuDict: PropTypes.object
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default LandingPageSubMenuCard; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,14 @@ | ||||||||||||||||
| import React, { useState, useEffect } from "react"; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Remove unused imports. The -import React, { useState, useEffect } from "react";
+import React from "react";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| import EGFFinance from "../../components/EGF"; | ||||||||||||||||
|
|
||||||||||||||||
| const EGF = () => { | ||||||||||||||||
| let location = window.location.pathname; | ||||||||||||||||
| location = location.substring(location.indexOf("/services")); | ||||||||||||||||
|
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider adding error handling for path manipulation. The current path manipulation assumes "/services" will always be present in the pathname. Consider adding validation to handle edge cases where this substring might not exist. let location = window.location.pathname;
-location = location.substring(location.indexOf("/services"));
+const servicesIndex = location.indexOf("/services");
+location = servicesIndex !== -1 ? location.substring(servicesIndex) : location;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| return ( | ||||||||||||||||
| <div> | ||||||||||||||||
| <EGFFinance location={location} /> | ||||||||||||||||
| </div> | ||||||||||||||||
| ); | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| export default EGF; | ||||||||||||||||
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.
remove the sandbox word from version