snapshots: Refactoring and indentation changes

- Made updating snapshot configuration more efficient by running a single
  snapper command instead of one per configuration.
- Set default configuration for NUMBER_LIMIT only once at the time of
  installation. Snapshot version has been incremented to support this.

Signed-off-by: Joseph Nuthalapati <njoseph@thoughtworks.com>

actions/snapshot

@@ -79,7 +79,8 @@ def _set_default_config():
         'snapper', 'set-config', 'TIMELINE_CREATE=yes',
         'TIMELINE_LIMIT_HOURLY=10', 'TIMELINE_LIMIT_MONTHLY=2',
         'TIMELINE_LIMIT_WEEKLY=2', 'TIMELINE_LIMIT_YEARLY=0',
-        'TIMELINE_LIMIT_DAILY=3', 'NUMBER_MIN_AGE=1296000'
+        'TIMELINE_LIMIT_DAILY=3', 'NUMBER_MIN_AGE=1296000', 'NUMBER_LIMIT=0',
+        'NUMBER_LIMIT_IMPORTANT=4-10'
     ]
     subprocess.run(command, check=True)
 

plinth/modules/snapshot/__init__.py

@@ -25,7 +25,7 @@ from django.utils.translation import ugettext_lazy as _
 from plinth import actions
 from plinth.menu import main_menu
 
-version = 2
+version = 3
 
 managed_packages = ['snapper']
 
@@ -61,12 +61,23 @@ def setup(helper, old_version=None):
 def get_configuration():
     output = actions.superuser_run('snapshot', ['get-config'])
     output = json.loads(output)
+
+    def get_boolean_choice(status):
+        return ('yes', 'Enabled') if status else ('no', 'Disabled')
+
     return {
-        'enable_timeline_snapshots': output['TIMELINE_CREATE'] == 'yes',
+        'enable_timeline_snapshots':
-        'hourly_limit': output['TIMELINE_LIMIT_HOURLY'],
+            get_boolean_choice(output['TIMELINE_CREATE'] == 'yes'),
-        'daily_limit': output['TIMELINE_LIMIT_DAILY'],
+        'hourly_limit':
-        'weekly_limit': output['TIMELINE_LIMIT_WEEKLY'],
+            output['TIMELINE_LIMIT_HOURLY'],
-        'yearly_limit': output['TIMELINE_LIMIT_YEARLY'],
+        'daily_limit':
-        'monthly_limit': output['TIMELINE_LIMIT_MONTHLY'],
+            output['TIMELINE_LIMIT_DAILY'],
-        'number_min_age': round(int(output['NUMBER_MIN_AGE']) / 86400),
+        'weekly_limit':
+            output['TIMELINE_LIMIT_WEEKLY'],
+        'yearly_limit':
+            output['TIMELINE_LIMIT_YEARLY'],
+        'monthly_limit':
+            output['TIMELINE_LIMIT_MONTHLY'],
+        'number_min_age':
+            round(int(output['NUMBER_MIN_AGE']) / 86400),
     }

plinth/modules/snapshot/forms.py

@@ -23,26 +23,27 @@ from django.utils.translation import ugettext_lazy as _
 
 
 class SnapshotForm(forms.Form):
