From 3e466fb011a94d4348fa7cfd4921b5768c67afed Mon Sep 17 00:00:00 2001
From: Tobias Kunze <r@rixx.de>
Date: Sun, 25 May 2025 22:55:37 +0200
Subject: [PATCH] Instance form and template improvements

---
 src/servala/core/models/service.py            | 12 ++++--
 src/servala/frontend/forms/service.py         | 29 +++++++++------
 .../service_instance_delete_form.html         | 11 +++++-
 .../service_instance_detail.html              | 37 ++++++++++++-------
 src/servala/frontend/views/service.py         | 37 +++++++++++++------
 5 files changed, 82 insertions(+), 44 deletions(-)

diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py
index 4824c47..5540d22 100644
--- a/src/servala/core/models/service.py
+++ b/src/servala/core/models/service.py
@@ -374,9 +374,11 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model):
 
     @cached_property
     def kind_plural(self):
-        if hasattr(self.resource_definition, 'status') and \
-           hasattr(self.resource_definition.status, 'accepted_names') and \
-           self.resource_definition.status.accepted_names:
+        if (
+            hasattr(self.resource_definition, "status")
+            and hasattr(self.resource_definition.status, "accepted_names")
+            and self.resource_definition.status.accepted_names
+        ):
             return self.resource_definition.status.accepted_names.plural
         if self.kind.endswith("s"):
             return self.kind.lower()
@@ -648,7 +650,9 @@ class ServiceInstance(ServalaModelMixin, models.Model):
         self.is_deleted = True
         self.deleted_at = timezone.now()
         self.deleted_by = user
-        self.save(update_fields=["is_deleted", "deleted_at", "deleted_by", "updated_at"])
+        self.save(
+            update_fields=["is_deleted", "deleted_at", "deleted_by", "updated_at"]
+        )
 
         self._clear_kubernetes_caches()
 
diff --git a/src/servala/frontend/forms/service.py b/src/servala/frontend/forms/service.py
index 09833d9..04fe2df 100644
--- a/src/servala/frontend/forms/service.py
+++ b/src/servala/frontend/forms/service.py
@@ -6,6 +6,7 @@ from servala.core.models import (
     ControlPlane,
     Service,
     ServiceCategory,
+    ServiceInstance,
     ServiceOffering,
 )
 
@@ -37,6 +38,8 @@ class ControlPlaneSelectForm(forms.Form):
     def __init__(self, *args, planes=None, **kwargs):
         super().__init__(*args, **kwargs)
         self.fields["control_plane"].queryset = planes
+        if planes and planes.count() == 1:
+            self.fields["control_plane"].initial = planes.first()
 
 
 class ServiceInstanceFilterForm(forms.Form):
@@ -68,23 +71,23 @@ class ServiceInstanceFilterForm(forms.Form):
     def filter_queryset(self, queryset):
         if self.is_valid():
             data = self.cleaned_data
-            if data["name"]:
+            if data.get("name"):
                 queryset = queryset.filter(name__icontains=data["name"])
-            if data["service"]:
+            if data.get("service"):
                 queryset = queryset.filter(
                     context__service_definition__service=data["service"]
                 )
-            if data["provider"]:
+            if data.get("provider"):
                 queryset = queryset.filter(
                     context__service_offering__provider=data["provider"]
                 )
-            if data["control_plane"]:
+            if data.get("control_plane"):
                 queryset = queryset.filter(context__control_plane=data["control_plane"])
-            if data["status"]:
-                if data["status"] == "active":
-                    queryset = queryset.filter(is_deleted=False)
-                else:
-                    queryset = queryset.filter(is_deleted=True)
+            status = data.get("status")
+            if status == "active":
+                queryset = queryset.filter(is_deleted=False)
+            elif status == "deleted":
+                queryset = queryset.filter(is_deleted=True)
         return queryset
 
 
@@ -105,11 +108,13 @@ class ServiceInstanceDeleteForm(forms.ModelForm):
     def clean_name(self):
         entered_name = self.cleaned_data.get("name")
         if entered_name != self.instance.name:
