[13.0][MIG] account_invoice_triple_discount #661

kittiu · 2020-01-28T11:30:12Z

Standard migration

Test error from other modules, awaiting for following merge,

[13.0][FIX] account_invoice_fixed_discount, missing param with new Odoo #663

kittiu · 2020-02-05T02:18:33Z

Since v13 is a big change in account invoice, so I have to change the calc logic quite a bit. But I tested it have the same result. I think it is ok, but you might want to look?

Due to v13 also, I do have difficulty writing test. I have the error here,
https://travis-ci.org/OCA/account-invoicing/jobs/643164653#L1019

Because when we change the discount, on screen onchange do not trigger write immediately, and so it works fine.
But on test script, when I assign the new discount, it fire the write() method immediatelly and so the error occur as you see.

Are there anyway I to imitate onchange in test script properly? (I try with Form, it also not work)

ps-tubtim · 2020-05-02T15:20:09Z

@kittiu I think I found some bug.

Process:

Create Customer Invoice
Add a Invoice line by
- Product = Product 1
- Quantity = 1
- Price = 100
- Discount 1 (%) = 10
- Discount 2 (%) = 10
System Compute "Subtotal" = 81
Click Save button
Subtotal = 81 as before but Discount 1, Discount 2 = 0

kittiu · 2020-05-02T19:15:06Z

@ps-tubtim thanks for reporting. This is interesting, I found resolution here,
https://www.odoo.com/forum/help-1/question/adding-field-to-invoice-line-ids-in-account-move-odoo-13-165052
Basically, we need to add the discount field in journal entry tab also, even it is not used. Otherwise, on create line, it won't saved.

kittiu · 2020-05-06T05:51:40Z

Travis failed coz of black. So I did the rebase.

legalsylvain · 2020-05-11T10:40:40Z

account_invoice_triple_discount/models/account_move.py

+    discount3 = fields.Float("Discount 3 (%)", digits="Discount")
+
+    @api.onchange("discount1", "discount2", "discount3")
+    def _onchange_triple_discount(self):


should not be a compute function ? otherwise, a self.env["account.move.line"].create({"discount1": 10, "discount2": 20}) will not raise the compute of discount. Don't you think ?

Thank you, this is a good comment.
Fixed.

benwillig · 2020-05-27T09:58:16Z

Could you fix travis please? Then we can see the results of the unit tests.

EDIT: tests fails when started them locally

benwillig · 2020-06-02T06:48:38Z

Hi @kittiu, any news about this?

kittiu · 2020-06-02T07:12:19Z

Hi @kittiu, any news about this?

Sorry, I am back now. :) Will continue.

Currently translated at 100,0% (4 of 4 strings) Translation: account-invoicing-11.0/account-invoicing-11.0-account_invoice_triple_discount Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-11-0/account-invoicing-11-0-account_invoice_triple_discount/de/

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_triple_discount Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_triple_discount/

Currently translated at 100.0% (4 of 4 strings) Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_triple_discount Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_triple_discount/pt_BR/

kittiu · 2020-06-02T09:59:45Z

Hi @kittiu, any news about this?

Sorry, I am back now. :) Will continue.

Done.

kittiu · 2020-06-02T16:33:27Z

@benwillig @legalsylvain @ps-tubtim ready for review. Thanks!

benwillig

functionnal tests and code review, LGTM

legalsylvain

Not sure but introducing discount 1 will break all the native feature when invoices are created from sale /purchase order. Dont you think ?

benwillig

Not sure but introducing discount 1 will break all the native feature when invoices are created from sale /purchase order. Dont you think ?

@legalsylvain might be right as we can see here. Don't we simply need to set an inverse function on discount to store the value in discount1? Not sure if we will get an infinite loop here.

legalsylvain · 2020-06-03T09:33:53Z

what about the previous design ? without discount_1 field ?

kittiu · 2020-06-03T10:09:46Z

@legalsylvain
I did try that but quite troublesome due to the new dynamic journal entry of new account.move. I did tried but remember it is very difficult.

If you still insist, I will try again.

kittiu · 2020-06-03T12:50:03Z

Not sure but introducing discount 1 will break all the native feature when invoices are created from sale /purchase order. Dont you think ?

Another though, what if when the original discount has value, and others don't. I will distribute it into discount_1. In other words, I will ensure that, the discount will always equal to discount_1 * discount_2 * discount_3

The new account.move in v13, is much more complex than v12, that is one reason. Another reasons is the code of v12 by itself is much more complex without having discount_1.
https://github.com/OCA/account-invoicing/blob/12.0/account_invoice_triple_discount/models/account_invoice.py

I still think, this PR is also the improvement. WDYT?

benwillig · 2020-06-04T09:09:09Z

The problem here is changing discount as a compute field will break other odoo processes which populate that field on creation (or modify it afterwards). such as the sale one as @legalsylvain pointed it out. Even if there is a discount on the sale order, it won't be propagated to the invoice because _compute_triple_discount will be triggered and will override that value. With the current PR, we would need to add other modules to fix it and this is not how it should be done.

kittiu · 2020-06-05T03:33:54Z

@benwillig OK.
@legalsylvain, WDYT?
I may open another PR and comply with all opinion here.

kittiu · 2020-06-05T11:17:44Z

@benwillig @legalsylvain I must confess.
I tried again, and I found that without discount_1 + the complexity of v13 account.move, I can't really proceed. It is too complex for me.
I have 2 options now.

withdraw myself from this PR (I may close it).
keep the discount_1, ensure passing discount alone is ok for non-triple discount cases, and for sales/purchase triple discount make sure that related addons comply with this, i.e.,
https://github.com/ecosoft-odoo/sale-workflow/blob/13.0-mig-sale_triple_discount/sale_triple_discount/models/sale_order_line.py#L90

