fix: resolve HRMS issues from Frappe Errors 2026#5
fix: resolve HRMS issues from Frappe Errors 2026#5mprinceb wants to merge 3 commits intocustom-patches2from
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f7d202075
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
frontend/src/components/ListView.vue
Outdated
| if (isTeamRequest.value && managedEmployees.length) { | ||
| docs = docs.filter((item) => managedEmployees.includes(item.employee)) |
There was a problem hiding this comment.
The new team-request scoping only runs when managedEmployees.length is non-zero, so users with no managed employees now skip scoping entirely and still keep the employee != current_user default filter. In Team tabs this returns other employees’ records instead of an empty result, which exposes non-team data (e.g., attendance/shift/expense/advance team views) to users who should see none.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3083c32: team request scoping now always applies, including when managed employee list is empty, so no non-team data leaks into team views.
frontend/src/components/ListView.vue
Outdated
| if (condition === "Like" || condition === "like") { | ||
| return `${docValue ?? ""}`.toLowerCase().includes(`${value ?? ""}`.toLowerCase()) | ||
| } | ||
|
|
||
| return true |
There was a problem hiding this comment.
Team Leaves now relies on applyLocalFilters, but this logic only handles =, !=, and Like; all other operators fall through to true. The filter UI allows date comparisons (>, <, >=, <=), so those selections are silently ignored on Team Leaves and users receive unfiltered results despite applying filters.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3083c32: local filter logic now handles >, <, >=, <= operators used by date filters in Team Leaves.
|
Addressed review feedback in commit 3083c32.\n\n- Team scoping now always applies in team requests, even when managed employees resolves to an empty list (returns empty, no data leakage).\n- Local filter handling now supports comparison operators (, , , ) used by date filters.\n- Added refresh watcher so team data is re-fetched after managed employees loading completes. |
|
Correction to previous comment formatting: review feedback is addressed in commit 3083c32.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3083c32139
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| transform(data) { | ||
| const start = teamLeaveDocuments.params?.start || 0 | ||
| const filtered = applyLocalFilters(Array.isArray(data) ? [...data] : []) |
There was a problem hiding this comment.
In Team Leaves mode this code applies all filters client-side, but hrms.api.get_leave_applications_for_approval only returns a limited field set (it does not include fields like department in hrms/api/__init__.py), so selecting Department in Leave History makes doc.department undefined for every row and the equality check filters out all records. This causes false empty results specifically when users apply filters that are present in the UI but missing from the Team Leaves payload.
Useful? React with 👍 / 👎.
| employee: employee.data.name, | ||
| approver_id: employee.data.user_id, | ||
| for_approval: 1, | ||
| limit: 500, |
There was a problem hiding this comment.
Team Leaves fetches are hard-limited to 500 records, and subsequent scroll loads only page through that same capped result, so approvers with more than 500 eligible leave applications can never reach older entries. This is a data-visibility regression that appears in larger datasets where Team Leaves exceeds the fixed cap.
Useful? React with 👍 / 👎.
|
Added follow-up fixes for remaining mobile issues from the PDF in commit fece1fb:
This targets issue #5 (user-list auth failure path) and issue frappe#8 (left employees still shown in mobile). |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fece1fbe18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (normalizedCondition === "=") { | ||
| return `${docValue ?? ""}` === `${value ?? ""}` |
There was a problem hiding this comment.
Team Leaves filtering is now done client-side, but this equality check treats absent fields as empty strings, so any filter on a field not present in the fetched payload (for example department) rejects every row and shows a false empty state. Since get_leave_applications_for_approval does not return all filterable fields, this path regresses valid filters into “no data” for approvers.
Useful? React with 👍 / 👎.
| employee: employee.data.name, | ||
| approver_id: employee.data.user_id, | ||
| for_approval: 1, | ||
| limit: 500, |
There was a problem hiding this comment.
The Team Leaves request hard-codes limit: 500 and then paginates by slicing client-side, so records beyond the first 500 can never be reached even when infinite scroll continues. In environments with large teams or long histories, approvers will silently miss older applications.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR fixes the
hrmsapp side of the issues from Frappe Errors 2026.pdf fortcr.localhost, focused on approver visibility, salary slip robustness, and requested list filters.Changes and Reasoning
Fix Team Leaves loading in Leave History (including direct
?tab=Team+Leavesroute)frontend/src/components/ListView.vuehrms.api.get_leave_applications_for_approval), local filter handling, safer infinite-scroll pagination source, and workflow-field dedupe.Unify managed-employee resolution for approver flows
hrms/api/__init__.py_get_managed_employees_for_approver()using all relevant mappings:custom_leave_approvers)leave_approvercustom_reporting_manager_l1get_managed_employeesget_leave_applications_for_approvalget_today_holidays_for_employeeget_attendance_regularization_requestsget_sunday_holiday_working_requests_for_approverSalary slip download fallback for print-format errors
hrms/api/__init__.pyStandardformat if default print format fails.Leave approval conflict handling for half-day attendance edge case
hrms/hr/doctype/leave_application/leave_application.pyhalf_day_datefor half-day leave applications.Add requested L1/status/date filters across HR list views
hrms/hr/doctype/attendance/attendance_list.jshrms/hr/doctype/employee_checkin/employee_checkin_list.jshrms/hr/doctype/leave_allocation/leave_allocation_list.jshrms/hr/doctype/leave_policy_assignment/leave_policy_assignment_list.jshrms/hr/doctype/shift_assignment/shift_assignment_list.jshrms/payroll/doctype/payroll_entry/payroll_entry_list.jshrms/payroll/doctype/salary_slip/salary_slip_list.jshrms/payroll/doctype/salary_structure_assignment/salary_structure_assignment_list.js(new)Validation
npm run build(HRMS frontend)python3 -m py_compileon changed Python modulesnode --checkon changed JS modulesbench --site tcr.localhost migratebench --site tcr.localhost clear-cache && clear-website-cache