👔(backend) update organization assignation logic

CHANGELOG.md

@@ -11,6 +11,8 @@ and this project adheres to
 ### Changed
 
 - Add course offer information into course webhook synchronization payload
+- Update organization assignation logic to ignore order with pending signature
+  or without payment method defined.
 
 ## [2.17.1] - 2025-03-06
 

src/backend/joanie/core/api/client/__init__.py

@@ -11,20 +11,12 @@
 from django.core.exceptions import ValidationError
 from django.core.files.storage import storages
 from django.db import IntegrityError, transaction
-from django.db.models import (
-    Count,
-    Exists,
-    OuterRef,
-    Prefetch,
-    Q,
-    Subquery,
-)
+from django.db.models import OuterRef, Prefetch, Subquery
 from django.http import FileResponse, Http404, HttpResponse, JsonResponse
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 
-from babel.util import distinct
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import OpenApiParameter, extend_schema
 from rest_framework import mixins, viewsets
@@ -42,6 +34,7 @@
 from joanie.core.tasks import generate_zip_archive_task
 from joanie.core.utils import contract as contract_utility
 from joanie.core.utils import contract_definition, issuers
+from joanie.core.utils.organization import get_least_active_organization
 from joanie.core.utils.payment_schedule import generate as generate_payment_schedule
 from joanie.core.utils.signature import check_signature
 from joanie.payment import enums as payment_enums
@@ -340,46 +333,6 @@ def perform_create(self, serializer):
         """Force the order's "owner" field to the logged-in user."""
         serializer.save(owner=self.request.user)
 
-    def _get_organization_with_least_active_orders(
-        self, product, course, enrollment=None
-    ):
-        """
-        Return the organization with the least not canceled order count
-        for a given product and course.
-        """
-        course_id = course.id if course else enrollment.course_run.course_id
-
-        try:
-            course_relation = product.course_relations.get(course_id=course_id)
-        except models.CourseProductRelation.DoesNotExist:
-            return None
-
-        order_count_filter = Q(order__product=product) & ~Q(
-            order__state__in=[
-                enums.ORDER_STATE_DRAFT,
-                enums.ORDER_STATE_ASSIGNED,
-                enums.ORDER_STATE_CANCELED,
-            ]
-        )
-        if enrollment:
-            order_count_filter &= Q(order__enrollment=enrollment)
-        else:
-            order_count_filter &= Q(order__course=course)
-
-        try:
-            organizations = course_relation.organizations.annotate(
-                order_count=Count("order", filter=order_count_filter, distinct=True),
-                is_author=Exists(
-                    models.Organization.objects.filter(
-                        pk=OuterRef("pk"), courses__id=course_id
-                    )
-                ),
-            )
-
-            return organizations.order_by("order_count", "-is_author", "?").first()
-        except models.Organization.DoesNotExist:
-            return None
-
     # pylint: disable=too-many-return-statements
     @transaction.atomic
     def create(self, request, *args, **kwargs):
@@ -427,9 +380,7 @@ def create(self, request, *args, **kwargs):
             course = enrollment.course_run.course
 
         if not serializer.initial_data.get("organization_id"):
-            organization = self._get_organization_with_least_active_orders(
-                product, course, enrollment
-            )
+            organization = get_least_active_organization(product, course, enrollment)
             if organization:
                 serializer.initial_data["organization_id"] = organization.id
 

src/backend/joanie/core/utils/organization.py

@@ -0,0 +1,41 @@
+"""Util to get the organization with the least binding orders count"""
+
+from django.db.models import Count, Exists, OuterRef, Q
+
+from joanie.core import enums, models
+
+
+def get_least_active_organization(product, course, enrollment=None):
+    """
+    Return the organization with the least binding orders count
+    for a given product and course.
+    """
+    course_id = course.id if course else enrollment.course_run.course_id
+
+    try:
+        course_relation = product.course_relations.get(course_id=course_id)
+    except models.CourseProductRelation.DoesNotExist:
+        return None
+
+    order_count_filter = Q(order__product=product) & Q(
+        order__state__in=enums.ORDER_STATES_BINDING
+    )
+
+    if enrollment:
+        order_count_filter &= Q(order__enrollment=enrollment)
+    else:
+        order_count_filter &= Q(order__course=course)
+
+    try:
+        organizations = course_relation.organizations.annotate(
+            order_count=Count("order", filter=order_count_filter, distinct=True),
+            is_author=Exists(
+                models.Organization.objects.filter(
+                    pk=OuterRef("pk"), courses__id=course_id
+                )
+            ),
+        )
+
+        return organizations.order_by("order_count", "-is_author", "?").first()
+    except models.Organization.DoesNotExist:
+        return None

src/backend/joanie/tests/core/api/order/test_create.py

@@ -11,7 +11,7 @@
 from django.utils import timezone
 
 from joanie.core import enums, factories, models
-from joanie.core.api.client import OrderViewSet
+from joanie.core.api import client as api_client
 from joanie.core.models import CourseState
 from joanie.core.serializers import fields
 from joanie.payment.factories import BillingAddressDictFactory, CreditCardFactory
@@ -593,9 +593,7 @@ def test_api_order_create_should_auto_assign_organization(self):
             # Now order should have an organization set
             self.assertIsNotNone(order.organization)
 
-    @mock.patch.object(
-        OrderViewSet, "_get_organization_with_least_active_orders", return_value=None
-    )
+    @mock.patch.object(api_client, "get_least_active_organization", return_value=None)
     def test_api_order_create_should_auto_assign_organization_if_needed(
         self, mocked_round_robin
     ):
