diff --git a/docs/configuration.rst b/docs/configuration.rst index 9cc30d7a..bbe99e38 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -94,4 +94,12 @@ You may add your own site specific navigation header to be included inside the < helpdesk/custom_navigation_header.html -2. Update the contents to display your custom navigation. \ No newline at end of file +2. Update the contents to display your custom navigation. + +Suppressible Log Messages +------------------------- +Some logging messages support being switched on or off according to deployment preferences. + +The following settings variables control emitting log messages for specific scenarios: + LOG_WARN_WHEN_CC_EMAIL_NOT_LINKED_TO_A_USER - there is no user matching the email address in the CC list. Defaults to False + LOG_WARN_WHEN_CC_EMAIL_LINKED_TO_MORE_THAN_1_USER - there is more than 1 user matching the email address in the CC list. Defaults to True diff --git a/helpdesk/email.py b/helpdesk/email.py index a85e35e0..2af33f64 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -46,6 +46,7 @@ from typing import List # import User model, which may be a custom model User = get_user_model() +MULTIPLE_USERS_SAME_EMAIL_MSG = "CC email address is linked to more than 1 active user" STRIPPED_SUBJECT_STRINGS = [ "Re: ", "Fw: ", @@ -531,7 +532,7 @@ def is_autoreply(message): return any(any_if_this) -def create_ticket_cc(ticket, cc_list): +def create_ticket_cc(ticket, cc_list, logger): if not cc_list: return [] # Local import to deal with non-defined / circular reference problem @@ -544,16 +545,32 @@ def create_ticket_cc(ticket, cc_list): if cced_email == ticket.queue.email_address: continue - user = None + user_id = None - try: - user = User.objects.get(email=cced_email) # @UndefinedVariable - except User.DoesNotExist: - pass + user_list = User.objects.filter(email=cced_email, is_active=True).values_list( + "id", flat=True + ) + count = user_list.count() + if count == 0: + if getattr( + helpdesk_settings, "LOG_WARN_WHEN_CC_EMAIL_NOT_LINKED_TO_A_USER", False + ): + logger.warning( + f"CC email address is not linked to an active user: {cced_email}" + ) + elif count > 1: + if getattr( + helpdesk_settings, + "LOG_WARN_WHEN_CC_EMAIL_LINKED_TO_MORE_THAN_1_USER", + True, + ): + logger.warning(f"{MULTIPLE_USERS_SAME_EMAIL_MSG}: {cced_email}") + else: + user_id = user_list[0] try: ticket_cc = subscribe_to_ticket_updates( - ticket=ticket, user=user, email=cced_email + ticket=ticket, user_id=user_id, email=cced_email ) new_ticket_ccs.append(ticket_cc) except ValidationError: @@ -673,7 +690,7 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger) context = safe_template_context(ticket) new_ticket_ccs = [] - new_ticket_ccs.append(create_ticket_cc(ticket, to_list + cc_list)) + new_ticket_ccs.append(create_ticket_cc(ticket, to_list + cc_list, logger)) autoreply = is_autoreply(message) if autoreply: diff --git a/helpdesk/settings.py b/helpdesk/settings.py index 426cf1dd..179267f3 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -487,3 +487,11 @@ HELPDESK_GET_NEW_TICKET_WEBHOOK_URLS = getattr( ) HELPDESK_WEBHOOK_TIMEOUT = getattr(settings, "HELPDESK_WEBHOOK_TIMEOUT", 3) + + +LOG_WARN_WHEN_CC_EMAIL_NOT_LINKED_TO_A_USER = getattr( + settings, "HELPDESK_LOG_WARN_WHEN_CC_EMAIL_NOT_LINKED_TO_A_USER", False +) +LOG_WARN_WHEN_CC_EMAIL_LINKED_TO_MORE_THAN_1_USER = getattr( + settings, "HELPDESK_LOG_WARN_WHEN_CC_EMAIL_LINKED_TO_MORE_THAN_1_USER", True +) diff --git a/helpdesk/tests/test_ticket_submission.py b/helpdesk/tests/test_ticket_submission.py index bbe37e67..0b29e8ac 100644 --- a/helpdesk/tests/test_ticket_submission.py +++ b/helpdesk/tests/test_ticket_submission.py @@ -4,7 +4,7 @@ from django.test import TestCase from django.test.client import Client from django.urls import reverse import email -from helpdesk.email import extract_email_metadata +from helpdesk.email import extract_email_metadata, MULTIPLE_USERS_SAME_EMAIL_MSG from helpdesk.models import ( CustomField, FollowUp, @@ -14,10 +14,12 @@ from helpdesk.models import ( Ticket, TicketCC, ) +from . import utils import logging from urllib.parse import urlparse import uuid +User = get_user_model() logger = logging.getLogger("helpdesk") @@ -46,7 +48,7 @@ class TicketBasicsTestCase(TestCase): "description": "Some Test Ticket", } - self.user = get_user_model().objects.create( + self.user = User.objects.create( username="User_1", ) @@ -480,6 +482,40 @@ class EmailInteractionsTestCase(TestCase): self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) + def test_create_ticket_from_email_with_cc_duplicate_users_same_email(self): + """ + Verify that the duplicated email warning log is invoked + """ + dup_email = "dup@helpdesk.test" + User.objects.create( + username="User_dup1", + email=dup_email, + ) + User.objects.create( + username="User_dup2", + email=dup_email, + ) + message, _, _ = utils.generate_text_email(locale="fr_FR") + + cc_list = [dup_email, "alpha@acme.test", "beta@acme.test", "gamma@delta.test"] + message_id = uuid.uuid4().hex + + message["Message-ID"] = message_id + message["Cc"] = ",".join(cc_list) + + with self.assertLogs("helpdesk", level="WARNING") as dup_warn: + extract_email_metadata(str(message), self.queue_public, logger=logger) + self.assertTrue( + MULTIPLE_USERS_SAME_EMAIL_MSG in dup_warn.output[0], + "The duplicated email across user ID's was was not logged.", + ) + # Check that the CC duplicate was still sent a notification email + email_count = len(mail.outbox) + found = any(dup_email in email.to for email in mail.outbox[: email_count - 1]) + self.assertTrue( + found, "The duplicated email across user ID's was not sent a notification." + ) + def test_create_followup_from_email_with_valid_message_id_with_no_initial_cc_list( self, ): diff --git a/helpdesk/update_ticket.py b/helpdesk/update_ticket.py index ed4fabdb..4941386a 100644 --- a/helpdesk/update_ticket.py +++ b/helpdesk/update_ticket.py @@ -29,7 +29,7 @@ def add_staff_subscription(user: User, ticket: Ticket) -> None: and user.is_authenticated and return_ticketccstring_and_show_subscribe(user, ticket)[1] ): - subscribe_to_ticket_updates(ticket, user) + subscribe_to_ticket_updates(ticket, user.id) def return_ticketccstring_and_show_subscribe(user, ticket): @@ -73,16 +73,16 @@ def return_ticketccstring_and_show_subscribe(user, ticket): def subscribe_to_ticket_updates( - ticket, user=None, email=None, can_view=True, can_update=False + ticket, user_id=None, email=None, can_view=True, can_update=False ): if ticket is not None: - queryset = TicketCC.objects.filter(ticket=ticket, user=user, email=email) + queryset = TicketCC.objects.filter(ticket=ticket, user_id=user_id, email=email) # Don't create duplicate entries for subscribers if queryset.count() > 0: return queryset.first() - if user is None and len(email) < 5: + if user_id is None and len(email) < 5: raise ValidationError( _( "When you add somebody on Cc, you must provide either a User or a valid email. Email: %s" @@ -91,7 +91,7 @@ def subscribe_to_ticket_updates( ) return ticket.ticketcc_set.create( - user=user, email=email, can_view=can_view, can_update=can_update + user_id=user_id, email=email, can_view=can_view, can_update=can_update ) diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index cc2b1ed4..63f9ea9d 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -412,7 +412,7 @@ def view_ticket(request, ticket_id): ] if show_subscribe: - subscribe_to_ticket_updates(ticket, request.user) + subscribe_to_ticket_updates(ticket, request.user.id) return HttpResponseRedirect(reverse("helpdesk:view", args=[ticket.id])) if "close" in request.GET and ticket.status == Ticket.RESOLVED_STATUS: diff --git a/setup.py b/setup.py index d8ebe85f..3cbc10b9 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ from setuptools import find_packages, setup import sys -version = "1.4.0" +version = "1.5.0" # Provided as an attribute, so you can append to these instead