Nudge test coverage up to 60%

Reviewed-on: #332
Reviewed-by: Tobias Brunner <tobias.brunner@vshn.ch>

src/servala/core/models/service.py

@@ -1,4 +1,5 @@
 import copy
+import hashlib
 import html
 import json
 import re
@@ -18,10 +19,6 @@ from encrypted_fields.fields import EncryptedJSONField
 from kubernetes import client, config
 from kubernetes.client.rest import ApiException
 
-import hashlib
-
-from django.conf import settings
-
 from servala.core import rules as perms
 from servala.core.models.mixins import ServalaModelMixin
 from servala.core.validators import kubernetes_name_validator

src/servala/frontend/templates/frontend/organizations/service_instance_create.html

@@ -26,7 +26,8 @@
                     {% translate "Oops! Something went wrong with the service form generation. Please try again later." %}
                 </div>
             {% else %}
-                {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form hide_form_errors=True %}
+                {% translate "Create" as create_label %}
+                {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form form_submit_label=create_label hide_form_errors=True %}
             {% endif %}
         </div>
     </div>

src/servala/frontend/templates/frontend/organizations/service_instance_update.html

@@ -22,7 +22,8 @@
                     {% translate "Oops! Something went wrong with the service form generation. Please try again later." %}
                 </div>
             {% else %}
-                {% include "includes/tabbed_fieldset_form.html" with form=custom_form expert_form=form hide_form_errors=True %}
+                {% translate "Update" as update_label %}
+                {% include "includes/tabbed_fieldset_form.html" with form=custom_form expert_form=form form_submit_label=update_label hide_form_errors=True %}
             {% endif %}
         </div>
     </div>

src/servala/frontend/templates/frontend/organizations/service_instances.html

@@ -36,7 +36,9 @@
                                     <td>
                                         <a href="{{ instance.urls.base }}">{{ instance.display_name }}</a>
                                     </td>
-                                    <td><code>{{ instance.name }}</code></td>
+                                    <td>
+                                        <code>{{ instance.name }}</code>
+                                    </td>
                                     <td>{{ instance.context.service_definition.service.name }}</td>
                                     <td>{{ instance.context.service_offering.provider.name }}</td>
                                     <td>{{ instance.context.control_plane.name }}</td>

src/servala/frontend/views/service.py

