Add initial grade signaling

Tells the Credential service about user grades when a cert is awarded
or a grade for a course with an awarded cert changes.

LEARNER-5051

lms/djangoapps/courseware/tests/test_views.py

@@ -1440,7 +1440,7 @@ class ProgressPageTests(ProgressPageBaseTests):
 
     @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
     @ddt.data(
-        (False, 44, 28),
+        (False, 45, 29),
         (True, 37, 24)
     )
     @ddt.unpack

lms/djangoapps/grades/tests/test_course_grade_factory.py

@@ -97,28 +97,28 @@ class TestCourseGradeFactory(GradeTestBase):
         with self.assertNumQueries(2), mock_get_score(1, 2):
             _assert_read(expected_pass=False, expected_percent=0)  # start off with grade of 0
 
-        num_queries = 40
+        num_queries = 41
         with self.assertNumQueries(num_queries), mock_get_score(1, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
         with self.assertNumQueries(2):
             _assert_read(expected_pass=True, expected_percent=0.5)  # updated to grade of .5
 
-        num_queries = 6
+        num_queries = 7
         with self.assertNumQueries(num_queries), mock_get_score(1, 4):
             grade_factory.update(self.request.user, self.course, force_update_subsections=False)
 
         with self.assertNumQueries(2):
             _assert_read(expected_pass=True, expected_percent=0.5)  # NOT updated to grade of .25
 
-        num_queries = 20
+        num_queries = 21
         with self.assertNumQueries(num_queries), mock_get_score(2, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
         with self.assertNumQueries(2):
             _assert_read(expected_pass=True, expected_percent=1.0)  # updated to grade of 1.0
 
-        num_queries = 23
+        num_queries = 24
         with self.assertNumQueries(num_queries), mock_get_score(0, 0):  # the subsection now is worth zero
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 

lms/djangoapps/grades/tests/test_tasks.py

@@ -168,10 +168,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEquals(mock_block_structure_create.call_count, 1)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 27, True),
-        (ModuleStoreEnum.Type.mongo, 1, 27, False),
-        (ModuleStoreEnum.Type.split, 3, 27, True),
-        (ModuleStoreEnum.Type.split, 3, 27, False),
+        (ModuleStoreEnum.Type.mongo, 1, 28, True),
+        (ModuleStoreEnum.Type.mongo, 1, 28, False),
+        (ModuleStoreEnum.Type.split, 3, 28, True),
+        (ModuleStoreEnum.Type.split, 3, 28, False),
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -183,8 +183,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                     self._apply_recalculate_subsection_grade()
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 27),
-        (ModuleStoreEnum.Type.split, 3, 27),
+        (ModuleStoreEnum.Type.mongo, 1, 28),
+        (ModuleStoreEnum.Type.split, 3, 28),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -229,8 +229,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
         )
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 11),
-        (ModuleStoreEnum.Type.split, 3, 11),
+        (ModuleStoreEnum.Type.mongo, 1, 12),
+        (ModuleStoreEnum.Type.split, 3, 12),
     )
     @ddt.unpack
     def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
@@ -244,8 +244,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 28),
-        (ModuleStoreEnum.Type.split, 3, 28),
+        (ModuleStoreEnum.Type.mongo, 1, 29),
+        (ModuleStoreEnum.Type.split, 3, 29),
     )
     @ddt.unpack
     def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):

openedx/core/djangoapps/credentials/admin.py

@@ -8,7 +8,7 @@ from django.contrib import admin
 from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
 
 
-class CredentialsApiConfigAdmin(ConfigurationModelAdmin):  # pylint: disable=missing-docstring
+class CredentialsApiConfigAdmin(ConfigurationModelAdmin):
     pass
 
 

openedx/core/djangoapps/credentials/apps.py

@@ -3,7 +3,7 @@ Credentials Configuration
 """
 from django.apps import AppConfig
 from django.utils.translation import ugettext_lazy as _
-from openedx.core.djangoapps.plugins.constants import ProjectType, SettingsType, PluginSettings
+from openedx.core.djangoapps.plugins.constants import ProjectType, SettingsType, PluginSettings, PluginSignals
 
 
 class CredentialsConfig(AppConfig):
@@ -21,5 +21,23 @@ class CredentialsConfig(AppConfig):
                 SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.devstack'},
                 SettingsType.TEST: {PluginSettings.RELATIVE_PATH: u'settings.test'},
             }
-        }
+        },
+        PluginSignals.CONFIG: {
+            ProjectType.LMS: {
+                PluginSignals.RECEIVERS: [
+                    {
+                        PluginSignals.RECEIVER_FUNC_NAME: u'handle_grade_change',
+                        PluginSignals.SIGNAL_PATH: u'openedx.core.djangoapps.signals.signals.COURSE_GRADE_CHANGED',
+                    },
+                    {
+                        PluginSignals.RECEIVER_FUNC_NAME: u'handle_cert_change',
+                        PluginSignals.SIGNAL_PATH: u'openedx.core.djangoapps.signals.signals.COURSE_CERT_CHANGED',
+                    },
+                ],
+            },
+        },
     }
+
+    def ready(self):
+        # Register celery workers
+        from .tasks.v1 import tasks  # pylint: disable=unused-variable

openedx/core/djangoapps/credentials/models.py

@@ -2,7 +2,6 @@
 Models for credentials support for the LMS and Studio.
 """
 
-import waffle
 from urlparse import urljoin
 
 from config_models.models import ConfigurationModel

openedx/core/djangoapps/credentials/signals.py

@@ -0,0 +1,89 @@
+"""
+This file contains signal handlers for credentials-related functionality.
+"""
+from logging import getLogger
+
+from course_modes.models import CourseMode
+from django.contrib.sites.models import Site
+from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
+from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
+from openedx.core.djangoapps.catalog.utils import get_programs
+
+from .tasks.v1.tasks import send_grade_to_credentials
+
+log = getLogger(__name__)
+
+
+# "interesting" here means "credentials will want to know about it"
+INTERESTING_MODES = CourseMode.VERIFIED_MODES + CourseMode.CREDIT_MODES
+INTERESTING_STATUSES = [
+    CertificateStatuses.notpassing,
+    CertificateStatuses.downloadable,
+]
+
+
+# These handlers have Credentials business logic that has bled into the LMS. But we want to filter here in order to
+# not flood our task queue with a bunch of signals. So we put up with it.
+
+def is_course_run_in_a_program(course_run_key):
+    """ Returns true if the given course key is in any program at all. """
+
+    # We don't have an easy way to go from course_run_key to a specific site that owns it. So just search each site.
+    sites = Site.objects.all()
+    str_key = str(course_run_key)
+    for site in sites:
+        for program in get_programs(site):
+            for course in program['courses']:
+                for course_run in course['course_runs']:
+                    if str_key == course_run['key']:
+                        return True
+    return False
+
+
+def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, percent_grade):
+    """ Checks if grade is interesting to Credentials and schedules a Celery task if so. """
+
+    # Grab mode/status if we don't have them in hand
+    if mode is None or status is None:
+        try:
+            cert = GeneratedCertificate.objects.get(user=user, course_id=course_run_key)  # pylint: disable=no-member
+            mode = cert.mode
+            status = cert.status
+        except GeneratedCertificate.DoesNotExist:
+            # We only care about grades for which there is a certificate.
+            return
+
+    # Don't worry about whether it's available as well as awarded. Just awarded is good enough to record a verified
+    # attempt at a course. We want even the grades that didn't pass the class because Credentials wants to know about
+    # those too.
+    if mode not in INTERESTING_MODES or status not in INTERESTING_STATUSES:
+        return
+
+    # If the course isn't in any program, don't bother telling Credentials about it. When Credentials grows support
+    # for course records as well as program records, we'll need to open this up.
+    if not is_course_run_in_a_program(course_run_key):
+        return
+
+    # Grab grades if we don't have them in hand
+    if letter_grade is None or percent_grade is None:
+        grade = CourseGradeFactory().read(user, course_key=course_run_key, create_if_needed=False)
+        if grade is None:
+            return
+        letter_grade = grade.letter_grade
+        percent_grade = grade.percent
+
+    send_grade_to_credentials.delay(user.username, str(course_run_key), True, letter_grade, percent_grade)
+
+
+def handle_grade_change(user, course_grade, course_key, **_kwargs):
+    """
+    Notifies the Credentials IDA about certain grades it needs for its records, when a grade changes.
+    """
+    send_grade_if_interesting(user, course_key, None, None, course_grade.letter_grade, course_grade.percent)
+
+
+def handle_cert_change(user, course_key, mode, status, **_kwargs):
+    """
+    Notifies the Credentials IDA about certain grades it needs for its records, when a cert changes.
+    """
+    send_grade_if_interesting(user, course_key, mode, status, None, None)

