From c533f59405d8e2d23eaec8ec4a6bbcfa0db48791 Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@gmail.com>
Date: Wed, 28 Oct 2020 23:33:33 +1100
Subject: [PATCH 1/3] 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:

From e049ca1a8521c70ec5d1ed4f76ae3fe92f1fe944 Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@gmail.com>
Date: Fri, 30 Oct 2020 16:54:05 +1100
Subject: [PATCH 2/3] More refactoring

---
 InvenTree/InvenTree/views.py |  34 +++++++-
 InvenTree/build/views.py     |  29 +++----
 InvenTree/order/models.py    |  22 ++++-
 InvenTree/order/views.py     | 150 ++++++++++-------------------------
 InvenTree/part/models.py     |  36 ++++++---
 InvenTree/part/views.py      |  24 +++---
 InvenTree/stock/models.py    |  14 ++++
 InvenTree/stock/views.py     |  63 +++++++--------
 8 files changed, 188 insertions(+), 184 deletions(-)

diff --git a/InvenTree/InvenTree/views.py b/InvenTree/InvenTree/views.py
index 43ec16c795..57f80f1be7 100644
--- a/InvenTree/InvenTree/views.py
+++ b/InvenTree/InvenTree/views.py
@@ -362,6 +362,17 @@ class AjaxCreateView(AjaxMixin, CreateView):
         form = self.get_form()
         return self.renderJsonResponse(request, form)
 
+    def do_save(self, form):
+        """
+        Method for actually saving the form to the database.
+        Default implementation is very simple,
+        but can be overridden if required.
+        """
+
+        self.object = form.save()
+
+        return self.object
+
     def post(self, request, *args, **kwargs):
         """ Responds to form POST. Validates POST data and returns status info.
 
@@ -385,13 +396,17 @@ class AjaxCreateView(AjaxMixin, CreateView):
             'form_valid': valid
         }
 
+        # Add in any extra class data
+        for value, key in enumerate(self.get_data()):
+            data[key] = value
+
         if valid:
 
             # Perform (optional) pre-save step
             self.pre_save(None, self.form)
 
             # Save the object to the database
-            self.object = self.form.save()
+            self.do_save(self.form)
 
             # Perform (optional) post-save step
             self.post_save(self.object, self.form)
@@ -425,6 +440,17 @@ class AjaxUpdateView(AjaxMixin, UpdateView):
         
         return self.renderJsonResponse(request, self.get_form(), context=self.get_context_data())
 
+    def do_save(self, form):
+        """
+        Method for updating the object in the database.
+        Default implementation is very simple,
+        but can be overridden if required.
+        """
+
+        self.object = form.save()
+
+        return self.object
+
     def post(self, request, *args, **kwargs):
         """ Respond to POST request.
 
@@ -453,13 +479,17 @@ class AjaxUpdateView(AjaxMixin, UpdateView):
             'form_valid': valid
         }
 
+        # Add in any extra class data
+        for value, key in enumerate(self.get_data()):
+            data[key] = value
+
         if valid:
 
             # Perform (optional) pre-save step
             self.pre_save(self.object, form)
 
             # Save the updated objec to the database
-            obj = form.save()
+            obj = self.do_save(form)
 
             # Perform (optional) post-save step
             self.post_save(obj, form)
diff --git a/InvenTree/build/views.py b/InvenTree/build/views.py
index dd6ad1a575..1a2bd0d4b7 100644
--- a/InvenTree/build/views.py
+++ b/InvenTree/build/views.py
@@ -60,30 +60,25 @@ class BuildCancel(AjaxUpdateView):
     form_class = forms.CancelBuildForm
     role_required = 'build.change'
 
-    def post(self, request, *args, **kwargs):
-        """ Handle POST request. Mark the build status as CANCELLED """
+    def validate(self, build, form, **kwargs):
 
-        build = self.get_object()
+        confirm = str2bool(form.cleaned_data.get('confirm_cancel', False))
 
-        form = self.get_form()
-
-        valid = form.is_valid()
-
-        confirm = str2bool(request.POST.get('confirm_cancel', False))
-
-        if confirm:
-            build.cancelBuild(request.user)
-        else:
+        if not confirm:
             form.add_error('confirm_cancel', _('Confirm build cancellation'))
-            valid = False
 
