Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Hrms bounday service #1544

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Hrms bounday service #1544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
994f56a
updated egov-location service to boundary-service
Swathi-eGov Oct 16, 2024
e5c9582
added includeParents
Swathi-eGov Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions micro-ui/web/micro-ui-internals/example/src/setupProxy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = function (app) {
"/mdms-v2",
"/egov-idgen",
"/egov-location",
"/boundary-service",
"/localization",
"/egov-workflow-v2",
"/pgr-services",
Expand Down
4 changes: 2 additions & 2 deletions micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useLocation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { useQuery } from 'react-query';
import { LocationService } from "../services/elements/Location";

const useLocation = (tenantId, locationType, config = {}) => {
const useLocation = (tenantId, locationType, config = {},includeChildren,includeParents) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider adding default values for new parameters to maintain backward compatibility.

The function signature has been updated to include two new parameters, includeChildren and includeParents. While this change allows for more flexibility, it could potentially break existing code that calls this function without providing these new parameters.

To maintain backward compatibility, consider adding default values for these new parameters:

- const useLocation = (tenantId, locationType, config = {},includeChildren,includeParents) => {
+ const useLocation = (tenantId, locationType, config = {}, includeChildren = false, includeParents = false) => {

This change would ensure that existing calls to useLocation continue to work as before, while still allowing the new functionality to be used when needed.

📝 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.

Suggested change
const useLocation = (tenantId, locationType, config = {},includeChildren,includeParents) => {
const useLocation = (tenantId, locationType, config = {}, includeChildren = false, includeParents = false) => {

switch(locationType) {
case 'Locality':
return useQuery(["LOCALITY_DETAILS", tenantId ], () => LocationService.getLocalities(tenantId), config);
case 'Ward':
return useQuery(["WARD_DETAILS", tenantId ], () => LocationService.getWards(tenantId), config);
return useQuery(["WARD_DETAILS", tenantId], () => LocationService.getWards(tenantId,includeChildren,includeParents), config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Update the query key to include new parameters for proper caching.

The 'Ward' case has been correctly updated to pass the new includeChildren and includeParents parameters to LocationService.getWards. However, the query key for this case hasn't been updated to reflect these new parameters.

To ensure proper caching behavior with React Query, update the query key to include the new parameters:

- return useQuery(["WARD_DETAILS", tenantId], () => LocationService.getWards(tenantId,includeChildren,includeParents), config);
+ return useQuery(["WARD_DETAILS", tenantId, includeChildren, includeParents], () => LocationService.getWards(tenantId, includeChildren, includeParents), config);

This change will ensure that React Query creates separate cache entries for different combinations of includeChildren and includeParents, preventing potential issues with stale or incorrect data.

📝 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.

Suggested change
return useQuery(["WARD_DETAILS", tenantId], () => LocationService.getWards(tenantId,includeChildren,includeParents), config);
return useQuery(["WARD_DETAILS", tenantId, includeChildren, includeParents], () => LocationService.getWards(tenantId, includeChildren, includeParents), config);

default:
break
}
Expand Down
9 changes: 6 additions & 3 deletions micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/urls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
const mdmsV1Path = window?.globalConfigs?.getConfig("MDMS_V1_CONTEXT_PATH") || "egov-mdms-service";
const mdmsV2Path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
const boundarySearchPath = window?.globalConfigs?.getConfig("BOUNDARY_CONTEXT") || "boundary-service/boundary-relationships/_search?";
const hierarchyType = window?.globalConfigs?.getConfig("HIERARCHY_TYPE") || "hierarchyType=ADMIN";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

LGTM: Well-implemented configuration constant with a minor suggestion.

The hierarchyType constant is well-implemented, consistent with boundarySearchPath. However, consider simplifying the default value:

const hierarchyType = window?.globalConfigs?.getConfig("HIERARCHY_TYPE") || "ADMIN";

Then, when using this constant, you can add the "hierarchyType=" prefix:

`hierarchyType=${hierarchyType}`

This approach separates the configuration value from its usage, potentially making it more flexible for future changes.


const Urls = {
MDMS_V2:`/${mdmsV2Path}/v1/_search`,
MDMS: `/${mdmsV1Path}/v1/_search`,
WorkFlow: `/egov-workflow-v2/egov-wf/businessservice/_search`,
WorkFlowProcessSearch: `/egov-workflow-v2/egov-wf/process/_search`,
localization: `/localization/messages/v1/_search`,
location: {
localities: `/egov-location/location/v11/boundarys/_search?hierarchyTypeCode=ADMIN&boundaryType=Locality`,
wards: `/egov-location/location/v11/boundarys/_search?hierarchyTypeCode=ADMIN&boundaryType=Ward`,
revenue_localities: `/egov-location/location/v11/boundarys/_search?hierarchyTypeCode=REVENUE&boundaryType=Locality`,
localities: `/${boundarySearchPath}includeChildren=true&${hierarchyType}&boundaryType=locality`,
wards: `/${boundarySearchPath}includeChildren=true&${hierarchyType}&boundaryType=Ward&includeParents=true`,
revenue_localities: `/boundary-service/boundary-relationships/_search?hierarchyType=REVENUE&boundaryType=locality`,
},

pgr_search: `/pgr-services/v2/request/_search`,
Expand Down
2 changes: 1 addition & 1 deletion micro-ui/web/micro-ui-internals/packages/libraries/src/services/elements/Localities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { LocalizationService } from "./Localization/service";

const ADMIN_CODE = ({ tenantId, hierarchyType }) => {
return tenantId.replace(".", "_").toUpperCase() + "_" + hierarchyType.code;
return tenantId.replace(".", "_").toUpperCase() + "_" + hierarchyType;
};

const getI18nKeys = (localitiesWithLocalizationKeys) => {
Expand Down
4 changes: 2 additions & 2 deletions micro-ui/web/micro-ui-internals/packages/libraries/src/services/elements/Location.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ export const LocationService = {
});
return response;
},
getWards: (tenantId) => {
getWards: (tenantId,includeChildren,includeParents) => {
return ServiceRequest({
serviceName: "getWards",
url: Urls.location.wards,
params: { tenantId: tenantId },
params: { tenantId: tenantId,includeChildren: includeChildren, includeParents},
useCache: true,
});
Comment on lines +22 to 28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

⚠️ Potential issue

Update getWards method to correctly pass includeParents parameter

The changes to the getWards method look good overall. The method signature has been updated to include the new parameters, and they are being passed to the ServiceRequest function. However, there's a small issue with how the includeParents parameter is being passed.

Apply this diff to fix the includeParents parameter:

   getWards: (tenantId,includeChildren,includeParents) => {
     return ServiceRequest({
       serviceName: "getWards",
       url: Urls.location.wards,
-      params: { tenantId: tenantId,includeChildren: includeChildren, includeParents},
+      params: { tenantId, includeChildren, includeParents },
       useCache: true,
     });
   }

This change ensures that:

  1. The includeParents parameter is correctly passed with its value.
  2. We use object property shorthand for consistency and brevity.

Consider adding spaces after commas in the method signature for better readability:

getWards: (tenantId, includeChildren, includeParents) => {

}
Expand Down
34 changes: 25 additions & 9 deletions ...eb/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,22 @@ const Jurisdictions = ({ t, config, onSelect, userType, formData }) => {
let boundaryTypeoption = [];
const [focusIndex, setFocusIndex] = useState(-1);

const { isBoundaryServiceLoading, data: hierarchyOptions } = Digit.Hooks.useLocation(
tenantId,
"Ward",
{
select: (data) => {
return data;
},
},
true
);

Comment on lines +115 to +125
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Simplify the useLocation hook by removing the redundant select function.

The select function in the Digit.Hooks.useLocation hook is returning the data without any modification, making it unnecessary. Removing it will simplify the code without affecting its functionality.

Apply this diff to simplify the code:

 const { isBoundaryServiceLoading, data: hierarchyOptions } = Digit.Hooks.useLocation(
     tenantId,
     "Ward",
-    {
-      select: (data) => {
-        return data;
-      },
-    },
     true
   );
📝 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.

Suggested change
const { isBoundaryServiceLoading, data: hierarchyOptions } = Digit.Hooks.useLocation(
tenantId,
"Ward",
{
select: (data) => {
return data;
},
},
true
);
const { isBoundaryServiceLoading, data: hierarchyOptions } = Digit.Hooks.useLocation(
tenantId,
"Ward",
true
);

function gethierarchylistdata() {
return data?.MdmsRes?.["egov-location"]["TenantBoundary"].map((ele) => ele.hierarchyType);
return hierarchyOptions?.TenantBoundary?.map((ele) => ({
code: ele.hierarchyType,
name: ele.hierarchyType
}));
}

function getboundarydata() {
Expand All @@ -124,7 +138,7 @@ const Jurisdictions = ({ t, config, onSelect, userType, formData }) => {
return data?.MdmsRes?.["ACCESSCONTROL-ROLES"].roles.map(role => { return { code: role.code, name: role?.name ? role?.name : " " , labelKey: 'ACCESSCONTROL_ROLES_ROLES_' + role.code } });
}

if (isLoading) {
if (isLoading || isBoundaryServiceLoading) {
return <Loader />;
}
return (
Expand All @@ -148,6 +162,7 @@ const Jurisdictions = ({ t, config, onSelect, userType, formData }) => {
getboundarydata={getboundarydata}
getroledata={getroledata}
handleRemoveUnit={handleRemoveUnit}
hierarchyOptions={hierarchyOptions}
/>
))}
<label onClick={handleAddUnit} className="link-label" style={{ width: "12rem" }}>
Expand All @@ -168,18 +183,19 @@ function Jurisdiction({
getroledata,
roleoption,
index,
hierarchyOptions
}) {
const [BoundaryType, selectBoundaryType] = useState([]);
const [Boundary, selectboundary] = useState([]);
useEffect(() => {
selectBoundaryType(
data?.MdmsRes?.["egov-location"]["TenantBoundary"]
.filter((ele) => {
return ele?.hierarchyType?.code == jurisdiction?.hierarchy?.code;
})
.map((item) => { return { ...item.boundary, i18text: Digit.Utils.locale.convertToLocale(item.boundary.label, 'EGOV_LOCATION_BOUNDARYTYPE') } })
const filteredBoundaryTypes = hierarchyOptions?.TenantBoundary?.filter((ele) => ele.hierarchyType === jurisdiction?.hierarchy?.code).map(
(item) => ({
...item.boundary[0],
i18text: Digit.Utils.locale.convertToLocale(item.boundary[0].boundaryType, "EGOV_LOCATION_BOUNDARYTYPE"),
})
);
}, [jurisdiction?.hierarchy, data?.MdmsRes]);
selectBoundaryType(filteredBoundaryTypes);
}, [jurisdiction?.hierarchy, hierarchyOptions]);
Comment on lines +191 to +198
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure safe access to item.boundary[0] to prevent potential runtime errors.

In the mapping function within the useEffect hook, accessing item.boundary[0] without checking if item.boundary exists and contains elements could lead to runtime errors if item.boundary is undefined or empty. Please add a check to ensure item.boundary is defined and not empty before accessing item.boundary[0].

Apply this diff to add safety checks:

 useEffect(() => {
   const filteredBoundaryTypes = hierarchyOptions?.TenantBoundary?.filter(
     (ele) => ele.hierarchyType === jurisdiction?.hierarchy?.code
   ).map((item) => {
+    if (!item.boundary || item.boundary.length === 0) {
+      return null; // or handle the case accordingly
+    }
     return {
       ...item.boundary[0],
       i18text: Digit.Utils.locale.convertToLocale(item.boundary[0].boundaryType, "EGOV_LOCATION_BOUNDARYTYPE"),
     };
   }).filter(Boolean);
   selectBoundaryType(filteredBoundaryTypes);
 }, [jurisdiction?.hierarchy, hierarchyOptions]);
📝 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.

Suggested change
const filteredBoundaryTypes = hierarchyOptions?.TenantBoundary?.filter((ele) => ele.hierarchyType === jurisdiction?.hierarchy?.code).map(
(item) => ({
...item.boundary[0],
i18text: Digit.Utils.locale.convertToLocale(item.boundary[0].boundaryType, "EGOV_LOCATION_BOUNDARYTYPE"),
})
);
}, [jurisdiction?.hierarchy, data?.MdmsRes]);
selectBoundaryType(filteredBoundaryTypes);
}, [jurisdiction?.hierarchy, hierarchyOptions]);
const filteredBoundaryTypes = hierarchyOptions?.TenantBoundary?.filter((ele) => ele.hierarchyType === jurisdiction?.hierarchy?.code).map(
(item) => {
if (!item.boundary || item.boundary.length === 0) {
return null; // or handle the case accordingly
}
return {
...item.boundary[0],
i18text: Digit.Utils.locale.convertToLocale(item.boundary[0].boundaryType, "EGOV_LOCATION_BOUNDARYTYPE"),
};
}
).filter(Boolean);
selectBoundaryType(filteredBoundaryTypes);
}, [jurisdiction?.hierarchy, hierarchyOptions]);

const tenant = Digit.ULBService.getCurrentTenantId();
useEffect(() => {
selectboundary(data?.MdmsRes?.tenant?.tenants.filter(city => city.code != Digit.ULBService.getStateId()).map(city => { return { ...city, i18text: Digit.Utils.locale.getCityLocale(city.code) } }));
Expand Down
6 changes: 3 additions & 3 deletions ...web/micro-ui-internals/packages/react-components/src/molecules/LocationDropdownWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ const LocationDropdownWrapper = ({populators,formData,props,inputRef,errors,setV
const wards = []
const localities = {}
data?.TenantBoundary[0]?.boundary.forEach((item) => {
localities[item?.code] = item?.children.map(item => ({ code: item.code, name: item.name, i18nKey: `${headerLocale}_ADMIN_${item?.code}`, label: item?.label }))
wards.push({ code: item.code, name: item.name, i18nKey: `${headerLocale}_ADMIN_${item?.code}` })
localities[item?.code] = item?.children.map(item => ({ code: item.code, name: t(`${headerLocale}_ADMIN_${item?.code}`), i18nKey: `${headerLocale}_ADMIN_${item?.code}`, label: item?.label }))
wards.push({ code: item.code, name: t(`${headerLocale}_ADMIN_${item?.code}`), i18nKey: `${headerLocale}_ADMIN_${item?.code}` })
Comment on lines +24 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

LGTM! Consider extracting the translation key construction.

The changes correctly implement localization for ward and locality names using the t function. This is a good practice for internationalization.

To improve readability, consider extracting the translation key construction into a separate function:

const getTranslationKey = (code) => `${headerLocale}_ADMIN_${code}`;

// Then use it like this:
localities[item?.code] = item?.children.map(item => ({ 
  code: item.code, 
  name: t(getTranslationKey(item?.code)), 
  i18nKey: getTranslationKey(item?.code), 
  label: item?.label 
}));

wards.push({ 
  code: item.code, 
  name: t(getTranslationKey(item?.code)), 
  i18nKey: getTranslationKey(item?.code) 
});

This would make the code more DRY and easier to maintain.

});

return {
wards, localities
}
}
});
},true);


useEffect(() => {
Expand Down