diagnostics: Simplify getting translated description in results

Tests:

- Unit tests pass.

- Run full diagnostics tests and see that results and app name are translated
when language preference is not English.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>

plinth/modules/apache/tests/test_components.py

@@ -12,8 +12,7 @@ from plinth import app
 from plinth.modules.apache.components import (Uwsgi, Webserver, check_url,
                                               diagnose_url,
                                               diagnose_url_on_all)
-from plinth.modules.diagnostics.check import (DiagnosticCheck, Result,
+from plinth.modules.diagnostics.check import DiagnosticCheck, Result
-                                              translate, translate_checks)
 
 
 def test_webserver_init():
@@ -249,16 +248,16 @@ def test_diagnose_url(get_addresses, check):
     }
     parameters = {key: args[key] for key in ['url', 'kind']}
     check.return_value = True
-    result = translate(diagnose_url(**args))
+    result = diagnose_url(**args)
     assert result == DiagnosticCheck(
         'apache-url-kind-https://localhost/test-4',
-        'Access URL https://localhost/test on tcp4', Result.PASSED, parameters)
+        'Access URL {url} on tcp{kind}', Result.PASSED, parameters)
 
     check.return_value = False
-    result = translate(diagnose_url(**args))
+    result = diagnose_url(**args)
     assert result == DiagnosticCheck(
         'apache-url-kind-https://localhost/test-4',
-        'Access URL https://localhost/test on tcp4', Result.FAILED, parameters)
+        'Access URL {url} on tcp{kind}', Result.FAILED, parameters)
 
     del args['kind']
     args['url'] = 'https://{host}/test'
@@ -284,14 +283,14 @@ def test_diagnose_url(get_addresses, check):
             'kind': '6'
         },
     ]
-    results = translate_checks(diagnose_url_on_all(**args))
+    results = diagnose_url_on_all(**args)
     assert results == [
         DiagnosticCheck('apache-url-kind-https://test-host-1/test-4',
-                        'Access URL https://test-host-1/test on tcp4',
+                        'Access URL {url} on tcp{kind}', Result.PASSED,
-                        Result.PASSED, parameters[0]),
+                        parameters[0]),
         DiagnosticCheck('apache-url-kind-https://test-host-2/test-6',
-                        'Access URL https://test-host-2/test on tcp6',
+                        'Access URL {url} on tcp{kind}', Result.PASSED,
-                        Result.PASSED, parameters[1]),
+                        parameters[1]),
     ]
 
 

plinth/modules/diagnostics/__init__.py

@@ -21,7 +21,7 @@ from plinth.modules.apache.components import diagnose_url_on_all
 from plinth.modules.backups.components import BackupRestore
 
 from . import manifest
