From 61dd12abd9f2fbccf567f2ced5327401ccba8705 Mon Sep 17 00:00:00 2001 From: Timothy Hobbs Date: Wed, 16 Oct 2019 17:36:55 +0200 Subject: [PATCH] Fix tests --- helpdesk/tests/helpers.py | 4 +++ helpdesk/tests/test_navigation.py | 2 +- .../tests/test_per_queue_staff_permission.py | 22 +++++++++++----- helpdesk/tests/test_ticket_actions.py | 8 +++--- helpdesk/tests/test_time_spent.py | 1 - helpdesk/views/login.py | 2 +- helpdesk/views/staff.py | 26 +------------------ 7 files changed, 26 insertions(+), 39 deletions(-) diff --git a/helpdesk/tests/helpers.py b/helpdesk/tests/helpers.py index 285e922e..57775c76 100644 --- a/helpdesk/tests/helpers.py +++ b/helpdesk/tests/helpers.py @@ -26,6 +26,10 @@ def get_user(username='helpdesk.staff', return user +def get_staff_user(): + return get_user(is_staff=True) + + def reload_urlconf(urlconf=None): from importlib import reload diff --git a/helpdesk/tests/test_navigation.py b/helpdesk/tests/test_navigation.py index 2078d93f..24fcc3fc 100644 --- a/helpdesk/tests/test_navigation.py +++ b/helpdesk/tests/test_navigation.py @@ -25,7 +25,7 @@ class KBDisabledTestCase(TestCase): """Test proper rendering of navigation.html by accessing the dashboard""" from django.urls import NoReverseMatch - self.client.login(username=get_user(is_staff=True).get_username(), password='password') + self.client.login(username=get_staff_user().get_username(), password='password') self.assertRaises(NoReverseMatch, reverse, 'helpdesk:kb_index') try: response = self.client.get(reverse('helpdesk:dashboard')) diff --git a/helpdesk/tests/test_per_queue_staff_permission.py b/helpdesk/tests/test_per_queue_staff_permission.py index f1bd8be7..b70d7816 100644 --- a/helpdesk/tests/test_per_queue_staff_permission.py +++ b/helpdesk/tests/test_per_queue_staff_permission.py @@ -6,6 +6,8 @@ from django.test.client import Client from helpdesk.models import Queue, Ticket from helpdesk import settings +from helpdesk.query import get_query +from helpdesk.user import HelpdeskUser class PerQueueStaffMembershipTestCase(TestCase): @@ -14,8 +16,8 @@ class PerQueueStaffMembershipTestCase(TestCase): def setUp(self): """ - Create user_1 with access to queue_1 containing 1 ticket - and user_2 with access to queue_2 containing 2 tickets + Create user_1 with access to queue_1 containing 2 ticket + and user_2 with access to queue_2 containing 4 tickets and superuser who should be able to access both queues """ self.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION @@ -31,6 +33,8 @@ class PerQueueStaffMembershipTestCase(TestCase): self.superuser.set_password('superuser') self.superuser.save() + self.identifier_users = {} + for identifier in self.IDENTIFIERS: queue = self.__dict__['queue_%d' % identifier] = Queue.objects.create( title='Queue %d' % identifier, @@ -40,9 +44,11 @@ class PerQueueStaffMembershipTestCase(TestCase): user = self.__dict__['user_%d' % identifier] = User.objects.create( username='User_%d' % identifier, is_staff=True, + email="foo%s@example.com" % identifier ) user.set_password(str(identifier)) user.save() + self.identifier_users[identifier] = user # The prefix 'helpdesk.' must be trimmed p = Permission.objects.get(codename=queue.permission_name[9:]) @@ -68,7 +74,7 @@ class PerQueueStaffMembershipTestCase(TestCase): def test_dashboard_ticket_counts(self): """ Check that the regular users' dashboard only shows 1 of the 2 queues, - that user_1 only sees a total of 1 ticket, that user_2 sees a total of 2 + that user_1 only sees a total of 2 tickets, that user_2 sees a total of 4 tickets, but that the superuser's dashboard shows all queues and tickets. """ @@ -105,7 +111,7 @@ class PerQueueStaffMembershipTestCase(TestCase): def test_report_ticket_counts(self): """ Check that the regular users' report only shows 1 of the 2 queues, - that user_1 only sees a total of 1 ticket, that user_2 sees a total of 2 + that user_1 only sees a total of 2 tickets, that user_2 sees a total of 4 tickets, but that the superuser's report shows all queues and tickets. """ @@ -153,15 +159,16 @@ class PerQueueStaffMembershipTestCase(TestCase): def test_ticket_list_per_queue_user_restrictions(self): """ Ensure that while the superuser can list all tickets, user_1 can only - list the 1 ticket in his queue and user_2 can list only the 2 tickets + list the 2 tickets in his queue and user_2 can list only the 4 tickets in his queue. """ # Regular users for identifier in self.IDENTIFIERS: self.client.login(username='User_%d' % identifier, password=str(identifier)) response = self.client.get(reverse('helpdesk:list')) + tickets = get_query(response.context['urlsafe_query'], HelpdeskUser(self.identifier_users[identifier])) self.assertEqual( - len(response.context['tickets']), + len(tickets), identifier * 2, 'Ticket list was not properly limited by queue membership' ) @@ -179,8 +186,9 @@ class PerQueueStaffMembershipTestCase(TestCase): # Superuser self.client.login(username='superuser', password='superuser') response = self.client.get(reverse('helpdesk:list')) + tickets = get_query(response.context['urlsafe_query'], HelpdeskUser(self.superuser)) self.assertEqual( - len(response.context['tickets']), + len(tickets), 6, 'Ticket list was limited by queue membership for a superuser' ) diff --git a/helpdesk/tests/test_ticket_actions.py b/helpdesk/tests/test_ticket_actions.py index 8b2b4627..407004a2 100644 --- a/helpdesk/tests/test_ticket_actions.py +++ b/helpdesk/tests/test_ticket_actions.py @@ -13,7 +13,7 @@ except ImportError: # python 2 from urlparse import urlparse from helpdesk.templatetags.ticket_to_link import num_to_link -from helpdesk.views.staff import _is_my_ticket +from helpdesk.user import HelpdeskUser class TicketActionsTestCase(TestCase): @@ -150,7 +150,7 @@ class TicketActionsTestCase(TestCase): response = self.client.post(reverse('helpdesk:update', kwargs={'ticket_id': ticket_id}), post_data, follow=True) self.assertContains(response, 'Changed Status from Open to Closed') - def test_is_my_ticket(self): + def test_can_access_ticket(self): """Tests whether non-staff but assigned user still counts as owner""" # make non-staff user @@ -173,8 +173,8 @@ class TicketActionsTestCase(TestCase): # create ticket helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = True ticket = Ticket.objects.create(**initial_data) - self.assertEqual(_is_my_ticket(self.user, ticket), True) - self.assertEqual(_is_my_ticket(self.user2, ticket), False) + self.assertEqual(HelpdeskUser(self.user).can_access_ticket(ticket), True) + self.assertEqual(HelpdeskUser(self.user2).can_access_ticket(ticket), False) def test_num_to_link(self): """Test that we are correctly expanding links to tickets from IDs""" diff --git a/helpdesk/tests/test_time_spent.py b/helpdesk/tests/test_time_spent.py index a71c3ac9..5caf7df5 100644 --- a/helpdesk/tests/test_time_spent.py +++ b/helpdesk/tests/test_time_spent.py @@ -17,7 +17,6 @@ except ImportError: # python 2 from urlparse import urlparse from helpdesk.templatetags.ticket_to_link import num_to_link -from helpdesk.views.staff import _is_my_ticket class TimeSpentTestCase(TestCase): diff --git a/helpdesk/views/login.py b/helpdesk/views/login.py index f2eef0dc..87a429e4 100644 --- a/helpdesk/views/login.py +++ b/helpdesk/views/login.py @@ -11,7 +11,7 @@ default_login_view = auth_views.LoginView.as_view( def login(request): login_url = settings.LOGIN_URL # Prevent redirect loop by checking that LOGIN_URL is not this view's name - if login_url and login_url != request.resolver_match.view_name: + if login_url and (login_url != resolve_url(request.resolver_match.view_name) and (login_url != request.resolver_match.view_name)): if 'next' in request.GET: return_to = request.GET['next'] else: diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index cb84108f..1e09328b 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -98,18 +98,6 @@ def _get_queue_choices(queues): return queue_choices -def _is_my_ticket(user, ticket): - """Check to see if the user has permission to access - a ticket. If not then deny access.""" - if (user, ticket.queue): - return True - elif user.is_superuser or user.is_staff or \ - (ticket.assigned_to and user.id == ticket.assigned_to.id): - return True - else: - return False - - @helpdesk_staff_member_required def dashboard(request): """ @@ -908,21 +896,10 @@ def ticket_list(request): urlsafe_query = query_to_base64(query_params) - tickets_base = get_query(urlsafe_query, huser) + get_query(urlsafe_query, huser) user_saved_queries = SavedSearch.objects.filter(Q(user=request.user) | Q(shared__exact=True)) - ticket_qs = None - try: - ticket_qs = apply_query(tickets_base, query_params) - except ValidationError: - # invalid parameters in query, return default query - query_params = { - 'filtering': {'status__in': [1, 2, 3]}, - 'sorting': 'created', - } - ticket_qs = apply_query(tickets_base, query_params) - search_message = '' if query_params['search_string'] and settings.DATABASES['default']['ENGINE'].endswith('sqlite'): search_message = _( @@ -935,7 +912,6 @@ def ticket_list(request): return render(request, 'helpdesk/ticket_list.html', dict( context, - tickets=ticket_qs, default_tickets_per_page=request.user.usersettings_helpdesk.tickets_per_page, user_choices=User.objects.filter(is_active=True, is_staff=True), queue_choices=huser.get_queues(),