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

[14.0][REF] account_payment_*: Use native payment object #979

Merged

Conversation

pedrobaeza
Copy link
Member

@pedrobaeza pedrobaeza commented Nov 6, 2022

The previous approach creates manually the journal entries and does all the hard work, plus not being 100% compatible with the bank statement reconciliation widget (requiring a patch on OCB to see blue lines).

That decision made sense on the moment it was done (v9), where the native payment model (account.payment) was very limited, and wasn't able to store all the needed information for the bank transaction.

Now that the limitations are gone, we can get rid off this extra model, and generate instead account.payment records, using both the native model + methods to perform the same operations.

This serves also to workaround the problem found in #966.

All the code, views and tests of main module have been adapted to this new approach

@Tecnativa TT39832

@pedrobaeza pedrobaeza added this to the 14.0 milestone Nov 6, 2022
@pedrobaeza
Copy link
Member Author

pedrobaeza commented Nov 6, 2022

@alexis-via @sbidoul @HaraldPanten @ValentinVinagre @carlosdauden here it's the long awaited refactoring to make use of the native account.payment object for handling the bank lines and the move generation. The stats are good: +205 lines added against −810 lines removed.

See individual commits in order to have better comprehension of the process.

Working on the migration scripts now.

@pedrobaeza pedrobaeza force-pushed the 14.0-ref-account_payment_order-native_payment branch from b614d96 to ea0c8d7 Compare November 6, 2022 15:30
Copy link
Contributor

@carlosdauden carlosdauden left a comment

Choose a reason for hiding this comment

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

At some point we should extend the account.move method def _get_invoice_in_payment_state(self): to return 'in_payment' to activate the payment status "In process" at the invoices.
https://github.com/odoo/odoo/blob/e337a7b4b0da970041ea26d6fdc6e2d39eb3e5be/addons/account/models/account_move.py#L1389

@pedrobaeza
Copy link
Member Author

Indeed, but that's the next step not included in this PR.

Copy link
Contributor

@carlosdauden carlosdauden left a comment

Choose a reason for hiding this comment

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

I have reviewed and tested and everything is correct, I am waiting for the migration scripts to approve

@pedrobaeza pedrobaeza force-pushed the 14.0-ref-account_payment_order-native_payment branch 4 times, most recently from 10a5c79 to 5f96b29 Compare November 7, 2022 16:57
@HaraldPanten
Copy link

First tests seem to be working fine. But I'll need more time to check it properly.

Have you tested it in "semi-real" use cases?

@pedrobaeza pedrobaeza force-pushed the 14.0-ref-account_payment_order-native_payment branch 3 times, most recently from 48ef4f9 to 927ac5e Compare November 7, 2022 20:55
Copy link
Contributor

@carlosdauden carlosdauden left a comment

Choose a reason for hiding this comment

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

@pedrobaeza pedrobaeza force-pushed the 14.0-ref-account_payment_order-native_payment branch 2 times, most recently from e54f205 to 347d38e Compare November 8, 2022 08:21
@pedrobaeza
Copy link
Member Author

Migration scripts are completed and tested on a real environment with all the possible cases, as well as the features of the module. We have an slower payment order confirmation due to the full creation of the account.payment + account.move, but faster times later on "File successfully uploaded" phase.

pedrobaeza added a commit to Tecnativa/account-payment that referenced this pull request Nov 8, 2022
pedrobaeza added a commit to Tecnativa/account-payment that referenced this pull request Nov 8, 2022
pedrobaeza added a commit to Tecnativa/account-payment that referenced this pull request Nov 8, 2022
pedrobaeza added a commit to Tecnativa/account-reconcile that referenced this pull request Nov 8, 2022
@pedrobaeza
Copy link
Member Author

We should synchronize PR merge with changes in these modules so as not to cause malfunctions:

Done on:

OCA/account-payment#555
OCA/account-reconcile#505

@pedrobaeza pedrobaeza force-pushed the 14.0-ref-account_payment_order-native_payment branch from 347d38e to 121913d Compare November 9, 2022 07:35
pedrobaeza added a commit to Tecnativa/account-payment that referenced this pull request Nov 9, 2022
pedrobaeza added a commit to Tecnativa/account-reconcile that referenced this pull request Nov 9, 2022
@pedrobaeza
Copy link
Member Author

I'm removing that leftovers in #1015, but there can't be equivalent options, as now each account.payment generates one move, and you can't group several payments from different partners in one record.

pedrobaeza added a commit to Tecnativa/bank-payment that referenced this pull request Jan 5, 2023
They are ignored since OCA#979, and there can't be equivalent.
@hhgabelgaard
Copy link

Great work, thank you !
Has this change been forward ported to 15.0?
Or is there any plans for this?
The upgrade path is kind of broken now it seems ?

@pedrobaeza
Copy link
Member Author

Yes, indeed. I want to do it for 15.0, but no time for now. Do you want to do it? I will review it. It should be pretty straight-forward.

alexis-via added a commit to akretion/l10n-france that referenced this pull request Feb 14, 2023
Adapt to the "revolution" in account_payment_order that now uses native
payment object, introduced by PR OCA/bank-payment#979
hhgabelgaard pushed a commit to steingabelgaard/bank-payment that referenced this pull request Feb 16, 2023
They are ignored since OCA#979, and there can't be equivalent.
hhgabelgaard pushed a commit to steingabelgaard/bank-payment that referenced this pull request Feb 16, 2023
They are ignored since OCA#979, and there can't be equivalent.
victoralmau pushed a commit to Tecnativa/account-payment that referenced this pull request Mar 7, 2023
carlosdauden pushed a commit to Tecnativa/account-payment that referenced this pull request Mar 21, 2023
pedrobaeza added a commit to Tecnativa/account-payment that referenced this pull request Mar 23, 2023
heliaktiv pushed a commit to heliaktiv/account-payment that referenced this pull request Apr 17, 2023
ernesto-garcia-tecnativa pushed a commit to Tecnativa/account-reconcile that referenced this pull request May 4, 2023
ernesto-garcia-tecnativa pushed a commit to Tecnativa/account-reconcile that referenced this pull request May 4, 2023
@@ -281,18 +278,19 @@ def generated2uploaded(self):
to_expire_mandates = abmo.browse([])
first_mandates = abmo.browse([])
all_mandates = abmo.browse([])
for bline in order.bank_line_ids:
if bline.mandate_id in all_mandates:
for payment in order.payment_ids:
Copy link
Contributor

Choose a reason for hiding this comment

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

@pedrobaeza This field payment_ids doesn't exist on account.payment.order model.
Or maybe there is a missing dependency?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ricardoalso pushed a commit to camptocamp/account-reconcile that referenced this pull request Sep 1, 2023
Ricardoalso pushed a commit to camptocamp/account-reconcile that referenced this pull request Sep 1, 2023
Ricardoalso pushed a commit to camptocamp/account-reconcile that referenced this pull request Sep 4, 2023
Ricardoalso pushed a commit to camptocamp/account-reconcile that referenced this pull request Sep 7, 2023
agyamuta pushed a commit to ursais/bank-payment that referenced this pull request Sep 11, 2023
They are ignored since OCA#979, and there can't be equivalent.
Ricardoalso pushed a commit to camptocamp/account-payment that referenced this pull request Nov 14, 2023
Ricardoalso pushed a commit to camptocamp/account-payment that referenced this pull request Nov 14, 2023
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.

None yet