Merge pull request #731 from OpenGeoLabs/images_for_kb

adding support for images as knowledgebase attachment

helpdesk/admin.py

@@ -2,7 +2,7 @@ from django.contrib import admin
 from django.utils.translation import ugettext_lazy as _
 from helpdesk.models import Queue, Ticket, FollowUp, PreSetReply, KBCategory
 from helpdesk.models import EscalationExclusion, EmailTemplate, KBItem
-from helpdesk.models import TicketChange, Attachment, IgnoreEmail
+from helpdesk.models import TicketChange, KBIAttachment, FollowUpAttachment, IgnoreEmail
 from helpdesk.models import CustomField
 
 
@@ -43,15 +43,22 @@ class TicketAdmin(admin.ModelAdmin):
 
 class TicketChangeInline(admin.StackedInline):
     model = TicketChange
+    extra = 0
 
 
-class AttachmentInline(admin.StackedInline):
-    model = Attachment
+class FollowUpAttachmentInline(admin.StackedInline):
+    model = FollowUpAttachment
+    extra = 0
+
+
+class KBIAttachmentInline(admin.StackedInline):
+    model = KBIAttachment
+    extra = 0
 
 
 @admin.register(FollowUp)
 class FollowUpAdmin(admin.ModelAdmin):
-    inlines = [TicketChangeInline, AttachmentInline]
+    inlines = [TicketChangeInline, FollowUpAttachmentInline]
     list_display = ('ticket_get_ticket_for_url', 'title', 'date', 'ticket',
                     'user', 'new_status', 'time_spent')
     list_filter = ('user', 'date', 'new_status')
@@ -64,6 +71,7 @@ class FollowUpAdmin(admin.ModelAdmin):
 @admin.register(KBItem)
 class KBItemAdmin(admin.ModelAdmin):
     list_display = ('category', 'title', 'last_updated',)
+    inlines = [KBIAttachmentInline]
     readonly_fields = ('voted_by',)
 
     list_display_links = ('title',)

helpdesk/forms.py

@@ -17,7 +17,7 @@ from django.contrib.auth import get_user_model
 from django.utils import timezone
 
 from helpdesk.lib import safe_template_context, process_attachments
-from helpdesk.models import (Ticket, Queue, FollowUp, Attachment, IgnoreEmail, TicketCC,
+from helpdesk.models import (Ticket, Queue, FollowUp, IgnoreEmail, TicketCC,
                              CustomField, TicketCustomFieldValue, TicketDependency, UserSettings)
 from helpdesk import settings as helpdesk_settings
 

helpdesk/lib.py

@@ -15,7 +15,7 @@ from django.db.models import Q
 from django.utils.encoding import smart_text, smart_str
 from django.utils.safestring import mark_safe
 
-from helpdesk.models import Attachment, EmailTemplate
+from helpdesk.models import FollowUpAttachment, EmailTemplate
 
 from model_utils import Choices
 
@@ -218,7 +218,7 @@ def process_attachments(followup, attached_files):
 
         if attached.size:
             filename = smart_text(attached.name)
-            att = Attachment(
+            att = FollowUpAttachment(
                 followup=followup,
                 file=attached,
                 filename=filename,

helpdesk/migrations/0026_kbitem_attachments.py

@@ -0,0 +1,36 @@
+# Generated by Django 2.0.5 on 2019-03-07 20:30
+
+from django.db import migrations, models
+import django.db.models.deletion
+import helpdesk.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('helpdesk', '0025_queue_dedicated_time'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='KBIAttachment',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('file', models.FileField(max_length=1000, upload_to=helpdesk.models.attachment_path, verbose_name='File')),
+                ('filename', models.CharField(max_length=1000, verbose_name='Filename')),
+                ('mime_type', models.CharField(max_length=255, verbose_name='MIME Type')),
+                ('size', models.IntegerField(help_text='Size of this file in bytes', verbose_name='Size')),
+                ('kbitem', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='helpdesk.KBItem', verbose_name='Knowledge base item')),
+            ],
+            options={
+                'verbose_name': 'Attachment',
+                'verbose_name_plural': 'Attachments',
+                'ordering': ('filename',),
+                'abstract': False,
+            },
+        ),
+        migrations.RenameModel(
+            old_name='Attachment',
+            new_name='FollowUpAttachment',
+        ),
+    ]

helpdesk/models.py

@@ -17,6 +17,8 @@ from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _, ugettext
 from io import StringIO
 import re
+import os
+import mimetypes
 import datetime
 
 from django.utils.safestring import mark_safe
