diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 9e98bfd965..7e348177b5 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -581,9 +581,15 @@ class CourseEnrollment(models.Model): ) @classmethod - def enrollments_in(cls, course_id): - """Return a queryset of CourseEnrollment for every active enrollment in the course.""" - return cls.objects.filter(course_id=course_id, is_active=True,) + def enrollments_in(cls, course_id, mode=None): + """ + Return a queryset of CourseEnrollment for every active enrollment in the course course_id. + Returns only CourseEnrollments with the given mode, if a mode is supplied by the caller. + """ + if mode is None: + return cls.objects.filter(course_id=course_id, is_active=True,) + else: + return cls.objects.filter(course_id=course_id, is_active=True, mode=mode) def activate(self): """Makes this `CourseEnrollment` record active. Saves immediately.""" diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 685f320e4d..cf22b1756c 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -205,13 +205,13 @@ class OrderItem(models.Model): # this is denormalized, but convenient for SQL queries for reports, etc. user should always be = order.user user = models.ForeignKey(User, db_index=True) # this is denormalized, but convenient for SQL queries for reports, etc. status should always be = order.status - status = models.CharField(max_length=32, default='cart', choices=ORDER_STATUSES) + status = models.CharField(max_length=32, default='cart', choices=ORDER_STATUSES, db_index=True) qty = models.IntegerField(default=1) unit_cost = models.DecimalField(default=0.0, decimal_places=2, max_digits=30) line_desc = models.CharField(default="Misc. Item", max_length=1024) currency = models.CharField(default="usd", max_length=8) # lower case ISO currency codes - fulfilled_time = models.DateTimeField(null=True) - refund_requested_time = models.DateTimeField(null=True) + fulfilled_time = models.DateTimeField(null=True, db_index=True) + refund_requested_time = models.DateTimeField(null=True, db_index=True) service_fee = models.DecimalField(default=0.0, decimal_places=2, max_digits=30) # general purpose field, not user-visible. Used for reporting report_comments = models.TextField(default="") @@ -568,3 +568,8 @@ class CertificateItem(OrderItem): "Please include your order number in your e-mail. " "Please do NOT include your credit card information.").format( billing_email=settings.PAYMENT_SUPPORT_EMAIL) + + @classmethod + def verified_certificates_in(cls, course_id, status): + """Return a queryset of CertificateItem for every verified enrollment in course_id with the given status.""" + return CertificateItem.objects.filter(course_id=course_id, mode='verified', status=status) diff --git a/lms/djangoapps/shoppingcart/reports.py b/lms/djangoapps/shoppingcart/reports.py index b4047b22d5..360797f595 100644 --- a/lms/djangoapps/shoppingcart/reports.py +++ b/lms/djangoapps/shoppingcart/reports.py @@ -9,31 +9,25 @@ from course_modes.models import CourseMode from decimal import Decimal -class Report(models.Model): +class Report(Object): """ Base class for making CSV reports related to revenue, enrollments, etc To make a different type of report, write a new subclass that implements - the methods get_report_data, csv_report_header_row, and csv_report_row. + the methods report_row_generator and csv_report_header_row. """ - def get_report_data(self, start_date, end_date, start_letter=None, end_letter=None): + def report_row_generator(self, start_date, end_date, start_letter=None, end_letter=None): """ - Performs database queries necessary for the report. May return either a query result - or a list of lists, depending on the particular type of report--see Report subclasses - for sample implementations. + Performs database queries necessary for the report. Returns an generator of + lists, in which each list is a separate row of the report. """ raise NotImplementedError def csv_report_header_row(self): """ - Returns the appropriate header based on the report type. - """ - raise NotImplementedError - - def csv_report_row(self, item): - """ - Given the results of get_report_data, this function generates a single row of a csv. + Returns the appropriate header based on the report type, in the form of a + list of strings. """ raise NotImplementedError @@ -42,23 +36,36 @@ class Report(models.Model): Given the string report_type, a file object to write to, and start/end date bounds, generates a CSV report of the appropriate type. """ - items = self.get_report_data(start_date, end_date, start_letter, end_letter) + items = self.report_row_generator(start_date, end_date, start_letter, end_letter) writer = unicodecsv.writer(filelike, encoding="utf-8") writer.writerow(self.csv_report_header_row()) for item in items: - writer.writerow(self.csv_report_row(item)) + writer.writerow(item) class RefundReport(Report): """ Subclass of Report, used to generate Refund Reports for finance purposes. + + For each refund between a given start_date and end_date, we find the relevant + order number, customer name, date of transaction, date of refund, and any service + fees. """ - def get_report_data(self, start_date, end_date, start_letter=None, end_letter=None): - return CertificateItem.objects.filter( + def report_row_generator(self, start_date, end_date, start_letter=None, end_letter=None): + query = CertificateItem.objects.select_related('user__profile').filter( status="refunded", refund_requested_time__gte=start_date, refund_requested_time__lt=end_date, - ) + ).order_by('refund_requested_time') + for item in query: + yield [ + item.order_id, + item.user.profile.name, + item.fulfilled_time, + item.refund_requested_time, + item.line_cost, + item.service_fee, + ] def csv_report_header_row(self): return [ @@ -70,28 +77,35 @@ class RefundReport(Report): "Service Fees (if any)", ] - def csv_report_row(self, item): - return [ - item.order_id, - item.user.get_full_name(), - item.fulfilled_time, - item.refund_requested_time, # TODO Change this torefund_fulfilled once we start recording that value - item.line_cost, - item.service_fee, - ] - class ItemizedPurchaseReport(Report): """ Subclass of Report, used to generate itemized purchase reports. + + For all purchases (verified certificates, paid course registrations, etc) between + 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 get_report_data(self, start_date, end_date, start_letter=None, end_letter=None): - return OrderItem.objects.filter( + def report_row_generator(self, start_date, end_date, start_letter=None, end_letter=None): + query = OrderItem.objects.filter( status="purchased", fulfilled_time__gte=start_date, fulfilled_time__lt=end_date, ).order_by("fulfilled_time") + for item in query: + yield [ + item.fulfilled_time, + item.order_id, # pylint: disable=no-member + item.status, + item.qty, + item.unit_cost, + item.line_cost, + item.currency, + item.line_desc, + item.report_comments, + ] + def csv_report_header_row(self): return [ "Purchase Time", @@ -105,47 +119,50 @@ class ItemizedPurchaseReport(Report): "Comments" ] - def csv_report_row(self, item): - return [ - item.fulfilled_time, - item.order_id, # pylint: disable=no-member - item.status, - item.qty, - item.unit_cost, - item.line_cost, - item.currency, - item.line_desc, - item.report_comments, - ] - class CertificateStatusReport(Report): """ - Subclass of Report, used to generate Certificate Status Reports for ed services. + Subclass of Report, used to generate Certificate Status Reports for Ed Services. + + For each course in each university whose name is within the range start_letter and end_letter, + inclusive, (i.e., the letter range H-J includes both Ithaca College and Harvard University), we + calculate the total enrollment, audit enrollment, honor enrollment, verified enrollment, total + gross revenue, gross revenue over the minimum, and total dollars refunded. """ - def get_report_data(self, start_date, end_date, start_letter=None, end_letter=None): + def report_row_generator(self, start_date, end_date, start_letter=None, end_letter=None): results = [] for course_id in settings.COURSE_LISTINGS['default']: + # If the first letter of the university is between start_letter and end_letter, then we include + # it in the report. These comparisons are unicode-safe. if (start_letter.lower() <= course_id.lower()) and (end_letter.lower() >= course_id.lower()) and (get_course_by_id(course_id) is not None): cur_course = get_course_by_id(course_id) university = cur_course.org - course = cur_course.number + " " + cur_course.display_name # TODO add term (i.e. Fall 2013)? + course = cur_course.number + " " + cur_course.display_name_with_default # TODO add term (i.e. Fall 2013)? enrollments = CourseEnrollment.enrollments_in(course_id) total_enrolled = enrollments.count() - audit_enrolled = enrollments.filter(mode="audit").count() - honor_enrolled = enrollments.filter(mode="honor").count() + audit_enrolled = CourseEnrollment.enrollments_in(course_id, "audit").count() + honor_enrolled = CourseEnrollment.enrollments_in(course_id, "honor").count() + # Since every verified enrollment has 1 and only 1 cert item, let's just query those - verified_enrollments = CertificateItem.objects.filter(course_id=course_id, mode="verified", status="purchased") - verified_enrolled = verified_enrollments.count() - gross_rev_temp = CertificateItem.objects.filter(course_id=course_id, mode="verified", status="purchased").aggregate(Sum('unit_cost')) - gross_rev = gross_rev_temp['unit_cost__sum'] - gross_rev_over_min = gross_rev - (CourseMode.objects.get(course_id=course_id, mode_slug="verified").min_price * verified_enrolled) - refunded_enrollments = CertificateItem.objects.filter(course_id='course_id', mode="verified", status="refunded") - number_of_refunds = refunded_enrollments.count() - dollars_refunded_temp = refunded_enrollments.aggregate(Sum('unit_cost')) - if dollars_refunded_temp['unit_cost__sum'] is None: + verified_enrollments = CertificateItem.verified_certificates_in(course_id, 'purchased') + if verified_enrollments is None: + verified_enrolled = 0 + gross_rev = Decimal(0.00) + gross_rev_over_min = Decimal(0.00) + else: + verified_enrolled = verified_enrollments.count() + gross_rev_temp = verified_enrollments.aggregate(Sum('unit_cost')) + gross_rev = gross_rev_temp['unit_cost__sum'] + gross_rev_over_min = gross_rev - (CourseMode.objects.get(course_id=course_id, mode_slug="verified").min_price * verified_enrolled) + + # should I be worried about is_active here? + refunded_enrollments = CertificateItem.verified_certificates_in(course_id, 'refunded') + if refunded_enrollments is None: + number_of_refunds = 0 dollars_refunded = Decimal(0.00) else: + number_of_refunds = refunded_enrollments.count() + dollars_refunded_temp = refunded_enrollments.aggregate(Sum('unit_cost')) dollars_refunded = dollars_refunded_temp['unit_cost__sum'] result = [ @@ -162,7 +179,8 @@ class CertificateStatusReport(Report): ] results.append(result) - return results + for item in results: + yield item def csv_report_header_row(self): return [ @@ -178,41 +196,43 @@ class CertificateStatusReport(Report): "Dollars Refunded", ] - def csv_report_row(self, item): - return item - class UniversityRevenueShareReport(Report): """ Subclass of Report, used to generate University Revenue Share Reports for finance purposes. + + For each course in each university whose name is within the range start_letter and end_letter, + inclusive, (i.e., the letter range H-J includes both Ithaca College and Harvard University), we calculate + 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 get_report_data(self, start_date, end_date, start_letter=None, end_letter=None): + def report_row_generator(self, start_date, end_date, start_letter=None, end_letter=None): results = [] for course_id in settings.COURSE_LISTINGS['default']: + # If the first letter of the university is between start_letter and end_letter, then we include + # it in the report. These comparisons are unicode-safe. if (start_letter.lower() <= course_id.lower()) and (end_letter.lower() >= course_id.lower()): try: cur_course = get_course_by_id(course_id) except: break university = cur_course.org - course = cur_course.number + " " + cur_course.display_name + 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) - all_paid_certs = CertificateItem.objects.filter(course_id=course_id, status="purchased") - - total_payments_collected_temp = all_paid_certs.aggregate(Sum('unit_cost')) - if total_payments_collected_temp['unit_cost__sum'] is None: - total_payments_collected = Decimal(0.00) - else: + all_paid_certs = CertificateItem.verified_certificates_in(course_id, "purchased") + try: + total_payments_collected_temp = all_paid_certs.aggregate(Sum('unit_cost')) total_payments_collected = total_payments_collected_temp['unit_cost__sum'] - - total_service_fees_temp = all_paid_certs.aggregate(Sum('service_fee')) - if total_service_fees_temp['service_fee__sum'] is None: - service_fees = Decimal(0.00) - else: + except: + total_payments_collected = Decimal(0.00) + try: + total_service_fees_temp = all_paid_certs.aggregate(Sum('service_fee')) service_fees = total_service_fees_temp['service_fee__sum'] + except: + service_fees = Decimal(0.00) - refunded_enrollments = CertificateItem.objects.filter(course_id=course_id, status="refunded") + refunded_enrollments = CertificateItem.verified_certificates_in(course_id, "refunded") num_refunds = refunded_enrollments.count() amount_refunds_temp = refunded_enrollments.aggregate(Sum('unit_cost')) @@ -232,7 +252,8 @@ class UniversityRevenueShareReport(Report): ] results.append(result) - return results + for item in results: + yield item def csv_report_header_row(self): return [ @@ -244,6 +265,3 @@ class UniversityRevenueShareReport(Report): "Number of Successful Refunds", "Total Amount of Refunds", ] - - def csv_report_row(self, item): - return item diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 37b73ae506..a9b0ede4d5 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -356,32 +356,47 @@ class ItemizedPurchaseReportTest(ModuleStoreTestCase): self.cart.purchase() self.now = datetime.datetime.now(pytz.UTC) + # We can't modify the values returned by report_row_generator directly, since it's a generator, but + # we need the times on CORRECT_CSV and the generated report to match. So, we extract the times from + # the report_row_generator and place them in CORRECT_CSV. + self.time_str = {} + report = initialize_report("itemized_purchase_report") + purchases = report.report_row_generator(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) + num_of_item = 0 + for item in purchases: + num_of_item += 1 + self.time_str[num_of_item] = item[0] + + self.CORRECT_CSV = dedent(""" + Purchase Time,Order ID,Status,Quantity,Unit Cost,Total Cost,Currency,Description,Comments + {time_str1},1,purchased,1,40,40,usd,Registration for Course: Robot Super Course,Ba\xc3\xbc\xe5\x8c\x85 + {time_str2},1,purchased,1,40,40,usd,"Certificate of Achievement, verified cert for course Robot Super Course", + """.format(time_str1=str(self.time_str[1]), time_str2=str(self.time_str[2]))) + def test_purchased_items_btw_dates(self): report = initialize_report("itemized_purchase_report") - purchases = report.get_report_data(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) - self.assertEqual(len(purchases), 2) - self.assertIn(self.reg.orderitem_ptr, purchases) - self.assertIn(self.cert_item.orderitem_ptr, purchases) - no_purchases = report.get_report_data(self.now + self.FIVE_MINS, self.now + self.FIVE_MINS + self.FIVE_MINS) - self.assertFalse(no_purchases) + purchases = report.report_row_generator(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) - test_time = datetime.datetime.now(pytz.UTC) + # 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) - 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(test_time))) + no_purchases = report.report_row_generator(self.now + self.FIVE_MINS, self.now + self.FIVE_MINS + self.FIVE_MINS) + + 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") - for item in report.get_report_data(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS): - item.fulfilled_time = self.test_time - item.save() - csv_file = StringIO.StringIO() report.make_report(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) csv = csv_file.getvalue() diff --git a/lms/djangoapps/shoppingcart/tests/test_reports.py b/lms/djangoapps/shoppingcart/tests/test_reports.py index 3b80ba61ec..6c90cd8af8 100644 --- a/lms/djangoapps/shoppingcart/tests/test_reports.py +++ b/lms/djangoapps/shoppingcart/tests/test_reports.py @@ -29,44 +29,36 @@ class ReportTypeTests(ModuleStoreTestCase): def setUp(self): # Need to make a *lot* of users for this one self.user1 = UserFactory.create() - self.user1.first_name = "John" - self.user1.last_name = "Doe" - self.user1.save() + self.user1.profile.name = "John Doe" + self.user1.profile.save() self.user2 = UserFactory.create() - self.user2.first_name = "Jane" - self.user2.last_name = "Deer" - self.user2.save() + self.user2.profile.name = "Jane Deer" + self.user2.profile.save() self.user3 = UserFactory.create() - self.user3.first_name = "Joe" - self.user3.last_name = "Miller" - self.user3.save() + self.user3.profile.name = "Joe Miller" + self.user3.profile.save() self.user4 = UserFactory.create() - self.user4.first_name = "Simon" - self.user4.last_name = "Blackquill" - self.user4.save() + self.user4.profile.name = "Simon Blackquill" + self.user4.profile.save() self.user5 = UserFactory.create() - self.user5.first_name = "Super" - self.user5.last_name = "Mario" - self.user5.save() + self.user5.profile.name = "Super Mario" + self.user5.profile.save() self.user6 = UserFactory.create() - self.user6.first_name = "Princess" - self.user6.last_name = "Peach" - self.user6.save() + self.user6.profile.name = "Princess Peach" + self.user6.profile.save() self.user7 = UserFactory.create() - self.user7.first_name = "King" - self.user7.last_name = "Bowser" - self.user7.save() + self.user7.profile.name = "King Bowser" + self.user7.profile.save() self.user8 = UserFactory.create() - self.user8.first_name = "Susan" - self.user8.last_name = "Smith" - self.user8.save() + self.user8.profile.name = "Susan Smith" + self.user8.profile.save() # Two are verified, three are audit, one honor @@ -116,16 +108,29 @@ class ReportTypeTests(ModuleStoreTestCase): self.cart.purchase(self.user8, self.course_id) CourseEnrollment.unenroll(self.user8, self.course_id) - self.test_time = datetime.datetime.now(pytz.UTC) + # We can't modify the values returned by report_row_generator directly, since it's a generator, but + # we need the times on CORRECT_CSV and the generated report to match. So, we extract the times from + # the report_row_generator and place them in CORRECT_CSV. + self.time_str = {} + report = initialize_report("refund_report") + refunds = report.report_row_generator(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) + time_index = 0 + for item in refunds: + self.time_str[time_index] = item[2] + time_index += 1 + self.time_str[time_index] = item[3] + time_index += 1 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 - """.format(time_str=str(self.test_time))) + 3,King Bowser,{time_str0},{time_str1},40,0 + 4,Susan Smith,{time_str2},{time_str3},40,0 + """.format(time_str0=str(self.time_str[0]), time_str1=str(self.time_str[1]), time_str2=str(self.time_str[2]), time_str3=str(self.time_str[3]))) + + self.test_time = datetime.datetime.now(pytz.UTC) 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,0,0 + MITx,999 Robot Super Course,6,3,1,2,80.00,0.00,2,80.00 """.format(time_str=str(self.test_time))) self.CORRECT_UNI_REVENUE_SHARE_CSV = dedent(""" @@ -133,10 +138,16 @@ class ReportTypeTests(ModuleStoreTestCase): MITx,999 Robot Super Course,0,80.00,0.00,2,80.00 """.format(time_str=str(self.test_time))) - def test_refund_report_get_report_data(self): + def test_refund_report_report_row_generator(self): report = initialize_report("refund_report") - refunded_certs = report.get_report_data(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) - self.assertEqual(len(refunded_certs), 2) + refunded_certs = report.report_row_generator(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) + + # check that we have the right number + num_certs = 0 + for cert in refunded_certs: + num_certs += 1 + self.assertEqual(num_certs, 2) + self.assertTrue(CertificateItem.objects.get(user=self.user7, course_id=self.course_id)) self.assertTrue(CertificateItem.objects.get(user=self.user8, course_id=self.course_id)) @@ -145,11 +156,6 @@ class ReportTypeTests(ModuleStoreTestCase): Tests that a generated purchase report CSV is as we expect """ report = initialize_report("refund_report") - for item in report.get_report_data(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS): - item.fulfilled_time = self.test_time - item.refund_requested_time = self.test_time # hm do we want to make these different - item.save() - csv_file = StringIO.StringIO() report.make_report(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) csv = csv_file.getvalue() diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index 617cd2c58a..eac4684253 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -213,7 +213,7 @@ def csv_report(request): return _render_report_form(start_str, end_str, start_letter, end_letter, report_type, date_fmt_error=True) report = initialize_report(report_type) - items = report.get_report_data(start_date, end_date, start_letter, end_letter) + items = report.report_row_generator(start_date, end_date, start_letter, end_letter) # TODO add this back later as a query-est function or something try: