From 058702f2b8f28571fb8e0a3afa35049ba1695562 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 18 May 2020 21:44:37 -0700 Subject: [PATCH] openvpn: Use app toggle button and common app view Tests performed: - When app is not setup, app toggle button is not shown. Running status of the app is also not present in the page. Profile download is not shown. Setup button is shown. - When app is being setup, app toggle button is not shown. Running status of the app is also not present in the page. Page keeps refreshing every 3 seconds during setup. Profile download is not shown. A progress spinner is shown that setup is currently running. - When app setup has completed, app toggle button is shown. Running status is shown. When daemon is stopped, a message that daemon is not running is show. Profile download is shown. - Transition from being setup into setup completed is done with a single page refresh. Message that setup is completed is shown. - Port forwarding information is always shown (before, during and after setup). - Run functional tests for OpenVPN. Signed-off-by: Sunil Mohan Adapa Reviewed-by: Joseph Nuthalapati --- functional_tests/support/application.py | 1 - plinth/modules/openvpn/__init__.py | 7 ++ plinth/modules/openvpn/forms.py | 13 --- plinth/modules/openvpn/templates/openvpn.html | 16 ++-- plinth/modules/openvpn/urls.py | 3 +- plinth/modules/openvpn/views.py | 96 +++++-------------- 6 files changed, 41 insertions(+), 95 deletions(-) delete mode 100644 plinth/modules/openvpn/forms.py diff --git a/functional_tests/support/application.py b/functional_tests/support/application.py index 128a58f3f..6f52037dd 100644 --- a/functional_tests/support/application.py +++ b/functional_tests/support/application.py @@ -26,7 +26,6 @@ app_module = { app_checkbox_id = { 'tor': 'id_tor-enabled', - 'openvpn': 'id_openvpn-enabled', } default_url = config['DEFAULT']['url'] diff --git a/plinth/modules/openvpn/__init__.py b/plinth/modules/openvpn/__init__.py index bfc7964cc..29d612c94 100644 --- a/plinth/modules/openvpn/__init__.py +++ b/plinth/modules/openvpn/__init__.py @@ -36,12 +36,19 @@ port_forwarding_info = [('UDP', 1194)] app = None +setup_process = None + class OpenVPNApp(app_module.App): """FreedomBox app for OpenVPN.""" app_id = 'openvpn' + @property + def can_be_disabled(self): + """Return whether the app can be disabled.""" + return is_setup() and not setup_process + def __init__(self): """Create components for the app.""" super().__init__() diff --git a/plinth/modules/openvpn/forms.py b/plinth/modules/openvpn/forms.py deleted file mode 100644 index 31ec5c3f5..000000000 --- a/plinth/modules/openvpn/forms.py +++ /dev/null @@ -1,13 +0,0 @@ -# SPDX-License-Identifier: AGPL-3.0-or-later -""" -FreedomBox app for configuring OpenVPN. -""" - -from django import forms -from django.utils.translation import ugettext_lazy as _ - - -class OpenVpnForm(forms.Form): # pylint: disable=W0232 - """OpenVPN configuration form.""" - enabled = forms.BooleanField(label=_('Enable OpenVPN server'), - required=False) diff --git a/plinth/modules/openvpn/templates/openvpn.html b/plinth/modules/openvpn/templates/openvpn.html index 54b29bc0b..46a96bdef 100644 --- a/plinth/modules/openvpn/templates/openvpn.html +++ b/plinth/modules/openvpn/templates/openvpn.html @@ -20,6 +20,10 @@ {% block status %} + {% if status.is_setup and not status.setup_running %} + {{ block.super }} + {% endif %} + {% if not status.is_setup and not status.setup_running %}

{% trans "Status" %}

@@ -41,7 +45,7 @@ {% endif %} - {% if not status.is_setup and status.setup_running %} + {% if status.setup_running %}

{% trans "Status" %}

@@ -59,15 +63,11 @@

{% endif %} - {% if status.is_setup %} - {{ block.super }} - {% endif %} - {% endblock %} {% block configuration %} - {% if status.is_setup %} + {% if status.is_setup and not status.setup_running %}

{% trans "Profile" %}

@@ -98,10 +98,6 @@ {% endif %} - {% if status.is_setup %} - {{ block.super }} - {% endif %} - {% endblock %} {% block page_js %} diff --git a/plinth/modules/openvpn/urls.py b/plinth/modules/openvpn/urls.py index 793915bf5..3bd56c108 100644 --- a/plinth/modules/openvpn/urls.py +++ b/plinth/modules/openvpn/urls.py @@ -6,10 +6,11 @@ URLs for the OpenVPN module. from django.conf.urls import url from plinth.utils import non_admin_view + from . import views urlpatterns = [ - url(r'^apps/openvpn/$', views.index, name='index'), + url(r'^apps/openvpn/$', views.OpenVPNAppView.as_view(), name='index'), url(r'^apps/openvpn/setup/$', views.setup, name='setup'), url(r'^apps/openvpn/profile/$', non_admin_view(views.profile), name='profile'), diff --git a/plinth/modules/openvpn/views.py b/plinth/modules/openvpn/views.py index 064678089..422e91527 100644 --- a/plinth/modules/openvpn/views.py +++ b/plinth/modules/openvpn/views.py @@ -8,59 +8,45 @@ import logging from django.contrib import messages from django.http import HttpResponse from django.shortcuts import redirect -from django.template.response import TemplateResponse from django.utils.translation import ugettext as _ from django.views.decorators.http import require_POST -from plinth import actions, daemon +from plinth import actions from plinth.modules import config, openvpn - -from .forms import OpenVpnForm +from plinth.views import AppView logger = logging.getLogger(__name__) -setup_process = None +class OpenVPNAppView(AppView): + """Show OpenVPN app main page.""" + app_id = 'openvpn' + template_name = 'openvpn.html' + port_forwarding_info = openvpn.port_forwarding_info -def index(request): - """Serve configuration page.""" - status = get_status() + def dispatch(self, request, *args, **kwargs): + """Collect the result of running setup process.""" + if bool(openvpn.setup_process): + _collect_setup_result(request) - if status['setup_running']: - _collect_setup_result(request) + return super().dispatch(request, *args, **kwargs) - form = None - - if request.method == 'POST': - form = OpenVpnForm(request.POST, prefix='openvpn') - # pylint: disable=E1101 - if form.is_valid(): - _apply_changes(request, status, form.cleaned_data) - status = get_status() - form = OpenVpnForm(initial=status, prefix='openvpn') - else: - form = OpenVpnForm(initial=status, prefix='openvpn') - - return TemplateResponse( - request, 'openvpn.html', { - 'app_id': 'openvpn', - 'app_info': openvpn.app.info, - 'port_forwarding_info': openvpn.port_forwarding_info, - 'status': status, - 'form': form, - 'is_running': status['is_running'], - 'has_diagnostics': True, - 'is_enabled': status['enabled'], - }) + def get_context_data(self, *args, **kwargs): + """Add additional context data for template.""" + context = super().get_context_data(*args, **kwargs) + context['status'] = { + 'is_setup': openvpn.is_setup(), + 'setup_running': bool(openvpn.setup_process), + } + return context @require_POST def setup(request): """Start the setup process.""" - global setup_process - if not openvpn.is_setup() and not setup_process: - setup_process = actions.superuser_run('openvpn', ['setup'], - run_in_background=True) + if not openvpn.is_setup() and not openvpn.setup_process: + openvpn.setup_process = actions.superuser_run('openvpn', ['setup'], + run_in_background=True) openvpn.app.enable() @@ -86,24 +72,12 @@ def profile(request): return response -def get_status(): - """Get the current settings from OpenVPN server.""" - - return { - 'is_setup': openvpn.is_setup(), - 'setup_running': bool(setup_process), - 'enabled': openvpn.app.is_enabled(), - 'is_running': daemon.app_is_running(openvpn.app) - } - - def _collect_setup_result(request): """Handle setup process is completion.""" - global setup_process - if not setup_process: + if not openvpn.setup_process: return - return_code = setup_process.poll() + return_code = openvpn.setup_process.poll() # Setup process is not complete yet if return_code is None: @@ -114,22 +88,4 @@ def _collect_setup_result(request): else: messages.info(request, _('Setup failed.')) - setup_process = None - - -def _apply_changes(request, old_status, new_status): - """Apply the changes.""" - modified = False - - if old_status['enabled'] != new_status['enabled']: - if new_status['enabled']: - openvpn.app.enable() - else: - openvpn.app.disable() - - modified = True - - if modified: - messages.success(request, _('Configuration updated')) - else: - messages.info(request, _('Setting unchanged')) + openvpn.setup_process = None