@@ -904,18 +906,8 @@ class TicketChange(models.Model):
 
 
 def attachment_path(instance, filename):
-    """
-    Provide a file path that will help prevent files being overwritten, by
-    putting attachments in a folder off attachments for ticket/followup_id/.
-    """
-    import os
-    os.umask(0)
-    path = 'helpdesk/attachments/%s-%s/%s' % (instance.followup.ticket.ticket_for_url, instance.followup.ticket.secret_key, instance.followup.id)
-    att_path = os.path.join(settings.MEDIA_ROOT, path)
-    if settings.DEFAULT_FILE_STORAGE == "django.core.files.storage.FileSystemStorage":
-        if not os.path.exists(att_path):
-            os.makedirs(att_path, 0o777)
-    return os.path.join(path, filename)
+    """Just bridge"""
+    return instance.attachment_path(filename)
 
 
 class Attachment(models.Model):
@@ -924,12 +916,6 @@ class Attachment(models.Model):
     attachment, or it could be uploaded via the web interface.
     """
 
-    followup = models.ForeignKey(
-        FollowUp,
-        on_delete=models.CASCADE,
-        verbose_name=_('Follow-up'),
-    )
-
     file = models.FileField(
         _('File'),
         upload_to=attachment_path,
@@ -938,26 +924,102 @@ class Attachment(models.Model):
 
     filename = models.CharField(
         _('Filename'),
+        blank=True,
         max_length=1000,
     )
 
     mime_type = models.CharField(
         _('MIME Type'),
+        blank=True,
         max_length=255,
     )
 
     size = models.IntegerField(
         _('Size'),
+        blank=True,
         help_text=_('Size of this file in bytes'),
     )
 
     def __str__(self):
         return '%s' % self.filename
 
+    def save(self, *args, **kwargs):
+
+        if not self.size:
+            self.size = self.get_size()
+
+        if not self.filename:
+            self.filename = self.get_filename()
+
+        if not self.mime_type:
+            self.mime_type = \
+                mimetypes.guess_type(self.filename, strict=False)[0] or \
+                'application/octet-stream',
+
+        return super(Attachment, self).save(*args, **kwargs)
+
+    def get_filename(self):
+        return str(self.file)
+
+    def get_size(self):
+        return self.file.file.size
+
+    def attachment_path(self, filename):
+        """Provide a file path that will help prevent files being overwritten, by
+        putting attachments in a folder off attachments for ticket/followup_id/.
+        """
+        assert NotImplementedError(
+            "This method is to be implemented by Attachment classes"
+        )
+
     class Meta:
         ordering = ('filename',)
         verbose_name = _('Attachment')
         verbose_name_plural = _('Attachments')
+        abstract = True
+
+
+class FollowUpAttachment(Attachment):
+
+    followup = models.ForeignKey(
+        FollowUp,
+        on_delete=models.CASCADE,
+        verbose_name=_('Follow-up'),
+    )
+
+    def attachment_path(self, filename):
+
+        os.umask(0)
+        path = 'helpdesk/attachments/{ticket_for_url}-{secret_key}/{id_}'.format(
+            ticket_for_url=self.followup.ticket.ticket_for_url,
+            secret_key=self.followup.ticket.secret_key,
+            id_=self.followup.id)
+        att_path = os.path.join(settings.MEDIA_ROOT, path)
+        if settings.DEFAULT_FILE_STORAGE == "django.core.files.storage.FileSystemStorage":
+            if not os.path.exists(att_path):
+                os.makedirs(att_path, 0o777)
+        return os.path.join(path, filename)
+
+
+class KBIAttachment(Attachment):
+
+    kbitem = models.ForeignKey(
+        "KBItem",
+        on_delete=models.CASCADE,
+        verbose_name=_('Knowledge base item'),
+    )
+
+    def attachment_path(self, filename):
+
+        os.umask(0)
+        path = 'helpdesk/attachments/kb/{category}/{kbi}'.format(
+            category=self.kbitem.category,
+            kbi=self.kbitem.id)
+        att_path = os.path.join(settings.MEDIA_ROOT, path)
+        if settings.DEFAULT_FILE_STORAGE == "django.core.files.storage.FileSystemStorage":
+            if not os.path.exists(att_path):
+                os.makedirs(att_path, 0o777)
+        return os.path.join(path, filename)
 
 
 class PreSetReply(models.Model):

helpdesk/templates/helpdesk/public_view_ticket.html

@@ -66,7 +66,7 @@
 <li>{% blocktrans with change.field as field and change.old_value as old_value and change.new_value as new_value %}Changed {{ field }} from {{ old_value }} to {{ new_value }}.{% endblocktrans %}</li>
 {% endfor %}
 </ul></div>{% endif %}
-{% for attachment in followup.attachment_set.all %}{% if forloop.first %}<div class='attachments'><ul>{% endif %}
+{% for attachment in followup.followupattachment_set.all %}{% if forloop.first %}<div class='attachments'><ul>{% endif %}
 <li><a href='{{ attachment.file.url }}'>{{ attachment.filename }}</a> ({{ attachment.mime_type }}, {{ attachment.size|filesizeformat }})</li>
 {% if forloop.last %}</ul></div>{% endif %}
 {% endfor %}

helpdesk/tests/test_attachments.py

@@ -58,7 +58,7 @@ class AttachmentIntegrationTests(TestCase):
         self.assertContains(response, test_file.name)
 
         # Ensure attachment is available with correct content
-        att = models.Attachment.objects.get(followup__ticket=response.context['ticket'])
+        att = models.FollowUpAttachment.objects.get(followup__ticket=response.context['ticket'])
         with open(os.path.join(MEDIA_DIR, att.file.name)) as file_on_disk:
             disk_content = file_on_disk.read()
         self.assertEqual(disk_content, 'attached file content')
@@ -76,7 +76,7 @@ class AttachmentIntegrationTests(TestCase):
         self.assertContains(response, test_file.name)
 
         # Ensure attachment is available with correct content
-        att = models.Attachment.objects.get(followup__ticket=response.context['ticket'])
+        att = models.FollowUpAttachment.objects.get(followup__ticket=response.context['ticket'])
         with open(os.path.join(MEDIA_DIR, att.file.name)) as file_on_disk:
             disk_content = smart_text(file_on_disk.read(), 'utf-8')
         self.assertEqual(disk_content, 'โจ')
@@ -96,7 +96,7 @@ class AttachmentUnitTests(TestCase):
         self.test_file = SimpleUploadedFile.from_dict(self.file_attrs)
         self.follow_up = models.FollowUp(ticket=models.Ticket(queue=models.Queue()))
 
-    @mock.patch('helpdesk.lib.Attachment', autospec=True)
+    @mock.patch('helpdesk.lib.FollowUpAttachment', autospec=True)
     def test_unicode_attachment_filename(self, mock_att_save, mock_queue_save, mock_ticket_save, mock_follow_up_save):
         """ check utf-8 data is parsed correcltly """
         filename, fileobj = lib.process_attachments(self.follow_up, [self.test_file])[0]
@@ -109,7 +109,22 @@ class AttachmentUnitTests(TestCase):
         )
         self.assertEqual(filename, self.file_attrs['filename'])
 
-    @mock.patch.object(lib.Attachment, 'save', autospec=True)
+    @mock.patch('helpdesk.lib.FollowUpAttachment', autospec=True)
+    def xest_autofill_(self, mock_att_save, mock_queue_save, mock_ticket_save, mock_follow_up_save):
+        """ check utf-8 data is parsed correcltly """
+        filename, fileobj = lib.process_attachments(self.follow_up, [self.test_file])[0]
+        obj = mock_att_save.assert_called_with(
+            file=self.test_file,
+            filename=None,
+            mime_type=None,
+            size=None,
+            followup=self.follow_up
+        )
+        self.assertEqual(obj.filename, self.file_attrs['filename'])
+        self.assertEqual(obj.size, len(self.file_attrs['content']))
+        self.assertEqual(obj.mime_type, self.file_attrs['content-type'])
+
+    @mock.patch.object(lib.FollowUpAttachment, 'save', autospec=True)
     @override_settings(MEDIA_ROOT=MEDIA_DIR)
     def test_unicode_filename_to_filesystem(self, mock_att_save, mock_queue_save, mock_ticket_save, mock_follow_up_save):
         """ don't mock saving to filesystem to test file renames caused by storage layer """
