OCA / hr-attendance

HR Attendance OCA modules for Odoo
GNU Affero General Public License v3.0
50 stars 119 forks source link

[ADD] hr_attendance_missing_days #140

Closed fkantelberg closed 6 months ago

fkantelberg commented 1 year ago

The overtime calculation of Odoo 15.0 ignores working days without attendances. To compensate this behaviour this modules implements a cron job which generates 0min attendances for working days without an attendance.

tv-openbig commented 1 year ago

Hello @fkantelberg i could not create the attendance entries with 0.00 hours on working days with no attendance created for the employee on regular working days. i have the following log on my server. It seems the con-job is running: 2023-08-13 18:05:55,099 5749 INFO verdigado_initos odoo.addons.base.models.ir_cron: Starting job Missing Attendance. 2023-08-13 18:05:55,100 5749 INFO verdigado_initos odoo.addons.base.models.ir_cron: Job Missing Attendance done.

However if i check the attendance entries of my employee i only can see the entries created by hand. Bildschirmfoto 2023-08-13 um 20 12 28

Normally i would expect also entries with 0.00 at least for the following days: 06.04.2023 (holiday) 10.04.2023 (public holiday) and further more: 11.04.2023 ... ... 14.04.2023

Do you have any advise ?

tv-openbig commented 1 year ago

@fkantelberg Ok, i have forgotten to set the autoclose reason in the settings. Now i can see the automatic entries.

Bildschirmfoto 2023-08-13 um 20 51 00

hbrunn commented 1 year ago

oh, and I think we also should delete 0 attendances when an employee records some time after the fact, right?

fkantelberg commented 1 year ago

oh, and I think we also should delete 0 attendances when an employee records some time after the fact, right?

I don't know if we should do it automatically. With the latest patch Odoo only generates those if the working day is over. This means the employee creates the attendance on a different date. The missing attendances aren't hitting the computation of the worked hours or overtime but can also signal that it missed (e.g. for the HR officer). It would make the code more and more complex and would require an additional flag because the reason might not be unique because it's configurable. I would favor to keep those for now but if you need it I can make the attendance creation hookable if you like.

If the employee is late on the working day it can get strange. I tried to handle it with the latest commit but it really gets weird around the edges independent of the timezone. E.g. if an employee has a working shift around midnight but comes late (=post midnight). Technically speaking the attendance is on the next day BUT the original shift starts before and a 0 attendance would be created because we are only checking against the starting times.

albig commented 1 year ago

The cron fails in my case, if an employee has a valid attendance record NOW()-31 days. In my example the attendance is from 2023-07-14 12:35 to 2023-07-14 17:15. I executed the cron on 2023-08-14 14:36. The message is missleading and wrong.

2023-08-14_14-45 2023-08-14_14-44

If I change the relevant record 2023-07-14 12:35 to 2023-07-14 13:15, the cron run's as expected.

tv-openbig commented 1 year ago

@fkantelberg @hbrunn Hello, i can confirm the overtime caIculation is correct now. I have tested the following scenario, which should cover all potential working hour scenarios:

Bildschirmfoto 2023-08-22 um 10 23 21

I have modified this chart a little bit: 02:00 hours work on holiday: 2023-04-05 00:45 working hours on weekend

The overtime calculation is correct for several cases:

I assume this PR is ready to be merged.

OCA-git-bot commented 1 year ago

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

fkantelberg commented 1 year ago

FYI We discovered a problem in combination with contracts. Currently the cron will also generate attendances for employees who have no contract causing the high negative overtimes. I'll update the PR in the next days with a solution.

fkantelberg commented 1 year ago

I pushed a 2nd module to the PR to handle contracts. If you have contract installed the cron will only generate for employee if there is a open/closed contract during the time frame. If you have an employee (like Administrator) without a contract the cron won't create any missing attendances as a side effect.

Highcooley commented 10 months ago

I wanted to do a functional review, but the attendance app does not load (white screen) on runboat without displaying any error message. Other than that, it would be great if we could merge it soon, so we can port it to 16 and 17.

hbrunn commented 10 months ago

@Highcooley there seems to be an issue with hr_attendance_geolocation on runboat, so I just uninstalled this module there (you'll have to redo this if this runboat instance dies until you look at this).

That said, it's not a prerequisite for this PR to be merged to migrate it to other versions. Actually, having this migrated to other versions will show @OCA/human-resources-maintainers that there's interest for this functionality from several parties.

github-actions[bot] commented 6 months ago

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

ortlam commented 6 months ago

Is there a way to restart the CI? It seems to be stuck. Apart from that, anything missing that needs to be done for this PR to be merge-ready?

ortlam commented 6 months ago

@pedrobaeza Do you know how to reset the CI?

pedrobaeza commented 6 months ago

A forced push by @fkantelberg should work

ortlam commented 6 months ago

A forced push by @fkantelberg should work

Thank you, I updated the branch and fixed the unit test issue that came up. I think this PR should be ready to merge.

OSevangelist commented 6 months ago

/ocabot merge nobump

OCA-git-bot commented 6 months ago

On my way to merge this fine PR! Prepared branch 15.0-ocabot-merge-pr-140-by-OSevangelist-bump-nobump, awaiting test results.

OCA-git-bot commented 6 months ago

Congratulations, your PR was merged at 93bc5193e57032f30c83c63fe5b51889f490b727. Thanks a lot for contributing to OCA. ❤️