[17.0][MIG] hr_expense_cancel #233

imlopes · 2024-05-15T13:50:02Z

Depends on #229

Some edits for this module :

the method on hr.expense.sheet is named action_approve_expense_sheets and no more approve_expense_sheets
on hr.expense.sheet the field account_move_id was changed to account_move_ids

vvrossem

LGTM, thank you.
Pre-approving regarding Red Travis

yankinmax · 2024-05-21T07:59:42Z

hr_expense_cancel/models/hr_expense.py

            payments = sheet.payment_ids.filtered(lambda line: line.state != "cancel")
            # case : cancel invoice from hr_expense
            self._remove_reconcile_hr_invoice(account_move)
            # If the sheet is paid then remove payments
-            if sheet.state == "done":
+            if sheet.state in ("done", "approve"):
                if sheet.expense_line_ids[:1].payment_mode == "own_account":
                    self._remove_move_reconcile(payments, account_move)


You're changing account_move_id single record to multiple account_move_ids.
Have you checked for example self._remove_move_reconcile(payments, account_move) can take as an argument multiple moves?

Hello @yankinmax
I'm not changing the field account_move_id.
It was changed on odoo source when migrating to 17.0

Here if I'm not wrong : OCA/OCB@68fbdc9#diff-42c75e4d29952aadf0abdd1bfe7e066bef3db1275c16d64dd3f3b96a332a2ff3

This module lets to cancel and correct expenses. It adds a cancel button on the expense sheet that undo reconciliations and delete payments and journal entries.

Since odoo/odoo@64b2580 TT33774

The multi own account test has been removed as it is no longer possible in odoo core to register the payment of several expense sheet reports at the same time. Co-Authored By: Stefan Ungureanu aiendry-aktivsoftware

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: hr-expense-16.0/hr-expense-16.0-hr_expense_cancel Translate-URL: https://translation.odoo-community.org/projects/hr-expense-16-0/hr-expense-16-0-hr_expense_cancel/

Currently translated at 100.0% (3 of 3 strings) Translation: hr-expense-16.0/hr-expense-16.0-hr_expense_cancel Translate-URL: https://translation.odoo-community.org/projects/hr-expense-16-0/hr-expense-16-0-hr_expense_cancel/it/

In this version, if a product doesn't have cost, you are only able to enter total amount, so the test should be adjusted to this condition.

SilvioC2C

Few things to fix here and there

SilvioC2C · 2024-05-27T13:55:15Z

hr_expense_cancel/models/hr_expense.py

+
+    def action_cancel(self):
+        for sheet in self:
+            account_move = sheet.account_move_ids


Since we have a X2many field, variable name should be plural

Suggested change

account_move = sheet.account_move_ids

account_moves = sheet.account_move_ids

SilvioC2C · 2024-05-27T13:57:07Z

hr_expense_cancel/models/hr_expense.py

+                if sheet.expense_line_ids[:1].payment_mode == "own_account":
+                    self._remove_move_reconcile(payments, account_move)
+                    self._cancel_payments(payments)
+                else:
+                    # In this case, during the cancellation the journal entry
+                    # will be deleted
+                    self._cancel_payments(payments)


We can remove the else block:

Suggested change

if sheet.expense_line_ids[:1].payment_mode == "own_account":

self._remove_move_reconcile(payments, account_move)

self._cancel_payments(payments)

else:

# In this case, during the cancellation the journal entry

# will be deleted

self._cancel_payments(payments)

if sheet.expense_line_ids[:1].payment_mode == "own_account":

self._remove_move_reconcile(payments, account_move)

self._cancel_payments(payments)

SilvioC2C · 2024-05-27T13:57:27Z

hr_expense_cancel/models/hr_expense.py

+            # (if the expense sheet is paid and payment_mode == 'own_account')
+            # it has not been deleted
+            if account_move.exists():
+                move_to_cancel = account_move.filtered(


Rename to moves_to_cancel

SilvioC2C · 2024-05-27T14:01:12Z

hr_expense_cancel/models/hr_expense.py

+        reconcile = account_move.mapped("line_ids.full_reconcile_id")
+        aml = self.env["account.move.line"].search(
+            [("full_reconcile_id", "in", reconcile.ids)]
+        )
+        exp_move_line = aml.filtered(
+            lambda line: line.move_id.id not in account_move.ids
+        )


Why run a search() only to filter out records right after? All this could be implemented via search() only:

Suggested change

reconcile = account_move.mapped("line_ids.full_reconcile_id")

aml = self.env["account.move.line"].search(

[("full_reconcile_id", "in", reconcile.ids)]

)

exp_move_line = aml.filtered(

lambda line: line.move_id.id not in account_move.ids

)

exp_move_line = self.env["account.move.line"].search(

[

("full_reconcile_id", "in", account_move.line_ids.full_reconcile_id.ids),

("move_id", "not in", account_move.ids),

]

)

SilvioC2C · 2024-05-27T14:01:58Z

hr_expense_cancel/models/hr_expense.py

+        exp_move_line.move_id.button_draft()
+        exp_move_line.move_id.button_cancel()


Suggested change

exp_move_line.move_id.button_draft()

exp_move_line.move_id.button_cancel()

if exp_move_line:

moves = exp_move_line.move_id

moves.button_draft()

moves.button_cancel()

SilvioC2C · 2024-05-27T14:02:46Z

hr_expense_cancel/models/hr_expense.py

+        reconcile = account_move.mapped("line_ids.full_reconcile_id")
+        payments_aml = payments.move_id.line_ids
+        aml_unreconcile = payments_aml.filtered(
+            lambda r: r.full_reconcile_id in reconcile
+        )
+
+        aml_unreconcile.remove_move_reconcile()


This method could be improved just like the one above

SilvioC2C · 2024-05-27T14:03:56Z

hr_expense_cancel/models/hr_expense.py

+    def _cancel_payments(self, payments):
+        for rec in payments:
+            rec.move_id.button_draft()
+            rec.move_id.button_cancel()


self is not even used, maybe we could move this method to the payments model instead?

github-actions · 2024-09-29T12:14:41Z

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.

imlopes force-pushed the 17.0-mig-hr_expense_cancel branch 2 times, most recently from c059b8a to c5c7475 Compare May 15, 2024 19:34

imlopes marked this pull request as ready for review May 15, 2024 19:57

vvrossem approved these changes May 16, 2024

View reviewed changes

yankinmax reviewed May 21, 2024

View reviewed changes

ernestotejeda and others added 23 commits May 24, 2024 11:01

hr_expense_cancel: lets to cancel and correct expenses

6742ea2

This module lets to cancel and correct expenses. It adds a cancel button on the expense sheet that undo reconciliations and delete payments and journal entries.

[MIG] hr_expense_cancel: Migration to 12.0

1f6bc06

[FIX] hr_expense_cancel: Avoid singleton error in post_init_hook

44f5def

[FIX] Avoid not intended warnings

2f397b4

[IMP] hr_expense_cancel : black, isort

ecef6dd

[13.0][MIG] hr_expense_cancel

13f8f2f

[14.0][MIG] hr_expense_cancel

4fcda51

Overwrite core function create payment

6ead784

[FIX] hr_expense_cancel: Fix tests + Cancel move payments.

b82523a

Since odoo/odoo@64b2580 TT33774

[ENH] hr_expense_cancel depend on hr_expense_payment

cceddad

[MIG]hr_expense_cancel: Migration to 15.0

fc4201d

[MIG] hr_expense_cancel: Migration to 16.0

f12197f

The multi own account test has been removed as it is no longer possible in odoo core to register the payment of several expense sheet reports at the same time. Co-Authored By: Stefan Ungureanu aiendry-aktivsoftware

[UPD] Update hr_expense_cancel.pot

59d76d1

[UPD] README.rst

302d2ae

Update translation files

4686efc

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: hr-expense-16.0/hr-expense-16.0-hr_expense_cancel Translate-URL: https://translation.odoo-community.org/projects/hr-expense-16-0/hr-expense-16-0-hr_expense_cancel/

Added translation using Weblate (Italian)

6bd7f3f

Translated using Weblate (Italian)

d547481

Currently translated at 100.0% (3 of 3 strings) Translation: hr-expense-16.0/hr-expense-16.0-hr_expense_cancel Translate-URL: https://translation.odoo-community.org/projects/hr-expense-16-0/hr-expense-16-0-hr_expense_cancel/it/

[UPD] README.rst

26f832f

[FIX] hr_expense_cancel: Adjust test to product w/o cost behavior

a74cff8

In this version, if a product doesn't have cost, you are only able to enter total amount, so the test should be adjusted to this condition.

oca-github-bot post-merge updates

8c8ce19

[FIX] no create payments with paid by company

ab4a85f

[BOT] post-merge updates

20c897c

[IMP] hr_expense_cancel: pre-commit auto fixes

0d378d9

imlopes force-pushed the 17.0-mig-hr_expense_cancel branch 2 times, most recently from 386e050 to 1c49c52 Compare May 24, 2024 14:56

[MIG] hr_expense_cancel: Migration to 17.0

8b5454b

imlopes force-pushed the 17.0-mig-hr_expense_cancel branch from 1c49c52 to 8b5454b Compare May 24, 2024 14:57

SilvioC2C suggested changes May 27, 2024

View reviewed changes

vvrossem mentioned this pull request May 29, 2024

[17.0][MIG] hr_expense_cancel: Migration to version 17.0 #238

Merged

github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[17.0][MIG] hr_expense_cancel #233

[17.0][MIG] hr_expense_cancel #233

imlopes commented May 15, 2024 •

edited

Loading

vvrossem left a comment

yankinmax May 21, 2024

imlopes May 24, 2024

SilvioC2C left a comment

SilvioC2C May 27, 2024

SilvioC2C May 27, 2024

SilvioC2C May 27, 2024

SilvioC2C May 27, 2024

SilvioC2C May 27, 2024

SilvioC2C May 27, 2024

SilvioC2C May 27, 2024

github-actions bot commented Sep 29, 2024

	account_move = sheet.account_move_ids
	account_moves = sheet.account_move_ids

		exp_move_line.move_id.button_draft()
		exp_move_line.move_id.button_cancel()

-        exp_move_line.move_id.button_draft()
-        exp_move_line.move_id.button_cancel()
+        if exp_move_line:
+            moves = exp_move_line.move_id
+            moves.button_draft()
+            moves.button_cancel()

[17.0][MIG] hr_expense_cancel #233

Are you sure you want to change the base?

[17.0][MIG] hr_expense_cancel #233

Conversation

imlopes commented May 15, 2024 • edited Loading

vvrossem left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SilvioC2C left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 29, 2024

imlopes commented May 15, 2024 •

edited

Loading