Skip to content

Commit

Permalink
Fix QueueMembership bug revealed by django.test's DiscoverRunner
Browse files Browse the repository at this point in the history
If HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP was True but a user had
no QueueMembership entries, then restricting queue access generated
RelatedObjectDoesNotExist exceptions.

 - Ask for forgiveness whenever we try to limit a queryset by the
queuemembership related object set.

 - Since tests can now be run with the project's settings active,
rather than only with quicktest.py's settings, restore the initial
HELPDESK_ENABLE_PER_QUEUE_MEMBERSHIP value after having tested the
related functionality.
  • Loading branch information
reduxionist committed Nov 16, 2015
1 parent ddd5b21 commit 0610a66
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 20 deletions.
13 changes: 7 additions & 6 deletions helpdesk/tests/test_per_queue_staff_membership.py
Expand Up @@ -17,6 +17,7 @@ def setUp(self):
and user_2 with access to queue_2 containing 2 tickets
and superuser who should be able to access both queues
"""
self.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP
settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = True
self.client = Client()
User = get_user_model()
Expand Down Expand Up @@ -59,6 +60,12 @@ def setUp(self):
assigned_to=user,
)

def tearDown(self):
"""
Reset HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP to original value
"""
settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = self.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP

def test_dashboard_ticket_counts(self):
"""
Check that the regular users' dashboard only shows 1 of the 2 queues,
Expand Down Expand Up @@ -213,9 +220,3 @@ def test_ticket_reports_per_queue_user_restrictions(self):
3,
'Queue choices were improperly limited by queue membership for a superuser'
)

def tearDown(self):
"""
Don't interfere with subsequent tests that do not expect this setting
"""
settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = False
40 changes: 26 additions & 14 deletions helpdesk/views/staff.py
Expand Up @@ -39,7 +39,7 @@

from helpdesk.forms import TicketForm, UserSettingsForm, EmailIgnoreForm, EditTicketForm, TicketCCForm, EditFollowUpForm, TicketDependencyForm
from helpdesk.lib import send_templated_mail, query_to_dict, apply_query, safe_template_context
from helpdesk.models import Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch, IgnoreEmail, TicketCC, TicketDependency
from helpdesk.models import Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch, IgnoreEmail, TicketCC, TicketDependency, QueueMembership
from helpdesk import settings as helpdesk_settings

if helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE:
Expand Down Expand Up @@ -78,9 +78,12 @@ def dashboard(request):
)
limit_queues_by_user = helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP and not request.user.is_superuser
if limit_queues_by_user:
unassigned_tickets = unassigned_tickets.filter(
queue__in=request.user.queuemembership.queues.all(),
)
try:
unassigned_tickets = unassigned_tickets.filter(
queue__in=request.user.queuemembership.queues.all(),
)
except QueueMembership.DoesNotExist:
unassigned_tickets = unassigned_tickets.none()

# all tickets, reported by current user
all_tickets_reported_by_current_user = ''
Expand All @@ -92,9 +95,12 @@ def dashboard(request):

Tickets = Ticket.objects
if limit_queues_by_user:
Tickets = Tickets.filter(
queue__in=request.user.queuemembership.queues.all(),
)
try:
Tickets = Tickets.filter(
queue__in=request.user.queuemembership.queues.all(),
)
except QueueMembership.DoesNotExist:
Tickets = Tickets.none()
basic_ticket_stats = calc_basic_ticket_stats(Tickets)

# The following query builds a grid of queues & ticket statuses,
Expand Down Expand Up @@ -367,7 +373,7 @@ def update_ticket(request, ticket_id, public=False):
# if comment contains some django code, like "why does {% if bla %} crash",
# then the following line will give us a crash, since django expects {% if %}
# to be closed with an {% endif %} tag.


# get_template_from_string was removed in Django 1.8 http://django.readthedocs.org/en/1.8.x/ref/templates/upgrading.html
try:
Expand Down Expand Up @@ -1001,9 +1007,12 @@ def run_report(request, report):
report_queryset = Ticket.objects.all().select_related()
limit_queues_by_user = helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP and not request.user.is_superuser
if limit_queues_by_user:
report_queryset = report_queryset.filter(
queue__in=request.user.queuemembership.queues.all(),
)
try:
report_queryset = report_queryset.filter(
queue__in=request.user.queuemembership.queues.all(),
)
except QueueMembership.DoesNotExist:
report_queryset = report_queryset.none()

from_saved_query = False
saved_query = None
Expand Down Expand Up @@ -1065,9 +1074,12 @@ def run_report(request, report):
col1heading = _('User')
queue_options = Queue.objects.all()
if limit_queues_by_user:
queue_options = queue_options.filter(
pk__in=request.user.queuemembership.queues.all(),
)
try:
queue_options = queue_options.filter(
pk__in=request.user.queuemembership.queues.all(),
)
except QueueMembership.DoesNotExist:
queue_options = queue_options.none()
possible_options = [q.title.encode('utf-8') for q in queue_options]
charttype = 'bar'

Expand Down

0 comments on commit 0610a66

Please sign in to comment.