[16.0][IMP] subscription_oca: recurrent payment#1331
[16.0][IMP] subscription_oca: recurrent payment#1331adasatorres wants to merge 1 commit intoOCA:16.0from
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.
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. |
|
This PR has the |
|
@carlos-domatix @carolina-domatix @tarteo would appreciate your reviews on this and also V17 + V18 here please: |
|
@yvaucher I wonder if you'd be so kind as to look at this and the related PRs for 17.0 and 18.0? |
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failure occurs because cls.cash_journal is assigned the first result from a search on account.journal with type = 'cash', but no such journal exists in the test environment, causing an IndexError: tuple index out of range.
2. Suggested Fix
In subscription_oca/tests/test_subscription_oca.py, line 29, replace:
cls.cash_journal = cls.env["account.journal"].search(
[
("type", "=", "cash"),
("company_id", "=", cls.env.ref("base.main_company").id),
]
)[0]with:
cls.cash_journal = cls.env["account.journal"].search(
[
("type", "=", "cash"),
("company_id", "=", cls.env.ref("base.main_company").id),
],
limit=1,
)
if not cls.cash_journal:
cls.cash_journal = cls.env["account.journal"].create({
"name": "Cash Journal",
"type": "cash",
"company_id": cls.env.ref("base.main_company").id,
"code": "CASH",
})3. Additional Code Issues
- Missing
company_idinpayment.tokencreation: Increate_payment()method (line 434),payment.tokencreation lackscompany_id, which may cause issues in multi-company setups. - Hardcoded
payment_typeincreate_payment(): Thepayment_typeis hardcoded to"inbound"; it should be configurable or derived from context, especially for different payment scenarios.
4. Test Improvements
Add a TransactionCase test to verify:
payment_token_idis correctly computed wheninvoicing_mode = 'invoice_and_payment'.create_payment()fails gracefully when no payment token exists.create_payment()fails gracefully when no payment method line is found.- Invoice and payment are created successfully when all conditions are met.
Use @tagged('post_install', 'manual') for tests that require manual setup or interaction with external systems (e.g., payment providers).
⏰ PR Aging Alert
This PR by @adasatorres has been open for 136 days (4 months).
Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b





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.