{% trans "Add Another File" %}
diff --git a/helpdesk/tests/test_attachments.py b/helpdesk/tests/test_attachments.py
index e53ebb5e..fcb9286b 100644
--- a/helpdesk/tests/test_attachments.py
+++ b/helpdesk/tests/test_attachments.py
@@ -10,6 +10,7 @@ import shutil
from tempfile import gettempdir
from unittest import mock
from unittest.case import skip
+from django.contrib.auth import get_user_model
MEDIA_DIR = os.path.join(gettempdir(), "helpdesk_test_media")
@@ -92,6 +93,89 @@ class AttachmentIntegrationTests(TestCase):
self.assertEqual(disk_content, "โจ")
+@override_settings(MEDIA_ROOT=MEDIA_DIR)
+class AttachmentIntegrationStaffTests(TestCase):
+ def setUp(self):
+ self.ticket = models.Ticket.objects.create(
+ queue=models.Queue.objects.create(),
+ title="Test attachments via ticket update",
+ )
+ self.default_update_post_data = {
+ "queue": self.ticket.queue_id,
+ "title": self.ticket.title,
+ "priority": self.ticket.priority,
+ }
+
+ def loginUser(self, is_staff=True):
+ """Create a staff user and login"""
+ User = get_user_model()
+ self.user = User.objects.create(
+ username="User_1",
+ is_staff=is_staff,
+ )
+ self.user.set_password("pass")
+ self.user.save()
+ self.client.login(username="User_1", password="pass")
+
+ def test_update_ticket_with_attachment_valid_extension(self):
+ self.loginUser(is_staff=True)
+ file_content = "staff attached file content"
+ test_file = SimpleUploadedFile(
+ "test_staff_att.txt", bytes(file_content, "utf-8"), "text/plain"
+ )
+ post_data = {
+ "attachment": test_file,
+ **self.default_update_post_data,
+ }
+ # Ensure ticket form submits with attachment successfully
+ self.client.post(
+ reverse(
+ "helpdesk:update",
+ kwargs={"ticket_id": self.ticket.id},
+ ),
+ post_data,
+ follow=True,
+ )
+ # Ensure attachment is available with correct content
+ att = models.FollowUpAttachment.objects.get(followup__ticket=self.ticket)
+ with open(os.path.join(MEDIA_DIR, att.file.name)) as file_on_disk:
+ disk_content = file_on_disk.read()
+ self.assertEqual(disk_content, file_content)
+
+ def test_update_ticket_with_attachment_invalid_extension(self):
+ self.loginUser(is_staff=True)
+ file_content = "staff attached file content with invalid extension"
+ file_extension = ".crash"
+ test_file = SimpleUploadedFile(
+ f"test_staff_att{file_extension}",
+ bytes(file_content, "utf-8"),
+ "text/plain",
+ )
+ post_data = {
+ "attachment": test_file,
+ **self.default_update_post_data,
+ }
+ # Ensure ticket form submits with attachment successfully
+ response = self.client.post(
+ reverse(
+ "helpdesk:update",
+ kwargs={"ticket_id": self.ticket.id},
+ ),
+ post_data,
+ follow=True,
+ )
+ error_msg = response.context_data["form"].errors["attachment"][0]
+ self.assertTrue(
+ file_extension in error_msg,
+ "Response indicates there were no errors attaching illegal file extension",
+ )
+ # Ensure attachment is not uploaded
+ has_att = models.FollowUpAttachment.objects.filter(
+ followup__ticket=self.ticket
+ ).exists()
+ self.assertFalse(has_att, "File was attached with invalid extension")
+
+
@mock.patch.object(models.FollowUp, "save", autospec=True)
@mock.patch.object(models.FollowUpAttachment, "save", autospec=True)
@mock.patch.object(models.Ticket, "save", autospec=True)
diff --git a/helpdesk/tests/test_checklist.py b/helpdesk/tests/test_checklist.py
index 3454f47a..657e045d 100644
--- a/helpdesk/tests/test_checklist.py
+++ b/helpdesk/tests/test_checklist.py
@@ -17,8 +17,13 @@ class TicketChecklistTestCase(TestCase):
self.client.login(username="User", password="pass")
self.ticket = Ticket.objects.create(
- queue=Queue.objects.create(title="Queue", slug="queue")
+ queue=Queue.objects.create(title="Queue", slug="queue"), title="Test Queue"
)
+ self.default_update_post_data = {
+ "queue": self.ticket.queue_id,
+ "title": self.ticket.title,
+ "priority": self.ticket.priority,
+ }
def test_create_checklist(self):
self.assertEqual(self.ticket.checklists.count(), 0)
@@ -141,7 +146,10 @@ class TicketChecklistTestCase(TestCase):
response = self.client.post(
reverse("helpdesk:update", kwargs={"ticket_id": self.ticket.id}),
- data={f"checklist-{checklist.id}": task.id},
+ data={
+ f"checklist-{checklist.id}": task.id,
+ **self.default_update_post_data,
+ },
follow=True,
)
self.assertEqual(response.status_code, 200)
@@ -167,6 +175,7 @@ class TicketChecklistTestCase(TestCase):
response = self.client.post(
reverse("helpdesk:update", kwargs={"ticket_id": self.ticket.id}),
+ data={**self.default_update_post_data},
follow=True,
)
self.assertEqual(response.status_code, 200)
diff --git a/helpdesk/tests/test_navigation.py b/helpdesk/tests/test_navigation.py
index 9c695d20..ab6aae85 100644
--- a/helpdesk/tests/test_navigation.py
+++ b/helpdesk/tests/test_navigation.py
@@ -279,7 +279,7 @@ class ReturnToTicketTestCase(TestCase):
user = get_staff_user()
ticket = create_ticket()
- response = return_to_ticket(user, helpdesk_settings, ticket)
+ response = return_to_ticket(user, ticket)
self.assertEqual(response["location"], ticket.get_absolute_url())
def test_non_staff_user(self):
@@ -291,5 +291,5 @@ class ReturnToTicketTestCase(TestCase):
email="wensleydale@example.com",
)
ticket = create_ticket()
- response = return_to_ticket(user, helpdesk_settings, ticket)
+ response = return_to_ticket(user, ticket)
self.assertEqual(response["location"], ticket.ticket_url)
diff --git a/helpdesk/tests/test_ticket_actions.py b/helpdesk/tests/test_ticket_actions.py
index 92da0b1f..537b02f0 100644
--- a/helpdesk/tests/test_ticket_actions.py
+++ b/helpdesk/tests/test_ticket_actions.py
@@ -119,9 +119,15 @@ class TicketActionsTestCase(TestCase):
ticket = Ticket.objects.create(**initial_data)
ticket_id = ticket.id
+ default_post_data = {
+ "title": ticket.title,
+ "priority": ticket.priority,
+ "queue": ticket.queue_id,
+ }
# assign new owner
post_data = {
"owner": self.user2.id,
+ **default_post_data,
}
response = self.client.post(
reverse("helpdesk:update", kwargs={"ticket_id": ticket_id}),
@@ -139,7 +145,11 @@ class TicketActionsTestCase(TestCase):
self.user2.save()
self.user.email = "user1@test.com"
self.user.save()
- post_data = {"new_status": Ticket.CLOSED_STATUS, "public": True}
+ post_data = {
+ "new_status": Ticket.CLOSED_STATUS,
+ "public": True,
+ **default_post_data,
+ }
# do this also to a newly assigned user (different from logged in one)
ticket.assigned_to = self.user
@@ -153,6 +163,7 @@ class TicketActionsTestCase(TestCase):
"new_status": Ticket.OPEN_STATUS,
"owner": self.user2.id,
"public": True,
+ **default_post_data,
}
response = self.client.post(
reverse("helpdesk:update", kwargs={"ticket_id": ticket_id}),
@@ -363,6 +374,8 @@ class TicketActionsTestCase(TestCase):
slug="newqueue",
)
post_data = {
+ "title": ticket.title,
+ "priority": ticket.priority,
"comment": "first follow-up in new queue",
"queue": str(new_queue.id),
}
diff --git a/helpdesk/tests/test_time_spent_auto.py b/helpdesk/tests/test_time_spent_auto.py
index 0055b08a..4f4f9967 100644
--- a/helpdesk/tests/test_time_spent_auto.py
+++ b/helpdesk/tests/test_time_spent_auto.py
@@ -416,6 +416,7 @@ class TimeSpentAutoTestCase(TestCase):
"created": datetime.strptime(
"2024-04-09T08:00:00+00:00", "%Y-%m-%dT%H:%M:%S%z"
),
+ "description": "Followup time spent auto exclude queues",
}
ticket = Ticket.objects.create(**initial_data)
@@ -427,6 +428,8 @@ class TimeSpentAutoTestCase(TestCase):
post_data = {
"comment": "ticket in queue {}".format(queue),
"queue": queues[queue].id,
+ "title": ticket.title,
+ "priority": ticket.priority,
}
self.client.post(
reverse("helpdesk:update", kwargs={"ticket_id": ticket.id}), post_data
diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py
index 108b7d35..cc2b1ed4 100644
--- a/helpdesk/views/staff.py
+++ b/helpdesk/views/staff.py
@@ -11,7 +11,7 @@ from ..lib import format_time_spent
from ..templated_email import send_templated_mail
from collections import defaultdict
from copy import deepcopy
-from datetime import datetime, time, timedelta
+from datetime import datetime, timedelta
from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.decorators import user_passes_test
@@ -21,14 +21,13 @@ from django.core.exceptions import PermissionDenied
from django.core.handlers.wsgi import WSGIRequest
from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
from django.db.models import Q, Case, When
+from django.db.models.query import QuerySet
from django.forms import HiddenInput, inlineformset_factory, TextInput
from django.http import Http404, HttpResponse, HttpResponseRedirect, JsonResponse
from django.shortcuts import get_object_or_404, redirect, render
from django.urls import reverse, reverse_lazy
from django.utils import timezone
-from django.utils.dateparse import parse_date, parse_datetime
from django.utils.html import escape
-from django.utils.timezone import make_aware
from django.utils.translation import gettext as _
from django.views.decorators.csrf import requires_csrf_token
from django.views.generic.edit import FormView, UpdateView
@@ -119,7 +118,6 @@ def _get_queue_choices(queues):
idea is to return only one choice if there is only one queue or add empty
choice at the beginning of the list, if there are more queues
"""
-
queue_choices = []
if len(queues) > 1:
queue_choices = [("", "--------")]
@@ -127,6 +125,29 @@ def _get_queue_choices(queues):
return queue_choices
+def get_active_users() -> QuerySet:
+ if helpdesk_settings.HELPDESK_STAFF_ONLY_TICKET_OWNERS:
+ users = User.objects.filter(is_active=True, is_staff=True).order_by(
+ User.USERNAME_FIELD
+ )
+ else:
+ users = User.objects.filter(is_active=True).order_by(User.USERNAME_FIELD)
+ return users
+
+
+def get_user_queues(user) -> dict[str, str]:
+ queues = HelpdeskUser(user).get_queues()
+ return _get_queue_choices(queues)
+
+
+def get_form_extra_kwargs(user) -> dict[str, object]:
+ return {
+ "active_users": get_active_users(),
+ "queues": get_user_queues(user),
+ "priorities": Ticket.PRIORITY_CHOICES,
+ }
+
+
@helpdesk_staff_member_required
def dashboard(request):
"""
@@ -381,7 +402,7 @@ def view_ticket(request, ticket_id):
if "take" in request.GET:
update_ticket(request.user, ticket, owner=request.user.id)
- return return_to_ticket(request.user, helpdesk_settings, ticket)
+ return return_to_ticket(request.user, ticket)
if "subscribe" in request.GET:
# Allow the user to subscribe him/herself to the ticket whilst viewing
@@ -406,22 +427,13 @@ def view_ticket(request, ticket_id):
owner=owner,
comment=_("Accepted resolution and closed ticket"),
)
- return return_to_ticket(request.user, helpdesk_settings, ticket)
+ return return_to_ticket(request.user, ticket)
- if helpdesk_settings.HELPDESK_STAFF_ONLY_TICKET_OWNERS:
- users = User.objects.filter(is_active=True, is_staff=True).order_by(
- User.USERNAME_FIELD
- )
- else:
- users = User.objects.filter(is_active=True).order_by(User.USERNAME_FIELD)
-
- queues = HelpdeskUser(request.user).get_queues()
- queue_choices = _get_queue_choices(queues)
- # TODO: shouldn't this template get a form to begin with?
+ extra_context_kwargs = get_form_extra_kwargs(request.user)
form = TicketForm(
- initial={"due_date": ticket.due_date}, queue_choices=queue_choices
+ initial={"due_date": ticket.due_date},
+ queue_choices=extra_context_kwargs["queues"],
)
-
ticketcc_string, show_subscribe = return_ticketccstring_and_show_subscribe(
request.user, ticket
)
@@ -467,9 +479,6 @@ def view_ticket(request, ticket_id):
"dependencies": dependencies,
"submitter_userprofile_url": submitter_userprofile_url,
"form": form,
- "active_users": users,
- "priorities": Ticket.PRIORITY_CHOICES,
- "queues": queue_choices,
"preset_replies": PreSetReply.objects.filter(
Q(queues=ticket.queue) | Q(queues__isnull=True)
),
@@ -477,6 +486,7 @@ def view_ticket(request, ticket_id):
"SHOW_SUBSCRIBE": show_subscribe,
"checklist_form": checklist_form,
"customfields_form": customfields_form,
+ **extra_context_kwargs,
},
)
@@ -571,23 +581,17 @@ def get_ticket_from_request_with_authorisation(
return get_object_or_404(Ticket, id=ticket_id)
-def get_due_date_from_request_or_ticket(
- request: WSGIRequest, ticket: Ticket
+def get_due_date_from_form_or_ticket(
+ form, ticket: Ticket
) -> typing.Optional[datetime.date]:
- """Tries to locate the due date for a ticket from the `request.POST`
+ """Tries to locate the due date for a ticket from the form
'due_date' parameter or the `due_date_*` paramaters.
"""
- due_date = request.POST.get("due_date", None) or None
-
- if due_date is not None:
- parsed_date = parse_datetime(due_date) or datetime.combine(
- parse_date(due_date), time()
- )
- due_date = make_aware(parsed_date)
- else:
- due_date_year = int(request.POST.get("due_date_year", 0))
- due_date_month = int(request.POST.get("due_date_month", 0))
- due_date_day = int(request.POST.get("due_date_day", 0))
+ due_date = form.cleaned_data.get("due_date") or None
+ if due_date is None:
+ due_date_year = int(form.cleaned_data.get("due_date_year", 0))
+ due_date_month = int(form.cleaned_data.get("due_date_month", 0))
+ due_date_day = int(form.cleaned_data.get("due_date_day", 0))
# old way, probably deprecated?
if not (due_date_year and due_date_month and due_date_day):
due_date = ticket.due_date
@@ -602,36 +606,38 @@ def get_due_date_from_request_or_ticket(
return due_date
-def get_time_spent_from_request(request: WSGIRequest) -> typing.Optional[timedelta]:
- if request.POST.get("time_spent"):
- (hours, minutes) = [int(f) for f in request.POST.get("time_spent").split(":")]
+def get_time_spent_from_form(form: dict) -> typing.Optional[timedelta]:
+ if form.data.get("time_spent"):
+ (hours, minutes) = [int(f) for f in form.data.get("time_spent").split(":")]
return timedelta(hours=hours, minutes=minutes)
return None
-def update_ticket_view(request, ticket_id, public=False):
- try:
- ticket = get_ticket_from_request_with_authorisation(request, ticket_id, public)
- except PermissionDenied:
- return redirect_to_login(request.path, "helpdesk:login")
+def update_ticket_view(request, ticket_id, *args, **kwargs):
+ return UpdateTicketView.as_view()(request, *args, ticket_id=ticket_id, **kwargs)
- comment = request.POST.get("comment", "")
- new_status = int(request.POST.get("new_status", ticket.status))
- title = request.POST.get("title", ticket.title)
- owner = int(request.POST.get("owner", -1))
- priority = int(request.POST.get("priority", ticket.priority))
- queue = int(request.POST.get("queue", ticket.queue.id))
+
+def save_ticket_update(form, ticket, user):
+ comment = form.data.get("comment", "")
+ new_status = int(form.data.get("new_status", ticket.status))
+ title = form.cleaned_data.get("title", ticket.title)
+ owner = int(form.data.get("owner", -1))
+ priority = int(form.cleaned_data.get("priority", ticket.priority))
+ queue = int(form.cleaned_data.get("queue", ticket.queue.id))
# custom fields
- customfields_form = EditTicketCustomFieldForm(request.POST or None, instance=ticket)
+ customfields_form = EditTicketCustomFieldForm(
+ form.cleaned_data or None, instance=ticket
+ )
# Check if a change happened on checklists
new_checklists = {}
changes_in_checklists = False
for checklist in ticket.checklists.all():
old_completed = set(checklist.tasks.completed().values_list("id", flat=True))
+ # Checklists will not be in the cleaned_data so access the submitted data
new_checklist = set(
- map(int, request.POST.getlist(f"checklist-{checklist.id}", []))
+ map(int, form.data.getlist(f"checklist-{checklist.id}", []))
)
new_checklists[checklist.id] = new_checklist
if new_checklist != old_completed:
@@ -640,10 +646,10 @@ def update_ticket_view(request, ticket_id, public=False):
# NOTE: jQuery's default for dates is mm/dd/yy
# very US-centric but for now that's the only format supported
# until we clean up code to internationalize a little more
- due_date = get_due_date_from_request_or_ticket(request, ticket)
+ due_date = get_due_date_from_form_or_ticket(form, ticket)
no_changes = all(
[
- not request.FILES,
+ not form.files,
not comment,
not changes_in_checklists,
new_status == ticket.status,
@@ -658,29 +664,29 @@ def update_ticket_view(request, ticket_id, public=False):
]
)
if no_changes:
- return return_to_ticket(request.user, helpdesk_settings, ticket)
+ return ticket
update_ticket(
- request.user,
+ user,
ticket,
title=title,
comment=comment,
- files=request.FILES.getlist("attachment"),
- public=request.POST.get("public", False),
- owner=int(request.POST.get("owner", -1)),
- priority=int(request.POST.get("priority", -1)),
- queue=int(request.POST.get("queue", -1)),
+ files=form.files.getlist("attachment"),
+ public=form.data.get("public", False),
+ owner=owner or -1,
+ priority=priority or -1,
+ queue=queue or -1,
new_status=new_status,
- time_spent=get_time_spent_from_request(request),
- due_date=get_due_date_from_request_or_ticket(request, ticket),
+ time_spent=get_time_spent_from_form(form),
+ due_date=due_date,
new_checklists=new_checklists,
customfields_form=customfields_form,
)
- return return_to_ticket(request.user, helpdesk_settings, ticket)
+ return ticket
-def return_to_ticket(user, helpdesk_settings, ticket):
+def return_to_ticket(user, ticket):
"""Helper function for update_ticket"""
if is_helpdesk_staff(user):
@@ -1277,8 +1283,7 @@ class CreateTicketView(
def get_form_kwargs(self):
kwargs = super().get_form_kwargs()
- queues = HelpdeskUser(self.request.user).get_queues()
- kwargs["queue_choices"] = _get_queue_choices(queues)
+ kwargs["queue_choices"] = get_user_queues(self.request.user)
return kwargs
def form_valid(self, form):
@@ -1295,6 +1300,56 @@ class CreateTicketView(
return reverse("helpdesk:dashboard")
+class UpdateTicketView(
+ MustBeStaffMixin, abstract_views.AbstractCreateTicketMixin, UpdateView
+):
+ template_name = "helpdesk/ticket.html"
+ form_class = TicketForm
+
+ def get_initial(self):
+ initial_data = super().get_initial()
+ return initial_data
+
+ def get_context_data(self, **kwargs):
+ """Insert view context that would be lost after a POST."""
+ extra = get_form_extra_kwargs(self.request.user)
+ kwargs.update(extra)
+ # Copy all data submitted that is not in the forms defined fields
+ form_fields = kwargs["form"].base_fields
+ all_fields = kwargs["form"].data
+ self.extra_context = {
+ "xform": {
+ k: v
+ for k, v in all_fields.items()
+ if k != "csrfmiddlewaretoken" and k not in form_fields
+ }
+ }
+ return super().get_context_data(**kwargs)
+
+ def get_form_kwargs(self):
+ kwargs = super().get_form_kwargs()
+ # The ModelFormMixin adds "instance" which is then a problem in the
+ kwargs["queue_choices"] = get_user_queues(self.request.user)
+ kwargs["body_reqd"] = False
+ return kwargs
+
+ def get_object(self, queryset=None):
+ ticket_id = self.kwargs["ticket_id"]
+ return Ticket.objects.get(id=ticket_id)
+
+ def form_valid(self, form):
+ ticket_id = self.kwargs["ticket_id"]
+ try:
+ self.ticket = get_ticket_from_request_with_authorisation(
+ self.request, ticket_id, False
+ )
+ except PermissionDenied:
+ return redirect_to_login(self.request.path, "helpdesk:login")
+ # Avoid calling super as it will call the save() method on the form
+ save_ticket_update(form, self.ticket, self.request.user)
+ return return_to_ticket(self.request.user, self.ticket)
+
+
@helpdesk_staff_member_required
def raw_details(request, type_):
# TODO: This currently only supports spewing out 'PreSetReply' objects,