Refactor search logic to a create an 'omnisearch' (#484)

PyRIGS/decorators.py

@@ -9,9 +9,8 @@ from RIGS import models
 
 def get_oembed(login_url, request, oembed_view, kwargs):
     context = {}
-    context['oembed_url'] = "{0}://{1}{2}".format(request.scheme, request.META['HTTP_HOST'],
+    context['oembed_url'] = f"{request.scheme}://{request.META['HTTP_HOST']}{reverse(oembed_view, kwargs=kwargs)}"
-                                                  reverse(oembed_view, kwargs=kwargs))
+    context['login_url'] = f"{login_url}?{REDIRECT_FIELD_NAME}={request.get_full_path()}"
-    context['login_url'] = "{0}?{1}={2}".format(login_url, REDIRECT_FIELD_NAME, request.get_full_path())
     resp = render(request, 'login_redirect.html', context=context)
     return resp
 
@@ -25,7 +24,7 @@ def has_oembed(oembed_view, login_url=settings.LOGIN_URL):
                 if oembed_view is not None:
                     return get_oembed(login_url, request, oembed_view, kwargs)
                 else:
-                    return HttpResponseRedirect('%s?%s=%s' % (login_url, REDIRECT_FIELD_NAME, request.get_full_path()))
+                    return HttpResponseRedirect(f'{login_url}?{REDIRECT_FIELD_NAME}={request.get_full_path()}')
 
         _checklogin.__doc__ = view_func.__doc__
         _checklogin.__dict__ = view_func.__dict__
@@ -55,7 +54,7 @@ def user_passes_test_with_403(test_func, login_url=None, oembed_view=None):
                 if oembed_view is not None:
                     return get_oembed(login_url, request, oembed_view, kwargs)
                 else:
-                    return HttpResponseRedirect('%s?%s=%s' % (login_url, REDIRECT_FIELD_NAME, request.get_full_path()))
+                    return HttpResponseRedirect(f'{login_url}?{REDIRECT_FIELD_NAME}={request.get_full_path()}')
             else:
                 resp = render(request, '403.html')
                 resp.status_code = 403

PyRIGS/tests/test_unit.py

@@ -10,6 +10,7 @@ from pytest_django.asserts import assertTemplateUsed, assertInHTML
 from PyRIGS import urls
 from RIGS.models import Event, Profile
 from assets.models import Asset
+from training.tests.test_unit import get_response
 from django.db import connection
 from django.template.defaultfilters import striptags
 from django.urls.exceptions import NoReverseMatch
@@ -135,3 +136,11 @@ def test_keyholder_access(client):
     assertContains(response, 'View Revision History')
     client.logout()
     call_command('deleteSampleData')
+
+
+def test_search(admin_client, admin_user):
+    url = reverse('search')
+    response = admin_client.get(url, {'q': "Definetelynothingfoundifwesearchthis"})
+    assertContains(response, "No results found")
+    response = admin_client.get(url, {'q': admin_user.first_name})
+    assertContains(response, admin_user.first_name)

PyRIGS/urls.py

@@ -23,6 +23,7 @@ urlpatterns = [
          name="api_secure"),
 
     path('closemodal/', views.CloseModal.as_view(), name='closemodal'),
+    path('search/', login_required(views.Search.as_view()), name='search'),
     path('search_help/', login_required(views.SearchHelp.as_view()), name='search_help'),
 
     path('', include('users.urls')),

PyRIGS/views.py

@@ -1,6 +1,7 @@
 import datetime
 import operator
 from functools import reduce
+from itertools import chain
 
 from django.contrib.auth.decorators import login_required
 from django.contrib import messages
@@ -120,7 +121,7 @@ class SecureAPIRequest(generic.View):
                     'text': o.name,
                 }
                 try:  # See if there is a valid update URL
-                    data['update'] = reverse("%s_update" % model, kwargs={'pk': o.pk})
+                    data['update'] = reverse(f"{model}_update", kwargs={'pk': o.pk})
                 except NoReverseMatch:
                     pass
                 results.append(data)
@@ -182,20 +183,7 @@ class GenericListView(generic.ListView):
         return context
 
     def get_queryset(self):
-        q = self.request.GET.get('q', "")
+        object_list = self.model.objects.search(query=self.request.GET.get('q', ""))
-
-        filter = Q(name__icontains=q) | Q(email__icontains=q) | Q(address__icontains=q) | Q(notes__icontains=q) | Q(
-            phone__startswith=q) | Q(phone__endswith=q)
-
-        # try and parse an int
-        try:
-            val = int(q)
-            filter = filter | Q(pk=val)
-        except:  # noqa
-            # not an integer
-            pass
-
-        object_list = self.model.objects.filter(filter)
 
         orderBy = self.request.GET.get('orderBy', "name")
         if orderBy != "":
@@ -236,6 +224,53 @@ class GenericCreateView(generic.CreateView):
         return context
 
 
