👌(!fixup) first review

src/backend/joanie/core/admin.py

@@ -1,38 +1,45 @@
 """
 Core application admin
 """
-from operator import concat
 
 from django.contrib import admin, messages
 from django.contrib.auth import admin as auth_admin
-from django.core import management
 from django.http import HttpResponseRedirect
 from django.urls import re_path, reverse
 from django.utils.html import format_html
 from django.utils.translation import gettext_lazy as _
+from django.utils.translation import ngettext_lazy
 
 from adminsortable2.admin import SortableInlineAdminMixin
 from django_object_actions import DjangoObjectActions, takes_instance_or_queryset
 from parler.admin import TranslatableAdmin
 
+from joanie.core import helpers
 from joanie.core.enums import PRODUCT_TYPE_CERTIFICATE_ALLOWED
 
 from . import models
 
+ACTION_NAME_GENERATE_CERTIFICATES = "generate_certificates"
+ACTION_NAME_CANCEL = "cancel"
 
-def resume_certification_to_user(request, count):
+
+def summarize_certification_to_user(request, count):
     """
     Display a message after create_certificates command has been launched
     """
-    if count == "0":
+    if count == 0:
         messages.warning(
             request,
-            _("No certificate has been generated.").format(count),
+            _("No certificates have been generated."),
         )
     else:
         messages.success(
             request,
-            _("{:s} certificate(s) has been generated.").format(count),
+            ngettext_lazy(  # pylint: disable=no-member
+                "{:d} certificate has been generated.",
+                "{:d} certificates have been generated.",
+                count,
+            ).format(count),
         )
 
 
@@ -54,8 +61,8 @@ class CertificateAdmin(admin.ModelAdmin):
 class CourseAdmin(DjangoObjectActions, TranslatableAdmin):
     """Admin class for the Course model"""
 
