Merge pull request #16048 from edx/sstudent/EDUCATOR-1288

Sstudent/educator 1288

common/djangoapps/student/models.py

@@ -52,6 +52,7 @@ from certificates.models import GeneratedCertificate
 from course_modes.models import CourseMode
 from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration
 from enrollment.api import _default_course_mode
+
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.schedules.models import ScheduleConfig
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
@@ -62,6 +63,8 @@ from util.milestones_helpers import is_entrance_exams_enabled
 from util.model_utils import emit_field_changed_events, get_changed_fields_dict
 from util.query import use_read_replica_if_available
 
+from .signals.signals import ENROLLMENT_TRACK_UPDATED
+
 UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"])
 ENROLL_STATUS_CHANGE = Signal(providing_args=["event", "user", "course_id", "mode", "cost", "currency"])
 REFUND_ORDER = Signal(providing_args=["course_enrollment"])
@@ -1191,6 +1194,7 @@ class CourseEnrollment(models.Model):
             # Only emit mode change events when the user's enrollment
             # mode has changed from its previous setting
             self.emit_event(EVENT_NAME_ENROLLMENT_MODE_CHANGED)
+            ENROLLMENT_TRACK_UPDATED.send(sender=None, user=self.user, course_key=self.course_id)
 
     def send_signal(self, event, cost=None, currency=None):
         """

common/djangoapps/student/signals/signals.py

+"""
+Enrollment track related signals.
+"""
+from django.dispatch import Signal
+
+ENROLLMENT_TRACK_UPDATED = Signal(providing_args=['user', 'course_key'])

lms/djangoapps/grades/signals/handlers.py

@@ -11,8 +11,10 @@ from xblock.scorable import ScorableXBlockMixin, Score
 from courseware.model_data import get_score, set_score
 from eventtracking import tracker
 from lms.djangoapps.instructor_task.tasks_helper.module_state import GRADES_OVERRIDE_EVENT_TYPE
+from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED
 from openedx.core.lib.grade_utils import is_score_higher_or_equal
 from student.models import user_by_anonymous_id
+from student.signals.signals import ENROLLMENT_TRACK_UPDATED
 from submissions.models import score_reset, score_set
 from track.event_transaction_utils import (
     create_new_event_transaction_id,
@@ -22,6 +24,7 @@ from track.event_transaction_utils import (
 )
 from util.date_utils import to_timestamp
 
+from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED
 from ..constants import ScoreDatabaseTableEnum
 from ..new.course_grade_factory import CourseGradeFactory
 from ..scores import weighted_score
@@ -31,7 +34,7 @@ from .signals import (
     PROBLEM_WEIGHTED_SCORE_CHANGED,
     SCORE_PUBLISHED,
     SUBSECTION_SCORE_CHANGED,
-    SUBSECTION_OVERRIDE_CHANGED
+    SUBSECTION_OVERRIDE_CHANGED,
 )
 
 log = getLogger(__name__)
@@ -237,13 +240,28 @@ def enqueue_subsection_update(sender, **kwargs):  # pylint: disable=unused-argum
 
 
 @receiver(SUBSECTION_SCORE_CHANGED)
-def recalculate_course_grade(sender, course, course_structure, user, **kwargs):  # pylint: disable=unused-argument
+def recalculate_course_grade_only(sender, course, course_structure, user, **kwargs):  # pylint: disable=unused-argument
     """
-    Updates a saved course grade.
+    Updates a saved course grade, but does not update the subsection
+    grades the user has in this course.
     """
     CourseGradeFactory().update(user, course=course, course_structure=course_structure)
 
 
+@receiver(ENROLLMENT_TRACK_UPDATED)
+@receiver(COHORT_MEMBERSHIP_UPDATED)
+def force_recalculate_course_and_subsection_grades(sender, user, course_key, **kwargs):
+    """
+    Updates a saved course grade, forcing the subsection grades
+    from which it is calculated to update along the way.
+
+    Does not create a grade if the user has never attempted a problem,
+    even if the WRITE_ONLY_IF_ENGAGED waffle switch is off.
+    """
+    if waffle().is_enabled(WRITE_ONLY_IF_ENGAGED) or CourseGradeFactory().read(user, course_key=course_key):
+        CourseGradeFactory().update(user=user, course_key=course_key, force_update_subsections=True)
+
+
 def _emit_event(kwargs):
     """
     Emits a problem submitted event only if there is no current event