+class Search(generic.ListView):
+    template_name = 'search_results.html'
+    paginate_by = 20
+    count = 0
+
+    def get_context_data(self, *args, **kwargs):
+        context = super().get_context_data(*args, **kwargs)
+        context['count'] = self.count or 0
+        context['query'] = self.request.GET.get('q')
+        context['page_title'] = f"{context['count']} search results for <b>{context['query']}</b>"
+        return context
+
+    def get_queryset(self):
+        request = self.request
+        query = request.GET.get('q', None)
+
+        if query is not None:
+            event_results = models.Event.objects.search(query)
+            person_results = models.Person.objects.search(query)
+            organisation_results = models.Organisation.objects.search(query)
+            venue_results = models.Venue.objects.search(query)
+            invoice_results = models.Invoice.objects.search(query)
+            asset_results = asset_models.Asset.objects.search(query)
+            supplier_results = asset_models.Supplier.objects.search(query)
+            trainee_results = training_models.Trainee.objects.search(query)
+            training_item_results = training_models.TrainingItem.objects.search(query)
+
+            # combine querysets
+            queryset_chain = chain(
+                event_results,
+                person_results,
+                organisation_results,
+                venue_results,
+                invoice_results,
+                asset_results,
+                supplier_results,
+                trainee_results,
+                training_item_results,
+            )
+            qs = sorted(queryset_chain,
+                        key=lambda instance: instance.pk,
+                        reverse=True)
+            self.count = len(qs)  # since qs is actually a list
+            return qs
+        return models.Event.objects.none()  # just an empty queryset as default
+
+
 class SearchHelp(generic.TemplateView):
     template_name = 'search_help.html'
 

RIGS/forms.py

@@ -172,9 +172,9 @@ class EventRiskAssessmentForm(forms.ModelForm):
         unexpected_values = []
         for field, value in models.RiskAssessment.expected_values.items():
             if self.cleaned_data.get(field) != value:
-                unexpected_values.append("<li>{}</li>".format(self._meta.model._meta.get_field(field).help_text))
+                unexpected_values.append(f"<li>{self._meta.model._meta.get_field(field).help_text}</li>")
         if len(unexpected_values) > 0 and not self.cleaned_data.get('supervisor_consulted'):
-            raise forms.ValidationError("Your answers to these questions: <ul>{}</ul> require consulting with a supervisor.".format(''.join([str(elem) for elem in unexpected_values])), code='unusual_answers')
+            raise forms.ValidationError(f"Your answers to these questions: <ul>{''.join([str(elem) for elem in unexpected_values])}</ul> require consulting with a supervisor.", code='unusual_answers')
         return super(EventRiskAssessmentForm, self).clean()
 
     class Meta:
@@ -235,9 +235,9 @@ class EventChecklistForm(forms.ModelForm):
             pk = int(key.split('_')[1])
 
             for field in other_fields:
-                value = self.data['{}_{}'.format(field, pk)]
+                value = self.data[f'{field}_{pk}']
                 if value == '':
-                    raise forms.ValidationError('Add a {} to crewmember {}'.format(field, pk), code='{}_mismatch'.format(field))
+                    raise forms.ValidationError(f'Add a {field} to crewmember {pk}', code=f'{field}_mismatch')
 
             try:
                 item = models.EventChecklistCrew.objects.get(pk=pk)

RIGS/models.py

@@ -8,6 +8,7 @@ from urllib.parse import urlparse
 
 import pytz
 from django import forms
+from django.db.models import Q, F
 from django.conf import settings
 from django.contrib.auth.models import AbstractUser
 from django.core.exceptions import ValidationError
@@ -20,6 +21,17 @@ from reversion.models import Version
 from versioning.versioning import RevisionMixin
 
 
+def filter_by_pk(filt, query):
+    # try and parse an int
+    try:
+        val = int(query)
+        filt = filt | Q(pk=val)
+    except:  # noqa
+        # not an integer
+        pass
+    return filt
+
+
 class Profile(AbstractUser):
     initials = models.CharField(max_length=5, null=True, blank=False)
     phone = models.CharField(max_length=13, blank=True, default='')
@@ -51,7 +63,7 @@ class Profile(AbstractUser):
     def name(self):
         name = self.get_full_name()
         if self.initials:
-            name += ' "{}"'.format(self.initials)
+            name += f' "{self.initials}"'
         return name
 
     @property
@@ -70,15 +82,28 @@ class Profile(AbstractUser):
         return self.name
 
 
+class ContactableManager(models.Manager):
+    def search(self, query=None):
+        qs = self.get_queryset()
+        if query is not None:
+            or_lookup = Q(name__icontains=query) | Q(email__icontains=query) | Q(address__icontains=query) | Q(notes__icontains=query) | Q(
+                phone__startswith=query) | Q(phone__endswith=query)
+
+            or_lookup = filter_by_pk(or_lookup, query)
+
+            qs = qs.filter(or_lookup).distinct()  # distinct() is often necessary with Q lookups
+        return qs
+
+
 class Person(models.Model, RevisionMixin):
     name = models.CharField(max_length=50)
     phone = models.CharField(max_length=15, blank=True, default='')
     email = models.EmailField(blank=True, default='')
-
     address = models.TextField(blank=True, default='')
-
     notes = models.TextField(blank=True, default='')
 
