From 045e69f3c50f063b683a835594c7ec041f776f96 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Thu, 17 Oct 2013 21:43:19 +0000 Subject: [PATCH 01/11] Can check verified-ness and expiration date --- common/djangoapps/course_modes/models.py | 13 +++- .../course_modes/tests/factories.py | 2 + .../course_modes/tests/test_models.py | 11 +++- common/djangoapps/student/tests/tests.py | 25 ++++++++ common/djangoapps/student/views.py | 59 +++++++++++++++---- lms/djangoapps/courseware/access.py | 24 +++++++- .../courseware/tests/test_access.py | 17 ++++++ lms/djangoapps/shoppingcart/models.py | 28 ++++++++- .../shoppingcart/tests/test_models.py | 25 ++++++++ lms/envs/dev.py | 3 +- lms/templates/dashboard.html | 22 +------ .../dashboard/_dashboard_course_listing.html | 33 ++++++++++- 12 files changed, 220 insertions(+), 42 deletions(-) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 8e475d1ec1..b6849cd07b 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -2,7 +2,7 @@ Add and create new modes for running courses on this particular LMS """ import pytz -from datetime import datetime +from datetime import datetime, date from django.db import models from collections import namedtuple @@ -101,6 +101,17 @@ class CourseMode(models.Model): modes = cls.modes_for_course(course_id) return min(mode.min_price for mode in modes if mode.currency == currency) + @classmethod + def refund_expiration_date(cls, course_id, mode_slug): + """ + Returns the expiration date for verified certificate refunds. After this date, refunds are + no longer possible. Note that this is currently set to be identical to the expiration date for + verified cert signups, but this could be changed in the future + """ + print "TODO fix this" + return date(1990, 1, 1) + #return cls.mode_for_course(course_id,mode_slug).expiration_date + def __unicode__(self): return u"{} : {}, min={}, prices={}".format( self.course_id, self.mode_slug, self.min_price, self.suggested_prices diff --git a/common/djangoapps/course_modes/tests/factories.py b/common/djangoapps/course_modes/tests/factories.py index 3e35b2f05c..72f30bf383 100644 --- a/common/djangoapps/course_modes/tests/factories.py +++ b/common/djangoapps/course_modes/tests/factories.py @@ -1,5 +1,6 @@ from course_modes.models import CourseMode from factory import DjangoModelFactory +import datetime # Factories don't have __init__ methods, and are self documenting # pylint: disable=W0232 @@ -11,3 +12,4 @@ class CourseModeFactory(DjangoModelFactory): mode_display_name = 'audit course' min_price = 0 currency = 'usd' + expiration_date = datetime.date(1990, 1, 1) diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 7a01c30dc4..36b1e72bdd 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -5,7 +5,7 @@ when you run "manage.py test". Replace this with more appropriate tests for your application. """ -from datetime import datetime, timedelta +from datetime import datetime, date, timedelta import pytz from django.test import TestCase @@ -20,6 +20,7 @@ class CourseModeModelTest(TestCase): def setUp(self): self.course_id = 'TestCourse' CourseMode.objects.all().delete() + #todo use different default date def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices='', currency='usd'): """ @@ -31,7 +32,7 @@ class CourseModeModelTest(TestCase): mode_slug=mode_slug, min_price=min_price, suggested_prices=suggested_prices, - currency=currency + currency=currency, ) def test_modes_for_course_empty(self): @@ -112,3 +113,9 @@ class CourseModeModelTest(TestCase): modes = CourseMode.modes_for_course('second_test_course') self.assertEqual([CourseMode.DEFAULT_MODE], modes) + + def test_refund_expiration_date(self): + self.create_mode('verified', 'Verified Certificate') + modes = CourseMode.modes_for_course(self.course_id) + mode = Mode(u'verified', u'Verified Certificate', 0, '', 'usd') + self.assertEqual(CourseMode.refund_expiration_date(self.course_id, 'verified'), date(1990, 1, 1)) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 315b6e9285..ed8237870f 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -422,3 +422,28 @@ class PaidRegistrationTest(ModuleStoreTestCase): self.assertEqual(response.content, reverse('shoppingcart.views.show_cart')) self.assertTrue(shoppingcart.models.PaidCourseRegistration.contained_in_order( shoppingcart.models.Order.get_cart_for_user(self.user), self.course.id)) + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class CertificateItemTest(ModuleStoreTestCase): + """ + Tests for paid certificate functionality (verified student), involves shoppingcart + """ + # test data + COURSE_SLUG = "100" + COURSE_NAME = "test_course" + COURSE_ORG = "EDX" + + def setUp(self): + # Create course + self.req_factory = RequestFactory() + self.course = CourseFactory.create(org=self.COURSE_ORG, display_name=self.COURSE_NAME, number=self.COURSE_SLUG) + self.assertIsNotNone(self.course) + self.user = User.objects.create(username="test", email="test@test.org") + + def test_unenroll_and_refund(self): + request = self.req_factory.post(reverse('change_enrollment'), {'course_id': self.course.id, 'enrollment_action': 'unenroll'}) + request.user = self.user + response = change_enrollment(request) + self.assertEqual(response.status_code, 200) + # add more later; see if this even works diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 75c9b75821..6b30927c46 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -2,6 +2,7 @@ Student Views """ import datetime +from datetime import date import json import logging import random @@ -65,6 +66,7 @@ import external_auth.views from bulk_email.models import Optout, CourseAuthorization import shoppingcart +from shoppingcart.models import (Order, OrderItem, CertificateItem) import track.views @@ -300,6 +302,7 @@ def dashboard(request): # exist (because the course IDs have changed). Still, we don't delete those # enrollments, because it could have been a data push snafu. courses = [] + refund_status = [] for enrollment in CourseEnrollment.enrollments_for_user(user): try: courses.append((course_from_id(enrollment.course_id), enrollment)) @@ -335,15 +338,19 @@ def dashboard(request): CourseAuthorization.instructor_email_enabled(course.id) ) ) + # Verification Attempts verification_status, verification_msg = SoftwareSecurePhotoVerification.user_status(user) + + show_refund_option_for = frozenset(course.id for course, _enrollment in courses + if (has_access(request.user, course, 'refund') and (_enrollment.mode == "verified"))) + # get info w.r.t ExternalAuthMap external_auth_map = None try: external_auth_map = ExternalAuthMap.objects.get(user=user) except ExternalAuthMap.DoesNotExist: pass - context = {'courses': courses, 'course_optouts': course_optouts, 'message': message, @@ -356,6 +363,7 @@ def dashboard(request): 'show_email_settings_for': show_email_settings_for, 'verification_status': verification_status, 'verification_msg': verification_msg, + 'show_refund_option_for': show_refund_option_for, } return render_to_response('dashboard.html', context) @@ -424,6 +432,8 @@ def change_enrollment(request): .format(user.username, course_id)) return HttpResponseBadRequest(_("Course id is invalid")) + course = course_from_id(course_id) + if not has_access(user, course, 'enroll'): return HttpResponseBadRequest(_("Enrollment is closed")) @@ -464,19 +474,42 @@ def change_enrollment(request): elif action == "unenroll": try: - CourseEnrollment.unenroll(user, course_id) + course = course_from_id(course_id) + except ItemNotFoundError: + log.warning("User {0} tried to unenroll from non-existent course {1}" + .format(user.username, course_id)) + return HttpResponseBadRequest(_("Course id is invalid")) - org, course_num, run = course_id.split("/") - dog_stats_api.increment( - "common.student.unenrollment", - tags=["org:{0}".format(org), - "course:{0}".format(course_num), - "run:{0}".format(run)] - ) + course = course_from_id(course_id) + verified = CourseEnrollment.enrollment_mode_for_user(user, course_id) + # did they sign up for verified certs? + if(verified): - return HttpResponse() - except CourseEnrollment.DoesNotExist: - return HttpResponseBadRequest(_("You are not enrolled in this course")) + # If the user is allowed a refund, do so + if has_access(user, course, 'refund'): + subject = _("[Refund] User-Requested Refund") + # todo: make this reference templates/student/refund_email.html + message = "Important info here." + to_email = [settings.PAYMENT_SUPPORT_EMAIL] + from_email = "support@edx.org" + try: + send_mail(subject, message, from_email, to_email, fail_silently=False) + except: + log.warning('Unable to send reimbursement request to billing', exc_info=True) + js['value'] = _('Could not send reimbursement request.') + return HttpResponse(json.dumps(js)) + # email has been sent, let's deal with the order now + CertificateItem.refund_cert(user, course_id) + CourseEnrollment.unenroll(user, course_id) + + org, course_num, run = course_id.split("/") + dog_stats_api.increment( + "common.student.unenrollment", + tags=["org:{0}".format(org), + "course:{0}".format(course_num), + "run:{0}".format(run)] + ) + return HttpResponse() else: return HttpResponseBadRequest(_("Enrollment action is invalid")) @@ -891,7 +924,7 @@ def create_account(request, post_override=None): subject = ''.join(subject.splitlines()) message = render_to_string('emails/activation_email.txt', d) - # dont send email if we are doing load testing or random user generation for some reason + # don't send email if we are doing load testing or random user generation for some reason if not (settings.MITX_FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING')): try: if settings.MITX_FEATURES.get('REROUTE_ACTIVATION_EMAIL'): diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 7836ab8bbc..e8b527766d 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -2,7 +2,7 @@ Ideally, it will be the only place that needs to know about any special settings like DISABLE_START_DATES""" import logging -from datetime import datetime, timedelta +from datetime import datetime, timedelta, date from functools import partial from django.conf import settings @@ -202,11 +202,33 @@ def _has_access_course_desc(user, course, action): return can_enroll() or can_load() + def can_refund(): + """ + For paid/verified certificates, students may receive a refund IFF the deadline + for refunds has not yet passed. Note that this function *only* checks whether + or not that deadline has passed; checking whether the student actually *purchased* + a paid/verified certificate must be done elsewhere. + """ + now = datetime.now(UTC()) + course_start = course.enrollment_start + # If there *is* no start date, user can be refunded + if course_start is None: + return True + # Presently, refunds are only allowed up to two weeks after the course + # start date. + grace_period = timedelta(days=14) + refund_end = course_start + grace_period + if (now.date() <= refund_end.date()): + return True + return False + + checkers = { 'load': can_load, 'load_forum': can_load_forum, 'enroll': can_enroll, 'see_exists': see_exists, + 'refund': can_refund, 'staff': lambda: _has_staff_access_to_descriptor(user, course), 'instructor': lambda: _has_instructor_access_to_descriptor(user, course), } diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index a1cd12ae24..22dd7e25fb 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -106,3 +106,20 @@ class AccessTestCase(TestCase): # TODO: # Non-staff cannot enroll outside the open enrollment period if not specifically allowed + + def test__has_access_refund(self): + u = Mock() + today = datetime.datetime.now(UTC()) + grace_period = datetime.timedelta(days=14) + one_day_extra = datetime.timedelta(days=1) + + # User is allowed to receive refund if it is within two weeks of course start date + c = Mock(enrollment_start=(today-one_day_extra), id='edX/tests/Whenever') + self.assertTrue(access._has_access_course_desc(u, c, 'refund')) + + c = Mock(enrollment_start=(today-grace_period), id='edX/test/Whenever') + self.assertTrue(access._has_access_course_desc(u, c, 'refund')) + + # After two weeks, user may no longer receive a refund + c = Mock(enrollment_start=(today-grace_period-one_day_extra), id='edX/test/Whenever') + self.assertFalse(access._has_access_course_desc(u, c, 'refund')) diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 9ae11f6554..761290d557 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -9,7 +9,7 @@ from boto.exception import BotoServerError # this is a super-class of SESError from django.db import models from django.conf import settings -from django.core.exceptions import ObjectDoesNotExist +from django.core.exceptions import (ObjectDoesNotExist, MultipleObjectsReturned) from django.core.mail import send_mail from django.contrib.auth.models import User from django.utils.translation import ugettext as _ @@ -22,7 +22,6 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from course_modes.models import CourseMode from mitxmako.shortcuts import render_to_string -from student.views import course_from_id from student.models import CourseEnrollment from verify_student.models import SoftwareSecurePhotoVerification @@ -34,13 +33,19 @@ log = logging.getLogger("shoppingcart") ORDER_STATUSES = ( ('cart', 'cart'), ('purchased', 'purchased'), - ('refunded', 'refunded'), # Not used for now + ('refunded', 'refunded'), ) # we need a tuple to represent the primary key of various OrderItem subclasses OrderItemSubclassPK = namedtuple('OrderItemSubclassPK', ['cls', 'pk']) # pylint: disable=C0103 +def course_from_id(course_id): + """Return the CourseDescriptor corresponding to this course_id""" + course_loc = CourseDescriptor.id_to_location(course_id) + return modulestore().get_instance(course_id, course_loc) + + class Order(models.Model): """ This is the model for an order. Before purchase, an Order and its related OrderItems are used @@ -398,6 +403,23 @@ class CertificateItem(OrderItem): course_enrollment = models.ForeignKey(CourseEnrollment) mode = models.SlugField() + @classmethod + def refund_cert(cls, target_user, target_course_id): + try: + target_cert = CertificateItem.objects.get(course_id=target_course_id, user_id=target_user, status='purchased', mode='verified') + target_cert.status = 'refunded' + # todo return success + return target_cert + except MultipleObjectsReturned: + # this seems like a thing that shouldn't happen + log.exception("Multiple entries for single verified cert found") + # but we can recover; select one item and refund it + # todo + except ObjectDoesNotExist: + # todo log properly + log.exception("No certificate found") + # handle the exception + @classmethod @transaction.commit_on_success def add_to_order(cls, order, course_id, cost, mode, currency='usd'): diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 1224bb5093..14d136853a 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -20,6 +20,7 @@ from student.tests.factories import UserFactory from student.models import CourseEnrollment from course_modes.models import CourseMode from shoppingcart.exceptions import PurchasedCallbackException +from django.core.exceptions import (ObjectDoesNotExist, MultipleObjectsReturned) @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) @@ -360,3 +361,27 @@ class CertificateItemTest(ModuleStoreTestCase): cert_item = CertificateItem.add_to_order(cart, self.course_id, self.cost, 'honor') self.assertEquals(cert_item.single_item_receipt_template, 'shoppingcart/receipt.html') + + def test_refund_cert_single_cert(self): + # enroll and buy; dup from test_existing_enrollment + CourseEnrollment.enroll(self.user, self.course_id) + cart = Order.get_cart_for_user(user=self.user) + CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified') + cart.purchase() + enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_id) + # now that it's there, let's try refunding it + order = CertificateItem.refund_cert(target_user=self.user, target_course_id=self.course_id) + self.assertEquals(order.status, 'refunded') + + def test_refund_cert_no_cert_exists(self): + order = CertificateItem.refund_cert(target_user=self.user, target_course_id=self.course_id) + self.assertRaises(ObjectDoesNotExist) + + def test_refund_cert_duplicate_certs_exist(self): + for i in range(0, 2): + CourseEnrollment.enroll(self.user, self.course_id) + cart = Order.get_cart_for_user(user=self.user) + CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified') + cart.purchase() + enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_id) + self.assertRaises(MultipleObjectsReturned) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index e1eadf1331..94362e055f 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -20,6 +20,7 @@ USE_I18N = True TEMPLATE_DEBUG = True +MITX_FEATURES['ENABLE_PAYMENT_FAKE'] = True MITX_FEATURES['DISABLE_START_DATES'] = False MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True MITX_FEATURES['SUBDOMAIN_COURSE_LISTINGS'] = False # Enable to test subdomains--otherwise, want all courses to show up @@ -269,7 +270,7 @@ if SEGMENT_IO_LMS_KEY: CC_PROCESSOR['CyberSource']['SHARED_SECRET'] = os.environ.get('CYBERSOURCE_SHARED_SECRET', '') CC_PROCESSOR['CyberSource']['MERCHANT_ID'] = os.environ.get('CYBERSOURCE_MERCHANT_ID', '') CC_PROCESSOR['CyberSource']['SERIAL_NUMBER'] = os.environ.get('CYBERSOURCE_SERIAL_NUMBER', '') -CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = os.environ.get('CYBERSOURCE_PURCHASE_ENDPOINT', '') +CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = '/shoppingcart/payment_fake/' ########################## USER API ######################## diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index eb657d13e4..3a59ed1ba4 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -190,7 +190,8 @@ <% cert_status = cert_statuses.get(course.id) %> <% show_email_settings = (course.id in show_email_settings_for) %> <% course_mode_info = all_course_modes.get(course.id) %> - <%include file='dashboard/_dashboard_course_listing.html' args="course=course, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info" /> + <% show_refund_option = (course.id in show_refund_option_for) %> + <%include file='dashboard/dashboard_course_listing.html' args="course=course, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option = show_refund_option" /> % endfor @@ -244,26 +245,7 @@ - + + From 63940141c8eda5c432be0f74c51f3d71cd6f174c Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Fri, 25 Oct 2013 22:05:01 +0000 Subject: [PATCH 02/11] End-to-end refunding with tests --- common/djangoapps/course_modes/models.py | 13 +--- .../course_modes/tests/factories.py | 2 - .../course_modes/tests/test_models.py | 9 +-- common/djangoapps/student/tests/tests.py | 10 ++- common/djangoapps/student/views.py | 74 +++++++++---------- lms/djangoapps/courseware/access.py | 2 +- .../courseware/tests/test_access.py | 6 +- lms/djangoapps/shoppingcart/models.py | 23 +++--- .../shoppingcart/tests/test_models.py | 2 - 9 files changed, 62 insertions(+), 79 deletions(-) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index b6849cd07b..8e475d1ec1 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -2,7 +2,7 @@ Add and create new modes for running courses on this particular LMS """ import pytz -from datetime import datetime, date +from datetime import datetime from django.db import models from collections import namedtuple @@ -101,17 +101,6 @@ class CourseMode(models.Model): modes = cls.modes_for_course(course_id) return min(mode.min_price for mode in modes if mode.currency == currency) - @classmethod - def refund_expiration_date(cls, course_id, mode_slug): - """ - Returns the expiration date for verified certificate refunds. After this date, refunds are - no longer possible. Note that this is currently set to be identical to the expiration date for - verified cert signups, but this could be changed in the future - """ - print "TODO fix this" - return date(1990, 1, 1) - #return cls.mode_for_course(course_id,mode_slug).expiration_date - def __unicode__(self): return u"{} : {}, min={}, prices={}".format( self.course_id, self.mode_slug, self.min_price, self.suggested_prices diff --git a/common/djangoapps/course_modes/tests/factories.py b/common/djangoapps/course_modes/tests/factories.py index 72f30bf383..3e35b2f05c 100644 --- a/common/djangoapps/course_modes/tests/factories.py +++ b/common/djangoapps/course_modes/tests/factories.py @@ -1,6 +1,5 @@ from course_modes.models import CourseMode from factory import DjangoModelFactory -import datetime # Factories don't have __init__ methods, and are self documenting # pylint: disable=W0232 @@ -12,4 +11,3 @@ class CourseModeFactory(DjangoModelFactory): mode_display_name = 'audit course' min_price = 0 currency = 'usd' - expiration_date = datetime.date(1990, 1, 1) diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 36b1e72bdd..7f09fbf7cc 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -5,7 +5,7 @@ when you run "manage.py test". Replace this with more appropriate tests for your application. """ -from datetime import datetime, date, timedelta +from datetime import datetime, timedelta import pytz from django.test import TestCase @@ -20,7 +20,6 @@ class CourseModeModelTest(TestCase): def setUp(self): self.course_id = 'TestCourse' CourseMode.objects.all().delete() - #todo use different default date def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices='', currency='usd'): """ @@ -113,9 +112,3 @@ class CourseModeModelTest(TestCase): modes = CourseMode.modes_for_course('second_test_course') self.assertEqual([CourseMode.DEFAULT_MODE], modes) - - def test_refund_expiration_date(self): - self.create_mode('verified', 'Verified Certificate') - modes = CourseMode.modes_for_course(self.course_id) - mode = Mode(u'verified', u'Verified Certificate', 0, '', 'usd') - self.assertEqual(CourseMode.refund_expiration_date(self.course_id, 'verified'), date(1990, 1, 1)) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index ed8237870f..6c802977d0 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -35,6 +35,7 @@ from student.tests.factories import UserFactory, CourseModeFactory from student.tests.test_email import mock_render_to_string import shoppingcart +from shoppingcart.models import CertificateItem COURSE_1 = 'edX/toy/2012_Fall' COURSE_2 = 'edx/full/6.002_Spring_2012' @@ -435,15 +436,20 @@ class CertificateItemTest(ModuleStoreTestCase): COURSE_ORG = "EDX" def setUp(self): - # Create course + # Create course, user, and enroll them as a verified student self.req_factory = RequestFactory() self.course = CourseFactory.create(org=self.COURSE_ORG, display_name=self.COURSE_NAME, number=self.COURSE_SLUG) self.assertIsNotNone(self.course) self.user = User.objects.create(username="test", email="test@test.org") + CourseEnrollment.enroll(self.user, self.course.id, mode='verified') + # Student is verified and paid; we should be able to refund them def test_unenroll_and_refund(self): request = self.req_factory.post(reverse('change_enrollment'), {'course_id': self.course.id, 'enrollment_action': 'unenroll'}) request.user = self.user response = change_enrollment(request) self.assertEqual(response.status_code, 200) - # add more later; see if this even works + self.assertFalse(CourseEnrollment.is_enrolled(self.user,self.course.id)) + target_certs = CertificateItem.objects.filger(course_id=self.course.id, user_id=self.user, status='refunded') + self.assertTrue(target_certs[0].status == 'refunded') + diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 6b30927c46..46ef470fb4 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -2,7 +2,6 @@ Student Views """ import datetime -from datetime import date import json import logging import random @@ -51,6 +50,8 @@ from verify_student.models import SoftwareSecurePhotoVerification from certificates.models import CertificateStatuses, certificate_status_for_student +from shoppingcart.models import CertificateItem + from xmodule.course_module import CourseDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.django import modulestore @@ -66,7 +67,6 @@ import external_auth.views from bulk_email.models import Optout, CourseAuthorization import shoppingcart -from shoppingcart.models import (Order, OrderItem, CertificateItem) import track.views @@ -302,7 +302,6 @@ def dashboard(request): # exist (because the course IDs have changed). Still, we don't delete those # enrollments, because it could have been a data push snafu. courses = [] - refund_status = [] for enrollment in CourseEnrollment.enrollments_for_user(user): try: courses.append((course_from_id(enrollment.course_id), enrollment)) @@ -343,7 +342,7 @@ def dashboard(request): verification_status, verification_msg = SoftwareSecurePhotoVerification.user_status(user) show_refund_option_for = frozenset(course.id for course, _enrollment in courses - if (has_access(request.user, course, 'refund') and (_enrollment.mode == "verified"))) + if (has_access(request.user, course, 'refund') and (_enrollment.mode == "verified"))) # get info w.r.t ExternalAuthMap external_auth_map = None @@ -351,6 +350,7 @@ def dashboard(request): external_auth_map = ExternalAuthMap.objects.get(user=user) except ExternalAuthMap.DoesNotExist: pass + context = {'courses': courses, 'course_optouts': course_optouts, 'message': message, @@ -432,8 +432,6 @@ def change_enrollment(request): .format(user.username, course_id)) return HttpResponseBadRequest(_("Course id is invalid")) - course = course_from_id(course_id) - if not has_access(user, course, 'enroll'): return HttpResponseBadRequest(_("Enrollment is closed")) @@ -475,41 +473,39 @@ def change_enrollment(request): elif action == "unenroll": try: course = course_from_id(course_id) - except ItemNotFoundError: - log.warning("User {0} tried to unenroll from non-existent course {1}" - .format(user.username, course_id)) - return HttpResponseBadRequest(_("Course id is invalid")) + enrollment_mode = CourseEnrollment.enrollment_mode_for_user(user, course_id) - course = course_from_id(course_id) - verified = CourseEnrollment.enrollment_mode_for_user(user, course_id) - # did they sign up for verified certs? - if(verified): - - # If the user is allowed a refund, do so - if has_access(user, course, 'refund'): - subject = _("[Refund] User-Requested Refund") - # todo: make this reference templates/student/refund_email.html - message = "Important info here." - to_email = [settings.PAYMENT_SUPPORT_EMAIL] - from_email = "support@edx.org" - try: - send_mail(subject, message, from_email, to_email, fail_silently=False) - except: - log.warning('Unable to send reimbursement request to billing', exc_info=True) - js['value'] = _('Could not send reimbursement request.') - return HttpResponse(json.dumps(js)) + # did they sign up for verified certs? + if(enrollment_mode=='verified'): + # If the user is allowed a refund, do so + if has_access(user, course, 'refund'): + subject = _("[Refund] User-Requested Refund") + # todo: make this reference templates/student/refund_email.html + message = "Important info here." + to_email = [settings.PAYMENT_SUPPORT_EMAIL] + from_email = "support@edx.org" + try: + send_mail(subject, message, from_email, to_email, fail_silently=False) + except: + log.warning('Unable to send reimbursement request to billing', exc_info=True) + js['value'] = _('Could not send reimbursement request.') + return HttpResponse(json.dumps(js)) # email has been sent, let's deal with the order now CertificateItem.refund_cert(user, course_id) - CourseEnrollment.unenroll(user, course_id) - - org, course_num, run = course_id.split("/") - dog_stats_api.increment( - "common.student.unenrollment", - tags=["org:{0}".format(org), - "course:{0}".format(course_num), - "run:{0}".format(run)] - ) - return HttpResponse() + CourseEnrollment.unenroll(user, course_id) + org, course_num, run = course_id.split("/") + dog_stats_api.increment( + "common.student.unenrollment", + tags=["org:{0}".format(org), + "course:{0}".format(course_num), + "run:{0}".format(run)] + ) + return HttpResponse() + except CourseEnrollment.DoesNotExist: + return HttpResponseBadRequest(_("You are not enrolled in this course")) + except ItemNotFoundError: + log.warning("User {0} tried to unenroll from non-existent course {1}".format(user.username, course_id)) + return HttpResponseBadRequest(_("Course id is invalid")) else: return HttpResponseBadRequest(_("Enrollment action is invalid")) @@ -924,7 +920,7 @@ def create_account(request, post_override=None): subject = ''.join(subject.splitlines()) message = render_to_string('emails/activation_email.txt', d) - # don't send email if we are doing load testing or random user generation for some reason + # dont send email if we are doing load testing or random user generation for some reason if not (settings.MITX_FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING')): try: if settings.MITX_FEATURES.get('REROUTE_ACTIVATION_EMAIL'): diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index e8b527766d..980e86f27d 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -2,7 +2,7 @@ Ideally, it will be the only place that needs to know about any special settings like DISABLE_START_DATES""" import logging -from datetime import datetime, timedelta, date +from datetime import datetime, timedelta from functools import partial from django.conf import settings diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 22dd7e25fb..bd4f6e9841 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -114,12 +114,12 @@ class AccessTestCase(TestCase): one_day_extra = datetime.timedelta(days=1) # User is allowed to receive refund if it is within two weeks of course start date - c = Mock(enrollment_start=(today-one_day_extra), id='edX/tests/Whenever') + c = Mock(enrollment_start=(today - one_day_extra), id='edX/tests/Whenever') self.assertTrue(access._has_access_course_desc(u, c, 'refund')) - c = Mock(enrollment_start=(today-grace_period), id='edX/test/Whenever') + c = Mock(enrollment_start=(today - grace_period), id='edX/test/Whenever') self.assertTrue(access._has_access_course_desc(u, c, 'refund')) # After two weeks, user may no longer receive a refund - c = Mock(enrollment_start=(today-grace_period-one_day_extra), id='edX/test/Whenever') + c = Mock(enrollment_start=(today - grace_period - one_day_extra), id='edX/test/Whenever') self.assertFalse(access._has_access_course_desc(u, c, 'refund')) diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 761290d557..77d6d7898a 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -9,7 +9,7 @@ from boto.exception import BotoServerError # this is a super-class of SESError from django.db import models from django.conf import settings -from django.core.exceptions import (ObjectDoesNotExist, MultipleObjectsReturned) +from django.core.exceptions import ObjectDoesNotExist from django.core.mail import send_mail from django.contrib.auth.models import User from django.utils.translation import ugettext as _ @@ -405,18 +405,21 @@ class CertificateItem(OrderItem): @classmethod def refund_cert(cls, target_user, target_course_id): + """ + When refunded, this should find a verified certificate purchase for target_user in target_course_id, change that + certificate's status to "refunded", save that result, and return the refunded certificate. + + Note the actual mechanics of refunding money occurs elsewhere; this simply changes the relevant certificate's + status for the refund. + """ try: - target_cert = CertificateItem.objects.get(course_id=target_course_id, user_id=target_user, status='purchased', mode='verified') + # If there's duplicate entries, just grab the first one and refund it (though in most cases we should only get one) + target_certs = CertificateItem.objects.filter(course_id=target_course_id, user_id=target_user, status='purchased', mode='verified') + target_cert = target_certs[0] target_cert.status = 'refunded' - # todo return success + target_cert.save() return target_cert - except MultipleObjectsReturned: - # this seems like a thing that shouldn't happen - log.exception("Multiple entries for single verified cert found") - # but we can recover; select one item and refund it - # todo - except ObjectDoesNotExist: - # todo log properly + except IndexError or ObjectDoesNotExist: log.exception("No certificate found") # handle the exception diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 14d136853a..bf96878f13 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -368,7 +368,6 @@ class CertificateItemTest(ModuleStoreTestCase): cart = Order.get_cart_for_user(user=self.user) CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified') cart.purchase() - enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_id) # now that it's there, let's try refunding it order = CertificateItem.refund_cert(target_user=self.user, target_course_id=self.course_id) self.assertEquals(order.status, 'refunded') @@ -383,5 +382,4 @@ class CertificateItemTest(ModuleStoreTestCase): cart = Order.get_cart_for_user(user=self.user) CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified') cart.purchase() - enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_id) self.assertRaises(MultipleObjectsReturned) From fcab46b138dba9e4218de36fd40e3ab010c3611b Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Mon, 28 Oct 2013 15:29:04 +0000 Subject: [PATCH 03/11] Refactored to use signals; full test coverage --- common/djangoapps/student/models.py | 4 +- common/djangoapps/student/tests/tests.py | 40 ++++++++++++----- common/djangoapps/student/views.py | 21 +++------ .../courseware/tests/test_access.py | 14 +++--- lms/djangoapps/shoppingcart/models.py | 43 ++++++++++++------- .../shoppingcart/tests/test_models.py | 22 +++------- lms/envs/dev.py | 3 +- .../dashboard/_dashboard_course_listing.html | 22 ++++------ 8 files changed, 89 insertions(+), 80 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 78e673df71..ae5e10dc75 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -17,7 +17,6 @@ import json import logging import uuid - from django.conf import settings from django.contrib.auth.models import User from django.contrib.auth.signals import user_logged_in, user_logged_out @@ -29,6 +28,9 @@ from django.forms import ModelForm, forms import comment_client as cc from pytz import UTC +import django.dispatch + +verified_unenroll_done = django.dispatch.Signal(providing_args=["user", "user_email", "course_id"]) log = logging.getLogger(__name__) AUDIT_LOG = logging.getLogger("audit") diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 6c802977d0..3366296aa0 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -35,7 +35,8 @@ from student.tests.factories import UserFactory, CourseModeFactory from student.tests.test_email import mock_render_to_string import shoppingcart -from shoppingcart.models import CertificateItem + +from course_modes.models import CourseMode COURSE_1 = 'edX/toy/2012_Fall' COURSE_2 = 'edx/full/6.002_Spring_2012' @@ -426,9 +427,9 @@ class PaidRegistrationTest(ModuleStoreTestCase): @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) -class CertificateItemTest(ModuleStoreTestCase): +class RefundUnenrollmentTests(ModuleStoreTestCase): """ - Tests for paid certificate functionality (verified student), involves shoppingcart + Tests views for unenrollment with refunds """ # test data COURSE_SLUG = "100" @@ -437,19 +438,36 @@ class CertificateItemTest(ModuleStoreTestCase): def setUp(self): # Create course, user, and enroll them as a verified student + self.user = UserFactory.create() + self.course_id = "org/test/Test_Course" + self.cost = 40 + CourseFactory.create(org='org', number='test', run='course', display_name='Test Course') + course_mode = CourseMode(course_id=self.course_id, + mode_slug="honor", + mode_display_name="honor cert", + min_price=self.cost) + course_mode.save() + course_mode = CourseMode(course_id=self.course_id, + mode_slug="verified", + mode_display_name="verified cert", + min_price=self.cost) + course_mode.save() + self.req_factory = RequestFactory() - self.course = CourseFactory.create(org=self.COURSE_ORG, display_name=self.COURSE_NAME, number=self.COURSE_SLUG) - self.assertIsNotNone(self.course) - self.user = User.objects.create(username="test", email="test@test.org") - CourseEnrollment.enroll(self.user, self.course.id, mode='verified') + + course_enrollment = CourseEnrollment.create_enrollment(self.user, self.course_id, 'verified', is_active=True) + course_enrollment.save() # Student is verified and paid; we should be able to refund them def test_unenroll_and_refund(self): - request = self.req_factory.post(reverse('change_enrollment'), {'course_id': self.course.id, 'enrollment_action': 'unenroll'}) + request = self.req_factory.post(reverse('change_enrollment'), {'course_id': self.course_id, 'enrollment_action': 'unenroll'}) request.user = self.user response = change_enrollment(request) self.assertEqual(response.status_code, 200) - self.assertFalse(CourseEnrollment.is_enrolled(self.user,self.course.id)) - target_certs = CertificateItem.objects.filger(course_id=self.course.id, user_id=self.user, status='refunded') - self.assertTrue(target_certs[0].status == 'refunded') + self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course_id)) + def test_unenroll_but_no_course(self): + request = self.req_factory.post(reverse('change_enrollment'), {'course_id': 'non/existent/course', 'enrollment_action': 'unenroll'}) + request.user = self.user + response = change_enrollment(request) + self.assertEqual(response.status_code, 400) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 46ef470fb4..00480a7401 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -43,13 +43,12 @@ from student.models import ( TestCenterRegistration, TestCenterRegistrationForm, PendingNameChange, PendingEmailChange, CourseEnrollment, unique_id_for_user, get_testcenter_registration, CourseEnrollmentAllowed, UserStanding, + verified_unenroll_done ) from student.forms import PasswordResetFormNoActive from verify_student.models import SoftwareSecurePhotoVerification - from certificates.models import CertificateStatuses, certificate_status_for_student - from shoppingcart.models import CertificateItem from xmodule.course_module import CourseDescriptor @@ -75,6 +74,7 @@ from pytz import UTC from util.json_request import JsonResponse + log = logging.getLogger("mitx.student") AUDIT_LOG = logging.getLogger("audit") @@ -476,22 +476,11 @@ def change_enrollment(request): enrollment_mode = CourseEnrollment.enrollment_mode_for_user(user, course_id) # did they sign up for verified certs? - if(enrollment_mode=='verified'): + if(enrollment_mode == 'verified'): # If the user is allowed a refund, do so if has_access(user, course, 'refund'): - subject = _("[Refund] User-Requested Refund") - # todo: make this reference templates/student/refund_email.html - message = "Important info here." - to_email = [settings.PAYMENT_SUPPORT_EMAIL] - from_email = "support@edx.org" - try: - send_mail(subject, message, from_email, to_email, fail_silently=False) - except: - log.warning('Unable to send reimbursement request to billing', exc_info=True) - js['value'] = _('Could not send reimbursement request.') - return HttpResponse(json.dumps(js)) - # email has been sent, let's deal with the order now - CertificateItem.refund_cert(user, course_id) + # triggers the callback to mark the certificate as refunded + verified_unenroll_done.send(sender=request, user=user, user_email=user.email, course_id=course_id) CourseEnrollment.unenroll(user, course_id) org, course_num, run = course_id.split("/") dog_stats_api.increment( diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index bd4f6e9841..97e64eea91 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -108,18 +108,18 @@ class AccessTestCase(TestCase): # Non-staff cannot enroll outside the open enrollment period if not specifically allowed def test__has_access_refund(self): - u = Mock() + user = Mock() today = datetime.datetime.now(UTC()) grace_period = datetime.timedelta(days=14) one_day_extra = datetime.timedelta(days=1) # User is allowed to receive refund if it is within two weeks of course start date - c = Mock(enrollment_start=(today - one_day_extra), id='edX/tests/Whenever') - self.assertTrue(access._has_access_course_desc(u, c, 'refund')) + course = Mock(enrollment_start=(today - one_day_extra), id='edX/tests/Whenever') + self.assertTrue(access._has_access_course_desc(user, course, 'refund')) - c = Mock(enrollment_start=(today - grace_period), id='edX/test/Whenever') - self.assertTrue(access._has_access_course_desc(u, c, 'refund')) + course = Mock(enrollment_start=(today - grace_period), id='edX/test/Whenever') + self.assertTrue(access._has_access_course_desc(user, course, 'refund')) # After two weeks, user may no longer receive a refund - c = Mock(enrollment_start=(today - grace_period - one_day_extra), id='edX/test/Whenever') - self.assertFalse(access._has_access_course_desc(u, c, 'refund')) + course = Mock(enrollment_start=(today - grace_period - one_day_extra), id='edX/test/Whenever') + self.assertFalse(access._has_access_course_desc(user, course, 'refund')) diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 77d6d7898a..834034624e 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -7,6 +7,9 @@ from model_utils.managers import InheritanceManager from collections import namedtuple from boto.exception import BotoServerError # this is a super-class of SESError and catches connection errors +from django.dispatch import receiver +from student.models import verified_unenroll_done + from django.db import models from django.conf import settings from django.core.exceptions import ObjectDoesNotExist @@ -22,7 +25,9 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from course_modes.models import CourseMode from mitxmako.shortcuts import render_to_string +from student.views import course_from_id from student.models import CourseEnrollment + from verify_student.models import SoftwareSecurePhotoVerification from .exceptions import (InvalidCartItem, PurchasedCallbackException, ItemAlreadyInCartException, @@ -40,12 +45,6 @@ ORDER_STATUSES = ( OrderItemSubclassPK = namedtuple('OrderItemSubclassPK', ['cls', 'pk']) # pylint: disable=C0103 -def course_from_id(course_id): - """Return the CourseDescriptor corresponding to this course_id""" - course_loc = CourseDescriptor.id_to_location(course_id) - return modulestore().get_instance(course_id, course_loc) - - class Order(models.Model): """ This is the model for an order. Before purchase, an Order and its related OrderItems are used @@ -404,24 +403,38 @@ class CertificateItem(OrderItem): mode = models.SlugField() @classmethod - def refund_cert(cls, target_user, target_course_id): + @receiver(verified_unenroll_done) + def refund_cert_callback(sender, **kwargs): """ - When refunded, this should find a verified certificate purchase for target_user in target_course_id, change that - certificate's status to "refunded", save that result, and return the refunded certificate. - - Note the actual mechanics of refunding money occurs elsewhere; this simply changes the relevant certificate's - status for the refund. + When a CourseEnrollment object whose mode is 'verified' has its is_active field set to false (i.e. when a student + is unenrolled), this callback ensures that the associated CertificateItem is marked as refunded, and that an + appropriate email is sent to billing. """ try: + course_id = kwargs['course_id'] + user = kwargs['user'] + user_email = kwargs['user_email'] + # If there's duplicate entries, just grab the first one and refund it (though in most cases we should only get one) - target_certs = CertificateItem.objects.filter(course_id=target_course_id, user_id=target_user, status='purchased', mode='verified') + target_certs = CertificateItem.objects.filter(course_id=course_id, user_id=user, status='purchased', mode='verified') target_cert = target_certs[0] target_cert.status = 'refunded' target_cert.save() + + order_number = target_cert.order_id + + # send billing an email so they can handle refunding + subject = _("[Refund] User-Requested Refund") + message = "User " + str(user) + "(" + str(user_email) + ") has requested a refund on Order #" + str(order_number) + "." + to_email = [settings.PAYMENT_SUPPORT_EMAIL] + from_email = "support@edx.org" + send_mail(subject, message, from_email, to_email, fail_silently=False) + return target_cert - except IndexError or ObjectDoesNotExist: + + except IndexError: log.exception("No certificate found") - # handle the exception + raise IndexError @classmethod @transaction.commit_on_success diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index bf96878f13..6891cf140d 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -17,10 +17,9 @@ from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE from shoppingcart.models import (Order, OrderItem, CertificateItem, InvalidCartItem, PaidCourseRegistration, OrderItemSubclassPK) from student.tests.factories import UserFactory -from student.models import CourseEnrollment +from student.models import CourseEnrollment, verified_unenroll_done from course_modes.models import CourseMode from shoppingcart.exceptions import PurchasedCallbackException -from django.core.exceptions import (ObjectDoesNotExist, MultipleObjectsReturned) @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) @@ -362,24 +361,17 @@ class CertificateItemTest(ModuleStoreTestCase): self.assertEquals(cert_item.single_item_receipt_template, 'shoppingcart/receipt.html') - def test_refund_cert_single_cert(self): + def test_refund_cert_callback(self): # enroll and buy; dup from test_existing_enrollment CourseEnrollment.enroll(self.user, self.course_id) cart = Order.get_cart_for_user(user=self.user) CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified') cart.purchase() # now that it's there, let's try refunding it - order = CertificateItem.refund_cert(target_user=self.user, target_course_id=self.course_id) - self.assertEquals(order.status, 'refunded') + verified_unenroll_done.send(sender=self, user=self.user, user_email=self.user.email, course_id=self.course_id) + target_certs = CertificateItem.objects.filter(course_id=self.course_id, user_id=self.user, status='refunded', mode='verified') + self.assertTrue(target_certs[0]) def test_refund_cert_no_cert_exists(self): - order = CertificateItem.refund_cert(target_user=self.user, target_course_id=self.course_id) - self.assertRaises(ObjectDoesNotExist) - - def test_refund_cert_duplicate_certs_exist(self): - for i in range(0, 2): - CourseEnrollment.enroll(self.user, self.course_id) - cart = Order.get_cart_for_user(user=self.user) - CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified') - cart.purchase() - self.assertRaises(MultipleObjectsReturned) + with self.assertRaises(IndexError): + verified_unenroll_done.send(sender=self, user=self.user, user_email=self.user.email, course_id=self.course_id) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 94362e055f..e1eadf1331 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -20,7 +20,6 @@ USE_I18N = True TEMPLATE_DEBUG = True -MITX_FEATURES['ENABLE_PAYMENT_FAKE'] = True MITX_FEATURES['DISABLE_START_DATES'] = False MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True MITX_FEATURES['SUBDOMAIN_COURSE_LISTINGS'] = False # Enable to test subdomains--otherwise, want all courses to show up @@ -270,7 +269,7 @@ if SEGMENT_IO_LMS_KEY: CC_PROCESSOR['CyberSource']['SHARED_SECRET'] = os.environ.get('CYBERSOURCE_SHARED_SECRET', '') CC_PROCESSOR['CyberSource']['MERCHANT_ID'] = os.environ.get('CYBERSOURCE_MERCHANT_ID', '') CC_PROCESSOR['CyberSource']['SERIAL_NUMBER'] = os.environ.get('CYBERSOURCE_SERIAL_NUMBER', '') -CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = '/shoppingcart/payment_fake/' +CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = os.environ.get('CYBERSOURCE_PURCHASE_ENDPOINT', '') ########################## USER API ######################## diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 40dae64cba..13346c9842 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -141,9 +141,13 @@ % endif % endif - ${_('Unregister')} - - + % if enrollment.mode != "verified": + ${_('Unregister')} + % elif show_refund_option: + ${_('Unregister')} + % else: + ${_('Unregister')} + % endif % if show_email_settings: @@ -158,20 +162,12 @@ + + diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index cb0efb33b7..7c68d45e2a 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -158,22 +158,3 @@ - - From e66f9a243b5fad6c0dcc9e95c88f3cbb07d272c3 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Mon, 4 Nov 2013 15:57:57 +0000 Subject: [PATCH 09/11] Clarified wording in a docstring --- lms/djangoapps/courseware/access.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 6c8e7ec8ee..4535ff2616 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -205,10 +205,8 @@ def _has_access_course_desc(user, course, action): def can_refund(): """ - For paid/verified certificates, students may receive a refund IFF the deadline - for refunds has not yet passed. Note that this function *only* checks whether - or not that deadline has passed; checking whether the student actually *purchased* - a paid/verified certificate must be done elsewhere. + For paid/verified certificates, students may receive a refund IFF they have + a verified certificate and the deadline for refunds has not yet passed. """ course_mode = CourseMode.mode_for_course(course.id, 'verified') if course_mode is None: From 1a3f4cb8f8b8bcba7fc54fad4e36c978ed57dc68 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Mon, 4 Nov 2013 17:15:49 +0000 Subject: [PATCH 10/11] Moved can_refund logic to CourseEnrollment --- common/djangoapps/student/models.py | 13 +++++++++ common/djangoapps/student/tests/tests.py | 16 ++++++++++ common/djangoapps/student/views.py | 3 +- lms/djangoapps/courseware/access.py | 14 --------- .../courseware/tests/test_access.py | 29 ------------------- lms/templates/dashboard.html | 2 +- 6 files changed, 31 insertions(+), 46 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 3958dfe48f..794c6ef7b9 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -26,6 +26,7 @@ from django.dispatch import receiver import django.dispatch from django.forms import ModelForm, forms +from course_modes.models import CourseMode import comment_client as cc from pytz import UTC @@ -926,6 +927,18 @@ class CourseEnrollment(models.Model): self.is_active = False self.save() + def refundable(self): + """ + For paid/verified certificates, students may receive a refund IFF they have + a verified certificate and the deadline for refunds has not yet passed. + """ + course_mode = CourseMode.mode_for_course(self.course_id, 'verified') + if course_mode is None: + return False + else: + return True + + class CourseEnrollmentAllowed(models.Model): """ diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 315b6e9285..41a95ff13a 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -256,6 +256,22 @@ class DashboardTest(TestCase): self.assertFalse(course_mode_info['show_upsell']) self.assertIsNone(course_mode_info['days_for_upsell']) + def test_refundable(self): + verified_mode = CourseModeFactory.create( + course_id=self.course.id, + mode_slug='verified', + mode_display_name='Verified', + expiration_date=(datetime.now(pytz.UTC) + timedelta(days=1)).date() + ) + enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode='verified') + + self.assertTrue(enrollment.refundable()) + + verified_mode.expiration_date = (datetime.now(pytz.UTC) - timedelta(days=1)).date() + verified_mode.save() + self.assertFalse(enrollment.refundable()) + + class EnrollInCourseTest(TestCase): """Tests enrolling and unenrolling in courses.""" diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 00a06495a4..a3195e11a4 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -48,7 +48,6 @@ from student.forms import PasswordResetFormNoActive from verify_student.models import SoftwareSecurePhotoVerification from certificates.models import CertificateStatuses, certificate_status_for_student -from shoppingcart.models import CertificateItem from xmodule.course_module import CourseDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError @@ -341,7 +340,7 @@ def dashboard(request): verification_status, verification_msg = SoftwareSecurePhotoVerification.user_status(user) show_refund_option_for = frozenset(course.id for course, _enrollment in courses - if (has_access(request.user, course, 'refund') and (_enrollment.mode == "verified"))) + if _enrollment.refundable()) # get info w.r.t ExternalAuthMap external_auth_map = None diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 4535ff2616..7836ab8bbc 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -14,7 +14,6 @@ from xmodule.modulestore import Location from xmodule.x_module import XModule, XModuleDescriptor from student.models import CourseEnrollmentAllowed -from course_modes.models import CourseMode from external_auth.models import ExternalAuthMap from courseware.masquerade import is_masquerading_as_student from django.utils.timezone import UTC @@ -203,24 +202,11 @@ def _has_access_course_desc(user, course, action): return can_enroll() or can_load() - def can_refund(): - """ - For paid/verified certificates, students may receive a refund IFF they have - a verified certificate and the deadline for refunds has not yet passed. - """ - course_mode = CourseMode.mode_for_course(course.id, 'verified') - if course_mode is None: - return False - else: - return True - - checkers = { 'load': can_load, 'load_forum': can_load_forum, 'enroll': can_enroll, 'see_exists': see_exists, - 'refund': can_refund, 'staff': lambda: _has_staff_access_to_descriptor(user, course), 'instructor': lambda: _has_instructor_access_to_descriptor(user, course), } diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index d05a753f44..bad5f95e0c 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -10,10 +10,6 @@ from .factories import CourseEnrollmentAllowedFactory import datetime import pytz -from student.tests.factories import UserFactory -from xmodule.modulestore.tests.factories import CourseFactory -from course_modes.tests.factories import CourseModeFactory - @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class AccessTestCase(TestCase): @@ -116,28 +112,3 @@ class AccessTestCase(TestCase): # TODO: # Non-staff cannot enroll outside the open enrollment period if not specifically allowed - - def test__has_access_refund(self): - today = datetime.datetime.now(pytz.utc) - one_day_extra = datetime.timedelta(days=1) - user = UserFactory.create() - - course_nonrefundable_id = 'nonrefundable/test/Test_Course' - course_nonrefundable = CourseFactory.create(org='nonrefundable', number='test', run='course', display_name='Test Course') - course_mode_nonrefundable = CourseModeFactory.create(course_id=course_nonrefundable_id, - mode_slug='verified', - expiration_date=(today - one_day_extra)) - course_mode_nonrefundable.save() - - course_refundable_id = 'refundable/test/Test_Course' - course_refundable = CourseFactory.create(org='refundable', number='test', run='course', display_name='Test Course') - course_mode_refundable = CourseModeFactory.create(course_id=course_refundable_id, - mode_slug='verified', - expiration_date=(today + one_day_extra)) - course_mode_refundable.save() - - # User cannot receive a refund one day after the expiration date - self.assertFalse(access._has_access_course_desc(user, course_nonrefundable, 'refund')) - - # After two weeks, user may no longer receive a refund - self.assertTrue(access._has_access_course_desc(user, course_refundable, 'refund')) diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 10cccbc075..2a24912633 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -191,7 +191,7 @@ <% show_email_settings = (course.id in show_email_settings_for) %> <% course_mode_info = all_course_modes.get(course.id) %> <% show_refund_option = (course.id in show_refund_option_for) %> - <%include file='dashboard/dashboard_course_listing.html' args="course=course, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option = show_refund_option" /> + <%include file='dashboard/_dashboard_course_listing.html' args="course=course, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option = show_refund_option" /> % endfor From 448f5cc41fc53ff329300437c7bea7d69718bfd4 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Mon, 4 Nov 2013 20:32:47 +0000 Subject: [PATCH 11/11] Response to CR --- common/djangoapps/student/views.py | 22 +++++++++---------- lms/djangoapps/shoppingcart/models.py | 5 +---- lms/templates/dashboard.html | 6 ++--- .../dashboard/_dashboard_course_listing.html | 2 +- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index a3195e11a4..b6904f2fb7 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -296,13 +296,13 @@ def complete_course_mode_info(course_id, enrollment): def dashboard(request): user = request.user - # Build our courses list for the user, but ignore any courses that no longer - # exist (because the course IDs have changed). Still, we don't delete those + # Build our (course, enorllment) list for the user, but ignore any courses that no + # longer exist (because the course IDs have changed). Still, we don't delete those # enrollments, because it could have been a data push snafu. - courses = [] + course_enrollment_pairs = [] for enrollment in CourseEnrollment.enrollments_for_user(user): try: - courses.append((course_from_id(enrollment.course_id), enrollment)) + course_enrollment_pairs.append((course_from_id(enrollment.course_id), enrollment)) except ItemNotFoundError: log.error("User {0} enrolled in non-existent course {1}" .format(user.username, enrollment.course_id)) @@ -321,15 +321,15 @@ def dashboard(request): staff_access = True errored_courses = modulestore().get_errored_courses() - show_courseware_links_for = frozenset(course.id for course, _enrollment in courses + show_courseware_links_for = frozenset(course.id for course, _enrollment in course_enrollment_pairs if has_access(request.user, course, 'load')) - course_modes = {course.id: complete_course_mode_info(course.id, enrollment) for course, enrollment in courses} - cert_statuses = {course.id: cert_info(request.user, course) for course, _enrollment in courses} + course_modes = {course.id: complete_course_mode_info(course.id, enrollment) for course, enrollment in course_enrollment_pairs} + cert_statuses = {course.id: cert_info(request.user, course) for course, _enrollment in course_enrollment_pairs} # only show email settings for Mongo course and when bulk email is turned on show_email_settings_for = frozenset( - course.id for course, _enrollment in courses if ( + course.id for course, _enrollment in course_enrollment_pairs if ( settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and modulestore().get_modulestore_type(course.id) == MONGO_MODULESTORE_TYPE and CourseAuthorization.instructor_email_enabled(course.id) @@ -339,7 +339,7 @@ def dashboard(request): # Verification Attempts verification_status, verification_msg = SoftwareSecurePhotoVerification.user_status(user) - show_refund_option_for = frozenset(course.id for course, _enrollment in courses + show_refund_option_for = frozenset(course.id for course, _enrollment in course_enrollment_pairs if _enrollment.refundable()) # get info w.r.t ExternalAuthMap @@ -349,7 +349,7 @@ def dashboard(request): except ExternalAuthMap.DoesNotExist: pass - context = {'courses': courses, + context = {'course_enrollment_pairs': course_enrollment_pairs, 'course_optouts': course_optouts, 'message': message, 'external_auth_map': external_auth_map, @@ -1515,4 +1515,4 @@ def change_email_settings(request): log.info(u"User {0} ({1}) opted out of receiving emails from course {2}".format(user.username, user.email, course_id)) track.views.server_track(request, "change-email-settings", {"receive_emails": "no", "course": course_id}, page='dashboard') - return HttpResponse(json.dumps({'success': True})) + return HttpResponse(json.dumps({'success': True})) \ No newline at end of file diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index d9fb051e9a..03be80861a 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -411,10 +411,7 @@ class CertificateItem(OrderItem): """ # Only refund verified cert unenrollments that are within bounds of the expiration date - if course_enrollment.mode != 'verified': - return - - if CourseMode.mode_for_course(course_enrollment.course_id, 'verified') is None: + if not course_enrollment.refundable(): return target_certs = CertificateItem.objects.filter(course_id=course_enrollment.course_id, user_id=course_enrollment.user, status='purchased', mode='verified') diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 2a24912633..9d9600475c 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -183,9 +183,9 @@

${_("Current Courses")}

- % if len(courses) > 0: + % if len(course_enrollment_pairs) > 0:
    - % for course, enrollment in courses: + % for course, enrollment in course_enrollment_pairs: <% show_courseware_link = (course.id in show_courseware_links_for) %> <% cert_status = cert_statuses.get(course.id) %> <% show_email_settings = (course.id in show_email_settings_for) %> @@ -341,4 +341,4 @@ - + \ No newline at end of file diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 7c68d45e2a..30c60fb9f1 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -157,4 +157,4 @@ - + \ No newline at end of file