@@ -604,6 +604,9 @@ class ServiceInstanceUpdateView(
                 current_spec = dict(self.object.spec) if self.object.spec else {}
                 spec_data = self._deep_merge(current_spec, spec_data)
 
+            if display_name := form_data.get("display_name"):
+                self.object.display_name = display_name
+
             compute_plan_assignment = None
             if self.plan_form.is_valid():
                 compute_plan_assignment = self.plan_form.cleaned_data.get(

src/tests/conftest.py

@@ -18,6 +18,7 @@ from servala.core.models.service import (
     Service,
     ServiceCategory,
     ServiceDefinition,
+    ServiceInstance,
     ServiceOffering,
 )
 
@@ -42,11 +43,6 @@ def other_organization(origin):
     return Organization.objects.create(name="Test Org Alternate", origin=origin)
 
 
-@pytest.fixture
-def user():
-    return User.objects.create(email="testuser@example.org", password="test")
-
-
 @pytest.fixture
 def org_owner(organization):
     owner = User.objects.create(email="owner@example.org", password="example")
@@ -80,7 +76,7 @@ def user():
 
 
 @pytest.fixture
-def test_service_category():
+def service_category():
     return ServiceCategory.objects.create(
         name="Databases",
         description="Database services",
@@ -88,18 +84,18 @@ def test_service_category():
 
 
 @pytest.fixture
-def test_service(test_service_category):
+def service(service_category):
     return Service.objects.create(
         name="Redis",
         slug="redis",
-        category=test_service_category,
+        category=service_category,
         description="Redis database service",
         osb_service_id="test-service-123",
     )
 
 
 @pytest.fixture
-def test_cloud_provider():
+def cloud_provider():
     return CloudProvider.objects.create(
         name="Exoscale",
         description="Exoscale cloud provider",
@@ -107,10 +103,10 @@ def test_cloud_provider():
 
 
 @pytest.fixture
-def test_service_offering(test_service, test_cloud_provider):
+def service_offering(service, cloud_provider):
     return ServiceOffering.objects.create(
-        service=test_service,
-        provider=test_cloud_provider,
+        service=service,
+        provider=cloud_provider,
         description="Redis on Exoscale",
         osb_plan_id="test-plan-123",
     )
@@ -153,11 +149,11 @@ def mock_odoo_failure(mocker):
 
 
 @pytest.fixture
-def test_control_plane(test_cloud_provider):
+def control_plane(cloud_provider):
     return ControlPlane.objects.create(
         name="Geneva (CH-GVA-2)",
         description="Geneva control plane",
-        cloud_provider=test_cloud_provider,
+        cloud_provider=cloud_provider,
         api_credentials={
             "server": "https://k8s.example.com",
             "token": "test-token",
@@ -167,10 +163,10 @@ def test_control_plane(test_cloud_provider):
 
 
 @pytest.fixture
-def test_service_definition(test_service):
+def service_definition(service):
     return ServiceDefinition.objects.create(
         name="Redis Standard",
-        service=test_service,
+        service=service,
         api_definition={
             "group": "vshn.appcat.vshn.io",
             "version": "v1",
@@ -180,13 +176,11 @@ def test_service_definition(test_service):
 
 
 @pytest.fixture
-def test_control_plane_crd(
-    test_service_offering, test_control_plane, test_service_definition
-):
+def control_plane_crd(service_offering, control_plane, service_definition):
     return ControlPlaneCRD.objects.create(
-        service_offering=test_service_offering,
-        control_plane=test_control_plane,
-        service_definition=test_service_definition,
+        service_offering=service_offering,
+        control_plane=control_plane,
+        service_definition=service_definition,
     )
 
 
@@ -204,10 +198,10 @@ def compute_plan():
 
 
 @pytest.fixture
-def compute_plan_assignment(compute_plan, test_control_plane_crd):
+def compute_plan_assignment(compute_plan, control_plane_crd):
     return ComputePlanAssignment.objects.create(
         compute_plan=compute_plan,
-        control_plane_crd=test_control_plane_crd,
+        control_plane_crd=control_plane_crd,
         sla="besteffort",
         odoo_product_id="test-product-id",
         odoo_unit_id="test-unit-id",
@@ -215,3 +209,30 @@ def compute_plan_assignment(compute_plan, test_control_plane_crd):
         unit="hour",
         is_active=True,
     )
+
+
+@pytest.fixture
+def service_instance(organization, control_plane_crd):
+    return ServiceInstance.objects.create(
+        name="test-abc12345",
+        display_name="My Test Instance",
+        organization=organization,
+        context=control_plane_crd,
+    )
+
+
+@pytest.fixture
+def control_plane_with_storage(cloud_provider):
+    return ControlPlane.objects.create(
+        name="Storage Zone",
+        description="Zone with storage billing",
+        cloud_provider=cloud_provider,
+        api_credentials={
+            "server": "https://k8s.example.com",
+            "token": "test-token",
+            "certificate-authority-data": "test-ca-data",
+        },
+        storage_plan_odoo_product_id="storage-product-123",
+        storage_plan_odoo_unit_id="storage-unit-456",
+        storage_plan_price_per_gib="0.10",
+    )

src/tests/test_api_exoscale.py

@@ -55,14 +55,14 @@ def valid_osb_payload():
 def test_successful_onboarding_new_organization(
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     exoscale_origin,
     instance_id,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
 
     response = osb_client.put(
         f"/api/osb/v2/service_instances/{instance_id}",
@@ -107,15 +107,15 @@ def test_successful_onboarding_new_organization(
 @pytest.mark.django_db
 def test_new_organization_inherits_origin(
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     exoscale_origin,
     instance_id,
     billing_entity,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
     exoscale_origin.billing_entity = billing_entity
     exoscale_origin.save()
 
@@ -137,8 +137,8 @@ def test_new_organization_inherits_origin(
 @pytest.mark.django_db
 def test_duplicate_organization_returns_existing(
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     exoscale_origin,
     instance_id,
@@ -148,10 +148,10 @@ def test_duplicate_organization_returns_existing(
         osb_guid="test-org-guid-123",
         origin=exoscale_origin,
     )
-    org.limit_osb_services.add(test_service)
+    org.limit_osb_services.add(service)
 
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
 
     response = osb_client.put(
         f"/api/osb/v2/service_instances/{instance_id}",
@@ -169,13 +169,13 @@ def test_duplicate_organization_returns_existing(
 @pytest.mark.django_db
 def test_unauthenticated_osb_api_request_fails(
     client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     instance_id,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
 
     response = client.put(
         f"/api/osb/v2/service_instances/{instance_id}",
@@ -205,15 +205,15 @@ def test_unauthenticated_osb_api_request_fails(
 )
 def test_missing_required_fields_error(
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     field_to_remove,
     expected_error,
     instance_id,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
 
     if isinstance(field_to_remove, tuple):
         if field_to_remove[0] == "context":
@@ -251,10 +251,8 @@ def test_invalid_service_id_error(osb_client, valid_osb_payload, instance_id):
 
 
 @pytest.mark.django_db
-def test_invalid_plan_id_error(
-    osb_client, test_service, valid_osb_payload, instance_id
-):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
+def test_invalid_plan_id_error(osb_client, service, valid_osb_payload, instance_id):
+    valid_osb_payload["service_id"] = service.osb_service_id
     valid_osb_payload["plan_id"] = 99999
 
     response = osb_client.put(
@@ -266,17 +264,17 @@ def test_invalid_plan_id_error(
     assert response.status_code == 400
     response_data = json.loads(response.content)
     assert (
-        f"Unknown plan_id: 99999 for service_id: {test_service.osb_service_id}"
+        f"Unknown plan_id: 99999 for service_id: {service.osb_service_id}"
         in response_data["error"]
     )
 
 
 @pytest.mark.django_db
 def test_empty_users_array_error(
-    osb_client, test_service, test_service_offering, valid_osb_payload, instance_id
+    osb_client, service, service_offering, valid_osb_payload, instance_id
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
     valid_osb_payload["parameters"]["users"] = []
 
     response = osb_client.put(
@@ -292,10 +290,10 @@ def test_empty_users_array_error(
 
 @pytest.mark.django_db
 def test_multiple_users_error(
-    osb_client, test_service, test_service_offering, valid_osb_payload, instance_id
+    osb_client, service, service_offering, valid_osb_payload, instance_id
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
     valid_osb_payload["parameters"]["users"] = [
         {"email": "user1@example.com", "full_name": "User One"},
         {"email": "user2@example.com", "full_name": "User Two"},
@@ -314,10 +312,10 @@ def test_multiple_users_error(
 
 @pytest.mark.django_db
 def test_empty_email_address_error(
-    osb_client, test_service, test_service_offering, valid_osb_payload, instance_id
+    osb_client, service, service_offering, valid_osb_payload, instance_id
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
     valid_osb_payload["parameters"]["users"] = [
         {"email": "", "full_name": "User With No Email"},
     ]
@@ -350,14 +348,14 @@ def test_invalid_json_error(osb_client, instance_id):
 def test_user_creation_with_name_parsing(
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     exoscale_origin,
     instance_id,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
     valid_osb_payload["parameters"]["users"][0]["full_name"] = "John Doe Smith"
 
     response = osb_client.put(
@@ -376,14 +374,14 @@ def test_user_creation_with_name_parsing(
 def test_email_normalization(
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     exoscale_origin,
     instance_id,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
     valid_osb_payload["parameters"]["users"][0]["email"] = "  TEST@EXAMPLE.COM  "
 
     response = osb_client.put(
@@ -401,14 +399,14 @@ def test_email_normalization(
 def test_odoo_integration_failure_handling(
     mock_odoo_failure,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     valid_osb_payload,
     exoscale_origin,
     instance_id,
 ):
-    valid_osb_payload["service_id"] = test_service.osb_service_id
-    valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id
+    valid_osb_payload["service_id"] = service.osb_service_id
+    valid_osb_payload["plan_id"] = service_offering.osb_plan_id
 
     response = osb_client.put(
         f"/api/osb/v2/service_instances/{instance_id}",
@@ -425,14 +423,14 @@ def test_odoo_integration_failure_handling(
 def test_organization_creation_with_context_only(
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     exoscale_origin,
     instance_id,
 ):
     payload = {
-        "service_id": test_service.osb_service_id,
-        "plan_id": test_service_offering.osb_plan_id,
+        "service_id": service.osb_service_id,
+        "plan_id": service_offering.osb_plan_id,
         "context": {
             "organization_guid": "fallback-org-guid",
             "organization_name": "Fallback Organization",
@@ -462,13 +460,13 @@ def test_organization_creation_with_context_only(
 def test_delete_offboarding_success(
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     instance_id,
 ):
     response = osb_client.delete(
         f"/api/osb/v2/service_instances/{instance_id}"
-        f"?service_id={test_service.osb_service_id}&plan_id={test_service_offering.osb_plan_id}"
+        f"?service_id={service.osb_service_id}&plan_id={service_offering.osb_plan_id}"
     )
 
     assert response.status_code == 200
@@ -476,9 +474,9 @@ def test_delete_offboarding_success(
 
 
 @pytest.mark.django_db
-def test_delete_missing_service_id(osb_client, test_service_offering, instance_id):
+def test_delete_missing_service_id(osb_client, service_offering, instance_id):
     response = osb_client.delete(
-        f"/api/osb/v2/service_instances/{instance_id}?plan_id={test_service_offering.osb_plan_id}"
+        f"/api/osb/v2/service_instances/{instance_id}?plan_id={service_offering.osb_plan_id}"
     )
 
     assert response.status_code == 400
@@ -487,9 +485,9 @@ def test_delete_missing_service_id(osb_client, test_service_offering, instance_i
 
 
 @pytest.mark.django_db
-def test_delete_missing_plan_id(osb_client, test_service, instance_id):
+def test_delete_missing_plan_id(osb_client, service, instance_id):
     response = osb_client.delete(
-        f"/api/osb/v2/service_instances/{instance_id}?service_id={test_service.osb_service_id}"
+        f"/api/osb/v2/service_instances/{instance_id}?service_id={service.osb_service_id}"
     )
 
     assert response.status_code == 400
@@ -509,16 +507,16 @@ def test_delete_invalid_service_id(osb_client, instance_id):
 
 
 @pytest.mark.django_db
-def test_delete_invalid_plan_id(osb_client, test_service, instance_id):
+def test_delete_invalid_plan_id(osb_client, service, instance_id):
     response = osb_client.delete(
         f"/api/osb/v2/service_instances/{instance_id}"
-        f"?service_id={test_service.osb_service_id}&plan_id=invalid"
+        f"?service_id={service.osb_service_id}&plan_id=invalid"
     )
 
     assert response.status_code == 400
     response_data = json.loads(response.content)
     assert (
-        f"Unknown plan_id: invalid for service_id: {test_service.osb_service_id}"
+        f"Unknown plan_id: invalid for service_id: {service.osb_service_id}"
         in response_data["error"]
     )
 
@@ -527,13 +525,13 @@ def test_delete_invalid_plan_id(osb_client, test_service, instance_id):
 def test_patch_suspension_success(
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     instance_id,
 ):
     payload = {
-        "service_id": test_service.osb_service_id,
-        "plan_id": test_service_offering.osb_plan_id,
+        "service_id": service.osb_service_id,
+        "plan_id": service_offering.osb_plan_id,
         "parameters": {
             "users": [
                 {
@@ -556,9 +554,9 @@ def test_patch_suspension_success(
 
 
 @pytest.mark.django_db
-def test_patch_missing_service_id(osb_client, test_service_offering, instance_id):
+def test_patch_missing_service_id(osb_client, service_offering, instance_id):
     payload = {
-        "plan_id": test_service_offering.osb_plan_id,
+        "plan_id": service_offering.osb_plan_id,
         "parameters": {"users": []},
     }
 
@@ -574,9 +572,9 @@ def test_patch_missing_service_id(osb_client, test_service_offering, instance_id
 
 
 @pytest.mark.django_db
-def test_patch_missing_plan_id(osb_client, test_service, instance_id):
+def test_patch_missing_plan_id(osb_client, service, instance_id):
     payload = {
-        "service_id": test_service.osb_service_id,
+        "service_id": service.osb_service_id,
         "parameters": {"users": []},
     }
 
@@ -609,8 +607,8 @@ def test_delete_creates_ticket_with_admin_links(
     mocker,
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     instance_id,
 ):
     # Mock the create_helpdesk_ticket function
@@ -618,7 +616,7 @@ def test_delete_creates_ticket_with_admin_links(
 
     response = osb_client.delete(
         f"/api/osb/v2/service_instances/{instance_id}"
-        f"?service_id={test_service.osb_service_id}&plan_id={test_service_offering.osb_plan_id}"
+        f"?service_id={service.osb_service_id}&plan_id={service_offering.osb_plan_id}"
     )
 
     assert response.status_code == 200
@@ -629,10 +627,10 @@ def test_delete_creates_ticket_with_admin_links(
 
     # Check that the description contains an admin URL
     assert "admin/core/serviceoffering" in call_kwargs["description"]
-    assert f"/{test_service_offering.pk}/" in call_kwargs["description"]
+    assert f"/{service_offering.pk}/" in call_kwargs["description"]
     assert (
         call_kwargs["title"]
-        == f"Exoscale OSB Offboard - {test_service.name} - {instance_id}"
+        == f"Exoscale OSB Offboard - {service.name} - {instance_id}"
     )
 
 
@@ -641,8 +639,8 @@ def test_patch_creates_ticket_with_user_admin_links(
     mocker,
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     instance_id,
     org_owner,
 ):
@@ -650,8 +648,8 @@ def test_patch_creates_ticket_with_user_admin_links(
     mock_create_ticket = mocker.patch("servala.api.views.create_helpdesk_ticket")
 
     payload = {
-        "service_id": test_service.osb_service_id,
-        "plan_id": test_service_offering.osb_plan_id,
+        "service_id": service.osb_service_id,
+        "plan_id": service_offering.osb_plan_id,
         "parameters": {
             "users": [
                 {
@@ -680,8 +678,7 @@ def test_patch_creates_ticket_with_user_admin_links(
     assert "admin/core/user" in call_kwargs["description"]
     assert f"/{org_owner.pk}/" in call_kwargs["description"]
     assert (
-        call_kwargs["title"]
-        == f"Exoscale OSB Suspend - {test_service.name} - {instance_id}"
+        call_kwargs["title"] == f"Exoscale OSB Suspend - {service.name} - {instance_id}"
     )
 
 
@@ -690,8 +687,8 @@ def test_ticket_includes_organization_and_instance_when_found(
     mocker,
     mock_odoo_success,
     osb_client,
-    test_service,
-    test_service_offering,
+    service,
+    service_offering,
     organization,
 ):
     # Mock the create_helpdesk_ticket function
@@ -699,12 +696,12 @@ def test_ticket_includes_organization_and_instance_when_found(
 
     service_definition = ServiceDefinition.objects.create(
         name="Test Definition",
-        service=test_service,
+        service=service,
         api_definition={"group": "test.example.com", "version": "v1", "kind": "Test"},
     )
     control_plane = ControlPlane.objects.create(
         name="Test Control Plane",
-        cloud_provider=test_service_offering.provider,
+        cloud_provider=service_offering.provider,
         api_credentials={
             "certificate-authority-data": "test",
             "server": "https://test",
@@ -712,7 +709,7 @@ def test_ticket_includes_organization_and_instance_when_found(
         },
     )
     crd = ControlPlaneCRD.objects.create(
-        service_offering=test_service_offering,
+        service_offering=service_offering,
         control_plane=control_plane,
         service_definition=service_definition,
     )
@@ -727,7 +724,7 @@ def test_ticket_includes_organization_and_instance_when_found(
 
     response = osb_client.delete(
         f"/api/osb/v2/service_instances/{instance_name}"
-        f"?service_id={test_service.osb_service_id}&plan_id={test_service_offering.osb_plan_id}"
+        f"?service_id={service.osb_service_id}&plan_id={service_offering.osb_plan_id}"
     )
 
     assert response.status_code == 200

src/tests/test_management_commands.py

@@ -109,7 +109,7 @@ class TestReencryptFieldsCommand:
         assert "Starting re-encryption" in output
         assert "Re-encrypted 0 ControlPlane objects" in output
 
-    def test_reencrypt_fields_with_control_plane(self, test_control_plane):
+    def test_reencrypt_fields_with_control_plane(self, control_plane):
         out = StringIO()
         call_command("reencrypt_fields", stdout=out)
 
@@ -147,11 +147,11 @@ class TestSyncBillingMetadataCommand:
 
         assert "No control planes found with the specified IDs" in out.getvalue()
 
-    def test_sync_billing_metadata_dry_run_with_control_plane(self, test_control_plane):
+    def test_sync_billing_metadata_dry_run_with_control_plane(self, control_plane):
         out = StringIO()
         call_command("sync_billing_metadata", "--dry-run", stdout=out)
 
         output = out.getvalue()
         assert "DRY RUN" in output
         assert "Syncing billing metadata on 1 control plane(s)" in output
-        assert test_control_plane.name in output
+        assert control_plane.name in output

src/tests/test_service_models.py

@@ -0,0 +1,349 @@
+"""Tests for servala.core.models.service module."""
+
+from unittest.mock import MagicMock
+
+import pytest
+from django.core.exceptions import ValidationError
+
+from servala.core.models.service import (
+    ControlPlane,
+    Service,
+    ServiceInstance,
+    prune_empty_data,
+    validate_api_credentials,
+    validate_dict,
+)
+
+pytestmark = pytest.mark.django_db
+
+
+@pytest.fixture
+def service_with_external_links(service_category):
+    return Service.objects.create(
+        name="PostgreSQL",
+        slug="postgresql",
+        category=service_category,
+        description="PostgreSQL database",
+        external_links=[
+            {"url": "https://docs.example.com", "title": "Docs", "featured": True},
+            {"url": "https://github.com/example", "title": "GitHub", "featured": False},
+            {"url": "https://api.example.com", "title": "API", "featured": True},
+        ],
+    )
+
+
+@pytest.mark.parametrize("data", [None, {}])
+def test_validate_dict_allows_empty_by_default(data):
+    validate_dict(data)
+
+
+@pytest.mark.parametrize("data", [None, {}])
+def test_validate_dict_raises_when_empty_not_allowed(data):
+    with pytest.raises(ValidationError, match="Data may not be empty"):
+        validate_dict(data, allow_empty=False)
+
+
+def test_validate_dict_missing_required_fields_raises():
+    with pytest.raises(ValidationError, match="Missing required fields"):
+        validate_dict({"field1": "v"}, required_fields={"field1", "field2", "field3"})
+
+
+def test_validate_dict_all_required_fields_present_passes():
+    validate_dict({"a": 1, "b": 2, "extra": 3}, required_fields={"a", "b"})
+
+
+@pytest.mark.parametrize("data", [None, {}])
+def test_validate_api_credentials_allows_empty(data):
+    validate_api_credentials(data)
+
+
+@pytest.mark.parametrize(
+    "input_data,expected",
+    [
+        ({"a": 1, "b": None, "c": 3}, {"a": 1, "c": 3}),
+        ({"a": "hello", "b": "", "c": "world"}, {"a": "hello", "c": "world"}),
+        ({"a": [1, 2], "b": [], "c": [3]}, {"a": [1, 2], "c": [3]}),
+        (
+            {"a": {"nested": 1}, "b": {}, "c": {"x": 2}},
+            {"a": {"nested": 1}, "c": {"x": 2}},
+        ),
+        ({"outer": {"inner": {"empty": None}}}, {}),
+        (
+            {"false_val": False, "zero": 0, "none": None},
+            {"false_val": False, "zero": 0},
+        ),
+        ("string", "string"),
+        (42, 42),
+    ],
+)
+def test_prune_empty_data(input_data, expected):
+    assert prune_empty_data(input_data) == expected
+
+
+def test_prune_empty_data_nested_dicts():
+    data = {"level1": {"level2": {"keep": "value", "remove": None, "empty": ""}}}
+    assert prune_empty_data(data) == {"level1": {"level2": {"keep": "value"}}}
+
+
+def test_prune_empty_data_in_lists():
+    data = {"items": [{"keep": 1}, {"remove": None}, {"also_keep": 2}]}
+    assert prune_empty_data(data) == {"items": [{"keep": 1}, {"also_keep": 2}]}
+
+
+def test_service_featured_links_filters_correctly(service_with_external_links):
+    featured = service_with_external_links.featured_links
+    assert len(featured) == 2
+    assert all(link["featured"] for link in featured)
+    assert {link["title"] for link in featured} == {"Docs", "API"}
+
+
+@pytest.mark.parametrize(
+    "external_links,expected_count",
+    [
+        (None, 0),
+        ([], 0),
+        ([{"url": "https://x.com", "title": "X", "featured": False}], 0),
+    ],
+)
+def test_service_featured_links_empty_cases(
+    service_category, external_links, expected_count
+):
+    svc = Service.objects.create(
+        name="Test",
+        slug="test-svc",
+        category=service_category,
+        external_links=external_links,
+    )
+    assert len(svc.featured_links) == expected_count
+
+
+def test_service_str(service):
+    assert str(service) == "Redis"
+
+
+def test_service_category_str(service_category):
+    assert str(service_category) == "Databases"
+
+
+def test_cloud_provider_str(cloud_provider):
+    assert str(cloud_provider) == "Exoscale"
+
+
+def test_control_plane_str(control_plane):
+    assert str(control_plane) == "Geneva (CH-GVA-2)"
+
+
+def test_control_plane_test_connection_no_credentials(cloud_provider):
+    plane = ControlPlane.objects.create(
+        name="No Creds", cloud_provider=cloud_provider, api_credentials={}
+    )
+    success, message = plane.test_connection()
+    assert success is False
+    assert "No API credentials" in str(message)
+
+
+def test_service_definition_str(service_definition):
+    assert str(service_definition) == "Redis Standard"
+
+
+def test_service_definition_control_planes(service_definition, control_plane_crd):
+    assert control_plane_crd.control_plane in service_definition.control_planes
+
+
+def test_control_plane_crd_str(control_plane_crd):
+    result = str(control_plane_crd)
+    assert "Redis" in result and "Exoscale" in result and "Geneva" in result
+
+
+@pytest.mark.parametrize(
+    "prop,expected",
+    [("group", "vshn.appcat.vshn.io"), ("version", "v1"), ("kind", "VSHNRedis")],
+)
+def test_control_plane_crd_api_properties(control_plane_crd, prop, expected):
+    assert getattr(control_plane_crd, prop) == expected
+
+
+def test_service_offering_str(service_offering):
+    result = str(service_offering)
+    assert "Redis" in result and "Exoscale" in result
+
+
+def test_service_offering_control_planes(service_offering, control_plane_crd):
+    assert control_plane_crd.control_plane in service_offering.control_planes
+
+
+def test_generate_resource_name_consistent(organization, service):
+    name1 = ServiceInstance.generate_resource_name(organization, "My Instance", service)
+    name2 = ServiceInstance.generate_resource_name(organization, "My Instance", service)
+    assert name1 == name2
+
+
+def test_generate_resource_name_different_inputs(organization, service):
+    name_a = ServiceInstance.generate_resource_name(organization, "Instance A", service)
+    name_b = ServiceInstance.generate_resource_name(organization, "Instance B", service)
+    assert name_a != name_b
+
+
+def test_generate_resource_name_attempt_changes_hash(organization, service):
+    name0 = ServiceInstance.generate_resource_name(
+        organization, "X", service, attempt=0
+    )
+    name1 = ServiceInstance.generate_resource_name(
+        organization, "X", service, attempt=1
+    )
+    assert name0 != name1
+
+
+def test_generate_resource_name_format(organization, service, settings):
+    name = ServiceInstance.generate_resource_name(organization, "Test", service)
+    assert name.startswith(f"{settings.SERVALA_INSTANCE_NAME_PREFIX}-")
+    assert len(name.split("-")[-1]) == 8
+
+
+@pytest.mark.parametrize(
+    "display_name", ["My Instance", "MY INSTANCE", "  My Instance  "]
+)
+def test_generate_resource_name_normalizes_display_name(
+    organization, service, display_name
+):
+    canonical = ServiceInstance.generate_resource_name(
+        organization, "my instance", service
+    )
+    assert (
+        ServiceInstance.generate_resource_name(organization, display_name, service)
+        == canonical
+    )
+
+
+@pytest.mark.parametrize("spec_data", [None, {}])
+def test_prepare_spec_data_handles_empty(spec_data):
+    assert ServiceInstance._prepare_spec_data(spec_data) == {}
+
+
+def test_prepare_spec_data_prunes_empty_values():
+    assert ServiceInstance._prepare_spec_data({"keep": "v", "rm": None}) == {
+        "keep": "v"
+    }
+
+
+def test_apply_compute_plan_to_spec(compute_plan_assignment):
+    result = ServiceInstance._apply_compute_plan_to_spec({}, compute_plan_assignment)
+    assert result["parameters"]["size"]["memory"] == "2Gi"
+    assert result["parameters"]["size"]["cpu"] == "1000m"
+    assert result["parameters"]["size"]["requests"]["memory"] == "1Gi"
+    assert result["parameters"]["size"]["requests"]["cpu"] == "500m"
+    assert result["parameters"]["service"]["serviceLevel"] == "besteffort"
+
+
+def test_apply_compute_plan_to_spec_none_assignment():
+    spec = {"existing": "value"}
+    assert ServiceInstance._apply_compute_plan_to_spec(spec, None) == {
+        "existing": "value"
+    }
+
+
+def test_apply_compute_plan_preserves_existing(compute_plan_assignment):
+    spec = {"parameters": {"custom": "setting", "service": {"other": "config"}}}
+    result = ServiceInstance._apply_compute_plan_to_spec(spec, compute_plan_assignment)
+    assert result["parameters"]["custom"] == "setting"
+    assert result["parameters"]["service"]["other"] == "config"
+
+
+def test_build_billing_annotations_display_name():
+    cp = MagicMock(storage_plan_odoo_product_id=None, storage_plan_odoo_unit_id=None)
+    annotations = ServiceInstance._build_billing_annotations(
+        compute_plan_assignment=None, control_plane=cp, display_name="My Service"
+    )
+    assert annotations["servala.com/displayName"] == "My Service"
+
+
+def test_build_billing_annotations_no_display_name():
+    cp = MagicMock(storage_plan_odoo_product_id=None, storage_plan_odoo_unit_id=None)
+    annotations = ServiceInstance._build_billing_annotations(
+        compute_plan_assignment=None, control_plane=cp, display_name=None
+    )
+    assert "servala.com/displayName" not in annotations
+
+
+def test_build_billing_annotations_compute_plan(compute_plan_assignment):
+    cp = MagicMock(storage_plan_odoo_product_id=None, storage_plan_odoo_unit_id=None)
+    annotations = ServiceInstance._build_billing_annotations(
+        compute_plan_assignment=compute_plan_assignment, control_plane=cp
+    )
+    assert annotations["servala.com/erp_product_id_resource"] == "test-product-id"
+    assert annotations["servala.com/erp_unit_id_resource"] == "test-unit-id"
+
+
+def test_build_billing_annotations_storage_plan(control_plane_with_storage):
+    annotations = ServiceInstance._build_billing_annotations(
+        compute_plan_assignment=None, control_plane=control_plane_with_storage
+    )
+    assert annotations["servala.com/erp_product_id_storage"] == "storage-product-123"
+    assert annotations["servala.com/erp_unit_id_storage"] == "storage-unit-456"
+
+
+@pytest.mark.parametrize(
+    "error_msg,expected_has_list,expected_errors",
+    [
+        ("", False, None),
+        (None, False, None),
+        ("Something went wrong", False, None),
+        ("Error: [single error]", False, None),
+        ("Validation failed: [e1, e2, e3]", True, ["e1", "e2", "e3"]),
+    ],
+)
+def test_format_kubernetes_error(error_msg, expected_has_list, expected_errors):
+    result = ServiceInstance._format_kubernetes_error(error_msg)
+    assert result["has_list"] == expected_has_list
+    assert result["errors"] == expected_errors
+
+
+def test_format_kubernetes_error_strips_quotes():
+    result = ServiceInstance._format_kubernetes_error("Errors: [\"quoted\", 'single']")
+    assert "quoted" in result["errors"]
+    assert "single" in result["errors"]
+
+
+def test_safe_format_error_non_dict():
+    assert ServiceInstance._safe_format_error("plain string") == "plain string"
+
+
+def test_safe_format_error_escapes_html():
+    error_data = {"message": "<script>alert('xss')</script>", "has_list": False}
+    result = ServiceInstance._safe_format_error(error_data)
+    assert "<script>" not in result
+    assert "&lt;script&gt;" in result
+
+
+def test_safe_format_error_formats_list():
+    error_data = {"message": "Failed", "errors": ["e1", "e2"], "has_list": True}
+    result = str(ServiceInstance._safe_format_error(error_data))
+    assert "Failed" in result and "<ul>" in result and "<li>e1</li>" in result
+
+
+@pytest.mark.parametrize(
+    "spec,expected",
+    [
+        ({}, None),
+        ({"parameters": {"service": {}}}, None),
+        ({"parameters": {"service": {"fqdn": "app.example.com"}}}, "app.example.com"),
+        (
+            {"parameters": {"service": {"fqdn": ["first.com", "second.com"]}}},
+            "first.com",
+        ),
+        ({"parameters": {"service": {"fqdn": 12345}}}, None),
+    ],
+)
+def test_fqdn_url(service_instance, mocker, spec, expected):
+    mocker.patch.object(
+        ServiceInstance, "spec", new_callable=mocker.PropertyMock, return_value=spec
+    )
+    assert service_instance.fqdn_url == expected
+
+
+def test_clear_kubernetes_caches(service_instance):
+    service_instance.__dict__["kubernetes_object"] = {"cached": True}
+    service_instance.__dict__["spec"] = {"cached": True}
+    service_instance._clear_kubernetes_caches()
+    assert "kubernetes_object" not in service_instance.__dict__
+    assert "spec" not in service_instance.__dict__

src/tests/test_user.py

@@ -0,0 +1,208 @@
+import pytest
+
+from servala.core.models import User
+
+pytestmark = pytest.mark.django_db
+
+
+@pytest.fixture
+def user():
+    return User.objects.create(email="testuser@example.org", password="test")
+
+
+def test_create_user():
+    user = User.objects.create_user(
+        email="test@example.org",
+        password="testpassword123",
+        first_name="Test",
+        last_name="User",
+    )
+    assert user.email == "test@example.org"
+    assert user.first_name == "Test"
+    assert user.last_name == "User"
+    assert user.check_password("testpassword123")
+    assert not user.is_staff
+    assert not user.is_superuser
+
+
+def test_create_user_normalizes_email():
+    user = User.objects.create_user(
+        email="  TEST@EXAMPLE.ORG  ",
+        password="testpassword123",
+    )
+    assert user.email == "TEST@example.org"
+
+
+def test_create_user_without_email_raises_error():
+    with pytest.raises(ValueError, match="Please provide an email address"):
+        User.objects.create_user(email="", password="testpassword123")
+
+
+def test_create_superuser():
+    user = User.objects.create_superuser(
+        email="admin@example.org",
+        password="adminpassword123",
+    )
+    assert user.email == "admin@example.org"
+    assert user.check_password("adminpassword123")
+    assert user.is_staff
+    assert user.is_superuser
+
+
+@pytest.mark.parametrize(
+    "first_name,last_name,email,expected",
+    [
+        ("John", "Doe", "john@example.org", "John Doe"),
+        ("John", "", "john@example.org", "John"),
+        ("", "Doe", "john@example.org", "Doe"),
+        ("", "", "john@example.org", "john@example.org"),
+    ],
+)
+def test_user_str(first_name, last_name, email, expected):
+    user = User(email=email, first_name=first_name, last_name=last_name)
+    assert str(user) == expected
+
+
+def test_normalize_username():
+    user = User()
+    assert user.normalize_username("  TEST@EXAMPLE.ORG  ") == "test@example.org"
+
+
+def test_get_odoo_contact_returns_none_without_billing_entity(user, organization):
+    assert organization.billing_entity is None
+    result = user.get_odoo_contact(organization)
+    assert result is None
+
+
+def test_get_odoo_contact_returns_none_without_odoo_company_id(
+    user, organization, billing_entity
+):
+    organization.billing_entity = billing_entity
+    organization.save()
+    assert billing_entity.odoo_company_id is None
+    result = user.get_odoo_contact(organization)
+    assert result is None
+
+
+def test_get_odoo_contact_returns_contact_from_odoo(
+    user, organization, billing_entity, mocker
+):
+    billing_entity.odoo_company_id = 123
+    billing_entity.save()
+    organization.billing_entity = billing_entity
+    organization.save()
+
+    mock_client = mocker.patch("servala.core.models.user.odoo.CLIENT")
+    mock_client.search_read.return_value = [
+        {"id": 456, "name": "Test User", "email": user.email}
+    ]
+
+    result = user.get_odoo_contact(organization)
+
+    assert result == {"id": 456, "name": "Test User", "email": user.email}
+    mock_client.search_read.assert_called_once()
+
+
+def test_get_odoo_contact_returns_none_when_not_found(
+    user, organization, billing_entity, mocker
+):
+    billing_entity.odoo_company_id = 123
+    billing_entity.save()
+    organization.billing_entity = billing_entity
+    organization.save()
+
+    mock_client = mocker.patch("servala.core.models.user.odoo.CLIENT")
+    mock_client.search_read.return_value = []
+
+    result = user.get_odoo_contact(organization)
+
+    assert result is None
+
+
+def test_get_or_create_odoo_contact_returns_none_without_billing_entity(
+    user, organization
+):
+    assert organization.billing_entity is None
+    result = user.get_or_create_odoo_contact(organization)
+    assert result is None
+
+
+def test_get_or_create_odoo_contact_returns_none_without_odoo_company_id(
+    user, organization, billing_entity
+):
+    organization.billing_entity = billing_entity
+    organization.save()
+    assert billing_entity.odoo_company_id is None
+    result = user.get_or_create_odoo_contact(organization)
+    assert result is None
+
+
+def test_get_or_create_odoo_contact_returns_existing_contact(
+    user, organization, billing_entity, mocker
+):
+    billing_entity.odoo_company_id = 123
+    billing_entity.save()
+    organization.billing_entity = billing_entity
+    organization.save()
+
+    mock_client = mocker.patch("servala.core.models.user.odoo.CLIENT")
+    mock_client.search_read.return_value = [{"id": 456, "name": "Test User"}]
+
+    result = user.get_or_create_odoo_contact(organization)
+
+    assert result == 456
+    mock_client.execute.assert_not_called()
+
+
+def test_get_or_create_odoo_contact_creates_new_contact(
+    organization, billing_entity, mocker
+):
+    new_user = User.objects.create_user(
+        email="newuser@example.org",
+        password="test",
+        first_name="New",
+        last_name="User",
+    )
+    billing_entity.odoo_company_id = 123
+    billing_entity.save()
+    organization.billing_entity = billing_entity
+    organization.save()
+
+    mock_client = mocker.patch("servala.core.models.user.odoo.CLIENT")
+    mock_client.search_read.return_value = []
+    mock_client.execute.return_value = 789
+
+    result = new_user.get_or_create_odoo_contact(organization)
+
+    assert result == 789
+    mock_client.execute.assert_called_once()
+    call_args = mock_client.execute.call_args
+    assert call_args[0][0] == "res.partner"
+    assert call_args[0][1] == "create"
+    partner_data = call_args[0][2][0]
+    assert partner_data["name"] == "New User"
+    assert partner_data["email"] == "newuser@example.org"
+    assert partner_data["parent_id"] == 123
+
+
+def test_get_or_create_odoo_contact_uses_email_as_name_fallback(
+    organization, billing_entity, mocker
+):
+    new_user = User.objects.create_user(
+        email="noname@example.org",
+        password="test",
+    )
+    billing_entity.odoo_company_id = 123
+    billing_entity.save()
+    organization.billing_entity = billing_entity
+    organization.save()
+
+    mock_client = mocker.patch("servala.core.models.user.odoo.CLIENT")
+    mock_client.search_read.return_value = []
+    mock_client.execute.return_value = 789
+
+    new_user.get_or_create_odoo_contact(organization)
+
+    call_args = mock_client.execute.call_args
+    partner_data = call_args[0][2][0]
+    assert partner_data["name"] == "noname@example.org"

src/tests/test_views.py

@@ -1,6 +1,14 @@
+from unittest.mock import MagicMock, patch
+
 import pytest
 
-from servala.core.models.service import CloudProvider, ServiceOffering
+from servala.core.models.service import (
+    CloudProvider,
+    ControlPlane,
+    ControlPlaneCRD,
+    ServiceInstance,
+    ServiceOffering,
+)
 
 
 @pytest.mark.parametrize(
@@ -51,76 +59,76 @@ def test_organization_linked_in_sidebar(
 
 @pytest.mark.django_db
 def test_service_detail_redirects_with_single_offering(
-    client, org_owner, organization, test_service, test_service_offering
+    client, org_owner, organization, service, service_offering
 ):
     client.force_login(org_owner)
-    url = f"/org/{organization.slug}/services/{test_service.slug}/"
+    url = f"/org/{organization.slug}/services/{service.slug}/"
     response = client.get(url)
 
     assert response.status_code == 302
-    expected_url = f"/org/{organization.slug}/services/{test_service.slug}/offering/{test_service_offering.pk}/"
+    expected_url = f"/org/{organization.slug}/services/{service.slug}/offering/{service_offering.pk}/"
     assert response.url == expected_url
 
 
 @pytest.mark.django_db
 def test_service_detail_shows_multiple_offerings(
-    client, org_owner, organization, test_service, test_service_offering
+    client, org_owner, organization, service, service_offering
 ):
     second_provider = CloudProvider.objects.create(
         name="AWS", description="Amazon Web Services"
     )
     second_offering = ServiceOffering.objects.create(
-        service=test_service,
+        service=service,
         provider=second_provider,
         description="Redis on AWS",
         osb_plan_id="test-plan-456",
     )
 
     client.force_login(org_owner)
-    url = f"/org/{organization.slug}/services/{test_service.slug}/"
+    url = f"/org/{organization.slug}/services/{service.slug}/"
     response = client.get(url)
 
     assert response.status_code == 200
     content = response.content.decode()
 
-    assert test_service_offering.provider.name in content
+    assert service_offering.provider.name in content
     assert second_offering.provider.name in content
     assert "Create Instance" in content
 
 
 @pytest.mark.django_db
 def test_service_detail_respects_cloud_provider_restrictions(
-    client, org_owner, organization, test_service, test_service_offering
+    client, org_owner, organization, service, service_offering
 ):
     second_provider = CloudProvider.objects.create(
         name="AWS", description="Amazon Web Services"
     )
     ServiceOffering.objects.create(
-        service=test_service,
+        service=service,
         provider=second_provider,
         description="Redis on AWS",
         osb_plan_id="test-plan-456",
     )
-    organization.origin.limit_cloudproviders.add(test_service_offering.provider)
+    organization.origin.limit_cloudproviders.add(service_offering.provider)
 
     client.force_login(org_owner)
-    url = f"/org/{organization.slug}/services/{test_service.slug}/"
+    url = f"/org/{organization.slug}/services/{service.slug}/"
     response = client.get(url)
 
     assert response.status_code == 302
-    expected_url = f"/org/{organization.slug}/services/{test_service.slug}/offering/{test_service_offering.pk}/"
+    expected_url = f"/org/{organization.slug}/services/{service.slug}/offering/{service_offering.pk}/"
     assert response.url == expected_url
 
 
 @pytest.mark.django_db
 def test_service_detail_no_redirect_with_restricted_multiple_offerings(
-    client, org_owner, organization, test_service, test_service_offering
+    client, org_owner, organization, service, service_offering
 ):
     second_provider = CloudProvider.objects.create(
         name="AWS", description="Amazon Web Services"
     )
     second_offering = ServiceOffering.objects.create(
-        service=test_service,
+        service=service,
         provider=second_provider,
         description="Redis on AWS",
         osb_plan_id="test-plan-456",
@@ -129,21 +137,80 @@ def test_service_detail_no_redirect_with_restricted_multiple_offerings(
         name="Azure", description="Microsoft Azure"
     )
     third_offering = ServiceOffering.objects.create(
-        service=test_service,
+        service=service,
         provider=third_provider,
         description="Redis on Azure",
         osb_plan_id="test-plan-789",
     )
     organization.origin.limit_cloudproviders.add(
-        test_service_offering.provider, second_provider
+        service_offering.provider, second_provider
     )
 
     client.force_login(org_owner)
-    url = f"/org/{organization.slug}/services/{test_service.slug}/"
+    url = f"/org/{organization.slug}/services/{service.slug}/"
     response = client.get(url)
 
     assert response.status_code == 200
     content = response.content.decode()
-    assert test_service_offering.provider.name in content
+    assert service_offering.provider.name in content
     assert second_offering.provider.name in content
     assert third_offering.provider.name not in content
+
+
+@pytest.mark.django_db
+def test_service_instance_update_spec_pushes_display_name_annotation(
+    organization, control_plane_crd, org_owner
+):
+    instance = ServiceInstance.objects.create(
+        name="test-instance",
+        display_name="Original Name",
+        organization=organization,
+        context=control_plane_crd,
+    )
+    mock_api = MagicMock()
+    with (
+        patch.object(ControlPlane, "custom_objects_api", mock_api),
+        patch.object(ControlPlaneCRD, "kind_plural", "testkinds"),
+    ):
+        instance.display_name = "Updated Name"
+        instance.update_spec(spec_data={}, updated_by=org_owner)
+        mock_api.patch_namespaced_custom_object.assert_called_once()
+        call_kwargs = mock_api.patch_namespaced_custom_object.call_args[1]
+        assert "metadata" in call_kwargs["body"]
+        annotations = call_kwargs["body"]["metadata"]["annotations"]
+        assert annotations["servala.com/displayName"] == "Updated Name"
+
+    instance.refresh_from_db()
+    assert instance.display_name == "Updated Name"
+
+
+@pytest.mark.django_db
+def test_build_billing_annotations_includes_display_name():
+    annotations = ServiceInstance._build_billing_annotations(
+        compute_plan_assignment=None,
+        control_plane=MagicMock(
+            storage_plan_odoo_product_id=None,
+            storage_plan_odoo_unit_id=None,
+        ),
+        instance_name="test-instance",
+        display_name="My Display Name",
+        organization=None,
+        service=None,
+    )
+    assert annotations["servala.com/displayName"] == "My Display Name"
+
+
+@pytest.mark.django_db
+def test_build_billing_annotations_omits_display_name_when_none():
+    annotations = ServiceInstance._build_billing_annotations(
+        compute_plan_assignment=None,
+        control_plane=MagicMock(
+            storage_plan_odoo_product_id=None,
+            storage_plan_odoo_unit_id=None,
+        ),
+        instance_name="test-instance",
+        display_name=None,
+        organization=None,
+        service=None,
+    )
+    assert "servala.com/displayName" not in annotations