-from .check import CheckJSONDecoder, CheckJSONEncoder, Result, translate_checks
+from .check import CheckJSONDecoder, CheckJSONEncoder, Result
 
 _description = [
     _('The system diagnostic test will run a number of checks on your '
@@ -338,14 +338,9 @@ def get_results():
         results = json.loads(results, cls=CheckJSONDecoder)
         results = {'results': results, 'progress_percentage': 100}
 
-    # Translate and format diagnostic check descriptions for each app
+    # Add a translated name for each app
     for app_id in results['results']:
         app = app_module.App.get(app_id)
-        app_name = app.info.name or app_id
+        results['results'][app_id]['name'] = app.info.name or app_id
-        results['results'][app_id]['name'] = app_name
-        if 'diagnosis' in results['results'][app_id]:
-            diagnosis = results['results'][app_id]['diagnosis']
-            results['results'][app_id]['diagnosis'] = translate_checks(
-                diagnosis)
 
     return results

plinth/modules/diagnostics/check.py

@@ -2,9 +2,9 @@
 """Diagnostic check data type."""
 
 import dataclasses
+import json
 from dataclasses import dataclass, field
 from enum import StrEnum
-import json
 
 from django.utils.translation import gettext
 
@@ -28,21 +28,14 @@ class DiagnosticCheck:
     result: Result = Result.NOT_DONE
     parameters: dict = field(default_factory=dict)
 
+    @property
+    def translated_description(self):
+        """Return translated string for description."""
+        description = gettext(self.description)
+        if self.parameters:
+            return SafeFormatter().vformat(description, [], self.parameters)
 
-def translate(check: DiagnosticCheck) -> DiagnosticCheck:
+        return description
-    """Translate and format description using parameters."""
-    description = gettext(check.description)
-    if check.parameters:
-        description = SafeFormatter().vformat(description, [],
-                                              check.parameters)
-
-    return DiagnosticCheck(check.check_id, description, check.result,
-                           check.parameters)
-
-
-def translate_checks(checks: list[DiagnosticCheck]) -> list[DiagnosticCheck]:
-    """Translate and format diagnostic checks."""
-    return [translate(check) for check in checks]
 
 
 class CheckJSONEncoder(json.JSONEncoder):

plinth/modules/diagnostics/templates/diagnostics_results.html

@@ -15,7 +15,7 @@
     <tbody>
       {% for result in results %}
         <tr>
-          <td>{{ result.description }}</td>
+          <td>{{ result.translated_description }}</td>
           <td class="diagnostics-result">
             {% if result.result == 'passed' %}
               <span class="badge badge-success">{% trans result.result %}</span>

plinth/modules/diagnostics/tests/test_check.py

@@ -2,12 +2,12 @@
 """Tests for diagnostic check data type."""
 
 import json
+
 import pytest
 
-from plinth.modules.diagnostics.check import (DiagnosticCheck,
+from plinth.modules.diagnostics.check import (CheckJSONDecoder,
                                               CheckJSONEncoder,
-                                              CheckJSONDecoder, Result,
+                                              DiagnosticCheck, Result)
-                                              translate)
 
 
 def test_result():
@@ -29,6 +29,7 @@ def test_diagnostic_check():
     check = DiagnosticCheck('some-check-id', 'sample check')
     assert check.check_id == 'some-check-id'
     assert check.description == 'sample check'
+    assert check.translated_description == 'sample check'
     assert check.result == Result.NOT_DONE
     assert not check.parameters
 
@@ -43,24 +44,13 @@ def test_diagnostic_check():
 
 def test_translate():
     """Test formatting the translated description."""
-    check = DiagnosticCheck('some-check-id', 'sample check', Result.PASSED)
-    translated = translate(check)
-    assert translated.check_id == 'some-check-id'
-    assert translated.description == 'sample check'
-    assert translated.result == Result.PASSED
-    assert not translated.parameters
-
     check = DiagnosticCheck('some-check-id', 'sample check {key}',
                             Result.FAILED, {'key': 'value'})
-    translated = translate(check)
+    assert check.translated_description == 'sample check value'
-    assert translated.description == 'sample check value'
-    assert translated.result == Result.FAILED
-    assert translated.parameters == {'key': 'value'}
 
     check = DiagnosticCheck('some-check-id', 'sample check {missing}',
                             Result.PASSED, {'key': 'value'})
-    translated = translate(check)
+    assert check.translated_description == 'sample check ?missing?'
-    assert translated.description == 'sample check ?missing?'
 
 
 def test_json_encoder_decoder():

plinth/modules/diagnostics/views.py

@@ -17,7 +17,7 @@ from plinth.app import App
 from plinth.modules import diagnostics
 from plinth.views import AppView
 
-from .check import Result, translate_checks
+from .check import Result
 
 logger = logging.getLogger(__name__)
 
@@ -83,8 +83,6 @@ def diagnose_app(request, app_id):
                          exception)
         diagnosis_exception = str(exception)
 
-    # Translate and format diagnostic check descriptions
-    diagnosis = translate_checks(diagnosis)
     show_rerun_setup = False
     for check in diagnosis:
         if check.result in [Result.FAILED, Result.WARNING]:

plinth/modules/firewall/tests/test_components.py

@@ -8,8 +8,7 @@ from unittest.mock import call, patch
 import pytest
 
 from plinth.app import App
-from plinth.modules.diagnostics.check import (DiagnosticCheck, Result,
+from plinth.modules.diagnostics.check import DiagnosticCheck, Result
-                                              translate_checks)
 from plinth.modules.firewall.components import (Firewall,
                                                 FirewallLocalProtection)
 
@@ -154,32 +153,32 @@ def test_diagnose(get_enabled_services, get_port_details):
     get_port_details.side_effect = get_port_details_side_effect
     firewall = Firewall('test-firewall-1', ports=['test-port1', 'test-port2'],
                         is_external=False)
-    results = translate_checks(firewall.diagnose())
+    results = firewall.diagnose()
     assert results == [
         DiagnosticCheck(
             'firewall-port-internal-test-port1',
-            'Port test-port1 (1234/tcp, 1234/udp) available for internal '
+            'Port {name} ({details}) available for internal '
             'networks', Result.PASSED, {
                 'name': 'test-port1',
                 'details': '1234/tcp, 1234/udp'
             }),
         DiagnosticCheck(
             'firewall-port-external-unavailable-test-port1',
-            'Port test-port1 (1234/tcp, 1234/udp) unavailable for external '
+            'Port {name} ({details}) unavailable for external '
             'networks', Result.PASSED, {
                 'name': 'test-port1',
                 'details': '1234/tcp, 1234/udp'
             }),
         DiagnosticCheck(
             'firewall-port-internal-test-port2',
-            'Port test-port2 (2345/udp) available for internal networks',
+            'Port {name} ({details}) available for internal networks',
             Result.FAILED, {
                 'name': 'test-port2',
                 'details': '2345/udp'
             }),
         DiagnosticCheck(
             'firewall-port-external-unavailable-test-port2',
-            'Port test-port2 (2345/udp) unavailable for external networks',
+            'Port {name} ({details}) unavailable for external networks',
             Result.FAILED, {
                 'name': 'test-port2',
                 'details': '2345/udp'
@@ -188,32 +187,32 @@ def test_diagnose(get_enabled_services, get_port_details):
 
     firewall = Firewall('test-firewall-1', ports=['test-port3', 'test-port4'],
                         is_external=True)
-    results = translate_checks(firewall.diagnose())
+    results = firewall.diagnose()
     assert results == [
         DiagnosticCheck(
             'firewall-port-internal-test-port3',
-            'Port test-port3 (3456/tcp) available for internal networks',
+            'Port {name} ({details}) available for internal networks',
             Result.PASSED, {
                 'name': 'test-port3',
                 'details': '3456/tcp'
             }),
         DiagnosticCheck(
             'firewall-port-external-available-test-port3',
-            'Port test-port3 (3456/tcp) available for external networks',
+            'Port {name} ({details}) available for external networks',
             Result.PASSED, {
                 'name': 'test-port3',
                 'details': '3456/tcp'
             }),
         DiagnosticCheck(
             'firewall-port-internal-test-port4',
-            'Port test-port4 (4567/udp) available for internal networks',
+            'Port {name} ({details}) available for internal networks',
             Result.FAILED, {
                 'name': 'test-port4',
                 'details': '4567/udp'
             }),
         DiagnosticCheck(
             'firewall-port-external-available-test-port4',
-            'Port test-port4 (4567/udp) available for external networks',
+            'Port {name} ({details}) available for external networks',
             Result.FAILED, {
                 'name': 'test-port4',
                 'details': '4567/udp'

plinth/tests/test_config.py

@@ -9,8 +9,7 @@ import pytest
 
 from plinth.app import App
 from plinth.config import DropinConfigs
-from plinth.modules.diagnostics.check import (DiagnosticCheck, Result,
+from plinth.modules.diagnostics.check import DiagnosticCheck, Result
-                                              translate_checks)
 
 pytestmark = pytest.mark.usefixtures('mock_privileged')
 privileged_modules_to_mock = ['plinth.privileged.config']
@@ -161,16 +160,15 @@ def test_dropin_config_diagnose_symlinks(dropin_configs, tmp_path):
     """Test diagnosing dropin configs for symlinks."""
     with patch('plinth.config.DropinConfigs.ROOT', new=tmp_path):
         # Nothing exists
-        results = translate_checks(dropin_configs.diagnose())
+        results = dropin_configs.diagnose()
         assert results == [
             DiagnosticCheck(
                 f'dropin-config-{tmp_path}/etc/test/path1',
-                f'Static configuration {tmp_path}/etc/test/path1 is setup '
+                'Static configuration {etc_path} is setup properly',
-                'properly', Result.FAILED,
+                Result.FAILED, {'etc_path': f'{tmp_path}/etc/test/path1'}),
-                {'etc_path': f'{tmp_path}/etc/test/path1'}),
             DiagnosticCheck(
                 f'dropin-config-{tmp_path}/etc/path2',
-                f'Static configuration {tmp_path}/etc/path2 is setup properly',
+                'Static configuration {etc_path} is setup properly',
                 Result.FAILED, {'etc_path': f'{tmp_path}/etc/path2'}),
         ]
 

plinth/tests/test_daemon.py

@@ -12,8 +12,7 @@ import pytest
 from plinth.app import App, FollowerComponent, Info
 from plinth.daemon import (Daemon, RelatedDaemon, app_is_running,
                            diagnose_netcat, diagnose_port_listening)
-from plinth.modules.diagnostics.check import (DiagnosticCheck, Result,
+from plinth.modules.diagnostics.check import DiagnosticCheck, Result
-                                              translate, translate_checks)
 
 privileged_modules_to_mock = ['plinth.privileged.service']
 
@@ -149,10 +148,10 @@ def test_diagnose(port_listening, service_is_running, daemon):
                                                               (345, 'udp')])
     port_listening.side_effect = side_effect
     service_is_running.return_value = True
-    results = translate_checks(daemon.diagnose())
+    results = daemon.diagnose()
     assert results == [
         DiagnosticCheck('daemon-running-test-unit',
-                        'Service test-unit is running', Result.PASSED,
+                        'Service {service_name} is running', Result.PASSED,
                         {'service_name': 'test-unit'}),
         DiagnosticCheck('test-result-8273-tcp4', 'test-result-8273-tcp4',
                         Result.PASSED),
@@ -215,34 +214,34 @@ def test_diagnose_port_listening(connections):
     ]
 
     # Check that message is correct
-    results = translate(diagnose_port_listening(1234))
+    results = diagnose_port_listening(1234)
     assert results == DiagnosticCheck('daemon-listening-tcp-1234',
-                                      'Listening on tcp port 1234',
+                                      'Listening on {kind} port {port}',
                                       Result.PASSED, {
                                           'kind': 'tcp',
                                           'port': 1234
                                       })
-    results = translate(diagnose_port_listening(1234, 'tcp', '0.0.0.0'))
+    results = diagnose_port_listening(1234, 'tcp', '0.0.0.0')
     assert results == DiagnosticCheck(
         'daemon-listening-address-tcp-1234-0.0.0.0',
-        'Listening on tcp port 0.0.0.0:1234', Result.PASSED, {
+        'Listening on {kind} port {listen_address}:{port}', Result.PASSED, {
             'kind': 'tcp',
             'port': 1234,
             'listen_address': '0.0.0.0'
         })
 
     # Failed results
-    results = translate(diagnose_port_listening(4321))
+    results = diagnose_port_listening(4321)
     assert results == DiagnosticCheck('daemon-listening-tcp-4321',
-                                      'Listening on tcp port 4321',
+                                      'Listening on {kind} port {port}',
                                       Result.FAILED, {
                                           'kind': 'tcp',
                                           'port': 4321
                                       })
-    results = translate(diagnose_port_listening(4321, 'tcp', '0.0.0.0'))
+    results = diagnose_port_listening(4321, 'tcp', '0.0.0.0')
     assert results == DiagnosticCheck(
         'daemon-listening-address-tcp-4321-0.0.0.0',
-        'Listening on tcp port 0.0.0.0:4321', Result.FAILED, {
+        'Listening on {kind} port {listen_address}:{port}', Result.FAILED, {
             'kind': 'tcp',
             'port': 4321,
             'listen_address': '0.0.0.0'

plinth/tests/test_package.py

@@ -10,8 +10,7 @@ import pytest
 
 from plinth.app import App
 from plinth.errors import MissingPackageError
-from plinth.modules.diagnostics.check import (DiagnosticCheck, Result,
+from plinth.modules.diagnostics.check import DiagnosticCheck, Result
-                                              translate_checks)
 from plinth.package import Package, Packages, packages_installed
 
 
@@ -239,33 +238,37 @@ def test_diagnose(cache):
         Package('package4') | Package('package5'),
         Package('package6') | Package('package7')
     ])
-    results = translate_checks(component.diagnose())
+    results = component.diagnose()
     assert results == [
-        DiagnosticCheck('package-available-package1',
+        DiagnosticCheck(
-                        'Package package1 is not available for install',
+            'package-available-package1',
-                        Result.FAILED, {'package_expression': 'package1'}),
+            'Package {package_expression} is not available for install',
-        DiagnosticCheck('package-latest-package2',
+            Result.FAILED, {'package_expression': 'package1'}),
-                        'Package package2 is the latest version (2.0)',
+        DiagnosticCheck(
-                        Result.PASSED, {
+            'package-latest-package2',
-                            'package_name': 'package2',
+            'Package {package_name} is the latest version ({latest_version})',
-                            'latest_version': '2.0'
+            Result.PASSED, {
-                        }),
+                'package_name': 'package2',
-        DiagnosticCheck('package-latest-package3',
+                'latest_version': '2.0'
-                        'Package package3 is the latest version (3.0)',
+            }),
-                        Result.WARNING, {
+        DiagnosticCheck(
-                            'package_name': 'package3',
+            'package-latest-package3',
-                            'latest_version': '3.0'
+            'Package {package_name} is the latest version ({latest_version})',
-                        }),
+            Result.WARNING, {
+                'package_name': 'package3',
+                'latest_version': '3.0'
+            }),
         DiagnosticCheck(
             'package-available-package4 | package5',
-            'Package package4 | package5 is not available for install',
+            'Package {package_expression} is not available for install',
             Result.FAILED, {'package_expression': 'package4 | package5'}),
-        DiagnosticCheck('package-latest-package7',
+        DiagnosticCheck(
-                        'Package package7 is the latest version (4.0)',
+            'package-latest-package7',
-                        Result.PASSED, {
+            'Package {package_name} is the latest version ({latest_version})',
-                            'package_name': 'package7',
+            Result.PASSED, {
-                            'latest_version': '4.0'
+                'package_name': 'package7',
-                        }),
+                'latest_version': '4.0'
+            }),
     ]