openedx/core/djangoapps/credentials/tasks/v1/tasks.py

@@ -0,0 +1,53 @@
+"""
+This file contains celery tasks for credentials-related functionality.
+"""
+from celery import task
+from celery.utils.log import get_task_logger
+from django.conf import settings
+from django.contrib.auth.models import User
+from opaque_keys.edx.keys import CourseKey
+
+from openedx.core.djangoapps.credentials.utils import get_credentials_api_client
+
+
+logger = get_task_logger(__name__)
+
+# Under cms the following setting is not defined, leading to errors during tests.
+# These tasks aren't strictly credentials generation, but are similar in the sense
+# that they generate records on the credentials side. And have a similar SLA.
+ROUTING_KEY = getattr(settings, 'CREDENTIALS_GENERATION_ROUTING_KEY', None)
+
+# Maximum number of retries before giving up.
+# For reference, 11 retries with exponential backoff yields a maximum waiting
+# time of 2047 seconds (about 30 minutes). Setting this to None could yield
+# unwanted behavior: infinite retries.
+MAX_RETRIES = 11
+
+
+@task(bind=True, ignore_result=True, routing_key=ROUTING_KEY)
+def send_grade_to_credentials(self, username, course_run_key, verified, letter_grade, percent_grade):
+    """ Celery task to notify the Credentials IDA of a grade change via POST. """
+    logger.info('Running task send_grade_to_credentials for username %s and course %s', username, course_run_key)
+
+    countdown = 2 ** self.request.retries
+    course_key = CourseKey.from_string(course_run_key)
+
+    try:
+        credentials_client = get_credentials_api_client(
+            User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME),
+            org=course_key.org,
+        )
+
+        credentials_client.grades.post({
+            'username': username,
+            'course_run': str(course_key),
+            'letter_grade': letter_grade,
+            'percent_grade': percent_grade,
+            'verified': verified,
+        })
+
+        logger.info('Sent grade for course %s to user %s', course_run_key, username)
+
+    except Exception as exc:
+        logger.exception('Failed to send grade for course %s to user %s', course_run_key, username)
+        raise self.retry(exc=exc, countdown=countdown, max_retries=MAX_RETRIES)

openedx/core/djangoapps/credentials/tests/factories.py

@@ -1,11 +1,9 @@
 """Factories for generating fake credentials-related data."""
 # pylint: disable=missing-docstring, invalid-name
-from functools import partial
 
 import factory
 
 from openedx.core.djangoapps.catalog.tests.factories import (
-    generate_instances,
     generate_course_run_key,
     DictFactoryBase,
 )

openedx/core/djangoapps/credentials/tests/test_models.py

@@ -30,6 +30,9 @@ class TestCredentialsApiConfig(CredentialsApiConfigMixin, TestCase):
         expected = '{root}/api/{version}/'.format(root=CREDENTIALS_INTERNAL_SERVICE_URL.strip('/'), version=API_VERSION)
         self.assertEqual(credentials_config.internal_api_url, expected)
 
+        expected = '{root}/api/{version}/'.format(root=CREDENTIALS_INTERNAL_SERVICE_URL.strip('/'), version=API_VERSION)
+        self.assertEqual(credentials_config.get_internal_api_url_for_org('nope'), expected)
+
         expected = '{root}/api/{version}/'.format(root=CREDENTIALS_PUBLIC_SERVICE_URL.strip('/'), version=API_VERSION)
         self.assertEqual(credentials_config.public_api_url, expected)
 

openedx/core/djangoapps/credentials/tests/test_signals.py

