snapshots: Minor refactoring

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

plinth/modules/snapshot/__init__.py

@@ -14,17 +14,16 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
-
 """
 Plinth module to manage filesystem snapshots.
 """
 
+import json
+
 from django.utils.translation import ugettext_lazy as _
 
 from plinth import actions
 from plinth.menu import main_menu
-import json
-
 
 version = 1
 
@@ -36,7 +35,6 @@ description = [
     _('Snapshots allows creating and managing filesystem snapshots. These can '
       'be used to roll back the system to a previously known good state in '
       'case of unwanted changes to the system.'),
-
     _('Automatic snapshots are taken every hour, day, month and year. Older '
       'snapshots are automatically deleted keeping 10 of each kind and 50 in '
       'total. Although snapshots are efficient and only store the '
@@ -65,5 +63,4 @@ def is_timeline_snapshots_enabled():
     """Return whether timeline snapshots are enabled."""
     output = actions.superuser_run('snapshot', ['get-config'])
     output = json.loads(output)
-    return output['TIMELINE_CREATE'] == "yes"
+    return output['TIMELINE_CREATE'] == 'yes'
-

plinth/modules/snapshot/views.py

@@ -29,6 +29,7 @@ from django.utils.translation import ugettext as _
 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 . import is_timeline_snapshots_enabled
 from .forms import SnapshotForm
@@ -38,16 +39,16 @@ def index(request):
     """Show snapshot list."""
     status = get_status()
     if request.method == 'POST':
-        form = SnapshotForm(request.POST, prefix='snapshot')
+        form = SnapshotForm(request.POST)
         if 'create' in request.POST:
             actions.superuser_run('snapshot', ['create'])
             messages.success(request, _('Created snapshot.'))
         if 'update' in request.POST and form.is_valid():
-            _apply_changes(request, status, form.cleaned_data)
+            update_configuration(request, status, form.cleaned_data)
             status = get_status()
-            form = SnapshotForm(initial=status, prefix='snapshot')
+            form = SnapshotForm(initial=status)
     else:
-        form = SnapshotForm(initial=status, prefix='snapshot')
+        form = SnapshotForm(initial=status)
 
     output = actions.superuser_run('snapshot', ['list'])
     snapshots = json.loads(output)
@@ -60,6 +61,25 @@ def index(request):
     })
 
 
+def update_configuration(request, old_status, new_status):
+    """Update configuration of snapshots."""
+    try:
+        key = 'enable_timeline_snapshots'
+        if old_status[key] != new_status[key]:
+            enable_timeline = yes_or_no(
+                new_status['enable_timeline_snapshots'])
+            actions.superuser_run(
+                'snapshot',
+                ['configure', 'TIMELINE_CREATE={}'.format(enable_timeline)])
+            messages.success(request,
+                             _('Timeline Snapshots configuration updated'))
+    except ActionError as exception:
+        messages.error(request,
+                       _('Action error: {0} [{1}] [{2}]').format(
+                           exception.args[0], exception.args[1],
+                           exception.args[2]))
+
+
 def delete(request, number):
     """Show confirmation to delete a snapshot."""
     if request.method == 'POST':
@@ -121,24 +141,5 @@ def rollback(request, number):
 
 
 def get_status():
+    """Get current status of snapshot configuration."""
     return {'enable_timeline_snapshots': is_timeline_snapshots_enabled()}
-
-
-def _apply_changes(request, old_status, new_status):
-    """Try to apply changes and handle errors."""
-    try:
-        __apply_changes(request, old_status, new_status)
-    except ActionError as exception:
-        messages.error(request,
-                       _('Action error: {0} [{1}] [{2}]').format(
-                           exception.args[0], exception.args[1],
-                           exception.args[2]))
-
-
-def __apply_changes(request, old_status, new_status):
-    if old_status['enable_timeline_snapshots'] != new_status['enable_timeline_snapshots']:
-        timeline_create = "TIMELINE_CREATE=yes" if new_status[
-            'enable_timeline_snapshots'] else "TIMELINE_CREATE=no"
-        actions.superuser_run('snapshot', ['configure', timeline_create])
-        messages.success(request,
-                         _('Timeline Snapshots configuration updated'))

plinth/utils.py

@@ -115,3 +115,7 @@ class YAMLFile(object):
 
     def is_file_empty(self):
         return os.stat(self.yaml_file).st_size == 0
+
+
+def yes_or_no(cond):
+    return 'yes' if cond else 'no'