Merge pull request #1457 from beemines/feat/copy-followup

feat: copy follow-ups to related tickets

docs/settings.rst

@@ -567,6 +567,20 @@ Options that change ticket properties
    class is empty.
 
 
+.. setting:: HELPDESK_ENABLE_FOLLOWUP_COPY
+
+   *Default:* ``False``
+
+   Set ``HELPDESK_ENABLE_FOLLOWUP_COPY = True`` in your Django ``settings.py``
+   to enable the **Copy instead of moving** option on the existing follow-up
+   edit page. This is a deployment-wide setting, not a user preference.
+   Existing staff and ticket access checks still apply.
+
+   When disabled, the option is hidden and copy submissions are rejected
+   without changing the original follow-up. Normal editing and moving remain
+   available.
+
+
 Time Tracking Options
 ---------------------
 

src/helpdesk/copy_followup.py

@@ -0,0 +1,50 @@
+"""Copy follow-up content without duplicating ticket history or work logs."""
+
+from pathlib import PurePath
+
+from django.db import transaction
+from django.utils.translation import gettext as _
+
+from helpdesk.models import FollowUp, FollowUpAttachment
+
+
+def copy_followup(source, ticket, user, *, title, comment, public):
+    """The caller must authorize access to both source and destination tickets."""
+    saved_files = []
+    try:
+        with transaction.atomic():
+            title = _("Copied from ticket #%(ticket)s: %(title)s") % {
+                "ticket": source.ticket_id,
+                "title": title,
+            }
+            copied = FollowUp.objects.create(
+                ticket=ticket,
+                title=title[:200],
+                comment=comment,
+                user=user,
+                public=public,
+                email_recipients=[],
+            )
+            # Saving a follow-up may calculate work time automatically. Copying
+            # existing content is not additional work to bill to this ticket.
+            FollowUp.objects.filter(pk=copied.pk).update(time_spent=None)
+            copied.time_spent = None
+            for attachment in source.followupattachment_set.all():
+                duplicate = FollowUpAttachment(
+                    followup=copied,
+                    filename=attachment.filename,
+                    mime_type=attachment.mime_type,
+                    size=attachment.size,
+                )
+                with attachment.file.open("rb") as content:
+                    duplicate.file.save(
+                        PurePath(attachment.file.name).name, content, save=False
+                    )
+                    saved_files.append((duplicate.file.storage, duplicate.file.name))
+                duplicate.save()
+            return copied
+    except Exception:
+        # Storage writes are not rolled back by the database transaction.
+        for storage, name in saved_files:
+            storage.delete(name)
+        raise

src/helpdesk/forms.py

@@ -268,6 +268,14 @@ class EditTicketCustomFieldForm(EditTicketForm):
 
 
 class EditFollowUpForm(forms.ModelForm):
+    copy_to_ticket = forms.BooleanField(
+        required=False,
+        label=_("Copy instead of moving"),
+        help_text=_(
+            "Keep the original follow-up and copy the title, comment and attachments to the selected ticket. No email is sent; status changes and time spent are not copied. Check Public before sharing with the destination submitter."
+        ),
+    )
+
     class Meta:
         model = FollowUp
         exclude = (
@@ -282,6 +290,15 @@ class EditFollowUpForm(forms.ModelForm):
             status__in=Ticket.OPEN_STATUSES
         )
 
+    def clean(self):
+        cleaned_data = super().clean()
+        if (
+            cleaned_data.get("copy_to_ticket")
+            and not helpdesk_settings.HELPDESK_ENABLE_FOLLOWUP_COPY
+        ):
+            raise forms.ValidationError(_("Copying follow-ups is disabled."))
+        return cleaned_data
+
 
 class AbstractTicketForm(CustomFieldMixin, forms.Form):
     """

src/helpdesk/settings.py

@@ -582,3 +582,8 @@ LOG_WARN_WHEN_CC_EMAIL_LINKED_TO_MORE_THAN_1_USER = getattr(
 )
 HELPDESK_API_ENABLED = getattr(settings, "HELPDESK_API_ENABLED", True)
 HELPDESK_UI_ENABLED = getattr(settings, "HELPDESK_UI_ENABLED", True)
+
+# Enable copying through the existing follow-up editor per deployment.
+HELPDESK_ENABLE_FOLLOWUP_COPY = getattr(
+    settings, "HELPDESK_ENABLE_FOLLOWUP_COPY", False
+)

src/helpdesk/templates/helpdesk/followup_edit.html

@@ -37,7 +37,13 @@
             <fieldset>
             <dl>
                 <dt><label for='id_ticket'>{% translate "Reassign ticket:" %}</label></dt>
-                <dd>{{ form.ticket }}</dd>
+                <dd>{{ form.ticket }}{{ form.ticket.errors }}</dd>
+                {% if helpdesk_settings.HELPDESK_ENABLE_FOLLOWUP_COPY %}
+                <dd>
+                    <label for="{{ form.copy_to_ticket.id_for_label }}">{{ form.copy_to_ticket }} {{ form.copy_to_ticket.label }}</label>
+                    <p class="text-body-secondary small">{{ form.copy_to_ticket.help_text }}</p>
+                </dd>
+                {% endif %}
                 <dt><label for="id_title">{% translate "Title:" %}</label></dt>
                 <dd>{{ form.title }}</dd>
                 <dt><label for="id_comment">{% translate "Comment:" %}</label></dt>

src/helpdesk/views/staff.py

@@ -45,6 +45,7 @@ from rest_framework import status
 from rest_framework.decorators import api_view
 
 from helpdesk import settings as helpdesk_settings
+from helpdesk.copy_followup import copy_followup
 from helpdesk.decorators import (
     helpdesk_staff_member_required,
     is_helpdesk_staff,
@@ -443,7 +444,27 @@ def followup_edit(request, ticket_id, followup_id):
             .distinct()
         )
 
-        if form.is_valid():
+        if form.is_valid() and form.cleaned_data["copy_to_ticket"]:
+            destination = form.cleaned_data["ticket"]
+            if destination.pk == ticket.pk:
+                form.add_error("ticket", _("Choose another ticket for the copy."))
+            else:
+                try:
+                    copied = copy_followup(
+                        followup,
+                        destination,
+                        request.user,
+                        title=form.cleaned_data["title"],
+                        comment=form.cleaned_data["comment"],
+                        public=form.cleaned_data["public"],
+                    )
+                except OSError:
+                    form.add_error(
+                        None, _("Unable to copy the attachments. No copy was created.")
+                    )
+                else:
+                    return HttpResponseRedirect(copied.get_absolute_url())
+        elif form.is_valid():
             # Edit in place: a copy would lose the message ID and the
             # TicketChange rows, which cascade away with the old row.
             followup.title = form.cleaned_data["title"]

tests/test_followup_copy.py

@@ -0,0 +1,224 @@
+import tempfile
+from datetime import timedelta
+from pathlib import Path
+from unittest import mock
+
+from django.contrib.auth.models import Permission
+from django.core import mail
+from django.core.files.uploadedfile import SimpleUploadedFile
+from django.test import TestCase, override_settings
+from django.urls import reverse
+
+from helpdesk import settings as helpdesk_settings
+from helpdesk.models import FollowUp, FollowUpAttachment, Queue, Ticket, TicketChange
+
+from .helpers import get_user
+
+
+class FollowUpCopyTests(TestCase):
+    def setUp(self):
+        enabled = mock.patch.object(
+            helpdesk_settings, "HELPDESK_ENABLE_FOLLOWUP_COPY", True
+        )
+        enabled.start()
+        self.addCleanup(enabled.stop)
+        directory = tempfile.TemporaryDirectory()
+        self.addCleanup(directory.cleanup)
+        self.media_root = Path(directory.name)
+        settings = override_settings(MEDIA_ROOT=directory.name)
+        settings.enable()
+        self.addCleanup(settings.disable)
+        self.user = get_user(username="copier", is_staff=True)
+        self.client.force_login(self.user)
+        self.queue = Queue.objects.create(title="Support", slug="support")
+        self.source = Ticket.objects.create(title="Original request", queue=self.queue)
+        self.target = Ticket.objects.create(title="Related request", queue=self.queue)
+        self.followup = FollowUp.objects.create(
+            ticket=self.source,
+            title="Investigation",
+            comment="Restart the device.",
+            user=None,
+            public=True,
+            message_id="<original@example.com>",
+            new_status=Ticket.CLOSED_STATUS,
+            time_spent=timedelta(minutes=45),
+            email_recipients=["customer@example.com"],
+        )
+        self.url = reverse(
+            "helpdesk:followup_edit", args=[self.source.pk, self.followup.pk]
+        )
+
+    def post(self, **data):
+        return self.client.post(
+            self.url,
+            {
+                "ticket": self.target.pk,
+                "title": self.followup.title,
+                "comment": self.followup.comment,
+                "copy_to_ticket": "on",
+                **data,
+            },
+        )
+
+    def test_get_keeps_existing_ticket_choices_and_does_not_copy(self):
+        closed = Ticket.objects.create(
+            title="Closed", queue=self.queue, status=Ticket.CLOSED_STATUS
+        )
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertQuerySetEqual(
+            response.context["form"].fields["ticket"].queryset,
+            [self.target, self.source],
+        )
+        self.assertEqual(FollowUp.objects.count(), 1)
+        self.assertNotIn(closed, response.context["form"].fields["ticket"].queryset)
+
+    def test_copy_keeps_source_and_does_not_replay_history(self):
+        TicketChange.objects.create(
+            followup=self.followup, field="Status", old_value="Open", new_value="Closed"
+        )
+        source_date = self.followup.date
+        response = self.post()
+        copied = self.target.followup_set.get()
+        self.assertRedirects(response, copied.get_absolute_url())
+        self.followup.refresh_from_db()
+        self.assertEqual(self.followup.ticket, self.source)
+        self.assertEqual(self.followup.date, source_date)
+        self.assertEqual(copied.comment, self.followup.comment)
+        self.assertIn(f"#{self.source.pk}", copied.title)
+        self.assertEqual(copied.user, self.user)
+        self.assertFalse(copied.public)
+        self.assertIsNone(copied.message_id)
+        self.assertIsNone(copied.new_status)
+        self.assertIsNone(copied.time_spent)
+        self.assertEqual(copied.email_recipients, [])
+        self.assertFalse(copied.ticketchange_set.exists())
+        self.assertEqual(len(mail.outbox), 0)
+        self.target.refresh_from_db()
+        self.assertEqual(self.target.status, Ticket.OPEN_STATUS)
+
+    def test_copy_respects_public_field(self):
+        self.post(public="on")
+        self.assertTrue(self.target.followup_set.get().public)
+
+    def attachment(self, name="notes.txt"):
+        return FollowUpAttachment.objects.create(
+            followup=self.followup,
+            file=SimpleUploadedFile(name, b"investigation notes"),
+            filename=name,
+            mime_type="text/plain",
+            size=19,
+        )
+
+    def test_attachments_are_independent_files(self):
+        original = self.attachment()
+        self.post()
+        copied = self.target.followup_set.get().followupattachment_set.get()
+        self.assertNotEqual(original.file.name, copied.file.name)
+        with copied.file.open("rb") as content:
+            self.assertEqual(content.read(), b"investigation notes")
+        self.assertEqual(copied.filename, original.filename)
+        original.file.delete(save=False)
+        self.assertTrue(copied.file.storage.exists(copied.file.name))
+
+    def test_partial_attachment_failure_rolls_back_rows_and_files(self):
+        self.attachment("a.txt")
+        missing = self.attachment("b.txt")
+        missing.file.delete(save=False)
+        before = {p for p in self.media_root.rglob("*") if p.is_file()}
+        response = self.post()
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, "Unable to copy the attachments")
+        self.assertFalse(self.target.followup_set.exists())
+        self.assertEqual(FollowUpAttachment.objects.count(), 2)
+        self.assertEqual(before, {p for p in self.media_root.rglob("*") if p.is_file()})
+
+    def test_rejects_self_closed_and_missing_target(self):
+        closed = Ticket.objects.create(
+            title="Closed", queue=self.queue, status=Ticket.CLOSED_STATUS
+        )
+        for target in (self.source.pk, closed.pk, 99999, ""):
+            with self.subTest(target=target):
+                response = self.post(ticket=target)
+                self.assertEqual(response.status_code, 200)
+                self.assertTrue(response.context["form"].errors)
+        self.assertEqual(FollowUp.objects.count(), 1)
+
+    def test_followup_must_belong_to_source_ticket(self):
+        url = reverse("helpdesk:followup_edit", args=[self.target.pk, self.followup.pk])
+        self.assertEqual(
+            self.client.post(url, {"ticket": self.source.pk}).status_code, 404
+        )
+
+    def test_queue_permissions_apply_to_source_and_target(self):
+        other_queue = Queue.objects.create(title="Restricted", slug="restricted")
+        forbidden = Ticket.objects.create(title="Secret", queue=other_queue)
+        self.user.user_permissions.add(
+            Permission.objects.get(codename=self.queue.permission_name.split(".", 1)[1])
+        )
+        with mock.patch.object(
+            helpdesk_settings, "HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION", True
+        ):
+            response = self.client.get(self.url)
+            self.assertNotIn(
+                forbidden, response.context["form"].fields["ticket"].queryset
+            )
+            response = self.post(ticket=forbidden.pk)
+            self.assertTrue(response.context["form"].errors)
+            self.source.queue = other_queue
+            self.source.save()
+            self.assertEqual(self.post().status_code, 403)
+        self.assertEqual(FollowUp.objects.count(), 1)
+
+    def test_non_staff_and_anonymous_cannot_copy(self):
+        self.client.force_login(get_user(username="customer", is_staff=False))
+        self.assertIn(self.post().status_code, (302, 403))
+        self.client.logout()
+        self.assertIn(self.post().status_code, (302, 403))
+        self.assertEqual(FollowUp.objects.count(), 1)
+
+    def test_does_not_duplicate_automatically_calculated_time(self):
+        with mock.patch.object(helpdesk_settings, "FOLLOWUP_TIME_SPENT_AUTO", True):
+            self.post()
+        self.assertIsNone(self.target.followup_set.get().time_spent)
+
+    def test_unchecked_copy_preserves_existing_move_behavior(self):
+        response = self.post(copy_to_ticket="")
+        self.assertEqual(response.status_code, 302)
+        self.followup.refresh_from_db()
+        self.assertEqual(self.followup.ticket, self.target)
+        self.assertEqual(FollowUp.objects.count(), 1)
+        self.assertEqual(self.followup.message_id, "<original@example.com>")
+
+    def test_copy_uses_edited_content_without_modifying_source(self):
+        self.post(title="Updated title", comment="Updated comment")
+        copied = self.target.followup_set.get()
+        self.assertIn("Updated title", copied.title)
+        self.assertEqual(copied.comment, "Updated comment")
+        self.followup.refresh_from_db()
+        self.assertEqual(self.followup.title, "Investigation")
+        self.assertEqual(self.followup.comment, "Restart the device.")
+
+    def test_disabled_hides_copy_option(self):
+        with mock.patch.object(
+            helpdesk_settings, "HELPDESK_ENABLE_FOLLOWUP_COPY", False
+        ):
+            response = self.client.get(self.url)
+        self.assertNotContains(response, 'name="copy_to_ticket"')
+
+    def test_disabled_rejects_copy_without_moving_original(self):
+        with mock.patch.object(
+            helpdesk_settings, "HELPDESK_ENABLE_FOLLOWUP_COPY", False
+        ):
+            response = self.post()
+        self.assertEqual(response.status_code, 200)
+        self.assertTrue(response.context["form"].errors)
+        self.followup.refresh_from_db()
+        self.assertEqual(self.followup.ticket, self.source)
+        self.assertEqual(FollowUp.objects.count(), 1)
+
+    def test_disabled_preserves_normal_move(self):
+        with mock.patch.object(
+            helpdesk_settings, "HELPDESK_ENABLE_FOLLOWUP_COPY", False
+        ):
+            self.test_unchecked_copy_preserves_existing_move_behavior()