From 632f5e15bdd83c46bce1833e522b830b049be335 Mon Sep 17 00:00:00 2001 From: catborise Date: Sun, 25 Oct 2026 21:50:27 +0300 Subject: [PATCH] feat(appsettings): the Settings page groups its settings and saves a group at once The page listed 37 settings by name, each its own form saved as soon as its list or field changed, with the description only in a tooltip. It now shows them in groups (New VM defaults, Disks, Clones, Console, VM access and owners, Snapshots and jobs, Login, Lists and pages) with each description under its field and the label tied to it. A group's Save button saves its changed settings, or none when one is refused, and says which were saved; number settings are number fields with their range. Language and theme are under Appearance with their own buttons. A setting in no group shows under Other. --- appsettings/templates/appsettings.html | 106 ++++++++++++++----------- appsettings/tests/test_appsettings.py | 42 ++++++++++ appsettings/views.py | 85 +++++++++++++------- 3 files changed, 157 insertions(+), 76 deletions(-) diff --git a/appsettings/templates/appsettings.html b/appsettings/templates/appsettings.html index 6e60a7fd..af4748f3 100644 --- a/appsettings/templates/appsettings.html +++ b/appsettings/templates/appsettings.html @@ -6,60 +6,70 @@ {% block page_heading %}{% trans "Edit Settings" %}{% endblock page_heading %} {% block content %} -
-
-

{% trans "App Settings" %}

-
{% csrf_token %} -
- - +
+
+

{% trans "Appearance" %}

+ {% csrf_token %} + +
+ +
{% trans "Yours only, in this browser" %}
+
+
+ +
{% csrf_token %} + +
+ +
{% trans "For every user" %}
+
+
+
+
+
+ +{% for title, settings in groups %} +
+
+
{% csrf_token %} +

{% trans title %}

+ {% for setting in settings %} +
+
- + {% for choice in setting.choices_as_list %} + {% endfor %} + {% elif setting.bounds %} + + {% else %} + + {% endif %} +
{% trans setting.description %}
-
- {% if request.user.is_superuser %} -
{% csrf_token %} -
- -
- -
+ {% endfor %} +
+
- {% endif %} -

{% trans "Other Settings" %}

