-
-
Notifications
You must be signed in to change notification settings - Fork 586
[16.0][IMP] subscription_oca: recurrent payment #1331
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
base: 16.0
Are you sure you want to change the base?
Conversation
9e7db3c to
62928eb
Compare
|
@adasatorres I did a quick test on runbot.
|
|
Hi @chrisandrewmann, I followed the steps you mentioned to try to reproduce the error, but it doesn’t generate any error for me. I’m attaching some screenshots:
|
@adasatorres Sorry, not sure then. |
@chrisandrewmann I’ve been reviewing the base runboat instance, and it seems that the error you were getting was caused by an issue when generating the invoice. I created a new subscription using the same template, and I also installed the demo provider, which wasn’t configured. Once everything was set up, I triggered the scheduled action button, and it generated all the invoices for the subscriptions. |
|
@adasatorres thanks, that's good to know! Did you get around to comparing with my own V18 version of this feature? |
|
Hi @chrisandrewmann, I think the PR in its current state is the most streamlined option available. Personally, I would leave it as it is. I’d like to know your opinion and whether you would change anything, etc. |
Hi @adasatorres My queries would be:
Suggestions:
|
rrebollo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review complete. LGTM!
Please consider @chrisandrewmann's suggestions — they make sense and would improve the implementation.
|
Hi @chrisandrewmann, I’m implementing the changes you mentioned, but the field property_inbound_payment_method_line_id does not exist in versions 16 and 17. |
Ok in that case maybe best to use your existing logic as a fallback instead, only if the payment_token_id is not manually set on subscription? |
ed63f08 to
3e1800d
Compare
|
Hi, @chrisandrewmann, could you review it again? I’ve implemented the two functionalities you mentioned, ignoring the issue with the res.partner field, which we can implement in Odoo 18+ |
Thanks @adasatorres, i've just done a quick test and generally looks good. A few issues noted. Test 1 steps
Test 2 steps
|
3e1800d to
e4962ff
Compare
|
Good morning @chrisandrewmann , the issues you mentioned should already be resolved. Right now, if the partner doesn’t have a payment token to use, the invoice is sent for them to pay manually. If they do have a payment token, the invoice is sent but it already appears as paid. |
Hi @adasatorres thanks a lot for this. Just tested and looks great to me. Something I think we should add for future though, is that currently the recurring payment only works for invoices. |
chrisandrewmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
This PR has the |
|
@carlos-domatix @carolina-domatix @tarteo would appreciate your reviews on this and also V17 + V18 here please: |





This implementation adds an option in the template to create the invoice and automatically create the payment, using the last payment token used by the customer.