Transform UserSettings into a normal django model

helpdesk/forms.py

@@ -19,7 +19,7 @@ from django.utils import timezone
 
 from helpdesk.lib import send_templated_mail, safe_template_context, process_attachments
 from helpdesk.models import (Ticket, Queue, FollowUp, Attachment, IgnoreEmail, TicketCC,
-                             CustomField, TicketCustomFieldValue, TicketDependency)
+                             CustomField, TicketCustomFieldValue, TicketDependency, UserSettings)
 from helpdesk import settings as helpdesk_settings
 
 User = get_user_model()
@@ -253,7 +253,7 @@ class AbstractTicketForm(CustomFieldMixin, forms.Form):
 
         if ticket.assigned_to and \
                 ticket.assigned_to != user and \
-                ticket.assigned_to.usersettings_helpdesk.settings.get('email_on_ticket_assign', False) and \
+                ticket.assigned_to.usersettings_helpdesk.email_on_ticket_assign and \
                 ticket.assigned_to.email and \
                 ticket.assigned_to.email not in messages_sent_to:
             send_templated_mail(
@@ -406,40 +406,11 @@ class PublicTicketForm(AbstractTicketForm):
         return ticket
 
 
-class UserSettingsForm(forms.Form):
+class UserSettingsForm(forms.ModelForm):
-    login_view_ticketlist = forms.BooleanField(
-        label=_('Show Ticket List on Login?'),
-        help_text=_('Display the ticket list upon login? Otherwise, the dashboard is shown.'),
-        required=False,
-    )
 
-    email_on_ticket_change = forms.BooleanField(
+    class Meta:
-        label=_('E-mail me on ticket change?'),
+        model = UserSettings
-        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?'),
+        exclude = ['user', 'settings_pickled']
-        required=False,
-    )
-
-    email_on_ticket_assign = forms.BooleanField(
-        label=_('E-mail me when assigned a ticket?'),
-        help_text=_('If you are assigned a ticket via the web, do you want to receive an e-mail?'),
-        required=False,
-    )
-
-    tickets_per_page = forms.ChoiceField(
-        label=_('Number of tickets to show per page'),
-        help_text=_('How many tickets do you want to see on the Ticket List page?'),
-        required=False,
-        choices=((10, '10'), (25, '25'), (50, '50'), (100, '100')),
-    )
-
-    use_email_as_submitter = forms.BooleanField(
-        label=_('Use my e-mail address when submitting tickets?'),
-        help_text=_('When you submit a ticket, do you want to automatically '
-                    'use your e-mail address as the submitter address? You '
-                    'can type a different e-mail address when entering the '
-                    'ticket if needed, this option only changes the default.'),
-        required=False,
-    )
 
 
 class EmailIgnoreForm(forms.ModelForm):

helpdesk/management/commands/create_usersettings.py

@@ -29,5 +29,4 @@ class Command(BaseCommand):
     def handle(self, *args, **options):
         """handle command line"""
         for u in User.objects.all():
-            UserSettings.objects.get_or_create(user=u,
+            UserSettings.objects.get_or_create(user=u)
-                                               defaults={'settings': DEFAULT_USER_SETTINGS})

helpdesk/migrations/0020_depickle_user_settings.py

@@ -0,0 +1,69 @@
+# Generated by Django 2.0.7 on 2018-10-19 14:11
+
+from django.db import migrations, models
+import helpdesk.models
+
+def unpickle_settings(settings_pickled):
+    # return a python dictionary representing the pickled data.
+    try:
+        import pickle
+    except ImportError:
+        import cPickle as pickle
+    from helpdesk.lib import b64decode
+    try:
+        if six.PY2:
+            return pickle.loads(b64decode(str(settings_pickled)))
+        else:
+            return pickle.loads(b64decode(settings_pickled.encode('utf-8')))
+    except Exception:
+        return {}
+
+def move_old_values(apps, schema_editor):
+    UserSettings = apps.get_model("helpdesk", "UserSettings")
+    db_alias = schema_editor.connection.alias
+
+    for user_settings in UserSettings.objects.using(db_alias).all():
+        if user_settings.settings_pickled:
+            settings_dict = unpickle_settings(user_settings.settings_pickled)
+            for setting, value in settings_dict.items():
+                user_settings.__set_attr__(setting, value)
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('helpdesk', '0019_ticket_secret_key'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='usersettings',
+            name='email_on_ticket_assign',
+            field=models.BooleanField(default=helpdesk.models.email_on_ticket_assign_default, help_text='If you are assigned a ticket via the web, do you want to receive an e-mail?', verbose_name='E-mail me when assigned a ticket?'),
+        ),
+        migrations.AddField(
+            model_name='usersettings',
+            name='email_on_ticket_change',
+            field=models.BooleanField(default=helpdesk.models.email_on_ticket_change_default, 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?", verbose_name='E-mail me on ticket change?'),
+        ),
+        migrations.AddField(
+            model_name='usersettings',
+            name='login_view_ticketlist',
+            field=models.BooleanField(default=helpdesk.models.login_view_ticketlist_default, help_text='Display the ticket list upon login? Otherwise, the dashboard is shown.', verbose_name='Show Ticket List on Login?'),
+        ),
+        migrations.AddField(
+            model_name='usersettings',
+            name='tickets_per_page',
+            field=models.IntegerField(choices=[(10, '10'), (25, '25'), (50, '50'), (100, '100')], default=helpdesk.models.tickets_per_page_default, help_text='How many tickets do you want to see on the Ticket List page?', verbose_name='Number of tickets to show per page'),
+        ),
+        migrations.AddField(
+            model_name='usersettings',
+            name='use_email_as_submitter',
+            field=models.BooleanField(default=helpdesk.models.use_email_as_submitter_default, help_text='When you submit a ticket, do you want to automatically use your e-mail address as the submitter address? You can type a different e-mail address when entering the ticket if needed, this option only changes the default.', verbose_name='Use my e-mail address when submitting tickets?'),
+        ),
+        migrations.AlterField(
+            model_name='usersettings',
+            name='settings_pickled',
+            field=models.TextField(blank=True, help_text='DEPRECATED! This is a base64-encoded representation of a pickled Python dictionary. Do not change this field via the admin.', null=True, verbose_name='DEPRECATED! Settings Dictionary DEPRECATED!'),
+        ),
+        migrations.RunPython(move_old_values),
+    ]

helpdesk/models.py

@@ -1119,15 +1119,39 @@ class SavedSearch(models.Model):
         verbose_name_plural = _('Saved searches')
 
 
+def get_default_setting(setting):
+    from helpdesk.settings import DEFAULT_USER_SETTINGS
+    return DEFAULT_USER_SETTINGS[setting]
+
+
+def login_view_ticketlist_default():
+    return get_default_setting('login_view_ticketlist')
+
+
+def email_on_ticket_change_default():
+    return get_default_setting('email_on_ticket_change')
+
+
+def email_on_ticket_assign_default():
+    return get_default_setting('email_on_ticket_assign')
+
+
+def tickets_per_page_default():
+    return get_default_setting('tickets_per_page')
+
+
+def use_email_as_submitter_default():
+    return get_default_setting('use_email_as_submitter')
+
+
 @python_2_unicode_compatible
 class UserSettings(models.Model):
     """
     A bunch of user-specific settings that we want to be able to define, such
     as notification preferences and other things that should probably be
     configurable.
-
-    We should always refer to user.usersettings_helpdesk.settings['setting_name'].
     """
+    PAGE_SIZES = ((10, '10'), (25, '25'), (50, '50'), (100, '100'))
 
     user = models.OneToOneField(
         settings.AUTH_USER_MODEL,
@@ -1135,41 +1159,46 @@ class UserSettings(models.Model):
         related_name="usersettings_helpdesk")
 
     settings_pickled = models.TextField(
-        _('Settings Dictionary'),
+        _('DEPRECATED! Settings Dictionary DEPRECATED!'),
-        help_text=_('This is a base64-encoded representation of a pickled Python dictionary. '
+        help_text=_('DEPRECATED! This is a base64-encoded representation of a pickled Python dictionary. '
                     'Do not change this field via the admin.'),
         blank=True,
         null=True,
     )
 
-    def _set_settings(self, data):
+    login_view_ticketlist = models.BooleanField(
-        # data should always be a Python dictionary.
+        verbose_name=_('Show Ticket List on Login?'),
-        try:
+        help_text=_('Display the ticket list upon login? Otherwise, the dashboard is shown.'),
-            import pickle
+        default=login_view_ticketlist_default,
-        except ImportError:
+    )
-            import cPickle as pickle
-        from helpdesk.lib import b64encode
-        if six.PY2:
-            self.settings_pickled = b64encode(pickle.dumps(data))
-        else:
-            self.settings_pickled = b64encode(pickle.dumps(data)).decode()
 
-    def _get_settings(self):
+    email_on_ticket_change = models.BooleanField(
-        # return a python dictionary representing the pickled data.
+        verbose_name=_('E-mail me on ticket change?'),
-        try:
+        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?'),
-            import pickle
+        default=email_on_ticket_change_default,
-        except ImportError:
+    )
-            import cPickle as pickle
-        from helpdesk.lib import b64decode
-        try:
-            if six.PY2:
-                return pickle.loads(b64decode(str(self.settings_pickled)))
-            else:
-                return pickle.loads(b64decode(self.settings_pickled.encode('utf-8')))
-        except pickle.UnpicklingError:
-            return {}
 
-    settings = property(_get_settings, _set_settings)
+    email_on_ticket_assign = models.BooleanField(
+        verbose_name=_('E-mail me when assigned a ticket?'),
+        help_text=_('If you are assigned a ticket via the web, do you want to receive an e-mail?'),
+        default=email_on_ticket_assign_default,
+    )
+
+    tickets_per_page = models.IntegerField(
+        verbose_name=_('Number of tickets to show per page'),
+        help_text=_('How many tickets do you want to see on the Ticket List page?'),
+        default=tickets_per_page_default,
+        choices=PAGE_SIZES,
+    )
+
+    use_email_as_submitter = models.BooleanField(
+        verbose_name=_('Use my e-mail address when submitting tickets?'),
+        help_text=_('When you submit a ticket, do you want to automatically '
+                    'use your e-mail address as the submitter address? You '
+                    'can type a different e-mail address when entering the '
+                    'ticket if needed, this option only changes the default.'),
+        default=use_email_as_submitter_default,
+    )
 
     def __str__(self):
         return 'Preferences for %s' % self.user
@@ -1188,9 +1217,8 @@ def create_usersettings(sender, instance, created, **kwargs):
     If we end up with users with no UserSettings, then we get horrible
     'DoesNotExist: UserSettings matching query does not exist.' errors.
     """
-    from helpdesk.settings import DEFAULT_USER_SETTINGS
     if created:
-        UserSettings.objects.create(user=instance, settings=DEFAULT_USER_SETTINGS)
+        UserSettings.objects.create(user=instance)
 
 
 models.signals.post_save.connect(create_usersettings, sender=settings.AUTH_USER_MODEL)

helpdesk/settings.py

@@ -6,20 +6,18 @@ import warnings
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 
+DEFAULT_USER_SETTINGS = {
+    'login_view_ticketlist': True,
+    'email_on_ticket_change': True,
+    'email_on_ticket_assign': True,
+    'tickets_per_page': 25,
+    'use_email_as_submitter': True,
+}
 
 try:
-    DEFAULT_USER_SETTINGS = settings.HELPDESK_DEFAULT_SETTINGS
+    DEFAULT_USER_SETTINGS.update(settings.HELPDESK_DEFAULT_SETTINGS)
 except AttributeError:
-    DEFAULT_USER_SETTINGS = None
+    pass
-
-if not isinstance(DEFAULT_USER_SETTINGS, dict):
-    DEFAULT_USER_SETTINGS = {
-        'use_email_as_submitter': True,
-        'email_on_ticket_assign': True,
-        'email_on_ticket_change': True,
-        'login_view_ticketlist': True,
-        'tickets_per_page': 25
-    }
 
 
 HAS_TAG_SUPPORT = False

helpdesk/templates/helpdesk/user_settings.html

@@ -7,11 +7,14 @@
 
 <p>{% blocktrans %}Use the following options to change the way your helpdesk system works for you. These settings do not impact any other user.{% endblocktrans %}</p>
 
-<form role="form" method='post' action='./'>
+{% block form_content %}
+<form role="form" method="post">
+    {% csrf_token %}
     {{ form|bootstrap }}
     <div class="form-group">
-        <input class="btn btn-primary" type='submit' value='{% trans "Save Options" %}' />
+      <input type="submit" value="Submit"/>
     </div>
-{% csrf_token %}</form>
+</form>
+{% endblock %}
 
 {% endblock %}

helpdesk/tests/helpers.py

@@ -43,24 +43,6 @@ def reload_urlconf(urlconf=None):
     clear_url_caches()
 
 
-def update_user_settings(user, **kwargs):
-    usersettings = user.usersettings_helpdesk
-    settings = usersettings.settings
-    settings.update(kwargs)
-    usersettings.settings = settings
-    usersettings.save()
-
-
-def delete_user_settings(user, *args):
-    usersettings = user.usersettings_helpdesk
-    settings = usersettings.settings
-    for setting in args:
-        if setting in settings:
-            del settings[setting]
-    usersettings.settings = settings
-    usersettings.save()
-
-
 def create_ticket(**kwargs):
     q = kwargs.get('queue', None)
     if q is None:

helpdesk/tests/test_navigation.py

@@ -6,7 +6,7 @@ from django.test import TestCase
 
 from helpdesk import settings as helpdesk_settings
 from helpdesk.models import Queue
-from helpdesk.tests.helpers import (get_staff_user, reload_urlconf, User, update_user_settings, delete_user_settings, create_ticket, print_response)
+from helpdesk.tests.helpers import (get_staff_user, reload_urlconf, User, create_ticket, print_response)
 
 
 class KBDisabledTestCase(TestCase):
@@ -228,11 +228,8 @@ class HomePageTestCase(TestCase):
         user = get_staff_user()
 
         # login_view_ticketlist is False...
-        update_user_settings(user, login_view_ticketlist=False)
+        user.usersettings_helpdesk.login_view_ticketlist = False
-        self.assertUserRedirectedToView(user, 'helpdesk:dashboard')
+        user.usersettings_helpdesk.save()
-
-        # ... or missing
-        delete_user_settings(user, 'login_view_ticketlist')
         self.assertUserRedirectedToView(user, 'helpdesk:dashboard')
 
     def test_no_user_settings_redirect_to_dashboard(self):
@@ -246,7 +243,8 @@ class HomePageTestCase(TestCase):
     def test_redirect_to_ticket_list(self):
         """Authenticated users are redirected to the ticket list based on their user settings"""
         user = get_staff_user()
-        update_user_settings(user, login_view_ticketlist=True)
+        user.usersettings_helpdesk.login_view_ticketlist = True
+        user.usersettings_helpdesk.save()
 
         self.assertUserRedirectedToView(user, 'helpdesk:list')
 

helpdesk/urls.py

@@ -127,7 +127,7 @@ urlpatterns = [
         name='delete_query'),
 
     url(r'^settings/$',
-        staff.user_settings,
+        staff.EditUserSettingsView.as_view(),
         name='user_settings'),
 
     url(r'^ignore/$',

helpdesk/views/public.py

@@ -49,7 +49,7 @@ class CreateTicketView(FormView):
                 (request.user.is_authenticated and
                  helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE):
             try:
-                if request.user.usersettings_helpdesk.settings.get('login_view_ticketlist', False):
+                if request.user.usersettings_helpdesk.login_view_ticketlist:
                     return HttpResponseRedirect(reverse('helpdesk:list'))
                 else:
                     return HttpResponseRedirect(reverse('helpdesk:dashboard'))

helpdesk/views/staff.py

@@ -15,7 +15,7 @@ from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.auth.decorators import user_passes_test
 from django.contrib.contenttypes.models import ContentType
-from django.urls import reverse
+from django.urls import reverse, reverse_lazy
 from django.core.exceptions import ValidationError, PermissionDenied
 from django.db import connection
 from django.db.models import Q
@@ -26,7 +26,7 @@ from django.utils.translation import ugettext as _
 from django.utils.html import escape
 from django import forms
 from django.utils import timezone
-from django.views.generic.edit import FormView
+from django.views.generic.edit import FormView, UpdateView
 
 from django.utils import six
 
@@ -44,7 +44,7 @@ from helpdesk.lib import (
 )
 from helpdesk.models import (
     Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch,
-    IgnoreEmail, TicketCC, TicketDependency,
+    IgnoreEmail, TicketCC, TicketDependency, UserSettings,
 )
 from helpdesk import settings as helpdesk_settings
 from helpdesk.views.permissions import MustBeStaffMixin
@@ -635,11 +635,9 @@ def update_ticket(request, ticket_id, public=False):
 
         if (not reassigned or
                 (reassigned and
-                    ticket.assigned_to.usersettings_helpdesk.settings.get(
+                    ticket.assigned_to.usersettings_helpdesk.email_on_ticket_assign)) or \
-                        'email_on_ticket_assign', False))) or \
             (not reassigned and
-                ticket.assigned_to.usersettings_helpdesk.settings.get(
+                ticket.assigned_to.usersettings_helpdesk.email_on_ticket_change):
-                    'email_on_ticket_change', False)):
 
             send_templated_mail(
                 template_staff,
@@ -983,7 +981,7 @@ def ticket_list(request):
     return render(request, 'helpdesk/ticket_list.html', dict(
         context,
         tickets=ticket_qs,
-        default_tickets_per_page=request.user.usersettings_helpdesk.settings.get('tickets_per_page') or 25,
+        default_tickets_per_page=request.user.usersettings_helpdesk.tickets_per_page,
         user_choices=User.objects.filter(is_active=True, is_staff=True),
         queue_choices=user_queues,
         status_choices=Ticket.STATUS_CHOICES,
@@ -1028,7 +1026,7 @@ class CreateTicketView(MustBeStaffMixin, FormView):
     def get_initial(self):
         initial_data = {}
         request = self.request
-        if request.user.usersettings_helpdesk.settings.get('use_email_as_submitter', False) and request.user.email:
+        if request.user.usersettings_helpdesk.use_email_as_submitter and request.user.email:
             initial_data['submitter_email'] = request.user.email
         if 'queue' in request.GET:
             initial_data['queue'] = request.GET['queue']
@@ -1395,21 +1393,14 @@ def delete_saved_query(request, id):
 delete_saved_query = staff_member_required(delete_saved_query)
 
 
-@helpdesk_staff_member_required
+class EditUserSettingsView(MustBeStaffMixin, UpdateView):
-def user_settings(request):
+    template_name = 'helpdesk/user_settings.html'
-    s = request.user.usersettings_helpdesk
+    form_class = UserSettingsForm
-    if request.POST:
+    model = UserSettings
-        form = UserSettingsForm(request.POST)
+    success_url = reverse_lazy('helpdesk:dashboard')
-        if form.is_valid():
-            s.settings = form.cleaned_data
-            s.save()
-    else:
-        form = UserSettingsForm(s.settings)
 
-    return render(request, 'helpdesk/user_settings.html', {'form': form})
+    def get_object(self):
-
+        return UserSettings.objects.get_or_create(user=self.request.user)[0]
-
-user_settings = staff_member_required(user_settings)
 
 
 @helpdesk_superuser_required