diff --git a/src/backend/joanie/core/admin.py b/src/backend/joanie/core/admin.py index cd0dab43b1..9c67674da7 100644 --- a/src/backend/joanie/core/admin.py +++ b/src/backend/joanie/core/admin.py @@ -25,7 +25,7 @@ def summarize_certification_to_user(request, count): """ - Display a message after create_certificates command has been launched + Display a message after generate_certificates command has been launched """ if count == 0: messages.warning( @@ -61,13 +61,11 @@ def certificate_definition(self, obj): # pylint: disable=no-self-use """Retrieve the certification definition from the related order.""" certificate_definition = obj.order.product.certificate_definition - return format_html( - ( - f"" - f"{str(certificate_definition)}" - "" - ) + url = reverse( + "admin:core_certificatedefinition_change", + args=(certificate_definition.id,), ) + return format_html(f"" f"{certificate_definition!s}" "") @admin.register(models.Course) @@ -94,8 +92,8 @@ class CourseAdmin(DjangoObjectActions, TranslatableAdmin): @takes_instance_or_queryset def generate_certificates(self, request, queryset): # pylint: disable=no-self-use """ - Custom action to launch create_certificates management command - over the selected courses + Custom action to generate certificates for a collection of courses + passed as a queryset """ certificate_generated_count = helpers.generate_certificates_for_orders( models.Order.objects.filter(course__in=queryset) @@ -192,8 +190,8 @@ def get_urls(self): @takes_instance_or_queryset def generate_certificates(self, request, queryset): # pylint: disable=no-self-use """ - Custom action to launch create_certificates management command - over the selected products + Custom action to generate certificates for a collection of products + passed as a queryset """ certificate_generated_count = helpers.generate_certificates_for_orders( models.Order.objects.filter(product__in=queryset) @@ -248,7 +246,7 @@ def get_related_courses_as_html(obj): # pylint: disable=no-self-use ) if is_certifying: - # Add a button go generate certificate + # Add a button to generate certificate generate_certificates_url = reverse( f"admin:{ACTION_NAME_GENERATE_CERTIFICATES}", kwargs={"product_id": obj.id, "course_code": course.code}, @@ -286,7 +284,7 @@ def cancel(self, request, queryset): # pylint: disable=no-self-use @takes_instance_or_queryset def generate_certificates(self, request, queryset): # pylint: disable=no-self-use """ - Custom action to launch create_certificates management commands + Custom action to launch generate_certificates management commands over the order selected """ certificate_generated_count = helpers.generate_certificates_for_orders(queryset) diff --git a/src/backend/joanie/core/helpers.py b/src/backend/joanie/core/helpers.py index 5bf009fbd0..8556561e76 100644 --- a/src/backend/joanie/core/helpers.py +++ b/src/backend/joanie/core/helpers.py @@ -9,18 +9,20 @@ def generate_certificate_for_order(order): """ - Check if order is eligible for certification then generate certificate is it is. + Check if order is eligible for certification then generate certificate if it is. Eligibility means that order contains one passed enrollment per graded courses. Return: - 0 : if the order is not eligible for certification + 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 = ( + order.target_courses.filter(order_relations__is_graded=True) + .order_by("order_relations__position") + .prefetch_related("course_runs") + ) graded_courses_count = len(graded_courses) if graded_courses_count == 0: @@ -32,20 +34,20 @@ def generate_certificate_for_order(order): start__lte=timezone.now(), ) + # Retrieve all enrollments in one queries, as these enrollments which relies on + # order course runs, the count will be always pretty small. 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 + # for each graded course, if not it is useless to go further course_enrollments = [] for course in graded_courses: - for enrollment in enrollments.iterator(): + for enrollment in enrollments: # 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(): + if enrollment.course_run in course.course_runs.all(): course_enrollments.append(enrollment) break diff --git a/src/backend/joanie/core/management/commands/create_certificates.py b/src/backend/joanie/core/management/commands/generate_certificates.py similarity index 96% rename from src/backend/joanie/core/management/commands/create_certificates.py rename to src/backend/joanie/core/management/commands/generate_certificates.py index 5204938c2b..7e0f348a66 100644 --- a/src/backend/joanie/core/management/commands/create_certificates.py +++ b/src/backend/joanie/core/management/commands/generate_certificates.py @@ -1,4 +1,4 @@ -"""Management command to create all pending certificates.""" +"""Management command to generate all pending certificates.""" import logging from django.core.management import BaseCommand @@ -7,7 +7,7 @@ from joanie.core import models from joanie.core.helpers import generate_certificates_for_orders -logger = logging.getLogger("joanie.core.create_certificates") +logger = logging.getLogger("joanie.core.generate_certificates") class Command(BaseCommand): diff --git a/src/backend/joanie/tests/test_commands_create_certificates.py b/src/backend/joanie/tests/test_commands_generate_certificates.py similarity index 70% rename from src/backend/joanie/tests/test_commands_create_certificates.py rename to src/backend/joanie/tests/test_commands_generate_certificates.py index c6494968d7..4bb887da91 100644 --- a/src/backend/joanie/tests/test_commands_create_certificates.py +++ b/src/backend/joanie/tests/test_commands_generate_certificates.py @@ -1,4 +1,4 @@ -"""Test suite for the management command 'create_certificates'""" +"""Test suite for the management command 'generate_certificates'""" import uuid from datetime import timedelta @@ -10,9 +10,9 @@ class CreateCertificatesTestCase(TestCase): - """Test case for the management command 'create_certificates'""" + """Test case for the management command 'generate_certificates'""" - def test_commands_create_certificates_has_options( + def test_commands_generate_certificates_has_options( self, ): # pylint: disable=no-self-use """ @@ -28,9 +28,9 @@ def test_commands_create_certificates_has_options( } # TypeError: Unknown option(s) should not be raised - call_command("create_certificates", **options) + call_command("generate_certificates", **options) - def test_commands_create_certificates(self): + def test_commands_generate_certificates(self): """ If a certifying product contains graded courses with gradable course runs and a user purchased this product and passed all gradable course runs, @@ -56,16 +56,14 @@ def test_commands_create_certificates(self): self.assertEqual(certificate.count(), 0) # DB queries should be minimized - with self.assertNumQueries(9): - call_command("create_certificates") + call_command("generate_certificates") self.assertEqual(certificate.count(), 1) # But call it again, should not create a new certificate - with self.assertNumQueries(1): - call_command("create_certificates") + call_command("generate_certificates") self.assertEqual(certificate.count(), 1) - def test_commands_create_certificates_can_be_restricted_to_order(self): + def test_commands_generate_certificates_can_be_restricted_to_order(self): """ If `order` option is used, the review is restricted to it. """ @@ -88,16 +86,14 @@ def test_commands_create_certificates_can_be_restricted_to_order(self): self.assertEqual(certificate.count(), 0) # A certificate should be generated for the 1st order - with self.assertNumQueries(9): - call_command("create_certificates", order=orders[0].uid) + call_command("generate_certificates", order=orders[0].uid) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the 2nd order - with self.assertNumQueries(7): - call_command("create_certificates", order=orders[1].uid) + call_command("generate_certificates", order=orders[1].uid) self.assertEqual(certificate.count(), 2) - def test_commands_create_certificates_can_be_restricted_to_course(self): + def test_commands_generate_certificates_can_be_restricted_to_course(self): """ If `course` option is used, the review is restricted to it. """ @@ -126,16 +122,14 @@ def test_commands_create_certificates_can_be_restricted_to_course(self): self.assertEqual(certificate.count(), 0) # A certificate should be generated for the 1st course - with self.assertNumQueries(9): - call_command("create_certificates", course=course_1.code) + call_command("generate_certificates", course=course_1.code) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the 2nd course - with self.assertNumQueries(8): - call_command("create_certificates", course=course_2.code) + call_command("generate_certificates", course=course_2.code) self.assertEqual(certificate.count(), 2) - def test_commands_create_certificates_can_be_restricted_to_product(self): + def test_commands_generate_certificates_can_be_restricted_to_product(self): """ If `product` option is used, the review is restricted to it. """ @@ -168,15 +162,15 @@ def test_commands_create_certificates_can_be_restricted_to_product(self): # A certificate should be generated for the 1st product with self.assertNumQueries(9): - call_command("create_certificates", product=product_1.uid) + call_command("generate_certificates", product=product_1.uid) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the 2nd product with self.assertNumQueries(8): - call_command("create_certificates", product=product_2.uid) + call_command("generate_certificates", product=product_2.uid) self.assertEqual(certificate.count(), 2) - def test_commands_create_certificates_can_be_restricted_to_product_course(self): + def test_commands_generate_certificates_can_be_restricted_to_product_course(self): """ `product` and `course` options can be used together to restrict review to them. """ @@ -214,29 +208,66 @@ def test_commands_create_certificates_can_be_restricted_to_product_course(self): self.assertEqual(certificate.count(), 0) # A certificate should be generated for the couple course_1 - product_1 - with self.assertNumQueries(9): - call_command( - "create_certificates", course=course_1.code, product=product_1.uid - ) + call_command( + "generate_certificates", course=course_1.code, product=product_1.uid + ) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the couple course_1 - product_2 - with self.assertNumQueries(8): - call_command( - "create_certificates", course=course_1.code, product=product_2.uid - ) + call_command( + "generate_certificates", course=course_1.code, product=product_2.uid + ) self.assertEqual(certificate.count(), 2) # Then a certificate should be generated for the couple course_2 - product_1 - with self.assertNumQueries(8): - call_command( - "create_certificates", course=course_2.code, product=product_1.uid - ) + call_command( + "generate_certificates", course=course_2.code, product=product_1.uid + ) self.assertEqual(certificate.count(), 3) # Finally, a certificate should be generated for the couple course_2 - product_2 - with self.assertNumQueries(7): - call_command( - "create_certificates", course=course_2.code, product=product_2.uid - ) + call_command( + "generate_certificates", course=course_2.code, product=product_2.uid + ) self.assertEqual(certificate.count(), 4) + + def test_commands_generate_certificates_optimizes_db_queries(self): + """ + The management command should optimize db access + """ + # Create two certifying products with order eligible for certification. + [cr1, cr2, cr3, cr4, cr5, cr6] = factories.CourseRunFactory.create_batch( + 6, + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=True, + start=timezone.now() - timedelta(hours=1), + ) + product_1 = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[cr1.course, cr2.course, cr3.course], + ) + product_2 = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[cr4.course, cr5.course, cr6.course], + ) + course = factories.CourseFactory(products=[product_1, product_2]) + orders = [ + factories.OrderFactory(course=course, product=product_1), + factories.OrderFactory(course=course, product=product_2), + ] + certificate = models.Certificate.objects.filter(order__in=orders) + + self.assertEqual(certificate.count(), 0) + + # A certificate should be generated for the 1st product + with self.assertNumQueries(9): + call_command("generate_certificates", product=product_1.uid) + self.assertEqual(certificate.count(), 1) + + # Then a certificate should be generated for the 2nd product + with self.assertNumQueries(8): + call_command("generate_certificates", product=product_2.uid) + self.assertEqual(certificate.count(), 2)