Code review comment for lp:~camptocamp/hr-timesheet/7.0-hr_timesheet_reminder-migr

line 263: if status in ['Missing', 'Draft'] and employee.receive_timesheet_alerts
The second part of the test should be moved out to the outer loop (just after line 253) or even better to the search domain (line 243) : that way we don't make further queries for employees who don't recevie timesheet alerts (or am I missing something?)

653 + def _cron_nextcall():
654 + now = datetime.today() + timedelta(days=1)
655 + return time.strftime(
656 + DEFAULT_SERVER_DATETIME_FORMAT,
657 + now.timetuple())

1 -> I'd rename 'now' to 'tomorrow' or 'when'
2. why not use now.strftime(DEFAULT_SERVER_DATETIME_FORMAT) (or tomorrow.strftime...) instead of time.strftime(...) ?

There are probably a number of 'import time' which can be removed (pylint will tell you about unused imports)

607 + 'body_html': '<pre>%s</pre>' % message_data.message,

I expect mail clients to be fairly tolerant in what they accept, but this makes me uncomfortable.

Have you tested what happens if the user has typed in:

* html (e.g. "fill your timesheet <b>now!</b>")
* a raw text message with HTML special chars inside (e.g: "I'm very disapointed you did not fill your timesheet <:-( I'll make sure your manager is notified & takes proper action.

Best chance would be to allow the edition of the body of the message with a rich text editor. Is there such widget available in the oerp7 client ?

803 -# Author: Arnaud WÃŒst

-> Mojibake! (but you fixed it)

1638 + 'date': time.strftime('%Y-%m-%d'),
Is there not a constant defining this format?