Code review comment for lp:~the-clone-master/ocb-addons/6.1-lp1132963-hr_payroll_missing_AND_on_get_contract

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

The default operator is deceptive in combined expressions (not fully shown in the diff):

Here is the full expression before the fix:

('employee_id', '=', employee.id),
'|',
    '|'
        '&',
            ('date_end', '<=', date_to),('date_end','>=', date_from)
        '&',
            ('date_start', '<=', date_to),('date_start','>=', date_from)
    ('date_start','<=', date_from),
'|',
    ('date_end', '=', False),
    ('date_end','>=', date_to)

The highest level, including the two '|' operators are governed by two implicit '&' operators, if I understand correctly.

After Mario's fix, the expression looks different:

('employee_id', '=', employee.id),
'|',
    '|'
        '&',
            ('date_end', '<=', date_to),('date_end','>=', date_from)
        '&',
            ('date_start', '<=', date_to),('date_start','>=', date_from)
    '&',
        ('date_start','<=', date_from),
        '|',
            ('date_end', '=', False),
            ('date_end','>=', date_to)

Although it makes my brain hurt, this does validate contracts with an end date within the date range, so that it solves the bug.

review: Approve

« Back to merge proposal