Merge pull request #779 from auto-mat/fix-tests

develop branch: Fix most tests

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

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'))

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
+        Create user_1 with access to queue_1 containing 2 ticket
-        and    user_2 with access to queue_2 containing 2 tickets
+        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'
         )

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(HelpdeskUser(self.user).can_access_ticket(ticket), True)
-        self.assertEqual(_is_my_ticket(self.user2, ticket), False)
+        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"""

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):

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:

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(),