lms/djangoapps/grades/tests/test_signals.py

 """
 Tests for the score change signals defined in the courseware models module.
 """
-
+import itertools
 import re
 from datetime import datetime
 
@@ -10,15 +10,20 @@ import pytz
 from django.test import TestCase
 from mock import MagicMock, patch
 
+from opaque_keys.edx.locations import CourseLocator
+from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED
+from student.signals.signals import ENROLLMENT_TRACK_UPDATED
+from student.tests.factories import UserFactory
 from submissions.models import score_reset, score_set
 from util.date_utils import to_timestamp
 
+from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED
 from ..constants import ScoreDatabaseTableEnum
 from ..signals.handlers import (
     disconnect_submissions_signal_receiver,
     problem_raw_score_changed_handler,
     submissions_score_reset_handler,
-    submissions_score_set_handler
+    submissions_score_set_handler,
 )
 from ..signals.signals import PROBLEM_RAW_SCORE_CHANGED
 
@@ -251,3 +256,32 @@ class ScoreChangedSignalRelayTest(TestCase):
         with self.assertRaises(ValueError):
             with disconnect_submissions_signal_receiver(PROBLEM_RAW_SCORE_CHANGED):
                 pass
+
+
+@ddt.ddt
+class RecalculateUserGradeSignalsTest(TestCase):
+    def setUp(self):
+        super(RecalculateUserGradeSignalsTest, self).setUp()
+        self.user = UserFactory()
+        self.course_key = CourseLocator("test_org", "test_course_num", "test_run")
+
+    @patch('lms.djangoapps.grades.signals.handlers.CourseGradeFactory.update')
+    @patch('lms.djangoapps.grades.signals.handlers.CourseGradeFactory.read')
+    @ddt.data(*itertools.product((COHORT_MEMBERSHIP_UPDATED, ENROLLMENT_TRACK_UPDATED), (True, False), (True, False)))
+    @ddt.unpack
+    def test_recalculate_on_signal(self, signal, write_only_if_engaged, has_grade, read_mock, update_mock):
+        """
+        Tests the grades handler for signals that trigger regrading.
+        The handler should call CourseGradeFactory.update() with the
+        args below, *except* if the WRITE_ONLY_IF_ENGAGED waffle flag
+        is inactive and the user does not have a grade.
+        """
+        if not has_grade:
+            read_mock.return_value = None
+        with waffle().override(WRITE_ONLY_IF_ENGAGED, active=write_only_if_engaged):
+            signal.send(sender=None, user=self.user, course_key=self.course_key)
+
+        if not write_only_if_engaged and not has_grade:
+            update_mock.assert_not_called()
+        else:
+            update_mock.assert_called_with(course_key=self.course_key, user=self.user, force_update_subsections=True)

openedx/core/djangoapps/course_groups/cohorts.py

@@ -28,6 +28,7 @@ from .models import (
     CourseUserGroupPartitionGroup,
     UnregisteredLearnerCohortAssignments
 )
+from .signals.signals import COHORT_MEMBERSHIP_UPDATED
 
 log = logging.getLogger(__name__)
 
@@ -424,7 +425,9 @@ def remove_user_from_cohort(cohort, username_or_email):
 
     try:
         membership = CohortMembership.objects.get(course_user_group=cohort, user=user)
+        course_key = membership.course_id
         membership.delete()
+        COHORT_MEMBERSHIP_UPDATED.send(sender=None, user=user, course_key=course_key)
     except CohortMembership.DoesNotExist:
         raise ValueError("User {} was not present in cohort {}".format(username_or_email, cohort))
 
@@ -454,7 +457,7 @@ def add_user_to_cohort(cohort, username_or_email):
 
         membership = CohortMembership(course_user_group=cohort, user=user)
         membership.save()  # This will handle both cases, creation and updating, of a CohortMembership for this user.
