From 978e6d5c9bd94cc4317baf103815c793bf0c9eb0 Mon Sep 17 00:00:00 2001 From: Bruno Tikami Date: Thu, 17 Mar 2016 01:12:42 -0300 Subject: [PATCH] UPDATED: When notifying users about creation / updates on tickets, notify everybody using a single email message. --- helpdesk/forms.py | 10 -- helpdesk/management/commands/get_email.py | 36 ++-- helpdesk/tests/test_ticket_submission.py | 210 ++++++++++++++++------ helpdesk/views/staff.py | 25 +-- 4 files changed, 188 insertions(+), 93 deletions(-) diff --git a/helpdesk/forms.py b/helpdesk/forms.py index ee504dff..ecadc515 100644 --- a/helpdesk/forms.py +++ b/helpdesk/forms.py @@ -554,16 +554,6 @@ class TicketCCForm(forms.ModelForm): model = TicketCC exclude = ('ticket',) - def clean(self): - - cleaned_data = super(TicketCCForm, self).clean() - - user = cleaned_data.get('user', None) - email = cleaned_data.get('email', '') - - if user is None and len(email) == 0: - raise forms.ValidationError(_('When you add somebody on Cc, you must provided either an User or a valid email. Email: %s' %email)) - class TicketDependencyForm(forms.ModelForm): class Meta: model = TicketDependency diff --git a/helpdesk/management/commands/get_email.py b/helpdesk/management/commands/get_email.py index 9f21f27c..e43217af 100644 --- a/helpdesk/management/commands/get_email.py +++ b/helpdesk/management/commands/get_email.py @@ -281,6 +281,14 @@ def create_object_from_email_message(message, ticket_id, payload, files, quiet): new_ticket_ccs = create_ticket_cc(ticket, cc_list.split(',')) notification_template = None + notifications_to_be_sent = [sender_email,] + + if queue.enable_notifications_on_email_events and len(notifications_to_be_sent): + + ticket_cc_list = TicketCC.objects.filter(ticket=ticket).all().values_list('email', flat=True) + + for email in ticket_cc_list : + notifications_to_be_sent.append(email) if new: @@ -290,7 +298,7 @@ def create_object_from_email_message(message, ticket_id, payload, files, quiet): send_templated_mail( 'newticket_submitter', context, - recipients=sender_email, + recipients=notifications_to_be_sent, sender=queue.from_address, fail_silently=True, extra_headers={'In-Reply-To': message_id}, @@ -344,24 +352,18 @@ def create_object_from_email_message(message, ticket_id, payload, files, quiet): recipients=queue.updated_ticket_cc, sender=queue.from_address, fail_silently=True, - ) + ) - notifications_to_be_sent = [] - ticket_cc_list = TicketCC.objects.filter(ticket=ticket).all().values_list('email', flat=True) + if queue.enable_notifications_on_email_events: - for email in ticket_cc_list : - notifications_to_be_sent.append(email) - - if queue.enable_notifications_on_email_events and len(notifications_to_be_sent): - - send_templated_mail( - notification_template, - context, - recipients=notifications_to_be_sent, - sender=queue.from_address, - fail_silently=True, - extra_headers={'In-Reply-To': message_id}, - ) + if queue.updated_ticket_cc: + send_templated_mail( + 'updated_cc', + context, + recipients=notifications_to_be_sent, + sender=queue.from_address, + fail_silently=True, + ) return ticket diff --git a/helpdesk/tests/test_ticket_submission.py b/helpdesk/tests/test_ticket_submission.py index 3c871f2f..26844917 100644 --- a/helpdesk/tests/test_ticket_submission.py +++ b/helpdesk/tests/test_ticket_submission.py @@ -174,6 +174,9 @@ class EmailInteractionsTestCase(TestCase): # and the new and update queues (+2) self.assertEqual(email_count + 1 + 2, len(mail.outbox)) + # Ensure that the submitter is notified + self.assertIn(submitter_email,mail.outbox[0].to) + def test_create_ticket_from_email_without_message_id(self): """ @@ -203,6 +206,9 @@ class EmailInteractionsTestCase(TestCase): # and the new and update queues (+2) self.assertEqual(email_count + 1 + 2, len(mail.outbox)) + # Ensure that the submitter is notified + self.assertIn(submitter_email,mail.outbox[0].to) + def test_create_ticket_from_email_with_carbon_copy(self): """ @@ -232,13 +238,21 @@ class EmailInteractionsTestCase(TestCase): ticket = Ticket.objects.get(id=followup.ticket.id) self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - self.assertEqual(email_count + 1 + 2 + 1, len(mail.outbox)) + # As we have created an Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + self.assertEqual(email_count + 1 + 2, len(mail.outbox)) - # Ensure that is created + # Ensure that the submitter is notified + self.assertIn(submitter_email,mail.outbox[0].to) + + for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[0].to) + + # Ensure that exists ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) @@ -266,9 +280,31 @@ class EmailInteractionsTestCase(TestCase): email_count = len(mail.outbox) - self.assertRaises(ValidationError, object_from_message, str(msg), self.queue_public, quiet=True) + object_from_message(str(msg), self.queue_public, quiet=True) - def test_create_followup_from_email_with_valid_message_id_with_when_no_initial_cc_list(self): + followup = FollowUp.objects.get(message_id=message_id) + ticket = Ticket.objects.get(id=followup.ticket.id) + self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) + + # As we have created an Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + self.assertEqual(email_count + 1 + 2, len(mail.outbox)) + + # Ensure that the submitter is notified + self.assertIn(submitter_email, mail.outbox[0].to) + + for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[0].to) + + # Ensure that exists. Even if it's an invalid email. + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) + self.assertTrue(ticket_cc.ticket, ticket) + self.assertTrue(ticket_cc.email, cc_email) + + def test_create_followup_from_email_with_valid_message_id_with_no_initial_cc_list(self): """ Ensure that if a message is received with an valid In-Reply-To ID, @@ -295,6 +331,18 @@ class EmailInteractionsTestCase(TestCase): followup = FollowUp.objects.get(message_id=message_id) ticket = Ticket.objects.get(id=followup.ticket.id) + + # As we have created an Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + + # Ensure that the submitter is notified + self.assertIn(submitter_email,mail.outbox[0].to) + + # As we have created an Ticket from an email, we notify the sender (+1) + # and the new and update queues (+2) + expected_email_count = 1 + 2 + self.assertEqual(expected_email_count, len(mail.outbox)) ### end of the Ticket and TicketCCs creation ### # Reply message @@ -327,16 +375,23 @@ class EmailInteractionsTestCase(TestCase): self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) - # As we have created an Ticket from an email, we notify the sender (+1) - # and the new and update queues (+2) - expected_email_count = 1 + 2 - # As an update was made, we increase the expected_email_count with: # cc_list: +1 # public_update_queue: +1 - expected_email_count += 1 + 1 + expected_email_count += 1 + 1 self.assertEqual(expected_email_count, len(mail.outbox)) + # As we have created a FollowUp from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + + # Ensure that the submitter is notified + self.assertIn(submitter_email, mail.outbox[expected_email_count - 1].to) + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + for cc_email in cc_list: + self.assertIn(cc_email, mail.outbox[expected_email_count - 1].to) + def test_create_followup_from_email_with_valid_message_id_with_original_cc_list_included(self): """ @@ -374,10 +429,17 @@ class EmailInteractionsTestCase(TestCase): self.assertTrue(ticket_cc.email, cc_email) self.assertTrue(ticket_cc.can_view, True) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - self.assertEqual(email_count + 1 + 2 + 1, len(mail.outbox)) + # As we have created a Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + + # Ensure that the submitter is notified + self.assertIn(submitter_email,mail.outbox[0].to) + + # As we have created an Ticket from an email, we notify the sender (+1) + # and the new and update queues (+2) + expected_email_count = 1 + 2 + self.assertEqual(expected_email_count, len(mail.outbox)) ### end of the Ticket and TicketCCs creation ### # Reply message @@ -402,25 +464,28 @@ class EmailInteractionsTestCase(TestCase): ticket = Ticket.objects.get(id=followup.ticket.id) self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) - # Ensure that is created + # As an update was made, we increase the expected_email_count with: + # public_update_queue: +1 + expected_email_count += 1 + self.assertEqual(expected_email_count, len(mail.outbox)) + + # As we have created a FollowUp from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + + # Ensure that the submitter is notified + self.assertIn(submitter_email, mail.outbox[expected_email_count - 1].to) + + # Ensure that contacts on cc_list will be notified on the same email (index 0) for cc_email in cc_list: + self.assertIn(cc_email, mail.outbox[expected_email_count - 1].to) + # Even after 2 messages with the same cc_list, # MUST return only one object ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - expected_email_count = 1 + 2 + 1 - - # As an update was made, we increase the expected_email_count with: - # cc_list: +1 - # public_update_queue: +1 - expected_email_count += 1 + 1 - self.assertEqual(expected_email_count, len(mail.outbox)) - def test_create_followup_from_email_with_invalid_message_id(self): """ @@ -458,10 +523,25 @@ class EmailInteractionsTestCase(TestCase): self.assertTrue(ticket_cc.email, cc_email) self.assertTrue(ticket_cc.can_view, True) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - self.assertEqual(email_count + 1 + 2 + 1, len(mail.outbox)) + # As we have created an Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + expected_email_count = 1 + 2 + self.assertEqual(expected_email_count, len(mail.outbox)) + + # Ensure that the submitter is notified + self.assertIn(submitter_email,mail.outbox[0].to) + + # Ensure that is created + for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[0].to) + + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) + self.assertTrue(ticket_cc.ticket, ticket) + self.assertTrue(ticket_cc.email, cc_email) + ### end of the Ticket and TicketCCs creation ### # Reply message @@ -491,6 +571,7 @@ class EmailInteractionsTestCase(TestCase): ticket = Ticket.objects.get(id=followup.ticket.id) self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) + # Ensure that is created for cc_email in cc_list: # Even after 2 messages with the same cc_list, MUST return only @@ -502,7 +583,7 @@ class EmailInteractionsTestCase(TestCase): # As we have created an Ticket from an email, we notify the sender (+1), # the new and update queues (+2) and contacts on the cc_list (+1 as it's # treated as a list) - self.assertEqual(email_count + 1 + 2 + 1, len(mail.outbox)) + self.assertEqual(email_count + 1 + 2, len(mail.outbox)) def test_create_ticket_from_email_to_a_notification_enabled_queue(self): @@ -526,24 +607,32 @@ class EmailInteractionsTestCase(TestCase): msg.set_payload(self.ticket_data['description']) email_count = len(mail.outbox) - object_from_message(str(msg), self.queue_public, quiet=True) + followup = FollowUp.objects.get(message_id=message_id) ticket = Ticket.objects.get(id=followup.ticket.id) self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - self.assertEqual(email_count + 1 + 2 + 1, len(mail.outbox)) + # As we have created a Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + self.assertEqual(email_count + 1 + 2, len(mail.outbox)) - # Ensure that is created + # Ensure that the submitter is notified + self.assertIn(submitter_email, mail.outbox[0].to) + + # Ensure that exist for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[0].to) + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) + def test_create_ticket_from_email_to_a_notification_disabled_queue(self): """ @@ -581,6 +670,10 @@ class EmailInteractionsTestCase(TestCase): # Ensure that is created even if the Queue notifications are disabled # so when staff members interact with the , they get notified for cc_email in cc_list: + + # Ensure that contacts on the cc_list are not notified + self.assertNotIn(cc_email, mail.outbox[0].to) + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) @@ -615,14 +708,18 @@ class EmailInteractionsTestCase(TestCase): ticket = Ticket.objects.get(id=followup.ticket.id) self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - expected_email_count = email_count + 1 + 2 + 1 + # As we have created an Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + expected_email_count = email_count + 1 + 2 self.assertEqual(expected_email_count, len(mail.outbox)) # Ensure that is created for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[0].to) + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) @@ -649,11 +746,21 @@ class EmailInteractionsTestCase(TestCase): self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) # As an update was made, we increase the expected_email_count with: - # the ticket submitter: +1 + # a new email to all TicketCC subscribers : +1 # public_update_queue: +1 - expected_email_count += 1 + 1 + expected_email_count += 1 + 1 self.assertEqual(expected_email_count, len(mail.outbox)) + # Ensure that exist + for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[expected_email_count - 1].to) + + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) + self.assertTrue(ticket_cc.ticket, ticket) + self.assertTrue(ticket_cc.email, cc_email) + def test_create_followup_from_email_to_a_notification_diabled_queue(self): """ @@ -684,15 +791,19 @@ class EmailInteractionsTestCase(TestCase): ticket = Ticket.objects.get(id=followup.ticket.id) self.assertEqual(ticket.ticket_for_url, "mq1-%s" % ticket.id) - # As we have created an Ticket from an email, we notify the sender (+1), - # the new and update queues (+2) and contacts on the cc_list (+1 as it's - # treated as a list) - expected_email_count = email_count + 1 + 2 + 1 + # As we have created an Ticket from an email, we notify the sender + # and contacts on the cc_list (+1 as it's treated as a list), + # the new and update queues (+2) + expected_email_count = email_count + 1 + 2 self.assertEqual(expected_email_count, len(mail.outbox)) # Ensure that is created for cc_email in cc_list: + + # Ensure that contacts on cc_list will be notified on the same email (index 0) + self.assertIn(cc_email, mail.outbox[0].to) + ticket_cc = TicketCC.objects.get(ticket=ticket, email=cc_email) self.assertTrue(ticket_cc.ticket, ticket) self.assertTrue(ticket_cc.email, cc_email) @@ -724,7 +835,6 @@ class EmailInteractionsTestCase(TestCase): self.assertEqual(expected_email_count, len(mail.outbox)) - def test_create_followup_from_email_with_valid_message_id_with_original_cc_list_included(self): """ diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index 51969dd3..d9752392 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -340,30 +340,23 @@ def return_ticketccstring_and_show_subscribe(user, ticket): def subscribe_to_ticket_updates(ticket, user=None, email=None, can_view=True, can_update=False): - data = { - 'user': user, - 'email': email, - 'can_view': can_view, - 'can_update': can_update - } - - ticket_cc_form = TicketCCForm(data) - if ticket is not None and ticket_cc_form.is_valid(): + if ticket is not None: queryset = TicketCC.objects.filter(ticket=ticket, user=user, email=email) + # Dont'create duplicate entries for subscribers if queryset.count() > 0: return queryset.first() - ticketcc = ticket_cc_form.save(commit=False) - ticketcc.ticket = ticket - ticketcc.save() - return ticketcc - else: - raise ValidationError( - _('Could not create subscribe contact to ticket updated. Errors: {}'.format(ticket_cc_form.errors)) + if user is None and len(email) < 5: + raise ValidationError( + _('When you add somebody on Cc, you must provided either an User or a valid email. Email: %s' %email) ) + ticketcc = TicketCC.objects.create(ticket=ticket, user=user, email=email,can_view=can_view, can_update=can_update) + + return ticketcc + def subscribe_staff_member_to_ticket(ticket, user, email=''):