CR response

common/djangoapps/course_modes/models.py

@@ -96,6 +96,14 @@ class CourseMode(models.Model):
             return None
 
     @classmethod
+    def min_course_price_for_verified_for_currency(cls, course_id, currency):
+        modes = cls.modes_for_course(course_id)
+        for mode in modes:
+            if (mode.currency == currency) and (mode.slug == 'verified'):
+                return mode.min_price
+        return 0
+
+    @classmethod
     def min_course_price_for_currency(cls, course_id, currency):
         """
         Returns the minimum price of the course in the appropriate currency over all the course's

common/djangoapps/student/models.py

@@ -17,11 +17,13 @@ import json
 import logging
 from pytz import UTC
 import uuid
+from collections import defaultdict
 
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.contrib.auth.signals import user_logged_in, user_logged_out
 from django.db import models, IntegrityError
+from django.db.models import Sum, Count
 from django.db.models.signals import post_save
 from django.dispatch import receiver, Signal
 import django.dispatch
@@ -585,11 +587,14 @@ class CourseEnrollment(models.Model):
         Returns a dictionary that stores the total enrollment count for a course, as well as the
         enrollment count for each individual mode.
         """
-        d = {}
-        d['total'] = cls.objects.filter(course_id=course_id, is_active=True).count()
-        d['honor'] = cls.objects.filter(course_id=course_id, is_active=True, mode='honor').count()
-        d['audit'] = cls.objects.filter(course_id=course_id, is_active=True, mode='audit').count()
-        d['verified'] = cls.objects.filter(course_id=course_id, is_active=True, mode='verified').count()
+        # Unfortunately, Django's "group by"-style queries look super-awkward
+        query = cls.objects.filter(course_id=course_id, is_active=True).values('mode').order_by().annotate(Count('mode'))
+        total = 0
+        d = defaultdict(int)
+        for item in query:
+            d[item['mode']] = item['mode__count']
+            total += item['mode__count']
+        d['total'] = total
         return d
 
     def activate(self):

lms/djangoapps/shoppingcart/models.py

@@ -587,8 +587,17 @@ class CertificateItem(OrderItem):
         etc
         """
         query = use_read_replica_if_available(
-            CertificateItem.objects.filter(course_id=course_id, mode='verified', status='purchased').aggregate(Sum(field_to_aggregate)))[field_to_aggregate + '__sum']
+            CertificateItem.objects.filter(course_id=course_id, mode='verified', status=status).aggregate(Sum(field_to_aggregate)))[field_to_aggregate + '__sum']
         if query is None:
             return Decimal(0.00)
         else:
             return query
+
+    @classmethod
+    def verified_certificates_contributing_more_than_minimum(cls, course_id):
+        return use_read_replica_if_available(
+            CertificateItem.objects.filter(
+                course_id=course_id,
+                mode='verified',
+                status='purchased',
+                unit_cost__gt=(CourseMode.min_course_price_for_verified_for_currency(course_id, 'usd'))).count())

lms/djangoapps/shoppingcart/reports.py

 from decimal import Decimal
 import unicodecsv
+import logging
 
 from django.db import models
 from django.conf import settings
+from django.utils.translation import ugettext as _
 
 from courseware.courses import get_course_by_id
 from course_modes.models import CourseMode
 from shoppingcart.models import CertificateItem, OrderItem
 from student.models import CourseEnrollment
 from util.query import use_read_replica_if_available
+from xmodule.error_module import ErrorDescriptor
+from xmodule.modulestore.django import modulestore
 
 
 class Report(object):
@@ -18,8 +22,13 @@ class Report(object):
     To make a different type of report, write a new subclass that implements
     the methods rows and header.
     """
+    def __init__(self, start_date, end_date, start_word=None, end_word=None):
+        self.start_date = start_date
+        self.end_date = end_date
+        self.start_word = start_word
+        self.end_word = end_word
 
-    def rows(self, start_date, end_date, start_word=None, end_word=None):
+    def rows(self):
         """
         Performs database queries necessary for the report and eturns an generator of
         lists, in which each list is a separate row of the report.
@@ -36,12 +45,12 @@ class Report(object):
         """
         raise NotImplementedError
 