-
+        COHORT_MEMBERSHIP_UPDATED.send(sender=None, user=user, course_key=membership.course_id)
         tracker.emit(
             "edx.cohort.user_add_requested",
             {

openedx/core/djangoapps/course_groups/signals/signals.py

+"""
+Cohorts related signals.
+"""
+from django.dispatch import Signal
+
+COHORT_MEMBERSHIP_UPDATED = Signal(providing_args=['user', 'course_key'])

openedx/core/djangoapps/course_groups/tests/test_cohorts.py

@@ -591,7 +591,8 @@ class TestCohorts(ModuleStoreTestCase):
         )
 
     @patch("openedx.core.djangoapps.course_groups.cohorts.tracker")
-    def test_add_user_to_cohort(self, mock_tracker):
+    @patch("openedx.core.djangoapps.course_groups.cohorts.COHORT_MEMBERSHIP_UPDATED")
+    def test_add_user_to_cohort(self, mock_signal, mock_tracker):
         """
         Make sure cohorts.add_user_to_cohort() properly adds a user to a cohort and
         handles errors.
@@ -603,6 +604,10 @@ class TestCohorts(ModuleStoreTestCase):
         first_cohort = CohortFactory(course_id=course.id, name="FirstCohort")
         second_cohort = CohortFactory(course_id=course.id, name="SecondCohort")
 
+        def check_and_reset_signal():
+            mock_signal.send.assert_called_with(sender=None, user=course_user, course_key=self.toy_course_key)
+            mock_signal.reset_mock()
+
         # Success cases
         # We shouldn't get back a previous cohort, since the user wasn't in one
         self.assertEqual(
@@ -619,6 +624,8 @@ class TestCohorts(ModuleStoreTestCase):
                 "previous_cohort_name": None,
             }
         )
+        check_and_reset_signal()
+
         # Should get (user, previous_cohort_name) when moved from one cohort to
         # another
         self.assertEqual(
@@ -635,6 +642,8 @@ class TestCohorts(ModuleStoreTestCase):
                 "previous_cohort_name": first_cohort.name,
             }
         )
+        check_and_reset_signal()
+
         # Should preregister email address for a cohort if an email address
         # not associated with a user is added
         (user, previous_cohort, prereg) = cohorts.add_user_to_cohort(first_cohort, "new_email@example.com")
@@ -650,6 +659,7 @@ class TestCohorts(ModuleStoreTestCase):
                 "cohort_name": first_cohort.name,
             }
         )
+
         # Error cases
         # Should get ValueError if user already in cohort
         self.assertRaises(

openedx/core/djangoapps/credit/tests/test_api.py

@@ -158,7 +158,8 @@ class CreditApiTestBase(ModuleStoreTestCase):
 
     def setUp(self, **kwargs):
         super(CreditApiTestBase, self).setUp()
-        self.course_key = CourseKey.from_string("edX/DemoX/Demo_Course")
+        self.course = CourseFactory.create(org="edx", course="DemoX", run="Demo_Course")
+        self.course_key = self.course.id
 
     def add_credit_course(self, course_key=None, enabled=True):
         """Mark the course as a credit """
@@ -631,8 +632,6 @@ class CreditRequirementApiTests(CreditApiTestBase):
         # Configure a course with two credit requirements
         self.add_credit_course()
         user = self.create_and_enroll_user(username=self.USER_INFO['username'], password=self.USER_INFO['password'])
-        CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course')
-
         requirements = [
             {
                 "namespace": "grade",
@@ -664,7 +663,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
         self.assertFalse(api.is_user_eligible_for_credit(user.username, self.course_key))
 
         # Satisfy the other requirement
-        with self.assertNumQueries(24):
+        with self.assertNumQueries(23):
             api.set_credit_requirement_status(
                 user,
                 self.course_key,
@@ -822,7 +821,6 @@ class CreditRequirementApiTests(CreditApiTestBase):
         # Configure a course with two credit requirements
         self.add_credit_course()
         user = self.create_and_enroll_user(username=self.USER_INFO['username'], password=self.USER_INFO['password'])
-        CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course')
         requirements = [
             {
                 "namespace": "grade",