Skip to content

fix: resolve IndentationError in get_leaves_for_period - #5035

Open
Ramymurugan wants to merge 1 commit into
frappe:developfrom
Ramymurugan:develop
Open

fix: resolve IndentationError in get_leaves_for_period#5035
Ramymurugan wants to merge 1 commit into
frappe:developfrom
Ramymurugan:develop

Conversation

@Ramymurugan

Copy link
Copy Markdown

Summary

This PR fixes an IndentationError in the get_leaves_for_period function.

Changes

  • Corrected the indentation in the affected code.
  • Restored the proper execution flow of the function.
  • Ensured that the HRMS application runs without the indentation-related error.

Testing

  • Ran the HRMS application successfully after applying the fix.
  • Verified that the affected functionality executes without the IndentationError.

Related Issue

Closes #5034

@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The PR appears safe to merge, but the new per-entry database lookup should be replaced with a bulk fetch or query join.

The functional change has no established correctness failure, while its unconditional lookup creates a concrete non-blocking N+1 performance regression.

Files Needing Attention: hrms/hr/doctype/leave_application/leave_application.py

Reviews (1): Last reviewed commit: "fix: resolve IndentationError in get_lea..." | Re-trigger Greptile

Comment on lines +1322 to +1327
leave_application = frappe.db.get_value(
"Leave Application",
leave_entry.transaction_name,
["leave_type", "half_day", "half_day_date"],
as_dict=True,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Avoid per-entry database queries

get_leaves_for_period now performs frappe.db.get_value for every matching ledger entry, making leave-balance reports, encashment, and balance checks progressively slower as ledger history grows; fetch these fields in bulk or join them into the existing query.

Context Used: Guidelines for reviewing Frappe Framework applicat... (source)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant