From fc8f60e98d25973a68f48c52b62469a0825755a8 Mon Sep 17 00:00:00 2001 From: mrmn2 <188248364+mrmn2@users.noreply.github.com> Date: Mon, 19 Jan 2026 11:57:53 +0100 Subject: [PATCH 1/2] feat: Delete workspace folder when workspace is deleted --- pdfding/pdf/models/helpers.py | 27 +++++++++++++++++++ pdfding/pdf/models/pdf_models.py | 15 +++-------- pdfding/pdf/models/shared_pdf_models.py | 5 ++-- pdfding/pdf/models/workspace_models.py | 16 +++++++++++ pdfding/pdf/tests/test_migrations.py | 4 +-- pdfding/pdf/tests/test_models/test_helpers.py | 21 +++++++++++++++ .../pdf/tests/test_models/test_pdf_models.py | 15 +---------- .../test_models/test_workspace_models.py | 13 +++++++++ 8 files changed, 87 insertions(+), 29 deletions(-) create mode 100644 pdfding/pdf/models/helpers.py create mode 100644 pdfding/pdf/tests/test_models/test_helpers.py diff --git a/pdfding/pdf/models/helpers.py b/pdfding/pdf/models/helpers.py new file mode 100644 index 00000000..766469e0 --- /dev/null +++ b/pdfding/pdf/models/helpers.py @@ -0,0 +1,27 @@ +from pathlib import Path + +from core.settings import MEDIA_ROOT + + +def get_workspace_path(workspace) -> Path: # pragma: no cover + """Get the path of a workspace.""" + + return MEDIA_ROOT / get_workspace_dir(workspace) + + +def get_collection_path(collection) -> Path: # pragma: no cover + """Get the path of a collection.""" + + return MEDIA_ROOT / get_collection_dir(collection) + + +def get_workspace_dir(workspace) -> str: + """Get the directory of a workspace.""" + + return workspace.id + + +def get_collection_dir(collection) -> str: + """Get the directory of a collection.""" + + return f'{get_workspace_dir(collection.workspace)}/{collection.name.lower()}' diff --git a/pdfding/pdf/models/pdf_models.py b/pdfding/pdf/models/pdf_models.py index 659d58be..0acdcff0 100644 --- a/pdfding/pdf/models/pdf_models.py +++ b/pdfding/pdf/models/pdf_models.py @@ -11,18 +11,11 @@ from django.utils.safestring import mark_safe from django.utils.text import slugify from pdf.models.collection_models import Collection +from pdf.models.helpers import get_collection_dir from pdf.models.tag_models import Tag from pdf.models.workspace_models import Workspace -def get_pdf_parent_dirs(instance) -> str: - """Get the parent directories (worskpace_id/collection_name) of a PDF file.""" - - parent_dirs = f'{instance.workspace.id}/{instance.collection.name.lower()}' - - return parent_dirs - - def get_file_path(instance, _) -> str: """ Get the file path for a PDF inside the media root based on the pdf name. File paths are @@ -51,7 +44,7 @@ def get_file_path(instance, _) -> str: else: file_path = '/'.join(['pdf', file_name]) - file_path = f'{get_pdf_parent_dirs(instance)}/{file_path}' + file_path = f'{get_collection_dir(instance.collection)}/{file_path}' existing_pdf = Pdf.objects.filter(file=file_path).first() # make sure there each file path is unique @@ -87,7 +80,7 @@ def get_thumbnail_path(instance, _) -> str: """Get the file path for the thumbnail of a PDF.""" file_name = f'thumbnails/{instance.id}.png' - file_path = f'{get_pdf_parent_dirs(instance)}/{file_name}' + file_path = f'{get_collection_dir(instance.collection)}/{file_name}' return file_path @@ -96,7 +89,7 @@ def get_preview_path(instance, _) -> str: """Get the file path for the preview of a PDF.""" file_name = f'previews/{instance.id}.png' - file_path = f'{get_pdf_parent_dirs(instance)}/{file_name}' + file_path = f'{get_collection_dir(instance.collection)}/{file_name}' return file_path diff --git a/pdfding/pdf/models/shared_pdf_models.py b/pdfding/pdf/models/shared_pdf_models.py index e968346d..fce4cde4 100644 --- a/pdfding/pdf/models/shared_pdf_models.py +++ b/pdfding/pdf/models/shared_pdf_models.py @@ -3,14 +3,15 @@ from django.contrib.humanize.templatetags.humanize import naturaltime from django.db import models -from pdf.models.pdf_models import Pdf, get_pdf_parent_dirs +from pdf.models.helpers import get_collection_dir +from pdf.models.pdf_models import Pdf def get_qrcode_file_path(instance, _) -> str: """Get the file path for the qr code of a shared PDF.""" file_name = f'qr/{instance.id}.svg' - file_path = f'{get_pdf_parent_dirs(instance.pdf)}/{file_name}' + file_path = f'{get_collection_dir(instance.pdf.collection)}/{file_name}' return str(file_path) diff --git a/pdfding/pdf/models/workspace_models.py b/pdfding/pdf/models/workspace_models.py index 04f2c1a9..7f6c5057 100644 --- a/pdfding/pdf/models/workspace_models.py +++ b/pdfding/pdf/models/workspace_models.py @@ -1,7 +1,9 @@ +from shutil import rmtree from uuid import uuid4 from django.contrib.auth.models import User from django.db import models +from pdf.models.helpers import get_workspace_path def get_uuid4_str() -> str: @@ -31,6 +33,20 @@ class Workspace(models.Model): def __str__(self): # pragma: no cover return str(self.name) + def delete(self, *args, **kwargs) -> None: + """ + Override default delete method so that workspace directory gets deleted after the workspace is deleted. + """ + + ws_path = get_workspace_path(self) + super().delete(*args, **kwargs) + + try: + rmtree(ws_path) + except Exception as e: + print(e) + pass + @property def users(self) -> models.QuerySet[User]: """Get the users of the workspace.""" diff --git a/pdfding/pdf/tests/test_migrations.py b/pdfding/pdf/tests/test_migrations.py index 59a4b397..e37e119b 100644 --- a/pdfding/pdf/tests/test_migrations.py +++ b/pdfding/pdf/tests/test_migrations.py @@ -177,8 +177,8 @@ def test_fill_collections_workspaces(self): self.assertEqual(changed_pdf.collection, profile.collections[0]) self.assertEqual(changed_tag.workspace, workspace) - @patch('pdf.models.shared_pdf_models.get_pdf_parent_dirs') - @patch('pdf.models.pdf_models.get_pdf_parent_dirs') + @patch('pdf.models.shared_pdf_models.get_collection_dir') + @patch('pdf.models.pdf_models.get_collection_dir') def test_adjust_file_paths_to_ws_collection(self, mock_get_parent_dirs, mock_shared_get_parent_dirs): self.pdf.delete() # we need to delete the pdf created by setUp user = User.objects.create_user(username='user', password='12345') diff --git a/pdfding/pdf/tests/test_models/test_helpers.py b/pdfding/pdf/tests/test_models/test_helpers.py new file mode 100644 index 00000000..1fbabccd --- /dev/null +++ b/pdfding/pdf/tests/test_models/test_helpers.py @@ -0,0 +1,21 @@ +from django.contrib.auth.models import User +from django.test import TestCase +from pdf.models.helpers import get_collection_dir +from pdf.models.pdf_models import Pdf +from pdf.services.workspace_services import create_collection, create_workspace + + +class TestHelpers(TestCase): + def setUp(self): + self.user = User.objects.create_user(username='testuser', password='12345') + self.pdf = Pdf(collection=self.user.profile.current_collection, name='pdf') + + def test_get_collection_dir(self): + ws = create_workspace('bla', self.user) + ws.id = '12345' + ws.save() + collection = create_collection(ws, 'Test') + + expected_path = '12345/test' + + self.assertEqual(expected_path, get_collection_dir(collection)) diff --git a/pdfding/pdf/tests/test_models/test_pdf_models.py b/pdfding/pdf/tests/test_models/test_pdf_models.py index a73090fd..83b24d97 100644 --- a/pdfding/pdf/tests/test_models/test_pdf_models.py +++ b/pdfding/pdf/tests/test_models/test_pdf_models.py @@ -9,12 +9,10 @@ convert_to_natural_age, delete_empty_dirs_after_rename_or_delete, get_file_path, - get_pdf_parent_dirs, get_preview_path, get_thumbnail_path, ) - -from pdfding.pdf.services.workspace_services import create_collection, create_workspace +from pdf.services.workspace_services import create_workspace class TestPdf(TestCase): @@ -63,17 +61,6 @@ def test_delete_with_file_directory(self, mock_delete_empty_dirs_after_rename_or file_name, collection.workspace.id, collection.name ) - def test_get_pdf_parent_dirs(self): - ws = create_workspace('bla', self.user) - ws.id = '12345' - ws.save() - collection = create_collection(ws, 'Test') - pdf = Pdf.objects.create(name='asd', collection=collection) - - expected_parent_dir = '12345/test' - - self.assertEqual(expected_parent_dir, get_pdf_parent_dirs(pdf)) - def test_get_file_path(self): collection = self.user.profile.current_collection pdf = Pdf(collection=collection, name='PDF_3! 寝る 12/3? ') diff --git a/pdfding/pdf/tests/test_models/test_workspace_models.py b/pdfding/pdf/tests/test_models/test_workspace_models.py index 859ee2ef..65717ac3 100644 --- a/pdfding/pdf/tests/test_models/test_workspace_models.py +++ b/pdfding/pdf/tests/test_models/test_workspace_models.py @@ -1,5 +1,6 @@ from django.contrib.auth.models import User from django.test import TestCase +from pdf.models.helpers import get_workspace_path from pdf.models.workspace_models import WorkspaceError, WorkspaceRoles, WorkspaceUser from pdf.services.workspace_services import create_collection, create_workspace @@ -10,6 +11,18 @@ def setUp(self): self.ws = create_workspace(name='ws', creator=self.user) self.other_user = User.objects.create_user(username='other_user', password='password') + def test_delete(self): + ws_path = get_workspace_path(self.ws) + pdfs_path = ws_path / 'some_collection' / 'pdf' + pdfs_path.mkdir(parents=True) + + dummy_file_path = pdfs_path / 'dummy.pdf' + dummy_file_path.touch() + + self.ws.delete() + + self.assertFalse(ws_path.exists()) + def test_user_property(self): self.assertEqual(self.ws.users.count(), 1) From 256d98fdb07628784e2b6d1ac8119c30309343fa Mon Sep 17 00:00:00 2001 From: mrmn2 <188248364+mrmn2@users.noreply.github.com> Date: Tue, 20 Jan 2026 10:54:32 +0100 Subject: [PATCH 2/2] feat: Delete collections --- pdfding/e2e/test_collection_e2e.py | 31 +++++++++++++ pdfding/e2e/test_workspace_e2e.py | 1 - pdfding/pdf/models/collection_models.py | 19 ++++++++ pdfding/pdf/models/workspace_models.py | 3 +- pdfding/pdf/templates/collection_details.html | 10 ++--- .../templates/partials/delete_collection.html | 19 ++++++++ .../test_models/test_collection_models.py | 14 ++++++ .../tests/test_views/test_collection_views.py | 45 +++++++++++++++++++ .../tests/test_views/test_workspace_views.py | 1 - pdfding/pdf/urls.py | 2 +- pdfding/pdf/views/collection_views.py | 35 ++++++++++++++- 11 files changed, 169 insertions(+), 11 deletions(-) create mode 100644 pdfding/pdf/templates/partials/delete_collection.html diff --git a/pdfding/e2e/test_collection_e2e.py b/pdfding/e2e/test_collection_e2e.py index 7718227d..fbde0fde 100644 --- a/pdfding/e2e/test_collection_e2e.py +++ b/pdfding/e2e/test_collection_e2e.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import User from django.urls import reverse from helpers import PdfDingE2ETestCase +from pdf.models.collection_models import Collection from pdf.services.workspace_services import create_collection from playwright.sync_api import expect, sync_playwright @@ -75,3 +76,33 @@ def test_details_change_collection(self): self.page.locator("#current_collection_name").click() self.page.get_by_text("other_collection").click() expect(self.page.locator("#current_collection_name")).to_contain_text("other_collection") + + def test_cancel_delete(self): + other_collection = create_collection(self.user.profile.current_workspace, 'other_collection') + + with sync_playwright() as p: + self.open(reverse('collection_details', kwargs={'identifier': other_collection.id}), p) + + expect(self.page.locator("#delete_collection_modal").first).not_to_be_visible() + self.page.locator("#delete-collection").click() + expect(self.page.locator("#delete_collection_modal").first).to_be_visible() + self.page.locator("#cancel_delete").get_by_text("Cancel").click() + expect(self.page.locator("#delete_collection_modal").first).not_to_be_visible() + + def test_delete(self): + other_collection = create_collection(self.user.profile.current_workspace, 'other_collection') + + with sync_playwright() as p: + self.open(reverse('collection_details', kwargs={'identifier': other_collection.id}), p) + + self.page.locator("#delete-collection").click() + self.page.locator("#confirm_delete").get_by_text("Submit").click() + expect(self.page.locator("#delete_collection_modal").first).not_to_be_visible() + + assert not Collection.objects.filter(id=other_collection.id).count() + + def test_delete_not_visible_default(self): + with sync_playwright() as p: + self.open(reverse('collection_details', kwargs={'identifier': self.user.profile.current_collection_id}), p) + + expect(self.page.locator("#delete-collection").first).not_to_be_visible() diff --git a/pdfding/e2e/test_workspace_e2e.py b/pdfding/e2e/test_workspace_e2e.py index 1668776d..4d8a77bb 100644 --- a/pdfding/e2e/test_workspace_e2e.py +++ b/pdfding/e2e/test_workspace_e2e.py @@ -77,7 +77,6 @@ def test_cancel_delete(self): self.user.profile.save() with sync_playwright() as p: - # only display one pdf self.open(reverse('workspace_details'), p) expect(self.page.locator("#delete_workspace_modal").first).not_to_be_visible() diff --git a/pdfding/pdf/models/collection_models.py b/pdfding/pdf/models/collection_models.py index 75ead13a..a72f6a82 100644 --- a/pdfding/pdf/models/collection_models.py +++ b/pdfding/pdf/models/collection_models.py @@ -1,6 +1,8 @@ +from shutil import rmtree from uuid import uuid4 from django.db import models +from pdf.models.helpers import get_collection_path from pdf.models.workspace_models import Workspace @@ -8,6 +10,10 @@ def get_uuid4_str() -> str: return str(uuid4()) +class CollectionError(Exception): + """Exceptions for collection related problems""" + + class Collection(models.Model): """The model for the collections used for organizing PDF files.""" @@ -21,6 +27,19 @@ class Collection(models.Model): def __str__(self): # pragma: no cover return str(self.name) + def delete(self, *args, **kwargs) -> None: + """ + Override default delete method so that the collection directory gets deleted after the collection is deleted. + """ + + collection_path = get_collection_path(self) + super().delete(*args, **kwargs) + + try: + rmtree(collection_path) + except Exception: # pragma: no cover # nosec B110 + pass + @property def pdfs(self) -> models.QuerySet: """Get the pdfs of the collection""" diff --git a/pdfding/pdf/models/workspace_models.py b/pdfding/pdf/models/workspace_models.py index 7f6c5057..2434a391 100644 --- a/pdfding/pdf/models/workspace_models.py +++ b/pdfding/pdf/models/workspace_models.py @@ -43,8 +43,7 @@ def delete(self, *args, **kwargs) -> None: try: rmtree(ws_path) - except Exception as e: - print(e) + except Exception: # pragma: no cover # nosec B110 pass @property diff --git a/pdfding/pdf/templates/collection_details.html b/pdfding/pdf/templates/collection_details.html index 9cfd06f3..7897f1e7 100644 --- a/pdfding/pdf/templates/collection_details.html +++ b/pdfding/pdf/templates/collection_details.html @@ -1,9 +1,9 @@ {% extends 'layouts/blank.html' %} {% block content %} -
+
+ :class="{ 'opacity-15': show_delete_collection_modal }">
{% include 'includes/workspace_sidebar.html' with page='collection_details' %}
@@ -80,7 +80,7 @@ {% if not collection.default_collection %}
Danger Zone
- Delete Account + Delete Collection
@@ -88,8 +88,8 @@
Delete diff --git a/pdfding/pdf/templates/partials/delete_collection.html b/pdfding/pdf/templates/partials/delete_collection.html new file mode 100644 index 00000000..1b1c8ce1 --- /dev/null +++ b/pdfding/pdf/templates/partials/delete_collection.html @@ -0,0 +1,19 @@ +
+
+

