pagekite: Merge set-kite and set-frontend actions

- Simplifies the form submission code.

- When service is disabled, don't add the abort_not_configured configuration
flag. There is not need to change the configuration as disabling the daemon from
running is reliable enough.

- When the configuration is set for the first time, drop the
abort_not_configured flag and don't ever add it back.

- Set default values properly for frontend fields when no value is supplied.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>

actions/pagekite

@@ -44,18 +44,13 @@ def parse_arguments():
 
     # Configuration
     subparsers.add_parser('get-config', help='Return current configuration')
-
+    set_config = subparsers.add_parser(
-    # Frontend
+        'set-config',
-    set_frontend = subparsers.add_parser('set-frontend',
+        help='Configure kite name, its secret and frontend. Secret is read '
-                                         help='Set pagekite frontend')
+        'from stdin.')
-    set_frontend.add_argument('url', help='frontend url')
+    set_config.add_argument('--kite-name',
-
+                            help='Name of the kite (eg: mybox.pagekite.me)')
-    # Kite details (name + secret)
+    set_config.add_argument('--frontend', help='Frontend url')
-    set_kite = subparsers.add_parser(
-        'set-kite',
-        help='Configure kite name and its secret. Secret is read from stdin.')
-    set_kite.add_argument('--kite-name',
-                          help='Name of the kite (eg: mybox.pagekite.me)')
 
     # Add/remove pagekite services (service_on entries)
     add_service = subparsers.add_parser('add-service',
@@ -76,8 +71,6 @@ def subcommand_restart(_):
 
 
 def subcommand_start_and_enable(_):
-    aug.remove(PATHS['abort_not_configured'])
-    aug.save()
     # 'start' alone sometimes fails, even if the service is not running
     action_utils.service_restart('pagekite')
     print('enabled')
@@ -85,8 +78,6 @@ def subcommand_start_and_enable(_):
 
 def subcommand_stop_and_disable(_):
     action_utils.service_stop('pagekite')
-    aug.set(PATHS['abort_not_configured'], '')
-    aug.save()
     print('disabled')
 
 
@@ -144,26 +135,22 @@ def subcommand_get_config(_):
     print(json.dumps(status))
 
 
-def subcommand_set_frontend(arguments):
+def subcommand_set_config(arguments):
-    """Set pagekite frontend url, taking care of defaults and pagekite.net"""
+    """Set pagekite kite name, secret and frontend URL."""
-    frontend_domain = arguments.url.split(':')[0]
+    aug.remove(PATHS['abort_not_configured'])
+
+    aug.set(PATHS['kitename'], arguments.kite_name)
+    aug.set(PATHS['kitesecret'], sys.stdin.read())
+
+    frontend_domain = arguments.frontend.split(':')[0]
     if frontend_domain in ('pagekite.net', 'defaults', 'default'):
-        enable_pagekitenet_frontend()
+        aug.set(PATHS['defaults'], '')
+        aug.remove(PATHS['frontend'])
     else:
         aug.remove(PATHS['defaults'])
-        aug.set(PATHS['frontend'], arguments.url)
+        aug.set(PATHS['frontend'], arguments.frontend)
-        aug.save()
 
-
-def enable_pagekitenet_frontend():
-    """Enable using default pageket.net frontend
-
-    This disables any other frontends.
-    """
-    aug.set(PATHS['defaults'], '')
-    aug.remove(PATHS['frontend'])
     aug.save()
-    print("enabled")
 
 
 def subcommand_remove_service(arguments):
@@ -240,13 +227,6 @@ def get_new_service_path(protocol):
     return os.path.join(root, str(new_index))
 
 
-def subcommand_set_kite(arguments):
-    """Set details of the kite"""
-    aug.set(PATHS['kitename'], arguments.kite_name)
-    aug.set(PATHS['kitesecret'], sys.stdin.read())
-    aug.save()
-
-
 def augeas_load():
     """Initialize Augeas."""
     global aug

plinth/modules/pagekite/forms.py

@@ -69,26 +69,30 @@ class ConfigurationForm(AppForm):
 
     def save(self, request):
         """Save the form on submission after validation."""
-        old = self.initial
+        def _filter(data):
-        new = self.cleaned_data
+            return {
-        LOGGER.info('New status is - %s', new)
+                key: str(value)
+                for key, value in data.items() if key in
+                ['kite_name', 'kite_secret', 'server_domain', 'server_port']
+            }
+
+        if not self.cleaned_data['server_domain']:
+            self.cleaned_data['server_domain'] = 'pagekite.net'
+
+        if not self.cleaned_data['server_port']:
+            self.cleaned_data['server_port'] = '80'
+
+        old = _filter(self.initial)
+        new = _filter(self.cleaned_data)
 
         if old != new:
-            config_changed = False
+            frontend = f"{new['server_domain']}:{new['server_port']}"
-
+            utils.run([
-            if old['kite_name'] != new['kite_name'] or \
+                'set-config', '--kite-name', new['kite_name'], '--frontend',
-               old['kite_secret'] != new['kite_secret']:
+                frontend
-                utils.run(['set-kite', '--kite-name', new['kite_name']],
+            ], input=new['kite_secret'].encode())
-                          input=new['kite_secret'].encode())
+            messages.success(request, _('Configuration updated'))
-                messages.success(request, _('Kite details set'))
+            config_changed = True
-                config_changed = True
-
-            if old['server_domain'] != new['server_domain'] or \
-               old['server_port'] != new['server_port']:
-                server = "%s:%s" % (new['server_domain'], new['server_port'])
-                utils.run(['set-frontend', server])
-                messages.success(request, _('Pagekite server set'))
-                config_changed = True
 
             if old['is_enabled'] != new['is_enabled']:
                 if new['is_enabled']: