From 4174c40d6dfa627bdc9e07edbe8beb45b7eb3ede Mon Sep 17 00:00:00 2001 From: jbpenrath Date: Fri, 1 Apr 2022 10:58:27 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=8C(!fixup)=20first=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/joanie/core/admin.py | 129 +++++++----- src/backend/joanie/core/helpers.py | 100 +++++++++ .../commands/create_certificates.py | 168 +++------------ src/backend/joanie/core/models/products.py | 10 +- .../joanie/tests/test_admin_product.py | 12 +- .../test_commands_create_certificates.py | 158 ++------------ src/backend/joanie/tests/test_helpers.py | 196 ++++++++++++++++++ .../joanie/tests/test_models_enrollment.py | 71 +++++++ 8 files changed, 498 insertions(+), 346 deletions(-) create mode 100644 src/backend/joanie/core/helpers.py create mode 100644 src/backend/joanie/tests/test_helpers.py diff --git a/src/backend/joanie/core/admin.py b/src/backend/joanie/core/admin.py index 0e9775d2c6..5b63bdc311 100644 --- a/src/backend/joanie/core/admin.py +++ b/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.+)/generate-certificates/(?P.+)/$", 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( - ( - '
  • ' - f"" - f"{course.code} | {course.title}" - "" - ), - ( - f'' # 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 = ( + '
  • ' + f"{course.code} | {course.title}" + ) + + 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'' # noqa pylint: disable=line-too-long f'{_("Generate certificates")}' "" - "
    • " ) - if is_certifying - else "", - ) - for course in obj.courses.all() - ] + + raw_html += "" + items.append(raw_html) + return format_html(f"
      {''.join(items)}
    ") + 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.""" diff --git a/src/backend/joanie/core/helpers.py b/src/backend/joanie/core/helpers.py new file mode 100644 index 0000000000..5bf009fbd0 --- /dev/null +++ b/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 diff --git a/src/backend/joanie/core/management/commands/create_certificates.py b/src/backend/joanie/core/management/commands/create_certificates.py index a250b55d60..5204938c2b 100644 --- a/src/backend/joanie/core/management/commands/create_certificates.py +++ b/src/backend/joanie/core/management/commands/create_certificates.py @@ -1,11 +1,11 @@ """Management command to create all pending certificates.""" import logging -from django.core.exceptions import ValidationError from django.core.management import BaseCommand -from django.utils import timezone +from django.utils.translation import ngettext_lazy -from joanie.core import enums, models +from joanie.core import models +from joanie.core.helpers import generate_certificates_for_orders logger = logging.getLogger("joanie.core.create_certificates") @@ -13,8 +13,8 @@ class Command(BaseCommand): """ A command to generate all pending certificates. - It browses all certifying products and check if related orders are eligible for - certification then generate the certificate if it is. + It browses all certifying products, checks if related orders are eligible for + certification and generates a certificate if they are. Through options, you are able to restrict this command to a list of courses (-c), products (-p) or orders (-o). @@ -56,7 +56,7 @@ def handle(self, *args, **options): """ Retrieve all certifying products then for each of them check eligibility for certification of all related orders. - If `order` option is used, this order is directly retrieve. + If `order` option is used, this order is directly retrieved. """ order_uids = None product_uids = None @@ -69,32 +69,6 @@ def handle(self, *args, **options): else [options["orders"]] ) - total = 0 - if order_uids: - filters = { - "is_canceled": False, - "certificate__isnull": True, - "product__type__in": enums.PRODUCT_TYPE_CERTIFICATE_ALLOWED, - "uid__in": order_uids, - } - - try: - orders = [ - order - for order in models.Order.objects.select_related( - "course__organization" - ).filter(**filters) - if order.state == enums.ORDER_STATE_VALIDATED - ] - except models.Order.DoesNotExist: - return str(total) - - for order in orders: - result = self._generate_certificate_for_order(order) - total += result - - return str(total) - if options["courses"]: course_codes = ( options["courses"] @@ -109,115 +83,23 @@ def handle(self, *args, **options): else [options["products"]] ) - filters = {"type__in": enums.PRODUCT_TYPE_CERTIFICATE_ALLOWED} - if course_codes: - filters.update({"courses__code__in": course_codes}) - if product_uids: - filters.update({"uid__in": product_uids}) - - products = models.Product.objects.filter(**filters) - - for product in products: - result = self._generate_certificate_for_product(product, course_codes) - total += result - - return str(total) - - def _generate_certificate_for_product(self, product, course_codes): - """ - Retrieve orders related to the product and course code then check if - it is eligible for certification. - """ - filters = {"is_canceled": False, "certificate__isnull": True} - if course_codes: - filters.update({"course__code__in": course_codes}) - - orders = [ - order - for order in product.orders.filter(**filters).select_related( - "course__organization" - ) - if order.state == enums.ORDER_STATE_VALIDATED - ] - - certificate_counter = 0 - for order in orders: - result = self._generate_certificate_for_order(order) - certificate_counter += result - - if certificate_counter > 0: - logger.info( - '%s certificate(s) for "%s" have been generated.', - certificate_counter, - product.title, - ) - - return certificate_counter - - @staticmethod - 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 to 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(), + filters = {} + if order_uids: + filters.update({"uid__in": order_uids}) + else: + if course_codes: + filters.update({"course__code__in": course_codes}) + if product_uids: + filters.update({"product__uid__in": product_uids}) + + certificate_generated_count = generate_certificates_for_orders( + models.Order.objects.filter(**filters) + ) + logger.info( + ngettext_lazy( + "%d certificate has been generated.", + "%d certificates have been generated.", + certificate_generated_count, + ), + certificate_generated_count, ) - - 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: - # Check if the enrollment relies on course by crossing - # all course runs implied - intersections = len( - {enrollment.course_run} & set(course.course_runs.all()) - ) - if intersections == 1: - 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 diff --git a/src/backend/joanie/core/models/products.py b/src/backend/joanie/core/models/products.py index 1b4203135a..3247bdc6b4 100644 --- a/src/backend/joanie/core/models/products.py +++ b/src/backend/joanie/core/models/products.py @@ -467,10 +467,12 @@ def get_grade(self): ) except GradeError: pass - - cache.set( - self.grade_cache_key, grade, settings.JOANIE_ENROLLMENT_GRADE_CACHE_TTL - ) + else: + cache.set( + self.grade_cache_key, + grade, + settings.JOANIE_ENROLLMENT_GRADE_CACHE_TTL, + ) return grade diff --git a/src/backend/joanie/tests/test_admin_product.py b/src/backend/joanie/tests/test_admin_product.py index c153e85b23..b49e2289ae 100644 --- a/src/backend/joanie/tests/test_admin_product.py +++ b/src/backend/joanie/tests/test_admin_product.py @@ -342,8 +342,10 @@ def test_admin_product_should_allow_to_generate_certificate_for_related_course( ), ) - @mock.patch("django.core.management.call_command", return_value="0") - def test_admin_product_generate_certificate_for_course(self, mock_call_command): + @mock.patch("joanie.core.helpers.generate_certificates_for_orders", return_value=0) + def test_admin_product_generate_certificate_for_course( + self, mock_generate_certificates + ): """ Product Admin should contain an endpoint which trigger the `create_certificates` management command with product and course as options. @@ -362,14 +364,12 @@ def test_admin_product_generate_certificate_for_course(self, mock_call_command): ) # - Create certificates command should have been called - mock_call_command.assert_called_once_with( - "create_certificates", courses=course.code, products=product.uid - ) + mock_generate_certificates.assert_called_once() # Check the presence of a confirmation message messages = list(get_messages(response.wsgi_request)) self.assertEqual(len(messages), 1) - self.assertEqual(str(messages[0]), "No certificate has been generated.") + self.assertEqual(str(messages[0]), "No certificates have been generated.") # - User should be redirected to the product change view self.assertRedirects( diff --git a/src/backend/joanie/tests/test_commands_create_certificates.py b/src/backend/joanie/tests/test_commands_create_certificates.py index 489adbfe7e..c6494968d7 100644 --- a/src/backend/joanie/tests/test_commands_create_certificates.py +++ b/src/backend/joanie/tests/test_commands_create_certificates.py @@ -1,14 +1,12 @@ """Test suite for the management command 'create_certificates'""" import uuid from datetime import timedelta -from unittest import mock from django.core.management import call_command from django.test import TestCase from django.utils import timezone from joanie.core import enums, factories, models -from joanie.lms_handler.backends.dummy import DummyLMSBackend class CreateCertificatesTestCase(TestCase): @@ -16,7 +14,7 @@ class CreateCertificatesTestCase(TestCase): def test_commands_create_certificates_has_options( self, - ): + ): # pylint: disable=no-self-use """ This command should accept three optional arguments: - courses @@ -30,7 +28,7 @@ def test_commands_create_certificates_has_options( } # TypeError: Unknown option(s) should not be raised - self.assertEqual(call_command("create_certificates", **options), "0") + call_command("create_certificates", **options) def test_commands_create_certificates(self): """ @@ -59,106 +57,12 @@ def test_commands_create_certificates(self): # DB queries should be minimized with self.assertNumQueries(9): - self.assertEqual(call_command("create_certificates"), "1") + call_command("create_certificates") self.assertEqual(certificate.count(), 1) # But call it again, should not create a new certificate - with self.assertNumQueries(2): - self.assertEqual(call_command("create_certificates"), "0") - self.assertEqual(certificate.count(), 1) - - def test_commands_create_certificates_needs_graded_courses(self): - """ - If a certifying product does not contain graded courses, - any certificate should be generated. - """ - # Create a certifying product with one order eligible for certification - course_run = factories.CourseRunFactory( - enrollment_end=timezone.now() + timedelta(hours=1), - enrollment_start=timezone.now() - timedelta(hours=1), - is_gradable=True, - start=timezone.now() - timedelta(hours=1), - ) - product = factories.ProductFactory( - price="0.00", - type=enums.PRODUCT_TYPE_CREDENTIAL, - target_courses=[course_run.course], - ) - # Mark the only product's course as non graded - course_run.course.product_relations.update(is_graded=False) - course = factories.CourseFactory(products=[product]) - order = factories.OrderFactory(product=product, course=course) - certificate = models.Certificate.objects.filter(order=order) - self.assertEqual(certificate.count(), 0) - - self.assertEqual(call_command("create_certificates"), "0") - self.assertEqual(certificate.count(), 0) - - def test_commands_create_certificates_needs_gradable_course_runs(self): - """ - If a certifying product does not rely on gradable course runs, - any certificate should be generated. - """ - # Create a certifying product with one order eligible for certification - course_run = factories.CourseRunFactory( - enrollment_end=timezone.now() + timedelta(hours=1), - enrollment_start=timezone.now() - timedelta(hours=1), - is_gradable=False, - start=timezone.now() - timedelta(hours=1), - ) - product = factories.ProductFactory( - price="0.00", - type=enums.PRODUCT_TYPE_CREDENTIAL, - target_courses=[course_run.course], - ) - course = factories.CourseFactory(products=[product]) - order = factories.OrderFactory(product=product, course=course) - certificate = models.Certificate.objects.filter(order=order) - self.assertEqual(certificate.count(), 0) - - self.assertEqual(call_command("create_certificates"), "0") - self.assertEqual(certificate.count(), 0) - - # - Now flag the course run as gradable - course_run.is_gradable = True - course_run.save() - - self.assertEqual(call_command("create_certificates"), "1") - self.assertEqual(certificate.count(), 1) - - def test_commands_create_certificates_needs_enrollments_has_been_passed(self): - """ - Certificate is generated only if user has passed all graded courses. - """ - # Create a certifying product with one order eligible for certification - course_run = factories.CourseRunFactory( - enrollment_end=timezone.now() + timedelta(hours=1), - enrollment_start=timezone.now() - timedelta(hours=1), - is_gradable=True, - start=timezone.now() - timedelta(hours=1), - ) - product = factories.ProductFactory( - price="0.00", - type=enums.PRODUCT_TYPE_CREDENTIAL, - target_courses=[course_run.course], - ) - course = factories.CourseFactory(products=[product]) - order = factories.OrderFactory(product=product, course=course) - certificate = models.Certificate.objects.filter(order=order) - self.assertEqual(certificate.count(), 0) - - # Simulate that all enrollments are not passed - with mock.patch.object(DummyLMSBackend, "get_grades") as mock_get_grades: - mock_get_grades.return_value = {"passed": False} - self.assertEqual(call_command("create_certificates"), "0") - - self.assertEqual(certificate.count(), 0) - - # Simulate that all enrollments are passed - with mock.patch.object(DummyLMSBackend, "get_grades") as mock_get_grades: - mock_get_grades.return_value = {"passed": True} - self.assertEqual(call_command("create_certificates"), "1") - + with self.assertNumQueries(1): + call_command("create_certificates") self.assertEqual(certificate.count(), 1) def test_commands_create_certificates_can_be_restricted_to_order(self): @@ -185,16 +89,12 @@ def test_commands_create_certificates_can_be_restricted_to_order(self): # A certificate should be generated for the 1st order with self.assertNumQueries(9): - self.assertEqual( - call_command("create_certificates", order=orders[0].uid), "1" - ) + call_command("create_certificates", order=orders[0].uid) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the 2nd order with self.assertNumQueries(7): - self.assertEqual( - call_command("create_certificates", order=orders[1].uid), "1" - ) + call_command("create_certificates", order=orders[1].uid) self.assertEqual(certificate.count(), 2) def test_commands_create_certificates_can_be_restricted_to_course(self): @@ -227,16 +127,12 @@ def test_commands_create_certificates_can_be_restricted_to_course(self): # A certificate should be generated for the 1st course with self.assertNumQueries(9): - self.assertEqual( - call_command("create_certificates", course=course_1.code), "1" - ) + call_command("create_certificates", course=course_1.code) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the 2nd course with self.assertNumQueries(8): - self.assertEqual( - call_command("create_certificates", course=course_2.code), "1" - ) + call_command("create_certificates", course=course_2.code) self.assertEqual(certificate.count(), 2) def test_commands_create_certificates_can_be_restricted_to_product(self): @@ -272,16 +168,12 @@ def test_commands_create_certificates_can_be_restricted_to_product(self): # A certificate should be generated for the 1st product with self.assertNumQueries(9): - self.assertEqual( - call_command("create_certificates", product=product_1.uid), "1" - ) + call_command("create_certificates", product=product_1.uid) self.assertEqual(certificate.count(), 1) # Then a certificate should be generated for the 2nd product with self.assertNumQueries(8): - self.assertEqual( - call_command("create_certificates", product=product_2.uid), "1" - ) + call_command("create_certificates", product=product_2.uid) self.assertEqual(certificate.count(), 2) def test_commands_create_certificates_can_be_restricted_to_product_course(self): @@ -323,40 +215,28 @@ def test_commands_create_certificates_can_be_restricted_to_product_course(self): # A certificate should be generated for the couple course_1 - product_1 with self.assertNumQueries(9): - self.assertEqual( - call_command( - "create_certificates", course=course_1.code, product=product_1.uid - ), - "1", + call_command( + "create_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): - self.assertEqual( - call_command( - "create_certificates", course=course_1.code, product=product_2.uid - ), - "1", + call_command( + "create_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): - self.assertEqual( - call_command( - "create_certificates", course=course_2.code, product=product_1.uid - ), - "1", + call_command( + "create_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): - self.assertEqual( - call_command( - "create_certificates", course=course_2.code, product=product_2.uid - ), - "1", + call_command( + "create_certificates", course=course_2.code, product=product_2.uid ) self.assertEqual(certificate.count(), 4) diff --git a/src/backend/joanie/tests/test_helpers.py b/src/backend/joanie/tests/test_helpers.py new file mode 100644 index 0000000000..5fb7cf8300 --- /dev/null +++ b/src/backend/joanie/tests/test_helpers.py @@ -0,0 +1,196 @@ +"""Joanie core helpers tests suite""" +from datetime import timedelta +from unittest import mock + +from django.test.testcases import TestCase +from django.utils import timezone + +from joanie.core import enums, factories, helpers, models +from joanie.lms_handler.backends.dummy import DummyLMSBackend + + +class HelpersTestCase(TestCase): + """Joanie core helpers tests case""" + + def test_helpers_generate_certificate_for_order_needs_graded_courses(self): + """ + If the order relies on a certifying product which does not contain + graded courses any certificate should be generated. + """ + # Create a certifying product with one order eligible for certification + course_run = factories.CourseRunFactory( + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=True, + start=timezone.now() - timedelta(hours=1), + ) + product = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[course_run.course], + ) + # Mark the only product's course as non graded + course_run.course.product_relations.update(is_graded=False) + course = factories.CourseFactory(products=[product]) + order = factories.OrderFactory(product=product, course=course) + certificate = models.Certificate.objects.filter(order=order) + self.assertEqual(certificate.count(), 0) + + self.assertEqual(helpers.generate_certificate_for_order(order), 0) + self.assertEqual(certificate.count(), 0) + + def test_helpers_generate_certificate_for_order_needs_gradable_course_runs(self): + """ + If the order does not rely on gradable course runs, + any certificate should be generated. + """ + # Create a certifying product with one order eligible for certification + course_run = factories.CourseRunFactory( + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=False, + start=timezone.now() - timedelta(hours=1), + ) + product = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[course_run.course], + ) + course = factories.CourseFactory(products=[product]) + order = factories.OrderFactory(product=product, course=course) + certificate = models.Certificate.objects.filter(order=order) + self.assertEqual(certificate.count(), 0) + + self.assertEqual(helpers.generate_certificate_for_order(order), 0) + self.assertEqual(certificate.count(), 0) + + # - Now flag the course run as gradable + course_run.is_gradable = True + course_run.save() + + self.assertEqual(helpers.generate_certificate_for_order(order), 1) + self.assertEqual(certificate.count(), 1) + + def test_helpers_generate_certificate_for_order_needs_enrollments_has_been_passed( + self, + ): + """ + Certificate is generated only if owner has passed all graded courses. + """ + # Create a certifying product with one order eligible for certification + course_run = factories.CourseRunFactory( + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=True, + start=timezone.now() - timedelta(hours=1), + ) + product = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[course_run.course], + ) + course = factories.CourseFactory(products=[product]) + order = factories.OrderFactory(product=product, course=course) + certificate = models.Certificate.objects.filter(order=order) + self.assertEqual(certificate.count(), 0) + + # Simulate that all enrollments are not passed + with mock.patch.object(DummyLMSBackend, "get_grades") as mock_get_grades: + mock_get_grades.return_value = {"passed": False} + self.assertEqual(helpers.generate_certificate_for_order(order), 0) + + self.assertEqual(certificate.count(), 0) + + # Simulate that all enrollments are passed + with mock.patch.object(DummyLMSBackend, "get_grades") as mock_get_grades: + mock_get_grades.return_value = {"passed": True} + self.assertEqual(helpers.generate_certificate_for_order(order), 1) + + self.assertEqual(certificate.count(), 1) + + def test_helpers_generate_certificate_for_order(self): + """ + If the provided order relies on a certifying product containing graded courses + with gradable course runs and the owner passed all gradable course runs, + a certificate should be generated + """ + + # Create a certifying product with one order eligible for certification + course_run = factories.CourseRunFactory( + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=True, + start=timezone.now() - timedelta(hours=1), + ) + product = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[course_run.course], + ) + course = factories.CourseFactory(products=[product]) + order = factories.OrderFactory(product=product, course=course) + certificate = models.Certificate.objects.filter(order=order) + + self.assertEqual(certificate.count(), 0) + + # DB queries should be minimized + with self.assertNumQueries(7): + self.assertEqual(helpers.generate_certificate_for_order(order), 1) + self.assertEqual(certificate.count(), 1) + + # But call it again, should not create a new certificate + with self.assertNumQueries(4): + self.assertEqual(helpers.generate_certificate_for_order(order), 0) + self.assertEqual(certificate.count(), 1) + + def test_helpers_generate_certificates_for_orders(self): + """ + This method should generate a certificate for each order eligible for certification. + """ + # Create a certifying product with one order eligible for certification + course_run = factories.CourseRunFactory( + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=True, + start=timezone.now() - timedelta(hours=1), + ) + not_gradable_course_run = factories.CourseRunFactory( + enrollment_end=timezone.now() + timedelta(hours=1), + enrollment_start=timezone.now() - timedelta(hours=1), + is_gradable=False, + start=timezone.now() - timedelta(hours=1), + ) + product_1 = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[course_run.course], + ) + product_2 = factories.ProductFactory( + price="0.00", + type=enums.PRODUCT_TYPE_CREDENTIAL, + target_courses=[not_gradable_course_run.course], + ) + course = factories.CourseFactory(products=[product_1, product_2]) + orders = [ + # - 10 eligible orders + *factories.OrderFactory.create_batch(10, product=product_1, course=course), + # - 10 non eligible orders + *factories.OrderFactory.create_batch(10, product=product_2, course=course), + # - 1 canceled order + factories.OrderFactory(product=product_1, course=course, is_canceled=True), + ] + + certificate = models.Certificate.objects.filter(order__in=orders) + + self.assertEqual(certificate.count(), 0) + + self.assertEqual( + helpers.generate_certificates_for_orders(models.Order.objects.all()), 10 + ) + self.assertEqual(certificate.count(), 10) + + # But call it again, should not create a new certificate + self.assertEqual( + helpers.generate_certificates_for_orders(models.Order.objects.all()), 0 + ) + self.assertEqual(certificate.count(), 10) diff --git a/src/backend/joanie/tests/test_models_enrollment.py b/src/backend/joanie/tests/test_models_enrollment.py index 6962cd1741..cdc1e389ed 100644 --- a/src/backend/joanie/tests/test_models_enrollment.py +++ b/src/backend/joanie/tests/test_models_enrollment.py @@ -10,6 +10,7 @@ from django.utils import timezone from joanie.core import factories +from joanie.core.exceptions import GradeError from joanie.lms_handler.backends.openedx import OpenEdXLMSBackend @@ -99,3 +100,73 @@ def test_models_enrollment_unique_course_run_user(self): "{'__all__': ['Enrollment with this Course run and User already exists.']}", str(context.exception), ) + + @override_settings(JOANIE_ENROLLMENT_GRADE_CACHE_TTL=600) + @mock.patch.object(OpenEdXLMSBackend, "set_enrollment", return_value=True) + @mock.patch.object(OpenEdXLMSBackend, "get_grades", return_value={"passed": True}) + def test_models_enrollment_is_passed(self, mock_get_grades, _): + """ + The `is_passed` property should use the get_grades method of the LMS to retrieve + information then store in cache the result. + """ + resource_link = ( + "http://openedx.test/courses/course-v1:edx+000001+Demo_Course/course" + ) + + course_run = factories.CourseRunFactory( + start=timezone.now() - timedelta(hours=1), + end=timezone.now() + timedelta(hours=2), + enrollment_end=timezone.now() + timedelta(hours=1), + resource_link=resource_link, + ) + + enrollment = factories.EnrollmentFactory(course_run=course_run) + + self.assertIs(enrollment.is_passed, True) + mock_get_grades.assert_called_once_with( + username=enrollment.user.username, resource_link=course_run.resource_link + ) + + # - Call it again should return the same result + mock_get_grades.reset_mock() + self.assertIs(enrollment.is_passed, True) + # - But `get_grades` should have not been called again + mock_get_grades.assert_not_called() + + @override_settings(JOANIE_ENROLLMENT_GRADE_CACHE_TTL=600) + @mock.patch.object(OpenEdXLMSBackend, "set_enrollment", return_value=True) + @mock.patch.object(OpenEdXLMSBackend, "get_grades", side_effect=GradeError()) + def test_models_enrollment_is_passed_not_cached_on_failure( + self, mock_get_grades, _ + ): + """ + In case of get_grades LMS request fails, `is_passed` property should be False + and the result should not be cached. + """ + resource_link = ( + "http://openedx.test/courses/course-v1:edx+000001+Demo_Course/course" + ) + + course_run = factories.CourseRunFactory( + start=timezone.now() - timedelta(hours=1), + end=timezone.now() + timedelta(hours=2), + enrollment_end=timezone.now() + timedelta(hours=1), + resource_link=resource_link, + ) + + enrollment = factories.EnrollmentFactory(course_run=course_run) + + self.assertIs(enrollment.is_passed, False) + + mock_get_grades.assert_called_once_with( + username=enrollment.user.username, resource_link=course_run.resource_link + ) + + # - Call it again should trigger the `get_grades` method + mock_get_grades.reset_mock() + mock_get_grades.return_value = {"passed": True} + mock_get_grades.side_effect = None + self.assertIs(enrollment.is_passed, True) + mock_get_grades.assert_called_once_with( + username=enrollment.user.username, resource_link=course_run.resource_link + )