@@ -0,0 +1,133 @@
+"""Tests covering Credentials signals."""
+import ddt
+import mock
+from django.conf import settings
+from django.test import TestCase
+from opaque_keys.edx.keys import CourseKey
+
+from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
+from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
+from lms.djangoapps.grades.tests.utils import mock_passing_grade
+from openedx.core.djangoapps.catalog.tests.factories import CourseFactory, CourseRunFactory, ProgramFactory
+from openedx.core.djangoapps.credentials.signals import is_course_run_in_a_program, send_grade_if_interesting
+from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
+from openedx.core.djangolib.testing.utils import skip_unless_lms
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory as XModuleCourseFactory
+
+
+SIGNALS_MODULE = 'openedx.core.djangoapps.credentials.signals'
+
+
+@ddt.ddt
+@skip_unless_lms
+@mock.patch(SIGNALS_MODULE + '.send_grade_to_credentials')
+@mock.patch(SIGNALS_MODULE + '.is_course_run_in_a_program')
+class TestCredentialsSignalsSendGrade(TestCase):
+    """ Tests for send_grade_if_interesting, the main utility function that sends a grade """
+
+    def setUp(self):
+        super(TestCredentialsSignalsSendGrade, self).setUp()
+        self.user = UserFactory()
+        self.key = CourseKey.from_string(CourseRunFactory()['key'])
+
+    @ddt.data(
+        [True, 'verified', 'downloadable'],
+        [True, 'professional', 'downloadable'],
+        [True, 'credit', 'downloadable'],
+        [True, 'verified', 'notpassing'],
+        [False, 'audit', 'downloadable'],
+        [False, 'professional', 'generating'],
+    )
+    @ddt.unpack
+    def test_send_grade_if_right_cert(self, called, mode, status, mock_is_course_run_in_a_program,
+                                      mock_send_grade_to_credentials):
+        mock_is_course_run_in_a_program.return_value = True
+
+        # Test direct send
+        send_grade_if_interesting(self.user, self.key, mode, status, 'A', 1.0)
+        self.assertIs(mock_send_grade_to_credentials.delay.called, called)
+        mock_send_grade_to_credentials.delay.reset_mock()
+
+        # Test query
+        GeneratedCertificateFactory(
+            user=self.user,
+            course_id=self.key,
+            status=status,
+            mode=mode
+        )
+        send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0)
+        self.assertIs(mock_send_grade_to_credentials.delay.called, called)
+
+    def test_send_grade_missing_cert(self, _, mock_send_grade_to_credentials):
+        send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0)
+        self.assertFalse(mock_send_grade_to_credentials.delay.called)
+
+    @ddt.data([True], [False])
+    @ddt.unpack
+    def test_send_grade_if_in_a_program(self, in_program, mock_is_course_run_in_a_program,
+                                        mock_send_grade_to_credentials):
+        mock_is_course_run_in_a_program.return_value = in_program
+        send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', 'A', 1.0)
+        self.assertIs(mock_send_grade_to_credentials.delay.called, in_program)
+
+    def test_send_grade_queries_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials):
+        mock_is_course_run_in_a_program.return_value = True
+
+        with mock_passing_grade('B', 0.81):
+            send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None)
+        self.assertTrue(mock_send_grade_to_credentials.delay.called)
+        self.assertEqual(mock_send_grade_to_credentials.delay.call_args[0],
+                         (self.user.username, str(self.key), True, 'B', 0.81))
+        mock_send_grade_to_credentials.delay.reset_mock()
+
+    @mock.patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
+    def test_send_grade_without_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials):
+        mock_is_course_run_in_a_program.return_value = True
+        send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None)
+        self.assertFalse(mock_send_grade_to_credentials.delay.called)
+
+
+@skip_unless_lms
+@mock.patch(SIGNALS_MODULE + '.get_programs')
+class TestCredentialsSignalsUtils(TestCase):
+    """ Tests helper utility functions in our signal handling. """
+
+    def setUp(self):
+        super(TestCredentialsSignalsUtils, self).setUp()
+        self.site = SiteFactory()
+        self.course_run = CourseRunFactory()
+        course = CourseFactory(course_runs=[self.course_run])
+        self.data = [ProgramFactory(courses=[course])]
+
+    def test_is_course_run_in_a_program_success(self, mock_get_programs):
+        mock_get_programs.return_value = self.data
+        self.assertTrue(is_course_run_in_a_program(self.course_run['key']))
+        self.assertEqual(mock_get_programs.call_args[0], (self.site,))
+
+    def test_is_course_run_in_a_program_failure(self, mock_get_programs):
+        mock_get_programs.return_value = self.data
+        course_run2 = CourseRunFactory()
+        self.assertFalse(is_course_run_in_a_program(course_run2['key']))
+
+
+@skip_unless_lms
+@mock.patch(SIGNALS_MODULE + '.send_grade_if_interesting')
+class TestCredentialsSignalsEmissions(ModuleStoreTestCase):
+    """ Tests for whether we are receiving signal emissions correctly. """
+
+    def test_cert_changed(self, mock_send_grade_if_interesting):
+        user = UserFactory()
+
+        self.assertFalse(mock_send_grade_if_interesting.called)
+        GeneratedCertificateFactory(user=user)
+        self.assertTrue(mock_send_grade_if_interesting.called)
+
+    def test_grade_changed(self, mock_send_grade_if_interesting):
+        user = UserFactory()
+        course = XModuleCourseFactory()
+
+        self.assertFalse(mock_send_grade_if_interesting.called)
+        CourseGradeFactory().update(user, course=course)
+        self.assertTrue(mock_send_grade_if_interesting.called)

