[16.0][MIG] pos_require_product_quantity: Migration to 16.0 #1438
Conversation
Co-authored-by: Pierrick Brun <[email protected]>
…ctionpadWidget, causing issues with other modules
This 'good practice' improves compatibility with other modules.
This improves readability Also, avoid setting the click event handler if the option is disabled in pos.config
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: pos-14.0/pos-14.0-pos_require_product_quantity Translate-URL: https://translation.odoo-community.org/projects/pos-14-0/pos-14-0-pos_require_product_quantity/
polchampion
left a comment
polchampion
left a comment
There was a problem hiding this comment.
@mihien failed test on runboat : there are pos lines with quantity zero but no warning is shown
@polchampion You first have to check the option "Require Product Quantity" in the pos settings to activate it. The option was already in the settings in 12.0 but wasn't used in the module's logic. Whether you checked it or not, quantities of 0 used to result in a popup either way. If that's better, I could have it turned on by default when installing the module or if we want the same behaviour as in 12.0, I can remove the option from the settings entirely |
polchampion
left a comment
polchampion
left a comment
There was a problem hiding this comment.
@mihien sucessfully tested, can you add a "configuration" section in the readme so that it's clear how it needs to be set up ?
also, why put this in the session properties and not in the general pos settings? it seems not to respect the general logic
mihien
force-pushed
the
16.0-mig-pos_require_product_quantity
branch
from
b15ee4f to
9f89d78
Compare
@polchampion I've set the setting to true by default, so that there is no need for a configuration anymore and took the opportunity to move it to the general pos settings as well. |
perfect thank you! |
| class ResConfigSettings(models.TransientModel): | ||
| _inherit = "res.config.settings" | ||
|
|
||
| require_product_quantity = fields.Boolean( |
There was a problem hiding this comment.
the res.config.settings field should be prefixed with pos_.
| const PosRequireProductQuantityProductScreen = (OriginalProductScreen) => | ||
| class extends OriginalProductScreen { | ||
| async _onClickPay() { | ||
| const result = super._onClickPay(...arguments); |
There was a problem hiding this comment.
i think that this should be called after the rest of the checks, because it will try to switch to the payment screen. if a check fails, it is better to not run this code at all. this way, there is even no need to switch back to the ProductScreen in case of error.
There was a problem hiding this comment.
Would it be okay to do an empty return when a case fails and return the call to super otherwise?
There was a problem hiding this comment.
actually, the parent method doesn’t return anything, so yes, you can do an empty return (to exit the function) after showing the popup, and otherwise simply call super._onClickPay(...arguments); without returning its value.
mihien
force-pushed
the
16.0-mig-pos_require_product_quantity
branch
2 times, most recently
from
63df7e3 to
a49bf1a
Compare
mihien
force-pushed
the
16.0-mig-pos_require_product_quantity
branch
from
a49bf1a to
41f2d0b
Compare
|
This PR has the |
13 participants
Internal task T14562