Add l10n_ch_hr_payroll. #66

open-net-sarl · 2014-11-26T13:30:23Z

Swizerland Payroll Rules for Odoo.

Features list :

Add Swiss salary rule categories
Add Swiss salary rules
Add children in school to employee
Add LPP range to contract

eLBati · 2014-11-26T13:43:29Z

l10n_ch_hr_payroll/__init__.py

+##############################################################################
+
+import hr_contract
+import hr_employee


please

from . import hr_contract from . import hr_employee

eLBati · 2014-11-26T13:44:52Z

Thanks @open-net-sarl
See travis for some remarks:

./l10n_ch_hr_payroll/hr_contract.py:31:1: F401 '_' imported but unused
./l10n_ch_hr_payroll/hr_contract.py:34:1: E302 expected 2 blank lines, found 1
./l10n_ch_hr_payroll/hr_contract.py:37:80: E501 line too long (94 > 79 characters)
./l10n_ch_hr_payroll/hr_employee.py:31:1: F401 '_' imported but unused
./l10n_ch_hr_payroll/__openerp__.py:33:36: E231 missing whitespace after ','

As pointed out by @nbessi in mailing list, do you plan to add any automated tests?

Add l10n_ch_hr_payroll to readme.

yvaucher · 2014-11-26T15:20:38Z

l10n_ch_hr_payroll/hr_contract.py

+        'lpp_rate': fields.float('LPP Rate', digits_compute=dp.get_precision('Payroll Rate')),
+        }
+
+hr_contract()


This line can be removed. No need to call model() anymore.

yvaucher · 2014-11-26T15:41:15Z

Hello @open-net-sarl

Welcome on OCA.
Thanks you for your contribution!

I added some inline comments to comply with our coding conventions, we tend to be rigorous as it ease us the work to maintain now nearly 500k of line of code of the whole OCA community base code.

… into 8.0 For add the change in main readme.

open-net-sarl · 2014-11-27T14:24:33Z

yvaucher, eLBati, I apply your requests.

yvaucher · 2014-11-27T14:50:57Z

@open-net-sarl still 2 PEP8 errors pointed out by travis: https://travis-ci.org/OCA/l10n-switzerland/jobs/42311237

And a little tip about git, to update your branch with upstream, you can use:

git fetch origin
git rebase -i origin/8.0

Instead of doing merge. Then you can push here by doing a git push -f

nbessi · 2014-12-03T08:46:23Z

l10n_ch_hr_payroll/__openerp__.py

+    * Add LPP range to contract
+
+**For functionnal information:**
+http://ur1.ca/ir5ou


It is ok with OCA @jgrandguillaume ? also link in page does not point to swiss localization

I think basically all information should be in README. External links could be used for reference.
At least it should warn this link is french only ?

Thought I am concerned about having all module README linking to external pages. The user who search for modules will surely want more information when reading README and not being redirected to any other website.

Plus it would be tempting for every one to put a link to make some advertisement this could create a link war nightmare in each modules.

Nevertheless, I think it's ok to have additional link if the README itself provides enough info.
More opinions on this from community would be welcome.

nbessi · 2014-12-03T08:49:30Z

Thanks for the works there is still flake8 issues, and such a module certainly deserve some test yaml or unittest.

Regards

nbessi · 2014-12-03T08:51:29Z

@fclementic2c @lucmaurer I would greatly appreciate a functional review from your side.

yvaucher · 2014-12-03T10:00:52Z

l10n_ch_hr_payroll/README.rst

+    * Add LPP range to contract
+
+**For functionnal information:**
+http://ur1.ca/ir5ou


I'm concerned about this link, this is french only documentation.
The README.rst deserve at least a better english explaination for our neightbour beyond the Röstigraben.

open-net-sarl · 2014-12-08T10:40:07Z

@yvaucher: The 2 PEP8 are fixed.

About your command tofetch and rebase, by 'origin' you mean OCA repo?

coveralls · 2014-12-08T10:42:18Z

Coverage remained the same when pulling f4afeae on open-net-sarl:8.0 into 1aa4ad9 on OCA:8.0.

yvaucher · 2014-12-08T11:10:34Z

@open-net-sarl yes rebase must be done on master 8.0 branch. It was a guess that your origin was OCA repo. It would also work on top of you own repo if you synced it with OCA repo.

coveralls · 2014-12-08T11:39:18Z

Coverage remained the same when pulling 4f25f8b on open-net-sarl:8.0 into 1aa4ad9 on OCA:8.0.

