feat: Salary Withholding #1726
feat: Salary Withholding #1726
Conversation
vinyselopal
commented
Apr 30, 2024
vinyselopal
commented
vinyselopal
force-pushed
the
salary_withhold
branch
from
0d365b9
to
3827d73
Compare
…on in Employee form
…e details from backend
vinyselopal
force-pushed
the
salary_withhold
branch
from
3827d73
to
76ed868
Compare
| "date_of_joining": frappe.get_value("Employee", employee, "date_of_joining"), | ||
| "relieving_date": frappe.get_value("Employee", employee, "relieving_date"), | ||
| "employee_name": frappe.get_value("Employee", employee, "employee_name"), | ||
| "resignation_letter_date": frappe.get_value("Employee", employee, "resignation_letter_date"), | ||
| "notice_number_of_days": frappe.get_value("Employee", employee, "notice_number_of_days"), |
There was a problem hiding this comment.
This will lead to 5 DB calls from a single table. All of this should be fetched in a single query. You can just set fetch from properties for these fields and eliminate this code completely
| @@ -1158,6 +1158,7 @@ def get_filtered_employees( | |||
| query = set_fields_to_select(query, fields) | |||
| query = set_searchfield(query, searchfield, search_string, qb_object=Employee) | |||
| query = set_filter_conditions(query, filters, qb_object=Employee) | |||
| salary_withholding_documents = frappe.db.get_all("Salary Withholding") | |||
There was a problem hiding this comment.
The date filtering for fetching salary withholding documents should happen here itself instead of fetching all records
There was a problem hiding this comment.
I meant the date filtering should happen in this get_all query based on dates instead of looping and filtering in python later.
Not "join" in the main query since this won't be a widely used feature initially
| employee = frappe.db.get_all( | ||
| "Employee", | ||
| filters={"name": employee}, | ||
| fields=[ | ||
| "date_of_joining", | ||
| "relieving_date", | ||
| "employee_name", | ||
| "resignation_letter_date", | ||
| "notice_number_of_days", | ||
| ], | ||
| ) |
There was a problem hiding this comment.
| employee = frappe.db.get_all( | |
| "Employee", | |
| filters={"name": employee}, | |
| fields=[ | |
| "date_of_joining", | |
| "relieving_date", | |
| "employee_name", | |
| "resignation_letter_date", | |
| "notice_number_of_days", | |
| ], | |
| ) | |
| employee = frappe.db.get_value( | |
| "Employee", | |
| employee, | |
| fields=[ | |
| "date_of_joining", | |
| "relieving_date", | |
| "employee_name", | |
| "resignation_letter_date", | |
| "notice_number_of_days", | |
| ], | |
| as_dict=True, | |
| ) |
get_all is for fetching multiple records. We just need a single record here and also know the name of the record so get_value is the correct way
This will eliminate employee[0].date_of_joining too because this will directly return the dictionary not a list of dictionaries
…d Number of Withheld Cycles field
| @@ -725,6 +725,7 @@ | |||
| "oldfieldtype": "Section Break" | |||
| }, | |||
| { | |||
| "default": "Released", | |||
There was a problem hiding this comment.
There is already a status field in Salary Slip. Use the same?
… to payroll Entry
… field to doctypes
vinyselopal
force-pushed
the
salary_withhold
branch
from
a33191a
to
97a425b
Compare
…e statuses on cancel
