Review: Needs Fixing

Thanks for the MP.

Some high level remarks:

The HR Manager group has full access to all HR modelss and records.
The HR Officer is the one with a record rule limiting access based on 
Departments.
Don't you mean to extend HR Officer role instead?
And if the issue is that HR Officer's record rule does not work for child 
records, wouldn't be better fixing it?


Some specific comments:

l.77: you should run the description text against a spell checker - there are a 
few typos to be fixed.

l.88: dependency on 'mail' is unnecessary, and dependency on 'base' is 
redundant.

l.95: why do you set 'active' to False?

l.193, hr_payroll_manager_group.py: it seems to me that this file isn't needed. 
Can it be removed?

l.264: group_hr_payroll extends group_hr_manager, and the later has already 
full access on HR models. Is this ACL necessary?

l.279: this __init__.py file has no statements. Can't it be removed?

There are several empty directories, could they be removed?


-- 
https://code.launchpad.net/~vauxoo/openerp-hr/7.0-dev-hr_payroll_manager_groups/+merge/190460
Your team OpenERP Community is subscribed to branch 
lp:~openerp-community/openerp-hr/7.0-modules-from-addons-hr-ng.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

Reply via email to