From 94f95a866477638bb587e476d2a9d27fcd1aa6b9 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Fri, 5 Dec 2025 15:21:20 +0100 Subject: [PATCH 1/4] Improve error message --- src/servala/frontend/templates/frontend/forms/errors.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/servala/frontend/templates/frontend/forms/errors.html b/src/servala/frontend/templates/frontend/forms/errors.html index 964687d..1d31737 100644 --- a/src/servala/frontend/templates/frontend/forms/errors.html +++ b/src/servala/frontend/templates/frontend/forms/errors.html @@ -11,7 +11,7 @@ {{ form.non_field_errors.0 }} {% endif %} {% else %} - {% translate "We could not save your changes." %} + {% translate "Please review and correct the errors highlighted in the form below." %} {% endif %} From acb6ac15387c36e432025a93b4aa005454321ebb Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Fri, 5 Dec 2025 15:21:34 +0100 Subject: [PATCH 2/4] Stop showing duplicate error messages --- .../frontend/organizations/service_instance_update.html | 2 +- .../frontend/organizations/service_offering_detail.html | 2 +- .../frontend/templates/includes/tabbed_fieldset_form.html | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_update.html b/src/servala/frontend/templates/frontend/organizations/service_instance_update.html index 021be3c..c528474 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_instance_update.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instance_update.html @@ -22,7 +22,7 @@ {% translate "Oops! Something went wrong with the service form generation. Please try again later." %} {% else %} - {% include "includes/tabbed_fieldset_form.html" with form=custom_form expert_form=form %} + {% include "includes/tabbed_fieldset_form.html" with form=custom_form expert_form=form hide_form_errors=True %} {% endif %} diff --git a/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html b/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html index 39b69a8..8743abf 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html +++ b/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html @@ -26,7 +26,7 @@ {% translate "Oops! Something went wrong with the service form generation. Please try again later." %} {% else %} - {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form %} + {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form hide_form_errors=True %} {% endif %} diff --git a/src/servala/frontend/templates/includes/tabbed_fieldset_form.html b/src/servala/frontend/templates/includes/tabbed_fieldset_form.html index f54b1ae..c72976a 100644 --- a/src/servala/frontend/templates/includes/tabbed_fieldset_form.html +++ b/src/servala/frontend/templates/includes/tabbed_fieldset_form.html @@ -1,7 +1,9 @@ {% load i18n %} {% load get_field %} {% load static %} -{% include "frontend/forms/errors.html" %} +{% if not hide_form_errors %} + {% include "frontend/forms/errors.html" %} +{% endif %} {% if form and expert_form and not hide_expert_mode %}
Date: Fri, 5 Dec 2025 15:24:41 +0100 Subject: [PATCH 3/4] Improve view naming for easier dev navigation --- ...ce_offering_detail.html => service_instance_create.html} | 0 src/servala/frontend/urls.py | 4 ++-- src/servala/frontend/views/__init__.py | 4 ++-- src/servala/frontend/views/service.py | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) rename src/servala/frontend/templates/frontend/organizations/{service_offering_detail.html => service_instance_create.html} (100%) diff --git a/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html b/src/servala/frontend/templates/frontend/organizations/service_instance_create.html similarity index 100% rename from src/servala/frontend/templates/frontend/organizations/service_offering_detail.html rename to src/servala/frontend/templates/frontend/organizations/service_instance_create.html diff --git a/src/servala/frontend/urls.py b/src/servala/frontend/urls.py index 73d0759..68f0b89 100644 --- a/src/servala/frontend/urls.py +++ b/src/servala/frontend/urls.py @@ -47,8 +47,8 @@ urlpatterns = [ ), path( "services//offering//", - views.ServiceOfferingDetailView.as_view(), - name="organization.offering", + views.ServiceInstanceCreateView.as_view(), + name="organization.instance.create", ), path( "", diff --git a/src/servala/frontend/views/__init__.py b/src/servala/frontend/views/__init__.py index 33b0560..3307754 100644 --- a/src/servala/frontend/views/__init__.py +++ b/src/servala/frontend/views/__init__.py @@ -16,12 +16,12 @@ from .organization import ( ) from .service import ( ServiceDetailView, + ServiceInstanceCreateView, ServiceInstanceDeleteView, ServiceInstanceDetailView, ServiceInstanceListView, ServiceInstanceUpdateView, ServiceListView, - ServiceOfferingDetailView, ) from .support import SupportView @@ -35,12 +35,12 @@ __all__ = [ "OrganizationSelectionView", "OrganizationUpdateView", "ServiceDetailView", + "ServiceInstanceCreateView", "ServiceInstanceDeleteView", "ServiceInstanceDetailView", "ServiceInstanceListView", "ServiceInstanceUpdateView", "ServiceListView", - "ServiceOfferingDetailView", "ProfileView", "SupportView", "custom_404", diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index b9f7d56..66e9f75 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -83,7 +83,7 @@ class ServiceDetailView(OrganizationViewMixin, DetailView): if self.visible_offerings.count() == 1: offering = self.visible_offerings.first() return redirect( - "frontend:organization.offering", + "frontend:organization.instance.create", organization=self.request.organization.slug, slug=self.object.slug, pk=offering.pk, @@ -97,8 +97,8 @@ class ServiceDetailView(OrganizationViewMixin, DetailView): return context -class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView): - template_name = "frontend/organizations/service_offering_detail.html" +class ServiceInstanceCreateView(OrganizationViewMixin, HtmxViewMixin, DetailView): + template_name = "frontend/organizations/service_instance_create.html" context_object_name = "offering" model = ServiceOffering permission_type = "view" From a68ad6b8a564f29029352760898b76040c084cf1 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Fri, 5 Dec 2025 16:39:11 +0100 Subject: [PATCH 4/4] Increase test coverage --- .../frontend/templatetags/version_tags.py | 2 +- src/tests/conftest.py | 29 +++- src/tests/test_rules.py | 151 ++++++++++++++++++ 3 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 src/tests/test_rules.py diff --git a/src/servala/frontend/templatetags/version_tags.py b/src/servala/frontend/templatetags/version_tags.py index 6019738..2a94030 100644 --- a/src/servala/frontend/templatetags/version_tags.py +++ b/src/servala/frontend/templatetags/version_tags.py @@ -13,4 +13,4 @@ def get_version_or_env(): env = os.environ.get("SERVALA_ENVIRONMENT", "development") if env == "production": return __version__ - return env + return env # pragma: no cover diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 859b4ca..d80aed8 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -42,13 +42,36 @@ 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): - user = User.objects.create(email="user@example.org", password="example") + owner = User.objects.create(email="owner@example.org", password="example") OrganizationMembership.objects.create( - organization=organization, user=user, role="owner" + organization=organization, user=owner, role="owner" ) - return user + return owner + + +@pytest.fixture +def org_admin(organization): + admin = User.objects.create(email="admin@example.org", password="example") + OrganizationMembership.objects.create( + organization=organization, user=admin, role="admin" + ) + return admin + + +@pytest.fixture +def org_member(organization): + member = User.objects.create(email="member@example.org", password="example") + OrganizationMembership.objects.create( + organization=organization, user=member, role="member" + ) + return member @pytest.fixture diff --git a/src/tests/test_rules.py b/src/tests/test_rules.py new file mode 100644 index 0000000..9a83163 --- /dev/null +++ b/src/tests/test_rules.py @@ -0,0 +1,151 @@ +import pytest +from django_scopes import scope + +from servala.core.models.organization import OrganizationRole +from servala.core.rules import ( + has_organization_role, + is_organization_admin, + is_organization_member, + is_organization_owner, +) + +pytestmark = pytest.mark.django_db + + +def test_has_organization_role_returns_false_for_non_organization_object(user): + assert has_organization_role(user, "not an organization", None) is False + + +def test_has_organization_role_returns_false_for_non_member(user, organization): + with scope(organization=organization): + assert has_organization_role(user, organization, None) is False + + +@pytest.mark.parametrize("user_fixture", ["org_owner", "org_admin", "org_member"]) +def test_has_organization_role_returns_true_for_any_member( + user_fixture, organization, request +): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert has_organization_role(user, organization, None) is True + + +@pytest.mark.parametrize( + "user_fixture,roles,expected", + [ + ("org_owner", [OrganizationRole.OWNER], True), + ("org_owner", [OrganizationRole.ADMIN], False), + ("org_owner", [OrganizationRole.MEMBER], False), + ("org_owner", [OrganizationRole.OWNER, OrganizationRole.ADMIN], True), + ("org_admin", [OrganizationRole.ADMIN], True), + ("org_admin", [OrganizationRole.OWNER], False), + ("org_admin", [OrganizationRole.OWNER, OrganizationRole.ADMIN], True), + ("org_member", [OrganizationRole.MEMBER], True), + ("org_member", [OrganizationRole.OWNER], False), + ("org_member", [OrganizationRole.ADMIN], False), + ], +) +def test_has_organization_role_filters_by_roles( + user_fixture, roles, expected, organization, request +): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert has_organization_role(user, organization, roles) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", False), + ("org_member", False), + ], +) +def test_is_organization_owner(user_fixture, expected, organization, request): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert is_organization_owner(user, organization) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", False), + ("org_member", False), + ], +) +def test_is_organization_owner_with_related_object( + user_fixture, expected, organization, mocker, request +): + user = request.getfixturevalue(user_fixture) + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_owner(user, obj) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", True), + ("org_member", False), + ], +) +def test_is_organization_admin(user_fixture, expected, organization, request): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert is_organization_admin(user, organization) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", True), + ("org_member", False), + ], +) +def test_is_organization_admin_with_related_object( + user_fixture, expected, organization, mocker, request +): + user = request.getfixturevalue(user_fixture) + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_admin(user, obj) is expected + + +@pytest.mark.parametrize("user_fixture", ["org_owner", "org_admin", "org_member"]) +def test_is_organization_member_returns_true_for_members( + user_fixture, organization, request +): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert is_organization_member(user, organization) is True + + +def test_is_organization_member_returns_false_for_non_member(user, organization): + with scope(organization=organization): + assert is_organization_member(user, organization) is False + + +@pytest.mark.parametrize("user_fixture", ["org_owner", "org_admin", "org_member"]) +def test_is_organization_member_with_related_object( + user_fixture, organization, mocker, request +): + user = request.getfixturevalue(user_fixture) + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_member(user, obj) is True + + +def test_is_organization_member_with_related_object_returns_false_for_non_member( + user, organization, mocker +): + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_member(user, obj) is False