Delete {{ collection_name }}

+
Are you sure you want to delete this collections and all its PDFs? This action cannot be reversed!
+
+ +
+ Cancel +
+
+
+
diff --git a/pdfding/pdf/tests/test_models/test_collection_models.py b/pdfding/pdf/tests/test_models/test_collection_models.py index 869d3143..e8668854 100644 --- a/pdfding/pdf/tests/test_models/test_collection_models.py +++ b/pdfding/pdf/tests/test_models/test_collection_models.py @@ -1,5 +1,6 @@ from django.contrib.auth.models import User from django.test import TestCase +from pdf.models.helpers import get_collection_path from pdf.models.pdf_models import Pdf @@ -7,6 +8,19 @@ class TestWorkspace(TestCase): def setUp(self): self.user = User.objects.create_user(username='user_1', password='password') + def test_delete(self): + collection = self.user.profile.current_collection + collection_path = get_collection_path(collection) + pdfs_path = collection_path / 'pdf' + pdfs_path.mkdir(parents=True, exist_ok=True) + + dummy_file_path = pdfs_path / 'dummy.pdf' + dummy_file_path.touch() + + collection.delete() + + self.assertFalse(collection_path.exists()) + def test_pdf_property(self): collection = self.user.profile.current_collection pdf_1 = Pdf.objects.create(name='pdf_1', collection=collection) diff --git a/pdfding/pdf/tests/test_views/test_collection_views.py b/pdfding/pdf/tests/test_views/test_collection_views.py index f5715607..3cfa48c4 100644 --- a/pdfding/pdf/tests/test_views/test_collection_views.py +++ b/pdfding/pdf/tests/test_views/test_collection_views.py @@ -7,6 +7,7 @@ from django.test import Client, TestCase from django.urls import reverse from pdf.forms import CollectionDescriptionForm, CollectionForm, CollectionNameForm +from pdf.models.collection_models import CollectionError from pdf.services.workspace_services import create_collection, create_workspace from pdf.views import collection_views @@ -119,3 +120,47 @@ def test_process_field_name_existing(self): self.assertEqual( list(messages)[0].message, 'This name is already used by another collection in this workspace!' ) + + +class TestDelete(CollectionTestCase): + def test_delete_get_no_htmx(self): + created_collection = create_collection(self.user.profile.current_workspace, 'created_collection') + + response = self.client.get(reverse('delete_collection', kwargs={'identifier': created_collection.id})) + self.assertRedirects(response, reverse('pdf_overview'), status_code=302) + + def test_delete_get(self): + created_collection = create_collection(self.user.profile.current_workspace, 'created_collection') + headers = {'HTTP_HX-Request': 'true'} + + response = self.client.get( + reverse('delete_collection', kwargs={'identifier': created_collection.id}), **headers + ) + + self.assertEqual(response.context['collection_name'], 'created_collection') + self.assertEqual(response.context['collection_id'], str(created_collection.id)) + self.assertTemplateUsed(response, 'partials/delete_collection.html') + + def test_pre_delete_default_collection(self): + headers = {'HTTP_HX-Request': 'true'} + + with pytest.raises(CollectionError, match='Default collections cannot be deleted!'): + self.client.delete( + reverse('delete_collection', kwargs={'identifier': self.user.profile.current_collection_id}), **headers + ) + + def test_post_delete_current_collection_adjusted(self): + created_collection = create_collection(self.user.profile.current_workspace, 'created_collection') + headers = {'HTTP_HX-Request': 'true'} + + profile = self.user.profile + profile.current_collection_id = created_collection.id + profile.save() + + changed_user = User.objects.get(id=self.user.id) + assert changed_user.profile.current_collection_id == created_collection.id + + self.client.delete(reverse('delete_collection', kwargs={'identifier': created_collection.id}), **headers) + + changed_user = User.objects.get(id=self.user.id) + assert changed_user.profile.current_collection_id == 'all' diff --git a/pdfding/pdf/tests/test_views/test_workspace_views.py b/pdfding/pdf/tests/test_views/test_workspace_views.py index d9e02004..fe000c70 100644 --- a/pdfding/pdf/tests/test_views/test_workspace_views.py +++ b/pdfding/pdf/tests/test_views/test_workspace_views.py @@ -106,7 +106,6 @@ def test_delete_get(self): created_ws = create_workspace('created_ws', self.user) headers = {'HTTP_HX-Request': 'true'} - # archive the pdf response = self.client.get(reverse('delete_workspace', kwargs={'identifier': created_ws.id}), **headers) self.assertEqual(response.context['workspace_name'], 'created_ws') diff --git a/pdfding/pdf/urls.py b/pdfding/pdf/urls.py index be20e33c..c4a307cc 100644 --- a/pdfding/pdf/urls.py +++ b/pdfding/pdf/urls.py @@ -91,6 +91,6 @@ path('collection/create', collection_views.Create.as_view(), name='create_collection'), path('collection/details/', workspace_views.CollectionDetails.as_view(), name='collection_details'), path('collection/edit//', collection_views.Edit.as_view(), name='edit_collection'), - path('collection/delete/', workspace_views.Delete.as_view(), name='delete_collection'), + path('collection/delete/', collection_views.Delete.as_view(), name='delete_collection'), path('', pdf_views.Overview.as_view(), name='collection_overview'), # needed for base views working ] diff --git a/pdfding/pdf/views/collection_views.py b/pdfding/pdf/views/collection_views.py index 8bcc8cdd..f87f0511 100644 --- a/pdfding/pdf/views/collection_views.py +++ b/pdfding/pdf/views/collection_views.py @@ -1,8 +1,9 @@ from base import base_views from django.contrib import messages from django.http import HttpRequest +from django.shortcuts import redirect, render from pdf.forms import CollectionDescriptionForm, CollectionForm, CollectionNameForm -from pdf.models.collection_models import Collection +from pdf.models.collection_models import Collection, CollectionError from pdf.services.collection_services import move_collection from pdf.services.pdf_services import check_object_access_allowed from pdf.services.workspace_services import create_collection @@ -97,3 +98,35 @@ class Edit(EditCollectionMixin, base_views.BaseDetailsEdit): The view for editing a collection's name and description. The field, that is to be changed, is specified by the 'field' argument. """ + + +class Delete(CollectionMixin, base_views.BaseDelete): + """View for deleting the collection specified by its ID.""" + + def get(self, request: HttpRequest, identifier: str): + """Triggered by htmx. Display an inline form for deleting the collection.""" + + if request.htmx: + collection = self.get_object(request, identifier) + + return render( + request, + 'partials/delete_collection.html', + {'collection_id': identifier, 'collection_name': collection.name}, + ) + + return redirect('pdf_overview') + + def pre_delete(self, obj: Collection, request: HttpRequest): + """Execute before deleting object.""" + + if obj.default_collection: + raise CollectionError('Default collections cannot be deleted!') + + def post_delete(self, identifier: str, request: HttpRequest): + """Execute after deleting object.""" + + if identifier == request.user.profile.current_collection_id: + profile = request.user.profile + profile.current_collection_id = 'all' + profile.save()