From d4b21ef1e4e4df54061e3a16a968035d4cfabb96 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 30 Jan 2023 10:46:03 -0800 Subject: [PATCH] views: Use dedicated view when showing an app with operations Closes: #2309. - This prevents processing of AppView when the app is being uninstalled. For at least two apps, this has failed because the AppView assumes that app and its dependencies are installed. - Use a dedicated template as well is simplify app template. Tests: - Installing and uninstalling an app works. - Refreshing the app page during uninstall does not lead to an error for samba and email apps. - Unit tests pass. - Functional tests for samba and email work. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/samba/tests/test_views.py | 10 ++-- plinth/modules/users/tests/test_views.py | 5 +- plinth/templates/app-operations.html | 16 +++++ plinth/templates/app.html | 74 +++++++++++------------- plinth/views.py | 38 +++++++++++- 5 files changed, 94 insertions(+), 49 deletions(-) create mode 100644 plinth/templates/app-operations.html diff --git a/plinth/modules/samba/tests/test_views.py b/plinth/modules/samba/tests/test_views.py index 70dd0c061..e7936dff6 100644 --- a/plinth/modules/samba/tests/test_views.py +++ b/plinth/modules/samba/tests/test_views.py @@ -91,11 +91,11 @@ def make_request(request, view, **kwargs): def test_samba_shares_view(rf): """Test that a share list has correct view data.""" - with patch('plinth.views.AppView.get_context_data', return_value={ - 'is_enabled': True - }), patch('plinth.modules.samba.get_users', - return_value=USERS), patch('plinth.modules.storage.get_mounts', - return_value=DISKS): + with (patch('plinth.views.AppView.get_context_data', + return_value={'is_enabled': True}), + patch('plinth.modules.samba.get_users', return_value=USERS), + patch('plinth.modules.storage.get_mounts', return_value=DISKS), + patch('plinth.views.AppView.app', return_value=None)): view = views.SambaAppView.as_view() response, _ = make_request(rf.get(''), view) diff --git a/plinth/modules/users/tests/test_views.py b/plinth/modules/users/tests/test_views.py index a622c7a58..badb05c59 100644 --- a/plinth/modules/users/tests/test_views.py +++ b/plinth/modules/users/tests/test_views.py @@ -84,8 +84,9 @@ def make_request(request, view, as_admin=True, **kwargs): def test_users_list_view(rf): """Test that users list view has correct view data.""" - with patch('plinth.views.AppView.get_context_data', - return_value={'is_enabled': True}): + with (patch('plinth.views.AppView.get_context_data', + return_value={'is_enabled': True}), + patch('plinth.views.AppView.app', return_value=None)): view = views.UserList.as_view() response, messages = make_request(rf.get('/'), view) diff --git a/plinth/templates/app-operations.html b/plinth/templates/app-operations.html new file mode 100644 index 000000000..329878bc2 --- /dev/null +++ b/plinth/templates/app-operations.html @@ -0,0 +1,16 @@ +{% extends "base.html" %} +{% comment %} +# SPDX-License-Identifier: AGPL-3.0-or-later +{% endcomment %} + +{# Template to show an App with operations, used by views.AppOperationsView #} + +{% block content %} + + {% include "app-header.html" %} + + {% include "toolbar.html" %} + + {% include "operations.html" %} + +{% endblock %} diff --git a/plinth/templates/app.html b/plinth/templates/app.html index 8f9beef85..bc30d849a 100644 --- a/plinth/templates/app.html +++ b/plinth/templates/app.html @@ -14,53 +14,49 @@ {% include "app-header.html" %} - {% if not operations %} - {% include "toolbar.html" with enabled=is_enabled %} + {% include "toolbar.html" with enabled=is_enabled %} - {% block subsubmenu %} - {% if subsubmenu %} - {% show_subsubmenu subsubmenu %} - {% endif %} - {% endblock %} + {% block subsubmenu %} + {% if subsubmenu %} + {% show_subsubmenu subsubmenu %} + {% endif %} + {% endblock %} - {% block status %} - {% if is_running is not None and not is_running %} - - {% endif %} - {% endblock %} + {% block status %} + {% if is_running is not None and not is_running %} + + {% endif %} + {% endblock %} - {% block internal_zone %} - {% include "internal-zone.html" %} - {% endblock %} + {% block internal_zone %} + {% include "internal-zone.html" %} + {% endblock %} - {% block port_forwarding_info %} - {% include "port-forwarding-info.html" with service_name=app_info.name %} - {% endblock %} + {% block port_forwarding_info %} + {% include "port-forwarding-info.html" with service_name=app_info.name %} + {% endblock %} - {% block configuration %} - {% if form %} -

{% trans "Configuration" %}

+ {% block configuration %} + {% if form %} +

{% trans "Configuration" %}

-
- {% csrf_token %} + + {% csrf_token %} - {{ form|bootstrap }} + {{ form|bootstrap }} - -
- {% endif %} - {% endblock %} + + + {% endif %} + {% endblock %} - {% block extra_content %} - {% endblock %} - {% else %} - {% include "operations.html" %} - {% endif %} + {% block extra_content %} + {% endblock %} {% endblock %} diff --git a/plinth/views.py b/plinth/views.py index aed1278a0..b7c12c0d1 100644 --- a/plinth/views.py +++ b/plinth/views.py @@ -162,6 +162,15 @@ class AppView(FormView): super().__init__(*args, **kwargs) self._common_status = None + def dispatch(self, request, *args, **kwargs): + """If operations are running on the app, use a different view.""" + operations = operation.manager.filter(self.app.app_id) + if operations: + view = AppOperationsView.as_view(app_id=self.app.app_id) + return view(request, *args, **kwargs) + + return super().dispatch(request, *args, **kwargs) + def post(self, request, *args, **kwargs): """Handle app enable/disable button separately.""" if 'app_enable_disable_button' not in request.POST: @@ -258,10 +267,7 @@ class AppView(FormView): context['port_forwarding_info'] = get_port_forwarding_info(self.app) context['app_enable_disable_form'] = self.get_enable_disable_form() context['show_uninstall'] = not self.app.info.is_essential - context['operations'] = operation.manager.filter(self.app.app_id) context['refresh_page_sec'] = None - if context['operations']: - context['refresh_page_sec'] = 3 from plinth.modules.firewall.components import Firewall context['firewall'] = self.app.get_components_of_type(Firewall) @@ -269,6 +275,32 @@ class AppView(FormView): return context +class AppOperationsView(TemplateView): + """View to show app page when some app operations are running.""" + app_id = None # Set when app is instantiated. + template_name = "app-operations.html" + + @property + def app(self): + """Return the app for which this view is configured.""" + if not self.app_id: + raise ImproperlyConfigured('Missing attribute: app_id') + + return app_module.App.get(self.app_id) + + def get_context_data(self, *args, **kwargs): + """Add additional context data for template.""" + context = super().get_context_data(*args, **kwargs) + context['app_id'] = self.app.app_id + context['app_info'] = self.app.info + context['operations'] = operation.manager.filter(self.app.app_id) + context['refresh_page_sec'] = 0 + if context['operations']: + context['refresh_page_sec'] = 3 + + return context + + class SetupView(TemplateView): """View to prompt and setup applications.""" template_name = 'setup.html'