- {% for setting in appsettings %} -
{% csrf_token %} -
- -
- {% if setting.choices %} - - {% else %} - - {% endif%} -
-
-
- {% endfor %}
-
+
+{% endfor %} {% endblock %} diff --git a/appsettings/tests/test_appsettings.py b/appsettings/tests/test_appsettings.py index 52b34cab..f15390b7 100644 --- a/appsettings/tests/test_appsettings.py +++ b/appsettings/tests/test_appsettings.py @@ -83,3 +83,45 @@ class AppSettingsTestCase(TestCase): page = self.client.get(reverse("appsettings")).content.decode() self.assertIn('name="CLONE_INSTANCE_DEFAULT_PREFIX" value="web-"', page) + def page(self): + from lxml import html + + return html.fromstring(self.client.get(reverse("appsettings")).content) + + def test_settings_are_grouped_with_their_descriptions_shown(self): + page = self.page() + titles = [h.text_content().strip() for h in page.xpath("//form//h2")] + self.assertEqual(titles[:3], ["New VM defaults", "Disks", "Clones"]) + # every setting is in one group, with a label tied to its field and its description shown + keys = page.xpath("//form//*[self::input or self::select][@name and @name!='csrfmiddlewaretoken']/@name") + names = set(AppSettings.objects.exclude(key="BOOTSTRAP_THEME").values_list("key", flat=True)) + self.assertEqual(sorted(k for k in keys if k not in ("BOOTSTRAP_THEME", "language")), sorted(names)) + (field,) = page.xpath("//*[@name='LOGS_PER_PAGE']") + self.assertEqual((field.get("type"), field.get("min"), field.get("max")), ("number", "1", "1000")) + self.assertEqual(page.xpath(f"//label[@for='{field.get('id')}']/text()"), ["Logs per Page"]) + self.assertEqual(page.xpath(f"//*[@id='{field.get('aria-describedby')}']/text()"), ["Pagination for logs"]) + # a group is saved with its button, not on every change + self.assertEqual(page.xpath("//*[@onchange]"), []) + + def test_a_group_saves_its_changed_settings_only(self): + from logs.models import Logs + + Logs.objects.all().delete() + response = self.client.post(reverse("appsettings"), { + "CONSOLE_SCALE": "True", "CONSOLE_VIEW_ONLY": "False", + "CONSOLE_RESIZE_SESSION": "False", "CONSOLE_CLIP_VIEWPORT": "True"}) + self.assertRedirects(response, reverse("appsettings")) + values = dict(AppSettings.objects.filter(key__startswith="CONSOLE_").values_list("key", "value")) + self.assertEqual(values, {"CONSOLE_SCALE": "True", "CONSOLE_VIEW_ONLY": "False", + "CONSOLE_RESIZE_SESSION": "False", "CONSOLE_CLIP_VIEWPORT": "True"}) + self.assertEqual(sorted(Logs.objects.values_list("message", flat=True)), + ["Setting Console Clip Viewport: change to True", "Setting Console Scale: change to True"]) + self.assertEqual([str(m) for m in get_messages(response.wsgi_request)], + ["Saved: Console Clip Viewport, Console Scale"]) + + def test_one_bad_value_saves_nothing_of_its_group(self): + response = self.client.post(reverse("appsettings"), {"LOGIN_FAILURE_LIMIT": "10", "LOGIN_LOCKOUT_MINUTES": "x"}) + self.assertEqual(AppSettings.objects.get(key="LOGIN_FAILURE_LIMIT").value, "5") + self.assertEqual([str(m) for m in get_messages(response.wsgi_request)], + ["Login Lockout Minutes must be a whole number between 1 and 10080"]) + diff --git a/appsettings/views.py b/appsettings/views.py index 290ee934..b0f5e773 100644 --- a/appsettings/views.py +++ b/appsettings/views.py @@ -3,6 +3,7 @@ from pathlib import Path from admin.decorators import superuser_only from django.conf import settings from django.contrib import messages +from django.db import DatabaseError from django.http import HttpResponseRedirect from django.shortcuts import render from django.utils.translation import gettext_noop as _ @@ -29,8 +30,32 @@ NUMBERS = { } +# the Settings page's groups, in order; a setting in none shows under Other +GROUPS = [ + (_("New VM defaults"), ["INSTANCE_ARCH_DEFAULT_TYPE", "INSTANCE_MACHINE_DEFAULT_TYPE", + "INSTANCE_FIRMWARE_DEFAULT_TYPE", "INSTANCE_CPU_DEFAULT_MODE", "INSTANCE_NIC_DEFAULT_TYPE", + "INSTANCE_VIDEO_DEFAULT_TYPE", "INSTANCE_INPUT_DEFAULT_DEVICE", "INSTANCE_CDROM_ADD"]), + (_("Disks"), ["INSTANCE_VOLUME_DEFAULT_BUS", "INSTANCE_VOLUME_DEFAULT_FORMAT", "INSTANCE_VOLUME_DEFAULT_CACHE", + "INSTANCE_VOLUME_DEFAULT_IO", "INSTANCE_VOLUME_DEFAULT_DISCARD", + "INSTANCE_VOLUME_DEFAULT_DETECT_ZEROES", "INSTANCE_VOLUME_DEFAULT_SCSI_CONTROLLER", + "INSTANCE_VOLUME_DEFAULT_OWNER_UID", "INSTANCE_VOLUME_DEFAULT_OWNER_GID"]), + (_("Clones"), ["CLONE_INSTANCE_AUTO_NAME", "CLONE_INSTANCE_DEFAULT_PREFIX"]), + (_("Console"), ["CONSOLE_SCALE", "CONSOLE_VIEW_ONLY", "CONSOLE_RESIZE_SESSION", "CONSOLE_CLIP_VIEWPORT"]), + (_("VM access and owners"), ["SHOW_ACCESS_ROOT_PASSWORD", "SHOW_ACCESS_SSH_KEYS", + "ALLOW_INSTANCE_MULTIPLE_OWNER", "QUOTA_DEBUG"]), + (_("Snapshots and jobs"), ["SNAPSHOT_FREE_SPACE_PERCENT", "SNAPSHOT_FREE_SPACE_MIN_GIB", + "WORKER_CONCURRENCY", "JOB_RETENTION_DAYS"]), + (_("Login"), ["LOGIN_FAILURE_LIMIT", "LOGIN_LOCKOUT_MINUTES"]), + (_("Lists and pages"), ["VIEW_INSTANCES_LIST_STYLE", "VIEW_INSTANCE_DETAIL_BOTTOM_BAR", "VM_DRBD_STATUS", + "LOGS_PER_PAGE"]), +] + + def refusal(setting, value): """Why the value cannot be saved for the setting, or None.""" + if len(value) > AppSettings._meta.get_field("value").max_length: + return _("%(setting)s takes at most %(length)s characters") % { + "setting": setting.name, "length": AppSettings._meta.get_field("value").max_length} if setting.key in NUMBERS: low, high = NUMBERS[setting.key] if not (value.isascii() and value.isdigit() and len(value) <= 10 and low <= int(value) <= high): @@ -44,17 +69,12 @@ def refusal(setting, value): @superuser_only def appsettings(request): - """ - :param request: - :return: - """ + """The settings, in groups; a group's form saves its changed settings, + or none of them when one is refused.""" bootstrap_theme = AppSettings.objects.get(key="BOOTSTRAP_THEME") themes_list = sorted(p.name.removesuffix(".min.css") for p in THEMES_DIR.glob("*.min.css")) - - # the theme has its own form; the other settings are listed one by one - appsettings = AppSettings.objects.exclude( - description__startswith="Bootstrap" - ).order_by("name") + # the theme has its own form + settings_by_key = {s.key: s for s in AppSettings.objects.exclude(key="BOOTSTRAP_THEME")} if request.method == "POST": if "BOOTSTRAP_THEME" in request.POST: @@ -70,27 +90,36 @@ def appsettings(request): messages.success(request, msg) return HttpResponseRedirect(request.get_full_path()) - for setting in appsettings: - if setting.key in request.POST: - value = request.POST.get(setting.key, "") - refused = refusal(setting, value) - if refused: - messages.error(request, refused) - return HttpResponseRedirect(request.get_full_path()) - try: + posted = sorted(((s, request.POST[s.key]) for s in settings_by_key.values() if s.key in request.POST), + key=lambda item: item[0].name) + refusals = [refused for refused in (refusal(s, value) for s, value in posted) if refused] + for refused in refusals: + messages.error(request, refused) + if not refusals: + saved = [] + try: + for setting, value in posted: if value != setting.value: with audit(request, None, f"Setting {setting.name}", f"change to {value}"): setting.value = value setting.save() + saved.append(setting.name) + except DatabaseError as err: # the audit row records it as failed + messages.error(request, err) + else: + if saved: + messages.success(request, _("Saved: %(settings)s") % {"settings": ", ".join(saved)}) + else: + messages.info(request, _("Nothing changed")) + return HttpResponseRedirect(request.get_full_path()) - msg = _("%(setting)s is changed. Now: %(value)s") % { - "setting": setting.name, - "value": setting.value, - } - messages.success(request, msg) - except Exception as err: - msg = err - messages.error(request, msg) - return HttpResponseRedirect(request.get_full_path()) - - return render(request, "appsettings.html", locals()) + groups = [] + for title, keys in GROUPS: + groups.append((title, [settings_by_key.pop(key) for key in keys if key in settings_by_key])) + if settings_by_key: + groups.append((_("Other"), sorted(settings_by_key.values(), key=lambda s: s.name))) + for _title, settings_list in groups: + for setting in settings_list: + setting.bounds = NUMBERS.get(setting.key) + return render(request, "appsettings.html", + {"bootstrap_theme": bootstrap_theme, "themes_list": themes_list, "groups": groups})