-    def write_csv(self, filelike, start_date, end_date, start_word=None, end_word=None):
+    def write_csv(self, filelike):
         """
         Given a file object to write to and {start/end date, start/end letter} bounds,
         generates a CSV report of the appropriate type.
         """
-        items = self.rows(start_date, end_date, start_word, end_word)
+        items = self.rows()
         writer = unicodecsv.writer(filelike, encoding="utf-8")
         writer.writerow(self.header())
         for item in items:
@@ -56,13 +65,20 @@ class RefundReport(Report):
     order number, customer name, date of transaction, date of refund, and any service
     fees.
     """
-    def rows(self, start_date, end_date, start_word=None, end_word=None):
-        query = use_read_replica_if_available(
+    def rows(self):
+        query1 = use_read_replica_if_available(
             CertificateItem.objects.select_related('user__profile').filter(
                 status="refunded",
-                refund_requested_time__gte=start_date,
-                refund_requested_time__lt=end_date,
+                refund_requested_time__gte=self.start_date,
+                refund_requested_time__lt=self.end_date,
             ).order_by('refund_requested_time'))
+        query2 = use_read_replica_if_available(
+            CertificateItem.objects.select_related('user__profile').filter(
+                status="refunded",
+                refund_requested_time=None,
+            ))
+
+        query = query1 | query2
 
         for item in query:
             yield [
@@ -76,12 +92,12 @@ class RefundReport(Report):
 
     def header(self):
         return [
-            "Order Number",
-            "Customer Name",
-            "Date of Original Transaction",
-            "Date of Refund",
-            "Amount of Refund",
-            "Service Fees (if any)",
+            _("Order Number"),
+            _("Customer Name"),
+            _("Date of Original Transaction"),
+            _("Date of Refund"),
+            _("Amount of Refund"),
+            _("Service Fees (if any)"),
         ]
 
 
@@ -93,12 +109,12 @@ class ItemizedPurchaseReport(Report):
     a given start_date and end_date, we find that purchase's time, order ID, status,
     quantity, unit cost, total cost, currency, description, and related comments.
     """
-    def rows(self, start_date, end_date, start_word=None, end_word=None):
+    def rows(self):
         query = use_read_replica_if_available(
             OrderItem.objects.filter(
                 status="purchased",
-                fulfilled_time__gte=start_date,
-                fulfilled_time__lt=end_date,
+                fulfilled_time__gte=self.start_date,
+                fulfilled_time__lt=self.end_date,
             ).order_by("fulfilled_time"))
 
         for item in query:
@@ -116,15 +132,15 @@ class ItemizedPurchaseReport(Report):
 
     def header(self):
         return [
-            "Purchase Time",
-            "Order ID",
-            "Status",
-            "Quantity",
-            "Unit Cost",
-            "Total Cost",
-            "Currency",
-            "Description",
-            "Comments"
+            _("Purchase Time"),
+            _("Order ID"),
+            _("Status"),
+            _("Quantity"),
+            _("Unit Cost"),
+            _("Total Cost"),
+            _("Currency"),
+            _("Description"),
+            _("Comments")
         ]
 
 
