From fce6d864c800c03757315c679e8deb6e1a4b725c Mon Sep 17 00:00:00 2001 From: 1sakshm Date: Sun, 25 Oct 2026 04:35:38 +0530 Subject: [PATCH] Keep template PDFs that are still in use when deleting a template Deleting a template removed its PDF even when another template pointed at the same file, or when it was the bundled seed PDF, which left the remaining or re-seeded template with a missing file. Fixes #738 --- app/core/config.py | 2 ++ app/db/init_db.py | 3 ++- app/services/template.py | 21 ++++++++++++++++++++- tests/test_deletion.py | 31 +++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/app/core/config.py b/app/core/config.py index 83eaa1a..07d3e0e 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -24,6 +24,8 @@ # Directory new uploads land in, as a project-relative string (was "src/inputs" # before the restructure). Override with FIREFORM_TEMPLATE_DIR. DEFAULT_TEMPLATE_DIR = os.getenv("FIREFORM_TEMPLATE_DIR", "data/inputs") +# Bundled PDF behind the seeded "Manual Test Template"; never deleted with a template. +SEED_TEMPLATE_PDF_PATH = "data/inputs/file_template_manual.pdf" # --- Database ------------------------------------------------------------- DATABASE_URL = os.getenv( diff --git a/app/db/init_db.py b/app/db/init_db.py index 7d88995..ffda9e5 100644 --- a/app/db/init_db.py +++ b/app/db/init_db.py @@ -7,6 +7,7 @@ from sqlmodel import Session, select, text from alembic import command +from app.core.config import SEED_TEMPLATE_PDF_PATH from app.db.database import engine from app.models import Template @@ -69,7 +70,7 @@ def seed_db(): "their contact details, and their signature." ), fields=fields, - pdf_path="data/inputs/file_template_manual.pdf", + pdf_path=SEED_TEMPLATE_PDF_PATH, created_at=datetime.datetime.now(datetime.timezone.utc), ) session.add(default_template) diff --git a/app/services/template.py b/app/services/template.py index 797b603..2931826 100644 --- a/app/services/template.py +++ b/app/services/template.py @@ -12,6 +12,7 @@ TemplateUploadResponse, ) from app.core import paths +from app.core.config import SEED_TEMPLATE_PDF_PATH from app.core.errors.base import AppError from app.core.pdf_utils import _count_pdf_widgets, _extract_pdf_fields from app.db.repositories import ( @@ -205,13 +206,31 @@ def delete_template(self, session: Session, template: Template) -> None: if template.pdf_path: try: resolved_pdf = paths._resolve_project_file(template.pdf_path) - if resolved_pdf.exists() and resolved_pdf.is_file(): + if ( + resolved_pdf.exists() + and resolved_pdf.is_file() + and not self._pdf_still_needed(session, template, resolved_pdf) + ): resolved_pdf.unlink() except Exception: pass delete_template(session, template) + def _pdf_still_needed(self, session: Session, template: Template, resolved_pdf: Path) -> bool: + # The seed PDF ships with the repo, and other templates may point at the same file. + if resolved_pdf == paths._resolve_project_file(SEED_TEMPLATE_PDF_PATH): + return True + for other in repo_list_templates(session): + if other.id == template.id or not other.pdf_path: + continue + try: + if paths._resolve_project_file(other.pdf_path) == resolved_pdf: + return True + except Exception: + continue + return False + def update_template( self, session: Session, template: Template, changes: TemplateUpdate ) -> TemplateResponse: diff --git a/tests/test_deletion.py b/tests/test_deletion.py index 80ca76b..dd762e0 100644 --- a/tests/test_deletion.py +++ b/tests/test_deletion.py @@ -80,6 +80,37 @@ def test_delete_template_deletes_pdf_file(self, client, tmp_path, monkeypatch): client.delete(f"{API_PREFIX}/templates/{tpl_id}") assert not pdf_file.exists() + def test_delete_template_keeps_pdf_shared_with_another_template(self, client, db, tmp_path, monkeypatch): + """A PDF still referenced by another template is not removed.""" + monkeypatch.setattr("app.core.paths.PROJECT_ROOT", tmp_path) + pdf_file = tmp_path / "shared.pdf" + pdf_file.write_bytes(b"%PDF-1.4 fake") + + first_id = _seed_template(client, name="First", pdf_path="shared.pdf") + second_id = _seed_template(client, name="Second", pdf_path="shared.pdf") + + client.delete(f"{API_PREFIX}/templates/{first_id}") + assert pdf_file.exists() + assert db.get(Template, second_id) is not None + + # Once the last template using it is gone, the PDF is removed too + client.delete(f"{API_PREFIX}/templates/{second_id}") + assert not pdf_file.exists() + + def test_delete_template_keeps_seed_pdf(self, client, tmp_path, monkeypatch): + """The bundled seed PDF is never removed, even with no other template using it.""" + from app.core.config import SEED_TEMPLATE_PDF_PATH + + monkeypatch.setattr("app.core.paths.PROJECT_ROOT", tmp_path) + seed_pdf = tmp_path / SEED_TEMPLATE_PDF_PATH + seed_pdf.parent.mkdir(parents=True) + seed_pdf.write_bytes(b"%PDF-1.4 seed") + + tpl_id = _seed_template(client, pdf_path=SEED_TEMPLATE_PDF_PATH) + + client.delete(f"{API_PREFIX}/templates/{tpl_id}") + assert seed_pdf.exists() + def test_delete_template_deletes_submission_output_pdfs(self, client, db, tmp_path, monkeypatch): """Output PDFs of related submissions should be wiped on template deletion.""" monkeypatch.setattr("app.core.paths.PROJECT_ROOT", tmp_path)