Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][MIG] sale_invoice_plan: Migrate to 16.0 #2438

Open
wants to merge 57 commits into
base: 16.0
Choose a base branch
from

Conversation

MS-OSI
Copy link

@MS-OSI MS-OSI commented Mar 28, 2023

Sales Invoice Plan

By standard feature, user can gradually create partial invoices, one by one.
This module add ability to create invoices based on the predefined invoice plan,
either all at once, or one by one.
The plan support both advance invoice and installment invoices.

@MS-OSI MS-OSI changed the title 16.0 mig sale invoice plan [16.0][MIG] sale_invoice_plan: Migrate to 16.0 Mar 28, 2023
@rousseldenis
Copy link
Contributor

/ocabot migration sale_invoice_plan

Copy link

@imlopes imlopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DigitalAutomations
Copy link

Any update about this?

@dreispt
Copy link
Member

dreispt commented Nov 23, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-2438-by-dreispt-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 23, 2023
Signed-off-by dreispt
@OCA-git-bot
Copy link
Contributor

@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-2438-by-dreispt-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@dreispt
Copy link
Member

dreispt commented Nov 23, 2023

Added forward port of #2408

@dreispt dreispt force-pushed the 16.0-MIG-sale_invoice_plan branch 3 times, most recently from 9216663 to 0083fb8 Compare November 23, 2023 19:10
@dreispt dreispt self-requested a review November 24, 2023 11:29
@dreispt
Copy link
Member

dreispt commented Nov 24, 2023

@kittiu I see you are the maintainer.
With just the basic migration to 16.0 this worked terribly wrong.
I added this fix to try make it work.

This migration might be worth deeper review and testing from you, if possible. Thanks!

@dreispt dreispt force-pushed the 16.0-MIG-sale_invoice_plan branch 2 times, most recently from 8bb0edc to a52b260 Compare November 24, 2023 13:54
@Borruso
Copy link
Contributor

Borruso commented Dec 6, 2023

can you see this PR?
ursais#10

@kittiu
Copy link
Member

kittiu commented Dec 17, 2023

@dreispt let me check.

@kittiu I see you are the maintainer. With just the basic migration to 16.0 this worked terribly wrong. I added this fix to try make it work.

This migration might be worth deeper review and testing from you, if possible. Thanks!

Sorry for delayed response, I will review this shortly. What is in the "fix" link, clicked and see nothing there.

@kittiu
Copy link
Member

kittiu commented Dec 20, 2023

I think I found something. Let me explain.

We only use this module until version 14 without problem. I think bug about Advance was introduced in version 15, when following new fields was introduced.

https://github.com/OCA/sale-workflow/blob/15.0/sale_invoice_plan/models/sale.py#L28-L35

invoice_plan_total_percent and invoice_plan_total_amount -> trigger [sale.order]._compute_invoice_plan_total which also trigger [sale.order.plan]._compute_amount that mess up the percent.

OK the bug mentioned above was fixed here #2543

Let's merge it and cherry pick it here.

dreispt and others added 3 commits December 20, 2023 08:18
The Advance Payment use case still doesn't work, but at least invoicing
one by one works.
- Move menu item after Sales Orders, rather than first option
- Fix typo
@dreispt dreispt force-pushed the 16.0-MIG-sale_invoice_plan branch 2 times, most recently from 6eb6199 to 43397b0 Compare December 20, 2023 09:17
@dreispt
Copy link
Member

dreispt commented Dec 20, 2023

@kittiu Thank you for looking into this.
You are right, I moved the changes to ursais#12
I also forward ported the 15.0 fix you mentioned.

@kittiu
Copy link
Member

kittiu commented Mar 2, 2024

Hello,

As I come back to check this module, sale_invoice_plan. I tested that everything including Advance works fine on version 14 and 15.
But when I migrate 15 to 16, the Advance is not working correctly. So, I think there must be changes on Odoo core.

I want to propose to close this PR first, and I will do the fresh upgrade from 15 to 16 make sure everything works according to its normality. Then, if you have any other features to add, you can do in separated PR (so it can be backport too)

cc @dreispt @MS-OSI

@kittiu
Copy link
Member

kittiu commented Mar 2, 2024

Here the new PR with minimum fix to make it works as in previous version #2988

@kittiu
Copy link
Member

kittiu commented Mar 8, 2024

@MS-OSI can you help closing this one? Thanks!

@kittiu
Copy link
Member

kittiu commented Mar 31, 2024

Can anyone help closing this PR please. Thanks!

@DigitalAutomations
Copy link

Any udpdate?
What is the correct pr to test ?

@Saran440
Copy link
Member

@DigitalAutomations you can test PR #2988

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.