transmission: New directory selection form

- use new directory selection form (#1703)
- add transmission-daemon to the freedombox-share group
- pass only needed data to the template
- do not change or validate download directory if transmission
  module is going to be disabled
- yapf3 validation changes

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/transmission/__init__.py

@@ -28,11 +28,11 @@ from plinth import frontpage, menu
 from plinth.daemon import Daemon
 from plinth.modules.apache.components import Webserver
 from plinth.modules.firewall.components import Firewall
-from plinth.modules.users import register_group
+from plinth.modules.users import register_group, add_user_to_share_group
 
 from .manifest import backup, clients  # noqa, pylint: disable=unused-import
 
-version = 2
+version = 3
 
 managed_services = ['transmission-daemon']
 
@@ -74,11 +74,10 @@ class TransmissionApp(app_module.App):
                               parent_url_name='apps')
         self.add(menu_item)
 
-        shortcut = frontpage.Shortcut('shortcut-transmission', name,
-                                      short_description=short_description,
-                                      icon=icon_filename, url='/transmission',
-                                      clients=clients, login_required=True,
-                                      allowed_groups=[group[0]])
+        shortcut = frontpage.Shortcut(
+            'shortcut-transmission', name, short_description=short_description,
+            icon=icon_filename, url='/transmission', clients=clients,
+            login_required=True, allowed_groups=[group[0]])
         self.add(shortcut)
 
         firewall = Firewall('firewall-transmission', name,
@@ -114,7 +113,7 @@ def setup(helper, old_version=None):
     helper.call('post', actions.superuser_run, 'transmission',
                 ['merge-configuration'],
                 input=json.dumps(new_configuration).encode())
-
+    add_user_to_share_group(reserved_usernames[0], managed_services[0])
     helper.call('post', app.enable)
 
 

plinth/modules/transmission/forms.py

@@ -18,16 +18,20 @@
 FreedomBox app for configuring Transmission.
 """
 
-from django import forms
 from django.utils.translation import ugettext_lazy as _
 
-from plinth.forms import AppForm
+from plinth.modules.transmission import reserved_usernames
+from plinth.modules.storage.forms import (DirectorySelectForm,
+                                          DirectoryValidator)
 
 
-class TransmissionForm(AppForm):  # pylint: disable=W0232
+class TransmissionForm(DirectorySelectForm):
     """Transmission configuration form"""
-    download_dir = forms.CharField(
-        label=_('Download directory'),
-        help_text=_('Directory where downloads are saved.  If you change the '
-                    'default directory, ensure that the new directory exists '
-                    'and is writable by "debian-transmission" user.'))
+
+    def __init__(self, *args, **kw):
+        validator = DirectoryValidator(
+            username=reserved_usernames[0], check_writable=True)
+        super(TransmissionForm, self).__init__(
+            title=_('Download directory'),
+            default='/var/lib/transmission-daemon/downloads/',
+            validator=validator, *args, **kw)

plinth/modules/transmission/views.py

@@ -20,11 +20,11 @@ FreedomBox app for configuring Transmission Server.
 
 import json
 import logging
+import os
 import socket
 
 from django.contrib import messages
 from django.utils.translation import ugettext as _
-
 from plinth import actions, views
 from plinth.modules import transmission
 
@@ -50,10 +50,8 @@ class TransmissionAppView(views.AppView):
         configuration = actions.superuser_run('transmission',
                                               ['get-configuration'])
         configuration = json.loads(configuration)
-        status.update({
-            key.translate(str.maketrans({'-': '_'})): value
-            for key, value in configuration.items()
-        })
+        status['storage_path'] = os.path.normpath(
+            configuration['download-dir'])
         status['hostname'] = socket.gethostname()
 
         return status
@@ -63,13 +61,15 @@ class TransmissionAppView(views.AppView):
         old_status = form.initial
         new_status = form.cleaned_data
 
-        if old_status['download_dir'] != new_status['download_dir']:
-            new_configuration = {
-                'download-dir': new_status['download_dir'],
-            }
+        if new_status['is_enabled'] or not old_status['is_enabled']:
+            if old_status['storage_path'] != new_status['storage_path']:
+                new_configuration = {
+                    'download-dir': new_status['storage_path'],
+                }
 
-            actions.superuser_run('transmission', ['merge-configuration'],
-                                  input=json.dumps(new_configuration).encode())
-            messages.success(self.request, _('Configuration updated'))
+                actions.superuser_run(
+                    'transmission', ['merge-configuration'],
+                    input=json.dumps(new_configuration).encode())
+                messages.success(self.request, _('Configuration updated'))
 
         return super().form_valid(form)