@@ -137,9 +153,8 @@ class CertificateStatusReport(Report):
     calculate the total enrollment, audit enrollment, honor enrollment, verified enrollment, total
     gross revenue, gross revenue over the minimum, and total dollars refunded.
     """
-    def rows(self, start_date, end_date, start_word=None, end_word=None):
-        results = []
-        for course_id in course_ids_between(start_word, end_word):
+    def rows(self):
+        for course_id in course_ids_between(self.start_word, self.end_word):
             # If the first letter of the university is between start_word and end_word, then we include
             # it in the report.  These comparisons are unicode-safe.
             cur_course = get_course_by_id(course_id)
@@ -157,7 +172,9 @@ class CertificateStatusReport(Report):
             else:
                 verified_enrolled = counts['verified']
                 gross_rev = CertificateItem.verified_certificates_monetary_field_sum(course_id, 'purchased', 'unit_cost')
-                gross_rev_over_min = gross_rev - (CourseMode.min_course_price_for_currency(course_id, 'usd') * verified_enrolled)
+                gross_rev_over_min = gross_rev - (CourseMode.min_course_price_for_verified_for_currency(course_id, 'usd') * verified_enrolled)
+
+            num_verified_over_the_minimum = CertificateItem.verified_certificates_contributing_more_than_minimum(course_id)
 
             # should I be worried about is_active here?
             number_of_refunds = CertificateItem.verified_certificates_count(course_id, 'refunded')
@@ -166,32 +183,46 @@ class CertificateStatusReport(Report):
             else:
                 dollars_refunded = CertificateItem.verified_certificates_monetary_field_sum(course_id, 'refunded', 'unit_cost')
 
-            result = [
+            course_announce_date = ""
+            course_reg_start_date = ""
+            course_reg_close_date = ""
+            registration_period = ""
+
+            yield [
                 university,
                 course,
+                "",
+                "",
+                "",
+                "",
                 total_enrolled,
                 audit_enrolled,
                 honor_enrolled,
                 verified_enrolled,
                 gross_rev,
                 gross_rev_over_min,
+                num_verified_over_the_minimum,
                 number_of_refunds,
                 dollars_refunded
             ]
-            yield result
 
     def header(self):
         return [
-            "University",
-            "Course",
-            "Total Enrolled",
-            "Audit Enrollment",
-            "Honor Code Enrollment",
-            "Verified Enrollment",
-            "Gross Revenue",
-            "Gross Revenue over the Minimum",
-            "Number of Refunds",
-            "Dollars Refunded",
+            _("University"),
+            _("Course"),
+            _("Course Announce Date"),
+            _("Course Start Date"),
+            _("Course Registration Close Date"),
+            _("Course Registration Period"),
+            _("Total Enrolled"),
+            _("Audit Enrollment"),
+            _("Honor Code Enrollment"),
+            _("Verified Enrollment"),
+            _("Gross Revenue"),
+            _("Gross Revenue over the Minimum"),
+            _("Number of Verified Students Contributing More than the Minimum"),
+            _("Number of Refunds"),
+            _("Dollars Refunded"),
         ]
 
 
@@ -204,19 +235,18 @@ class UniversityRevenueShareReport(Report):
     the total revenue generated by that particular course.  This includes the number of transactions,
     total payments collected, service fees, number of refunds, and total amount of refunds.
     """
-    def rows(self, start_date, end_date, start_word=None, end_word=None):
-        results = []
-        for course_id in course_ids_between(start_word, end_word):
+    def rows(self):
+        for course_id in course_ids_between(self.start_word, self.end_word):
             cur_course = get_course_by_id(course_id)
             university = cur_course.org
             course = cur_course.number + " " + cur_course.display_name_with_default
-            num_transactions = 0  # TODO clarify with billing what transactions are included in this (purchases? refunds? etc)
             total_payments_collected = CertificateItem.verified_certificates_monetary_field_sum(course_id, 'purchased', 'unit_cost')
             service_fees = CertificateItem.verified_certificates_monetary_field_sum(course_id, 'purchased', 'service_fee')
             num_refunds = CertificateItem.verified_certificates_count(course_id, "refunded")
             amount_refunds = CertificateItem.verified_certificates_monetary_field_sum(course_id, 'refunded', 'unit_cost')
+            num_transactions = (num_refunds * 2) + CertificateItem.verified_certificates_count(course_id, "purchased")
 
