From 93b1131192259d00d2cfb2515eccc91b98bcaf52 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Fri, 6 May 2016 13:46:34 -0400 Subject: [PATCH 1/5] Revert "Add js test for analytics event on LMS receipt page" This reverts commit f995114d1144c474260fca27752fb6a28fd0bb41. --- lms/static/js/commerce/views/receipt_view.js | 26 +++--- .../fixtures/commerce/checkout_receipt.html | 26 ------ lms/static/js/spec/commerce/receipt_spec.js | 90 ------------------- lms/static/js/spec/main.js | 6 -- 4 files changed, 16 insertions(+), 132 deletions(-) delete mode 100644 lms/static/js/fixtures/commerce/checkout_receipt.html delete mode 100644 lms/static/js/spec/commerce/receipt_spec.js diff --git a/lms/static/js/commerce/views/receipt_view.js b/lms/static/js/commerce/views/receipt_view.js index 8608de3b23..0b8adbcce9 100644 --- a/lms/static/js/commerce/views/receipt_view.js +++ b/lms/static/js/commerce/views/receipt_view.js @@ -3,7 +3,7 @@ */ var edx = edx || {}; -(function ($, _, Backbone) { +(function ($, _, _s, Backbone) { 'use strict'; edx.commerce = edx.commerce || {}; @@ -19,6 +19,11 @@ var edx = edx || {}; this.useEcommerceApi = this.ecommerceBasketId || this.ecommerceOrderNumber; _.bindAll(this, 'renderReceipt', 'renderError', 'getProviderData', 'renderProvider', 'getCourseData'); + /* Mix non-conflicting functions from underscore.string (all but include, contains, and reverse) into + * the Underscore namespace. + */ + _.mixin(_s.exports()); + this.render(); }, @@ -119,16 +124,17 @@ var edx = edx || {}; * @return {object} JQuery Promise. */ getReceiptData: function (orderId) { - var urlFormat = '/shoppingcart/receipt/{orderId}/'; + var urlFormat = '/shoppingcart/receipt/%s/'; if (this.ecommerceOrderNumber) { - urlFormat = '/api/commerce/v1/orders/{orderId}/'; + urlFormat = '/api/commerce/v1/orders/%s/'; } else if (this.ecommerceBasketId){ - urlFormat = '/api/commerce/v0/baskets/{orderId}/order/'; + urlFormat = '/api/commerce/v0/baskets/%s/order/'; } + return $.ajax({ - url: edx.StringUtils.interpolate(urlFormat, {orderId: orderId}), + url: _.sprintf(urlFormat, orderId), type: 'GET', dataType: 'json' }).retry({times: 5, timeout: 2000, statusCodes: [404]}); @@ -139,10 +145,10 @@ var edx = edx || {}; * @return {object} JQuery Promise. */ getProviderData: function (providerId) { - var providerUrl = '/api/credit/v1/providers/{providerId}/'; + var providerUrl = '/api/credit/v1/providers/%s/'; return $.ajax({ - url: edx.StringUtils.interpolate(providerUrl, {providerId: providerId}), + url: _.sprintf(providerUrl, providerId), type: 'GET', dataType: 'json', contentType: 'application/json', @@ -157,9 +163,9 @@ var edx = edx || {}; * @return {object} JQuery Promise. */ getCourseData: function (courseId) { - var courseDetailUrl = '/api/course_structure/v0/courses/{courseId}/'; + var courseDetailUrl = '/api/course_structure/v0/courses/%s/'; return $.ajax({ - url: edx.StringUtils.interpolate(courseDetailUrl, {courseId: courseId}), + url: _.sprintf(courseDetailUrl, courseId), type: 'GET', dataType: 'json' }); @@ -299,7 +305,7 @@ var edx = edx || {}; el: $('#receipt-container') }); -})(jQuery, _, Backbone); // jshint ignore:line +})(jQuery, _, _.str, Backbone); // jshint ignore:line function completeOrder(event) { // jshint ignore:line var courseKey = $(event).data("course-key"), diff --git a/lms/static/js/fixtures/commerce/checkout_receipt.html b/lms/static/js/fixtures/commerce/checkout_receipt.html deleted file mode 100644 index 3ac46d62b9..0000000000 --- a/lms/static/js/fixtures/commerce/checkout_receipt.html +++ /dev/null @@ -1,26 +0,0 @@ - - -
-
- -
-
diff --git a/lms/static/js/spec/commerce/receipt_spec.js b/lms/static/js/spec/commerce/receipt_spec.js deleted file mode 100644 index 79e5b92771..0000000000 --- a/lms/static/js/spec/commerce/receipt_spec.js +++ /dev/null @@ -1,90 +0,0 @@ -define([ - 'js/commerce/views/receipt_view' - ], - function (){ - 'use strict'; - describe("edx.commerce.ReceiptView", function(ReceiptView) { - var view, data = null; - - beforeEach(function(){ - loadFixtures("js/fixtures/commerce/checkout_receipt.html"); - - var receiptFixture = readFixtures("templates/commerce/receipt.underscore"); - appendSetFixtures( - "" - ); - data = { - "status": "Open", - "billed_to": { - "city": "dummy city", - "first_name": "john", - "last_name": "doe", - "country": "AL", - "line2": "line2", - "line1": "line1", - "state": "", - "postcode": "12345" - }, - "lines": [ - { - "status": "Open", - "unit_price_excl_tax": "10.00", - "product": { - "attribute_values": [ - { - "name": "certificate_type", - "value": "verified" - }, - { - "name": "course_key", - "value": "course-v1:edx+dummy+2015_T3" - } - ], - "stockrecords": [ - { - "price_currency": "USD", - "product": 123, - "partner_sku": "1234ABC", - "partner": 1, - "price_excl_tax": "10.00", - "id": 123 - } - ], - "product_class": "Seat", - "title": "Dummy title", - "url": "https://ecom.edx.org/api/v2/products/123/", - "price": "10.00", - "expires": null, - "is_available_to_buy": true, - "id": 123, - "structure": "child" - }, - "line_price_excl_tax": "10.00", - "description": "dummy description", - "title": "dummy title", - "quantity": 1 - } - ], - "number": "EDX-123456", - "date_placed": "2016-01-01T01:01:01Z", - "currency": "USD", - "total_excl_tax": "10.00" - }; - view = new ReceiptView({el: $('#receipt-container')}); - view.renderReceipt(data); - }); - it("sends analytic event when receipt is rendered", function() { - expect(window.analytics.track).toHaveBeenCalledWith( - "Completed Order", - { - orderId: "EDX-123456", - total: "10.00", - currency: "USD" - } - ); - - }); - - }); - } -); diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index ccbedb09b6..34781f79a0 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -95,7 +95,6 @@ 'js/bookmarks/views/bookmarks_list': 'js/bookmarks/views/bookmarks_list', 'js/bookmarks/views/bookmark_button': 'js/bookmarks/views/bookmark_button', 'js/views/message_banner': 'js/views/message_banner', - 'js/commerce/views/receipt_view': 'js/commerce/views/receipt_view', // edxnotes 'annotator_1.2.9': 'xmodule_js/common_static/js/vendor/edxnotes/annotator-full.min', @@ -315,10 +314,6 @@ exports: 'js/ccx/schedule', deps: ['jquery', 'underscore', 'backbone', 'gettext', 'moment'] }, - 'js/commerce/views/receipt_view': { - exports: 'edx.commerce.ReceiptView', - deps: ['jquery', 'backbone', 'underscore', 'string_utils'] - }, // Backbone classes loaded explicitly until they are converted to use RequireJS 'js/instructor_dashboard/ecommerce': { @@ -766,7 +761,6 @@ 'js/spec/learner_dashboard/sidebar_view_spec.js', 'js/spec/learner_dashboard/program_card_view_spec.js', 'js/spec/learner_dashboard/certificate_view_spec.js', - 'js/spec/commerce/receipt_spec.js', 'js/spec/api_admin/catalog_preview_spec.js', ]; From 46775d0f2d999a6fb9032e88427c348c957d97ba Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Thu, 5 May 2016 18:29:46 -0400 Subject: [PATCH 2/5] Exposing CreditRequirement and CreditRequirementStatus in admin ECOM-4379 --- openedx/core/djangoapps/credit/admin.py | 22 +++++++++++++++++++++- openedx/core/djangoapps/credit/models.py | 3 +++ openedx/core/djangoapps/credit/signals.py | 3 +-- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/credit/admin.py b/openedx/core/djangoapps/credit/admin.py index 3fd6d89943..054a399e04 100644 --- a/openedx/core/djangoapps/credit/admin.py +++ b/openedx/core/djangoapps/credit/admin.py @@ -2,8 +2,10 @@ Django admin page for credit eligibility """ from ratelimitbackend import admin + from openedx.core.djangoapps.credit.models import ( - CreditConfig, CreditCourse, CreditProvider, CreditEligibility, CreditRequest + CreditConfig, CreditCourse, CreditProvider, CreditEligibility, CreditRequest, CreditRequirement, + CreditRequirementStatus ) @@ -47,8 +49,26 @@ class CreditRequestAdmin(admin.ModelAdmin): model = CreditRequest +class CreditRequirementAdmin(admin.ModelAdmin): + """ Admin for CreditRequirement. """ + list_display = ('course', 'namespace', 'name', 'display_name', 'active',) + + class Meta(object): + model = CreditRequirement + + +class CreditRequirementStatusAdmin(admin.ModelAdmin): + """ Admin for CreditRequirementStatus. """ + list_display = ('username', 'requirement', 'status',) + + class Meta(object): + model = CreditRequirementStatus + + admin.site.register(CreditCourse, CreditCourseAdmin) admin.site.register(CreditProvider, CreditProviderAdmin) admin.site.register(CreditEligibility, CreditEligibilityAdmin) admin.site.register(CreditRequest, CreditRequestAdmin) admin.site.register(CreditConfig) +admin.site.register(CreditRequirement, CreditRequirementAdmin) +admin.site.register(CreditRequirementStatus, CreditRequirementStatusAdmin) diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index c657e7118b..8bd3071a5e 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/openedx/core/djangoapps/credit/models.py @@ -294,6 +294,9 @@ class CreditRequirement(TimeStampedModel): unique_together = ('namespace', 'name', 'course') ordering = ["order"] + def __unicode__(self): + return self.display_name + @classmethod def add_or_update_course_requirement(cls, credit_course, requirement, order): """ diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index fbec5c9bd8..30f40a0f2f 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -55,8 +55,7 @@ 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 - """Receive 'MIN_GRADE_REQUIREMENT_STATUS' signal and update minimum grade - requirement status. + """Receive 'MIN_GRADE_REQUIREMENT_STATUS' signal and update minimum grade requirement status. Args: sender: None From 17e92f5518f07dcb75e7fea14e3cf734182bcc15 Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Fri, 6 May 2016 02:13:17 -0400 Subject: [PATCH 3/5] Credit test cleanup Removed a lot of duplicate code! ECOM-4379 --- .../core/djangoapps/credit/tests/test_api.py | 30 ++++++++------- .../djangoapps/credit/tests/test_signals.py | 38 +++++++++---------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index d68fe77b6a..02c9be41a2 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -371,7 +371,15 @@ class CreditRequirementApiTests(CreditApiTestBase): eligibilities = api.get_eligibilities_for_user("staff") self.assertEqual(eligibilities, []) + def assert_grade_requirement_status(self, expected_status, expected_order): + """ Assert the status and order of the grade requirement. """ + req_status = api.get_credit_requirement_status(self.course_key, 'staff', namespace="grade", name="grade") + self.assertEqual(req_status[0]["status"], expected_status) + self.assertEqual(req_status[0]["order"], expected_order) + return req_status + def test_set_credit_requirement_status(self): + username = "staff" self.add_credit_course() requirements = [ { @@ -395,40 +403,34 @@ class CreditRequirementApiTests(CreditApiTestBase): self.assertEqual(len(course_requirements), 2) # Initially, the status should be None - req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade") - self.assertEqual(req_status[0]["status"], None) - self.assertEqual(req_status[0]["order"], 0) + self.assert_grade_requirement_status(None, 0) # Set the requirement to "satisfied" and check that it's actually set - api.set_credit_requirement_status("staff", self.course_key, "grade", "grade") - req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade") - self.assertEqual(req_status[0]["status"], "satisfied") - self.assertEqual(req_status[0]["order"], 0) + api.set_credit_requirement_status(username, self.course_key, "grade", "grade") + self.assert_grade_requirement_status('satisfied', 0) # Set the requirement to "failed" and check that it's actually set - api.set_credit_requirement_status("staff", self.course_key, "grade", "grade", status="failed") - req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade") - self.assertEqual(req_status[0]["status"], "failed") - self.assertEqual(req_status[0]["order"], 0) + api.set_credit_requirement_status(username, self.course_key, "grade", "grade", status="failed") + self.assert_grade_requirement_status('failed', 0) req_status = api.get_credit_requirement_status(self.course_key, "staff") self.assertEqual(req_status[0]["status"], "failed") self.assertEqual(req_status[0]["order"], 0) - # make sure the 'order' on the 2nd requiemtn is set correctly (aka 1) + # make sure the 'order' on the 2nd requirement is set correctly (aka 1) self.assertEqual(req_status[1]["status"], None) self.assertEqual(req_status[1]["order"], 1) # Set the requirement to "declined" and check that it's actually set api.set_credit_requirement_status( - "staff", self.course_key, + username, self.course_key, "reverification", "i4x://edX/DemoX/edx-reverification-block/assessment_uuid", status="declined" ) req_status = api.get_credit_requirement_status( self.course_key, - "staff", + username, namespace="reverification", name="i4x://edX/DemoX/edx-reverification-block/assessment_uuid" ) diff --git a/openedx/core/djangoapps/credit/tests/test_signals.py b/openedx/core/djangoapps/credit/tests/test_signals.py index f2480e9d5b..0dc8102b5a 100644 --- a/openedx/core/djangoapps/credit/tests/test_signals.py +++ b/openedx/core/djangoapps/credit/tests/test_signals.py @@ -2,24 +2,23 @@ Tests for minimum grade requirement status """ -import pytz import ddt +import pytz from datetime import timedelta, datetime -from nose.plugins.attrib import attr +from unittest import skipUnless from django.conf import settings from django.test.client import RequestFactory -from unittest import skipUnless +from nose.plugins.attrib import attr +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory from openedx.core.djangoapps.credit.api import ( set_credit_requirements, get_credit_requirement_status ) - from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider from openedx.core.djangoapps.credit.signals import listen_for_grade_calculation -from student.tests.factories import UserFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory @attr('shard_2') @@ -67,20 +66,23 @@ class TestMinGradedRequirementStatus(ModuleStoreTestCase): # Add a single credit requirement (final grade) set_credit_requirements(self.course.id, requirements) + 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) + req_status = get_credit_requirement_status(self.course.id, self.request.user.username, 'grade', 'grade') + self.assertEqual(req_status[0]['status'], expected_status) + @ddt.data( (0.6, VALID_DUE_DATE), (0.52, VALID_DUE_DATE), (0.70, EXPIRED_DUE_DATE), ) @ddt.unpack - def test_min_grade_requirement_with_valid_grade(self, grade_achieved, due_date): + def test_min_grade_requirement_with_valid_grade(self, grade, due_date): """Test with valid grades. Deadline date does not effect in case of valid grade. """ - - listen_for_grade_calculation(None, self.user.username, {'percent': grade_achieved}, 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"], 'satisfied') + self.assert_requirement_status(grade, due_date, 'satisfied') @ddt.data( (0.50, None), @@ -88,16 +90,10 @@ class TestMinGradedRequirementStatus(ModuleStoreTestCase): (0.40, VALID_DUE_DATE), ) @ddt.unpack - def test_min_grade_requirement_failed_grade_valid_deadline(self, grade_achieved, due_date): + def test_min_grade_requirement_failed_grade_valid_deadline(self, grade, due_date): """Test with failed grades and deadline is still open or not defined.""" - - listen_for_grade_calculation(None, self.user.username, {'percent': grade_achieved}, 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"], None) + self.assert_requirement_status(grade, due_date, None) def test_min_grade_requirement_failed_grade_expired_deadline(self): """Test with failed grades and deadline expire""" - - listen_for_grade_calculation(None, self.user.username, {'percent': 0.22}, self.course.id, self.EXPIRED_DUE_DATE) - req_status = get_credit_requirement_status(self.course.id, self.request.user.username, 'grade', 'grade') - self.assertEqual(req_status[0]["status"], 'failed') + self.assert_requirement_status(0.22, self.EXPIRED_DUE_DATE, 'failed') From 78f4af7d6f426a15090cae31dedf2b40d551e15c Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Fri, 6 May 2016 02:16:09 -0400 Subject: [PATCH 4/5] Updated credit requirements logic Credit eligibility can be updated up to the point that the course is closed or the student submits a request for credit. Credit eligibility cannot be removed. ECOM-4379 --- .../core/djangoapps/credit/api/eligibility.py | 33 +++++++------ openedx/core/djangoapps/credit/signals.py | 46 +++++++++++++++---- .../core/djangoapps/credit/tests/test_api.py | 21 +++++++-- .../djangoapps/credit/tests/test_signals.py | 22 +++++++-- 4 files changed, 88 insertions(+), 34 deletions(-) diff --git a/openedx/core/djangoapps/credit/api/eligibility.py b/openedx/core/djangoapps/credit/api/eligibility.py index 9ad0abff9e..e086b3eb74 100644 --- a/openedx/core/djangoapps/credit/api/eligibility.py +++ b/openedx/core/djangoapps/credit/api/eligibility.py @@ -7,13 +7,10 @@ import logging from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements, InvalidCreditCourse from openedx.core.djangoapps.credit.email_utils import send_credit_notifications +from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements, InvalidCreditCourse from openedx.core.djangoapps.credit.models import ( - CreditCourse, - CreditRequirement, - CreditRequirementStatus, - CreditEligibility, + CreditCourse, CreditRequirement, CreditRequirementStatus, CreditEligibility, CreditRequest ) # TODO: Cleanup this mess! ECOM-2908 @@ -228,13 +225,21 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name, ) """ - # Check if we're already eligible for credit. - # If so, short-circuit this process. - if CreditEligibility.is_user_eligible_for_credit(course_key, username): + # Do not allow students who have requested credit to change their eligibility + if CreditRequest.get_user_request_status(username, course_key): log.info( - u'Skipping update of credit requirement with namespace "%s" ' - u'and name "%s" because the user "%s" is already eligible for credit ' - u'in the course "%s".', + 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 + ) + 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) + 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 ) return @@ -273,9 +278,9 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name, 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. - if status == "satisfied": + # 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) if eligibility_record_created and is_eligible: try: diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index 30f40a0f2f..92973ad1f0 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -7,11 +7,10 @@ import logging from django.dispatch import receiver from django.utils import timezone from opaque_keys.edx.keys import CourseKey - -from openedx.core.djangoapps.signals.signals import GRADES_UPDATED -from openedx.core.djangoapps.credit.verification_access import update_verification_partitions from xmodule.modulestore.django import SignalHandler +from openedx.core.djangoapps.credit.verification_access import update_verification_partitions +from openedx.core.djangoapps.signals.signals import GRADES_UPDATED log = logging.getLogger(__name__) @@ -80,12 +79,39 @@ def listen_for_grade_calculation(sender, username, grade_summary, course_key, de criteria = requirements[0].get('criteria') if criteria: min_grade = criteria.get('min_grade') - if grade_summary['percent'] >= min_grade: - reason_dict = {'final_grade': grade_summary['percent']} + passing_grade = grade_summary['percent'] >= min_grade + now = timezone.now() + status = None + reason = None + + if (deadline and now < deadline) or not deadline: + # Student completed coursework on-time + + if passing_grade: + # Student received a passing grade + status = 'satisfied' + reason = {'final_grade': grade_summary['percent']} + else: + # Submission after deadline + + if passing_grade: + # Grade was good, but submission arrived too late + status = 'failed' + reason = { + 'current_date': now, + 'deadline': deadline + } + else: + # Student failed to receive minimum grade + status = 'failed' + reason = { + 'final_grade': grade_summary['percent'], + 'minimum_grade': min_grade + } + + # We do not record a status if the user has not yet earned the minimum grade, but still has + # time to do so. + if status and reason: api.set_credit_requirement_status( - username, course_id, 'grade', 'grade', status="satisfied", reason=reason_dict - ) - elif deadline and deadline < timezone.now(): - api.set_credit_requirement_status( - username, course_id, 'grade', 'grade', status="failed", reason={} + username, course_id, 'grade', 'grade', status=status, reason=reason ) diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index 02c9be41a2..755e95467e 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -33,7 +33,8 @@ from openedx.core.djangoapps.credit.models import ( CreditProvider, CreditRequirement, CreditRequirementStatus, - CreditEligibility + CreditEligibility, + CreditRequest ) from student.tests.factories import UserFactory from util.date_utils import from_timestamp @@ -380,7 +381,7 @@ class CreditRequirementApiTests(CreditApiTestBase): def test_set_credit_requirement_status(self): username = "staff" - self.add_credit_course() + credit_course = self.add_credit_course() requirements = [ { "namespace": "grade", @@ -405,6 +406,16 @@ class CreditRequirementApiTests(CreditApiTestBase): # Initially, the status should be None self.assert_grade_requirement_status(None, 0) + # Requirement statuses cannot be changed if a CreditRequest exists + credit_request = CreditRequest.objects.create( + course=credit_course, + provider=CreditProvider.objects.first(), + username=username, + ) + api.set_credit_requirement_status(username, self.course_key, "grade", "grade") + 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(username, self.course_key, "grade", "grade") self.assert_grade_requirement_status('satisfied', 0) @@ -532,7 +543,7 @@ class CreditRequirementApiTests(CreditApiTestBase): user = UserFactory.create(username=self.USER_INFO['username'], password=self.USER_INFO['password']) # Satisfy one of the requirements, but not the other - with self.assertNumQueries(11): + with self.assertNumQueries(12): api.set_credit_requirement_status( user.username, self.course_key, @@ -544,7 +555,7 @@ class CreditRequirementApiTests(CreditApiTestBase): self.assertFalse(api.is_user_eligible_for_credit("bob", self.course_key)) # Satisfy the other requirement - with self.assertNumQueries(19): + with self.assertNumQueries(20): api.set_credit_requirement_status( "bob", self.course_key, @@ -598,7 +609,7 @@ class CreditRequirementApiTests(CreditApiTestBase): # Delete the eligibility entries and satisfy the user's eligibility # requirement again to trigger eligibility notification CreditEligibility.objects.all().delete() - with self.assertNumQueries(15): + with self.assertNumQueries(16): api.set_credit_requirement_status( "bob", self.course_key, diff --git a/openedx/core/djangoapps/credit/tests/test_signals.py b/openedx/core/djangoapps/credit/tests/test_signals.py index 0dc8102b5a..1121b27eed 100644 --- a/openedx/core/djangoapps/credit/tests/test_signals.py +++ b/openedx/core/djangoapps/credit/tests/test_signals.py @@ -70,20 +70,32 @@ class TestMinGradedRequirementStatus(ModuleStoreTestCase): """ 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) req_status = get_credit_requirement_status(self.course.id, self.request.user.username, 'grade', 'grade') + self.assertEqual(req_status[0]['status'], expected_status) + if expected_status == 'satisfied': + expected_reason = {'final_grade': grade} + self.assertEqual(req_status[0]['reason'], expected_reason) + @ddt.data( (0.6, VALID_DUE_DATE), - (0.52, VALID_DUE_DATE), - (0.70, EXPIRED_DUE_DATE), + (0.52, None), ) @ddt.unpack def test_min_grade_requirement_with_valid_grade(self, grade, due_date): - """Test with valid grades. Deadline date does not effect in case - of valid grade. - """ + """Test with valid grades submitted before deadline""" self.assert_requirement_status(grade, due_date, 'satisfied') + def test_grade_changed(self): + """ Verify successive calls to update a satisfied grade requirement are recorded. """ + self.assert_requirement_status(0.6, self.VALID_DUE_DATE, 'satisfied') + self.assert_requirement_status(0.75, self.VALID_DUE_DATE, 'satisfied') + self.assert_requirement_status(0.70, self.VALID_DUE_DATE, 'satisfied') + + def test_min_grade_requirement_with_valid_grade_and_expired_deadline(self): + """ Verify the status is set to failure if a passing grade is received past the submission deadline. """ + self.assert_requirement_status(0.70, self.EXPIRED_DUE_DATE, 'failed') + @ddt.data( (0.50, None), (0.51, None), From c00355ae514060b4cb11c3818dbe2b94a5a59303 Mon Sep 17 00:00:00 2001 From: Ben Patterson Date: Mon, 9 May 2016 13:26:34 -0400 Subject: [PATCH 5/5] Fix linting issue. --- common/test/acceptance/tests/lms/test_lms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index a89fd9773b..4f7c2aaa2f 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -9,7 +9,6 @@ from unittest import skip from nose.plugins.attrib import attr import pytz import urllib -from ..helpers import skip_if_browser from bok_choy.promise import EmptyPromise from ..helpers import (