Response to CR 1-14

common/djangoapps/course_modes/models.py

@@ -97,6 +97,15 @@ class CourseMode(models.Model):
 
     @classmethod
     def min_course_price_for_verified_for_currency(cls, course_id, currency):
+        """
+        Returns the minimum price of the course int he appropriate currency over all the
+        course's *verified*, non-expired modes.
+
+        Assuming all verified courses have a minimum price of >0, this value should always
+        be >0.
+
+        If no verified mode is found, 0 is returned.
+        """
         modes = cls.modes_for_course(course_id)
         for mode in modes:
             if (mode.currency == currency) and (mode.slug == 'verified'):

common/djangoapps/student/models.py

@@ -23,7 +23,7 @@ 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 import Count
 from django.db.models.signals import post_save
 from django.dispatch import receiver, Signal
 import django.dispatch
@@ -35,6 +35,7 @@ from eventtracking import tracker
 
 from course_modes.models import CourseMode
 import lms.lib.comment_client as cc
+from util.query import use_read_replica_if_available
 
 unenroll_done = Signal(providing_args=["course_enrollment"])
 log = logging.getLogger(__name__)
@@ -588,7 +589,7 @@ class CourseEnrollment(models.Model):
         enrollment count for each individual mode.
         """
         # 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'))
+        query = use_read_replica_if_available(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:

common/djangoapps/util/query.py

+""" Utility functions related to database queries """
 from django.conf import settings
 
+
 def use_read_replica_if_available(queryset):
+    """
+    If there is a database called 'read_replica', use that database for the queryset.
+    """
     return queryset.using("read_replica") if "read_replica" in settings.DATABASES else queryset
\ No newline at end of file

lms/djangoapps/shoppingcart/models.py

+""" Models for the shopping cart and assorted purchase types """
+
 from collections import namedtuple
 from datetime import datetime
 from decimal import Decimal
@@ -581,7 +583,7 @@ class CertificateItem(OrderItem):
         """
         Returns a Decimal indicating the total sum of field_to_aggregate for all verified certificates with a particular status.
 
-        Sample usages: 
+        Sample usages:
         - status 'refunded' and field_to_aggregate 'unit_cost' will give the total amount of money refunded for course_id
         - status 'purchased' and field_to_aggregate 'service_fees' gives the sum of all service fees for purchased certificates
         etc

lms/djangoapps/shoppingcart/reports.py

+""" Objects and functions related to generating CSV reports """
+
 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
@@ -11,7 +10,6 @@ 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
 
 
@@ -191,10 +189,10 @@ class CertificateStatusReport(Report):
             yield [
                 university,
                 course,
-                "",
-                "",
-                "",
-                "",
+                course_announce_date,
+                course_reg_start_date,
+                course_reg_close_date,
+                registration_period,
                 total_enrolled,
                 audit_enrolled,
                 honor_enrolled,
@@ -267,10 +265,11 @@ class UniversityRevenueShareReport(Report):
             _("Total Amount of Refunds"),
         ]
 
+
 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. 
