matrix-synapse: Option public registrations fixed and simplified.

* Fixing wrong function calls leading to error 500.
* Merging enable/disabling/status into single action, to improve
  handling in cli, and reduce code duplication.
* Fixing order of restart and enabling of public registration option.
* Minor, cosmetic fixes of code and user-facing strings.
* Overall code design now almost identical to Ejabberd service page.

Signed-off-by: Johannes Keyser <johanneskeyser@posteo.de>

actions/matrixsynapse

@@ -21,9 +21,7 @@
 Configuration helper for Matrix-Synapse server.
 """
 import argparse
-
 from ruamel.yaml import round_trip_dump, round_trip_load
-
 from plinth import action_utils
 from plinth.modules.matrixsynapse import CONFIG_FILE_PATH
 
@@ -36,11 +34,10 @@ def parse_arguments():
     subparsers.add_parser('post-install', help='Perform post install steps')
     subparsers.add_parser('enable', help='Enable matrix-synapse service')
     subparsers.add_parser('disable', help='Disable matrix-synapse service')
-    subparsers.add_parser('restart', help='Restart matrix-synapse service')
-    subparsers.add_parser('enable-registration',
-                          help='Turn on Public registrations')
-    subparsers.add_parser('disable-registration',
-                          help='Turn off Public registrations')
+    help_pubreg = 'Enable/Disable/Status public user registration.'
+    pubreg = subparsers.add_parser('public_registration', help=help_pubreg)
+    pubreg.add_argument('command', choices=('enable', 'disable', 'status'),
+                        help=help_pubreg)
     setup = subparsers.add_parser('setup', help='Set domain name for Matrix')
     setup.add_argument(
         '--domain-name',
@@ -85,33 +82,32 @@ def subcommand_disable(_):
     action_utils.service_disable('matrix-synapse')
 
 
-def subcommand_enable_registration(_):
-    """ Enable public registration"""
+def subcommand_public_registration(argument):
+    """Enable/Disable/Status public user registration."""
     with open(CONFIG_FILE_PATH) as config_file:
         config = round_trip_load(config_file)
-    config['enable_registration'] = True
+
+    if argument.command == 'status':
+        if config['enable_registration']:
+            print('enabled')
+            return
+        else:
+            print('disabled')
+            return
+    elif argument.command == 'enable':
+        config['enable_registration'] = True
+    elif argument.command == 'disable':
+        config['enable_registration'] = False
+
     with open(CONFIG_FILE_PATH, 'w') as config_file:
         round_trip_dump(config, config_file)
 
-    if action_utils.is_service_running('matrix-synapse'):
-        action_utils.service_restart('matrix-synapse')
-
-
-def subcommand_disable_registration(_):
-    """ Disable public registration"""
-    with open(CONFIG_FILE_PATH) as config_file:
-        config = round_trip_load(config_file)
-    config['enable_registration'] = False
-    with open(CONFIG_FILE_PATH, 'w') as config_file:
-        round_trip_dump(config, config_file)
-
-    if action_utils.is_service_running('matrix-synapse'):
+    if action_utils.service_is_running('matrix-synapse'):
         action_utils.service_restart('matrix-synapse')
 
 
 def main():
     arguments = parse_arguments()
-
     sub_command = arguments.subcommand.replace('-', '_')
     sub_command_method = globals()['subcommand_' + sub_command]
     sub_command_method(arguments)

plinth/modules/matrixsynapse/__init__.py

@@ -152,11 +152,8 @@ def get_configured_domain_name():
 
 
 def get_public_registration_status():
-    """ Return whether public registration is turned on"""
-    if not is_setup():
-        return None
+    """Return whether public registration is enabled."""
+    output = actions.superuser_run('matrixsynapse', ['public_registration',
+                                                     'status'])
+    return output.strip() == 'enabled'
 
-    with open(CONFIG_FILE_PATH) as config_file:
-        config, _, _ = load_yaml_guess_indent(config_file)
-
-    return config['enable_registration']

plinth/modules/matrixsynapse/forms.py

@@ -21,7 +21,6 @@ Forms for the Matrix Synapse module.
 
 from django import forms
 from django.utils.translation import ugettext_lazy as _
-
 from plinth.forms import ServiceForm
 
 
@@ -30,7 +29,6 @@ class MatrixSynapseForm(ServiceForm):
         label=_('Enable Public Registration'),
         required=False,
         help_text=_(
-            'Enable or disable public registrations for matrix-synapse. '
-            'Enabling public registrations means that anyone on the internet '
-            'can register a new account on your Matrix server.Disable the '
-            'this if you only want existing users to be able to use it.'))
+            'Enabling public registration means that anyone on the Internet '
+            'can register a new account on your Matrix server. Disable this '
+            'if you only want existing users to be able to use it.'))

plinth/modules/matrixsynapse/templates/matrix-synapse.html

@@ -14,8 +14,7 @@
 # GNU Affero General Public License for more details.
 #
 # 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/>.
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 {% endcomment %}
 
@@ -33,5 +32,5 @@
       name after the initial setup is currently not supported.
     {% endblocktrans %}
   </p>
-  <p>New users can be registered from any client.</p>
+  <p>New users can be registered from any client if public registration is enabled.</p>
 {% endblock %}

plinth/modules/matrixsynapse/urls.py

@@ -21,9 +21,10 @@ URLs for the matrix-synapse module.
 
 from django.conf.urls import url
 
-from .views import SetupView, ServiceView
+from .views import SetupView, MatrixSynapseServiceView
 
 urlpatterns = [
     url(r'^apps/matrixsynapse/setup/$', SetupView.as_view(), name='setup'),
-    url(r'^apps/matrixsynapse/$', ServiceView.as_view(), name='index'),
+    url(r'^apps/matrixsynapse/$', MatrixSynapseServiceView.as_view(),
+        name='index'),
 ]

plinth/modules/matrixsynapse/views.py

@@ -26,7 +26,7 @@ from django.utils.translation import ugettext_lazy as _
 from django.views.generic import FormView
 
 from plinth import actions
-from plinth import views
+from plinth.views import ServiceView
 from plinth.modules import matrixsynapse
 from plinth.forms import DomainSelectionForm
 from plinth.utils import get_domain_names
@@ -34,6 +34,7 @@ from plinth.utils import get_domain_names
 from .forms import MatrixSynapseForm
 from . import get_public_registration_status
 
+
 class SetupView(FormView):
     """Show matrix-synapse setup page."""
     template_name = 'matrix-synapse-pre-setup.html'
@@ -59,7 +60,7 @@ class SetupView(FormView):
         return context
 
 
-class ServiceView(views.ServiceView):
+class MatrixSynapseServiceView(ServiceView):
     """Show matrix-synapse service page."""
     service_id = matrixsynapse.managed_services[0]
     template_name = 'matrix-synapse.html'
@@ -76,7 +77,7 @@ class ServiceView(views.ServiceView):
 
     def get_context_data(self, *args, **kwargs):
         """Add additional context data for template."""
-        context = super().get_context_data(**kwargs)
+        context = super().get_context_data(*args, **kwargs)
         context['domain_name'] = matrixsynapse.get_configured_domain_name()
         return context
 
@@ -91,17 +92,34 @@ class ServiceView(views.ServiceView):
         """Handle valid form submission."""
         old_config = self.get_initial()
         new_config = form.cleaned_data
-        if old_config['enable_public_registration'] !=\
-                new_config['enable_public_registration']:
-            enable_registration = new_config['enable_public_registration']
-            if enable_registration:
-                actions.superuser_run('matrixsynapse', ['enable-registration'])
+        app_same = old_config['is_enabled'] == new_config['is_enabled']
+        pubreg_same = old_config['enable_public_registration'] == \
+                      new_config['enable_public_registration']
+
+        if app_same and pubreg_same:
+            # TODO: find a more reliable/official way to check whether the
+            # request has messages attached.
+            if not self.request._messages._queued_messages:
+                messages.info(self.request, _('Setting unchanged'))
+        elif not app_same:
+            if new_config['is_enabled']:
+                self.service.enable()
+                messages.success(self.request, _('Application enabled'))
+            else:
+                self.service.disable()
+                messages.success(self.request, _('Application disabled'))
+
+        if not pubreg_same:
+            # note action public_registration restarts, if running now
+            if new_config['enable_public_registration']:
+                actions.superuser_run('matrixsynapse', ['public_registration',
+                                                        'enable'])
                 messages.success(self.request,
                                  _('Public registration enabled'))
             else:
-                actions.superuser_run('matrixsynapse', ['disable-registration'])
+                actions.superuser_run('matrixsynapse', ['public_registration',
+                                                        'disable'])
                 messages.success(self.request,
                                  _('Public registration disabled'))
-            if new_config['is_enabled'] == True:
-                actions.superuser_run('matrixsynapse', ['restart'])
-        return super().form_valid(form)
+
+        return super(ServiceView, self).form_valid(form)