-        data = {
-            'form_valid': valid,
+    def post_save(self, build, form, **kwargs):
+        """
+        Cancel the build.
+        """
+
+        build.cancelBuild(self.request.user)
+
+    def get_data(self):
+        return {
             'danger': _('Build was cancelled')
         }
 
-        return self.renderJsonResponse(request, form, data=data)
-
 
 class BuildAutoAllocate(AjaxUpdateView):
     """ View to auto-allocate parts for a build.
diff --git a/InvenTree/order/models.py b/InvenTree/order/models.py
index 7d561b95ba..b193454e1f 100644
--- a/InvenTree/order/models.py
+++ b/InvenTree/order/models.py
@@ -209,6 +209,7 @@ class PurchaseOrder(Order):
 
         line.save()
 
+    @transaction.atomic
     def place_order(self):
         """ Marks the PurchaseOrder as PLACED. Order must be currently PENDING. """
 
@@ -217,6 +218,7 @@ class PurchaseOrder(Order):
             self.issue_date = datetime.now().date()
             self.save()
 
+    @transaction.atomic
     def complete_order(self):
         """ Marks the PurchaseOrder as COMPLETE. Order must be currently PLACED. """
 
@@ -225,10 +227,16 @@ class PurchaseOrder(Order):
             self.complete_date = datetime.now().date()
             self.save()
 
+    def can_cancel(self):
+        return self.status not in [
+            PurchaseOrderStatus.PLACED,
+            PurchaseOrderStatus.PENDING
+        ]
+
     def cancel_order(self):
         """ Marks the PurchaseOrder as CANCELLED. """
 
-        if self.status in [PurchaseOrderStatus.PLACED, PurchaseOrderStatus.PENDING]:
+        if self.can_cancel():
             self.status = PurchaseOrderStatus.CANCELLED
             self.save()
 
@@ -377,6 +385,16 @@ class SalesOrder(Order):
 
         return True
 
+    def can_cancel(self):
+        """
+        Return True if this order can be cancelled
+        """
+
+        if not self.status == SalesOrderStatus.PENDING:
+            return False
+
+        return True
+
     @transaction.atomic
     def cancel_order(self):
         """
@@ -386,7 +404,7 @@ class SalesOrder(Order):
         - Delete any StockItems which have been allocated
         """
 
-        if not self.status == SalesOrderStatus.PENDING:
+        if not self.can_cancel():
             return False
 
         self.status = SalesOrderStatus.CANCELLED
diff --git a/InvenTree/order/views.py b/InvenTree/order/views.py
index 5d629df001..333509952b 100644
--- a/InvenTree/order/views.py
+++ b/InvenTree/order/views.py
@@ -404,29 +404,19 @@ class PurchaseOrderCancel(AjaxUpdateView):
     form_class = order_forms.CancelPurchaseOrderForm
     role_required = 'purchase_order.change'
 
-    def post(self, request, *args, **kwargs):
-        """ Mark the PO as 'CANCELLED' """
-
-        order = self.get_object()
-        form = self.get_form()
-
-        confirm = str2bool(request.POST.get('confirm', False))
-
-        valid = False
+    def validate(self, order, form, **kwargs):
+        
+        confirm = str2bool(form.cleaned_data.get('confirm', False))
 
         if not confirm:
             form.add_error('confirm', _('Confirm order cancellation'))
-        else:
-            valid = True
 
-        data = {
-            'form_valid': valid
-        }
+        if not order.can_cancel():
+            form.add_error(None, _('Order cannot be cancelled'))
 
-        if valid:
-            order.cancel_order()
+    def post_save(self, order, form, **kwargs):
 
-        return self.renderJsonResponse(request, form, data)
+        order.cancel_order()
 
 
 class SalesOrderCancel(AjaxUpdateView):
@@ -438,30 +428,19 @@ class SalesOrderCancel(AjaxUpdateView):
     form_class = order_forms.CancelSalesOrderForm
     role_required = 'sales_order.change'
 
-    def post(self, request, *args, **kwargs):
+    def validate(self, order, form, **kwargs):
 
-        order = self.get_object()
-        form = self.get_form()
-
-        confirm = str2bool(request.POST.get('confirm', False))
-
-        valid = False
+        confirm = str2bool(form.cleaned_data.get('confirm', False))
 
         if not confirm:
             form.add_error('confirm', _('Confirm order cancellation'))
-        else:
-            valid = True
 
-        if valid:
-            if not order.cancel_order():
-                form.add_error(None, _('Could not cancel order'))
-                valid = False
+        if not order.can_cancel():
+            form.add_error(None, _('Order cannot be cancelled'))
 
-        data = {
-            'form_valid': valid,
-        }
+    def post_save(self, order, form, **kwargs):
 
-        return self.renderJsonResponse(request, form, data)
+        order.cancel_order()
 
 
 class PurchaseOrderIssue(AjaxUpdateView):
@@ -473,30 +452,22 @@ class PurchaseOrderIssue(AjaxUpdateView):
     form_class = order_forms.IssuePurchaseOrderForm
     role_required = 'purchase_order.change'
 
-    def post(self, request, *args, **kwargs):
-        """ Mark the purchase order as 'PLACED' """
+    def validate(self, order, form, **kwargs):
 
-        order = self.get_object()
-        form = self.get_form()
-
-        confirm = str2bool(request.POST.get('confirm', False))
-
-        valid = False
+        confirm = str2bool(form.cleaned_data.get('confirm', False))
 
         if not confirm:
             form.add_error('confirm', _('Confirm order placement'))
-        else:
-            valid = True
 
-        data = {
-            'form_valid': valid,
+    def post_save(self, order, form, **kwargs):
+
+        order.place_order()
+
+    def get_data(self):
+        return {
+            'success': _('Purchase order issued')
         }
 
-        if valid:
-            order.place_order()
-
-        return self.renderJsonResponse(request, form, data)
-
 
 class PurchaseOrderComplete(AjaxUpdateView):
     """ View for marking a PurchaseOrder as complete.
@@ -517,24 +488,22 @@ class PurchaseOrderComplete(AjaxUpdateView):
 
         return ctx
 
-    def post(self, request, *args, **kwargs):
+    def validate(self, order, form, **kwargs):
 
-        confirm = str2bool(request.POST.get('confirm', False))
+        confirm = str2bool(form.cleaned_data.get('confirm', False))
 
-        if confirm:
-            po = self.get_object()
-            po.status = PurchaseOrderStatus.COMPLETE
-            po.save()
+        if not confirm:
+            form.add_error('confirm', _('Confirm order completion'))
 
-        data = {
-            'form_valid': confirm
+    def post_save(self, order, form, **kwargs):
+
+        order.complete_order()
+
+    def get_data(self):
+        return {
+            'success': _('Purchase order completed')
         }
 
-        form = self.get_form()
-
-        return self.renderJsonResponse(request, form, data)
-
-
 class SalesOrderShip(AjaxUpdateView):
     """ View for 'shipping' a SalesOrder """
     form_class = order_forms.ShipSalesOrderForm
@@ -1117,52 +1086,21 @@ class POLineItemCreate(AjaxCreateView):
     ajax_form_title = _('Add Line Item')
     role_required = 'purchase_order.add'
 
-    def post(self, request, *arg, **kwargs):
+    def validate(self, item, form, **kwargs):
 
-        self.request = request
+        order = form.cleaned_data.get('order', None)
 
-        form = self.get_form()
+        part = form.cleaned_data.get('part', None)
 
-        valid = form.is_valid()
+        if not part:
+            form.add_error('part', _('Supplier part must be specified'))
 
-        # Extract the SupplierPart ID from the form
-        part_id = form['part'].value()
-
-        # Extract the Order ID from the form
-        order_id = form['order'].value()
-
-        try:
-            order = PurchaseOrder.objects.get(id=order_id)
-        except (ValueError, PurchaseOrder.DoesNotExist):
-            order = None
-            form.add_error('order', _('Invalid Purchase Order'))
-            valid = False
-
-        try:
-            sp = SupplierPart.objects.get(id=part_id)
-
-            if order is not None:
-                if not sp.supplier == order.supplier:
-                    form.add_error('part', _('Supplier must match for Part and Order'))
-                    valid = False
-
-        except (SupplierPart.DoesNotExist, ValueError):
-            valid = False
-            form.add_error('part', _('Invalid SupplierPart selection'))
-
-        data = {
-            'form_valid': valid,
-        }
-
-        if valid:
-            self.object = form.save()
-
-            data['pk'] = self.object.pk
-            data['text'] = str(self.object)
-        else:
-            self.object = None
-        
-        return self.renderJsonResponse(request, form, data,)
+        if part and order:
+            if not part.supplier == order.supplier:
+                form.add_error(
+                    'part',
+                    _('Supplier must match for Part and Order')
+                )
 
     def get_form(self):
         """ Limit choice options based on the selected order, etc
diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py
index beda7a0577..34beb4b68e 100644
--- a/InvenTree/part/models.py
+++ b/InvenTree/part/models.py
@@ -1133,6 +1133,31 @@ class Part(MPTTModel):
 
             bom_item.save()
 
+    @transaction.atomic
+    def copy_parameters_from(self, other, **kwargs):
+        
+        clear = kwargs.get('clear', True)
+
+        if clear:
+            self.get_parameters().delete()
+
+        for parameter in other.get_parameters.all():
+
+            # If this part already has a parameter pointing to the same template,
+            # delete that parameter from this part first!
+
+            try:
+                existing = PartParameter.objects.get(part=self, template=parameter.template)
+                existing.delete()
+            except (PartParameter.DoesNotExist):
+                pass
+
+            parameter.part = self
+            parameter.pk = None
+
+            parameter.save()
+
+    @transaction.atomic
     def deepCopy(self, other, **kwargs):
         """ Duplicates non-field data from another part.
         Does not alter the normal fields of this part,
@@ -1156,15 +1181,8 @@ class Part(MPTTModel):
 
         # Copy the parameters data
         if kwargs.get('parameters', True):
-            # Get template part parameters
-            parameters = other.get_parameters()
-            # Copy template part parameters to new variant part
-            for parameter in parameters:
-                PartParameter.create(part=self,
-                                     template=parameter.template,
-                                     data=parameter.data,
-                                     save=True)
-
+            self.copy_parameters_from(other)
+        
         # Copy the fields that aren't available in the duplicate form
         self.salable = other.salable
         self.assembly = other.assembly
diff --git a/InvenTree/part/views.py b/InvenTree/part/views.py
index 2347020c55..2dd3a47c3f 100644
--- a/InvenTree/part/views.py
+++ b/InvenTree/part/views.py
@@ -371,6 +371,8 @@ class MakePartVariant(AjaxCreateView):
         initials = model_to_dict(part_template)
         initials['is_template'] = False
         initials['variant_of'] = part_template
+        initials['bom_copy'] = InvenTreeSetting.get_setting('PART_COPY_BOM')
+        initials['parameters_copy'] = InvenTreeSetting.get_seting('PART_COPY_PARAMETERS')
 
         return initials
 
@@ -906,23 +908,21 @@ class BomValidate(AjaxUpdateView):
 
         return self.renderJsonResponse(request, form, context=self.get_context())
 
-    def post(self, request, *args, **kwargs):
+    def validate(self, part, form, **kwargs):
 
-        form = self.get_form()
-        part = self.get_object()
+        confirm = str2bool(form.cleaned_data.get('validate', False))
 
-        confirmed = str2bool(request.POST.get('validate', False))
-
-        if confirmed:
-            part.validate_bom(request.user)
-        else:
+        if not confirm:
             form.add_error('validate', _('Confirm that the BOM is valid'))
 
-        data = {
-            'form_valid': confirmed
-        }
+    def post_save(self, part, form, **kwargs):
 
-        return self.renderJsonResponse(request, form, data, context=self.get_context())
+        part.validate_bom(self.request.user)
+
+    def get_data(self):
+        return {
+            'success': _('Validated Bill of Materials')
+        }
 
 
 class BomUpload(InvenTreeRoleMixin, FormView):
diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py
index e4c6d67cb2..1a58592f26 100644
--- a/InvenTree/stock/models.py
+++ b/InvenTree/stock/models.py
@@ -1126,6 +1126,20 @@ class StockItem(MPTTModel):
 
         return s
 
+    @transaction.atomic
+    def clear_test_results(self, **kwargs):
+        """
+        Remove all test results
+        """
+
+        # All test results
+        results = self.test_results.all()
+
+        # TODO - Perhaps some filtering options supplied by kwargs?
+
+        results.delete()
+
+
     def getTestResults(self, test=None, result=None, user=None):
         """
         Return all test results associated with this StockItem.
diff --git a/InvenTree/stock/views.py b/InvenTree/stock/views.py
index 09c2d4b159..aabf3ff7b9 100644
--- a/InvenTree/stock/views.py
+++ b/InvenTree/stock/views.py
@@ -245,32 +245,28 @@ class StockItemAssignToCustomer(AjaxUpdateView):
     form_class = StockForms.AssignStockItemToCustomerForm
     role_required = 'stock.change'
 
-    def post(self, request, *args, **kwargs):
+    def validate(self, item, form, **kwargs):
 
-        customer = request.POST.get('customer', None)
+        customer = form.cleaned_data.get('customer', None)
+
+        if not customer:
+            form.add_error('customer', _('Customer must be specified'))
+
+    def post_save(self, item, form, **kwargs):
+        """
+        Assign the stock item to the customer.
+        """
+
+        customer = form.cleaned_data.get('customer', None)
 
         if customer:
-            try:
-                customer = Company.objects.get(pk=customer)
-            except (ValueError, Company.DoesNotExist):
-                customer = None
-
-        if customer is not None:
-            stock_item = self.get_object()
-
-            item = stock_item.allocateToCustomer(
+            item = item.allocateToCustomer(
                 customer,
-                user=request.user
+                user=self.request.user
             )
 
             item.clearAllocations()
 
-        data = {
-            'form_valid': True,
-        }
-
-        return self.renderJsonResponse(request, self.get_form(), data)
-
 
 class StockItemReturnToStock(AjaxUpdateView):
     """
@@ -283,30 +279,25 @@ class StockItemReturnToStock(AjaxUpdateView):
     form_class = StockForms.ReturnStockItemForm
     role_required = 'stock.change'
 
-    def post(self, request, *args, **kwargs):
+    def validate(self, item, form, **kwargs):
 
-        location = request.POST.get('location', None)
+        location = form.cleaned_data.get('location', None)
+
+        if not location:
+            form.add_error('location', _('Specify a valid location'))
+
+    def post_save(self, item, form, **kwargs):
+
+        location = form.cleaned_data.get('location', None)
 
         if location:
-            try:
-                location = StockLocation.objects.get(pk=location)
-            except (ValueError, StockLocation.DoesNotExist):
-                location = None
+            item.returnFromCustomer(location, self.request.user)
 
-        if location:
-            stock_item = self.get_object()
-
-            stock_item.returnFromCustomer(location, request.user)
-        else:
-            raise ValidationError({'location': _("Specify a valid location")})
-
-        data = {
-            'form_valid': True,
-            'success': _("Stock item returned from customer")
+    def get_data(self):
+        return {
+            'success': _('Stock item returned from customer')
         }
 
-        return self.renderJsonResponse(request, self.get_form(), data)
-
 
 class StockItemSelectLabels(AjaxView):
     """

From 1caa341f8e588759f4375ded9e244da96ccbc95e Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@gmail.com>
Date: Fri, 30 Oct 2020 21:34:56 +1100
Subject: [PATCH 3/3] Fixes for unit tests

---
 InvenTree/order/forms.py      | 10 +++++-----
 InvenTree/order/test_views.py |  4 +---
 InvenTree/order/views.py      |  3 ++-
 InvenTree/part/models.py      |  2 +-
 InvenTree/part/views.py       |  2 +-
 InvenTree/stock/models.py     |  4 +++-
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/InvenTree/order/forms.py b/InvenTree/order/forms.py
index 1f412271e0..770dd392bf 100644
--- a/InvenTree/order/forms.py
+++ b/InvenTree/order/forms.py
@@ -21,7 +21,7 @@ from .models import SalesOrderAllocation
 
 class IssuePurchaseOrderForm(HelperForm):
 
-    confirm = forms.BooleanField(required=False, help_text=_('Place order'))
+    confirm = forms.BooleanField(required=True, initial=False, help_text=_('Place order'))
 
     class Meta:
         model = PurchaseOrder
@@ -32,7 +32,7 @@ class IssuePurchaseOrderForm(HelperForm):
 
 class CompletePurchaseOrderForm(HelperForm):
 
-    confirm = forms.BooleanField(required=False, help_text=_("Mark order as complete"))
+    confirm = forms.BooleanField(required=True, help_text=_("Mark order as complete"))
 
     class Meta:
         model = PurchaseOrder
@@ -43,7 +43,7 @@ class CompletePurchaseOrderForm(HelperForm):
 
 class CancelPurchaseOrderForm(HelperForm):
 
-    confirm = forms.BooleanField(required=False, help_text=_('Cancel order'))
+    confirm = forms.BooleanField(required=True, help_text=_('Cancel order'))
 
     class Meta:
         model = PurchaseOrder
@@ -54,7 +54,7 @@ class CancelPurchaseOrderForm(HelperForm):
 
 class CancelSalesOrderForm(HelperForm):
 
-    confirm = forms.BooleanField(required=False, help_text=_('Cancel order'))
+    confirm = forms.BooleanField(required=True, help_text=_('Cancel order'))
 
     class Meta:
         model = SalesOrder
@@ -65,7 +65,7 @@ class CancelSalesOrderForm(HelperForm):
 
 class ShipSalesOrderForm(HelperForm):
 
-    confirm = forms.BooleanField(required=False, help_text=_('Ship order'))
+    confirm = forms.BooleanField(required=True, help_text=_('Ship order'))
 
     class Meta:
         model = SalesOrder
diff --git a/InvenTree/order/test_views.py b/InvenTree/order/test_views.py
index 246cc2dd48..5c4e2dd405 100644
--- a/InvenTree/order/test_views.py
+++ b/InvenTree/order/test_views.py
@@ -113,6 +113,7 @@ class POTests(OrderViewTestCase):
         self.assertEqual(response.status_code, 200)
         
         data = json.loads(response.content)
+
         self.assertFalse(data['form_valid'])
 
         # Test WITH confirmation
@@ -151,7 +152,6 @@ class POTests(OrderViewTestCase):
         response = self.client.post(url, post_data, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
         data = json.loads(response.content)
         self.assertFalse(data['form_valid'])
-        self.assertIn('Invalid Purchase Order', str(data['html_form']))
 
         # POST with a part that does not match the purchase order
         post_data['order'] = 1
@@ -159,14 +159,12 @@ class POTests(OrderViewTestCase):
         response = self.client.post(url, post_data, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
         data = json.loads(response.content)
         self.assertFalse(data['form_valid'])
-        self.assertIn('must match for Part and Order', str(data['html_form']))
 
         # POST with an invalid part
         post_data['part'] = 12345
         response = self.client.post(url, post_data, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
         data = json.loads(response.content)
         self.assertFalse(data['form_valid'])
-        self.assertIn('Invalid SupplierPart selection', str(data['html_form']))
 
         # POST the form with valid data
         post_data['part'] = 100
diff --git a/InvenTree/order/views.py b/InvenTree/order/views.py
index 333509952b..e411afbdad 100644
--- a/InvenTree/order/views.py
+++ b/InvenTree/order/views.py
@@ -454,7 +454,7 @@ class PurchaseOrderIssue(AjaxUpdateView):
 
     def validate(self, order, form, **kwargs):
 
-        confirm = str2bool(form.cleaned_data.get('confirm', False))
+        confirm = str2bool(self.request.POST.get('confirm', False))
 
         if not confirm:
             form.add_error('confirm', _('Confirm order placement'))
@@ -504,6 +504,7 @@ class PurchaseOrderComplete(AjaxUpdateView):
             'success': _('Purchase order completed')
         }
 
+
 class SalesOrderShip(AjaxUpdateView):
     """ View for 'shipping' a SalesOrder """
     form_class = order_forms.ShipSalesOrderForm
diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py
index 34beb4b68e..715d1423eb 100644
--- a/InvenTree/part/models.py
+++ b/InvenTree/part/models.py
@@ -1141,7 +1141,7 @@ class Part(MPTTModel):
         if clear:
             self.get_parameters().delete()
 
-        for parameter in other.get_parameters.all():
+        for parameter in other.get_parameters():
 
             # If this part already has a parameter pointing to the same template,
             # delete that parameter from this part first!
diff --git a/InvenTree/part/views.py b/InvenTree/part/views.py
index 2dd3a47c3f..0b96addc84 100644
--- a/InvenTree/part/views.py
+++ b/InvenTree/part/views.py
@@ -372,7 +372,7 @@ class MakePartVariant(AjaxCreateView):
         initials['is_template'] = False
         initials['variant_of'] = part_template
         initials['bom_copy'] = InvenTreeSetting.get_setting('PART_COPY_BOM')
-        initials['parameters_copy'] = InvenTreeSetting.get_seting('PART_COPY_PARAMETERS')
+        initials['parameters_copy'] = InvenTreeSetting.get_setting('PART_COPY_PARAMETERS')
 
         return initials
 
diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py
index 1a58592f26..11ec0bd087 100644
--- a/InvenTree/stock/models.py
+++ b/InvenTree/stock/models.py
@@ -1130,6 +1130,9 @@ class StockItem(MPTTModel):
     def clear_test_results(self, **kwargs):
         """
         Remove all test results
+
+        kwargs:
+            TODO
         """
 
         # All test results
@@ -1139,7 +1142,6 @@ class StockItem(MPTTModel):
 
         results.delete()
 
-
     def getTestResults(self, test=None, result=None, user=None):
         """
         Return all test results associated with this StockItem.