-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[ADD] hr_payroll_tds: Introduced Tax Declaration Sub-menu #650
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.
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
base: 18.0
Are you sure you want to change the base?
Conversation
cac9c44
to
09411bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work! This is a substantial codebase, so I’m providing the first round of review.
"""Opens the TDS Declarations associated with the current employee.""" | ||
|
||
action = self.env["ir.actions.actions"]._for_xml_id("hr_payroll_tds.action_open_declarations") | ||
target_ids = self.env["hr.tds.declaration.details"].search([("employee_id", "=", self.id)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a large amount of data, more than 1000 declarations, during a certain period, does that mean it impacts performance when clicking on the button takes extra time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can impact performance if there more data to load. but I think making field indexed can optimize performance even for large dataset.
return [(previous_year, previous_year), (current_financial_year, current_financial_year)] | ||
|
||
name = fields.Char(string="TDS Declaration", required=True) | ||
tds_declaration_ids = fields.One2many("hr.tds.declaration.details", "tds_declaration_id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TDS comes under the indian localization just for your knowledge we prefer to define field with l10n_in prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
if self.financial_year: | ||
start_year = int(self.financial_year.split("-")[0]) | ||
end_year = int(self.financial_year.split("-")[1]) | ||
self.start_date = fields.Date.to_date(f"{start_year}-04-01") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will crash when there are multiple records. Computation should always iterate within the loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the insight. I will modify.
self.end_date = fields.Date.to_date(f"{end_year}-03-31") | ||
|
||
def _compute_declarations_count(self): | ||
self.tds_declaration_count = len(self.tds_declaration_ids) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too
def _compute_declarations_count(self): | ||
self.tds_declaration_count = len(self.tds_declaration_ids) | ||
|
||
def action_approved(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use self.ensure_one() at the beginning of the method, as it is designed for singleton actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l10n_in prefix on record id that is preferable
<t t-call="web.external_layout"> | ||
<div class="page"> | ||
<h5 id="tax_declaration_name"> | ||
<span t-esc="o.name"/> - Tax Declaration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<span t-esc="o.name"/> - Tax Declaration | |
<span t-out="o.name"/> - Tax Declaration |
t-esc
is deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted.
<?xml version="1.0"?> | ||
<odoo> | ||
<template id="report_tax_declaration"> | ||
<t t-call="web.external_layout"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is purpose of making tds external_layout on report?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to get default company information , logo , header & footer. So that we should not have to set it manually.
<td> | ||
<div id="employee_name"> | ||
<strong class="me-2">Name:</strong> | ||
<span t-field="o.employee_id.name"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not using t-out just to show something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier I believed that field like many2one also should be shown by t-field. But i have cleared my assumption that t-field should be used if field needs any formatting.
</div> | ||
<div id="employee_department"> | ||
<strong class="me-2">Department:</strong> | ||
<span t-field="o.employee_id.department_id.name"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for formatting or using widget in template t-field is good to go with but just display string t-out would be a good option not sure about translation you can have look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the insight, I will take look into that.
bf3b6e3
to
7f46486
Compare
With this commit ================ - Added a Tax Declaration sub-menu under the Contracts menu, allowing users to define new TDS declarations. - Introduced a "Generate Declarations" button in the TDS declaration form, opening a wizard for selecting employees. - Implemented four selection modes for employee filtering: - By Employee - By Department - By Job Position - By Salary Structure - Based on the selected mode, the relevant field (e.g., list of departments for "By Department") is dynamically displayed. - Employees are filtered according to the selected criteria, ensuring that only those with valid contracts are included. - TDS Declaration Form Enhancements: - A stat button now displays the count of generated tax declarations. - Clicking the stat button opens a list view of the generated declarations. - Clicking on a specific declaration opens its form view for detailed review. - TDS Calculation & Deductions: - The Tax Declaration tab in the form now computes TDS based on the New Tax Regime and an employee’s total income. - Added an "Other Deductions" tab to manage additional deductions. - Integration with Employee Form: - A TDS Declaration stat button is now available on the Employee form. - Clicking it opens a list of TDS declarations related to the selected employee.
7f46486
to
5294478
Compare
With this commit
Added a Tax Declaration sub-menu under the Contracts menu, allowing users to define new TDS declarations.
Introduced a "Generate Declarations" button in the TDS declaration form, opening a wizard for selecting employees.
Implemented four selection modes for employee filtering:
Based on the selected mode, the relevant field (e.g., list of departments for "By Department") is dynamically displayed.
Employees are filtered according to the selected criteria, ensuring that only those with valid contracts are included.
TDS Declaration Form Enhancements:
TDS Calculation & Deductions:
Integration with Employee Form: