pos
pos copied to clipboard
OCA
[16.0][MIG] pos_session_pay_invoice: Migration to version 16.0
Hi @carlos-lopez-tecnativa.
Thanks for porting this module tecnically, I don't understand why
- some button open a wizard that inherit from cash.pay.invoice,
- and in other button, it open a new model pos.box.cash.invoice.in / pos.box.cash.invoice.out.
could you elaborate ?
Hi @legalsylvain,
In this module, I have only migrated and made minimal changes for now. However, if you consider it necessary, I can refactor it similarly to this PR to inherit from the new wizard cash.pay.invoice for customer/vendor invoices instead of using two separate wizards.
Before the referenced PR, these wizards (cash.invoice.in, cash.invoice.out) inherited from cash.box.out, but as it no longer exists, I created a single, more generic wizard called cash.pay.invoice. In this module, the wizard that depends on cash.invoice.in is updated to inherit from cash.pay.invoice. However, the wizards pos.box.cash.invoice.out and pos.box.cash.invoice.in do not inherit from any, so I didn't touch them.
Let me know if the refactor is welcome.
Hi @carlos-lopez-tecnativa. Sorry. I didn't understood. I tested a little locally. Functionaly, why there are two different ways to do the same thing ?
Hi @carlos-lopez-tecnativa. Sorry. I didn't understood. I tested a little locally. Functionaly, why there are two different ways to do the same thing ?
@legalsylvain That was the behavior in the previous version; there were several wizards to do each thing. Now, based on my previous comment and the referenced PR(https://github.com/OCA/account-payment/pull/743), we could simplify this. Let me know if that works for you. @pedrobaeza what do you think?
Agree with Pedro. Let me know when you want review. Thanks for the work !
@pedrobaeza @legalsylvain I have updated the code to inherit functionality from the cash.pay.invoice wizard and removed the older wizards. Please review the changes and let me know if they work fine for you.
I don't understand why limiting the invoice payment to cash methods. Someone can come to your physical store to pay an invoice, and choose to pay with credit card. I think that limitation should be gone.
And anyway, even adding a cash payment method, I don't find the buttons referred in the README
And anyway, even adding a cash payment method, I don't find the buttons referred in the README
@pedrobaeza the buttons are here. Could you attach an image, please?
I was looking in the PoS session "frontend". In any moment the README mentions that you should go to the session backend form.
I don't understand why limiting the invoice payment to cash methods. Someone can come to your physical store to pay an invoice, and choose to pay with credit card. I think that limitation should be gone.
This behavior is from the previous version. Do you want us to change it to allow the user to select any payment method available on POS? (But only if the payment method has a journal set.) Please confirm so I can adapt the code.
OK, I see the functionality very limited having to go to backend, so let's not invest more time in this. Please clarify the usage README though.
OK, I see the functionality very limited having to go to backend, so let's not invest more time in this. Please clarify the usage README though.
Odoo, in the new version, has moved most functionalities to the frontend. This implies more effort to change it to the frontend. The task for now is to migrate the current module. I have updated the readme. Please let me know if it is clearer now. Thanks.
Hi @carlos-lopez-tecnativa. Thanks for the ping ! I just come back from hollidays today. I take a look this week.
thanks !
Hi @carlos-lopez-tecnativa. Thanks for the ping ! I just come back from hollidays today. I take a look this week.
thanks !
@legalsylvain I hope you had a great holiday! Would you kindly take another look when you have a chance? I'd appreciate any updates or feedback you may have.
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). 🤖
This PR looks fantastic, let's merge it! Prepared branch 16.0-ocabot-merge-pr-1226-by-pedrobaeza-bump-nobump, awaiting test results.
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1226-by-pedrobaeza-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.
On my way to merge this fine PR! Prepared branch 16.0-ocabot-merge-pr-1226-by-pedrobaeza-bump-nobump, awaiting test results.
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1226-by-pedrobaeza-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.
Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 16.0-ocabot-merge-pr-1226-by-legalsylvain-bump-nobump, awaiting test results.
I have no idea why the test failed. It works fine on my local environment. Does anyone have any ideas? The test that failed is in other modules.
@legalsylvain your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1226-by-legalsylvain-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.
I can see the error is general in the tour; other PRs have the same error in some OCA repos
https://github.com/OCA/calendar/actions/runs/10941425935/job/30376034658#step:8:159
https://github.com/OCA/pos/actions/runs/10921083459/job/30312371146#step:8:1086
https://github.com/OCA/e-commerce/actions/runs/10902558739/job/30254730888#step:8:306