Merge pull request #358 from appsembler/bryan/fix-prev-months-iterator

Fix previous_months_iterator

devsite/requirements/juniper_base.txt

@@ -11,7 +11,7 @@
 
 celery==3.1.26.post2
 django-celery==3.3.1
-six==1.14.0
+six==1.15.0
 
 # Faker is used to seed mock data in devsite
 Faker==4.1.0 

figures/helpers.py

@@ -54,7 +54,6 @@
 
 from dateutil.parser import parse as dateutil_parse
 from dateutil.relativedelta import relativedelta
-from dateutil.rrule import rrule, MONTHLY
 
 from opaque_keys.edx.keys import CourseKey
 import six
@@ -196,7 +195,9 @@ def days_in_month(month_for):
 
 # TODO: Consider changing name to 'months_back_iterator' or similar
 def previous_months_iterator(month_for, months_back):
-    """Iterator returns a year,month tuple for n months including the month_for
+    """Iterator returns a year,month tuple for n months including the month_for.
+    months_back is a misnomer as iteration includes the start month.  The actual
+    number of previous months iterated is months_back minus one.
 
     month_for is either a date, datetime, or tuple with year and month
     months back is the number of months to iterate
@@ -206,9 +207,10 @@ def previous_months_iterator(month_for, months_back):
         # TODO make sure we've got just two values in the tuple
         month_for = datetime.date(year=month_for[0], month=month_for[1], day=1)
     if isinstance(month_for, (datetime.datetime, datetime.date)):
-        start_month = month_for - relativedelta(months=months_back)
+        start_month = month_for - relativedelta(months=(max(0, months_back - 1)))
 
-    for dt in rrule(freq=MONTHLY, dtstart=start_month, until=month_for):
+    for n_months in range(max(1, months_back)):
+        dt = start_month + relativedelta(months=n_months)
         last_day_of_month = days_in_month(month_for=dt)
         yield (dt.year, dt.month, last_day_of_month)
 

tests/test_helpers.py

@@ -189,27 +189,23 @@ def test_prev_day(self):
 
 class TestMonthIterator(object):
 
-    @pytest.mark.parametrize('month_for, months_back, first_month', [
-        ((2018, 1, 31), 0, datetime.date(2018, 1, 1)),
-        ((2018, 1, 31), 6, datetime.date(2017, 7, 1)),
-        ])
-    def test_previous_months_iterator(self, month_for, months_back, first_month):
-
-        def as_month_tuple(month):
-            return (month.year, month.month)
+    @pytest.mark.parametrize('month_for, months_back', [
+        ((2018, 1, 31), 0),
+        ((2018, 1, 31), 6),
+        ((2020, 4, 30), 2),  # let's use a leap year
+    ])
+    def test_previous_months_iterator(self, month_for, months_back):
         expected_vals = []
-        for i in range(months_back):
-            a_month = first_month+relativedelta(months=i)
+        month_for = datetime.date(year=month_for[0], month=month_for[1], day=month_for[2])
+        for i in range(max(1, months_back), 0, -1):  # e.g., 6, 5, 4, 3, 2, 1
+            a_month = month_for - relativedelta(months=i - 1)
             last_day_in_month = calendar.monthrange(a_month.year, a_month.month)[1]
             expected_vals.append(
                 (a_month.year, a_month.month, last_day_in_month)
-                )
-        expected_vals.append(month_for)
-
+            )
         vals = list(previous_months_iterator(month_for, months_back))
         assert vals == expected_vals
 
-
 def test_first_last_days_for_month():
     month_for = '2/2020'
     month = 2

tests/views/test_course_monthly_metrics_viewset.py

@@ -129,7 +129,6 @@ def setup(self, db, settings):
         if organizations_support_sites():
             settings.FEATURES['FIGURES_IS_MULTISITE'] = True
         super(TestCourseMonthlyMetricsViewSet, self).setup(db)
-        # self.months_back = 6
 
     def check_response(self, response, endpoint):
         """Helper method to reduce duplication

tests/views/test_site_monthly_metrics_viewset.py

@@ -86,13 +86,13 @@ class TestSiteMonthlyMetricsViewSet(BaseViewTest):
     """
     request_path = 'api/site-metrics'
     view_class = SiteMonthlyMetricsViewSet
+    months_back = 6
 
     @pytest.fixture(autouse=True)
     def setup(self, db, settings):
         if organizations_support_sites():
             settings.FEATURES['FIGURES_IS_MULTISITE'] = True
         super(TestSiteMonthlyMetricsViewSet, self).setup(db)
-        self.months_back = 7
 
     def check_response(self, response, endpoint):
         assert response.status_code == status.HTTP_200_OK