Merge pull request 'Hide duplicate form error and improve error message' (#324) from 225-simple-form-errors into main

Reviewed-on: #324

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 %}
         </div>
     </div>

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

@@ -26,7 +26,7 @@
                     {% 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 %}
+                {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form hide_form_errors=True %}
             {% endif %}
         </div>
     </div>

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." %}
                 </div>
             {% 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 %}
         </div>
     </div>

src/servala/frontend/templates/includes/tabbed_fieldset_form.html

@@ -1,7 +1,9 @@
 {% load i18n %}
 {% load get_field %}
 {% load static %}
+{% if not hide_form_errors %}
     {% include "frontend/forms/errors.html" %}
+{% endif %}
 {% if form and expert_form and not hide_expert_mode %}
     <div class="mb-3 text-end">
         <a href="#"

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

src/servala/frontend/urls.py

@@ -47,8 +47,8 @@ urlpatterns = [
                 ),
                 path(
                     "services/<slug:slug>/offering/<int:pk>/",
-                    views.ServiceOfferingDetailView.as_view(),
+                    views.ServiceInstanceCreateView.as_view(),
-                    name="organization.offering",
+                    name="organization.instance.create",
                 ),
                 path(
                     "",

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",

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):
+class ServiceInstanceCreateView(OrganizationViewMixin, HtmxViewMixin, DetailView):
-    template_name = "frontend/organizations/service_offering_detail.html"
+    template_name = "frontend/organizations/service_instance_create.html"
     context_object_name = "offering"
     model = ServiceOffering
     permission_type = "view"

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

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