Soft delete photos and track filesize statistics (#132)

* Add soft deletion of photos

* Update time metric for deleted photos

* Update tests for soft deletion

* Add photo storage statistics to dashboard

app/core/management/commands/delete_invalid_photos.py

@@ -22,10 +22,7 @@ def handle(self, *args, **options):
             seconds=(int(settings.AWS_PRESIGNED_EXPIRY) + 60)
         )
 
-        invalid_photos = TripPhoto.objects.filter(
-            is_valid=False,
-            added__lte=td,
-        )
+        invalid_photos = TripPhoto.objects.invalid().filter(added__lte=td)
 
         count = invalid_photos.count()
         invalid_photos.delete()

app/logger/admin.py

@@ -15,7 +15,7 @@ class TripPhotoInline(admin.TabularInline):
     extra = 0
     max_num = 0
     show_change_link = True
-    fields = ("photo", "caption", "is_valid", "taken")
+    fields = ("photo", "caption", "is_valid", "deleted_at", "taken")
 
     def has_add_permission(self, request, obj=None):
         return False
@@ -132,9 +132,9 @@ class TripAdmin(admin.ModelAdmin):
 
 @admin.register(TripPhoto)
 class TripPhotoAdmin(admin.ModelAdmin):
-    list_display = ("user", "trip", "taken", "added")
+    list_display = ("user", "trip", "deleted_at", "taken", "added")
     list_display_links = ("taken",)
-    list_filter = ("is_valid", "added")
+    list_filter = ("is_valid", "deleted_at", "added")
     search_fields = ("user__username", "user__name", "user__email", "trip__uuid")
     search_help_text = "Search by author name, email or username, or trip UUID."
     autocomplete_fields = ("trip", "user")
@@ -148,6 +148,7 @@ class TripPhotoAdmin(admin.ModelAdmin):
                     "user",
                     "trip",
                     "is_valid",
+                    "deleted_at",
                     "uuid",
                     "taken",
                     "added",

app/logger/migrations/0019_tripphoto_deleted_at.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.3 on 2023-07-10 17:52
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("logger", "0018_alter_tripphoto_photo"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="tripphoto",
+            name="deleted_at",
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+    ]

app/logger/migrations/0020_alter_tripphoto_trip_alter_tripphoto_user.py

@@ -0,0 +1,34 @@
+# Generated by Django 4.2.3 on 2023-07-10 18:07
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ("logger", "0019_tripphoto_deleted_at"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="tripphoto",
+            name="trip",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="photos",
+                to="logger.trip",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="tripphoto",
+            name="user",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                to=settings.AUTH_USER_MODEL,
+            ),
+        ),
+    ]

app/logger/migrations/0021_tripphoto_filesize.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.3 on 2023-07-10 18:55
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("logger", "0020_alter_tripphoto_trip_alter_tripphoto_user"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="tripphoto",
+            name="filesize",
+            field=models.IntegerField(blank=True, null=True),
+        ),
+    ]

app/logger/migrations/0022_alter_tripphoto_filesize.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.3 on 2023-07-10 19:00
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("logger", "0021_tripphoto_filesize"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="tripphoto",
+            name="filesize",
+            field=models.IntegerField(default=0),
+        ),
+    ]

app/logger/models/trip.py

@@ -464,7 +464,9 @@ def custom_fields(self) -> tuple[(str, str)]:
 
     @property
     def valid_photos(self):
-        return self.photos.filter(is_valid=True).order_by("taken", "added")
+        return self.photos.filter(is_valid=True, deleted_at=None).order_by(
+            "taken", "added"
+        )
 
     @property
     def feed_photos(self):

app/logger/models/tripphoto.py

@@ -8,16 +8,33 @@
 from .trip import Trip
 
 
+class TripPhotoManager(models.Manager):
+    def invalid(self):
+        return self.filter(is_valid=False, deleted_at=None)
+
+    def valid(self):
+        return self.filter(is_valid=True, deleted_at=None)
+
+    def deleted(self):
+        return self.filter(deleted_at__isnull=False)
+
+
 def trip_photo_upload_path(instance, filename):
     """Returns the path to upload trip photos to"""
     original_filename, ext = os.path.splitext(filename)
     return f"{instance.user.uuid}/{instance.trip.uuid}/{instance.uuid}{ext}"
 
 
 class TripPhoto(models.Model):
+    objects = TripPhotoManager()
+
     uuid = models.UUIDField("UUID", default=uuid.uuid4, unique=True)
-    trip = models.ForeignKey(Trip, related_name="photos", on_delete=models.CASCADE)
-    user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
+    trip = models.ForeignKey(
+        Trip, related_name="photos", on_delete=models.SET_NULL, null=True
+    )
+    user = models.ForeignKey(
+        settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True
+    )
     photo = models.ImageField(
         storage=storages["photos"],
         upload_to=trip_photo_upload_path,
@@ -30,6 +47,8 @@ class TripPhoto(models.Model):
     taken = models.DateTimeField(blank=True, null=True)
     added = models.DateTimeField(auto_now_add=True)
     updated = models.DateTimeField(auto_now=True)
+    deleted_at = models.DateTimeField(blank=True, null=True)
+    filesize = models.IntegerField(default=0)
 
     def __str__(self):
         return f"Photo for {self.trip} by {self.user}"

app/logger/services.py

@@ -61,8 +61,14 @@ def get_trips_context(request, ordering, page=1):
                     pk=request.user.pk, liked_trips=OuterRef("pk")
                 ).only("pk")
             ),
-            has_photos=Exists(TripPhoto.objects.filter(trip=OuterRef("pk")).only("pk")),
-            photo_count=Count("photos", distinct=True),
+            has_photos=Exists(
+                TripPhoto.objects.valid().filter(trip=OuterRef("pk")).only("pk")
+            ),
+            photo_count=Count(
+                "photos",
+                filter=Q(photos__is_valid=True, photos__deleted_at=None),
+                distinct=True,
+            ),
             more_than_five_photos=Case(
                 When(photo_count__gt=5, then=Value(True)),
             ),

app/logger/tests/test_trip_photos.py

@@ -184,7 +184,7 @@ def test_trip_photo_delete(self):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "The photo has been deleted.")
 
-        qs = TripPhoto.objects.filter(uuid=uuid)
+        qs = TripPhoto.objects.valid().filter(uuid=uuid)
         self.assertEqual(qs.count(), 0)
 
     @tag("privacy")
@@ -201,7 +201,7 @@ def test_trip_photo_delete_as_other_user(self):
         )
         self.assertEqual(response.status_code, 403)
 
-        qs = TripPhoto.objects.filter(uuid=uuid)
+        qs = TripPhoto.objects.valid().filter(uuid=uuid)
         self.assertEqual(qs.count(), 1)
 
     def test_trip_photo_delete_invalid_uuid(self):
@@ -278,7 +278,7 @@ def test_trip_photo_delete_all(self):
             )
             photo.is_valid = True
             photo.save()
-        self.assertEqual(TripPhoto.objects.filter(trip=self.trip).count(), 10)
+        self.assertEqual(TripPhoto.objects.valid().filter(trip=self.trip).count(), 10)
 
         self.client.force_login(self.user)
         response = self.client.post(
@@ -287,19 +287,19 @@ def test_trip_photo_delete_all(self):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "All photos for the trip have been deleted.")
 
-        qs = TripPhoto.objects.filter(trip=self.trip)
+        qs = TripPhoto.objects.valid().filter(trip=self.trip)
         self.assertEqual(qs.count(), 0)
 
     @tag("privacy")
     def test_trip_photo_delete_all_as_other_user(self):
         """Test deleting all photos as another user"""
-        self.assertEqual(TripPhoto.objects.filter(trip=self.trip).count(), 1)
+        self.assertEqual(TripPhoto.objects.valid().filter(trip=self.trip).count(), 1)
         self.client.force_login(self.user2)
         response = self.client.post(
             reverse("log:trip_photos_delete_all", args=[self.trip.uuid]),
         )
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(TripPhoto.objects.filter(trip=self.trip).count(), 1)
+        self.assertEqual(TripPhoto.objects.valid().filter(trip=self.trip).count(), 1)
 
     def test_trip_photo_str_method(self):
         """Test the string representation of a trip photo"""

app/logger/views/tripphotos.py

@@ -11,6 +11,7 @@
 from django.http import JsonResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse
+from django.utils import timezone
 from django.utils.timezone import make_aware
 from django.views.generic import FormView, View
 
@@ -128,6 +129,7 @@ def post(self, request, *args, **kwargs):
                     )
                 )
 
+        photo.filesize = photo.photo.size
         photo.is_valid = True
         photo.save()
         return JsonResponse({"success": True})
@@ -158,10 +160,11 @@ def post(self, request, *args, **kwargs):
         if not photo.user == request.user:
             raise PermissionDenied
 
-        redirect_url = photo.trip.get_absolute_url()
-        photo.delete()
+        photo.deleted_at = timezone.now()
+        photo.save()
+
         messages.success(request, "The photo has been deleted.")
-        return redirect(redirect_url)
+        return redirect(photo.trip.get_absolute_url())
 
 
 class TripPhotosDeleteAll(LoginRequiredMixin, View):
@@ -170,13 +173,9 @@ def post(self, request, *args, **kwargs):
         if not trip.user == request.user:
             raise PermissionDenied
 
-        qs = TripPhoto.objects.filter(
-            trip=trip,
-            user=request.user,
-        )
-
+        qs = TripPhoto.objects.valid().filter(trip=trip, user=request.user)
         if qs.exists():
-            qs.delete()
+            qs.update(deleted_at=timezone.now())
             messages.success(request, "All photos for the trip have been deleted.")
         else:
             messages.error(request, "There were no photos to delete.")

app/staff/statistics.py

@@ -11,6 +11,7 @@ class Statistics:
     month: int
     year: int
     total: int
+    filesize: bool = False
 
 
 def get_time_statistics(queryset, metric="New", lookup="added__gte"):
@@ -29,3 +30,34 @@ def get_time_statistics(queryset, metric="New", lookup="added__gte"):
         year=queryset.filter(**{lookup: year}).count(),
         total=queryset.count(),
     )
+
+
+def _add_up_fields(queryset, field):
+    """
+    Get the filesize of a FileField or ImageField on each object
+    in a given QuerySet and return the total
+    """
+    total_size = 0
+    for obj in queryset:
+        total_size += getattr(obj, field)
+
+    return total_size
+
+
+def get_integer_field_statistics(queryset, metric, field, lookup="added__gte"):
+    now = timezone.now()
+    day = now - timezone.timedelta(days=1)
+    week = now - timezone.timedelta(days=7)
+    month = now - timezone.timedelta(days=30)
+    year = now - timezone.timedelta(days=365)
+
+    return Statistics(
+        model_name=queryset.model._meta.verbose_name_plural,
+        metric=metric,
+        day=_add_up_fields(queryset.filter(**{lookup: day}), field),
+        week=_add_up_fields(queryset.filter(**{lookup: week}), field),
+        month=_add_up_fields(queryset.filter(**{lookup: month}), field),
+        year=_add_up_fields(queryset.filter(**{lookup: year}), field),
+        total=_add_up_fields(queryset, field),
+        filesize=True,
+    )

app/staff/views.py

@@ -3,7 +3,7 @@
 from logger.models import Trip, TripPhoto, TripReport
 
 from .mixins import StaffRequiredMixin
-from .statistics import get_time_statistics
+from .statistics import get_integer_field_statistics, get_time_statistics
 
 User = get_user_model()
 
@@ -17,16 +17,23 @@ def get_context_data(self, *args, **kwargs):
         trips = Trip.objects.all()
         trip_reports = TripReport.objects.all()
         users = User.objects.all()
-        photos = TripPhoto.objects.all()
-        photos_valid = photos.filter(is_valid=True)
+        photos = TripPhoto.objects.filter(is_valid=True)
+        photos_valid = TripPhoto.objects.valid()
+        photos_deleted = TripPhoto.objects.deleted()
 
         statistics = [
             get_time_statistics(trips),
             get_time_statistics(trips, metric="Updated", lookup="updated__gte"),
             get_time_statistics(trip_reports),
             get_time_statistics(trip_reports, metric="Updated", lookup="updated__gte"),
-            get_time_statistics(photos, metric="New", lookup="added__gte"),
             get_time_statistics(photos_valid, metric="Valid", lookup="added__gte"),
+            get_time_statistics(
+                photos_deleted, metric="Deleted", lookup="deleted_at__gte"
+            ),
+            get_integer_field_statistics(photos, "Storage", "filesize"),
+            get_integer_field_statistics(
+                photos_deleted, "Deleted storage", "filesize", "deleted_at__gte"
+            ),
             get_time_statistics(users, metric="New", lookup="date_joined__gte"),
             get_time_statistics(users, metric="Active", lookup="last_seen__gte"),
         ]