[ADD] barcode_validation: Base barcode validation #607
Conversation
Abstract model to check barcode. MT-5927
| _( | ||
| "The barcode {barcode} is not a valid {barcode_type} barcode." | ||
| ).format(barcode=barcode, barcode_type=barcode_type) |
There was a problem hiding this comment.
issue: as a rule of thumb, don't use f-strings in translations.
Probably it's harmless in this case, but in other cases, like passing in a recordset as an argument, it could open a vulnerability door where you could hack odoo just through mis-using weblate.
Using %-format strings is always safe, and besides it's supported by _() directly:
| _( | |
| "The barcode {barcode} is not a valid {barcode_type} barcode." | |
| ).format(barcode=barcode, barcode_type=barcode_type) | |
| _( | |
| "The barcode %(barcode)s is not a valid %(barcode_type)s barcode." | |
| barcode=barcode, | |
| barcode_type=barcode_type, | |
| ) |
|
|
||
| If you need this module for those reasons, these might interest you too: | ||
|
|
||
| - product_varcode_validation |
There was a problem hiding this comment.
| - product_varcode_validation | |
| - product_barcode_validation |
| @@ -0,0 +1 @@ | |||
| This module extends the functionality of Odoo and to allow you validate barcode, so when print report with barcode, if it's illegal barcode for some barcode type, you will not have a ugly error. | |||
There was a problem hiding this comment.
| This module extends the functionality of Odoo and to allow you validate barcode, so when print report with barcode, if it's illegal barcode for some barcode type, you will not have a ugly error. | |
| This module extends the functionality of Odoo and to allow you validate barcode, so when print report with barcode, if it's illegal barcode for some barcode type, you will not have an ugly error. |
| @@ -0,0 +1,19 @@ | |||
| This module is an abstract module. You can configure barcode validation, but to enable this feature, you need to install an extra module for a given model. This repository provide 'product_barcode_validation' and 'stock_barcode_validation' to validate barcodes for products, stock lots, product packagings, ... | |||
There was a problem hiding this comment.
thought: is there really any use of barcode validation in a lower stack than the product module? Couldn't you just merge barcode_validation into product_barcode_validation and consider the latter one the base for others?
| class BarcodeValidation(models.AbstractModel): | ||
| _name = "barcode.validation.mixin" | ||
| _description = "Barcode Validation" | ||
|
|
There was a problem hiding this comment.
suggestion: I don't know if the answer to #607 (comment) will imply that you don't need a mixin anymore. But in case you still need it, I think the mixin is incomplete. I'd add this:
| _barcode_field = "barcode" # Override if needed when applying mixin | |
| @api.constrains(lambda self: (self._barcode_field,)) | |
| def _check_barcode(self): | |
| for record in self: | |
| record._validate_barcode(record[self._barcode_field]) | |
| @api.model |
| "QR": self.env.company.barcode_validation_qrcode, | ||
| } | ||
|
|
||
| def _validate_barcode(self, barcode): |
There was a problem hiding this comment.
| def _validate_barcode(self, barcode): | |
| @api.model | |
| def _validate_barcode(self, barcode): |
|
|
||
| name = fields.Char(required=True) | ||
| barcode = fields.Char(copy=False) | ||
|
|
||
| @api.constrains("barcode") | ||
| def check_barcode_validation(self): | ||
| for record in self.filtered("barcode"): | ||
| record._validate_barcode(record.barcode) |
There was a problem hiding this comment.
suggestion: following the other comments in this review, you won't need all this code.
| name = fields.Char(required=True) | |
| barcode = fields.Char(copy=False) | |
| @api.constrains("barcode") | |
| def check_barcode_validation(self): | |
| for record in self.filtered("barcode"): | |
| record._validate_barcode(record.barcode) |
| _description = "Barcode Validation" | ||
|
|
||
| def _get_validation_barcode_settings(self): | ||
| return { |
There was a problem hiding this comment.
IMHO, I think having these individual behaviours is not really useful. As you want to display a better error message than the Odoo one (for all barcodes).
IMHO, if you want to introduce other barcode types validation, why not overriding the barcode() function to validate it (and return a ValueError too) ?
There was a problem hiding this comment.
I think the point is also to display the error early: while entering the barcode. Otherwise, when you go to the warehouse and try to print, it becomes a bottleneck for the delivery process, because printing a big wave report with many barcodes won't tell which one fails.
There was a problem hiding this comment.
I have doubts in this module.
Maybe in some models you want to store EAN13 and other kind of barcodes on another.
Both should pass, but if you set up EAN13 validation and UPCA in packages, you couldn't check both of these.
I think this module should use the Nomenclature to check if the barcode fits with any rule.
barcode.nomenclature model has parse_barcode method that gives you information about what is this Barcode.
So I think getting the nomenclature (maybe in a manual form) and try to check what is going to discover Nomenclature when is scanned could be a great option too.
Another model with results could be great too to check what Barcodes has problems or its model doesn't fit the Nomenclature related object.
Odoo allows set illegal barcode for some barcode type but an ugly error can happen when you print report with this illegal barcode.
This a base module.
@yajo @Shide @rafaelbn please review
MT-5927 @moduon