Enhance Package Import to support modifications #84 (#120)

Signed-off-by: tdruez <tdruez@nexb.com>

Author: tdruez

CHANGELOG.rst

@@ -75,6 +75,9 @@ Release notes
 - Add a new AboutCode tab in Package details view.
   https://github.com/nexB/dejacode/issues/42
 
+- Enhance Package Import to support modifications.
+  https://github.com/nexB/dejacode/issues/84
+
 ### Version 5.0.1
 
 - Improve the stability of the "Check for new Package versions" feature.

component_catalog/importers.py

@@ -30,6 +30,7 @@
 from component_catalog.models import Package
 from component_catalog.models import Subcomponent
 from component_catalog.programming_languages import PROGRAMMING_LANGUAGES
+from dje.fields import SmartFileField
 from dje.forms import JSONListField
 from dje.importers import BaseImporter
 from dje.importers import BaseImportModelForm
@@ -305,10 +306,20 @@ def save(self, commit=True):
         return package
 
 
+class PackageImportableUploadFileForm(forms.Form):
+    file = SmartFileField(extensions=["csv", "json"])
+
+    @property
+    def header(self):
+        return "Select a <strong>CSV (.csv) or JSON (.json)</strong> file"
+
+
 class PackageImporter(BaseImporter):
     model_form = PackageImportForm
+    upload_form_class = PackageImportableUploadFileForm
     add_to_product_perm = "product_portfolio.add_productpackage"
     relation_model = ProductPackage
+    update_existing = True
 
     def prepare_data_json(self, data):
         """

component_catalog/tests/test_importers.py

@@ -1318,6 +1318,40 @@ def test_package_import_prepare_package(self):
         prepared_package = PackageImporter.prepare_package(package_data)
         self.assertEqual(expected, prepared_package)
 
+    def test_package_import_update_existing(self):
+        formset_data = {
+            "form-TOTAL_FORMS": "1",
+            "form-INITIAL_FORMS": "0",
+            "form-0-filename": "filename.zip",
+        }
+
+        importer = PackageImporter(self.super_user, formset_data=formset_data)
+        self.assertTrue(importer.formset.is_valid())
+        importer.save_all()
+        self.assertEqual(1, len(importer.results["added"]))
+
+        importer = PackageImporter(self.super_user, formset_data=formset_data)
+        self.assertTrue(importer.formset.is_valid())
+        importer.save_all()
+        self.assertEqual(1, len(importer.results["unmodified"]))
+
+        formset_data = {
+            "form-TOTAL_FORMS": "1",
+            "form-INITIAL_FORMS": "0",
+            "form-0-filename": "filename.zip",
+            "form-0-notes": "Notes",
+        }
+        importer = PackageImporter(self.super_user, formset_data=formset_data)
+        self.assertTrue(importer.formset.is_valid())
+        importer.save_all()
+        self.assertEqual(1, len(importer.results["modified"]))
+        modified_package = importer.results["modified"][0]
+        self.assertEqual("Notes", modified_package.notes)
+
+        history_entry = History.objects.get_for_object(modified_package).get()
+        expected_messages = "Updated notes from import"
+        self.assertEqual(expected_messages, history_entry.change_message)
+
     def test_package_import_add_to_product(self):
         admin_user = create_admin("admin_user", self.dataspace)
         self.client.login(username=admin_user.username, password="secret")

dje/importers.py

@@ -154,7 +154,7 @@ def pre_process_form(self, data, **kwargs):
             identifier_fields = model_class.get_identifier_fields()
             # Crafting the list of unique filters to match the instance
             for field_name in identifier_fields:
-                value = data.get(prefix + field_name, None)
+                value = data.get(prefix + field_name, "")
                 if value:
                     value = value.strip()
                 filters.update({field_name: value})
@@ -288,6 +288,10 @@ def clean(self):
             raise forms.ValidationError("One of the row is a duplicate.")
 
 
+class ImportableUploadFileForm(forms.Form):
+    file = SmartFileField(extensions=["csv"])
+
+
 class BaseImporter:
     """
     Import in 3 steps:
@@ -309,7 +313,9 @@ class BaseImporter:
 
     model_form = None
     formset_class = BaseImportModelFormSet
+    upload_form_class = ImportableUploadFileForm
     add_to_product = False
+    update_existing = False
 
     def __init__(self, user, file_location=None, formset_data=None):
         if not self.model_form and not isinstance(self.model_form, BaseImportModelForm):
@@ -564,7 +570,7 @@ def get_key_sorted_fields(self):
         """
         model_form_instance = self.model_form(dataspace=self.dataspace, user=self.user)
 
-        for _, field in model_form_instance.fields.items():
+        for field in model_form_instance.fields.values():
             field.supported_values = self.get_supported_values(field)
 
         return sorted(model_form_instance.fields.items())
@@ -582,14 +588,31 @@ def save_all(self):
         if not self.formset.is_valid():  # Just in case...
             return
 
-        self.results = {"added": [], "unmodified": []}
+        self.results = {"added": [], "modified": [], "unmodified": []}
         for form in self.formset:
-            if not form.instance.pk:  # Save only addition for now
-                saved_instance = form.save()
-                self.results["added"].append(saved_instance)
-                History.log_addition(self.user, saved_instance)
-            else:
-                self.results["unmodified"].append(form.instance)
+            self.save_form(form)
+
+    def save_form(self, form):
+        instance = form.instance
+
+        if not instance.pk:
+            saved_instance = form.save()
+            self.results["added"].append(saved_instance)
+            History.log_addition(self.user, saved_instance)
+            return
+
+        elif self.update_existing:
+            # We need to refresh the instance from the db because form.instance has
+            # the unsaved form.cleaned_data modification at that stage.
+            instance.refresh_from_db()
+            updated_fields = instance.update_from_data(self.user, form.cleaned_data, override=False)
+            if updated_fields:
+                self.results["modified"].append(instance)
+                msg = f'Updated {", ".join(updated_fields)} from import'
+                History.log_change(self.user, instance, message=msg)
+                return
+
+        self.results["unmodified"].append(instance)
 
     def get_added_instance_ids(self):
         """Return the list of added instance ids."""
@@ -625,25 +648,11 @@ def get_add_to_product_form(self, request):
             return form
 
 
-class ImportableUploadFileForm(forms.Form):
-    file = SmartFileField(extensions=["csv"])
-
-
-class PackageImportableUploadFileForm(forms.Form):
-    file = SmartFileField(extensions=["csv", "json"])
-
-    @property
-    def header(self):
-        return "Select a <strong>CSV (.csv) or JSON (.json)</strong> file"
-
-
 @login_required()
 def import_view(request, importer_class):
     user = request.user
     importer = importer_class(user)
-    upload_form_class = ImportableUploadFileForm
-    if importer_class.__name__ == "PackageImporter":
-        upload_form_class = PackageImportableUploadFileForm
+    upload_form_class = importer.upload_form_class
 
     opts = importer.model_form._meta.model._meta
     perm_codename = get_permission_codename("add", opts)

dje/templates/admin/object_import.html

@@ -30,7 +30,15 @@ <h1 class="header-title">
       <h5>Help for {{ importer.verbose_name|title }} Import:</h5>
       <ul class="mb-0">
         {% if importer.formset and not importer.fatal_errors %}
-          <li><span class="badge text-bg-secondary">M</span> The {{ importer.verbose_name|title }} matches an already existing {{ importer.verbose_name|title }}. Nothing will be done for the row.</li>
+          <li>
+            <span class="badge text-bg-secondary">M</span>
+            The {{ importer.verbose_name|title }} matches an already existing {{ importer.verbose_name|title }}.
+            {% if importer.update_existing %}
+              Empty fields will be updated with import data.
+            {% else %}
+              Nothing will be done for the row.
+            {% endif %}
+          </li>
           <li><span class="badge text-bg-success">A</span> The {{ importer.verbose_name|title }} will be added based on the input data</li>
           <li><span class="badge text-bg-danger">E</span> The row contains at least one error</li>
           <li><span class="badge text-bg-warning">W</span> The row is valid but should be manually reviewed</li>
@@ -41,11 +49,15 @@ <h5>Help for {{ importer.verbose_name|title }} Import:</h5>
           <li>The character encoding of the input file needs to be "UTF-8"</li>
           <li>The column order does not matter.</li>
           <li>The column names are case sensitive.</li>
-          <li>Required column{{ importer.required_fields|pluralize }}: <strong>{{ importer.required_fields|join:", " }}</strong></li>
-          <li>Supported values for boolean fields:<br>
-            <strong>Yes:</strong> True, T, Yes, Y<br>
-            <strong>No:</strong> False, F, No, N
+          {% if importer.required_fields %}
+            <li>Required column{{ importer.required_fields|pluralize }}: <strong>{{ importer.required_fields|join:", " }}</strong></li>
+          {% endif %}
+          <li>Supported values for boolean fields:
+            <strong>Yes:</strong> True, T, Yes, Y; <strong>No:</strong> False, F, No, N
           </li>
+          {% if importer.verbose_name == "package" %}
+            <li>Note that a <strong>Package is uniquely defined in DejaCode by a combination of filename, download_url, and the six Package URL fields</strong> type, namespace, name, version, qualifiers, and subpath. You are not required to provide values in all of these fields (qualifiers and subpath are less commonly used) but if any of them are different from an existing similar package already in DejaCode, then the importer will perform an addition rather than a modification.</li>
+          {% endif %}
         {% endif %}
       </ul>
         <div class="text-center">
@@ -265,7 +277,7 @@ <h3>Step 1: Select your file</h3>
             </div>
           {% endif %}
           <form enctype="multipart/form-data" method="post">{% csrf_token %}
-            <p><input type="file" id="id_file" name="file"></p>
+            <p>{{ file_form.file }}</p>
             <input class="btn btn-primary" type="submit" value="Upload" />
           </form>
         </div>