Mirrored_Repos/InvenTree
1
0
Fork 0
mirror of https://github.com/inventree/InvenTree synced 2024-08-30 18:33:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refactor how form errors are handled

Browse Source 
- When in doubt, refer to the django docs
- There was a *much* better way (thanks, form.add_error)!
- The anti-pattern was deleted, and lo, there was much rejoicing
- Some other refactoring too
This commit is contained in:
Oliver Walters 2020-10-28 23:33:33 +11:00
parent d06b4d7c9f
commit 091a9d9803
8 changed files with 112 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
InvenTree/InvenTree/views.py
View File

@ -321,14 +321,14 @@ class AjaxCreateView(AjaxMixin, CreateView):
- Handles form validation via AJAX POST requests
"""
def validate(self, cleaned_data, **kwargs):
def validate(self, request, form, cleaned_data, **kwargs):
"""
Hook for performing any extra validation, over and above the regular form.is_valid
If any errors exist, raise a ValidationError
If any errors exist, add them to the form, using form.add_error
"""
return True
pass
def pre_save(self, form, request, **kwargs):
"""
@ -370,13 +370,10 @@ class AjaxCreateView(AjaxMixin, CreateView):
valid = self.form.is_valid()
# Perform any extra validation steps
if valid:
try:
valid = valid and self.validate(self.form.cleaned_data)
except ValidationError as e:
valid = False
self.form.add_error(None, e)
self.validate(request, self.form, self.form.cleaned_data)
# Check if form is valid again (after performing any custom validation)
valid = self.form.is_valid()
# Extra JSON data sent alongside form
data = {
@ -467,7 +464,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'
@ -516,7 +513,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 = {
@ -531,7 +528,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):
@ -542,7 +539,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):
@ -561,9 +558,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

65
InvenTree/build/views.py
View File

@ -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 = {
@ -152,8 +152,8 @@ class BuildAutoAllocate(AjaxUpdateView):
build.auto_allocate(output)
valid = True
else:
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'))
data = {
'form_valid': valid,
@ -205,10 +205,10 @@ class BuildOutputDelete(AjaxUpdateView):
build.deleteBuildOutput(output)
valid = True
else:
form.non_field_errors = [_('Build or output not specified')]
form.add_error(None, _('Build or output not specified'))
else:
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'))
data = {
'form_valid': valid,
@ -271,8 +271,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(output=output, part=part)
valid = True
@ -374,15 +374,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 = []
@ -399,24 +397,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 = {
@ -560,7 +554,7 @@ class BuildCreate(AjaxCreateView):
'success': _('Created new build'),
}
def validate(self, cleaned_data, **kwargs):
def validate(self, request, form, cleaned_data, **kwargs):
"""
Perform extra form validation.
@ -574,18 +568,29 @@ class BuildCreate(AjaxCreateView):
if part.trackable:
# For a trackable part, either batch or serial nubmber must be specified
if not cleaned_data['serial_numbers']:
raise ValidationError({
'serial_numbers': _('Trackable part must have serial numbers specified')
})
form.add_error('serial_numbers', _('Trackable part must have serial numbers specified'))
else:
# If serial numbers are set...
serials = cleaned_data['serial_numbers']
quantity = cleaned_data['quantity']
# If serial numbers are set...
serials = cleaned_data['serial_numbers']
quantity = cleaned_data['quantity']
# Check that the provided serial numbers are sensible
try:
extracted = ExtractSerialNumbers(serials, quantity)
except ValidationError as e:
extracted = None
form.add_error('serial_numbers', e.messages)
extracted = ExtractSerialNumbers(serials, quantity)
if extracted:
# Check that the provided serial numbers are not duplicates
conflicts = part.find_conflicting_serial_numbers(extracted)
# Ok, no errors...
return True
if len(conflicts) > 0:
msg = ",".join([str(c) for c in conflicts])
form.add_error(
'serial_numbers',
_('Serial numbers already exist') + ': ' + msg
)
def post_save(self, **kwargs):
"""

18
InvenTree/order/views.py
View File

@ -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,

20
InvenTree/part/models.py
View File

@ -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,28 @@ 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.

17
InvenTree/part/views.py
View File

@ -445,9 +445,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 = {
@ -575,9 +575,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 = {
@ -862,7 +863,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
@ -1001,7 +1002,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)
@ -1012,7 +1013,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!

9
InvenTree/stock/models.py
View File

@ -822,14 +822,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:

7
InvenTree/stock/tests.py
View File

@ -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')

33
InvenTree/stock/views.py
View File

@ -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
@ -1621,14 +1621,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:
@ -1641,15 +1641,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(serial)
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:
@ -1681,7 +1680,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: