feat: move map to trash on delete

This also:
- adds a `umap empty_trash` command
- remove the previous purgatory concept

docs/config/settings.md

@@ -287,14 +287,6 @@ How many total maps to return in the search.
 
 How many maps to show in the user "my maps" page.
 
-#### UMAP_PURGATORY_ROOT
-
-Path where files are moved when a datalayer is deleted. They will stay there until
-`umap purge_purgatory` is run. May be useful in case a user deletes by mistake
-a datalayer, or even a map.
-Default is `/tmp/umappurgatory/`, so beware that this folder will be deleted on
-each server restart.
-
 #### UMAP_SEARCH_CONFIGURATION
 
 Use it if you take control over the search configuration.

umap/management/commands/empty_trash.py

@@ -0,0 +1,32 @@
+from datetime import UTC, datetime, timedelta
+
+from django.core.management.base import BaseCommand
+
+from umap.models import Map
+
+
+class Command(BaseCommand):
+    help = "Remove maps in trash. Eg.: umap purge_deleted --days 7"
+
+    def add_arguments(self, parser):
+        parser.add_argument(
+            "--days",
+            help="Number of days to consider maps for removal",
+            default=30,
+            type=int,
+        )
+        parser.add_argument(
+            "--dry-run",
+            help="Pretend to delete but just report",
+            action="store_true",
+        )
+
+    def handle(self, *args, **options):
+        days = options["days"]
+        since = datetime.now(UTC) - timedelta(days=days)
+        print(f"Deleting map in trash since {since}")
+        maps = Map.objects.filter(share_status=Map.DELETED, modified_at__lt=since)
+        for map in maps:
+            if not options["dry_run"]:
+                map.delete()
+            print(f"Deleted map {map.name} ({map.id}), trashed on {map.modified_at}")

umap/management/commands/purge_purgatory.py

@@ -1,28 +0,0 @@
-import time
-from pathlib import Path
-
-from django.conf import settings
-from django.core.management.base import BaseCommand
-
-
-class Command(BaseCommand):
-    help = "Remove old files from purgatory. Eg.: umap purge_purgatory --days 7"
-
-    def add_arguments(self, parser):
-        parser.add_argument(
-            "--days",
-            help="Number of days to consider files for removal",
-            default=30,
-            type=int,
-        )
-
-    def handle(self, *args, **options):
-        days = options["days"]
-        root = Path(settings.UMAP_PURGATORY_ROOT)
-        threshold = time.time() - days * 86400
-        for path in root.iterdir():
-            stats = path.stat()
-            filestamp = stats.st_mtime
-            if filestamp < threshold:
-                path.unlink()
-                print(f"Removed old file {path}")

umap/models.py

@@ -276,6 +276,10 @@ class Map(NamedModel):
         )
         return map_settings
 
+    def move_to_trash(self):
+        self.share_status = Map.DELETED
+        self.save()
+
     def delete(self, **kwargs):
         # Explicitely call datalayers.delete, so we can deal with removing files
         # (the cascade delete would not call the model delete method)
@@ -513,17 +517,8 @@ class DataLayer(NamedModel):
 
     def delete(self, **kwargs):
         self.purge_gzip()
-        self.to_purgatory()
         return super().delete(**kwargs)
 
-    def to_purgatory(self):
-        dest = Path(settings.UMAP_PURGATORY_ROOT)
-        dest.mkdir(parents=True, exist_ok=True)
-        src = Path(self.geojson.storage.location) / self.storage_root()
-        for version in self.versions:
-            name = version["name"]
-            shutil.move(src / name, dest / f"{self.map.pk}_{name}")
-
     def upload_to(self):
         root = self.storage_root()
         name = "%s_%s.geojson" % (self.pk, int(time.time() * 1000))

umap/settings/base.py

@@ -272,7 +272,6 @@ UMAP_DEFAULT_FEATURES_HAVE_OWNERS = False
 UMAP_HOME_FEED = "latest"
 UMAP_IMPORTERS = {}
 UMAP_HOST_INFOS = {}
-UMAP_PURGATORY_ROOT = "/tmp/umappurgatory"
 UMAP_LABEL_KEYS = ["name", "title"]
 
 UMAP_READONLY = env("UMAP_READONLY", default=False)

umap/tests/integration/test_anonymous_owned_map.py

