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.

Sign up for GitHub

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

Jump to bottom

[13.0][FIX] account_banking_sepa_direct_debit: Bank Account should not be required #875

Merged

OCA-git-bot merged 2 commits into OCA:13.0 from tegin:13.0-fix-data

Dec 20, 2021

Member

etobella commented Dec 14, 2021

The bank account is taken from the mandate. invoice_partner_bank_id is never used in this module, so it should not be taken as required.

pedrobaeza added this to the 13.0 milestone

pedrobaeza reviewed

View reviewed changes

account_banking_sepa_direct_debit/data/account_payment_method.xml Outdated Show resolved Hide resolved

JordiBForgeFlow approved these changes

View reviewed changes

etobella added 2 commits

December 14, 2021 12:58


          [FIX] account_banking_sepa_direct_debit: Bank Account and Mandate sho…

c78e694

…uld not be required

The bank account is taken from the mandate. Mandate is autopopulated if not found.


          [FIX] account_banking_sepa_credit_transfer: Bank account is not required

97ae592

etobella force-pushed the 13.0-fix-data branch from 25f1fc8 to 97ae592 Compare

December 14, 2021 11:59

pedrobaeza approved these changes

View reviewed changes

OCA-git-bot added the approved label

alexis-via reviewed

View reviewed changes

account_banking_sepa_credit_transfer/data/account_payment_method.xml Show resolved Hide resolved

OCA-git-bot added the ready to merge label

Contributor

OCA-git-bot commented Dec 19, 2021

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

Member

pedrobaeza commented Dec 19, 2021

@alexis-via we want to continue with the merging.

HaraldPanten approved these changes

View reviewed changes

HaraldPanten left a comment

Small changes. LGTM

Member

pedrobaeza commented Dec 20, 2021

/ocabot merge minor

pedrobaeza changed the title ~~[FIX] account_banking_sepa_direct_debit: Bank Account should not be required~~ [13.0][FIX] account_banking_sepa_direct_debit: Bank Account should not be required

Contributor

OCA-git-bot commented Dec 20, 2021

On my way to merge this fine PR!
Prepared branch 13.0-ocabot-merge-pr-875-by-pedrobaeza-bump-minor, awaiting test results.

OCA-git-bot added the bot is merging ⏳ label

OCA-git-bot merged commit db15a58 into OCA:13.0

Contributor

OCA-git-bot commented Dec 20, 2021

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

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels

olgamarcocb deleted the 13.0-fix-data branch

December 21, 2021 08:34

Member

pedrobaeza commented Jan 2, 2022

Cherry-picked for v14 at 24d7a06 and f8b0245

Contributor

alexis-via commented Jan 8, 2022

BRAVO BRAVO BRAVO
You just broke the OCA implementation of SEPA direct debit. According to my test in v14 and an audit of the code:

you can't select a mandate on an invoice any more (field hidden, cf https://github.com/OCA/bank-payment/blob/14.0/account_banking_mandate/views/account_move_view.xml#L16)
you can't select a mandate on a payment line any more (field hidden, cf https://github.com/OCA/bank-payment/blob/14.0/account_banking_mandate/views/account_payment_line.xml#L20)
with this change, the code that checks that a mandate is present when you confirm the payment order is disabled (cf https://github.com/OCA/bank-payment/blob/14.0/account_banking_mandate/models/account_payment_line.py#L60)

The change introduced by this PR don't reflect the title of the PR nor the message of @etobella at the begining of the PR. I'm ok to set bank_account_required to False on sepa_direct_debit, but:

I'm not ok to set mandate required to False on sepa_direct_debit
I'm not ok to set bank account required to False on sepa_credit_transfer
I am the author of this code and I had the feeling that we had a problem with this PR, but my messages were not taken seriously. I'll propose to partially revert it and then, it we want to make additional changes, take the time to discuss them and measure the impact of the changes.

alexis-via added a commit to akretion/banking that referenced this pull request


          sepa_direct_debit: mandate_required back to True

144d2d9

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

alexis-via mentioned this pull request

[14.0][FIX] mandate_required back to True on SDD, bank_account_required back to True on SCT #888

Merged

manuelregidor pushed a commit to sygel-technology/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

f0f5666

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

manuelregidor pushed a commit to sygel-technology/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

2e5a3eb

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

wpichler pushed a commit to wpichler/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

73ff3bd

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

dzungtran89 pushed a commit to dzungtran89/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

gfcapalbo pushed a commit to gfcapalbo/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

e318f3d

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

gfcapalbo pushed a commit to gfcapalbo/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

b16000b

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

wpichler pushed a commit to wpichler/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

de3af90

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

1eae391

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

AnizR pushed a commit to acsone/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

5c4c9c6

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

10bcb53

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

1786f87

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

dzungtran89 pushed a commit to dzungtran89/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

c34c94c

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

12f9f11

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

ca39944

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

dzungtran89 pushed a commit to dzungtran89/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

09019ad

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

34cefbc

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

dzungtran89 pushed a commit to dzungtran89/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

6ca28e3

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

dzungtran89 pushed a commit to dzungtran89/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

0147a68

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

Reyes4711-S73 pushed a commit to Studio73/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

2302c9c

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

agyamuta pushed a commit to ursais/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

1f3e95c

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

be865c1

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

d34658f

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

2adf524

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

6c6e00d

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

90ef3ca

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

cf51b13

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

nilshamerlinck pushed a commit to nilshamerlinck/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

3c700a3

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

7d7427f

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

JasminSForgeFlow pushed a commit to ForgeFlow/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

7a30442

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

trisdoan pushed a commit to trisdoan/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

5a39d70

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

JasminSForgeFlow pushed a commit to ForgeFlow/bank-payment that referenced this pull request


          sepa_direct_debit: mandate_required back to True

1d666e0

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

alexis-via added a commit to akretion/banking that referenced this pull request


          sepa_direct_debit: mandate_required back to True

050243b

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

alexis-via added a commit to akretion/banking that referenced this pull request


          sepa_direct_debit: mandate_required back to True

7c57478

sepa_credit_transfer: bank_account_required back to True
Partial revert of the breakage of PR OCA#875

account_banking_sepa_direct_debit 14.0.1.3.0

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

Labels

approved merged 🎉 ready to merge