[16.0][IMP] check quantity of share is not below minimum #174
Conversation
mihien
force-pushed
the
16.0-fix-cooperator_website_minimal_qty
branch
2 times, most recently
from
5b9bbed to
99979c5
Compare
| _( | ||
| "The number of shares must be more than the defined minimum " | ||
| f"quantity of this share. For {sub_req.share_product_id.short_name}," | ||
| " the minimum is" | ||
| f"{sub_req.share_product_id.product_tmpl_id.minimum_quantity}." | ||
| ) |
There was a problem hiding this comment.
f-strings prevent localization (as they would change the source string). the substitution must be done after the translation.
| _( | |
| "The number of shares must be more than the defined minimum " | |
| f"quantity of this share. For {sub_req.share_product_id.short_name}," | |
| " the minimum is" | |
| f"{sub_req.share_product_id.product_tmpl_id.minimum_quantity}." | |
| ) | |
| _( | |
| "The number of shares must be more than the defined minimum " | |
| "quantity of this share. For {share_type}, the minimum is " | |
| "{minimum_quantity}." | |
| ).format( | |
| share_type=sub_req.share_product_id.short_name, | |
| minimum_quantity=sub_req.share_product_id.product_tmpl_id.minimum_quantity | |
| ) |
| _( | ||
| "Number of shares must be greater than " | ||
| f"{self.share_product_id.product_tmpl_id.minimum_quantity}" | ||
| ) |
There was a problem hiding this comment.
same remarks about f-strings as below.
| if share_product["force_min_qty"] and qty_share <= share_product["min_qty"]: | ||
| values = self.fill_values(values, is_company, logged) | ||
| values["error_msg"] = _( | ||
| "Number of share must be greater than {min_qty}" |
There was a problem hiding this comment.
| "Number of share must be greater than {min_qty}" | |
| "Number of shares must be greater than {min_qty}." |
| # check subscription respect min qty of shares | ||
| share_product_id = int(kwargs.get("share_product_id")) | ||
| share_product = self.get_share_product(share_product_id)[share_product_id] | ||
| qty_share = int(kwargs.get("ordered_parts")) | ||
| if share_product["force_min_qty"] and qty_share <= share_product["min_qty"]: |
There was a problem hiding this comment.
this logic should be better moved to a separate method to keep this function as short as possible (even if it is already much too long) and to make it clear what is checked here. something like:
if not validate_minimum_quantity(kwargs):| $("#ordered_parts").change(); | ||
| var $share_price = $("#share_price").text(); | ||
| $('input[name="total_parts"]').val( | ||
| $("#ordered_parts").val() * $share_price | ||
| ); | ||
| $('input[name="total_parts"]').change(); |
There was a problem hiding this comment.
why were there calls to .change() previously and not anymore?
There was a problem hiding this comment.
I'm not exactly sure why they were there, but I've tested with and without, and I end up with the same result. I think it might be because the use of .data() requires a call to .change() to be applied whereas .val() doesn't, but I'm not sure.
There was a problem hiding this comment.
adding a small (python) test to cover these 2 cases would be nice.
mihien
force-pushed
the
16.0-fix-cooperator_website_minimal_qty
branch
from
99979c5 to
14bf8cb
Compare
|
@huguesdk thanks for the review ! I've made the suggested changes plus a fair amount more for the tests |
There was a problem hiding this comment.
@mihien unsuccessful test on the runboat:
yet all fields are filled
perhaps a missing config in the demo data?
|
@polchampion The reason is probably because there is no localization module installed, in which case it will always fail because there is nothing handling the iban field of the form. Can you try again with l10n_be_cooperator installed? (Just to clarify, this is the current behaviour of cooperator, it hasn't been introduced by this PR) |
mihien
force-pushed
the
16.0-fix-cooperator_website_minimal_qty
branch
from
14bf8cb to
f0f43d7
Compare
polchampion
left a comment
polchampion
left a comment
There was a problem hiding this comment.
successful functional test on runboat
3 participants
No description provided.