[16.0][MIG] pos customer required: Migration to 16.0 #1423
Conversation
Currently translated at 100.0% (14 of 14 strings) Translation: pos-12.0/pos-12.0-pos_customer_required Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_customer_required/ca/
Currently translated at 100.0% (12 of 12 strings) Translation: pos-12.0/pos-12.0-pos_customer_required Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_customer_required/es/
Currently translated at 100.0% (15 of 15 strings) Translation: pos-14.0/pos-14.0-pos_customer_required Translate-URL: https://translation.odoo-community.org/projects/pos-14-0/pos-14-0-pos_customer_required/it/
mihien
force-pushed
the
16.0-mig-pos_customer_required
branch
from
4500b10 to
40ea630
Compare
huguesdk
left a comment
huguesdk
left a comment
There was a problem hiding this comment.
nice conversion to owl, @mihien! a few remarks, but mostly lgtm.
if i understand correctly, you added the improvement of preventing the user to close or confirm the partner selection screen if no partner has been selected, right? nice addition! does it also work from the payment screen when require_customer is set to payment (or should it not work at that point)?
There was a problem hiding this comment.
as the setting is now available from the general configuration, is it still needed here?
There was a problem hiding this comment.
Good question, it's probably not but I'll ask an analyst what he thinks
pos_customer_required/static/src/xml/Screens/PartnerListScreen/PartnerListScreen.xml
Show resolved
Hide resolved
pos_customer_required/static/src/js/Screens/ProductScreen/ProductScreen.esm.js
Outdated
Show resolved
Hide resolved
pos_customer_required/static/src/js/Screens/PartnerListScreen/PartnerListScreen.esm.js
Outdated
Show resolved
Hide resolved
|
|
||
| const PosCustomerRequiredPartnerListScreen = (OriginalPartnerListScreen) => | ||
| class extends OriginalPartnerListScreen { | ||
| clickPartner(partner) { |
There was a problem hiding this comment.
a comment would help to understand why this is needed. i looked at the original function and if i understand it correctly, the goal is to prevent to call confirm() if the partner is unselected, right? would it work by overriding confirm() instead to avoid re-implementing the logic?
There was a problem hiding this comment.
Both your assumptions are correct, I've moved the change to confirm() (and added a comment)
| if( this.env.pos.config.require_customer === "order" ) { | ||
| if (this.state.selectedPartner && this.state.selectedPartner.id === partner.id) { | ||
| this.state.selectedPartner = null; | ||
| this.render(true); |
There was a problem hiding this comment.
this should not be needed, should it?
There was a problem hiding this comment.
this.render(true); was needed to reload the screen to show the user had been unselected. But now that the logic is moved to confirm() this.state.selectedPartner = null; will be handled in clickPartner already, so I've removed it.
pos_customer_required/static/src/js/Screens/PaymentScreen/PaymentScreen.esm.js
Outdated
Show resolved
Hide resolved
mihien
force-pushed
the
16.0-mig-pos_customer_required
branch
from
40ea630 to
bf006cf
Compare
mihien
force-pushed
the
16.0-mig-pos_customer_required
branch
from
bf006cf to
37a84e9
Compare
|
@huguesdk I've applied all you suggestions and added a test to increase coverage. The only thing left is this remark:
It doesn't work when |
huguesdk
left a comment
huguesdk
left a comment
There was a problem hiding this comment.
i’ve just functionally tested the module, and the fact that it is not possible to close the partner selection screen is actually annoying and confusing (in my opinion), especially when the mode is not order: if one clicks on the customer selection button and doesn’t want to select a customer, the only way to go back is to select one, then re-opening the same screen and unselect the customer. i think that the button should stay, and a pop-up should appear in case a customer is mandatory.
an opinion from the users of the module in previous versions would be useful.
8 participants
No description provided.