Repository: openwisp/openwisp-firmware-upgrader (language: python)

Issue:
[fw-upgrader] Do not delete upgrade operations when FirmwareImage is deleted
It should be possible to delete firmware images to free space but history shouldn't be lost.

1. Add a failing test for this case
2. Apply fix, setting on_delete=models.SET_NULL to UpgradeOperation.firmware_image should be enough


Discussion / hints:
I would like to work on this, @nemesisdesign it'd be great if you could provide more information. I went through the [quickstart](https://github.com/openwisp/openwisp-firmware-upgrader#quickstart), created a build, uploaded an image and launched a mass upgrade operation but I'm doing something wrong.
![image](https://user-images.githubusercontent.com/10897423/102493124-b6bd5480-4098-11eb-9e1f-b3f3be696b8f.png)
 
> I would like to work on this, @nemesisdesign it'd be great if you could provide more information. I went through the [quickstart](https://github.com/openwisp/openwisp-firmware-upgrader#quickstart), created a build, uploaded an image and launched a mass upgrade operation but I'm doing something wrong.
> ![image](https://user-images.githubusercontent.com/10897423/102493124-b6bd5480-4098-11eb-9e1f-b3f3be696b8f.png)

That looks like a bug..! :disappointed: 
> > I would like to work on this, @nemesisdesign it'd be great if you could provide more information. I went through the [quickstart](https://github.com/openwisp/openwisp-firmware-upgrader#quickstart), created a build, uploaded an image and launched a mass upgrade operation but I'm doing something wrong.
> > ![image](https://user-images.githubusercontent.com/10897423/102493124-b6bd5480-4098-11eb-9e1f-b3f3be696b8f.png)
> 
> That looks like a bug..!

Cannot replicate this.

Can you describe the exact steps you did for this to show up?

Is any device present in your test system at all?

Fix patch:
diff --git a/openwisp_firmware_upgrader/base/models.py b/openwisp_firmware_upgrader/base/models.py
index f5695549..b89c8553 100644
--- a/openwisp_firmware_upgrader/base/models.py
+++ b/openwisp_firmware_upgrader/base/models.py
@@ -490,7 +490,9 @@ class AbstractUpgradeOperation(TimeStampedEditableModel):
     device = models.ForeignKey(
         swapper.get_model_name('config', 'Device'), on_delete=models.CASCADE
     )
-    image = models.ForeignKey(get_model_name('FirmwareImage'), on_delete=models.CASCADE)
+    image = models.ForeignKey(
+        get_model_name('FirmwareImage'), null=True, on_delete=models.SET_NULL
+    )
     status = models.CharField(
         max_length=12, choices=STATUS_CHOICES, default=STATUS_CHOICES[0][0]
     )
diff --git a/openwisp_firmware_upgrader/migrations/0007_nullable_upgrade_operation_image.py b/openwisp_firmware_upgrader/migrations/0007_nullable_upgrade_operation_image.py
new file mode 100644
index 00000000..3424234b
--- /dev/null
+++ b/openwisp_firmware_upgrader/migrations/0007_nullable_upgrade_operation_image.py
@@ -0,0 +1,24 @@
+# Generated by Django 3.1.5 on 2021-01-21 16:57
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('firmware_upgrader', '0006_create_device_firmware'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='upgradeoperation',
+            name='image',
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                to=settings.FIRMWARE_UPGRADER_FIRMWAREIMAGE_MODEL,
+            ),
+        ),
+    ]


Test patch:
diff --git a/openwisp_firmware_upgrader/tests/test_models.py b/openwisp_firmware_upgrader/tests/test_models.py
index 90dacd1d..b60f02f3 100644
--- a/openwisp_firmware_upgrader/tests/test_models.py
+++ b/openwisp_firmware_upgrader/tests/test_models.py
@@ -249,6 +249,14 @@ def test_create_for_device_validation_error(self):
         result = DeviceFirmware.create_for_device(device_fw.device)
         self.assertIsNone(result)
 
+    def test_upgrade_operation_retention_on_image_delete(self):
+        device_fw = self._create_device_firmware()
+        uo = UpgradeOperation.objects.create(
+            device=device_fw.device, image=device_fw.image
+        )
+        FirmwareImage.objects.get(pk=device_fw.image.pk).delete()
+        self.assertEqual(UpgradeOperation.objects.get(pk=uo.pk).image, None)
+
 
 class TestModelsTransaction(TestUpgraderMixin, TransactionTestCase):
     _mock_updrade = 'openwisp_firmware_upgrader.upgraders.openwrt.OpenWrt.upgrade'
diff --git a/tests/openwisp2/sample_firmware_upgrader/migrations/0007_create_device_firmware.py b/tests/openwisp2/sample_firmware_upgrader/migrations/0007_create_device_firmware.py
new file mode 100644
index 00000000..20d733cc
--- /dev/null
+++ b/tests/openwisp2/sample_firmware_upgrader/migrations/0007_create_device_firmware.py
@@ -0,0 +1,24 @@
+# Generated by Django 3.0.8 on 2020-07-07 17:00
+
+from django.db import migrations
+
+from openwisp_firmware_upgrader.migrations import create_device_firmware_for_connections
+
+
+def create_device_firmware_for_connections_helper(apps, schema_editor):
+    app_label = 'firmware_upgrader'
+    create_device_firmware_for_connections(apps, schema_editor, app_label)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('sample_firmware_upgrader', '0006_build_os_identifier'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            create_device_firmware_for_connections_helper,
+            reverse_code=migrations.RunPython.noop,
+        ),
+    ]
diff --git a/tests/openwisp2/sample_firmware_upgrader/migrations/0008_nullable_upgrade_operation_image.py b/tests/openwisp2/sample_firmware_upgrader/migrations/0008_nullable_upgrade_operation_image.py
new file mode 100644
index 00000000..6f13dbb4
--- /dev/null
+++ b/tests/openwisp2/sample_firmware_upgrader/migrations/0008_nullable_upgrade_operation_image.py
@@ -0,0 +1,24 @@
+# Generated by Django 3.1.5 on 2021-01-21 16:57
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('sample_firmware_upgrader', '0007_create_device_firmware'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='upgradeoperation',
+            name='image',
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                to=settings.FIRMWARE_UPGRADER_FIRMWAREIMAGE_MODEL,
+            ),
+        ),
+    ]