From c533f59405d8e2d23eaec8ec4a6bbcfa0db48791 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Wed, 28 Oct 2020 23:33:33 +1100 Subject: [PATCH] Refactor how form errors are handled - Use form.add_error (as the django gods intended) --- InvenTree/InvenTree/views.py | 14 +++++++------- InvenTree/build/views.py | 28 +++++++++++----------------- InvenTree/order/views.py | 18 +++++++++--------- InvenTree/part/models.py | 19 ++++++++++++++++++- InvenTree/part/views.py | 17 +++++++++-------- InvenTree/stock/models.py | 9 +++------ InvenTree/stock/tests.py | 7 +++++++ InvenTree/stock/views.py | 33 ++++++++++++++++----------------- 8 files changed, 80 insertions(+), 65 deletions(-) diff --git a/InvenTree/InvenTree/views.py b/InvenTree/InvenTree/views.py index 0cb228836f..43ec16c795 100644 --- a/InvenTree/InvenTree/views.py +++ b/InvenTree/InvenTree/views.py @@ -485,7 +485,7 @@ class AjaxDeleteView(AjaxMixin, UpdateView): """ form_class = DeleteForm - ajax_form_title = "Delete Item" + ajax_form_title = _("Delete Item") ajax_template_name = "modal_delete_form.html" context_object_name = 'item' @@ -534,7 +534,7 @@ class AjaxDeleteView(AjaxMixin, UpdateView): if confirmed: obj.delete() else: - form.errors['confirm_delete'] = ['Check box to confirm item deletion'] + form.add_error('confirm_delete', _('Check box to confirm item deletion')) context[self.context_object_name] = self.get_object() data = { @@ -549,7 +549,7 @@ class EditUserView(AjaxUpdateView): """ View for editing user information """ ajax_template_name = "modal_form.html" - ajax_form_title = "Edit User Information" + ajax_form_title = _("Edit User Information") form_class = EditUserForm def get_object(self): @@ -560,7 +560,7 @@ class SetPasswordView(AjaxUpdateView): """ View for setting user password """ ajax_template_name = "InvenTree/password.html" - ajax_form_title = "Set Password" + ajax_form_title = _("Set Password") form_class = SetPasswordForm def get_object(self): @@ -579,9 +579,9 @@ class SetPasswordView(AjaxUpdateView): # Passwords must match if not p1 == p2: - error = 'Password fields must match' - form.errors['enter_password'] = [error] - form.errors['confirm_password'] = [error] + error = _('Password fields must match') + form.add_error('enter_password', error) + form.add_error('confirm_password', error) valid = False diff --git a/InvenTree/build/views.py b/InvenTree/build/views.py index 1296e42fae..dd6ad1a575 100644 --- a/InvenTree/build/views.py +++ b/InvenTree/build/views.py @@ -74,7 +74,7 @@ class BuildCancel(AjaxUpdateView): if confirm: build.cancelBuild(request.user) else: - form.errors['confirm_cancel'] = [_('Confirm build cancellation')] + form.add_error('confirm_cancel', _('Confirm build cancellation')) valid = False data = { @@ -128,8 +128,8 @@ class BuildAutoAllocate(AjaxUpdateView): valid = False if confirm is False: - form.errors['confirm'] = [_('Confirm stock allocation')] - form.non_field_errors = [_('Check the confirmation box at the bottom of the list')] + form.add_error('confirm', _('Confirm stock allocation')) + form.add_error(None, _('Check the confirmation box at the bottom of the list')) else: build.autoAllocate() valid = True @@ -163,8 +163,8 @@ class BuildUnallocate(AjaxUpdateView): valid = False if confirm is False: - form.errors['confirm'] = [_('Confirm unallocation of build stock')] - form.non_field_errors = [_('Check the confirmation box')] + form.add_error('confirm', _('Confirm unallocation of build stock')) + form.add_error(None, _('Check the confirmation box')) else: build.unallocateStock() valid = True @@ -266,15 +266,13 @@ class BuildComplete(AjaxUpdateView): valid = False if confirm is False: - form.errors['confirm'] = [ - _('Confirm completion of build'), - ] + form.add_error('confirm', _('Confirm completion of build')) else: try: location = StockLocation.objects.get(id=loc_id) valid = True except (ValueError, StockLocation.DoesNotExist): - form.errors['location'] = [_('Invalid location selected')] + form.add_error('location', _('Invalid location selected')) serials = [] @@ -291,24 +289,20 @@ class BuildComplete(AjaxUpdateView): # Exctract a list of provided serial numbers serials = ExtractSerialNumbers(sn, build.quantity) - existing = [] - - for serial in serials: - if build.part.checkIfSerialNumberExists(serial): - existing.append(serial) + existing = build.part.find_conflicting_serial_numbers(serials) if len(existing) > 0: exists = ",".join([str(x) for x in existing]) - form.errors['serial_numbers'] = [_('The following serial numbers already exist: ({sn})'.format(sn=exists))] + form.add_error('serial_numbers', _('The following serial numbers already exist: ({sn})'.format(sn=exists))) valid = False except ValidationError as e: - form.errors['serial_numbers'] = e.messages + form.add_error('serial_numbers', e.messages) valid = False if valid: if not build.completeBuild(location, serials, request.user): - form.non_field_errors = [('Build could not be completed')] + form.add_error(None, _('Build could not be completed')) valid = False data = { diff --git a/InvenTree/order/views.py b/InvenTree/order/views.py index 3a5bec3a7f..5d629df001 100644 --- a/InvenTree/order/views.py +++ b/InvenTree/order/views.py @@ -415,7 +415,7 @@ class PurchaseOrderCancel(AjaxUpdateView): valid = False if not confirm: - form.errors['confirm'] = [_('Confirm order cancellation')] + form.add_error('confirm', _('Confirm order cancellation')) else: valid = True @@ -448,13 +448,13 @@ class SalesOrderCancel(AjaxUpdateView): valid = False if not confirm: - form.errors['confirm'] = [_('Confirm order cancellation')] + form.add_error('confirm', _('Confirm order cancellation')) else: valid = True if valid: if not order.cancel_order(): - form.non_field_errors = [_('Could not cancel order')] + form.add_error(None, _('Could not cancel order')) valid = False data = { @@ -484,7 +484,7 @@ class PurchaseOrderIssue(AjaxUpdateView): valid = False if not confirm: - form.errors['confirm'] = [_('Confirm order placement')] + form.add_error('confirm', _('Confirm order placement')) else: valid = True @@ -558,13 +558,13 @@ class SalesOrderShip(AjaxUpdateView): valid = False if not confirm: - form.errors['confirm'] = [_('Confirm order shipment')] + form.add_error('confirm', _('Confirm order shipment')) else: valid = True if valid: if not order.ship_order(request.user): - form.non_field_errors = [_('Could not ship order')] + form.add_error(None, _('Could not ship order')) valid = False data = { @@ -1135,7 +1135,7 @@ class POLineItemCreate(AjaxCreateView): order = PurchaseOrder.objects.get(id=order_id) except (ValueError, PurchaseOrder.DoesNotExist): order = None - form.errors['order'] = [_('Invalid Purchase Order')] + form.add_error('order', _('Invalid Purchase Order')) valid = False try: @@ -1143,12 +1143,12 @@ class POLineItemCreate(AjaxCreateView): if order is not None: if not sp.supplier == order.supplier: - form.errors['part'] = [_('Supplier must match for Part and Order')] + form.add_error('part', _('Supplier must match for Part and Order')) valid = False except (SupplierPart.DoesNotExist, ValueError): valid = False - form.errors['part'] = [_('Invalid SupplierPart selection')] + form.add_error('part', _('Invalid SupplierPart selection')) data = { 'form_valid': valid, diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index a579547fba..beda7a0577 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -360,7 +360,7 @@ class Part(MPTTModel): # And recursively check too item.sub_part.checkAddToBOM(parent) - def checkIfSerialNumberExists(self, sn): + def checkIfSerialNumberExists(self, sn, exclude_self=False): """ Check if a serial number exists for this Part. @@ -369,10 +369,27 @@ class Part(MPTTModel): """ parts = Part.objects.filter(tree_id=self.tree_id) + stock = StockModels.StockItem.objects.filter(part__in=parts, serial=sn) + if exclude_self: + stock = stock.exclude(pk=self.pk) + return stock.exists() + def find_conflicting_serial_numbers(self, serials): + """ + For a provided list of serials, return a list of those which are conflicting. + """ + + conflicts = [] + + for serial in serials: + if self.checkIfSerialNumberExists(serial, exclude_self=True): + conflicts.append(serial) + + return conflicts + def getLatestSerialNumber(self): """ Return the "latest" serial number for this Part. diff --git a/InvenTree/part/views.py b/InvenTree/part/views.py index 57a274b5bf..2347020c55 100644 --- a/InvenTree/part/views.py +++ b/InvenTree/part/views.py @@ -446,9 +446,9 @@ class PartDuplicate(AjaxCreateView): confirmed = str2bool(request.POST.get('confirm_creation', False)) if not confirmed: - form.errors['confirm_creation'] = ['Possible matches exist - confirm creation of new part'] - - form.pre_form_warning = 'Possible matches exist - confirm creation of new part' + msg = _('Possible matches exist - confirm creation of new part') + form.add_error('confirm_creation', msg) + form.pre_form_warning = msg valid = False data = { @@ -576,9 +576,10 @@ class PartCreate(AjaxCreateView): confirmed = str2bool(request.POST.get('confirm_creation', False)) if not confirmed: - form.errors['confirm_creation'] = ['Possible matches exist - confirm creation of new part'] + msg = _('Possible matches exist - confirm creation of new part') + form.add_error('confirm_creation', msg) - form.pre_form_warning = 'Possible matches exist - confirm creation of new part' + form.pre_form_warning = msg valid = False data = { @@ -915,7 +916,7 @@ class BomValidate(AjaxUpdateView): if confirmed: part.validate_bom(request.user) else: - form.errors['validate'] = ['Confirm that the BOM is valid'] + form.add_error('validate', _('Confirm that the BOM is valid')) data = { 'form_valid': confirmed @@ -1054,7 +1055,7 @@ class BomUpload(InvenTreeRoleMixin, FormView): bom_file_valid = False if bom_file is None: - self.form.errors['bom_file'] = [_('No BOM file provided')] + self.form.add_error('bom_file', _('No BOM file provided')) else: # Create a BomUploadManager object - will perform initial data validation # (and raise a ValidationError if there is something wrong with the file) @@ -1065,7 +1066,7 @@ class BomUpload(InvenTreeRoleMixin, FormView): errors = e.error_dict for k, v in errors.items(): - self.form.errors[k] = v + self.form.add_error(k, v) if bom_file_valid: # BOM file is valid? Proceed to the next step! diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py index 1535ded420..e4c6d67cb2 100644 --- a/InvenTree/stock/models.py +++ b/InvenTree/stock/models.py @@ -814,14 +814,11 @@ class StockItem(MPTTModel): raise ValidationError({"quantity": _("Quantity does not match serial numbers")}) # Test if each of the serial numbers are valid - existing = [] - - for serial in serials: - if self.part.checkIfSerialNumberExists(serial): - existing.append(serial) + existing = self.part.find_conflicting_serial_numbers(serials) if len(existing) > 0: - raise ValidationError({"serial_numbers": _("Serial numbers already exist: ") + str(existing)}) + exists = ','.join([str(x) for x in existing]) + raise ValidationError({"serial_numbers": _("Serial numbers already exist") + ': ' + exists}) # Create a new stock item for each unique serial number for serial in serials: diff --git a/InvenTree/stock/tests.py b/InvenTree/stock/tests.py index 34fe8877f8..4632505831 100644 --- a/InvenTree/stock/tests.py +++ b/InvenTree/stock/tests.py @@ -407,6 +407,13 @@ class VariantTest(StockTest): self.assertEqual(chair.getLatestSerialNumber(), '22') + # Check for conflicting serial numbers + to_check = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + + conflicts = chair.find_conflicting_serial_numbers(to_check) + + self.assertEqual(len(conflicts), 6) + # Same operations on a sub-item variant = Part.objects.get(pk=10003) self.assertEqual(variant.getLatestSerialNumber(), '22') diff --git a/InvenTree/stock/views.py b/InvenTree/stock/views.py index 45c862da83..09c2d4b159 100644 --- a/InvenTree/stock/views.py +++ b/InvenTree/stock/views.py @@ -417,8 +417,8 @@ class StockItemDeleteTestData(AjaxUpdateView): confirm = str2bool(request.POST.get('confirm', False)) if confirm is not True: - form.errors['confirm'] = [_('Confirm test data deletion')] - form.non_field_errors = [_('Check the confirmation box')] + form.add_error('confirm', _('Confirm test data deletion')) + form.add_error(None, _('Check the confirmation box')) else: stock_item.test_results.all().delete() valid = True @@ -918,7 +918,7 @@ class StockItemUninstall(AjaxView, FormMixin): if not confirmed: valid = False - form.errors['confirm'] = [_('Confirm stock adjustment')] + form.add_error('confirm', _('Confirm stock adjustment')) data = { 'form_valid': valid, @@ -1116,7 +1116,7 @@ class StockAdjust(AjaxView, FormMixin): if not confirmed: valid = False - form.errors['confirm'] = [_('Confirm stock adjustment')] + form.add_error('confirm', _('Confirm stock adjustment')) data = { 'form_valid': valid, @@ -1416,7 +1416,7 @@ class StockItemSerialize(AjaxUpdateView): try: numbers = ExtractSerialNumbers(serials, quantity) except ValidationError as e: - form.errors['serial_numbers'] = e.messages + form.add_error('serial_numbers', e.messages) valid = False numbers = [] @@ -1428,9 +1428,9 @@ class StockItemSerialize(AjaxUpdateView): for k in messages.keys(): if k in ['quantity', 'destination', 'serial_numbers']: - form.errors[k] = messages[k] + form.add_error(k, messages[k]) else: - form.non_field_errors = [messages[k]] + form.add_error(None, messages[k]) valid = False @@ -1622,14 +1622,14 @@ class StockItemCreate(AjaxCreateView): part = None quantity = 1 valid = False - form.errors['quantity'] = [_('Invalid quantity')] + form.add_error('quantity', _('Invalid quantity')) if quantity < 0: - form.errors['quantity'] = [_('Quantity cannot be less than zero')] + form.add_error('quantity', _('Quantity cannot be less than zero')) valid = False if part is None: - form.errors['part'] = [_('Invalid part selection')] + form.add_error('part', _('Invalid part selection')) else: # A trackable part must provide serial numbesr if part.trackable: @@ -1642,15 +1642,14 @@ class StockItemCreate(AjaxCreateView): try: serials = ExtractSerialNumbers(sn, quantity) - existing = [] - - for serial in serials: - if part.checkIfSerialNumberExists(serial): - existing.append(serial) + existing = part.find_conflicting_serial_numbers(serials) if len(existing) > 0: exists = ",".join([str(x) for x in existing]) - form.errors['serial_numbers'] = [_('The following serial numbers already exist: ({sn})'.format(sn=exists))] + form.add_error( + 'serial_numbers', + _('Serial numbers already exist') + ': ' + exists + ) valid = False else: @@ -1682,7 +1681,7 @@ class StockItemCreate(AjaxCreateView): valid = True except ValidationError as e: - form.errors['serial_numbers'] = e.messages + form.add_error('serial_numbers', e.messages) valid = False else: