Merge pull request #2613 from SchrodingersGat/bom-import-unit-tests

Bom import unit tests

InvenTree/InvenTree/api_tester.py

@@ -106,12 +106,12 @@ class InvenTreeAPITestCase(APITestCase):
 
         return response
 
-    def post(self, url, data, expected_code=None):
+    def post(self, url, data, expected_code=None, format='json'):
         """
         Issue a POST request
         """
 
-        response = self.client.post(url, data=data, format='json')
+        response = self.client.post(url, data=data, format=format)
 
         if expected_code is not None:
             self.assertEqual(response.status_code, expected_code)
@@ -130,12 +130,12 @@ class InvenTreeAPITestCase(APITestCase):
 
         return response
 
-    def patch(self, url, data, files=None, expected_code=None):
+    def patch(self, url, data, expected_code=None, format='json'):
         """
         Issue a PATCH request
         """
 
-        response = self.client.patch(url, data=data, files=files, format='json')
+        response = self.client.patch(url, data=data, format=format)
 
         if expected_code is not None:
             self.assertEqual(response.status_code, expected_code)

InvenTree/InvenTree/serializers.py

@@ -328,4 +328,7 @@ class InvenTreeDecimalField(serializers.FloatField):
     def to_internal_value(self, data):
 
         # Convert the value to a string, and then a decimal
-        return Decimal(str(data))
+        try:
+            return Decimal(str(data))
+        except:
+            raise serializers.ValidationError(_("Invalid value"))

InvenTree/part/serializers.py

@@ -818,13 +818,22 @@ class BomExtractSerializer(serializers.Serializer):
             raise serializers.ValidationError(_("File is too large"))
 
         # Read file data into memory (bytes object)
-        data = bom_file.read()
+        try:
+            data = bom_file.read()
+        except Exception as e:
+            raise serializers.ValidationError(str(e))
 
         if ext in ['csv', 'tsv', 'xml']:
-            data = data.decode()
+            try:
+                data = data.decode()
+            except Exception as e:
+                raise serializers.ValidationError(str(e))
 
         # Convert to a tablib dataset (we expect headers)
-        self.dataset = tablib.Dataset().load(data, ext, headers=True)
+        try:
+            self.dataset = tablib.Dataset().load(data, ext, headers=True)
+        except Exception as e:
+            raise serializers.ValidationError(str(e))
 
         for header in self.REQUIRED_COLUMNS:
 
@@ -848,6 +857,9 @@ class BomExtractSerializer(serializers.Serializer):
         if not part_match:
             raise serializers.ValidationError(_("No part column found"))
 
+        if len(self.dataset) == 0:
+            raise serializers.ValidationError(_("No data rows found"))
+
         return bom_file
 
     def extract_data(self):
@@ -856,6 +868,9 @@ class BomExtractSerializer(serializers.Serializer):
         """
 
         rows = []
+        errors = []
+
+        found_parts = set()
 
         headers = self.dataset.headers
 
@@ -863,6 +878,8 @@ class BomExtractSerializer(serializers.Serializer):
 
         for row in self.dataset.dict:
 
+            row_error = {}
+
             """
             If the "level" column is specified, and this is not a top-level BOM item, ignore the row!
             """
@@ -907,25 +924,64 @@ class BomExtractSerializer(serializers.Serializer):
 
             if part is None:
 
-                if part_name is not None or part_ipn is not None:
+                if part_name or part_ipn:
                     queryset = Part.objects.all()
 
-                    if part_name is not None:
+                    if part_name:
                         queryset = queryset.filter(name=part_name)
 
-                    if part_ipn is not None:
+                    if part_ipn:
                         queryset = queryset.filter(IPN=part_ipn)
 
                     # Only if we have a single direct match
-                    if queryset.exists() and queryset.count() == 1:
+                    if queryset.exists():
-                        part = queryset.first()
+                        if queryset.count() == 1:
+                            part = queryset.first()
+                        else:
+                            # Multiple matches!
+                            row_error['part'] = _('Multiple matching parts found')
+
+            if part is None:
+                if 'part' not in row_error:
+                    row_error['part'] = _('No matching part found')
+            else:
+                if part.pk in found_parts:
+                    row_error['part'] = _("Duplicate part selected")
+
+                elif not part.component:
+                    row_error['part'] = _('Part is not designated as a component')
+
+                found_parts.add(part.pk)
 
             row['part'] = part.pk if part is not None else None
 
+            """
+            Read out the 'quantity' column - check that it is valid
+            """
+            quantity = self.find_matching_data(row, 'quantity', self.dataset.headers)
+
+            if quantity is None:
+                row_error['quantity'] = _('Quantity not provided')
+            else:
+                try:
+                    quantity = Decimal(quantity)
+
+                    if quantity <= 0:
+                        row_error['quantity'] = _('Quantity must be greater than zero')
+                except:
+                    row_error['quantity'] = _('Invalid quantity')
+
+            # For each "optional" column, ensure the column names are allocated correctly
+            for field_name in self.OPTIONAL_COLUMNS:
+                if field_name not in row:
+                    row[field_name] = self.find_matching_data(row, field_name, self.dataset.headers)
+
             rows.append(row)
+            errors.append(row_error)
 
         return {
             'rows': rows,
+            'errors': errors,
             'headers': headers,
             'filename': self.filename,
         }

InvenTree/part/templates/part/upload_bom.html

@@ -22,8 +22,11 @@
 <button type='button' class='btn btn-primary' id='bom-upload'>
     <span class='fas fa-file-upload'></span> {% trans "Upload BOM File" %}
 </button>
+<button type='button' class='btn btn-success' disabled='true' id='bom-submit-icon' style='display: none;'>
+    <span class="fas fa-spin fa-circle-notch"></span>
+</button>
 <button type='button' class='btn btn-success' id='bom-submit' style='display: none;'>
-    <span class='fas fa-sign-in-alt'></span> {% trans "Submit BOM Data" %}
+    <span class='fas fa-sign-in-alt' id='bom-submit-icon'></span> {% trans "Submit BOM Data" %}
 </button>
 {% endblock %}
 
@@ -43,7 +46,7 @@
     </div>
 
     <!-- This table is filled out after BOM file is uploaded and processed -->
-    <table class='table table-striped table-condensed' id='bom-import-table'>
+    <table class='table table-condensed' id='bom-import-table'>
         <thead>
             <tr>
                 <th style='max-width: 500px;'>{% trans "Part" %}</th>

InvenTree/part/test_bom_import.py

@@ -0,0 +1,298 @@
+"""
+Unit testing for BOM upload / import functionality
+"""
+
+import tablib
+
+from django.core.files.uploadedfile import SimpleUploadedFile
+from django.urls import reverse
+
+from InvenTree.api_tester import InvenTreeAPITestCase
+
+from part.models import Part
+
+
+class BomUploadTest(InvenTreeAPITestCase):
+    """
+    Test BOM file upload API endpoint
+    """
+
+    roles = [
+        'part.add',
+        'part.change',
+    ]
+
+    def setUp(self):
+        super().setUp()
+
+        self.part = Part.objects.create(
+            name='Assembly',
+            description='An assembled part',
+            assembly=True,
+            component=False,
+        )
+
+        for i in range(10):
+            Part.objects.create(
+                name=f"Component {i}",
+                IPN=f"CMP_{i}",
+                description="A subcomponent that can be used in a BOM",
+                component=True,
+                assembly=False,
+            )
+
+        self.url = reverse('api-bom-extract')
+
+    def post_bom(self, filename, file_data, part=None, clear_existing=None, expected_code=None, content_type='text/plain'):
+
+        bom_file = SimpleUploadedFile(
+            filename,
+            file_data,
+            content_type=content_type,
+        )
+
+        if part is None:
+            part = self.part.pk
+
+        if clear_existing is None:
+            clear_existing = False
+
+        response = self.post(
+            self.url,
+            data={
+                'bom_file': bom_file,
+                'part': part,
+                'clear_existing': clear_existing,
+            },
+            expected_code=expected_code,
+            format='multipart',
+        )
+
+        return response
+
+    def test_missing_file(self):
+        """
+        POST without a file
+        """
+
+        response = self.post(
+            self.url,
+            data={},
+            expected_code=400
+        )
+
+        self.assertIn('No file was submitted', str(response.data['bom_file']))
+        self.assertIn('This field is required', str(response.data['part']))
+        self.assertIn('This field is required', str(response.data['clear_existing']))
+
+    def test_unsupported_file(self):
+        """
+        POST with an unsupported file type
+        """
+
+        response = self.post_bom(
+            'sample.txt',
+            b'hello world',
+            expected_code=400,
+        )
+
+        self.assertIn('Unsupported file type', str(response.data['bom_file']))
+
+    def test_broken_file(self):
+        """
+        Test upload with broken (corrupted) files
+        """
+
+        response = self.post_bom(
+            'sample.csv',
+            b'',
+            expected_code=400,
+        )
+
+        self.assertIn('The submitted file is empty', str(response.data['bom_file']))
+
+        response = self.post_bom(
+            'test.xls',
+            b'hello world',
+            expected_code=400,
+            content_type='application/xls',
+        )
+
+        self.assertIn('Unsupported format, or corrupt file', str(response.data['bom_file']))
+
+    def test_invalid_upload(self):
+        """
+        Test upload of an invalid file
+        """
+
+        dataset = tablib.Dataset()
+
+        dataset.headers = [
+            'apple',
+            'banana',
+        ]
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            content_type='text/csv',
+            expected_code=400,
+        )
+
+        self.assertIn("Missing required column: 'quantity'", str(response.data))
+
+        # Try again, with an .xlsx file
+        response = self.post_bom(
+            'bom.xlsx',
+            dataset.xlsx,
+            content_type='application/xlsx',
+            expected_code=400,
+        )
+
+        self.assertIn("Missing required column: 'quantity'", str(response.data))
+
+        # Add the quantity field (or close enough)
+        dataset.headers.append('quAntiTy  ')
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            content_type='text/csv',
+            expected_code=400,
+        )
+
+        self.assertIn('No part column found', str(response.data))
+
+        dataset.headers.append('part_id')
+        dataset.headers.append('part_name')
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            content_type='text/csv',
+            expected_code=400,
+        )
+
+        self.assertIn('No data rows found', str(response.data))
+
+    def test_invalid_data(self):
+        """
+        Upload data which contains errors
+        """
+
+        dataset = tablib.Dataset()
+
+        # Only these headers are strictly necessary
+        dataset.headers = ['part_id', 'quantity']
+
+        components = Part.objects.filter(component=True)
+
+        for idx, cmp in enumerate(components):
+
+            if idx == 5:
+                cmp.component = False
+                cmp.save()
+
+            dataset.append([cmp.pk, idx])
+
+        # Add a duplicate part too
+        dataset.append([components.first().pk, 'invalid'])
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            content_type='text/csv',
+            expected_code=201
+        )
+
+        errors = response.data['errors']
+
+        self.assertIn('Quantity must be greater than zero', str(errors[0]))
+        self.assertIn('Part is not designated as a component', str(errors[5]))
+        self.assertIn('Duplicate part selected', str(errors[-1]))
+        self.assertIn('Invalid quantity', str(errors[-1]))
+
+        for idx, row in enumerate(response.data['rows'][:-1]):
+            self.assertEqual(str(row['part']), str(components[idx].pk))
+
+    def test_part_guess(self):
+        """
+        Test part 'guessing' when PK values are not supplied
+        """
+
+        dataset = tablib.Dataset()
+
+        # Should be able to 'guess' the part from the name
+        dataset.headers = ['part_name', 'quantity']
+
+        components = Part.objects.filter(component=True)
+
+        for idx, cmp in enumerate(components):
+            dataset.append([
+                f"Component {idx}",
+                10,
+            ])
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            expected_code=201,
+        )
+
+        rows = response.data['rows']
+
+        self.assertEqual(len(rows), 10)
+
+        for idx in range(10):
+            self.assertEqual(rows[idx]['part'], components[idx].pk)
+
+        # Should also be able to 'guess' part by the IPN value
+        dataset = tablib.Dataset()
+
+        dataset.headers = ['part_ipn', 'quantity']
+
+        for idx, cmp in enumerate(components):
+            dataset.append([
+                f"CMP_{idx}",
+                10,
+            ])
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            expected_code=201,
+        )
+
+        rows = response.data['rows']
+
+        self.assertEqual(len(rows), 10)
+
+        for idx in range(10):
+            self.assertEqual(rows[idx]['part'], components[idx].pk)
+
+    def test_levels(self):
+        """
+        Test that multi-level BOMs are correctly handled during upload
+        """
+
+        dataset = tablib.Dataset()
+
+        dataset.headers = ['level', 'part', 'quantity']
+
+        components = Part.objects.filter(component=True)
+
+        for idx, cmp in enumerate(components):
+            dataset.append([
+                idx % 3,
+                cmp.pk,
+                2,
+            ])
+
+        response = self.post_bom(
+            'test.csv',
+            bytes(dataset.csv, 'utf8'),
+            expected_code=201,
+        )
+
+        # Only parts at index 1, 4, 7 should have been returned
+        self.assertEqual(len(response.data['rows']), 3)

InvenTree/templates/js/translated/bom.js

@@ -40,6 +40,12 @@ function constructBomUploadTable(data, options={}) {
     function constructRow(row, idx, fields) {
         // Construct an individual row from the provided data
 
+        var errors = {};
+
+        if (data.errors && data.errors.length > idx) {
+            errors = data.errors[idx];
+        }
+
         var field_options = {
             hideLabels: true,
             hideClearButton: true,
@@ -72,7 +78,7 @@ function constructBomUploadTable(data, options={}) {
 
         var buttons = `<div class='btn-group float-right' role='group'>`;
 
-        // buttons += makeIconButton('fa-file-alt', 'button-row-data', idx, '{% trans "Display row data" %}');
+        buttons += makeIconButton('fa-info-circle', 'button-row-data', idx, '{% trans "Display row data" %}');
         buttons += makeIconButton('fa-times icon-red', 'button-row-remove', idx, '{% trans "Remove row" %}');
 
         buttons += `</div>`;
@@ -92,6 +98,15 @@ function constructBomUploadTable(data, options={}) {
 
         $('#bom-import-table tbody').append(html);
 
+        // Handle any errors raised by initial data import
+        if (errors.part) {
+            addFieldErrorMessage(`items_sub_part_${idx}`, errors.part);   
+        }
+
+        if (errors.quantity) {
+            addFieldErrorMessage(`items_quantity_${idx}`, errors.quantity);
+        }
+
         // Initialize the "part" selector for this row
         initializeRelatedField(
             {
@@ -114,6 +129,29 @@ function constructBomUploadTable(data, options={}) {
         $(`#button-row-remove-${idx}`).click(function() {
             $(`#items_${idx}`).remove();
         });
+
+        // Add callback for "show data" button
+        $(`#button-row-data-${idx}`).click(function() {
+
+            var modal = createNewModal({
+                title: '{% trans "Row Data" %}',
+                cancelText: '{% trans "Close" %}',
+                hideSubmitButton: true
+            });
+
+            // Prettify the original import data
+            var pretty = JSON.stringify(row, undefined, 4);
+
+            var html = `
+            <div class='alert alert-block'>
+            <pre><code>${pretty}</code></pre>
+            </div>`;
+
+            modalSetContent(modal, html);
+
+            $(modal).modal('show');
+
+        });
     }
 
     // Request API endpoint options
@@ -172,6 +210,10 @@ function submitBomTable(part_id, options={}) {
     getApiEndpointOptions(url, function(response) {
         var fields = response.actions.POST;
 
+        // Disable the "Submit BOM" button
+        $('#bom-submit').prop('disabled', true);
+        $('#bom-submit-icon').show();
+
         inventreePut(url, data, {
             method: 'POST',
             success: function(response) {
@@ -186,6 +228,10 @@ function submitBomTable(part_id, options={}) {
                     showApiError(xhr, url);
                     break;
                 }
+
+                // Re-enable the submit button
+                $('#bom-submit').prop('disabled', false);
+                $('#bom-submit-icon').hide();
             }
         });
     });

InvenTree/templates/js/translated/forms.js

@@ -1196,13 +1196,13 @@ function handleFormErrors(errors, fields={}, options={}) {
 /*
  * Add a rendered error message to the provided field
  */
-function addFieldErrorMessage(name, error_text, error_idx, options={}) {
+function addFieldErrorMessage(name, error_text, error_idx=0, options={}) {
 
     field_name = getFieldName(name, options);
 
     var field_dom = null;
 
-    if (options.modal) {
+    if (options && options.modal) {
         $(options.modal).find(`#div_id_${field_name}`).addClass('form-field-error');
         field_dom = $(options.modal).find(`#errors-${field_name}`);
     } else {
@@ -1210,7 +1210,7 @@ function addFieldErrorMessage(name, error_text, error_idx, options={}) {
         field_dom = $(`#errors-${field_name}`);
     }
 
-    if (field_dom) {
+    if (field_dom.exists()) {
 
         var error_html = `
         <span id='error_${error_idx}_id_${field_name}' class='help-block form-error-message'>
@@ -1953,7 +1953,13 @@ function constructField(name, parameters, options) {
         html += parameters.before;
     }
     
-    html += `<div id='div_id_${field_name}' class='${form_classes}'>`;
+    var hover_title = '';
+
+    if (parameters.help_text) {
+        hover_title = ` title='${parameters.help_text}'`;
+    }
+
+    html += `<div id='div_id_${field_name}' class='${form_classes}' ${hover_title}>`;
 
     // Add a label
     if (!options.hideLabels) {