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 <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

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)
 

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)
 

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 %}

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 %}
-        <div id='service-not-running' role="alert"
-             class="alert alert-danger {{ is_enabled|yesno:',d-none' }}">
-          {% blocktrans trimmed with service_name=app_info.name %}
-            Service <em>{{ service_name }}</em> is not running.
-          {% endblocktrans %}
-        </div>
-      {% endif %}
-    {% endblock %}
+  {% block status %}
+    {% if is_running is not None and not is_running %}
+      <div id='service-not-running' role="alert"
+           class="alert alert-danger {{ is_enabled|yesno:',d-none' }}">
+        {% blocktrans trimmed with service_name=app_info.name %}
+          Service <em>{{ service_name }}</em> is not running.
+        {% endblocktrans %}
+      </div>
+    {% 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 %}
-        <h3>{% trans "Configuration" %}</h3>
+  {% block configuration %}
+    {% if form %}
+      <h3>{% trans "Configuration" %}</h3>
 
-        <form id="app-form" class="form form-configuration" method="post">
-          {% csrf_token %}
+      <form id="app-form" class="form form-configuration" method="post">
+        {% csrf_token %}
 
-          {{ form|bootstrap }}
+        {{ form|bootstrap }}
 
-          <input type="submit" class="btn btn-primary"
-                value="{% trans "Update setup" %}"/>
-        </form>
-      {% endif %}
-    {% endblock %}
+        <input type="submit" class="btn btn-primary"
+              value="{% trans "Update setup" %}"/>
+      </form>
+    {% endif %}
+  {% endblock %}
 
-    {% block extra_content %}
-    {% endblock %}
-  {% else %}
-    {% include "operations.html" %}
-  {% endif %}
+  {% block extra_content %}
+  {% endblock %}
 
 {% endblock %}

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'