[18.0][MIG] sale_stock_picking_validation_blocking#4036
Conversation
63f6212 to
23129cf
Compare
|
/ocabot migration sale_stock_picking_validation_blocking |
| if picking.validation_blocked_by_so: | ||
| raise ValidationError( | ||
| _("Validation is blocked by SO for picking %s" % picking.name) | ||
| _(f"Validation is blocked by SO for picking {picking.name}") |
There was a problem hiding this comment.
You should not do that but using translation function arguments instead
alexey-pelykh
left a comment
There was a problem hiding this comment.
Thanks for the migration, Andrea.
I went through the full diff. The f-string issue that Denis flagged is the only blocker -- once that's fixed and the .pot is regenerated, this looks ready.
A few additional observations:
models/stock_picking.py -- As Denis noted, the _() call must use positional arguments for the translation system to work:
_("Validation is blocked by SO for picking %s", picking.name)The current f-string pattern means the extraction tool cannot match the runtime string to the .pot entry, so translations will silently fail.
i18n/*.pot -- The .pot file still has the %s placeholder form from 13.0/17.0, which is correct for the intended code but doesn't match the current f-string. After fixing the code, please regenerate to make sure everything is in sync.
models/sale_order.py -- Minor: the has_group() call inside _compute_hide_picking_validation_blocked doesn't depend on the record, only on the current user. You could hoist it before the loop for clarity:
is_manager = self.env.user.has_group("sales_team.group_sale_manager")
for rec in self:
rec.hide_button_picking_validation_blocked = (
rec.state != "sale" or not is_manager or rec.delivery_count == 0
)Not blocking, just a minor improvement.
Everything else (manifest, views, test coverage, pyproject.toml) looks good for an 18.0 migration. CI is green.
| for picking in self: | ||
| if picking.validation_blocked_by_so: | ||
| raise ValidationError( | ||
| _(f"Validation is blocked by SO for picking {picking.name}") |
There was a problem hiding this comment.
+1 to Denis's comment. This should be:
_("Validation is blocked by SO for picking %s", picking.name)f-strings inside _() bypass the translation extraction entirely -- the .pot entry won't match the runtime string, so all translations for this message will be silently ignored.
| def action_block_picking_validation(self): | ||
| self.write({"picking_validation_blocked": True}) | ||
|
|
||
| def action_unblock_picking_validation(self): |
There was a problem hiding this comment.
Nit: has_group() doesn't vary per record, so this could be hoisted before the loop to avoid repeated RPC/cache lookups on large recordsets:
is_manager = self.env.user.has_group("sales_team.group_sale_manager")
for rec in self:
rec.hide_button_picking_validation_blocked = (
rec.state != "sale" or not is_manager or rec.delivery_count == 0
)
No description provided.