-    enable_timeline_snapshots = forms.BooleanField(
+    enable_timeline_snapshots = forms.ChoiceField(
-        label=_('Enable Timeline Snapshots'), required=False, help_text=_(
+        label=_('Timeline Snapshots'),
-            'Uncheck this to disable timeline snapshots '
+        help_text=_('Enable or disable timeline snapshots '
-            '(hourly, daily, monthly and yearly).'))
+                    '(hourly, daily, monthly and yearly).'),
+        choices=[('yes', 'Enabled'), ('no', 'Disabled')])
 
     hourly_limit = forms.IntegerField(
-        label=_('Hourly Snapshots Limit'), min_value=0, help_text=_(
+        label=_('Hourly Snapshots Limit'), min_value=0,
-            'Keep a maximum of this many hourly snapshots.'))
+        help_text=_('Keep a maximum of this many hourly snapshots.'))
 
     daily_limit = forms.IntegerField(
-        label=_('Daily Snapshots Limit'), min_value=0, help_text=_(
+        label=_('Daily Snapshots Limit'), min_value=0,
-            'Keep a maximum of this many daily snapshots.'))
+        help_text=_('Keep a maximum of this many daily snapshots.'))
 
     weekly_limit = forms.IntegerField(
-        label=_('Weekly Snapshots Limit'), min_value=0, help_text=_(
+        label=_('Weekly Snapshots Limit'), min_value=0,
-            'Keep a maximum of this many weekly snapshots.'))
+        help_text=_('Keep a maximum of this many weekly snapshots.'))
 
     monthly_limit = forms.IntegerField(
-        label=_('Monthly Snapshots Limit'), min_value=0, help_text=_(
+        label=_('Monthly Snapshots Limit'), min_value=0,
-            'Keep a maximum of this many monthly snapshots.'))
+        help_text=_('Keep a maximum of this many monthly snapshots.'))
 
     yearly_limit = forms.IntegerField(
         label=_('Yearly Snapshots Limit'), min_value=0, help_text=_(

plinth/modules/snapshot/templates/snapshot.html

@@ -22,13 +22,13 @@
 {% load i18n %}
 
 {% block configuration %}
-  <p>
+
-   <form class="form-horizontal" method="post">
+  <form class="form-horizontal" method="post">
-      {% csrf_token %}
+    {% csrf_token %}
-      <h3>{% trans "Configuration" %}</h3>
+    <h3>{% trans "Configuration" %}</h3>
     {{ form|bootstrap_horizontal:'col-sm-4' }}
-      <input type="submit" class="btn btn-primary" name="update"
+    <input type="submit" class="btn btn-primary" name="update"
-             value="{% trans "Update setup" %}"/>
+           value="{% trans "Update setup" %}"/>
   </form>
 
 {% endblock %}

plinth/modules/snapshot/templates/snapshot_manage.html

@@ -26,19 +26,19 @@
     <div class="row">
       <form class="form" method="post">
         {% csrf_token %}
-          <div class="col-xs-6 text-left">
+        <div class="col-xs-6 text-left">
-            <input type="submit" class="btn btn-primary" name="create"
+          <input type="submit" class="btn btn-primary" name="create"
-                   value="{% trans 'Create Snapshot' %}"/>
+                 value="{% trans 'Create Snapshot' %}"/>
-          </div>
+        </div>
       </form>
       <div class="col-xs-6 text-right">
         <a title="{% trans 'Delete all the snapshots' %}"
            role="button" class="btn btn-danger"
-          {% if not has_deletable_snapshots %}
+           {% if not has_deletable_snapshots %}
-            disabled="disabled"
+           disabled="disabled"
-          {% else %}
+           {% else %}
-            href="{% url 'snapshot:delete-all' %}"
+           href="{% url 'snapshot:delete-all' %}"
-          {% endif %}>
+           {% endif %}>
           {% trans 'Delete All' %}
         </a>
       </div>

plinth/modules/snapshot/views.py

@@ -19,25 +19,29 @@ Views for snapshot module.
 """
 
 import json
+import subprocess
 
 from django.contrib import messages
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse, reverse_lazy
+from django.utils.translation import ugettext as _
+from django.utils.translation import ugettext_lazy
 
 from plinth import actions
 from plinth.errors import ActionError
 from plinth.modules import snapshot as snapshot_module
-from plinth.utils import yes_or_no
 
-from django.utils.translation import ugettext as _, ugettext_lazy
 from . import get_configuration
 from .forms import SnapshotForm
 
-subsubmenu = [{'url': reverse_lazy('snapshot:index'),
+subsubmenu = [{
-               'text': ugettext_lazy('Configure')},
+    'url': reverse_lazy('snapshot:index'),
-              {'url': reverse_lazy('snapshot:manage'),
+    'text': ugettext_lazy('Configure')
-               'text': ugettext_lazy('Manage Snapshots')}]
+}, {
+    'url': reverse_lazy('snapshot:manage'),
+    'text': ugettext_lazy('Manage Snapshots')
+}]
 
 
 def index(request):
@@ -84,31 +88,31 @@ def manage(request):
 def update_configuration(request, old_status, new_status):
     """Update configuration of snapshots."""
 
-    def update_key_configuration(key, stamp, threshold):
+    def make_config(args):
+        key, stamp = args[0], args[1]
         if old_status[key] != new_status[key]:
-            actions.superuser_run(
+            return stamp.format(new_status[key])
-                'snapshot', ['configure', stamp.format(threshold)])
+        else:
+            return None
+
+    new_status['number_min_age'] = int(new_status['number_min_age']) * 86400
+
+    config = filter(None,
+                    map(make_config, [
+                        ('enable_timeline_snapshots', 'TIMELINE_CREATE={}'),
+                        ('hourly_limit', 'TIMELINE_LIMIT_HOURLY={}'),
+                        ('daily_limit', 'TIMELINE_LIMIT_DAILY={}'),
+                        ('weekly_limit', 'TIMELINE_LIMIT_WEEKLY={}'),
+                        ('monthly_limit', 'TIMELINE_LIMIT_MONTHLY={}'),
+                        ('yearly_limit', 'TIMELINE_LIMIT_YEARLY={}'),
+                        ('number_min_age', 'NUMBER_MIN_AGE={}'),
+                    ]))
+
+    command = ['snapper', 'set-config'] + list(config)
 
     try:
-        update_key_configuration(
+        subprocess.run(command, check=True)
-            'enable_timeline_snapshots', 'TIMELINE_CREATE={}',
-            yes_or_no(new_status['enable_timeline_snapshots']))
-        update_key_configuration('hourly_limit', 'TIMELINE_LIMIT_HOURLY={}',
-                                 new_status['hourly_limit'])
-        update_key_configuration('daily_limit', 'TIMELINE_LIMIT_DAILY={}',
-                                 new_status['daily_limit'])
-        update_key_configuration('weekly_limit', 'TIMELINE_LIMIT_WEEKLY={}',
-                                 new_status['weekly_limit'])
-        update_key_configuration('monthly_limit', 'TIMELINE_LIMIT_MONTHLY={}',
-                                 new_status['monthly_limit'])
-        update_key_configuration('yearly_limit', 'TIMELINE_LIMIT_YEARLY={}',
-                                 new_status['yearly_limit'])
-        update_key_configuration('number_min_age', 'NUMBER_MIN_AGE={}',
-                                 int(new_status['number_min_age']) * 86400)
 
-        actions.superuser_run('snapshot', ['configure', 'NUMBER_LIMIT=0'])
-        actions.superuser_run('snapshot',
-                              ['configure', 'NUMBER_LIMIT_IMPORTANT=4-10'])
         messages.success(request, _('Storage snapshots configuration updated'))
     except ActionError as exception:
         messages.error(request,