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

hrms edit and deactivate bug fixes #1655

Merged
merged 2 commits into from
Oct 24, 2024
Merged

hrms edit and deactivate bug fixes #1655

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9d64fb
hrms edit and deactivate bug fixes
mithunhegde-egov Oct 24, 2024
d405b79
removed console log
mithunhegde-egov Oct 24, 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
15 changes: 10 additions & 5 deletions micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/EditEmployee/EditForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const EditForm = ({ tenantId, data }) => {
const [phonecheck, setPhonecheck] = useState(false);
const [checkfield, setcheck] = useState(false);
const mutationUpdate = Digit.Hooks.hrms.useHRMSUpdate(tenantId);
const isMultiRootTenant = Digit.Utils.getMultiRootTenant();

const { data: mdmsData, isLoading } = Digit.Hooks.useCommonMDMS(Digit.ULBService.getStateId(), "egov-hrms", ["CommonFieldsConfig"], {
select: (data) => {
return {
Expand Down Expand Up @@ -80,8 +82,8 @@ const EditForm = ({ tenantId, data }) => {
name: ele.hierarchy,
},
boundaryType: { label: ele.boundaryType, i18text: `EGOV_LOCATION_BOUNDARYTYPE_${ele.boundaryType.toUpperCase()}` },
boundary: Digit.Utils.getMultiRootTenant()?{ code: tenantId }:{ code: ele.boundary },
roles: data?.user?.roles.filter((item) => item.tenantId == ele.boundary),
boundary: { code: ele.boundary },
roles: isMultiRootTenant?data?.user?.roles:data?.user?.roles.filter((item) => item.tenantId == ele.boundary),
});
}),
Assignments: data?.assignments.map((ele, index) => {
Expand Down Expand Up @@ -171,11 +173,9 @@ const EditForm = ({ tenantId, data }) => {
input.Jurisdictions = input?.Jurisdictions?.map((juris) => {
return {
...juris,
boundary: tenantId,
tenantId: tenantId,
};
});

if (
!Object.values(
input.Jurisdictions.reduce((acc, sum) => {
Expand All @@ -191,7 +191,12 @@ const EditForm = ({ tenantId, data }) => {
}
let roles = input?.Jurisdictions?.map((ele) => {
return ele.roles?.map((item) => {
item["tenantId"] = ele.boundary;
if(isMultiRootTenant){
item["tenantId"] = tenantId;
}
else{
item["tenantId"] = ele.boundary;
}
return item;
});
});
Expand Down
7 changes: 6 additions & 1 deletion micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/EmployeeDetails.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const Details = () => {
const [mutationHappened, setMutationHappened, clear] = Digit.Hooks.useSessionStorage("EMPLOYEE_HRMS_MUTATION_HAPPENED", false);
const [successData, setsuccessData, clearSuccessData] = Digit.Hooks.useSessionStorage("EMPLOYEE_HRMS_MUTATION_SUCCESS_DATA", false);
const isMobile = window.Digit.Utils.browser.isMobile();
const isMultiRootTenant = Digit.Utils.getMultiRootTenant();

useEffect(() => {
setMutationHappened(false);
Expand Down Expand Up @@ -62,6 +63,9 @@ const Details = () => {
return <Loader />;
}

console.log(":data of the employee", data);
console.log("role data is", data?.Employees?.[0]?.user.roles.map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove them


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

Remove console.log statements.

Debug console logs should not be committed to production code. These statements expose sensitive employee data and role information which could be a security concern.

Apply this diff to remove the debug statements:

-  console.log(":data of the employee", data);
-  console.log("role data is", data?.Employees?.[0]?.user.roles.map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)));
-
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log(":data of the employee", data);
console.log("role data is", data?.Employees?.[0]?.user.roles.map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)));
🧰 Tools
🪛 Biome

[error] 67-67: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

return (
<React.Fragment>
<div style={isMobile ? {marginLeft: "-12px", fontFamily: "calibri", color: "#FF0000"} :{ marginLeft: "15px", fontFamily: "calibri", color: "#FF0000" }}>
Expand Down Expand Up @@ -169,7 +173,8 @@ const Details = () => {
<Row label={t("HR_BOUNDARY_LABEL")} text={t(element?.boundary)} />
<Row
label={t("HR_ROLE_LABEL")}
text={data?.Employees?.[0]?.user.roles.filter((ele) => ele.tenantId == element?.boundary).map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code))}
text={
isMultiRootTenant? data?.Employees?.[0]?.user.roles.map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)):(data?.Employees?.[0]?.user.roles.filter((ele) => ele.tenantId == element?.boundary).map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)))}
Comment on lines +174 to +175
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve role display logic for better maintainability and safety.

The current implementation has several areas for improvement:

  1. Uses string concatenation instead of template literals
  2. Uses loose equality comparison
  3. Lacks null safety checks
  4. Has complex nested logic

Consider this enhanced version of the previous refactoring suggestion:

  text={
-   isMultiRootTenant? data?.Employees?.[0]?.user.roles.map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)):(data?.Employees?.[0]?.user.roles.filter((ele) => ele.tenantId == element?.boundary).map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)))}
+   (() => {
+     const roles = data?.Employees?.[0]?.user?.roles;
+     if (!roles?.length) return 'NA';
+     
+     const filteredRoles = isMultiRootTenant 
+       ? roles 
+       : roles.filter((role) => role?.tenantId === element?.boundary);
+     
+     return filteredRoles.map((role) => 
+       t(`ACCESSCONTROL_ROLES_ROLES_${role?.code || ''}`)
+     );
+   })()
+ }

This refactoring:

  1. Uses template literals (fixes static analysis warning)
  2. Adds comprehensive null safety checks
  3. Uses strict equality comparison
  4. Handles empty roles gracefully
  5. Improves readability by breaking down the logic
  6. Uses meaningful variable names
📝 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
text={
isMultiRootTenant? data?.Employees?.[0]?.user.roles.map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)):(data?.Employees?.[0]?.user.roles.filter((ele) => ele.tenantId == element?.boundary).map((ele) => t(`ACCESSCONTROL_ROLES_ROLES_` + ele?.code)))}
text={
(() => {
const roles = data?.Employees?.[0]?.user?.roles;
if (!roles?.length) return 'NA';
const filteredRoles = isMultiRootTenant
? roles
: roles.filter((role) => role?.tenantId === element?.boundary);
return filteredRoles.map((role) =>
t(`ACCESSCONTROL_ROLES_ROLES_${role?.code || ''}`)
);
})()
}
🧰 Tools
🪛 Biome

[error] 175-175: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[error] 175-175: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

/>
</StatusTable>
);
Expand Down
3 changes: 1 addition & 2 deletions micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/createEmployee.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,10 @@ const CreateEmployee = () => {
data.Jurisdictions = data?.Jurisdictions?.map((juris) => {
return {
...juris,
boundary: selectedCity,
tenantId: tenantId
};
});
// If no current assignment, throw an error

if(!canSubmit){
setShowToast({ key: "error", label: "ERR_ALL_MANDATORY_FIELDS" });
return;
Expand Down
Loading