Merge pull request #13185 from edx/aj/ECOM5081-hide-credit-btn

Hide credit button from  audit track students

common/djangoapps/course_modes/models.py

@@ -128,6 +128,9 @@ class CourseMode(models.Model):
     # Modes that allow a student to earn credit with a university partner
     CREDIT_MODES = [CREDIT_MODE]
 
+    # Modes that are eligible to purchase credit
+    CREDIT_ELIGIBLE_MODES = [VERIFIED, PROFESSIONAL, NO_ID_PROFESSIONAL_MODE]
+
     # Modes that are allowed to upsell
     UPSELL_TO_VERIFIED_MODES = [HONOR, AUDIT]
 
@@ -489,6 +492,18 @@ class CourseMode(models.Model):
         return mode_slug in cls.VERIFIED_MODES
 
     @classmethod
+    def is_credit_eligible_slug(cls, mode_slug):
+        """Check whether the given mode_slug is credit eligible or not.
+
+        Args:
+            mode_slug(str): Mode Slug
+
+        Returns:
+            bool: True iff the course mode slug is credit eligible else False.
+        """
+        return mode_slug in cls.CREDIT_ELIGIBLE_MODES
+
+    @classmethod
     def is_credit_mode(cls, course_mode_tuple):
         """Check whether this is a credit mode.
 

common/djangoapps/student/tests/test_credit.py

@@ -177,7 +177,7 @@ class CreditCourseDashboardTest(ModuleStoreTestCase):
     def _make_eligible(self):
         """Make the user eligible for credit in the course. """
         credit_api.set_credit_requirement_status(
-            self.USERNAME,
+            self.user,
             self.course.id,  # pylint: disable=no-member
             "grade", "grade",
             status="satisfied",

lms/djangoapps/courseware/tests/test_credit_requirements.py

@@ -97,16 +97,19 @@ class ProgressPageCreditRequirementsTest(SharedModuleStoreTestCase):
         )
 
     def test_credit_requirements_eligible(self):
-        # Mark the user as eligible for all requirements
+        """
+        Mark the user as eligible for all requirements. Requirements are only displayed
+        for credit and verified enrollments.
+        """
         credit_api.set_credit_requirement_status(
-            self.user.username, self.course.id,
+            self.user, self.course.id,
             "grade", "grade",
             status="satisfied",
             reason={"final_grade": 0.95}
         )
 
         credit_api.set_credit_requirement_status(
-            self.user.username, self.course.id,
+            self.user, self.course.id,
             "reverification", "midterm",
             status="satisfied", reason={}
         )
@@ -123,9 +126,12 @@ class ProgressPageCreditRequirementsTest(SharedModuleStoreTestCase):
         self.assertNotContains(response, "95%")
 
     def test_credit_requirements_not_eligible(self):
-        # Mark the user as having failed both requirements
+        """
+        Mark the user as having failed both requirements. Requirements are only displayed
+        for credit and verified enrollments.
+        """
         credit_api.set_credit_requirement_status(
-            self.user.username, self.course.id,
+            self.user, self.course.id,
             "reverification", "midterm",
             status="failed", reason={}
         )

lms/djangoapps/courseware/tests/test_module_render.py

@@ -940,7 +940,6 @@ class TestProctoringRendering(SharedModuleStoreTestCase):
         Verifies gated content from the student view rendering of a sequence
         this is labeled as a proctored exam
         """
-
         usage_key = self._setup_test_data(enrollment_mode, is_practice_exam, attempt_status)
 
         # initialize some credit requirements, if so then specify
@@ -966,7 +965,7 @@ class TestProctoringRendering(SharedModuleStoreTestCase):
             )
 
             set_credit_requirement_status(
-                self.request.user.username,
+                self.request.user,
                 self.course_key,
                 'reverification',
                 'ICRV1'
@@ -1021,7 +1020,6 @@ class TestProctoringRendering(SharedModuleStoreTestCase):
         if attempt_status:
             create_exam_attempt(exam_id, self.request.user.id, taking_as_proctored=True)
             update_attempt_status(exam_id, self.request.user.id, attempt_status)
-
         return usage_key
 
     def _find_url_name(self, toc, url_name):

lms/djangoapps/courseware/tests/test_submitting_problems.py

@@ -2,6 +2,7 @@
 """
 Integration tests for submitting problem responses and getting grades.
 """
+import ddt
 import json
 import os
 from textwrap import dedent
@@ -19,10 +20,11 @@ from capa.tests.response_xml_factory import (
     CodeResponseXMLFactory,
 )
 from lms.djangoapps.grades import course_grades, progress
+from course_modes.models import CourseMode
 from courseware.models import StudentModule, BaseStudentModuleHistory
 from courseware.tests.helpers import LoginEnrollmentTestCase
 from lms.djangoapps.lms_xblock.runtime import quote_slashes
-from student.models import anonymous_id_for_user
+from student.models import anonymous_id_for_user, CourseEnrollment
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.partitions.partitions import Group, UserPartition
@@ -304,6 +306,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
 
 
 @attr(shard=3)
+@ddt.ddt
 class TestCourseGrader(TestSubmittingProblems):
     """
     Suite of tests for the course grader.
@@ -315,7 +318,6 @@ class TestCourseGrader(TestSubmittingProblems):
         """
         Set up a simple course for testing basic grading functionality.
         """
-
         grading_policy = {
             "GRADER": [{
                 "type": "Homework",
@@ -646,20 +648,26 @@ class TestCourseGrader(TestSubmittingProblems):
         self.assertEqual(self.earned_hw_scores(), [1.0, 2.0, 2.0])  # Order matters
         self.assertEqual(self.score_for_hw('homework3'), [1.0, 1.0])
 
-    def test_min_grade_credit_requirements_status(self):
+    @ddt.data(
+        *CourseMode.CREDIT_ELIGIBLE_MODES
+    )
+    def test_min_grade_credit_requirements_status(self, mode):
         """
         Test for credit course. If user passes minimum grade requirement then
         status will be updated as satisfied in requirement status table.
         """
         self.basic_setup()
+
+        # Enroll student in credit eligible mode.
+        # Note that we can't call self.enroll here since that goes through
+        # the Django student views, and does not update enrollment if it already exists.
+        CourseEnrollment.enroll(self.student_user, self.course.id, mode)
+
         self.submit_question_answer('p1', {'2_1': 'Correct'})
         self.submit_question_answer('p2', {'2_1': 'Correct'})
 
         # Enable the course for credit
-        credit_course = CreditCourse.objects.create(
-            course_key=self.course.id,
-            enabled=True,
-        )
+        CreditCourse.objects.create(course_key=self.course.id, enabled=True)
 
         # Configure a credit provider for the course
         CreditProvider.objects.create(

lms/djangoapps/grades/new/course_grade.py

@@ -155,7 +155,7 @@ class CourseGrade(object):
         """
         responses = GRADES_UPDATED.send_robust(
             sender=None,
-            username=self.student.username,
+            user=self.student,
             grade_summary=self.summary,
             course_key=self.course.id,
             deadline=self.course.end

lms/djangoapps/verify_student/services.py

@@ -112,7 +112,7 @@ class ReverificationService(object):
             # As a user skips the reverification it declines to fulfill the requirement so
             # requirement sets to declined.
             set_credit_requirement_status(
-                user.username,
+                user,
                 course_key,
                 'reverification',
                 checkpoint.checkpoint_location,

lms/djangoapps/verify_student/tests/test_services.py

@@ -124,7 +124,10 @@ class TestReverificationService(ModuleStoreTestCase):
             'skipped'
         )
 
-    def test_declined_verification_on_skip(self):
+    @ddt.data(
+        *CourseMode.CREDIT_ELIGIBLE_MODES
+    )
+    def test_declined_verification_on_skip(self, mode):
         """Test that status with value 'declined' is added in credit
         requirement status model when a user skip's an ICRV.
         """
@@ -135,6 +138,8 @@ class TestReverificationService(ModuleStoreTestCase):
         )
         # Create credit course and set credit requirements.
         CreditCourse.objects.create(course_key=self.course_key, enabled=True)
+        self.enrollment.update_enrollment(mode=mode)
+
         set_credit_requirements(
             self.course_key,
             [

lms/djangoapps/verify_student/views.py

@@ -1245,7 +1245,7 @@ def _set_user_requirement_status(attempt, namespace, status, reason=None):
     if checkpoint is not None:
         try:
             set_credit_requirement_status(
-                attempt.user.username,
+                attempt.user,
                 checkpoint.course_id,
                 namespace,
                 checkpoint.checkpoint_location,

openedx/core/djangoapps/credit/api/eligibility.py

@@ -13,6 +13,9 @@ from openedx.core.djangoapps.credit.models import (
     CreditCourse, CreditRequirement, CreditRequirementStatus, CreditEligibility, CreditRequest
 )
 
+from course_modes.models import CourseMode
+from student.models import CourseEnrollment
+
 # TODO: Cleanup this mess! ECOM-2908
 
 log = logging.getLogger(__name__)
@@ -196,7 +199,7 @@ def get_eligibilities_for_user(username, course_key=None):
     ]
 
 
-def set_credit_requirement_status(username, course_key, req_namespace, req_name, status="satisfied", reason=None):
+def set_credit_requirement_status(user, course_key, req_namespace, req_name, status="satisfied", reason=None):
     """
     Update the user's requirement status.
 
@@ -205,7 +208,7 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name,
     as eligible for credit in the course.
 
     Args:
-        username (str): Username of the user
+        user(User): User object to set credit requirement for.
         course_key (CourseKey): Identifier for the course associated with the requirement.
         req_namespace (str): Namespace of the requirement (e.g. "grade" or "reverification")
         req_name (str): Name of the requirement (e.g. "grade" or the location of the ICRV XBlock)
@@ -225,22 +228,30 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name,
             )
 
     """
+    # Check whether user has credit eligible enrollment.
+    enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(user, course_key)
+    has_credit_eligible_enrollment = (CourseMode.is_credit_eligible_slug(enrollment_mode) and is_active)
+
+    # Refuse to set status of requirement if the user enrollment is not credit eligible.
+    if not has_credit_eligible_enrollment:
+        return
+
     # Do not allow students who have requested credit to change their eligibility
-    if CreditRequest.get_user_request_status(username, course_key):
+    if CreditRequest.get_user_request_status(user.username, course_key):
         log.info(
             u'Refusing to set status of requirement with namespace "%s" and name "%s" because the '
             u'user "%s" has already requested credit for the course "%s".',
-            req_namespace, req_name, username, course_key
+            req_namespace, req_name, user.username, course_key
         )
         return
 
     # Do not allow a student who has earned eligibility to un-earn eligibility
-    eligible_before_update = CreditEligibility.is_user_eligible_for_credit(course_key, username)
+    eligible_before_update = CreditEligibility.is_user_eligible_for_credit(course_key, user.username)
     if eligible_before_update and status == 'failed':
         log.info(
             u'Refusing to set status of requirement with namespace "%s" and name "%s" to "failed" because the '
             u'user "%s" is already eligible for credit in the course "%s".',
-            req_namespace, req_name, username, course_key
+            req_namespace, req_name, user.username, course_key
         )
         return
 
@@ -269,22 +280,22 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name,
                 u'because the requirement does not exist. '
                 u'The user "%s" should have had his/her status updated to "%s".'
             ),
-            unicode(course_key), req_namespace, req_name, username, status
+            unicode(course_key), req_namespace, req_name, user.username, status
         )
         return
 
     # Update the requirement status
     CreditRequirementStatus.add_or_update_requirement_status(
-        username, req_to_update, status=status, reason=reason
+        user.username, req_to_update, status=status, reason=reason
     )
 
     # If we're marking this requirement as "satisfied", there's a chance that the user has met all eligibility
     # requirements, and should be notified. However, if the user was already eligible, do not send another notification.
     if status == "satisfied" and not eligible_before_update:
-        is_eligible, eligibility_record_created = CreditEligibility.update_eligibility(reqs, username, course_key)
+        is_eligible, eligibility_record_created = CreditEligibility.update_eligibility(reqs, user.username, course_key)
         if eligibility_record_created and is_eligible:
             try:
-                send_credit_notifications(username, course_key)
+                send_credit_notifications(user.username, course_key)
             except Exception:  # pylint: disable=broad-except
                 log.error("Error sending email")
 

openedx/core/djangoapps/credit/services.py

@@ -155,7 +155,7 @@ class CreditService(object):
             return None
 
         api_set_credit_requirement_status(
-            user.username,
+            user,
             course_key,
             req_namespace,
             req_name,

openedx/core/djangoapps/credit/signals.py

@@ -53,12 +53,12 @@ def on_pre_publish(sender, course_key, **kwargs):  # pylint: disable=unused-argu
 
 
 @receiver(GRADES_UPDATED)
-def listen_for_grade_calculation(sender, username, grade_summary, course_key, deadline, **kwargs):  # pylint: disable=unused-argument
+def listen_for_grade_calculation(sender, user, grade_summary, course_key, deadline, **kwargs):  # pylint: disable=unused-argument
     """Receive 'MIN_GRADE_REQUIREMENT_STATUS' signal and update minimum grade requirement status.
 
     Args:
         sender: None
-        username(string): user name
+        user(User): User Model object
         grade_summary(dict): Dict containing output from the course grader
         course_key(CourseKey): The key for the course
         deadline(datetime): Course end date or None
@@ -70,7 +70,6 @@ def listen_for_grade_calculation(sender, username, grade_summary, course_key, de
     # This needs to be imported here to avoid a circular dependency
     # that can cause syncdb to fail.
     from openedx.core.djangoapps.credit import api
-
     course_id = CourseKey.from_string(unicode(course_key))
     is_credit = api.is_credit_course(course_id)
     if is_credit:
@@ -113,5 +112,5 @@ def listen_for_grade_calculation(sender, username, grade_summary, course_key, de
                 # time to do so.
                 if status and reason:
                     api.set_credit_requirement_status(
-                        username, course_id, 'grade', 'grade', status=status, reason=reason
+                        user, course_id, 'grade', 'grade', status=status, reason=reason
                     )

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

@@ -2,7 +2,9 @@
 Tests for the Credit xBlock service
 """
 
+import ddt
 from nose.plugins.attrib import attr
+from course_modes.models import CourseMode
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -15,6 +17,7 @@ from student.models import CourseEnrollment, UserProfile
 
 
 @attr(shard=2)
+@ddt.ddt
 class CreditServiceTests(ModuleStoreTestCase):
     """
     Tests for the Credit xBlock service
@@ -28,14 +31,14 @@ class CreditServiceTests(ModuleStoreTestCase):
         self.credit_course = CreditCourse.objects.create(course_key=self.course.id, enabled=True)
         self.profile = UserProfile.objects.create(user_id=self.user.id, name='Foo Bar')
 
-    def enroll(self, course_id=None):
+    def enroll(self, course_id=None, mode=CourseMode.VERIFIED):
         """
-        Enroll the test user in the given course's honor mode, or the test
-        course if not provided.
+        Enroll the test user in the given course's mode. Use course/mode if they are
+        provided.
         """
         if course_id is None:
             course_id = self.course.id
-        return CourseEnrollment.enroll(self.user, course_id, mode='honor')
+        return CourseEnrollment.enroll(self.user, course_id, mode=mode)
 
     def test_user_not_found(self):
         """
@@ -127,7 +130,7 @@ class CreditServiceTests(ModuleStoreTestCase):
 
         self.assertIsNotNone(credit_state)
         self.assertTrue(credit_state['is_credit_course'])
-        self.assertEqual(credit_state['enrollment_mode'], 'honor')
+        self.assertEqual(credit_state['enrollment_mode'], 'verified')
         self.assertEqual(credit_state['profile_fullname'], 'Foo Bar')
         self.assertEqual(len(credit_state['credit_requirement_status']), 1)
         self.assertEqual(credit_state['credit_requirement_status'][0]['name'], 'grade')
@@ -286,6 +289,48 @@ class CreditServiceTests(ModuleStoreTestCase):
         self.assertFalse(credit_state['is_credit_course'])
         self.assertEqual(len(credit_state['credit_requirement_status']), 0)
 
+    @ddt.data(
+        CourseMode.AUDIT,
+        CourseMode.HONOR,
+        CourseMode.CREDIT_MODE
+    )
+    def test_set_status_non_verified_enrollment(self, mode):
+        """
+        Test that we can still try to update a credit status but return quickly if
+        user has non-credit eligible enrollment.
+        """
+        self.enroll(mode=mode)
+
+        # set course requirements
+        set_credit_requirements(
+            self.course.id,
+            [
+                {
+                    "namespace": "grade",
+                    "name": "grade",
+                    "display_name": "Grade",
+                    "criteria": {
+                        "min_grade": 0.8
+                    },
+                },
+            ]
+        )
+
+        # this should be a no-op
+        self.service.set_credit_requirement_status(
+            self.user.id,
+            self.course.id,
+            'grade',
+            'grade'
+        )
+        # Verify credit requirement status for user in the course should be None.
+        credit_state = self.service.get_credit_state(self.user.id, self.course.id)
+        self.assertIsNotNone(credit_state)
+        self.assertEqual(credit_state['enrollment_mode'], mode)
+        self.assertEqual(len(credit_state['credit_requirement_status']), 1)
+        self.assertIsNone(credit_state['credit_requirement_status'][0]['status'])
+        self.assertIsNone(credit_state['credit_requirement_status'][0]['status_date'])
+
     def test_bad_user(self):
         """
         Try setting requirements status with a bad user_id
@@ -348,7 +393,7 @@ class CreditServiceTests(ModuleStoreTestCase):
         credit_state = self.service.get_credit_state(self.user.id, unicode(self.course.id))
 
         self.assertIsNotNone(credit_state)
-        self.assertEqual(credit_state['enrollment_mode'], 'honor')
+        self.assertEqual(credit_state['enrollment_mode'], 'verified')
         self.assertEqual(credit_state['profile_fullname'], 'Foo Bar')
         self.assertEqual(len(credit_state['credit_requirement_status']), 1)
         self.assertEqual(credit_state['credit_requirement_status'][0]['name'], 'grade')

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

@@ -10,6 +10,8 @@ from unittest import skipUnless
 from django.conf import settings
 from django.test.client import RequestFactory
 from nose.plugins.attrib import attr
+from course_modes.models import CourseMode
+from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -66,9 +68,12 @@ class TestMinGradedRequirementStatus(ModuleStoreTestCase):
         # Add a single credit requirement (final grade)
         set_credit_requirements(self.course.id, requirements)
 
+        # Enroll user in verified mode.
+        self.enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode=CourseMode.VERIFIED)
+
     def assert_requirement_status(self, grade, due_date, expected_status):
         """ Verify the user's credit requirement status is as expected after simulating a grading calculation. """
-        listen_for_grade_calculation(None, self.user.username, {'percent': grade}, self.course.id, due_date)
+        listen_for_grade_calculation(None, self.user, {'percent': grade}, self.course.id, due_date)
         req_status = get_credit_requirement_status(self.course.id, self.request.user.username, 'grade', 'grade')
 
         self.assertEqual(req_status[0]['status'], expected_status)
@@ -109,3 +114,13 @@ class TestMinGradedRequirementStatus(ModuleStoreTestCase):
     def test_min_grade_requirement_failed_grade_expired_deadline(self):
         """Test with failed grades and deadline expire"""
         self.assert_requirement_status(0.22, self.EXPIRED_DUE_DATE, 'failed')
+
+    @ddt.data(
+        CourseMode.AUDIT,
+        CourseMode.HONOR,
+        CourseMode.CREDIT_MODE
+    )
+    def test_requirement_failed_for_non_verified_enrollment(self, mode):
+        """Test with valid grades submitted before deadline with non-verified enrollment."""
+        self.enrollment.update_enrollment(mode, True)
+        self.assert_requirement_status(0.8, self.VALID_DUE_DATE, None)

openedx/core/djangoapps/signals/signals.py

@@ -6,7 +6,7 @@ from django.dispatch import Signal
 
 
 # Signal that fires when a user is graded (in lms/grades/course_grades.py)
-GRADES_UPDATED = Signal(providing_args=["username", "grade_summary", "course_key", "deadline"])
+GRADES_UPDATED = Signal(providing_args=["user", "grade_summary", "course_key", "deadline"])
 
 # Signal that fires when a user is awarded a certificate in a course (in the certificates django app)
 # TODO: runtime coupling between apps will be reduced if this event is changed to carry a username