openerp-community team mailing list archive

[Daniel Reis <dgreis@xxxxxxx>]

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.