-            result = [
+            yield [
                 university,
                 course,
                 num_transactions,
@@ -225,17 +255,16 @@ class UniversityRevenueShareReport(Report):
                 num_refunds,
                 amount_refunds
             ]
-            yield result
 
     def header(self):
         return [
-            "University",
-            "Course",
-            "Number of Transactions",
-            "Total Payments Collected",
-            "Service Fees (if any)",
-            "Number of Successful Refunds",
-            "Total Amount of Refunds",
+            _("University"),
+            _("Course"),
+            _("Number of Transactions"),
+            _("Total Payments Collected"),
+            _("Service Fees (if any)"),
+            _("Number of Successful Refunds"),
+            _("Total Amount of Refunds"),
         ]
 
 def course_ids_between(start_word, end_word):
@@ -243,8 +272,9 @@ def course_ids_between(start_word, end_word):
     Returns a list of all valid course_ids that fall alphabetically between start_word and end_word.
     These comparisons are unicode-safe. 
     """
+
     valid_courses = []
-    for course_id in settings.COURSE_LISTINGS['default']:
-        if (start_word.lower() <= course_id.lower()) and (end_word.lower() >= course_id.lower()) and (get_course_by_id(course_id) is not None):
-            valid_courses.append(course_id)
+    for course in modulestore().get_courses():
+        if (start_word.lower() <= course.id.lower() <= course.id.lower()) and (get_course_by_id(course.id) is not None):
+            valid_courses.append(course.id)
     return valid_courses

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -373,8 +373,8 @@ class ItemizedPurchaseReportTest(ModuleStoreTestCase):
             """.format(time_str=str(self.now)))
 
     def test_purchased_items_btw_dates(self):
