Skip to content

[13.0][MIG] account_cash_discount modules #334

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

Merged
merged 31 commits into from
May 25, 2022

Conversation

benwillig
Copy link

@benwillig benwillig commented Jun 8, 2020

Migration of:

  • account_cash_discount_base
  • account_cash_discount_payment
  • account_cash_discount_write_off

@benwillig benwillig force-pushed the 13.0-mig-account_cash_discount branch from fa84b31 to 658bf9e Compare June 10, 2020 09:18
@benwillig benwillig changed the title [WIP][13.0][MIG] account_cash_discount modules [13.0][MIG] account_cash_discount modules Jun 10, 2020
Copy link

@Cedric-Pigeon Cedric-Pigeon left a comment

Choose a reason for hiding this comment

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

LGTM (code review only).

Could you please check why runbot is red ?

@pedrobaeza
Copy link
Member

Do you know account_global_discount and corresponding modules that perform the same in a general manner?

@benwillig benwillig force-pushed the 13.0-mig-account_cash_discount branch from 658bf9e to ff1c4a3 Compare June 16, 2020 05:56
@benwillig
Copy link
Author

LGTM (code review only).

Could you please check why runbot is red ?

travis is fixed now

@eguane
Copy link

eguane commented Jul 12, 2020

Hi, did you expect to backport it to v12 ?
between 80% and 90% of company in belgium using cash discount with this scenario: deducted discount from amount total without reducing VAT base, is it possible with your module.
Rgds

@benwillig
Copy link
Author

Hi, did you expect to backport it to v12 ?
between 80% and 90% of company in belgium using cash discount with this scenario: deducted discount from amount total without reducing VAT base, is it possible with your module.
Rgds

Not at the moment, but I think it shouldn't be so hard to migrate this from 10.0 to 12.0

@eguane
Copy link

eguane commented Jul 14, 2020

Hi, did you expect to backport it to v12 ?
between 80% and 90% of company in belgium using cash discount with this scenario: deducted discount from amount total without reducing VAT base, is it possible with your module.
Rgds

Not at the moment, but I think it shouldn't be so hard to migrate this from 10.0 to 12.0

Thank for your feedback, I tested V13 module, I probably miss something into the configuration. Company discount and payment term are both configured. When I try to add payment from the invoice, The amount proposed is always the invoice one, not discounted. Also if not already embeded, is it envisageable to add the value of the discount into a comment on the invoice, like "cash discount of xxx€ if paid before end date xx/xx/xx"
Can you confirm that the scenario deducted discount from amount total without reducing VAT base, is it possible with your module?
Rgds

@sbidoul
Copy link
Member

sbidoul commented Sep 30, 2020

cross referencing account_financial_discount that covers a similar scope: OCA/account-financial-tools#1040

@sanrav
Copy link

sanrav commented Oct 1, 2020

functional testing OK

@dreispt
Copy link
Member

dreispt commented Jul 19, 2021

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 13.0-ocabot-merge-pr-334-by-dreispt-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jul 19, 2021
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 13.0-ocabot-merge-pr-334-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.

adrienpeiffer and others added 6 commits January 12, 2022 10:43
[ADD] Allow to configure discount options directly on the payment term

[ADD] Add account cash discount payment module

[MIG] account_cash_discount_payment migration. Also removed force_discount_amount from the account.invoice

[ADD] Allow to tell if a payment line is paid using the discount or not. Keep the original amount in order to compute it again
@acsonefho acsonefho force-pushed the 13.0-mig-account_cash_discount branch from db66bef to 9abb088 Compare January 13, 2022 12:53
When the payment line create the account.move.line, tag_ids are not filled and so audit_tax still empty too. Fix + adapt unit test
@acsonefho acsonefho force-pushed the 13.0-mig-account_cash_discount branch from 9abb088 to 444ae4d Compare January 25, 2022 13:57
@acsonefho
Copy link
Contributor

@sbidoul @adrienpeiffer Is it possible to review and merge this PR please? Thanks! (I see you in contributors)

@acsonefho
Copy link
Contributor

@dreispt

/ocabot merge nobump

Can you review and merge please?

Copy link

@adrienpeiffer adrienpeiffer left a comment

Choose a reason for hiding this comment

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

LGTM

@OCA-git-bot
Copy link
Contributor

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

@dreispt
Copy link
Member

dreispt commented May 25, 2022

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 13.0-ocabot-merge-pr-334-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 64aa784 into OCA:13.0 May 25, 2022
@OCA-git-bot
Copy link
Contributor

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

@acsonefho acsonefho deleted the 13.0-mig-account_cash_discount branch May 30, 2022 06:33
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.