OCA / hr-expense

Human Resources Expenses OCA modules for Odoo
GNU Affero General Public License v3.0
50 stars 103 forks source link

[16.0][FIX+IMP] hr_expense_invoice: Proper accounting + UX #234

Closed pedrobaeza closed 3 months ago

pedrobaeza commented 4 months ago

@Tecnativa TT49143

pedrobaeza commented 4 months ago

@LoisRForgeFlow can you advice about the error on hr_expense_tier_validation ?

Traceback (most recent call last):
  File "/__w/hr-expense/hr-expense/hr_expense_tier_validation/tests/test_hr_expense_tier_validation.py", line 84, in test_edit_value_expense
    s.name = "New name"
  File "/opt/odoo/odoo/tests/common.py", line 2278, in __setattr__
    assert not self._get_modifier(field, 'readonly'), \
AssertionError: can't write on readonly field name
pedrobaeza commented 4 months ago

Checked that the failure is also in main branch: https://github.com/OCA/hr-expense/pull/235

pedrobaeza commented 4 months ago

Added the management of payment state and amount residual for expenses paid by employee linked to an invoice with the new transfer entry.

pedrobaeza commented 4 months ago

Rebased after the fix in the other module and everything green.

LoisRForgeFlow commented 3 months ago

@LoisRForgeFlow can you advice about the error on hr_expense_tier_validation ?

Traceback (most recent call last):
  File "/__w/hr-expense/hr-expense/hr_expense_tier_validation/tests/test_hr_expense_tier_validation.py", line 84, in test_edit_value_expense
    s.name = "New name"
  File "/opt/odoo/odoo/tests/common.py", line 2278, in __setattr__
    assert not self._get_modifier(field, 'readonly'), \
AssertionError: can't write on readonly field name

Hi @pedrobaeza

It seems something specific of the expense tier validation integration, maybe Ecosoft as the authors have a better insight?

pedrobaeza commented 3 months ago

Yes, thanks, it has already been fixed in #236. Sorry for not updating here (I just rebased).

LoisRForgeFlow commented 3 months ago

Yes, thanks, it has already been fixed in #236. Sorry for not updating here (I just rebased).

No worries. That was quick :rocket:

Thanks

pedrobaeza commented 3 months ago

/ocabot merge major

OCA-git-bot commented 3 months ago

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

OCA-git-bot commented 3 months ago

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

SoniaViciana commented 3 months ago

It's not OK this merge. Now, if this case occurs:

image

The accounting entries are wrong:

image

Before this merge, It was against a payable account (410), that would be the right thing to avoid having duplicate expenses.

Also, it's wrong in this other case:

image

If I "Cancel all related operations", the payment made to the invoice will not be canceled. And i have another button ("Register Payment") that right now it's of no use.

Can you review it please? @pedrobaeza Thanks in advance,

pedrobaeza commented 3 months ago

Please open an issue, but that was not the behavior got, as you can check in the tests. Please do the tests on a runboat and report back in an issue.

gonzalolenzi commented 1 month ago

Hi @pedrobaeza , we're having some problems using this module after this specific change.

I'm attaching a video (00:59 seconds) from runboat without any change in the configuration just so you understand the flow I'm trying to follow and the problem I'm having

https://drive.google.com/file/d/1celILhfky3xYFZw1WsYM3f0isrMIm1iu/view

The flow we were doing before this change was:

  1. Create the expense
  2. Create the report
  3. Create the vendor bill and validate it.
  4. Post journal entries and generate employee debt
  5. Register payment

I think with the change, the flow should be the same because we are generating the AP through the entry in the general journal, but for some reason I can not register the payment. I get "You can't register a payment because there is nothing left to pay on the selected journal items." like there is no debt related.

Can you help me?

Thanks in advance.

pedrobaeza commented 1 month ago

@gonzalolenzi please check #249. There's still a couple of things left that I have to complete though.