-        report = initialize_report("itemized_purchase_report")
-        purchases = report.rows(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
+        report = initialize_report("itemized_purchase_report", self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
+        purchases = report.rows()
 
         # since there's not many purchases, just run through the generator to make sure we've got the right number
         num_purchases = 0
@@ -384,7 +384,8 @@ class ItemizedPurchaseReportTest(ModuleStoreTestCase):
         #self.assertIn(self.reg.orderitem_ptr, purchases)
         #self.assertIn(self.cert_item.orderitem_ptr, purchases)
 
-        no_purchases = report.rows(self.now + self.FIVE_MINS, self.now + self.FIVE_MINS + self.FIVE_MINS)
+        report = initialize_report("itemized_purchase_report", self.now + self.FIVE_MINS, self.now + self.FIVE_MINS + self.FIVE_MINS)
+        no_purchases = report.rows()
 
         num_purchases = 0
         for item in no_purchases:
@@ -395,9 +396,9 @@ class ItemizedPurchaseReportTest(ModuleStoreTestCase):
         """
         Tests that a generated purchase report CSV is as we expect
         """
-        report = initialize_report("itemized_purchase_report")
+        report = initialize_report("itemized_purchase_report", self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
         csv_file = StringIO.StringIO()
-        report.write_csv(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
+        report.write_csv(csv_file)
         csv = csv_file.getvalue()
         csv_file.close()
         # Using excel mode csv, which automatically ends lines with \r\n, so need to convert to \n

lms/djangoapps/shoppingcart/tests/test_reports.py

+# -*- coding: utf-8 -*-
+
 """
 Tests for the Shopping Cart Models
 """
@@ -128,18 +130,18 @@ class ReportTypeTests(ModuleStoreTestCase):
             """.format(time_str=str(self.test_time)))
 
         self.CORRECT_CERT_STATUS_CSV = dedent("""
-            University,Course,Total Enrolled,Audit Enrollment,Honor Code Enrollment,Verified Enrollment,Gross Revenue,Gross Revenue over the Minimum,Number of Refunds,Dollars Refunded
-            MITx,999 Robot Super Course,6,3,1,2,80.00,0.00,2,80.00
+            University,Course,Course Announce Date,Course Start Date,Course Registration Close Date,Course Registration Period,Total Enrolled,Audit Enrollment,Honor Code Enrollment,Verified Enrollment,Gross Revenue,Gross Revenue over the Minimum,Number of Verified Students Contributing More than the Minimum,Number of Refunds,Dollars Refunded
+            MITx,999 Robot Super Course,,,,,6,3,1,2,80.00,0.00,0,2,80.00
             """.format(time_str=str(self.test_time)))
 
         self.CORRECT_UNI_REVENUE_SHARE_CSV = dedent("""
             University,Course,Number of Transactions,Total Payments Collected,Service Fees (if any),Number of Successful Refunds,Total Amount of Refunds
-            MITx,999 Robot Super Course,0,80.00,0.00,2,80.00
+            MITx,999 Robot Super Course,6,80.00,0.00,2,80.00
             """.format(time_str=str(self.test_time)))
 
     def test_refund_report_rows(self):
-        report = initialize_report("refund_report")
-        refunded_certs = report.rows(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
+        report = initialize_report("refund_report", self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
+        refunded_certs = report.rows()
 
         # check that we have the right number
         num_certs = 0
@@ -154,24 +156,24 @@ class ReportTypeTests(ModuleStoreTestCase):
         """
         Tests that a generated purchase report CSV is as we expect
         """
-        report = initialize_report("refund_report")
+        report = initialize_report("refund_report", self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
         csv_file = StringIO.StringIO()
-        report.write_csv(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS)
+        report.write_csv(csv_file)
         csv = csv_file.getvalue()
         csv_file.close()
         # Using excel mode csv, which automatically ends lines with \r\n, so need to convert to \n
         self.assertEqual(csv.replace('\r\n', '\n').strip(), self.CORRECT_REFUND_REPORT_CSV.strip())
 
     def test_basic_cert_status_csv(self):
-        report = initialize_report("certificate_status")
+        report = initialize_report("certificate_status", self.now - self.FIVE_MINS, self.now + self.FIVE_MINS, 'A', 'Z')
         csv_file = StringIO.StringIO()
-        report.write_csv(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS, 'A', 'Z')
+        report.write_csv(csv_file)
         csv = csv_file.getvalue()
         self.assertEqual(csv.replace('\r\n', '\n').strip(), self.CORRECT_CERT_STATUS_CSV.strip())
 
     def test_basic_uni_revenue_share_csv(self):
-        report = initialize_report("university_revenue_share")
+        report = initialize_report("university_revenue_share", self.now - self.FIVE_MINS, self.now + self.FIVE_MINS, 'A', 'Z')
         csv_file = StringIO.StringIO()
-        report.write_csv(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS, 'A', 'Z')
+        report.write_csv(csv_file)
         csv = csv_file.getvalue()
         self.assertEqual(csv.replace('\r\n', '\n').strip(), self.CORRECT_UNI_REVENUE_SHARE_CSV.strip())

lms/djangoapps/shoppingcart/tests/test_views.py

@@ -369,29 +369,35 @@ class CSVReportViewsTest(ModuleStoreTestCase):
 
     def test_report_csv_itemized(self):
         report_type = 'itemized_purchase_report'
+        start_date = '1970-01-01'
+        end_date = '2100-01-01'
         PaidCourseRegistration.add_to_order(self.cart, self.course_id)
         self.cart.purchase()
         self.login_user()
         self.add_to_download_group(self.user)
-        response = self.client.post(reverse('payment_csv_report'), {'start_date': '1970-01-01',
-                                                                    'end_date': '2100-01-01',
+        response = self.client.post(reverse('payment_csv_report'), {'start_date': start_date,
+                                                                    'end_date': end_date,
                                                                     'requested_report': report_type})
         self.assertEqual(response['Content-Type'], 'text/csv')
-        report = initialize_report(report_type)
+        report = initialize_report(report_type, start_date, end_date)
         self.assertIn(",".join(report.header()), response.content)
         self.assertIn(self.CORRECT_CSV_NO_DATE_ITEMIZED_PURCHASE, response.content)
 
     def test_report_csv_university_revenue_share(self):
         report_type = 'university_revenue_share'
+        start_date = '1970-01-01'
+        end_date = '2100-01-01'
+        start_letter = 'A'
+        end_letter = 'Z'
         self.login_user()
         self.add_to_download_group(self.user)
-        response = self.client.post(reverse('payment_csv_report'), {'start_date': '1970-01-01',
-                                                                    'end_date': '2100-01-01',
-                                                                    'start_letter': 'A',
-                                                                    'end_letter': 'Z',
+        response = self.client.post(reverse('payment_csv_report'), {'start_date': start_date,
+                                                                    'end_date': end_date,
+                                                                    'start_letter': start_letter,
+                                                                    'end_letter': end_letter,
                                                                     'requested_report': report_type})
         self.assertEqual(response['Content-Type'], 'text/csv')
-        report = initialize_report(report_type)
+        report = initialize_report(report_type, start_date, end_date, start_letter, end_letter)
         self.assertIn(",".join(report.header()), response.content)
         # TODO add another test here
 

lms/djangoapps/shoppingcart/urls.py

@@ -4,6 +4,7 @@ from django.conf import settings
 urlpatterns = patterns('shoppingcart.views',  # nopep8
     url(r'^postpay_callback/$', 'postpay_callback'),  # Both the ~accept and ~reject callback pages are handled here
     url(r'^receipt/(?P<ordernum>[0-9]*)/$', 'show_receipt'),
+    url(r'^csv_report/$', 'csv_report', name='payment_csv_report'),
 )
 
 if settings.FEATURES['ENABLE_SHOPPING_CART']:
@@ -13,7 +14,6 @@ if settings.FEATURES['ENABLE_SHOPPING_CART']:
         url(r'^clear/$', 'clear_cart'),
         url(r'^remove_item/$', 'remove_item'),
         url(r'^add/course/(?P<course_id>[^/]+/[^/]+/[^/]+)/$', 'add_course_to_cart', name='add_course_to_cart'),
-        url(r'^csv_report/$', 'csv_report', name='payment_csv_report'),
     )
 
 if settings.FEATURES.get('ENABLE_PAYMENT_FAKE'):

lms/djangoapps/shoppingcart/views.py

@@ -29,13 +29,13 @@ REPORT_TYPES = [
 ]
 
 
-def initialize_report(report_type):
+def initialize_report(report_type, start_date, end_date, start_letter=None, end_letter=None):
     """
     Creates the appropriate type of Report object based on the string report_type.
     """
     for item in REPORT_TYPES:
         if report_type in item:
-            return item[1]()
+            return item[1](start_date, end_date, start_letter, end_letter)
     raise ReportTypeDoesNotExistException
 
 @require_POST
@@ -193,32 +193,31 @@ def csv_report(request):
     """
     Downloads csv reporting of orderitems
     """
-
     if not _can_download_report(request.user):
         return HttpResponseForbidden(_('You do not have permission to view this page.'))
 
     # TODO temp filler for start letter, end letter
 
     if request.method == 'POST':
-        start_str = request.POST.get('start_date', '')
-        end_str = request.POST.get('end_date', '')
+        start_date = request.POST.get('start_date', '')
+        end_date = request.POST.get('end_date', '')
         start_letter = request.POST.get('start_letter', '')
         end_letter = request.POST.get('end_letter', '')
         report_type = request.POST.get('requested_report', '')
         try:
-            start_date = _get_date_from_str(start_str) + datetime.timedelta(days=0)
-            end_date = _get_date_from_str(end_str) + datetime.timedelta(days=1)
+            start_date = _get_date_from_str(start_date) + datetime.timedelta(days=0)
+            end_date = _get_date_from_str(end_date) + datetime.timedelta(days=1)
         except ValueError:
             # Error case: there was a badly formatted user-input date string
-            return _render_report_form(start_str, end_str, start_letter, end_letter, report_type, date_fmt_error=True)
+            return _render_report_form(start_date, end_date, start_letter, end_letter, report_type, date_fmt_error=True)
 
-        report = initialize_report(report_type)
-        items = report.rows(start_date, end_date, start_letter, end_letter)
+        report = initialize_report(report_type, start_date, end_date, start_letter, end_letter)
+        items = report.rows()
 
         response = HttpResponse(mimetype='text/csv')
         filename = "purchases_report_{}.csv".format(datetime.datetime.now(pytz.UTC).strftime("%Y-%m-%d-%H-%M-%S"))
         response['Content-Disposition'] = 'attachment; filename="{}"'.format(filename)
-        report.write_csv(response, start_date, end_date, start_letter, end_letter)
+        report.write_csv(response)
         return response
 
     elif request.method == 'GET':