openedx/core/djangoapps/credentials/tests/test_tasks.py

@@ -0,0 +1,62 @@
+"""
+Test credentials tasks
+"""
+import mock
+from django.conf import settings
+from django.test import override_settings, TestCase
+
+from openedx.core.djangolib.testing.utils import skip_unless_lms
+from student.tests.factories import UserFactory
+
+from ..tasks.v1 import tasks
+
+TASKS_MODULE = 'openedx.core.djangoapps.credentials.tasks.v1.tasks'
+
+
+def boom():
+    raise Exception('boom')
+
+
+@skip_unless_lms
+@mock.patch(TASKS_MODULE + '.get_credentials_api_client')
+@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username')
+class TestSendGradeToCredentialTask(TestCase):
+    """
+    Tests for the 'send_grade_to_credentials' method.
+    """
+    def setUp(self):
+        super(TestSendGradeToCredentialTask, self).setUp()
+        self.user = UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)
+
+    def test_happy_path(self, mock_get_api_client):
+        """
+        Test that we actually do check expiration on each entitlement (happy path)
+        """
+        api_client = mock.MagicMock()
+        mock_get_api_client.return_value = api_client
+
+        tasks.send_grade_to_credentials.delay('user', 'course-v1:org+course+run', True, 'A', 1.0).get()
+
+        self.assertEqual(mock_get_api_client.call_count, 1)
+        self.assertEqual(mock_get_api_client.call_args[0], (self.user,))
+        self.assertDictEqual(mock_get_api_client.call_args[1], {'org': 'org'})
+
+        self.assertEqual(api_client.grades.post.call_count, 1)
+        self.assertDictEqual(api_client.grades.post.call_args[0][0], {
+            'username': 'user',
+            'course_run': 'course-v1:org+course+run',
+            'letter_grade': 'A',
+            'percent_grade': 1.0,
+            'verified': True,
+        })
+
+    def test_retry(self, mock_get_api_client):
+        """
+        Test that we retry when an exception occurs.
+        """
+        mock_get_api_client.side_effect = boom
+
+        task = tasks.send_grade_to_credentials.delay('user', 'course-v1:org+course+run', True, 'A', 1.0)
+
+        self.assertRaises(Exception, task.get)
+        self.assertEqual(mock_get_api_client.call_count, tasks.MAX_RETRIES + 1)

openedx/core/djangoapps/credentials/tests/test_utils.py

@@ -21,6 +21,8 @@ UTILS_MODULE = 'openedx.core.djangoapps.credentials.utils'
 @attr(shard=2)
 @mock.patch(UTILS_MODULE + '.get_edx_api_data')
 class TestGetCredentials(CredentialsApiConfigMixin, CacheIsolationTestCase):
+    """ Tests for credentials utility functions. """
+
     ENABLED_CACHES = ['default']
 
     def setUp(self):

scripts/thresholds.sh

@@ -2,6 +2,6 @@
 set -e
 
 export LOWER_PYLINT_THRESHOLD=1000
-export UPPER_PYLINT_THRESHOLD=5900
+export UPPER_PYLINT_THRESHOLD=3965
 export ESLINT_THRESHOLD=5590
 export STYLELINT_THRESHOLD=973