From 76bc37e3f0fc5ab982fc940bd0900aa0dd468a99 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 3 Jul 2025 16:43:19 +0200 Subject: [PATCH 01/18] refactor odoo company type handling --- .../ROOT/pages/web-portal-billingentity.adoc | 4 ++-- src/servala/core/models/organization.py | 16 ++++++++-------- src/servala/core/models/user.py | 4 ++-- src/servala/core/odoo.py | 10 +++++----- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/modules/ROOT/pages/web-portal-billingentity.adoc b/docs/modules/ROOT/pages/web-portal-billingentity.adoc index cd7b662..0b485c8 100644 --- a/docs/modules/ROOT/pages/web-portal-billingentity.adoc +++ b/docs/modules/ROOT/pages/web-portal-billingentity.adoc @@ -17,9 +17,9 @@ Search is done this way: When choosing to add a new billing address, two new records are created in the Odoo `res.partner` model: -* A record with the field `company_type = company` +* A record with the field `is_company = False` * A record with the following field configuration: -** `company_type = person` +** `is_company = False` ** `type = invoice` ** `parent_id = company_id` diff --git a/src/servala/core/models/organization.py b/src/servala/core/models/organization.py index a9dcd4e..b8a4e2a 100644 --- a/src/servala/core/models/organization.py +++ b/src/servala/core/models/organization.py @@ -147,8 +147,8 @@ class BillingEntity(ServalaModelMixin, models.Model): ) # Odoo IDs are nullable for creation, should never be null in practice - # The company ID points at a record of type res.partner with company_type=company - # The invoice ID points at a record of type res.partner with company_type=person, + # The company ID points at a record of type res.partner with is_company=True + # The invoice ID points at a record of type res.partner with is_company=False, # type=invoice, parent_id=company_id (the invoice address). odoo_company_id = models.IntegerField(null=True) odoo_invoice_id = models.IntegerField(null=True) @@ -166,8 +166,8 @@ class BillingEntity(ServalaModelMixin, models.Model): """ Creates a BillingEntity and corresponding Odoo records. - This method creates a `res.partner` record in Odoo with `company_type='company'` - for the main company, and another `res.partner` record with `company_type='person'` + This method creates a `res.partner` record in Odoo with `is_company=True` + for the main company, and another `res.partner` record with `is_company=False` and `type='invoice'` (linked via `parent_id` to the first record) for the invoice address. The IDs of these Odoo records are stored in the BillingEntity. @@ -187,14 +187,14 @@ class BillingEntity(ServalaModelMixin, models.Model): instance = cls.objects.create(name=name) company_payload = { "name": odoo_data.get("company_name", name), - "company_type": "company", + "is_company": True, } company_id = CLIENT.execute("res.partner", "create", [company_payload]) instance.odoo_company_id = company_id invoice_address_payload = { "name": name, - "company_type": "person", + "is_company": False, "type": "invoice", "parent_id": company_id, } @@ -249,10 +249,10 @@ class BillingEntity(ServalaModelMixin, models.Model): "invoice_address": None, } - company_fields = ["name", "company_type"] + company_fields = ["name", "is_company"] invoice_address_fields = [ "name", - "company_type", + "is_company", "type", "parent_id", "street", diff --git a/src/servala/core/models/user.py b/src/servala/core/models/user.py index 38cf80c..b8adfa4 100644 --- a/src/servala/core/models/user.py +++ b/src/servala/core/models/user.py @@ -84,7 +84,7 @@ class User(ServalaModelMixin, PermissionsMixin, AbstractBaseUser): result = odoo.CLIENT.search_read( model="res.partner", domain=[ - ("company_type", "=", "person"), + ("is_company", "=", False), ("type", "=", "contact"), ("email", "ilike", self.email), ("parent_id", "=", organization.billing_entity.odoo_company_id), @@ -107,7 +107,7 @@ class User(ServalaModelMixin, PermissionsMixin, AbstractBaseUser): partner_data = { "name": f"{self.first_name} {self.last_name}".strip() or self.email, "email": self.email, - "company_type": "person", + "is_company": False, "type": "contact", "parent_id": organization.billing_entity.odoo_company_id, } diff --git a/src/servala/core/odoo.py b/src/servala/core/odoo.py index 3d98e18..ba91dc7 100644 --- a/src/servala/core/odoo.py +++ b/src/servala/core/odoo.py @@ -15,7 +15,7 @@ ADDRESS_FIELDS = [ "email", "phone", "vat", - "company_type", + "is_company", "type", "parent_id", ] @@ -118,8 +118,8 @@ def get_odoo_countries(): def get_odoo_access_conditions(user): - # We’re building our conditions in order: - # - in exceptions, users may be using a billing account’s email + # We're building our conditions in order: + # - in exceptions, users may be using a billing account's email # - if the user is an admin or owner of a Servala organization # - if the user is associated with an odoo user, return all billing # addresses / organizations created by the user @@ -153,7 +153,7 @@ def get_odoo_access_conditions(user): odoo_contacts = CLIENT.search_read( model="res.partner", domain=[ - ("company_type", "=", "person"), + ("is_company", "=", False), ("type", "=", "contact"), ("email", "ilike", email), ], @@ -186,7 +186,7 @@ def get_invoice_addresses(user): or_conditions = get_odoo_access_conditions(user) domain = [ - ("company_type", "=", "person"), + ("is_company", "=", False), ("type", "=", "invoice"), ] + or_conditions From 46d323528e0edf41f047c7d28fec3ff9afaf5af6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 7 Jul 2025 13:08:58 +0200 Subject: [PATCH 02/18] use organization role to check for edit and delete perms --- src/servala/frontend/views/organization.py | 7 ++--- src/servala/frontend/views/service.py | 30 ++++++++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/servala/frontend/views/organization.py b/src/servala/frontend/views/organization.py index 6545d39..8d07c04 100644 --- a/src/servala/frontend/views/organization.py +++ b/src/servala/frontend/views/organization.py @@ -3,6 +3,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import CreateView, DetailView from rules.contrib.views import AutoPermissionRequiredMixin +from servala.core.rules import is_organization_admin from servala.core.models import ( BillingEntity, Organization, @@ -75,10 +76,10 @@ class OrganizationDashboardView( ) recent_instances = service_instances.order_by("-created_at")[:5] + has_admin_permission = is_organization_admin(self.request.user, organization) + for instance in recent_instances: - instance.has_change_permission = self.request.user.has_perm( - "core.change_serviceinstance", instance - ) + instance.has_change_permission = has_admin_permission user_membership = OrganizationMembership.objects.filter( user=self.request.user, organization=organization diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index 2d74842..223a47f 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -6,6 +6,7 @@ from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView, UpdateView +from servala.core.rules import is_organization_admin from servala.core.crd import deslugify from servala.core.models import ( ControlPlaneCRD, @@ -219,12 +220,13 @@ class ServiceInstanceDetailView( 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 - ) - context["has_delete_permission"] = self.request.user.has_perm( - ServiceInstance.get_perm("delete"), self.object + + has_admin_permission = is_organization_admin( + self.request.user, self.object.organization ) + + context["has_change_permission"] = has_admin_permission + context["has_delete_permission"] = has_admin_permission return context def get_nested_spec(self): @@ -338,6 +340,15 @@ class ServiceInstanceUpdateView( template_name = "frontend/organizations/service_instance_update.html" permission_type = "change" + def has_permission(self): + """Override to use organization role-based permissions.""" + # First check if user has organization access + if not self.has_organization_permission(): + return False + + # Then check if user has admin or owner role + return is_organization_admin(self.request.user, self.object.organization) + def get_form_class(self): return self.object.context.model_form_class @@ -420,6 +431,15 @@ class ServiceInstanceDeleteView( form_class = ServiceInstanceDeleteForm permission_type = "delete" + def has_permission(self): + """Override to use organization role-based permissions.""" + # First check if user has organization access + if not self.has_organization_permission(): + return False + + # Then check if user has admin or owner role + return is_organization_admin(self.request.user, self.object.organization) + def form_valid(self, form): try: self.object.delete_instance(user=self.request.user) From e78a63c67f0ddfda3833633eecaa6f552055d692 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 7 Jul 2025 13:35:54 +0200 Subject: [PATCH 03/18] add link to support form on error message --- src/servala/core/models/service.py | 72 +++++++++++++++++++-------- src/servala/frontend/views/mixins.py | 24 +++++++++ src/servala/frontend/views/service.py | 23 ++++++--- 3 files changed, 92 insertions(+), 27 deletions(-) diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index 362661b..ebc2878 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -10,6 +10,7 @@ from django.core.exceptions import ValidationError from django.db import IntegrityError, models, transaction from django.utils import timezone from django.utils.functional import cached_property +from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ from encrypted_fields.fields import EncryptedJSONField from kubernetes import client, config @@ -19,6 +20,10 @@ from servala.core import rules as perms from servala.core.models.mixins import ServalaModelMixin from servala.core.validators import kubernetes_name_validator +SUPPORT_MESSAGE_TEMPLATE = _( + "Need help? We're happy to help via the support form." +) + class ServiceCategory(ServalaModelMixin, models.Model): """ @@ -615,10 +620,9 @@ class ServiceInstance(ServalaModelMixin, models.Model): context=context, ) except IntegrityError: - raise ValidationError( - _( - "An instance with this name already exists in this organization. Please choose a different name." - ) + raise cls._format_error_with_support( + "An instance with this name already exists in this organization. Please choose a different name.", + organization, ) try: @@ -657,10 +661,16 @@ class ServiceInstance(ServalaModelMixin, models.Model): try: error_body = json.loads(e.body) reason = error_body.get("message", str(e)) - raise ValidationError(_("Kubernetes API error: {}").format(reason)) + raise cls._format_error_with_support( + "Kubernetes API error: {error}.", organization, error=reason + ) except (ValueError, TypeError): - raise ValidationError(_("Kubernetes API error: {}").format(str(e))) - raise ValidationError(_("Error creating instance: {}").format(str(e))) + raise cls._format_error_with_support( + "Kubernetes API error: {error}.", organization, error=str(e) + ) + raise cls._format_error_with_support( + "Error creating instance: {error}.", organization, error=str(e) + ) return instance def update_spec(self, spec_data, updated_by): @@ -681,28 +691,27 @@ class ServiceInstance(ServalaModelMixin, models.Model): self.save() # Updates updated_at timestamp except ApiException as e: if e.status == 404: - raise ValidationError( - _( - "Service instance not found in Kubernetes. It may have been deleted externally." - ) + raise self._format_error_with_support( + "Service instance not found in Kubernetes. It may have been deleted externally.", + self.organization, ) try: error_body = json.loads(e.body) reason = error_body.get("message", str(e)) - raise ValidationError( - _("Kubernetes API error updating instance: {error}").format( - error=reason - ) + raise self._format_error_with_support( + "Kubernetes API error updating instance: {error}.", + self.organization, + error=reason, ) except (ValueError, TypeError): - raise ValidationError( - _("Kubernetes API error updating instance: {error}").format( - error=str(e) - ) + raise self._format_error_with_support( + "Kubernetes API error updating instance: {error}.", + self.organization, + error=str(e), ) except Exception as e: - raise ValidationError( - _("Error updating instance: {error}").format(error=str(e)) + raise self._format_error_with_support( + "Error updating instance: {error}.", self.organization, error=str(e) ) @transaction.atomic @@ -854,3 +863,24 @@ class ServiceInstance(ServalaModelMixin, models.Model): return {"error": str(e)} except Exception as e: return {"error": str(e)} + + @classmethod + def _format_error_with_support(cls, message, organization, **kwargs): + """ + Helper method to format error messages with support links. + + Args: + message: The error message template (without support text) + organization: The organization object to get the support URL + **kwargs: Additional format parameters for the message + + Returns: + A ValidationError with the formatted message including support link + """ + support_url = organization.urls.support + # Combine the main message with the support message template + full_message = _("{message} {support_message}").format( + message=_(message).format(**kwargs), + support_message=SUPPORT_MESSAGE_TEMPLATE.format(support_url=support_url), + ) + return ValidationError(mark_safe(full_message)) diff --git a/src/servala/frontend/views/mixins.py b/src/servala/frontend/views/mixins.py index fd5a494..7e7b453 100644 --- a/src/servala/frontend/views/mixins.py +++ b/src/servala/frontend/views/mixins.py @@ -1,9 +1,14 @@ from django.utils.functional import cached_property +from django.utils.safestring import mark_safe +from django.utils.translation import gettext_lazy as _ from django.views.generic import UpdateView from rules.contrib.views import AutoPermissionRequiredMixin, PermissionRequiredMixin from servala.core.models import Organization +# Import the support message template from the service model +from servala.core.models.service import SUPPORT_MESSAGE_TEMPLATE + class HtmxViewMixin: fragments = [] @@ -90,3 +95,22 @@ class OrganizationViewMixin(PermissionRequiredMixin): def has_permission(self): return self.has_organization_permission() and super().has_permission() + + def format_error_with_support(self, message, **kwargs): + """ + Helper method to format error messages with support links for frontend views. + + Args: + message: The error message template (without support text) + **kwargs: Additional format parameters for the message + + Returns: + A formatted message with support link + """ + support_url = self.organization.urls.support + # Combine the main message with the support message template + full_message = _("{message} {support_message}").format( + message=_(message).format(**kwargs), + support_message=SUPPORT_MESSAGE_TEMPLATE.format(support_url=support_url), + ) + return mark_safe(full_message) diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index 2d74842..49ac683 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -144,7 +144,10 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView form = self.get_instance_form() if not form: # Should not happen if context_object is valid, but as a safeguard - messages.error(self.request, _("Could not initialize service form.")) + messages.error( + self.request, + self.format_error_with_support("Could not initialize service form."), + ) return self.render_to_response(context) if form.is_valid(): @@ -161,7 +164,10 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView messages.error(self.request, e.message or str(e)) except Exception as e: messages.error( - self.request, _("Error creating instance: {}").format(str(e)) + self.request, + self.format_error_with_support( + "Error creating instance: {error}.", error=str(e) + ), ) # If the form is not valid or if the service creation failed, we render it again @@ -366,7 +372,10 @@ class ServiceInstanceUpdateView( return self.form_invalid(form) except Exception as e: messages.error( - self.request, _("Error updating instance: {error}").format(error=str(e)) + self.request, + self.format_error_with_support( + "Error updating instance: {error}.", error=str(e) + ), ) return self.form_invalid(form) @@ -435,9 +444,11 @@ 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)), + self.format_error_with_support( + "An error occurred while trying to delete instance '{name}': {error}.", + name=self.object.name, + error=str(e), + ), ) response = HttpResponse() response["HX-Redirect"] = str(self.object.urls.base) From 5fa3d32c57549b11c48bc285a09616c59150a0ce Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 7 Jul 2025 13:38:09 +0200 Subject: [PATCH 04/18] do not dismiss message when error message --- src/servala/frontend/templates/includes/message.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/servala/frontend/templates/includes/message.html b/src/servala/frontend/templates/includes/message.html index 59260d2..d5debc9 100644 --- a/src/servala/frontend/templates/includes/message.html +++ b/src/servala/frontend/templates/includes/message.html @@ -9,7 +9,7 @@