OCA / hr-attendance

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

[13.0][IMP] hr_attendance_reason: Show reasons on attendance screen #96

Closed victoralmau closed 1 year ago

victoralmau commented 1 year ago

Show reasons on attendance screen. ejemplo

Changes done:

Please @pedrobaeza and @chienandalu can you review it?

@Tecnativa TT40897

victoralmau commented 1 year ago
  • You are not adding the reason to the kiosk mode, but the regular attendance screen.

Ok, thank you for the clarification.

  • The shown reasons appear randomly. Sometimes "Felt sick", sometimes "Visit customer"...

They are not displayed randomly, as defined, some reasons will be displayed for check_in and different reasons for check_out. (perhaps the existing reasons are somewhat unclear, I agree).

  • No default reason should be selected, and you can't click on check-in if no reason is selected.

Ok, changes done. Now if a reason is not selected, an error is displayed.

victoralmau commented 1 year ago
  • Use some odoo css clases to format the drop-down so it matches with the UI

Changes done.

  • Only let the user choose reasons if the time of checking out/checking in is out of the employee schedule. If the employee it's checking in/out at the right time I suggest not to show the drop-down because it could be confusing.

While perhaps a good suggestion, I think this is beyond the scope of this enhancement; moreover, it would be necessary to add a screen refresh every x minutes to "know" whether or not to display the reasons.

In addition, in some cases you may want to allow the reason for check out (e.g. for the coffee break).

  • Do not provide a default value, it can cause bad inputs.

Changes done.

victoralmau commented 1 year ago

Remove the kiosk word from everywhere, as it's not being applied to kiosk mode. And I think it should work also on kiosk mode. I would call it "Show reasons on attendance screen".

Removed the word kiosk completely and added functionality to kiosk mode.

victoralmau commented 1 year ago

Thanks David, now everything works correctly.

chienandalu commented 1 year ago

@nimarosa Can you update your review?

pedrobaeza commented 1 year ago

Got this error checking-in:

Odoo Server Error

Traceback (most recent call last):
  File "/opt/odoo/odoo/http.py", line 624, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/opt/odoo/odoo/http.py", line 310, in _handle_exception
    raise pycompat.reraise(type(exception), exception, sys.exc_info()[2])
  File "/opt/odoo/odoo/tools/pycompat.py", line 14, in reraise
    raise value
  File "/opt/odoo/odoo/http.py", line 669, in dispatch
    result = self._call_function(**self.params)
  File "/opt/odoo/odoo/http.py", line 350, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/opt/odoo/odoo/service/model.py", line 94, in wrapper
    return f(dbname, *args, **kwargs)
  File "/opt/odoo/odoo/http.py", line 339, in checked_call
    result = self.endpoint(*a, **kw)
  File "/opt/odoo/odoo/http.py", line 915, in __call__
    return self.method(*args, **kw)
  File "/opt/odoo/odoo/http.py", line 515, in response_wrap
    response = f(*args, **kw)
  File "/opt/odoo/addons/web/controllers/main.py", line 1342, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/addons/web/controllers/main.py", line 1334, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 390, in call_kw
    result = _call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 377, in _call_kw_multi
    result = method(recs, *args, **kwargs)
TypeError: attendance_manual() takes from 2 to 3 positional arguments but 4 were given
victoralmau commented 1 year ago

Rebase done and fixed now (tested in runboat)

victoralmau commented 1 year ago

Any other functional review to merge it?

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). 🤖

victoralmau commented 1 year ago

Please promote Tecnativa as co-author, due to the big changes that modify totally the module.

Added.

pedrobaeza commented 1 year ago

/ocabot merge major

OCA-git-bot commented 1 year ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 13.0-ocabot-merge-pr-96-by-pedrobaeza-bump-major, awaiting test results.

OCA-git-bot commented 1 year ago

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