+    objects = ContactableManager()
+
     def __str__(self):
         string = self.name
         if self.notes is not None:
@@ -110,12 +135,12 @@ class Organisation(models.Model, RevisionMixin):
     name = models.CharField(max_length=50)
     phone = models.CharField(max_length=15, blank=True, default='')
     email = models.EmailField(blank=True, default='')
-
     address = models.TextField(blank=True, default='')
-
     notes = models.TextField(blank=True, default='')
     union_account = models.BooleanField(default=False)
 
+    objects = ContactableManager()
+
     def __str__(self):
         string = self.name
         if self.notes is not None:
@@ -184,9 +209,10 @@ class Venue(models.Model, RevisionMixin):
     email = models.EmailField(blank=True, default='')
     three_phase_available = models.BooleanField(default=False)
     notes = models.TextField(blank=True, default='')
-
     address = models.TextField(blank=True, default='')
 
+    objects = ContactableManager()
+
     def __str__(self):
         string = self.name
         if self.notes and len(self.notes) > 0:
@@ -260,6 +286,23 @@ class EventManager(models.Manager):
 
         return events
 
+    def search(self, query=None):
+        qs = self.get_queryset()
+        if query is not None:
+            or_lookup = Q(name__icontains=query) | Q(description__icontains=query) | Q(notes__icontains=query)
+
+            or_lookup = filter_by_pk(or_lookup, query)
+
+            try:
+                if query[0] == "N":
+                    val = int(query[1:])
+                    or_lookup = Q(pk=val)  # If string is N###### then do a simple PK filter
+            except:  # noqa
+                pass
+
+            qs = qs.filter(or_lookup).distinct()  # distinct() is often necessary with Q lookups
+        return qs
+
 
 @reversion.register(follow=['items'])
 class Event(models.Model, RevisionMixin):
@@ -314,10 +357,8 @@ class Event(models.Model, RevisionMixin):
     def display_id(self):
         if self.pk:
             if self.is_rig:
-                return str("N%05d" % self.pk)
+                return f"N{self.pk:05d}"
-
             return self.pk
-
         return "????"
 
     # Calculated values
@@ -530,6 +571,34 @@ class InvoiceManager(models.Manager):
         query = self.raw(sql)
         return query
 
+    def search(self, query=None):
+        qs = self.get_queryset()
+        if query is not None:
+            or_lookup = Q(event__name__icontains=query)
+
+            or_lookup = filter_by_pk(or_lookup, query)
+
+            # try and parse an int
+            try:
+                val = int(query)
+                or_lookup = or_lookup | Q(event__pk=val)
+            except:  # noqa
+                # not an integer
+                pass
+
+            try:
+                if query[0] == "N":
+                    val = int(query[1:])
+                    or_lookup = Q(event__pk=val)  # If string is Nxxxxx then filter by event number
+                elif query[0] == "#":
+                    val = int(query[1:])
+                    or_lookup = Q(pk=val)  # If string is #xxxxx then filter by invoice number
+            except:  # noqa
+                pass
+
+            qs = qs.filter(or_lookup).distinct()  # distinct() is often necessary with Q lookups
+        return qs
+
 
 @reversion.register(follow=['payment_set'])
 class Invoice(models.Model, RevisionMixin):
@@ -572,11 +641,11 @@ class Invoice(models.Model, RevisionMixin):
         return f"#{self.display_id} for Event {self.event.display_id}"
 
     def __str__(self):
-        return "%i: %s (%.2f)" % (self.pk, self.event, self.balance)
+        return f"{self.display_id}: {self.event} (£{self.balance:.2f})"
 
     @property
     def display_id(self):
-        return "{:05d}".format(self.pk)
+        return f"#{self.pk:05d}"
 
     class Meta:
         ordering = ['-invoice_date']
@@ -731,7 +800,7 @@ class RiskAssessment(models.Model, RevisionMixin):
         return reverse('ra_detail', kwargs={'pk': self.pk})
 
     def __str__(self):
-        return "%i - %s" % (self.pk, self.event)
+        return f"{self.pk} - {self.event}"
 
 
 @reversion.register(follow=['vehicles', 'crew'])
@@ -813,7 +882,7 @@ class EventChecklist(models.Model, RevisionMixin):
         return reverse('ec_detail', kwargs={'pk': self.pk})
 
     def __str__(self):
-        return "%i - %s" % (self.pk, self.event)
+        return f"{self.pk} - {self.event}"
 
 
 @reversion.register
@@ -825,7 +894,7 @@ class EventChecklistVehicle(models.Model, RevisionMixin):
     reversion_hide = True
 
     def __str__(self):
-        return "{} driven by {}".format(self.vehicle, str(self.driver))
+        return f"{self.vehicle} driven by {self.driver}"
 
 
 @reversion.register
@@ -843,4 +912,4 @@ class EventChecklistCrew(models.Model, RevisionMixin):
             raise ValidationError('Unless you\'ve invented time travel, crew can\'t finish before they have started.')
 
     def __str__(self):
-        return "{} ({})".format(str(self.crewmember), self.role)
+        return f"{self.crewmember} ({self.role})"

RIGS/signals.py