-            raise ValidationError(
-                _("The entered name does not match the instance name. Deletion not confirmed.")
+            raise forms.ValidationError(
+                _(
+                    "The entered name does not match the instance name. Deletion not confirmed."
+                )
             )
         return entered_name
 
     class Meta:
         model = ServiceInstance
-        fields = ("name", )
+        fields = ("name",)
diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html
index 08d4167..5296c56 100644
--- a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html
+++ b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html
@@ -1,11 +1,18 @@
 {% load i18n %}
-<form method="post" action="{{ view.request.path }}" hx-post="{{ view.request.path }}" hx-target="this" hx-swap="outerHTML">
+<form method="post"
+      action="{{ view.request.path }}"
+      hx-post="{{ view.request.path }}"
+      hx-target="this"
+      hx-swap="outerHTML">
     {% csrf_token %}
     <div class="modal-header">
         <h5 class="modal-title" id="deleteInstanceModalLabel">
             {% blocktranslate with instance_name=instance.name %}Confirm Deletion of {{ instance_name }}{% endblocktranslate %}
         </h5>
-        <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
+        <button type="button"
+                class="btn-close"
+                data-bs-dismiss="modal"
+                aria-label="Close"></button>
     </div>
     <div class="modal-body hide-form-errors">
         <p>{% translate "Do you really want to delete this service instance? This action cannot be undone." %}</p>
diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html b/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html
index 5d6779c..4c8ebf1 100644
--- a/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html
+++ b/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html
@@ -1,5 +1,5 @@
 {% extends "frontend/base.html" %}
-{% load i18n static %}
+{% load i18n static pprint_filters %}
 {% block html_title %}
     {% block page_title %}
         {{ instance.name }}
@@ -11,13 +11,12 @@
             <a href="{{ instance.urls.update }}" class="btn btn-primary me-1 mb-1">{% translate "Edit" %}</a>
         {% endif %}
         {% if has_delete_permission and not instance.is_deleted %}
-            <button type="button" class="btn btn-danger me-1 mb-1"
+            <button type="button"
+                    class="btn btn-danger me-1 mb-1"
                     hx-get="{{ instance.urls.delete }}"
                     hx-target="#deleteInstanceModalContent"
                     data-bs-toggle="modal"
-                    data-bs-target="#deleteInstanceModal">
-                {% translate "Delete" %}
-            </button>
+                    data-bs-target="#deleteInstanceModal">{% translate "Delete" %}</button>
         {% endif %}
     </div>
 {% endblock page_title_extra %}
@@ -45,7 +44,7 @@
                             </dd>
                             <dt class="col-sm-4">{% translate "Created By" %}</dt>
                             <dd class="col-sm-8">
-                                {{ instance.created_by }}
+                                {{ instance.created_by|default:"-" }}
                             </dd>
                             <dt class="col-sm-4">{% translate "Created At" %}</dt>
                             <dd class="col-sm-8">
@@ -106,9 +105,9 @@
                                                             <span class="badge text-bg-secondary">{{ condition.status }}</span>
                                                         {% endif %}
                                                     </td>
-                                                    <td>{{ condition.lastTransitionTime }}</td>
-                                                    <td>{{ condition.reason }}</td>
-                                                    <td>{{ condition.message }}</td>
+                                                    <td>{{ condition.lastTransitionTime|date:"SHORT_DATETIME_FORMAT" }}</td>
+                                                    <td>{{ condition.reason|default:"-" }}</td>
+                                                    <td>{{ condition.message|truncatewords:20|default:"-" }}</td>
                                                 </tr>
                                             {% endfor %}
                                         </tbody>
@@ -137,6 +136,8 @@
                                                    data-bs-toggle="tab"
                                                    role="tab">{{ fieldset.title }}</a>
                                             </li>
+                                        {% empty %}
+                                            <li class="nav-item ms-2">{% translate "No specification details available." %}</li>
                                         {% endfor %}
                                     </ul>
                                     <!-- Tab Content -->
@@ -153,7 +154,7 @@
                                                             {% if field.value|default:""|stringformat:"s"|slice:":1" == "{" or field.value|default:""|stringformat:"s"|slice:":1" == "[" %}
                                                                 <pre>{{ field.value|pprint }}</pre>
                                                             {% else %}
-                                                                {{ field.value }}
+                                                                {{ field.value|default:"-" }}
                                                             {% endif %}
                                                         </dd>
                                                     {% endfor %}
@@ -168,13 +169,15 @@
                                                                 {% if field.value|default:""|stringformat:"s"|slice:":1" == "{" or field.value|default:""|stringformat:"s"|slice:":1" == "[" %}
                                                                     <pre>{{ field.value|pprint }}</pre>
                                                                 {% else %}
-                                                                    {{ field.value }}
+                                                                    {{ field.value|default:"-" }}
                                                                 {% endif %}
                                                             </dd>
                                                         {% endfor %}
                                                     </dl>
                                                 {% endfor %}
                                             </div>
+                                        {% empty %}
+                                            <p>{% translate "No specification details to display." %}</p>
                                         {% endfor %}
                                     </div>
                                 </div>
@@ -218,15 +221,21 @@
             {% endif %}
         </div>
     </section>
-
     <!-- Delete Confirmation Modal -->
-    <div class="modal fade" id="deleteInstanceModal" tabindex="-1" aria-labelledby="deleteInstanceModalLabel" aria-hidden="true">
+    <div class="modal fade"
+         id="deleteInstanceModal"
+         tabindex="-1"
+         aria-labelledby="deleteInstanceModalLabel"
+         aria-hidden="true">
         <div class="modal-dialog modal-dialog-centered">
             <div class="modal-content" id="deleteInstanceModalContent">
                 {# Content will be loaded here by HTMX #}
                 <div class="modal-header">
                     <h5 class="modal-title" id="deleteInstanceModalLabel">{% translate "Confirm Deletion" %}</h5>
-                    <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
+                    <button type="button"
+                            class="btn-close"
+                            data-bs-dismiss="modal"
+                            aria-label="Close"></button>
                 </div>
                 <div class="modal-body">
                     <div class="d-flex justify-content-center">
diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py
index 89b3f3c..9cf90df 100644
--- a/src/servala/frontend/views/service.py
+++ b/src/servala/frontend/views/service.py
@@ -1,9 +1,10 @@
 from django.contrib import messages
 from django.core.exceptions import ValidationError
+from django.http import HttpResponse
 from django.shortcuts import redirect
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
-from django.views.generic import DetailView, UpdateView, ListView
+from django.views.generic import DetailView, ListView, UpdateView
 
 from servala.core.crd import deslugify
 from servala.core.models import (
@@ -35,10 +36,14 @@ class ServiceListView(OrganizationViewMixin, ListView):
 
     def get_queryset(self):
         """Return all services."""
-        services = Service.objects.all().select_related("category")
+        services = (
+            Service.objects.all()
+            .select_related("category")
+            .prefetch_related("offerings__provider")
+        )
         if self.filter_form.is_valid():
             services = self.filter_form.filter_queryset(services)
-        return services
+        return services.distinct()
 
     @cached_property
     def filter_form(self):
@@ -93,7 +98,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
 
     @cached_property
     def selected_plane(self):
-        if self.select_form.data and self.select_form.is_valid():
+        if self.select_form.is_valid() and self.select_form.cleaned_data:
             return self.select_form.cleaned_data["control_plane"]
         field = self.select_form.fields["control_plane"]
         return field.initial or field.queryset.first()
@@ -111,7 +116,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
         ).first()
 
     def get_instance_form(self):
-        if not self.context_object:
+        if not self.context_object or not self.context_object.model_form_class:
             return None
         return self.context_object.model_form_class(
             data=self.request.POST if self.request.method == "POST" else None,
@@ -208,7 +213,11 @@ class ServiceInstanceDetailView(
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
-        if not self.object.is_deleted and self.object.kubernetes_object and self.object.spec:
+        if (
+            not self.object.is_deleted
+            and self.object.kubernetes_object
+            and self.object.spec
+        ):
             context["spec_fieldsets"] = self.get_nested_spec()
         context["has_change_permission"] = self.request.user.has_perm(
             ServiceInstance.get_perm("change"), self.object
@@ -383,11 +392,15 @@ class ServiceInstanceListView(OrganizationViewMixin, ListView):
         )
         if self.filter_form.is_valid():
             queryset = self.filter_form.filter_queryset(queryset)
-        status_filter = self.filter_form.cleaned_data.get("status") if self.filter_form.is_valid() else "active"
+        status_filter = (
+            self.filter_form.cleaned_data.get("status")
+            if self.filter_form.is_valid()
+            else "active"
+        )
         if status_filter == "active":
-             queryset = queryset.filter(is_deleted=False)
+            queryset = queryset.filter(is_deleted=False)
         elif status_filter == "deleted":
-             queryset = queryset.filter(is_deleted=True)
+            queryset = queryset.filter(is_deleted=True)
         return queryset.order_by("-created_at")
 
     def get_context_data(self, **kwargs):
@@ -419,9 +432,9 @@ class ServiceInstanceDeleteView(
         except Exception as e:
             messages.error(
                 self.request,
-                _("An error occurred while trying to delete instance '{name}': {error}").format(
-                    name=self.object.name, error=str(e)
-                ),
+                _(
+                    "An error occurred while trying to delete instance '{name}': {error}"
+                ).format(name=self.object.name, error=str(e)),
             )
             response = HttpResponse()
             response["HX-Redirect"] = str(self.object.urls.base)