@@ -240,7 +240,7 @@ def test_anonymous_owner_can_delete_the_map(anonymap, live_server, owner_session
     owner_session.get_by_role("button", name="Delete").click()
     with owner_session.expect_response(re.compile(r".*/update/delete/.*")):
         owner_session.get_by_role("button", name="OK").click()
-    assert not Map.objects.count()
+    assert Map.objects.get(pk=anonymap.pk).share_status == Map.DELETED
 
 
 def test_non_owner_cannot_see_delete_button(anonymap, live_server, page):

umap/tests/integration/test_dashboard.py

@@ -22,7 +22,7 @@ def test_owner_can_delete_map_after_confirmation(map, live_server, login):
     with page.expect_navigation():
         delete_button.click()
     assert dialog_shown
-    assert Map.objects.all().count() == 0
+    assert Map.objects.get(pk=map.pk).share_status == Map.DELETED
 
 
 def test_dashboard_map_preview(map, live_server, datalayer, login):

umap/tests/integration/test_owned_map.py

@@ -141,7 +141,7 @@ def test_owner_has_delete_map_button(map, live_server, login):
     delete.click()
     with page.expect_navigation():
         page.get_by_role("button", name="OK").click()
-    assert Map.objects.all().count() == 0
+    assert Map.objects.get(pk=map.pk).share_status == Map.DELETED
 
 
 def test_editor_do_not_have_delete_map_button(map, live_server, login, user):

umap/tests/test_datalayer.py

@@ -273,7 +273,6 @@ def test_anonymous_can_edit_in_inherit_mode_and_map_in_public_mode(
 
 
 def test_should_remove_all_versions_on_delete(map, settings):
-    settings.UMAP_PURGATORY_ROOT = tempfile.mkdtemp()
     datalayer = DataLayerFactory(uuid="0f1161c0-c07f-4ba4-86c5-8d8981d8a813", old_id=17)
     root = Path(datalayer.storage_root())
     before = len(datalayer.geojson.storage.listdir(root)[1])
@@ -292,4 +291,3 @@ def test_should_remove_all_versions_on_delete(map, settings):
     datalayer.delete()
     found = datalayer.geojson.storage.listdir(root)[1]
     assert found == [other, f"{other}.gz"]
-    assert len(list(Path(settings.UMAP_PURGATORY_ROOT).iterdir())) == 4 + before

umap/tests/test_empty_trash.py

@@ -0,0 +1,34 @@
+from datetime import UTC, datetime, timedelta
+from unittest import mock
+
+import pytest
+from django.core.management import call_command
+
+from umap.models import Map
+
+from .base import MapFactory
+
+pytestmark = pytest.mark.django_db
+
+
+def test_purge_purgatory(user):
+    recent = MapFactory(owner=user)
+    recent_deleted = MapFactory(owner=user)
+    recent_deleted.move_to_trash()
+    recent_deleted.save()
+    with mock.patch("django.utils.timezone.now") as mocked:
+        mocked.return_value = datetime.now(UTC) - timedelta(days=8)
+        old_deleted = MapFactory(owner=user)
+        old_deleted.move_to_trash()
+        old_deleted.save()
+        old = MapFactory(owner=user)
+    assert Map.objects.count() == 4
+    call_command("empty_trash", "--days=7", "--dry-run")
+    assert Map.objects.count() == 4
+    call_command("empty_trash", "--days=9")
+    assert Map.objects.count() == 4
+    call_command("empty_trash", "--days=7")
+    assert not Map.objects.filter(pk=old_deleted.pk)
+    assert Map.objects.filter(pk=old.pk)
+    assert Map.objects.filter(pk=recent.pk)
+    assert Map.objects.filter(pk=recent_deleted.pk)

umap/tests/test_map.py

@@ -167,3 +167,10 @@ def test_can_change_default_share_status(user, settings):
     map = Map.objects.create(owner=user, center=DEFAULT_CENTER)
     map = MapFactory(owner=user)
     assert map.share_status == Map.PUBLIC
+
+
+def test_move_to_trash(user, map):
+    map.move_to_trash()
+    map.save()
+    reloaded = Map.objects.get(pk=map.pk)
+    assert reloaded.share_status == Map.DELETED

umap/tests/test_map_views.py

@@ -114,8 +114,10 @@ def test_delete(client, map, datalayer):
         url, headers={"X-Requested-With": "XMLHttpRequest"}, follow=True
     )
     assert response.status_code == 200
-    assert not Map.objects.filter(pk=map.pk).exists()
-    assert not DataLayer.objects.filter(pk=datalayer.pk).exists()
+    assert Map.objects.filter(pk=map.pk).exists()
+    assert DataLayer.objects.filter(pk=datalayer.pk).exists()
+    reloaded = Map.objects.get(pk=map.pk)
+    assert reloaded.share_status == Map.DELETED
     # Check that user has not been impacted
     assert User.objects.filter(pk=map.owner.pk).exists()
     # Test response is a json
@@ -406,7 +408,9 @@ def test_anonymous_delete(cookieclient, anonymap):
         url, headers={"X-Requested-With": "XMLHttpRequest"}, follow=True
     )
     assert response.status_code == 200
-    assert not Map.objects.filter(pk=anonymap.pk).count()
+    assert Map.objects.filter(pk=anonymap.pk).exists()
+    reloaded = Map.objects.get(pk=anonymap.pk)
+    assert reloaded.share_status == Map.DELETED
     # Test response is a json
     j = json.loads(response.content.decode())
     assert "redirect" in j

umap/tests/test_purge_purgatory.py

@@ -1,25 +0,0 @@
-import os
-import tempfile
-from pathlib import Path
-
-from django.core.management import call_command
-
-
-def test_purge_purgatory(settings):
-    settings.UMAP_PURGATORY_ROOT = tempfile.mkdtemp()
-    root = Path(settings.UMAP_PURGATORY_ROOT)
-    old = root / "old.json"
-    old.write_text("{}")
-    stat = old.stat()
-    os.utime(old, times=(stat.st_mtime - 31 * 86400, stat.st_mtime - 31 * 86400))
-    recent = root / "recent.json"
-    recent.write_text("{}")
-    stat = recent.stat()
-    os.utime(recent, times=(stat.st_mtime - 8 * 86400, stat.st_mtime - 8 * 86400))
-    now = root / "now.json"
-    now.write_text("{}")
-    assert {f.name for f in root.iterdir()} == {"old.json", "recent.json", "now.json"}
-    call_command("purge_purgatory")
-    assert {f.name for f in root.iterdir()} == {"recent.json", "now.json"}
-    call_command("purge_purgatory", "--days=7")
-    assert {f.name for f in root.iterdir()} == {"now.json"}

umap/views.py

@@ -1022,7 +1022,7 @@ class MapDelete(DeleteView):
         self.object = self.get_object()
         if not self.object.can_delete(self.request):
             return HttpResponseForbidden(_("Only its owner can delete the map."))
-        self.object.delete()
+        self.object.move_to_trash()
         home_url = reverse("home")
         messages.info(self.request, _("Map successfully deleted."))
         if is_ajax(self.request):