[ADD][9.0] hr expense operating unit #30
Conversation
AaronHForgeFlow
force-pushed
the
9.0-hr_expense_operating_unit
branch
from
04774eb
to
daf88a1
Compare
| default=lambda self: | ||
| self.env['res.users']. | ||
| operating_unit_default_get(self._uid)) | ||
|
|
There was a problem hiding this comment.
Its better if we add the constraint for company in OU and Expense :
@api.multi
@api.constrains('operating_unit_id', 'company_id')
def _check_company_operating_unit(self):
for rec in self:
if rec.company_id and rec.operating_unit_id and \
rec.company_id != rec.operating_unit_id.company_id:
raise Warning(_('The Company in the Expense and in '
'the Operating Unit must be the same.'))
There was a problem hiding this comment.
Done, Thank you @darshan-serpent
AaronHForgeFlow
force-pushed
the
9.0-hr_expense_operating_unit
branch
from
daf88a1
to
b2af538
Compare
|
@lreficent @mreficent can you review? |
| <field eval="0" name="perm_unlink"/> | ||
| <field eval="0" name="perm_write"/> | ||
| <field eval="1" name="perm_read"/> | ||
| <field eval="0" name="perm_create"/> |
There was a problem hiding this comment.
IMO it must be 1 for unlink, write and create too, otherwise the record rule won't apply for these operation, so a user could changes records by guessing their ids.
There was a problem hiding this comment.
I agree with @sbidoul
| <field eval="0" name="perm_unlink"/> | ||
| <field eval="0" name="perm_write"/> | ||
| <field eval="1" name="perm_read"/> | ||
| <field eval="0" name="perm_create"/> |
There was a problem hiding this comment.
I agree with @sbidoul
| def test_security(self): | ||
| # User 2 is only assigned to Operating Unit B2C, and cannot | ||
| # Access Expenses of Main Operating Unit. | ||
| record = self.hr_expense_model.sudo(self.user2.id).search( |
There was a problem hiding this comment.
It would be good to expand the test with creation or unlink. Following the fix proposed by @sbidoul
|
I didn't expand the rule to create operation because I think you cannot do a harmful action with this operation and also maybe some manager wants to create records for another op. |
|
Not working anymore on this |
This module introduces the following features: