[13.0][MIG] account_invoice_triple_discount

[benwillig]

Other proposal for the migration of account_invoice_triple_discount. (orginal one is #661).

@legalsylvain @kittiu This PR doesn't change the behavior of discount field. But as @kittiu said, it makes the code a bit more complex than the original PR.

To avoid that complexity, I think we should try to refactor odoo code to be able to inherit properly and to have the good data at the right time. I know it could not be a few lines changes so it might be impossible that odoo accept this. As you can see here, the subtotal price is computed during the create. It's not possible, without changing the original code, to give the method more data (here, the two new discount field) to have a correct computation of that price. In this method, if the balance given by the UI (computed during onchange) is different than the computed subtotal (during the create), the unit price of the line will be updated. As the odoo method handle only the first discount feld, it results in a difference and the unit price changes.

If I'm not wrong, at the moment it's almost impossible (without writing dirty code) to change the computation of the subtotal of an invoice line. I don't see the use case now, but it could be a problem for other modules.

[Cedric-Pigeon]

Hi,

from my point of view, I prefer this approach as it is less intrusive in core and does not change the original nature of discount field.

[pedrobaeza]

The problem I see here is that this is not inheritance friendly, as it requires to put glue modules when other fields need to touch the same fields and also copies logic from upstream that can be patched in the future. What is the problem with the other approach?

[kittiu]

@benwillig thank you for your close up look.
@pedrobaeza the problem with other approach #661 is

1. It is not the same as v12 approach, it introduce new field discount_1 (and change how discount works), so it will require migration script, from 12 -> 13.
2. People worry that when discount is passed, i.e., from purchase / sales or others to this field, it will be messed up. (on this point, I think it can be fixed).

I would prefer having discount as it was, if it is not too complex. What is your opinion on this?

[pedrobaeza]

Why v12 approach is not valid?

[kittiu]

Why v12 approach is not valid?

v12 is valid. v13, to use the same approach is just more complex because of the different nature of account.move.

[pedrobaeza]

OK, then the logic should be a bit modified for that, but the essence should be preserved IMO

[kittiu]

Suggested change

	old_discount = values.get("discount")
	old_discount = values.get("discount", 0.0)

[kittiu]

This will fix the error on installation, with steps:

1. Create new database with demo data.
2. Install module account_invoice_triple_discount without first install account module.

[benwillig]

Thanks, should be fixed now

[kittiu]

Functional tested OK, but when run test locally, I found this error message though it is not in travis.

2020-06-16 03:37:13,385 25419 INFO 13.0-mig-account_invoice_triple_discount-bwi odoo.addons.account_invoice_triple_discount.tests.test_invoice_triple_discount: ====================================================================== 
2020-06-16 03:37:13,385 25419 ERROR 13.0-mig-account_invoice_triple_discount-bwi odoo.addons.account_invoice_triple_discount.tests.test_invoice_triple_discount: ERROR: TestInvoiceTripleDiscount.test_02_discounts_multiple_lines
Traceback (most recent call last):
  File "/home/kittiu/addons/account-invoicing/account_invoice_triple_discount/tests/test_invoice_triple_discount.py", line 114, in test_02_discounts_multiple_lines
    invoice_form.save()
  File "/home/kittiu/odoo13/odoo/tests/common.py", line 1702, in save
    r.write(values)
  File "/home/kittiu/odoo13/addons/account/models/account_move.py", line 1661, in write
    self._check_balanced()
  File "/home/kittiu/odoo13/addons/account/models/account_move.py", line 1474, in _check_balanced
    raise UserError(_("Cannot create unbalanced journal entry. Ids: %s\nDifferences debit - credit: %s") % (ids, sums))
odoo.exceptions.UserError: ('Cannot create unbalanced journal entry. Ids: [11]\nDifferences debit - credit: [-500.0]', '')

[kittiu]

@benwillig thank you for this PR, I think your code is as clean as it could be already 👍

[ps-tubtim]

The functional test is ok but small code reviews.

[ps-tubtim]

Suggested change

	"author": "QubiQ," "Tecnativa," "Odoo Community Association (OCA)",
	"author": "QubiQ, Tecnativa, Odoo Community Association (OCA)",

[Cedric-Pigeon]

@ps-tubtim resolved, thanks

[kittiu]

@benwillig account_invoice_supplier_ref_unique fix is merged. #754

[kittiu]

@benwillig do you plan to continue with this? I think just rebase will do.

[santostelmo]

Hello @benwillig any progress expected soon on this ?

[santostelmo]

Tested functionaly and code LG

[Cedric-Pigeon]

@santostelmo Just rebased the PR.

[santostelmo]

@Cedric-Pigeon tests are still falling on travis

[Cedric-Pigeon]

yes but I do not think the error is linked to this PR.
Other PR fails with the same error. eg: #852

[HaraldPanten]

@OCA/accounting-maintainers should we merge this PR manually as the Travis error is not related to this module?

I mean... same as we did in #852 , for example

[HaraldPanten]

@benwillig @Cedric-Pigeon I was testing your module in a local environment and I found a strange issue. How to reproduce:

1- Create a PO with these details:

Line1: 50 units of product A// 7,05 unit price// discount 1: 50% // discount 2: 10% // discount 3: 10% // Taxes (21% Spanish VAT)

Subtotal for line 1 = 142,76

Line 2: Product B // 1 unit // 5,50 unit price // no discounts // Taxes (21% Spanish VAT)

Subtotal for line 2 = 5,5

PO details:

PO total amount = 179,40
PO total 21% Spanish VAT = 31,14

2- Generate the invoice of this PO. (Make sure you have received the product units if it's a storable product and it's purchase invoice policy is by delivered units.)

Invoice details (With the same data inherited from the PO):

Subtotal for line 1 = 142,76
Subtotal for line 2 = 5,5

Invoice total amount = 179,45
Invoice total 21% Spanish VAT = 31,19

I don't know why PO totals do not match Invoice totals... Could you check?

I've used purchase_triple_discount PR and your PR.

THX!

[JordiADARIA]

Hi @HaraldPanten , i'll check it on our local env

thanks

[HaraldPanten]

It's related to this. I think it's not easy to solve --> #876

[JordiADARIA]

the new account.move on v13, as it was already said, its really different than in v12, it's difficult to adapt it, including the new mixin class...im in a mess

[igallart]

Hello, this pull PR is functional? What problem it has?

[ghost]

Hello ,using this module ,i have a bug , please try that steps:
1- create invoice.
2- create line.
3- add discount.
4- without save , change invoice date
after look tax base.

for example:
one line with price unit 100$ , tax 21% ,
at this poing the tax base should be 21$ , then add discount 50%,
now the tax base should be 10,5$ , finally ,change invoice date now the tax base is 21$ again

I think I solved this by changing this code:

def _recompute_tax_lines(self, recompute_tax_base_amount=False):
        vals = {}

        for line in self.invoice_line_ids.filtered(
                lambda l: l.discount2 or l.discount3):

            vals[line] = {
                "price_unit": line.price_unit,
                "discount": line.discount
            }
            aggregated_discount = line._compute_aggregated_discount(
                line.discount)
            price_unit = line.price_unit * (1 - aggregated_discount / 100)
            line.update({"price_unit": price_unit, "discount": 0})

        res = super(AccountMove, self)._recompute_tax_lines(
            recompute_tax_base_amount=recompute_tax_base_amount)

        for line in vals.keys():
            line.update(vals[line])
        return res

[OCA-git-bot]

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

[github-actions]

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.