-    actions = ("generate_certificates",)
-    change_actions = ("generate_certificates",)
+    actions = (ACTION_NAME_GENERATE_CERTIFICATES,)
+    change_actions = (ACTION_NAME_GENERATE_CERTIFICATES,)
     list_display = ("code", "title", "organization", "state")
     filter_vertical = ("products",)
     fieldsets = (
@@ -77,12 +84,11 @@ def generate_certificates(self, request, queryset):  # pylint: disable=no-self-u
         Custom action to launch create_certificates management command
         over the selected courses
         """
-        codes = [course.code for course in queryset]
-        certificate_generated_count = management.call_command(
-            "create_certificates", courses=codes
+        certificate_generated_count = helpers.generate_certificates_for_orders(
+            models.Order.objects.filter(course__in=queryset)
         )
 
-        resume_certification_to_user(request, certificate_generated_count)
+        summarize_certification_to_user(request, certificate_generated_count)
 
 
 @admin.register(models.CourseRun)
@@ -138,20 +144,21 @@ class ProductAdmin(DjangoObjectActions, TranslatableAdmin):
 
     inlines = (ProductCourseRelationInline,)
     readonly_fields = ("related_courses",)
-    actions = ("generate_certificates",)
-    change_actions = ("generate_certificates",)
+    actions = (ACTION_NAME_GENERATE_CERTIFICATES,)
+    change_actions = (ACTION_NAME_GENERATE_CERTIFICATES,)
 
     def get_change_actions(self, request, object_id, form_url):
         """
-        Remove the generate_certificates action to the action list
+        Remove the generate_certificates action from list of actions
         if the product instance is not certifying
         """
         actions = super().get_change_actions(request, object_id, form_url)
         actions = list(actions)
 
-        obj = self.model.objects.get(pk=object_id)
-        if obj.type not in PRODUCT_TYPE_CERTIFICATE_ALLOWED:
-            actions.remove("generate_certificates")
+        if not self.model.objects.filter(
+            pk=object_id, type__in=PRODUCT_TYPE_CERTIFICATE_ALLOWED
+        ).exists():
+            actions.remove(ACTION_NAME_GENERATE_CERTIFICATES)
 
         return actions
 
@@ -165,7 +172,7 @@ def get_urls(self):
             re_path(
                 r"^(?P<product_id>.+)/generate-certificates/(?P<course_code>.+)/$",
                 self.admin_site.admin_view(self.generate_certificates_for_course),
-                name="generate_certificates",
+                name=ACTION_NAME_GENERATE_CERTIFICATES,
             )
         ] + url_patterns
 
@@ -175,24 +182,25 @@ def generate_certificates(self, request, queryset):  # pylint: disable=no-self-u
         Custom action to launch create_certificates management command
         over the selected products
         """
-        uids = [product.uid for product in queryset]
-        certificate_generated_count = management.call_command(
-            "create_certificates", products=uids
+        certificate_generated_count = helpers.generate_certificates_for_orders(
+            models.Order.objects.filter(product__in=queryset)
         )
 
-        resume_certification_to_user(request, certificate_generated_count)
+        summarize_certification_to_user(request, certificate_generated_count)
 
-    def generate_certificates_for_course(self, request, product_id, course_code):
+    def generate_certificates_for_course(
+        self, request, product_id, course_code
+    ):  # pylint: disable=no-self-use
         """
         A custom action to generate certificates for a course - product couple.
         """
-        product = self.model.objects.get(id=product_id)
-
-        certificate_generated_count = management.call_command(
-            "create_certificates", courses=course_code, products=product.uid
+        certificate_generated_count = helpers.generate_certificates_for_orders(
+            models.Order.objects.filter(
+                product__id=product_id, course__code=course_code
+            )
         )
 
-        resume_certification_to_user(request, certificate_generated_count)
+        summarize_certification_to_user(request, certificate_generated_count)
 
         return HttpResponseRedirect(
             reverse("admin:core_product_change", args=(product_id,))
@@ -203,31 +211,47 @@ def related_courses(self, obj):  # pylint: disable=no-self-use
         """
         Retrieve courses related to the product
         """
+        return self.get_related_courses_as_html(obj)
+
+    @staticmethod
+    def get_related_courses_as_html(obj):  # pylint: disable=no-self-use
+        """
+        Get the html representation of the product's related courses
+        """
         related_courses = obj.courses.all()
         is_certifying = obj.type in PRODUCT_TYPE_CERTIFICATE_ALLOWED
 
         if related_courses:
-            items = [
-                concat(
-                    (
-                        '<li style="margin-bottom: 1rem">'
-                        f"<a href='{reverse('admin:core_course_change', args=(course.id,),)}'>"
-                        f"{course.code} | {course.title}"
-                        "</a>"
-                    ),
-                    (
-                        f'<a style="margin-left: 1rem" class="button" '
-                        f'href="{reverse("admin:generate_certificates", kwargs={"product_id": obj.id, "course_code": course.code}, )}">'  # noqa pylint: disable=line-too-long
+            items = []
+            for course in obj.courses.all():
+                change_course_url = reverse(
+                    "admin:core_course_change",
+                    args=(course.id,),
+                )
+
+                raw_html = (
+                    '<li style="margin-bottom: 1rem">'
+                    f"<a href='{change_course_url}'>{course.code} | {course.title}</a>"
+                )
+
+                if is_certifying:
+                    # Add a button go generate certificate
+                    generate_certificates_url = reverse(
+                        f"admin:{ACTION_NAME_GENERATE_CERTIFICATES}",
+                        kwargs={"product_id": obj.id, "course_code": course.code},
+                    )
+
+                    raw_html += (
+                        f'<a style="margin-left: 1rem" class="button" href="{generate_certificates_url}">'  # noqa pylint: disable=line-too-long
                         f'{_("Generate certificates")}'
                         "</a>"
-                        "</li>"
                     )
-                    if is_certifying
-                    else "",
-                )
-                for course in obj.courses.all()
-            ]
+
+                raw_html += "</li>"
+                items.append(raw_html)
+
             return format_html(f"<ul style='margin: 0'>{''.join(items)}</ul>")
+
         return "-"
 
 
@@ -237,8 +261,8 @@ class OrderAdmin(DjangoObjectActions, admin.ModelAdmin):
 
     list_display = ("uid", "owner", "product", "state")
     readonly_fields = ("total", "invoice")
-    change_actions = ("generate_certificate",)
-    actions = ("cancel", "generate_certificate")
+    change_actions = (ACTION_NAME_GENERATE_CERTIFICATES,)
+    actions = (ACTION_NAME_CANCEL, ACTION_NAME_GENERATE_CERTIFICATES)
 
     @admin.action(description=_("Cancel selected orders"))
     def cancel(self, request, queryset):  # pylint: disable=no-self-use
@@ -247,16 +271,13 @@ def cancel(self, request, queryset):  # pylint: disable=no-self-use
             order.cancel()
 
     @takes_instance_or_queryset
-    def generate_certificate(self, request, queryset):  # pylint: disable=no-self-use
+    def generate_certificates(self, request, queryset):  # pylint: disable=no-self-use
         """
         Custom action to launch create_certificates management commands
         over the order selected
         """
-        ids = [order.uid for order in queryset]
-        certificate_generated_count = management.call_command(
-            "create_certificates", orders=ids
-        )
-        resume_certification_to_user(request, certificate_generated_count)
+        certificate_generated_count = helpers.generate_certificates_for_orders(queryset)
+        summarize_certification_to_user(request, certificate_generated_count)
 
     def invoice(self, obj):  # pylint: disable=no-self-use
         """Retrieve the root invoice related to the order."""

src/backend/joanie/core/helpers.py

@@ -0,0 +1,100 @@
+"""
+Helpers that can be useful throughout Joanie's core app
+"""
+from django.core.exceptions import ValidationError
+from django.utils import timezone
+
+from joanie.core import enums, models
+
+
+def generate_certificate_for_order(order):
+    """
+    Check if order is eligible for certification then generate certificate is it is.
+
+    Eligibility means that order contains
+    one passed enrollment per graded courses.
+
+    Return:
+        0 : if the order is not eligible for certification
+        1: if a certificate has been generated for the current order
+    """
+    graded_courses = order.target_courses.filter(
+        order_relations__is_graded=True
+    ).order_by("order_relations__position")
+    graded_courses_count = len(graded_courses)
+
+    if graded_courses_count == 0:
+        return 0
+
+    course_runs = models.CourseRun.objects.filter(
+        course__in=graded_courses,
+        is_gradable=True,
+        start__lte=timezone.now(),
+    )
+
+    enrollments = order.enrollments.filter(
+        course_run__in=course_runs, is_active=True
+    ).select_related("user", "course_run")
+
+    # Cross graded courses and enrollments to check there is an active enrollment
+    # for each graded courses, if not it is useless to go further
+    course_enrollments = []
+    for course in graded_courses:
+        for enrollment in enrollments.iterator():
+            # Check if the enrollment relies on course by crossing
+            # all course runs implied
+            if course.course_runs.filter(
+                resource_link=enrollment.course_run.resource_link
+            ).exists():
+                course_enrollments.append(enrollment)
+                break
+
+    # If we do not have one enrollment per graded course, there is no need to
+    # continue, we are sure that order is not eligible for certification.
+    if len(course_enrollments) != graded_courses_count:
+        return 0
+
+    # Otherwise, we now need to know if each enrollment has been passed
+    passed_enrollment_count = 0
+    for enrollment in course_enrollments:
+        if enrollment.is_passed is False:
+            # If one enrollment has not been passed, no need to continue,
+            # We are sure that order is not eligible for certification.
+            break
+        passed_enrollment_count += 1
+
+    if passed_enrollment_count != graded_courses_count:
+        return 0
+
+    try:
+        order.create_certificate()
+    except ValidationError:
+        return 0
+
+    return 1
+
+
+def generate_certificates_for_orders(orders):
+    """
+    Iterate over the provided orders and check if they are eligible for certification
+    then return the count of generated certificates.
+    """
+    total = 0
+
+    orders = [
+        order
+        for order in orders.filter(
+            is_canceled=False,
+            certificate__isnull=True,
+            product__type__in=enums.PRODUCT_TYPE_CERTIFICATE_ALLOWED,
+        )
+        .select_related("course__organization")
+        .iterator()
+        if order.state == enums.ORDER_STATE_VALIDATED
+    ]
+
+    for order in orders:
+        result = generate_certificate_for_order(order)
+        total += result
+
+    return total