+    These comparisons are unicode-safe.
     """
 
     valid_courses = []

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -18,12 +18,10 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from shoppingcart.models import (Order, OrderItem, CertificateItem, InvalidCartItem, PaidCourseRegistration,
                                  OrderItemSubclassPK, PaidCourseRegistrationAnnotation)
-from shoppingcart.views import initialize_report, REPORT_TYPES
-from shoppingcart.reports import ItemizedPurchaseReport, CertificateStatusReport, UniversityRevenueShareReport, RefundReport
 from student.tests.factories import UserFactory
 from student.models import CourseEnrollment
 from course_modes.models import CourseMode
-from shoppingcart.exceptions import PurchasedCallbackException, ReportTypeDoesNotExistException
+from shoppingcart.exceptions import PurchasedCallbackException
 import pytz
 import datetime
 
@@ -326,101 +324,6 @@ class PaidCourseRegistrationTest(ModuleStoreTestCase):
 
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
-class ItemizedPurchaseReportTest(ModuleStoreTestCase):
-    """
-    Tests for the models used to generate itemized purchase reports
-    """
-    FIVE_MINS = datetime.timedelta(minutes=5)
-    TEST_ANNOTATION = u'Ba\xfc\u5305'
-
-    def setUp(self):
-        self.user = UserFactory.create()
-        self.course_id = "MITx/999/Robot_Super_Course"
-        self.cost = 40
-        self.course = CourseFactory.create(org='MITx', number='999', display_name=u'Robot Super 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_mode2 = CourseMode(course_id=self.course_id,
-                                  mode_slug="verified",
-                                  mode_display_name="verified cert",
-                                  min_price=self.cost)
-        course_mode2.save()
-        self.annotation = PaidCourseRegistrationAnnotation(course_id=self.course_id, annotation=self.TEST_ANNOTATION)
-        self.annotation.save()
-        self.cart = Order.get_cart_for_user(self.user)
-        self.reg = PaidCourseRegistration.add_to_order(self.cart, self.course_id)
-        self.cert_item = CertificateItem.add_to_order(self.cart, self.course_id, self.cost, 'verified')
-        self.cart.purchase()
-        self.now = datetime.datetime.now(pytz.UTC)
-
-        paid_reg = PaidCourseRegistration.objects.get(course_id=self.course_id, user=self.user)
-        paid_reg.fulfilled_time = self.now
-        paid_reg.refund_requested_time = self.now
-        paid_reg.save()
-
-        cert = CertificateItem.objects.get(course_id=self.course_id, user=self.user)
-        cert.fulfilled_time = self.now
-        cert.refund_requested_time = self.now
-        cert.save()
-
-        self.CORRECT_CSV = dedent("""
-            Purchase Time,Order ID,Status,Quantity,Unit Cost,Total Cost,Currency,Description,Comments
-            {time_str},1,purchased,1,40,40,usd,Registration for Course: Robot Super Course,Ba\xc3\xbc\xe5\x8c\x85
-            {time_str},1,purchased,1,40,40,usd,"Certificate of Achievement, verified cert for course Robot Super Course",
-            """.format(time_str=str(self.now)))
-
-    def test_purchased_items_btw_dates(self):
-        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
-        for item in purchases:
-            num_purchases += 1
-        self.assertEqual(num_purchases, 2)
-        #self.assertIn(self.reg.orderitem_ptr, purchases)
-        #self.assertIn(self.cert_item.orderitem_ptr, purchases)
-
-        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:
-            num_purchases +=1
-        self.assertEqual(num_purchases, 0)
-
-    def test_purchased_csv(self):
-        """
-        Tests that a generated purchase report CSV is as we expect
-        """
-        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)
-        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_CSV.strip())
-
-    def test_csv_report_no_annotation(self):
-        """
-        Fill in gap in test coverage.  csv_report_comments for PaidCourseRegistration instance with no
-        matching annotation
-        """
-        # delete the matching annotation
-        self.annotation.delete()
-        self.assertEqual(u"", self.reg.csv_report_comments)
-
-    def test_paidcourseregistrationannotation_unicode(self):
-        """
-        Fill in gap in test coverage.  __unicode__ method of PaidCourseRegistrationAnnotation
-        """
-        self.assertEqual(unicode(self.annotation), u'{} : {}'.format(self.course_id, self.TEST_ANNOTATION))
-
-
-@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
 class CertificateItemTest(ModuleStoreTestCase):
     """
     Tests for verifying specific CertificateItem functionality

lms/djangoapps/shoppingcart/tests/test_reports.py

@@ -13,8 +13,7 @@ from django.test.utils import override_settings
 
 from course_modes.models import CourseMode
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
-from shoppingcart.models import (Order, CertificateItem)
-from shoppingcart.reports import ItemizedPurchaseReport, CertificateStatusReport, UniversityRevenueShareReport, RefundReport
+from shoppingcart.models import (Order, CertificateItem, PaidCourseRegistration, PaidCourseRegistrationAnnotation)
 from shoppingcart.views import initialize_report, REPORT_TYPES
 from student.tests.factories import UserFactory
 from student.models import CourseEnrollment
@@ -56,11 +55,11 @@ class ReportTypeTests(ModuleStoreTestCase):
         self.honor_user.profile.save()
 
         self.first_refund_user = UserFactory.create()
-        self.first_refund_user.profile.name = "King Bowser"
+        self.first_refund_user.profile.name = u"King Bowsér"
         self.first_refund_user.profile.save()
 
         self.second_refund_user = UserFactory.create()
-        self.second_refund_user.profile.name = "Susan Smith"
+        self.second_refund_user.profile.name = u"Súsan Smith"
         self.second_refund_user.profile.save()
 
         # Two are verified, three are audit, one honor
@@ -125,8 +124,8 @@ class ReportTypeTests(ModuleStoreTestCase):
 
         self.CORRECT_REFUND_REPORT_CSV = dedent("""
             Order Number,Customer Name,Date of Original Transaction,Date of Refund,Amount of Refund,Service Fees (if any)
-            3,King Bowser,{time_str},{time_str},40,0
-            4,Susan Smith,{time_str},{time_str},40,0
+            3,King Bowsér,{time_str},{time_str},40,0
+            4,Súsan Smith,{time_str},{time_str},40,0
             """.format(time_str=str(self.test_time)))
 
         self.CORRECT_CERT_STATUS_CSV = dedent("""
@@ -177,3 +176,96 @@ class ReportTypeTests(ModuleStoreTestCase):
         report.write_csv(csv_file)
         csv = csv_file.getvalue()
         self.assertEqual(csv.replace('\r\n', '\n').strip(), self.CORRECT_UNI_REVENUE_SHARE_CSV.strip())
+
+
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
+class ItemizedPurchaseReportTest(ModuleStoreTestCase):
+    """
+    Tests for the models used to generate itemized purchase reports
+    """
+    FIVE_MINS = datetime.timedelta(minutes=5)
+    TEST_ANNOTATION = u'Ba\xfc\u5305'
+
+    def setUp(self):
+        self.user = UserFactory.create()
+        self.course_id = "MITx/999/Robot_Super_Course"
+        self.cost = 40
+        self.course = CourseFactory.create(org='MITx', number='999', display_name=u'Robot Super 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_mode2 = CourseMode(course_id=self.course_id,
+                                  mode_slug="verified",
+                                  mode_display_name="verified cert",
+                                  min_price=self.cost)
+        course_mode2.save()
+        self.annotation = PaidCourseRegistrationAnnotation(course_id=self.course_id, annotation=self.TEST_ANNOTATION)
+        self.annotation.save()
+        self.cart = Order.get_cart_for_user(self.user)
+        self.reg = PaidCourseRegistration.add_to_order(self.cart, self.course_id)
+        self.cert_item = CertificateItem.add_to_order(self.cart, self.course_id, self.cost, 'verified')
+        self.cart.purchase()
+        self.now = datetime.datetime.now(pytz.UTC)
+
+        paid_reg = PaidCourseRegistration.objects.get(course_id=self.course_id, user=self.user)
+        paid_reg.fulfilled_time = self.now
+        paid_reg.refund_requested_time = self.now
+        paid_reg.save()
+
+        cert = CertificateItem.objects.get(course_id=self.course_id, user=self.user)
+        cert.fulfilled_time = self.now
+        cert.refund_requested_time = self.now
+        cert.save()
+
+        self.CORRECT_CSV = dedent("""
+            Purchase Time,Order ID,Status,Quantity,Unit Cost,Total Cost,Currency,Description,Comments
+            {time_str},1,purchased,1,40,40,usd,Registration for Course: Robot Super Course,Ba\xc3\xbc\xe5\x8c\x85
+            {time_str},1,purchased,1,40,40,usd,"Certificate of Achievement, verified cert for course Robot Super Course",
+            """.format(time_str=str(self.now)))
+
+    def test_purchased_items_btw_dates(self):
+        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
+        for item in purchases:
+            num_purchases += 1
+        self.assertEqual(num_purchases, 2)
+
+        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:
+            num_purchases += 1
+        self.assertEqual(num_purchases, 0)
+
+    def test_purchased_csv(self):
+        """
+        Tests that a generated purchase report CSV is as we expect
+        """
+        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)
+        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_CSV.strip())
+
+    def test_csv_report_no_annotation(self):
+        """
+        Fill in gap in test coverage.  csv_report_comments for PaidCourseRegistration instance with no
+        matching annotation
+        """
+        # delete the matching annotation
+        self.annotation.delete()
+        self.assertEqual(u"", self.reg.csv_report_comments)
+
+    def test_paidcourseregistrationannotation_unicode(self):
+        """
+        Fill in gap in test coverage.  __unicode__ method of PaidCourseRegistrationAnnotation
+        """
+        self.assertEqual(unicode(self.annotation), u'{} : {}'.format(self.course_id, self.TEST_ANNOTATION))

lms/djangoapps/shoppingcart/tests/test_views.py

@@ -21,7 +21,6 @@ from course_modes.models import CourseMode
 from edxmako.shortcuts import render_to_response
 from shoppingcart.processors import render_purchase_form_html
 from mock import patch, Mock, sentinel
-from shoppingcart.reports import ItemizedPurchaseReport
 from shoppingcart.views import initialize_report
 
 

lms/envs/aws.py

@@ -285,6 +285,8 @@ if AWS_SECRET_ACCESS_KEY == "":
 
 AWS_STORAGE_BUCKET_NAME = AUTH_TOKENS.get('AWS_STORAGE_BUCKET_NAME', 'edxuploads')
 
+# If there is a database called 'read_replica', you can use the use_read_replica_if_available
+# function in util/query.py, which is useful for very large database reads
 DATABASES = AUTH_TOKENS['DATABASES']
 
 XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE']

lms/envs/common.py

@@ -202,6 +202,8 @@ FEATURES = {
     # Give course staff unrestricted access to grade downloads (if set to False,
     # only edX superusers can perform the downloads)
     'ALLOW_COURSE_STAFF_GRADE_DOWNLOADS': False,
+
+    'ENABLED_PAYMENT_REPORTS': [ "refund_report", "itemized_purchase_report", "university_revenue_share",  "certificate_status"],
 }
 
 # Used for A/B testing

lms/envs/dev.py

@@ -52,6 +52,8 @@ LOGGING = get_logger_config(ENV_ROOT / "log",
                             dev_env=True,
                             debug=True)
 
+# If there is a database called 'read_replica', you can use the use_read_replica_if_available
+# function in util/query.py, which is useful for very large database reads
 DATABASES = {
     'default': {
         'ENGINE': 'django.db.backends.sqlite3',

lms/templates/shoppingcart/download_report.html

 <%! from django.utils.translation import ugettext as _ %>
 <%! from django.core.urlresolvers import reverse %>
+<%! from django.conf import settings %>
 <%inherit file="../main.html" />
 
 <%block name="title"><title>${_("Download CSV Reports")}</title></%block>
@@ -19,21 +20,35 @@
     <label for="end_date">${_("End Date: ")}</label>
     <input id="end_date" type="text" value="${end_date}" name="end_date"/>
     <br/>
+
+    %if "itemized_purchase_report" in settings.FEATURES['ENABLED_PAYMENT_REPORTS']:
     <button type = "submit" name="requested_report" value="itemized_purchase_report">Itemized Purchase Report</button>
     <br/>
+    %endif
+
+    %if "refund_report" in settings.FEATURES['ENABLED_PAYMENT_REPORTS']:
     <button type = "submit" name="requested_report" value="refund_report">Refund Report</button>
+    <br/>
+    %endif
+
+    <br/>
 
-    <br/><br/>
     <p>${_("These reports are delimited alphabetically by university name.  i.e., generating a report with 'Start Letter' A and 'End Letter' C will generate reports for all universities starting with A, B, and C.")}</p>
     <label for="start_letter">${_("Start Letter: ")}</label>
     <input id="start_letter" type="text" value="${start_letter}" name="start_letter"/>
     <label for="end_letter">${_("End Letter: ")}</label>
     <input id="end_letter" type="text" value="${end_letter}" name="end_letter"/>
     <input type="hidden" name="csrfmiddlewaretoken" value="${csrf_token}" />
-
     <br/>
+
+    %if "university_revenue_share" in settings.FEATURES['ENABLED_PAYMENT_REPORTS']:
     <button type = "submit" name="requested_report" value="university_revenue_share">University Revenue Share</button>
     <br/>
+    %endif
+
+    %if "certificate_status" in settings.FEATURES['ENABLED_PAYMENT_REPORTS']:
     <button type="submit" name="requested_report" value="certificate_status">Certiciate Status</button>
+    <br/>
+    %endif
   </form>
 </section>