Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #19940 from edx/jmyatt/filter-out-stale-generated-certificates

Browse Source 
Filter out certificates for nonexistent courses
This commit is contained in:
Jason Myatt
2019-03-08 12:09:21 -05:00
committed by GitHub
parent 99d2746da8 09d51b00b5
commit ccfce4ac27
5 changed files with 95 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
lms/djangoapps/certificates/models.py
View File

@@ -212,6 +212,25 @@ class EligibleCertificateManager(models.Manager):
)
class EligibleAvailableCertificateManager(EligibleCertificateManager):
"""
A manager for `GeneratedCertificate` models that automatically
filters out ineligible certs and any linked to nonexistent courses.
Adds to the super class filtering ot also exclude certificates for
courses that do not have a corresponding CourseOverview.
"""
def get_queryset(self):
"""
Return a queryset for `GeneratedCertificate` models, filtering out
ineligible certificates and any linked to nonexistent courses.
"""
return super(EligibleAvailableCertificateManager, self).get_queryset().filter(
course_id__in=list(CourseOverview.objects.values_list('id', flat=True))
)
class GeneratedCertificate(models.Model):
"""
Base model for generated certificates
@@ -228,6 +247,10 @@ class GeneratedCertificate(models.Model):
# preference to the default `objects` manager in most cases.
eligible_certificates = EligibleCertificateManager()
# Only returns eligible certificates for courses that have an
# associated CourseOverview
eligible_available_certificates = EligibleAvailableCertificateManager()
# Normal object manager, which should only be used when ineligible
# certificates (i.e. new audit certs) should be included in the
# results. Django requires us to explicitly declare this.

34
lms/djangoapps/certificates/tests/test_models.py
View File

@@ -7,7 +7,7 @@ from django.core.exceptions import ValidationError
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
from django.test.utils import override_settings
from opaque_keys.edx.locator import CourseLocator
from opaque_keys.edx.locator import CourseLocator, CourseKey
from path import Path as path
from lms.djangoapps.certificates.models import (
@@ -22,6 +22,7 @@ from lms.djangoapps.certificates.models import (
)
from lms.djangoapps.certificates.tests.factories import CertificateInvalidationFactory, GeneratedCertificateFactory
from lms.djangoapps.instructor_task.tests.factories import InstructorTaskFactory
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from student.tests.factories import AdminFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -200,37 +201,50 @@ class EligibleCertificateManagerTest(SharedModuleStoreTestCase):
out ineligible certs.
"""
@classmethod
def setUpClass(cls):
super(EligibleCertificateManagerTest, cls).setUpClass()
cls.courses = (CourseFactory(), CourseFactory())
def setUp(self):
super(EligibleCertificateManagerTest, self).setUp()
self.user = UserFactory()
self.course1 = CourseOverviewFactory()
self.course2 = CourseOverviewFactory(
id=CourseKey.from_string('{}a'.format(self.course1.id))
)
self.eligible_cert = GeneratedCertificateFactory.create(
status=CertificateStatuses.downloadable,
user=self.user,
course_id=self.courses[0].id
course_id=self.course1.id
)
self.ineligible_cert = GeneratedCertificateFactory.create(
status=CertificateStatuses.audit_passing,
user=self.user,
course_id=self.courses[1].id
course_id=self.course2.id
)
def test_filter_ineligible_certificates(self):
"""
Verify that the EligibleCertificateManager filters out
Verify that the EligibleAvailableCertificateManager filters out
certificates marked as ineligible, and that the default object
manager for GeneratedCertificate does not filter them out.
"""
self.assertEqual(list(GeneratedCertificate.eligible_certificates.filter(user=self.user)), [self.eligible_cert])
self.assertEqual(list(
GeneratedCertificate.eligible_available_certificates.filter(user=self.user)), [self.eligible_cert]
)
self.assertEqual(
list(GeneratedCertificate.objects.filter(user=self.user)),
[self.eligible_cert, self.ineligible_cert]
)
def test_filter_certificates_for_nonexistent_courses(self):
"""
Verify that the EligibleAvailableCertificateManager filters out
certificates for courses with no CourseOverview.
"""
self.course1.delete()
self.assertFalse(GeneratedCertificate.eligible_available_certificates.filter(
user=self.user)
)
@ddt.ddt
class TestCertificateGenerationHistory(TestCase):

4
openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py
View File

@@ -111,5 +111,7 @@ class Command(BaseCommand):
query = status_query & course_run_query
username_dicts = GeneratedCertificate.eligible_certificates.filter(query).values('user__username').distinct()
username_dicts = GeneratedCertificate.eligible_available_certificates.filter(
query
).values('user__username').distinct()
self.usernames = [d['user__username'] for d in username_dicts]

45
openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py
View File

@@ -8,6 +8,7 @@ from lms.djangoapps.certificates.models import CertificateStatuses
from course_modes.models import CourseMode
from lms.djangoapps.certificates.api import MODES
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.catalog.tests.factories import (
generate_course_run_key,
ProgramFactory,
@@ -15,6 +16,7 @@ from openedx.core.djangoapps.catalog.tests.factories import (
CourseRunFactory,
)
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
from openedx.core.djangolib.testing.utils import skip_unless_lms
from student.tests.factories import UserFactory
@@ -28,7 +30,9 @@ COMMAND_MODULE = 'openedx.core.djangoapps.programs.management.commands.backpopul
@skip_unless_lms
class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase):
"""Tests for the backpopulate_program_credentials management command."""
course_run_key, alternate_course_run_key = (generate_course_run_key() for __ in range(2))
course_run_key, alternate_course_run_key, course_run_key_no_course_overview = (
generate_course_run_key() for __ in range(3)
)
# Constants for the _get_programs_data hierarchy types used in test_flatten()
SEPARATE_PROGRAMS = 'separate_programs'
SEPARATE_COURSES = 'separate_courses'
@@ -40,6 +44,10 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp
self.alice = UserFactory()
self.bob = UserFactory()
# We need CourseOverview instances to exist for the test courses
CourseOverviewFactory(id=CourseKey.from_string(self.course_run_key))
CourseOverviewFactory(id=CourseKey.from_string(self.alternate_course_run_key))
# Disable certification to prevent the task from being triggered when
# setting up test data (i.e., certificates with a passing status), thereby
# skewing mock call counts.
@@ -299,6 +307,41 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp
mock_task.assert_called_once_with(self.alice.username)
def test_handle_no_course_overview(self, mock_task, mock_get_programs):
"""
Verify that the task is not enqueued for a user whose only certificate
is for a course with no CourseOverview.
"""
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
CourseRunFactory(key=self.course_run_key_no_course_overview),
]),
]
),
]
mock_get_programs.return_value = data
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.course_run_key_no_course_overview,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
call_command('backpopulate_program_credentials', commit=True)
mock_task.assert_called_once_with(self.alice.username)
mock_task.assert_not_called(self.bob.username)
@mock.patch(COMMAND_MODULE + '.logger.exception')
def test_handle_enqueue_failure(self, mock_log, mock_task, mock_get_programs):
"""Verify that failure to enqueue a task doesn't halt execution."""

2
openedx/core/djangoapps/programs/utils.py
View File

@@ -285,7 +285,7 @@ class ProgramProgressMeter(object):
Returns a dict of {uuid_string: available_datetime}
"""
# Query for all user certs up front, for performance reasons (rather than querying per course run).
user_certificates = GeneratedCertificate.eligible_certificates.filter(user=self.user)
user_certificates = GeneratedCertificate.eligible_available_certificates.filter(user=self.user)
certificates_by_run = {cert.course_id: cert for cert in user_certificates}
completed = {}