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.

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

[ADD] Module hr_employee_exemption #83

Merged
merged 6 commits into from
May 14, 2015

Conversation

ghost
Copy link

@ghost ghost commented Mar 15, 2015

This module adds exemptions from income tax. This is required in some complexe structure where the employee is subject to several types of income tax and can be exempted from one or another. This module adds the required function to verify in a salary rule whether an employee is exempted or not.

@ghost ghost mentioned this pull request Mar 15, 2015
@ghost ghost force-pushed the 7.0-ddufresne-employee_exemption branch from fa37775 to 7c3db47 Compare March 20, 2015 14:24
@max3903 max3903 added this to the 7.0 milestone Apr 3, 2015
@max3903
Copy link
Sponsor Member

max3903 commented Apr 6, 2015

👍

@max3903 max3903 mentioned this pull request Apr 6, 2015
7 tasks
@ghost
Copy link
Author

ghost commented Apr 6, 2015

@max3903 I don't know why runbot fails here, do you have any idea ?

@max3903
Copy link
Sponsor Member

max3903 commented Apr 6, 2015

@ghost
Copy link
Author

ghost commented Apr 6, 2015

@gurneyalex It seems that runbot shuts down while installing the account module and I don't have a clue why it does. All warnings in the log don't seem to be related. Can you help me.

@ghost ghost force-pushed the 7.0-ddufresne-employee_exemption branch 2 times, most recently from 3c2067c to ba3e4da Compare April 24, 2015 10:56
@ghost
Copy link
Author

ghost commented May 13, 2015

@feketemihai @bwrsandman @dreispt @pedrobaeza I would also appreciate reviews here as well as in #82

The method to call from a salary rule to check whether an employee
is exempted from a source deduction
"""
employee = self.browse(cr, uid, ids[0], context=context)

Choose a reason for hiding this comment

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

Make sure ids is a list of one

@bwrsandman
Copy link

👍

'description': """
Employee Exemption
==================
Add employee benefits in order to compute payslips.
Copy link
Member

Choose a reason for hiding this comment

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

Put here also complete description (although it's duplicated)

Copy link
Member

Choose a reason for hiding this comment

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

Is it the description from the hr_employee_benefits...like @pedrobaeza said, put the first paragraph at least from readme...

@pedrobaeza
Copy link
Member

The icon is not significant enough. I would put OCA's generic one: https://github.com/OCA/maintainer-tools/blob/master/template/module/static/description/icon.png

required=True,
ondelete='cascade',
),
'exemption_id': fields.many2one(
Copy link
Member

Choose a reason for hiding this comment

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

Since it is quite directly linked with the payslip, and from my point of view it affects a salary rule (i don't know how you structure the salaries, but for me, in salary rule you have gross, benefits, deductions, employee taxes, employer taxes, and net), isn't it easier to directly link the exemption_id with a salary rule, and on compute payslip the amount to be 0...what do you think??

@pedrobaeza @bwrsandman @max3903

I notice that on benefits module is making a link between benefit, exemtion and salary rule, but i think it should be easier to split it and add functionalities per module, if this is a exemption, it should work standalone, the same benefits, it has many dependancies, so probably is it ok to split it in many modules...

Copy link
Member

Choose a reason for hiding this comment

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

I agree that it can be more logical and you avoid the formula in your salary rule.

Copy link
Author

Choose a reason for hiding this comment

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

@feketemihai, @pedrobaeza I agree about linking the exemption to the salary rule. For the employee benefit module, what about that split:
hr_employee_benefit (depends on hr_payroll)

hr_employee_benefit_on_job (depends on hr_employee_benefit, hr_contract_multi_jobs and hr_worked_days_activity)
add employee benefits based on each worked hours of the employee

hr_employee_benefit_exemption (depends on hr_employee_benefit, hr_employee_exemption)
add exemptions on benefits

hr_employee_benefit_percent_gross (depends on hr_employee_benefit)
add benefits based on percent of the gross slary

What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

@pedrobaeza @dufresnedavid I agree with the split.

Copy link
Member

Choose a reason for hiding this comment

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

Also agree

@ghost
Copy link
Author

ghost commented May 14, 2015

@feketemihai @pedrobaeza Now, the rules are linked to exemption. The exemptions will apply automatically on selected rules.

@pedrobaeza
Copy link
Member

Seems good, thanks

👍

@bwrsandman
Copy link

@dufresnedavid lxml problem should be solved after the next commit or rebase.

@ghost ghost force-pushed the 7.0-ddufresne-employee_exemption branch from d9d06d8 to 72e1749 Compare May 14, 2015 15:01
bwrsandman added a commit that referenced this pull request May 14, 2015
…xemption

[ADD] Module hr_employee_exemption
@bwrsandman bwrsandman merged commit 9c0d854 into OCA:7.0 May 14, 2015
@max3903 max3903 deleted the 7.0-ddufresne-employee_exemption branch May 21, 2015 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants