Merge pull request #950 from koriaf/dev-pep8-stuff

chore(pep8): Fix bad imports, unused variables, pep8 warnings and some possible bugs

.flake8

@@ -0,0 +1,4 @@
+[flake8]
+max-line-length = 120
+exclude = .git,__pycache__,.tox,.eggs,*.egg,node_modules,.venv,migrations,docs,demo,tests,setup.py
+import-order-style = pep8

helpdesk/lib.py

@@ -8,13 +8,11 @@
 
 import logging
 import mimetypes
-import os
 
 from django.conf import settings
-from django.utils.encoding import smart_text, smart_str
-from django.utils.safestring import mark_safe
+from django.utils.encoding import smart_text
 
-from helpdesk.models import FollowUpAttachment, EmailTemplate
+from helpdesk.models import FollowUpAttachment
 
 
 logger = logging.getLogger('helpdesk')

helpdesk/management/commands/create_usersettings.py

@@ -13,7 +13,6 @@
 from django.contrib.auth import get_user_model
 
 from helpdesk.models import UserSettings
-from helpdesk.settings import DEFAULT_USER_SETTINGS
 
 User = get_user_model()
 

helpdesk/models.py

@@ -1516,7 +1516,10 @@ class UserSettings(models.Model):
 
     email_on_ticket_change = models.BooleanField(
         verbose_name=_('E-mail me on ticket change?'),
-        help_text=_('If you\'re the ticket owner and the ticket is changed via the web by somebody else, do you want to receive an e-mail?'),
+        help_text=_(
+            'If you\'re the ticket owner and the ticket is changed via the web by somebody else,'
+            'do you want to receive an e-mail?'
+        ),
         default=email_on_ticket_change_default,
     )
 

helpdesk/query.py

@@ -1,6 +1,7 @@
 from django.db.models import Q
 from django.core.cache import cache
 from django.urls import reverse
+from django.utils.html import escape
 from django.utils.translation import ugettext as _
 
 from base64 import b64encode
@@ -135,7 +136,8 @@ def __run__(self, queryset):
             if sortreverse:
                 sorting = "-%s" % sorting
             queryset = queryset.order_by(sorting)
-        return queryset.distinct()  # https://stackoverflow.com/questions/30487056/django-queryset-contains-duplicate-entries
+        # https://stackoverflow.com/questions/30487056/django-queryset-contains-duplicate-entries
+        return queryset.distinct()
 
     def get_cache_key(self):
         return str(self.huser.user.pk) + ":" + self.base64
@@ -200,8 +202,13 @@ def get_timeline_context(self):
                     'start_date': self.mk_timeline_date(followup.date),
                     'text': {
                         'headline': ticket.title + ' - ' + followup.title,
-                        'text': (followup.comment if followup.comment else _('No text')) + '<br/> <a href="%s" class="btn" role="button">%s</a>' %
-                        (reverse('helpdesk:view', kwargs={'ticket_id': ticket.pk}), _("View ticket")),
+                        'text': (
+                            (escape(followup.comment) if followup.comment else _('No text'))
+                            +
+                            '<br/> <a href="%s" class="btn" role="button">%s</a>'
+                            %
+                            (reverse('helpdesk:view', kwargs={'ticket_id': ticket.pk}), _("View ticket"))
+                        ),
                     },
                     'group': _('Messages'),
                 }

helpdesk/settings.py

@@ -96,7 +96,10 @@
 # can be True/False or a callable accepting the active user and returning True if they must be considered helpdesk staff
 HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = getattr(settings, 'HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE', False)
 if not (HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE in (True, False) or callable(HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE)):
-    warnings.warn("HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE should be set to either True/False or a callable.", RuntimeWarning)
+    warnings.warn(
+        "HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE should be set to either True/False or a callable.",
+        RuntimeWarning
+    )
 
 # show edit buttons in ticket follow ups.
 HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP = getattr(settings,

helpdesk/templated_email.py

@@ -1,5 +1,4 @@
 import os
-import mimetypes
 import logging
 from smtplib import SMTPException
 

helpdesk/templatetags/helpdesk_staff.py

@@ -1,11 +1,11 @@
 """
 django-helpdesk - A Django powered ticket tracker for small enterprise.
 
-templatetags/helpdesk_staff.py - The is_helpdesk_staff template filter returns True if the user qualifies as Helpdesk staff.
+The is_helpdesk_staff template filter returns True if the user qualifies as Helpdesk staff.
+templatetags/helpdesk_staff.py
 """
 import logging
 from django.template import Library
-from django.db.models import Q
 
 from helpdesk.decorators import is_helpdesk_staff
 
@@ -18,5 +18,5 @@
 def helpdesk_staff(user):
     try:
         return is_helpdesk_staff(user)
-    except Exception as e:
+    except Exception:
         logger.exception("'helpdesk_staff' template tag (django-helpdesk) crashed")

helpdesk/urls.py

@@ -16,7 +16,8 @@
 from helpdesk import settings as helpdesk_settings
 from helpdesk.views import feeds, staff, public, kb, login
 try:
-    import helpdesk.tasks
+    # TODO: why is it imported? due to some side-effect or by mistake?
+    import helpdesk.tasks  # NOQA
 except ImportError:
     pass
 

helpdesk/user.py

@@ -67,7 +67,11 @@ def can_access_queue(self, queue):
         if self.has_full_access():
             return True
         else:
-            return helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION and self.user.has_perm(queue.permission_name)
+            return (
+                helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION
+                and
+                self.user.has_perm(queue.permission_name)
+            )
 
     def can_access_ticket(self, ticket):
         """Check to see if the user has permission to access

helpdesk/views/abstract_views.py

@@ -1,5 +1,3 @@
-from django.views.generic.edit import FormView
-
 from helpdesk.models import CustomField, KBItem, Queue
 
 
@@ -11,8 +9,9 @@ def get_initial(self):
             initial_data['queue'] = Queue.objects.get(slug=request.GET.get('queue', None)).id
         except Queue.DoesNotExist:
             pass
-        if request.user.is_authenticated and request.user.usersettings_helpdesk.use_email_as_submitter and request.user.email:
-            initial_data['submitter_email'] = request.user.email
+        u = request.user
+        if u.is_authenticated and u.usersettings_helpdesk.use_email_as_submitter and u.email:
+            initial_data['submitter_email'] = u.email
 
         query_param_fields = ['submitter_email', 'title', 'body', 'queue', 'kbitem']
         custom_fields = ["custom_%s" % f.name for f in CustomField.objects.filter(staff_only=False)]

helpdesk/views/login.py

@@ -11,7 +11,14 @@
 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 != resolve_url(request.resolver_match.view_name) and (login_url != request.resolver_match.view_name)):
+    condition = (
+        login_url
+        and (
+            login_url != resolve_url(request.resolver_match.view_name)
+            and (login_url != request.resolver_match.view_name)
+        )
+    )
+    if condition:
         if 'next' in request.GET:
             return_to = request.GET['next']
         else:

helpdesk/views/public.py

@@ -28,7 +28,7 @@
 import helpdesk.views.staff as staff
 import helpdesk.views.abstract_views as abstract_views
 from helpdesk.lib import text_is_spam
-from helpdesk.models import CustomField, Ticket, Queue, UserSettings, KBCategory, KBItem
+from helpdesk.models import Ticket, Queue, UserSettings
 from helpdesk.user import huser_from_request
 
 logger = logging.getLogger(__name__)
@@ -119,9 +119,6 @@ def form_valid(self, form):
                 # if someone enters a non-int string for the ticket
                 return HttpResponseRedirect(reverse('helpdesk:home'))
 
-    def get_success_url(self):
-        request = self.request
-
 
 class CreateTicketIframeView(BaseCreateTicketView):
     template_name = 'helpdesk/public_create_ticket_iframe.html'

helpdesk/views/staff.py

@@ -151,16 +151,15 @@ def dashboard(request):
     #          Open  Resolved
     # Queue 1    10     4
     # Queue 2     4    12
-
-    queues = HelpdeskUser(request.user).get_queues().values_list('id', flat=True)
-
-    from_clause = """FROM    helpdesk_ticket t,
-                    helpdesk_queue q"""
-    if queues:
-        where_clause = """WHERE   q.id = t.queue_id AND
-                        q.id IN (%s)""" % (",".join(("%d" % pk for pk in queues)))
-    else:
-        where_clause = """WHERE   q.id = t.queue_id"""
+    # code never used (and prone to sql injections)
+    # queues = HelpdeskUser(request.user).get_queues().values_list('id', flat=True)
+    # from_clause = """FROM    helpdesk_ticket t,
+    #                 helpdesk_queue q"""
+    # if queues:
+    #     where_clause = """WHERE   q.id = t.queue_id AND
+    #                     q.id IN (%s)""" % (",".join(("%d" % pk for pk in queues)))
+    # else:
+    #     where_clause = """WHERE   q.id = t.queue_id"""
 
     # get user assigned tickets page
     paginator = Paginator(
@@ -554,7 +553,11 @@ def update_ticket(request, ticket_id, public=False):
     # broken into two stages to prevent changes from first replace being themselves
     # changed by the second replace due to conflicting syntax
     comment = comment.replace('{%', 'X-HELPDESK-COMMENT-VERBATIM').replace('%}', 'X-HELPDESK-COMMENT-ENDVERBATIM')
-    comment = comment.replace('X-HELPDESK-COMMENT-VERBATIM', '{% verbatim %}{%').replace('X-HELPDESK-COMMENT-ENDVERBATIM', '%}{% endverbatim %}')
+    comment = comment.replace(
+        'X-HELPDESK-COMMENT-VERBATIM', '{% verbatim %}{%'
+    ).replace(
+        'X-HELPDESK-COMMENT-ENDVERBATIM', '%}{% endverbatim %}'
+    )
     # render the neutralized template
     comment = template_func(comment).render(context)
 
@@ -591,7 +594,6 @@ def update_ticket(request, ticket_id, public=False):
         ticket.status = new_status
         ticket.save()
         f.new_status = new_status
-        ticket_status_changed = True
         if f.title:
             f.title += ' and %s' % ticket.get_status_display()
         else:
@@ -701,7 +703,10 @@ def update_ticket(request, ticket_id, public=False):
     else:
         template_staff = 'updated_owner'
 
-    if ticket.assigned_to and (ticket.assigned_to.usersettings_helpdesk.email_on_ticket_change or (reassigned and ticket.assigned_to.usersettings_helpdesk.email_on_ticket_assigned)):
+    if ticket.assigned_to and (
+        ticket.assigned_to.usersettings_helpdesk.email_on_ticket_change
+        or (reassigned and ticket.assigned_to.usersettings_helpdesk.email_on_ticket_assigned)
+    ):
         messages_sent_to.update(ticket.send(
             {'assigned_to': (template_staff, context)},
             dont_send_to=messages_sent_to,
@@ -1071,7 +1076,6 @@ def ticket_list(request):
         pass
     elif not {'queue', 'assigned_to', 'status', 'q', 'sort', 'sortreverse', 'kbitem'}.intersection(request.GET):
         # Fall-back if no querying is being done
-        all_queues = Queue.objects.all()
         query_params = deepcopy(default_query_params)
     else:
         filter_in_params = [