From 570942a96db69246664fa1d6f56e37f4b7def04e Mon Sep 17 00:00:00 2001 From: noraiz-anwar Date: Tue, 21 Mar 2017 13:57:59 +0500 Subject: [PATCH] Fix wrongly failed requirment to earn credit --- openedx/core/djangoapps/credit/models.py | 9 ++- .../core/djangoapps/credit/tests/test_api.py | 73 +++++++++++++++---- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index 0f6695c038..67cfb4ee72 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/openedx/core/djangoapps/credit/models.py @@ -465,8 +465,15 @@ class CreditRequirementStatus(TimeStampedModel): defaults={"reason": reason, "status": status} ) if not created: + # do not update status to `failed` if user has `satisfied` the requirement + if status == 'failed' and requirement_status.status == 'satisfied': + log.info( + u'Can not change status of credit requirement "%s" from satisfied to failed ', + requirement_status.requirement_id + ) + return requirement_status.status = status - requirement_status.reason = reason if reason else {} + requirement_status.reason = reason requirement_status.save() @classmethod diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index 95c3fe51a2..af71589bef 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -435,18 +435,14 @@ class CreditRequirementApiTests(CreditApiTestBase): self.assertEqual(req_status[0]["order"], expected_order) return req_status - @ddt.data( - *CourseMode.CREDIT_ELIGIBLE_MODES - ) - def test_set_credit_requirement_status(self, mode): - username = self.user.username - credit_course = self.add_credit_course() + def _set_credit_course_requirements(self): - # Enroll user and verify his enrollment. - self.enroll(self.user, self.course_key, mode) - self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key)) - self.assertTrue(CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key), (mode, True)) + """ + Sets requirements for the credit course. + Returns: + dict: Course requirements + """ requirements = [ { "namespace": "grade", @@ -467,6 +463,23 @@ class CreditRequirementApiTests(CreditApiTestBase): course_requirements = api.get_credit_requirements(self.course_key) self.assertEqual(len(course_requirements), 2) + @ddt.data( + *CourseMode.CREDIT_ELIGIBLE_MODES + ) + def test_set_credit_requirement_status(self, mode): + """ + Test set/update credit requirement status + """ + username = self.user.username + credit_course = self.add_credit_course() + + # Enroll user and verify his enrollment. + self.enroll(self.user, self.course_key, mode) + self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key)) + self.assertTrue(CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key), (mode, True)) + + self._set_credit_course_requirements() + # Initially, the status should be None self.assert_grade_requirement_status(None, 0) @@ -480,16 +493,20 @@ class CreditRequirementApiTests(CreditApiTestBase): self.assert_grade_requirement_status(None, 0) credit_request.delete() - # Set the requirement to "satisfied" and check that it's actually set - api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade") - self.assert_grade_requirement_status('satisfied', 0) + # order of below two assertions matter as: + # `failed` to `satisfied` is allowed + # `satisfied` to `failed` is not allowed - # Set the requirement to "failed" and check that it's actually set + # 1. Set the requirement to "failed" and check that it's actually set api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade", status="failed") self.assert_grade_requirement_status('failed', 0) + # 2. Set the requirement to "satisfied" and check that it's actually set + api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade") + self.assert_grade_requirement_status('satisfied', 0) + req_status = api.get_credit_requirement_status(self.course_key, username) - self.assertEqual(req_status[0]["status"], "failed") + self.assertEqual(req_status[0]["status"], "satisfied") self.assertEqual(req_status[0]["order"], 0) # make sure the 'order' on the 2nd requirement is set correctly (aka 1) @@ -511,6 +528,32 @@ class CreditRequirementApiTests(CreditApiTestBase): ) self.assertEqual(req_status[0]["status"], "declined") + @ddt.data( + *CourseMode.CREDIT_ELIGIBLE_MODES + ) + def test_set_credit_requirement_status_satisfied_to_failed(self, mode): + """ + Test that if credit requirment status is set to `satisfied`, it + can not not be changed to `failed` + """ + self.add_credit_course() + + # Enroll user and verify enrollment. + self.enroll(self.user, self.course_key, mode) + self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key)) + self.assertTrue(CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key), (mode, True)) + + self._set_credit_course_requirements() + + api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade", status="satisfied") + self.assert_grade_requirement_status('satisfied', 0) + + # try to set status to `failed` + api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade", status="failed") + + # status should not be changed to `failed`, rather should maintain already set status `satisfied` + self.assert_grade_requirement_status('satisfied', 0) + @ddt.data( *CourseMode.CREDIT_ELIGIBLE_MODES )