@@ -118,7 +133,7 @@ class AttachmentUnitTests(TestCase):
         attachment_obj = mock_att_save.call_args[0][0]
 
         mock_att_save.assert_called_once_with(attachment_obj)
-        self.assertIsInstance(attachment_obj, models.Attachment)
+        self.assertIsInstance(attachment_obj, models.FollowUpAttachment)
         self.assertEqual(attachment_obj.filename, self.file_attrs['filename'])
 
 

helpdesk/tests/test_get_email.py

@@ -5,7 +5,7 @@ from django.shortcuts import get_object_or_404
 from django.contrib.auth.models import User
 from django.contrib.auth.hashers import make_password
 
-from helpdesk.models import Queue, Ticket, TicketCC, FollowUp, Attachment
+from helpdesk.models import Queue, Ticket, TicketCC, FollowUp, FollowUpAttachment
 import helpdesk.email
 
 import itertools
@@ -53,7 +53,7 @@ class GetEmailCommonTests(TestCase):
         with open(os.path.join(THIS_DIR, "test_files/blank-body-with-attachment.eml")) as fd:
             test_email = fd.read()
         ticket = helpdesk.email.object_from_message(test_email, self.queue_public, self.logger)
-        self.assertEqual(ticket.title, "Attachment without body")
+        self.assertEqual(ticket.title, "FollowUpAttachment without body")
         self.assertEqual(ticket.description, "")
 
     def test_email_with_blank_body_and_attachment(self):