@@ -54,7 +54,7 @@ def send_eventauthorisation_success_email(instance):
     elif instance.event.organisation is not None and instance.email == instance.event.organisation.email:
         context['to_name'] = instance.event.organisation.name
 
-    subject = "N%05d | %s - Event Authorised" % (instance.event.pk, instance.event.name)
+    subject = f"{instance.event.display_id} | {instance.event.name} - Event Authorised"
 
     client_email = EmailMultiAlternatives(
         subject,
@@ -70,7 +70,7 @@ def send_eventauthorisation_success_email(instance):
 
     escapedEventName = re.sub(r'[^a-zA-Z0-9 \n\.]', '', instance.event.name)
 
-    client_email.attach('N%05d - %s - CONFIRMATION.pdf' % (instance.event.pk, escapedEventName),
+    client_email.attach(f'{instance.event.display_id} - {escapedEventName} - CONFIRMATION.pdf',
                         merged.getvalue(),
                         'application/pdf'
                         )
@@ -116,7 +116,7 @@ def send_admin_awaiting_approval_email(user, request, **kwargs):
             }
 
             email = EmailMultiAlternatives(
-                "%s new users awaiting approval on RIGS" % (context['number_of_users']),
+                f"{context['number_of_users']} new users awaiting approval on RIGS",
                 get_template("admin_awaiting_approval.txt").render(context),
                 to=[admin.email],
                 reply_to=[user.email],

RIGS/views/finance.py

@@ -28,7 +28,8 @@ class InvoiceIndex(generic.ListView):
         total = 0
         for i in context['object_list']:
             total += i.balance
-        context['page_title'] = "Outstanding Invoices ({} Events, £{:.2f})".format(len(list(context['object_list'])), total)
+        event_count = len(list(context['object_list']))
+        context['page_title'] = f"Outstanding Invoices ({event_count} Events, £{total:.2f})"
         context['description'] = "Paperwork for these events has been sent to treasury, but the full balance has not yet appeared on a ledger"
         return context
 
@@ -43,7 +44,7 @@ class InvoiceDetail(generic.DetailView):
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         invoice_date = self.object.invoice_date.strftime("%d/%m/%Y")
-        context['page_title'] = f"Invoice {self.object.display_id} ({invoice_date}) "
+        context['page_title'] = f"Invoice {self.object.display_id} ({invoice_date})"
         if self.object.void:
             context['page_title'] += "<span class='badge badge-warning float-right'>VOID</span>"
         elif self.object.is_closed:
@@ -59,11 +60,14 @@ class InvoicePrint(generic.View):
         object = invoice.event
         template = get_template('event_print.xml')
 
+        name = re.sub(r'[^a-zA-Z0-9 \n\.]', '', object.name)
+        filename = f"Invoice {invoice.display_id} for {object.display_id} {name}.pdf"
+
         context = {
             'object': object,
             'invoice': invoice,
             'current_user': request.user,
-            'filename': 'Invoice {} for {} {}.pdf'.format(invoice.display_id, object.display_id, re.sub(r'[^a-zA-Z0-9 \n\.]', '', object.name))
+            'filename': filename
         }
 
         rml = template.render(context)
@@ -73,7 +77,7 @@ class InvoicePrint(generic.View):
         pdfData = buffer.read()
 
         response = HttpResponse(content_type='application/pdf')
-        response['Content-Disposition'] = 'filename="{}"'.format(context['filename'])
+        response['Content-Disposition'] = f'filename="{filename}"'
         response.write(pdfData)
         return response
 
@@ -124,32 +128,7 @@ class InvoiceArchive(generic.ListView):
         return context
 
     def get_queryset(self):
-        q = self.request.GET.get('q', "")
+        return self.model.objects.search(self.request.GET.get('q')).order_by('-invoice_date')
-
-        filter = Q(event__name__icontains=q)
-
-        # try and parse an int
-        try:
-            val = int(q)
-            filter = filter | Q(pk=val)
-            filter = filter | Q(event__pk=val)
-        except:  # noqa
-            # not an integer
-            pass
-
-        try:
-            if q[0] == "N":
-                val = int(q[1:])
-                filter = Q(event__pk=val)  # If string is Nxxxxx then filter by event number
-            elif q[0] == "#":
-                val = int(q[1:])
-                filter = Q(pk=val)  # If string is #xxxxx then filter by invoice number
-        except:  # noqa
-            pass
-
-        object_list = self.model.objects.filter(filter).order_by('-invoice_date')
-
-        return object_list
 
 
 class InvoiceWaiting(generic.ListView):
@@ -163,7 +142,7 @@ class InvoiceWaiting(generic.ListView):
         objects = self.get_queryset()
         for obj in objects:
             total += obj.sum_total
-        context['page_title'] = "Events for Invoice ({} Events, £{:.2f})".format(len(objects), total)
+        context['page_title'] = f"Events for Invoice ({len(objects)} Events, £{total:.2f})"
         return context
 
     def get_queryset(self):

RIGS/views/hs.py

@@ -37,7 +37,7 @@ class EventRiskAssessmentCreate(generic.CreateView):
         epk = self.kwargs.get('pk')
         event = models.Event.objects.get(pk=epk)
         context['event'] = event
-        context['page_title'] = 'Create Risk Assessment for Event {}'.format(event.display_id)
+        context['page_title'] = f'Create Risk Assessment for Event {event.display_id}'
         return context
 
     def get_success_url(self):
@@ -62,7 +62,7 @@ class EventRiskAssessmentEdit(generic.UpdateView):
         ra = models.RiskAssessment.objects.get(pk=rpk)
         context['event'] = ra.event
         context['edit'] = True
-        context['page_title'] = 'Edit Risk Assessment for Event {}'.format(ra.event.display_id)
+        context['page_title'] = f'Edit Risk Assessment for Event {ra.event.display_id}'
         return context
 
 
@@ -72,7 +72,7 @@ class EventRiskAssessmentDetail(generic.DetailView):
 
     def get_context_data(self, **kwargs):
         context = super(EventRiskAssessmentDetail, self).get_context_data(**kwargs)
-        context['page_title'] = "Risk Assessment for Event <a href='{}'>{} {}</a>".format(self.object.event.get_absolute_url(), self.object.event.display_id, self.object.event.name)
+        context['page_title'] = f"Risk Assessment for Event <a href='{self.object.event.get_absolute_url()}'>{self.object.event.display_id} {self.object.event.name}</a>"
         return context
 
 
@@ -112,7 +112,7 @@ class EventChecklistDetail(generic.DetailView):
 
     def get_context_data(self, **kwargs):
         context = super(EventChecklistDetail, self).get_context_data(**kwargs)
-        context['page_title'] = "Event Checklist for Event <a href='{}'>{} {}</a>".format(self.object.event.get_absolute_url(), self.object.event.display_id, self.object.event.name)
+        context['page_title'] = f"Event Checklist for Event <a href='{self.object.event.get_absolute_url()}'>{self.object.event.display_id} {self.object.event.name}</a>"
         return context
 
 
@@ -134,7 +134,7 @@ class EventChecklistEdit(generic.UpdateView):
         ec = models.EventChecklist.objects.get(pk=pk)
         context['event'] = ec.event
         context['edit'] = True
-        context['page_title'] = 'Edit Event Checklist for Event {}'.format(ec.event.display_id)
+        context['page_title'] = f'Edit Event Checklist for Event {ec.event.display_id}'
         form = context['form']
         # Get some other objects to include in the form. Used when there are errors but also nice and quick.
         for field, model in form.related_models.items():
@@ -158,7 +158,7 @@ class EventChecklistCreate(generic.CreateView):
         ra = models.RiskAssessment.objects.filter(event=event).first()
 
         if ra is None:
-            messages.error(self.request, 'A Risk Assessment must exist prior to creating any Event Checklists for {}! Please create one now.'.format(event))
+            messages.error(self.request, f'A Risk Assessment must exist prior to creating any Event Checklists for {event}! Please create one now.')
             return HttpResponseRedirect(reverse_lazy('event_ra', kwargs={'pk': epk}))
 
         return super(EventChecklistCreate, self).get(self)
@@ -175,7 +175,7 @@ class EventChecklistCreate(generic.CreateView):
         epk = self.kwargs.get('pk')
         event = models.Event.objects.get(pk=epk)
         context['event'] = event
-        context['page_title'] = 'Create Event Checklist for Event {}'.format(event.display_id)
+        context['page_title'] = f'Create Event Checklist for Event {event.display_id}'
         return context
 
     def get_success_url(self):

RIGS/views/rigboard.py

@@ -188,11 +188,14 @@ class EventPrint(generic.View):
         user_str = f"by {request.user.name} " if request.user is not None else ""
         time = timezone.now().strftime('%d/%m/%Y %H:%I')
 
+        name = re.sub(r'[^a-zA-Z0-9 \n\.]', '', object.name)
+        filename = f"Event_{object.display_id}_{name}_{object.start_date}.pdf"
+
         context = {
             'object': object,
             'quote': True,
             'current_user': request.user,
-            'filename': 'Event_{}_{}_{}.pdf'.format(object.display_id, re.sub(r'[^a-zA-Z0-9 \n\.]', '', object.name), object.start_date),
+            'filename': filename,
             'info_string': f"[Paperwork generated {user_str}on {time} - {object.current_version_id}]",
         }
 
@@ -208,7 +211,7 @@ class EventPrint(generic.View):
         merger.write(merged)
 
         response = HttpResponse(content_type='application/pdf')
-        response['Content-Disposition'] = 'filename="{}"'.format(context['filename'])
+        response['Content-Disposition'] = f'filename="{filename}"'
         response.write(merged.getvalue())
         return response
 
@@ -244,32 +247,17 @@ class EventArchive(generic.ListView):
             filter &= Q(start_date__gte=start)
 
         q = self.request.GET.get('q', "")
+        objects = self.model.objects.all()
 
-        if q != "":
+        if q:
-            qfilter = Q(name__icontains=q) | Q(description__icontains=q) | Q(notes__icontains=q)
+            objects = self.model.objects.search(q)
-
-            # try and parse an int
-            try:
-                val = int(q)
-                qfilter = qfilter | Q(pk=val)
-            except:  # noqa not an integer
-                pass
-
-            try:
-                if q[0] == "N":
-                    val = int(q[1:])
-                    qfilter = Q(pk=val)  # If string is N###### then do a simple PK filter
-            except:  # noqa
-                pass
-
-            filter &= qfilter
 
         status = self.request.GET.getlist('status', "")
 
         if len(status) > 0:
             filter &= Q(status__in=status)
 
-        qs = self.model.objects.filter(filter).order_by('-start_date')
+        qs = objects.filter(filter).order_by('-start_date')
 
         # Preselect related for efficiency
         qs.select_related('person', 'organisation', 'venue', 'mic')
@@ -393,7 +381,7 @@ class EventAuthorisationRequest(generic.FormView, generic.detail.SingleObjectMix
             context['to_name'] = event.organisation.name
 
         msg = EmailMultiAlternatives(
-            "N%05d | %s - Event Authorisation Request" % (self.object.pk, self.object.name),
+            f"{self.object.display_id} | {self.object.name} - Event Authorisation Request",
             get_template("eventauthorisation_client_request.txt").render(context),
             to=[email],
             reply_to=[self.request.user.email],

assets/models.py

@@ -2,11 +2,12 @@ import re
 
 from django.core.exceptions import ValidationError
 from django.db import models, connection
+from django.db.models import Q
 from django.urls import reverse
 from reversion import revisions as reversion
 from reversion.models import Version
 
-from RIGS.models import Profile
+from RIGS.models import Profile, ContactableManager
 from versioning.versioning import RevisionMixin
 
 
@@ -46,6 +47,8 @@ class Supplier(models.Model, RevisionMixin):
 
     notes = models.TextField(blank=True, default="")
 
+    objects = ContactableManager()
+
     class Meta:
         ordering = ['name']
 
@@ -107,6 +110,15 @@ def get_available_asset_id(wanted_prefix=""):
         cursor.close()
 
 
+class AssetManager(models.Manager):
+    def search(self, query=None):
+        qs = self.get_queryset()
+        if query is not None:
+            or_lookup = (Q(asset_id__exact=query.upper()) | Q(description__icontains=query) | Q(serial_number__exact=query))
+            qs = qs.filter(or_lookup).distinct()  # distinct() is often necessary with Q lookups
+        return qs
+
+
 @reversion.register
 class Asset(models.Model, RevisionMixin):
     parent = models.ForeignKey(to='self', related_name='asset_parent',
@@ -142,6 +154,8 @@ class Asset(models.Model, RevisionMixin):
 
     reversion_perm = 'assets.asset_finance'
 
+    objects = AssetManager()
+
     class Meta:
         ordering = ['asset_id_prefix', 'asset_id_number']
         permissions = [

assets/templates/asset_list.html

@@ -75,13 +75,13 @@
                 <div class="col">
                     <div id="category-group" class="form-group px-1" style="margin-bottom: 0;">
                         <label for="category" class="sr-only">Category</label>
-                        {% render_field form.category|attr:'multiple'|add_class:'form-control custom-select selectpicker col-sm' data-none-selected-text="Categories" data-header="Categories" data-actions-box="true" %}
+                        {% render_field form.category|attr:'multiple'|add_class:'selectpicker col-sm' data-none-selected-text="Categories" data-header="Categories" data-actions-box="true" %}
                     </div>
                 </div>
                 <div class="col">
                     <div id="status-group" class="form-group px-1" style="margin-bottom: 0;">
                         <label for="status" class="sr-only">Status</label>
-                        {% render_field form.status|attr:'multiple'|add_class:'form-control custom-select selectpicker col-sm' data-none-selected-text="Statuses" data-header="Statuses" data-actions-box="true" %}
+                        {% render_field form.status|attr:'multiple'|add_class:'selectpicker col-sm' data-none-selected-text="Statuses" data-header="Statuses" data-actions-box="true" %}
                     </div>
                 </div>
                 <div class="col mt-2">

assets/urls.py

@@ -20,7 +20,7 @@ urlpatterns = [
     path('asset/id/<asset:pk>/duplicate/', permission_required_with_403('assets.add_asset')
          (views.AssetDuplicate.as_view()), name='asset_duplicate'),
     path('asset/id/<asset:pk>/label', login_required(views.GenerateLabel.as_view()), name='generate_label'),
-    path('asset/<list:ids>/list/label',  views.GenerateLabels.as_view(), name='generate_labels'),
+    path('asset/<list:ids>/list/label', views.GenerateLabels.as_view(), name='generate_labels'),
 
     path('cabletype/list/', login_required(views.CableTypeList.as_view()), name='cable_type_list'),
     path('cabletype/create/', permission_required_with_403('assets.add_cable_type')(views.CableTypeCreate.as_view()), name='cable_type_create'),

assets/views.py

@@ -50,13 +50,7 @@ class AssetList(LoginRequiredMixin, generic.ListView):
 
         # TODO Feedback to user when search fails
         query_string = form.cleaned_data['q'] or ""
-        if len(query_string) == 0:
+        queryset = models.Asset.objects.search(query=query_string)
-            queryset = self.model.objects.all()
-        elif len(query_string) >= 3:
-            queryset = self.model.objects.filter(
-                Q(asset_id__exact=query_string.upper()) | Q(description__icontains=query_string) | Q(serial_number__exact=query_string))
-        else:
-            queryset = self.model.objects.filter(Q(asset_id__exact=query_string.upper()))
 
         if form.cleaned_data['is_cable']:
             queryset = queryset.filter(is_cable=True)
@@ -176,6 +170,7 @@ class AssetOEmbed(OEmbedView):
 
 class AssetAuditList(AssetList):
     template_name = 'asset_audit_list.html'
+    hide_hidden_status = True
 
     # TODO Refresh this when the modal is submitted
     def get_queryset(self):
@@ -388,12 +383,14 @@ class GenerateLabels(generic.View):
             base64_encoded_result_str = base64_encoded_result_bytes.decode('ascii')
             images.append(base64_encoded_result_str)
 
+        name = f"Asset Label Sheet generated at {timezone.now()}"
+
         context = {
             'images0': images[::4],
             'images1': images[1::4],
             'images2': images[2::4],
             'images3': images[3::4],
-            'filename': "Asset Label Sheet generated at {}".format(timezone.now())
+            'filename': name
         }
         merger = PdfFileMerger()
 
@@ -405,6 +402,6 @@ class GenerateLabels(generic.View):
         merged = BytesIO()
         merger.write(merged)
 
-        response['Content-Disposition'] = 'filename="{}"'.format(context['filename'])
+        response['Content-Disposition'] = f'filename="{name}"'
         response.write(merged.getvalue())
         return response

templates/partials/search.html

@@ -1,38 +1,11 @@
 {% if user.is_authenticated %}
-<form id="searchForm" class="form-inline flex-nowrap mx-md-3 px-2 border border-light rounded w-75" role="form" method="GET" action="{% url 'event_archive' %}">
+<form id="searchForm" class="form-inline flex-nowrap mx-md-3 px-2 border border-light rounded" role="form" method="GET" action="{% url 'search' %}">
-    <div class="input-group input-group-sm flex-nowrap">
+    <div class="input-group">
-        <div class="input-group-prepend">
+        <input id="id_search_input" type="search" name="q" class="form-control form-control-sm" placeholder="Search..." value="{{ request.GET.q }}" />
-            <input id="id_search_input" type="search" name="q" class="form-control form-control-sm" placeholder="Search..." value="{{ request.GET.q }}" />
+        <div class="input-group-append">
+            <button class="btn btn-info form-control form-control-sm btn-sm"><span class="fas fa-search"></span><span class="sr-only"> Search</span></button>
         </div>
-        <select id="search-options" class="custom-select form-control" style="border-top-right-radius: 0px; border-bottom-right-radius: 0px; width: 15ch;">
-          <option selected data-action="{% url 'event_archive' %}" href="#">Events</option>
-          <option data-action="{% url 'person_list' %}" href="#">People</option>
-          <option data-action="{% url 'organisation_list' %}" href="#">Organisations</option>
-          <option data-action="{% url 'venue_list' %}" href="#">Venues</option>
-          {% if perms.RIGS.view_invoice %}
-          <option data-action="{% url 'invoice_archive' %}" href="#">Invoices</option>
-          {% endif %}
-          <option data-action="{% url 'asset_list' %}" href="#">Assets</option>
-          <option data-action="{% url 'supplier_list' %}" href="#">Suppliers</option>
-        </select>
     </div>
-    <button class="btn btn-info form-control form-control-sm btn-sm w-25" style="border-top-left-radius: 0px;border-bottom-left-radius: 0px;"><span class="fas fa-search"></span><span class="sr-only"> Search</span></button>
     <a href="{% url 'search_help' %}" class="nav-link modal-href ml-1"><span class="fas fa-question-circle"></span></a>
 </form>
 {% endif %}
-
-{% block js %}
-<script>
-    $('#search-options').change(function(){
-      $('#searchForm').attr('action', $(this).children('option:selected').data('action'));
-    });
-    $(document).ready(function(){
-        $('#id_search_input').keypress(function (e) {
-          if (e.which == 13) {
-            $('#searchForm').attr('action', $('#search-options option').first().data('action')).submit();
-            return false;
-          }
-        });
-    });
-</script>
-{% endblock %}

templates/search_results.html

@@ -0,0 +1,48 @@
+{% extends "base_rigs.html" %}
+
+{% load to_class_name from filters %}
+{% load markdown_tags %}
+
+{% block content %}
+{% include 'partials/search.html' %}
+{% for object in object_list %}
+    {% with object|to_class_name as klass %}
+    <div class="card m-2">
+       <h4 class="card-header"><a href='{{ object.get_absolute_url }}'>[{{ klass }}] {{ object }}</a>
+       <small>
+       {% if klass == "Event" %}
+        {% if object.venue %}
+        <strong>Venue:</strong> {{ object.venue }}
+        {% endif %}
+        {% if object.is_rig %}
+        <strong>Client:</strong> {{ object.person.name }}
+        {% if object.organisation %}
+        for {{ object.organisation.name }}
+        {% endif %}
+        {% if object.dry_hire %}(Dry Hire){% endif %}
+        {% else %}
+        <strong>Non-Rig</strong>
+        {% endif %}
+        <strong>Times:</strong>
+        {{ object.start_date|date:"D d/m/Y" }}
+        {% if object.has_start_time %}
+        {{ object.start_time|date:"H:i" }}
+        {% endif %}
+        {% if object.end_date or object.has_end_time %}
+        &ndash;
+        {% endif %}
+        {% if object.end_date and object.end_date != object.start_date %}
+        {{ object.end_date|date:"D d/m/Y"  }}
+        {% endif %}
+        {% if object.has_end_time %}
+        {{ object.end_time|date:"H:i" }}
+        {% endif %}
+       {% endif %}
+       </small>
+       </h4>
+    </div>
+    {% endwith %}
+{% empty %}
+<h3 class="py-3 text-warning">No results found</h3>
+{% endfor %}
+{% endblock content %}

training/models.py

@@ -1,6 +1,7 @@
-from RIGS.models import Profile
+from RIGS.models import Profile, filter_by_pk
 from reversion import revisions as reversion
 from django.db import models
+from django.db.models import Q
 from django.urls import reverse
 from django.utils.safestring import mark_safe
 from versioning.versioning import RevisionMixin
@@ -12,6 +13,16 @@ class TraineeManager(models.Manager):
     def get_queryset(self):
         return super().get_queryset().filter(is_active=True, is_approved=True)
 
+    def search(self, query=None):
+        qs = self.get_queryset()
+        if query is not None:
+            or_lookup = (Q(first_name__icontains=query) |
+                         Q(last_name__icontains=query) | Q(initials__icontains=query)
+                         )
+            or_lookup = filter_by_pk(or_lookup, query)
+            qs = qs.filter(or_lookup).distinct()  # distinct() is often necessary with Q lookups
+        return qs
+
 
 @reversion.register(for_concrete_model=False, fields=['is_supervisor'])
 class Trainee(Profile, RevisionMixin):
@@ -65,6 +76,16 @@ class TrainingCategory(models.Model):
         verbose_name_plural = 'Training Categories'
 
 
+class TrainingItemManager(QueryablePropertiesManager):
+    def search(self, query=None):
+        qs = self.get_queryset()
+        if query is not None:
+            or_lookup = (Q(description__icontains=query)
+                         )
+            qs = qs.filter(or_lookup).distinct()  # distinct() is often necessary with Q lookups
+        return qs
+
+
 @reversion.register
 class TrainingItem(models.Model):
     reference_number = models.IntegerField()
@@ -72,7 +93,7 @@ class TrainingItem(models.Model):
     description = models.CharField(max_length=50)
     active = models.BooleanField(default=True)
 
-    objects = QueryablePropertiesManager()
+    objects = TrainingItemManager()
 
     @property
     def name(self):
@@ -97,6 +118,9 @@ class TrainingItem(models.Model):
             name += " (inactive)"
         return name
 
+    def get_absolute_url(self):
+        return reverse('item_list')
+
     @staticmethod
     def user_has_qualification(item, user, depth):
         return user.qualifications_obtained.only('item', 'depth').filter(item=item, depth__gte=depth).exists()

training/views.py

@@ -95,23 +95,13 @@ class TraineeList(generic.ListView):
     paginate_by = 25
 
     def get_queryset(self):
-        q = self.request.GET.get('q', "")
+        objects = self.model.objects
-
-        filt = Q(first_name__icontains=q) | Q(last_name__icontains=q) | Q(initials__icontains=q)
-
-        # try and parse an int
-        try:
-            val = int(q)
-            filt = filt | Q(pk=val)
-        except:  # noqa
-            # not an integer
-            pass
 
         if self.request.GET.get('is_supervisor', ''):
-            filt = filt & Q(is_supervisor=True)
+            objects = objects.filter(is_supervisor=True)
 
-        return self.model.objects.filter(filt).annotate(num_qualifications=Count('qualifications_obtained', filter=Q(qualifications_obtained__depth=models.TrainingItemQualification.PASSED_OUT))
+        return objects.search(self.request.GET.get('q')).annotate(num_qualifications=Count('qualifications_obtained', filter=Q(qualifications_obtained__depth=models.TrainingItemQualification.PASSED_OUT))
-                                                        ).order_by('-num_qualifications').prefetch_related('level_qualifications', 'qualifications_obtained', 'qualifications_obtained__item')
+                                                                  ).order_by('-num_qualifications').prefetch_related('level_qualifications', 'qualifications_obtained', 'qualifications_obtained__item')
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)

users/views.py

@@ -41,7 +41,7 @@ class ProfileDetail(generic.DetailView):
 
     def get_context_data(self, **kwargs):
         context = super(ProfileDetail, self).get_context_data(**kwargs)
-        context['page_title'] = "Profile: {}".format(self.object)
+        context['page_title'] = f"Profile: {self.object}"
         context["completed_levels"] = self.object.level_qualifications.all().select_related('level')
         return context