From 63940141c8eda5c432be0f74c51f3d71cd6f174c Mon Sep 17 00:00:00 2001
From: Julia Hansbrough <julia@edx.org>
Date: Fri, 25 Oct 2013 22:05:01 +0000
Subject: [PATCH] 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 | 76 +++++++++----------
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, 63 insertions(+), 80 deletions(-)
diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py
index b6849cd07bd..8e475d1ec1c 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 72f30bf3832..3e35b2f05c4 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 36b1e72bdd0..7f09fbf7ccd 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 ed8237870ff..6c802977d09 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 6b30927c462..46ef470fb40 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"))
-
- 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))
+ enrollment_mode = CourseEnrollment.enrollment_mode_for_user(user, course_id)
+
+ # 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 e8b527766d0..980e86f27dc 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 22dd7e25fb6..bd4f6e98410 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 761290d5574..77d6d7898a3 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 14d136853aa..bf96878f134 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)
--
GitLab