@@ -68,7 +68,7 @@ class GetEmailCommonTests(TestCase):
         followups = FollowUp.objects.filter(ticket=ticket)
         self.assertEqual(len(followups), 1)
         followup = followups[0]
-        attachments = Attachment.objects.filter(followup=followup)
+        attachments = FollowUpAttachment.objects.filter(followup=followup)
         self.assertEqual(len(attachments), 1)
         attachment = attachments[0]
         self.assertEqual(attachment.file.read().decode("utf-8"), '<div dir="ltr">Tohle je test českých písmen odeslaných z gmailu.</div>\n')
@@ -363,7 +363,7 @@ class GetEmailParametricTemplate(object):
             # HTML MIME part should be attached to follow up
             followup1 = get_object_or_404(FollowUp, pk=1)
             self.assertEqual(followup1.ticket.id, 1)
-            attach1 = get_object_or_404(Attachment, pk=1)
+            attach1 = get_object_or_404(FollowUpAttachment, pk=1)
             self.assertEqual(attach1.followup.id, 1)
             self.assertEqual(attach1.filename, 'email_html_body.html')
             cc0 = get_object_or_404(TicketCC, pk=1)
@@ -382,7 +382,7 @@ class GetEmailParametricTemplate(object):
             # HTML MIME part should be attached to follow up
             followup2 = get_object_or_404(FollowUp, pk=2)
             self.assertEqual(followup2.ticket.id, 2)
-            attach2 = get_object_or_404(Attachment, pk=2)
+            attach2 = get_object_or_404(FollowUpAttachment, pk=2)
             self.assertEqual(attach2.followup.id, 2)
             self.assertEqual(attach2.filename, 'email_html_body.html')
 
@@ -449,7 +449,7 @@ class GetEmailParametricTemplate(object):
             # MIME part should be attached to follow up
             followup1 = get_object_or_404(FollowUp, pk=1)
             self.assertEqual(followup1.ticket.id, 1)
-            attach1 = get_object_or_404(Attachment, pk=1)
+            attach1 = get_object_or_404(FollowUpAttachment, pk=1)
             self.assertEqual(attach1.followup.id, 1)
             self.assertEqual(attach1.filename, 'signature.asc')
             self.assertEqual(attach1.file.read(), b"""-----BEGIN PGP SIGNATURE-----

helpdesk/views/staff.py

@@ -42,7 +42,7 @@ from helpdesk.lib import (
     process_attachments, queue_template_context,
 )
 from helpdesk.models import (
-    Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch,
+    Ticket, Queue, FollowUp, TicketChange, PreSetReply, FollowUpAttachment, SavedSearch,
     IgnoreEmail, TicketCC, TicketDependency, UserSettings,
 )
 from helpdesk import settings as helpdesk_settings
@@ -269,7 +269,7 @@ def followup_edit(request, ticket_id, followup_id):
                 new_followup.user = followup.user
             new_followup.save()
             # get list of old attachments & link them to new_followup
-            attachments = Attachment.objects.filter(followup=followup)
+            attachments = FolllowUpAttachment.objects.filter(followup=followup)
             for attachment in attachments:
                 attachment.followup = new_followup
                 attachment.save()
@@ -1581,7 +1581,7 @@ def attachment_del(request, ticket_id, attachment_id):
     if not _is_my_ticket(request.user, ticket):
         raise PermissionDenied()
 
-    attachment = get_object_or_404(Attachment, id=attachment_id)
+    attachment = get_object_or_404(FolllowUpAttachment, id=attachment_id)
     if request.method == 'POST':
         attachment.delete()
         return HttpResponseRedirect(reverse('helpdesk:view', args=[ticket_id]))