stevech091 · 2014-12-10T11:09:41Z

I have seen your work about payroll and I think is a good platform to start improvement.
I have some question: first of all, In Ticino we have a lot of foreign employees and we need to manage " impôt à la source". In your implementation I haven't seen it.

About "LPP " why you use rate and not a fixed amount? In my experience the most of my customer used this option.

In the next weeks/months I want to test and give you a feedback.

open-net-sarl · 2014-12-10T12:05:41Z

Hello,

Thanks for your input.

We choose to have 'LPP' as a rate because we want to be able to adjust to variable salary amount.

For the 'Impôt à la source', we are thinking to add another rate, next to the 'LPP' rate and a rule.
The rule will be active if the rate is not null, and a 'not null' rate on this will in turn deactivate some of the other rules

Would it fit your experience ?

Cordiales salutations

Eberhard J-A
CEO & Founder
Certified Odoo Functional Specialist

Open Net Sàrl
www.open-net.ch
59, Rue de l'industrie
1030 Bussigny
T: +41 (21) 701 42 45
Autoroute A1, Sortie "Crissier / Bussigny"
Bus TL 17, arrêt Croix-de-Plan

----- Mail original -----

De: "stevech091" [email protected]
À: "OCA/l10n-switzerland" [email protected]
Cc: "Open Net Ltd" [email protected]
Envoyé: Mercredi 10 Décembre 2014 12:09:43
Objet: Re: [l10n-switzerland] Add l10n_ch_hr_payroll. (#66)

I have seen your work about payroll and I think is a good platform to
start improvement.
I have some question: first of all, In Ticino we have a lot of
foreign employees and we need to manage " impôt à la source". In
your implementation I haven't seen it.
About "LPP " why you use rate and not a fixed amount? In my
experience the most of my customer used this option.
In the next weeks/months I want to test and give you a feedback.
—
Reply to this email directly or view it on GitHub .

stevech091 · 2014-12-10T13:35:22Z

About "LPP" I think that is convenient have rate and fix amount. Normally the amount of salary to calculate the lpp amount is fix the start of the current year (in my experience).
For 'Impôt à la source' the rate how is it calculated?

nbessi · 2014-12-16T14:20:47Z

Hello,
Thanks for the work and the fixes.

Actual code looks good to me, but there is no tests available in the module. That is generally a no go for commiting addons on serie 8.0 in OCA projects.

Can you please add some YAML or unit tests on the module. Until there it will probably be a ✋ to commit.

Regards

Nicolas

yvaucher · 2014-12-16T14:37:19Z

@nbessi I agree tests are almost every time needed but think for tests it's ok there:
Coverall says: "Coverage remained the same when pulling"

I see no python logic to test. Just few field are added. It could be useful to understand how it works but if enough documentation is provided it will be ok.

For me it's a 👍 for code. Still waiting on someone feed back on functional testing and an answer on #66 (comment).

nbessi · 2014-12-16T14:54:40Z

@yvaucher I acknowledge your point, but we may import faulty data that may cause wrong output.
I'm not a payroll expert but it is certainly a critical topic with legal consequences.

We are not importing bank or zip definitions here. The data set is more complex and sensitive.
I will be more confortable having a payroll managing system correctly tested. It may also detect nasty regression that can appear even on base addons.

Regards

Nicolas

open-net-sarl · 2014-12-17T09:19:27Z

Hello,

We add extra fields and pre formatted salary rules, so I don't see the uses of a YAML test

The salary rules has to be put together in a salary structure by the user, not by the program.

nbessi · 2014-12-17T09:34:00Z

@open-net-sarl, it is OK for me. 👍

As soon we have a functional review it will be merged.

Regards

Nicolas

jgrandguillaume · 2014-12-17T11:55:32Z

Hi,

Thanks for the contribs ! We do have tested it and it works. I give my 👍

Regards,

Joël

Add l10n_ch_hr_payroll.

stevech091 · 2014-12-19T15:58:07Z

Hello @open-net-sarl after my short test you have seen that you have configured AVS company contributor double of employ but I think is a mistake. Normally the company contributor are the same of employ (all my customer HR software have this configuration).

In your http://ur1.ca/ir5ou you talk about "Impôt à la source" in "montant fixè". Why? I think in better use some formula and table. In Ticino "Impôt à la source" are on years base (not montly) so is very hard for end user calculate every month the new "montant".
What do you think?

Work for other test.

Regards,

Stefano

nbessi · 2014-12-19T16:20:57Z

Hello

@stevech091 your question was reported in issue #74.
@stevech091 @open-net-sarl, please do not use this thread anymore.

Regards

Nicolas

Add l10n_ch_hr_payroll.

fix script logging

Add some minor changes, based on pull request CompassionCH#66 comments. Some changes in copyright Fix 2 pep8 errors. Fix error in big comment header. Remove description from l10n_ch_hr_payroll/__openerp__.py file. Add license key in __openerp__.py Add OCA as author of OCA addons In order to get visibility on https://www.odoo.com/apps the OCA board has decided to add the OCA as author of all the addons maintained as part of the association. Restarting 'l10n_ch_payroll' from a cleaner state Corrected: removed trailing blank line at the end of 'models/hr_contract.py' Restaured: the content of __openerp__.py, lots during the last git rebase. Corrected: the contributors lists and the link to the corresponding website page explaining the details. OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex [UPD] prefix versions with 8.0 [MIG] Make modules uninstallable Module ported to v8 Modify AM_C category Change module name in __init__.py Fix README wrong module name Short header on __init__.py, hr_contract.py, hr_employee.py OCA Transbot updated translations from Transifex Add two more rules for family allocation - ALFA_E_R - ALFA_F_R Add missing view to __openerp__.py Fix version format Fix author name OCA Transbot updated translations from Transifex

Add some minor changes, based on pull request #66 comments. Some changes in copyright Fix 2 pep8 errors. Fix error in big comment header. Remove description from l10n_ch_hr_payroll/__openerp__.py file. Add license key in __openerp__.py Add OCA as author of OCA addons In order to get visibility on https://www.odoo.com/apps the OCA board has decided to add the OCA as author of all the addons maintained as part of the association. Restarting 'l10n_ch_payroll' from a cleaner state Corrected: removed trailing blank line at the end of 'models/hr_contract.py' Restaured: the content of __openerp__.py, lots during the last git rebase. Corrected: the contributors lists and the link to the corresponding website page explaining the details. OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex [UPD] prefix versions with 8.0 [MIG] Make modules uninstallable Module ported to v8 Modify AM_C category Change module name in __init__.py Fix README wrong module name Short header on __init__.py, hr_contract.py, hr_employee.py OCA Transbot updated translations from Transifex Add two more rules for family allocation - ALFA_E_R - ALFA_F_R Add missing view to __openerp__.py Fix version format Fix author name OCA Transbot updated translations from Transifex

…oice BIZ-1508: Fix: Show top comments on account invoice

Add some minor changes, based on pull request OCA#66 comments. Some changes in copyright Fix 2 pep8 errors. Fix error in big comment header. Remove description from l10n_ch_hr_payroll/__openerp__.py file. Add license key in __openerp__.py Add OCA as author of OCA addons In order to get visibility on https://www.odoo.com/apps the OCA board has decided to add the OCA as author of all the addons maintained as part of the association. Restarting 'l10n_ch_payroll' from a cleaner state Corrected: removed trailing blank line at the end of 'models/hr_contract.py' Restaured: the content of __openerp__.py, lots during the last git rebase. Corrected: the contributors lists and the link to the corresponding website page explaining the details. OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex [UPD] prefix versions with 8.0 [MIG] Make modules uninstallable Module ported to v8 Modify AM_C category Change module name in __init__.py Fix README wrong module name Short header on __init__.py, hr_contract.py, hr_employee.py OCA Transbot updated translations from Transifex Add two more rules for family allocation - ALFA_E_R - ALFA_F_R Add missing view to __openerp__.py Fix version format Fix author name OCA Transbot updated translations from Transifex

Add l10n_ch_hr_payroll.

ae837e7

eLBati reviewed Nov 26, 2014
View reviewed changes

Update README.md

7aacba9

Add l10n_ch_hr_payroll to readme.

yvaucher reviewed Nov 26, 2014
View reviewed changes

Sébastien Gendre added 3 commits November 27, 2014 14:21

Add some minor changes, based on pull request OCA#66 comments.

d37148b

Some changes in copyright

37aaccc

Merge branch '8.0' of https://github.com/open-net-sarl/l10n-switzerland…

34ec7ee

… into 8.0 For add the change in main readme.

nbessi added the needs fixing label Dec 3, 2014

nbessi reviewed Dec 3, 2014
View reviewed changes

yvaucher reviewed Dec 3, 2014
View reviewed changes

Fix 2 pep8 errors.

f4afeae

Fix error in big comment header.

4bad798

Remove description from l10n_ch_hr_payroll/__openerp__.py file.

4f25f8b

nbessi added needs review needs functional review and removed needs fixing needs review labels Dec 17, 2014

yvaucher added the 8.0 label Dec 17, 2014

jgrandguillaume added a commit that referenced this pull request Dec 17, 2014

Merge pull request #66 from open-net-sarl/8.0

2af9826

Add l10n_ch_hr_payroll.

jgrandguillaume merged commit 2af9826 into OCA:8.0 Dec 17, 2014

yvaucher pushed a commit to yvaucher/l10n-switzerland that referenced this pull request Nov 14, 2017

Merge pull request OCA#66 from open-net-sarl/8.0

56ca1f3

Add l10n_ch_hr_payroll.

vrenaville pushed a commit to camptocamp/l10n-switzerland that referenced this pull request Jun 22, 2018

Merge pull request OCA#66 from leemannd/fix_script_10_12_1

b68d0a1

fix script logging

lucode pushed a commit to coobyHQ/l10n-switzerland that referenced this pull request Sep 19, 2018

Add some minor changes, based on pull request OCA#66 comments.

96967bd

quentingigon referenced this pull request in quentingigon/l10n-switzerland Mar 16, 2020

Add some minor changes, based on pull request CompassionCH#66 comments.

5f28d7b

JuMiSanAr pushed a commit to camptocamp/l10n-switzerland that referenced this pull request Sep 21, 2021

Merge pull request OCA#66 from Tonow-c2c/BIZ-1508_show_comment_in_inv…

cb7ee2c

…oice BIZ-1508: Fix: Show top comments on account invoice

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add l10n_ch_hr_payroll. #66

Add l10n_ch_hr_payroll. #66

open-net-sarl commented Nov 26, 2014

eLBati Nov 26, 2014

eLBati commented Nov 26, 2014

yvaucher Nov 26, 2014

yvaucher commented Nov 26, 2014

open-net-sarl commented Nov 27, 2014

yvaucher commented Nov 27, 2014

nbessi Dec 3, 2014

yvaucher Dec 3, 2014

nbessi commented Dec 3, 2014

nbessi commented Dec 3, 2014

yvaucher Dec 3, 2014

open-net-sarl commented Dec 8, 2014

coveralls commented Dec 8, 2014

yvaucher commented Dec 8, 2014

coveralls commented Dec 8, 2014

stevech091 commented Dec 10, 2014

open-net-sarl commented Dec 10, 2014

stevech091 commented Dec 10, 2014

nbessi commented Dec 16, 2014

yvaucher commented Dec 16, 2014

nbessi commented Dec 16, 2014

open-net-sarl commented Dec 17, 2014

nbessi commented Dec 17, 2014

jgrandguillaume commented Dec 17, 2014

stevech091 commented Dec 19, 2014

nbessi commented Dec 19, 2014

Add l10n_ch_hr_payroll. #66

Add l10n_ch_hr_payroll. #66

Conversation

open-net-sarl commented Nov 26, 2014

eLBati Nov 26, 2014

Choose a reason for hiding this comment

eLBati commented Nov 26, 2014

yvaucher Nov 26, 2014

Choose a reason for hiding this comment

yvaucher commented Nov 26, 2014

open-net-sarl commented Nov 27, 2014

yvaucher commented Nov 27, 2014

nbessi Dec 3, 2014

Choose a reason for hiding this comment

yvaucher Dec 3, 2014

Choose a reason for hiding this comment

nbessi commented Dec 3, 2014

nbessi commented Dec 3, 2014

yvaucher Dec 3, 2014

Choose a reason for hiding this comment

open-net-sarl commented Dec 8, 2014

coveralls commented Dec 8, 2014

yvaucher commented Dec 8, 2014

coveralls commented Dec 8, 2014

stevech091 commented Dec 10, 2014

open-net-sarl commented Dec 10, 2014

stevech091 commented Dec 10, 2014

nbessi commented Dec 16, 2014

yvaucher commented Dec 16, 2014

nbessi commented Dec 16, 2014

open-net-sarl commented Dec 17, 2014

nbessi commented Dec 17, 2014

jgrandguillaume commented Dec 17, 2014

stevech091 commented Dec 19, 2014

nbessi commented Dec 19, 2014