I understand this PR also need migration script from v12 to v13, not very elegant PR also.

benwillig · 2020-06-05T13:36:40Z

I will make further investigations on my side and make a PR if possible.

kittiu · 2020-06-16T02:48:40Z

As most people agree to move to this approach #733, so I would like to close this for now.

@ps-tubtim please help fix OCA/sale-workflow#1138 for this #733 (when it is ready)

kittiu force-pushed the 13.0-mig-account_invoice_triple_discount branch from 1fa83cd to 3e7e792 Compare January 29, 2020 00:12

kittiu changed the title ~~[13.0][MIG] account_invoice_triple_discount [WIP]~~ [13.0][MIG] account_invoice_triple_discount Feb 5, 2020

kittiu changed the title ~~[13.0][MIG] account_invoice_triple_discount~~ [13.0][MIG] account_invoice_triple_discount [WIP] Feb 5, 2020

kittiu mentioned this pull request Feb 6, 2020

Migration to version 13.0 #600

Closed

32 tasks

kittiu changed the title ~~[13.0][MIG] account_invoice_triple_discount [WIP]~~ [13.0][MIG] account_invoice_triple_discount Feb 13, 2020

kittiu force-pushed the 13.0-mig-account_invoice_triple_discount branch from 3e7e792 to f542381 Compare May 2, 2020 19:12

kittiu force-pushed the 13.0-mig-account_invoice_triple_discount branch 2 times, most recently from 77d0ff5 to 29cdd46 Compare May 6, 2020 05:50

This was referenced May 10, 2020

[13.0][MIG] sale_triple_discount OCA/sale-workflow#1138

Closed

[13.0][MIG] purchase_triple_discount OCA/purchase-workflow#912

Closed

legalsylvain reviewed May 11, 2020

View reviewed changes

chienandalu and others added 12 commits June 2, 2020 16:36

account_invoice_triple_discount: New module

4b123a6

account_invoice_triple_discount: Some improvements

cb30b96

OCA Transbot updated translations from Transifex

2724898

account_invoice_triple_discount

68f1143

Update account_invoice_triple_discount.pot

a0e6150

[MIG] account_invoice_triple_discount v11 to v12

a9a5888

[UPD] Update account_invoice_triple_discount.pot

59f4c5e

[ADD] icon.png

120979a

[IMP] : black, isort

0a40683

[13.0][MIG] account_invoice_triple_discount.

ad37c33

kittiu force-pushed the 13.0-mig-account_invoice_triple_discount branch from 29cdd46 to ad37c33 Compare June 2, 2020 09:36

benwillig approved these changes Jun 3, 2020

View reviewed changes

legalsylvain suggested changes Jun 3, 2020

View reviewed changes

benwillig suggested changes Jun 3, 2020

View reviewed changes

benwillig mentioned this pull request Jun 12, 2020

[13.0][MIG] account_invoice_triple_discount #733

Closed

kittiu closed this Jun 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[13.0][MIG] account_invoice_triple_discount #661

[13.0][MIG] account_invoice_triple_discount #661

kittiu commented Jan 28, 2020 •

edited

Loading

kittiu commented Feb 5, 2020

ps-tubtim commented May 2, 2020

kittiu commented May 2, 2020

kittiu commented May 6, 2020

legalsylvain May 11, 2020

kittiu Jun 2, 2020

benwillig commented May 27, 2020 •

edited

Loading

benwillig commented Jun 2, 2020

kittiu commented Jun 2, 2020

kittiu commented Jun 2, 2020

kittiu commented Jun 2, 2020

benwillig left a comment

legalsylvain left a comment

benwillig left a comment •

edited

Loading

legalsylvain commented Jun 3, 2020

kittiu commented Jun 3, 2020 •

edited

Loading

kittiu commented Jun 3, 2020 •

edited

Loading

benwillig commented Jun 4, 2020

kittiu commented Jun 5, 2020

kittiu commented Jun 5, 2020

benwillig commented Jun 5, 2020

kittiu commented Jun 16, 2020 •

edited

Loading

[13.0][MIG] account_invoice_triple_discount #661

[13.0][MIG] account_invoice_triple_discount #661

Conversation

kittiu commented Jan 28, 2020 • edited Loading

kittiu commented Feb 5, 2020

ps-tubtim commented May 2, 2020

kittiu commented May 2, 2020

kittiu commented May 6, 2020

legalsylvain May 11, 2020

Choose a reason for hiding this comment

kittiu Jun 2, 2020

Choose a reason for hiding this comment

benwillig commented May 27, 2020 • edited Loading

benwillig commented Jun 2, 2020

kittiu commented Jun 2, 2020

kittiu commented Jun 2, 2020

kittiu commented Jun 2, 2020

benwillig left a comment

Choose a reason for hiding this comment

legalsylvain left a comment

Choose a reason for hiding this comment

benwillig left a comment • edited Loading

Choose a reason for hiding this comment

legalsylvain commented Jun 3, 2020

kittiu commented Jun 3, 2020 • edited Loading

kittiu commented Jun 3, 2020 • edited Loading

benwillig commented Jun 4, 2020

kittiu commented Jun 5, 2020

kittiu commented Jun 5, 2020

benwillig commented Jun 5, 2020

kittiu commented Jun 16, 2020 • edited Loading

kittiu commented Jan 28, 2020 •

edited

Loading

benwillig commented May 27, 2020 •

edited

Loading

benwillig left a comment •

edited

Loading

kittiu commented Jun 3, 2020 •

edited

Loading

kittiu commented Jun 3, 2020 •

edited

Loading

kittiu commented Jun 16, 2020 •

edited

Loading