@@ -662,45 +660,35 @@ def test_api_order_create_auto_assign_organization_with_least_orders(self):
             organizations=[organization, expected_organization]
         )
 
-        # Create 3 orders for the first organization (1 draft, 1 pending, 1 canceled)
-        factories.OrderFactory(
-            organization=organization,
-            product=relation.product,
-            course=relation.course,
-            state=enums.ORDER_STATE_DRAFT,
-        )
+        ignored_states = [
+            state
+            for [state, _] in enums.ORDER_STATE_CHOICES
+            if state not in enums.ORDER_STATES_BINDING
+        ]
+
+        # Create orders for the first organization (1 for each ignored, 1 take in account)
+        for state in ignored_states:
+            factories.OrderFactory(
+                organization=organization,
+                product=relation.product,
+                course=relation.course,
+                state=state,
+            )
         factories.OrderFactory(
             organization=organization,
             product=relation.product,
             course=relation.course,
             state=enums.ORDER_STATE_PENDING,
         )
-        factories.OrderFactory(
-            organization=organization,
-            product=relation.product,
-            course=relation.course,
-            state=enums.ORDER_STATE_CANCELED,
-        )
 
-        # 3 ignored orders for the second organization (1 draft, 1 assigned, 1 canceled)
-        factories.OrderFactory(
-            organization=expected_organization,
-            product=relation.product,
-            course=relation.course,
-            state=enums.ORDER_STATE_DRAFT,
-        )
-        factories.OrderFactory(
-            organization=expected_organization,
-            product=relation.product,
-            course=relation.course,
-            state=enums.ORDER_STATE_ASSIGNED,
-        )
-        factories.OrderFactory(
-            organization=expected_organization,
-            product=relation.product,
-            course=relation.course,
-            state=enums.ORDER_STATE_CANCELED,
-        )
+        # ignored orders for the second organization
+        for state in ignored_states:
+            factories.OrderFactory(
+                organization=expected_organization,
+                product=relation.product,
+                course=relation.course,
+                state=state,
+            )
 
         # Then create an order without organization
         data = {

src/backend/joanie/tests/core/utils/test_utils_organization.py

@@ -0,0 +1,91 @@
+"""Test suite for utils organization methods"""
+
+from django.test import TestCase
+
+from joanie.core import enums, factories
+from joanie.core.utils.organization import get_least_active_organization
+
+
+class UtilsOrganizationTestCase(TestCase):
+    """Test suite for utils organization methods"""
+
+    def setUp(self):
+        """Set up the test case"""
+        super().setUp()
+        self.organization_1, self.organization_2 = (
+            factories.OrganizationFactory.create_batch(2)
+        )
+        self.relation = factories.CourseProductRelationFactory(
+            organizations=[self.organization_1, self.organization_2]
+        )
+        self.course = self.relation.course
+        self.product = self.relation.product
+
+    def test_utils_organization_get_least_active_organization_no_orders(self):
+        """
+        With no orders, a random organization is returned.
+        """
+        selected_organization = get_least_active_organization(self.product, self.course)
+
+        # the first selected organization is random
+        self.assertIn(selected_organization, [self.organization_1, self.organization_2])
+
+        # Add a completed order to the selected organization
+        factories.OrderFactory(
+            product=self.product,
+            course=self.course,
+            organization=selected_organization,
+            state=enums.ORDER_STATE_COMPLETED,
+        )
+
+        # The next selected organization should be the other one
+        next_selected_organization = get_least_active_organization(
+            self.product, self.course
+        )
+        self.assertEqual(type(next_selected_organization), type(selected_organization))
+        self.assertNotEqual(next_selected_organization, selected_organization)
+
+    def test_utils_organization_get_least_active_organization_is_author(self):
+        """
+        With no order, and the first organization is the author, it is returned.
+        """
+        self.organization_1.courses.add(self.course)
+
+        selected_organization = get_least_active_organization(self.product, self.course)
+
+        self.assertEqual(selected_organization, self.organization_1)
+
+    def test_utils_organization_get_least_active_organization_all_states(self):
+        """
+        With one order to the first organization, the second organization is returned.
+        """
+        for state, _ in enums.ORDER_STATE_CHOICES:
+            with self.subTest(f"{state} order to the first organization", state=state):
+                self.setUp()
+                factories.OrderFactory(
+                    product=self.product,
+                    course=self.course,
+                    organization=self.organization_1,
+                    state=state,
+                )
+                # Add the course to the first organization to avoid randomness
+                self.organization_1.courses.add(self.course)
+                self.assertEqual(self.organization_1.courses.count(), 1)
+
+                selected_organization = get_least_active_organization(
+                    self.product, self.course
+                )
+
+                if state in [
+                    enums.ORDER_STATE_DRAFT,
+                    enums.ORDER_STATE_ASSIGNED,
+                    enums.ORDER_STATE_TO_SAVE_PAYMENT_METHOD,
+                    enums.ORDER_STATE_TO_SIGN,
+                    enums.ORDER_STATE_SIGNING,
+                    enums.ORDER_STATE_CANCELED,
+                    enums.ORDER_STATE_REFUNDING,
+                    enums.ORDER_STATE_REFUNDED,
+                ]:
+                    self.assertEqual(selected_organization, self.organization_1)
+                else:
+                    self.assertEqual(selected_organization, self.organization_2)