From f55542618ac453db201bfdb12e9daff485880aee Mon Sep 17 00:00:00 2001 From: Bryan Wilson Date: Mon, 31 May 2021 13:51:04 -0700 Subject: [PATCH 1/5] Fix previous_months_iterator. Addresses #356 Avoid rrule when possibly iterating forward from days 29-31 of months, rrule simply skip month if it tries to iterate to e.g., February 30. previous_months_iterator start num_months -1 back to incl. current Since changing the method of providing a previous months generator, we need to start one fewer months back or we lose the current month in the output. current_month should be last month returned in history --- figures/helpers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/figures/helpers.py b/figures/helpers.py index 9bc0be09..ac3bea77 100644 --- a/figures/helpers.py +++ b/figures/helpers.py @@ -206,9 +206,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=(months_back - 1)) - for dt in rrule(freq=MONTHLY, dtstart=start_month, until=month_for): + for n_months in range(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) From 25f3078101c347eb1cc322a7cc1f9de98abac63d Mon Sep 17 00:00:00 2001 From: Bryan Wilson Date: Fri, 4 Jun 2021 12:53:34 -0700 Subject: [PATCH 2/5] Remove unused helpers rrule imports --- figures/helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/figures/helpers.py b/figures/helpers.py index ac3bea77..0ba57eb9 100644 --- a/figures/helpers.py +++ b/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 From 7bff21593aa96ea66920f8c8037122a2ae4d3cc3 Mon Sep 17 00:00:00 2001 From: Bryan Wilson Date: Fri, 4 Jun 2021 14:21:05 -0700 Subject: [PATCH 3/5] SMMViewset, CMMViewset Use months_back 6 frontend code expects last entry in history to be current_month: for example, in code for current month change since last month, comparison is to `this.state.valueHistory.size-2` -1 since array is 0-indexed, -2 to find previous month val in history Probably the months_back 7 was being used in tests because of the issue that this PR addresses. --- tests/views/test_course_monthly_metrics_viewset.py | 1 - tests/views/test_site_monthly_metrics_viewset.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/views/test_course_monthly_metrics_viewset.py b/tests/views/test_course_monthly_metrics_viewset.py index 1d87f233..fe948e8d 100644 --- a/tests/views/test_course_monthly_metrics_viewset.py +++ b/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 diff --git a/tests/views/test_site_monthly_metrics_viewset.py b/tests/views/test_site_monthly_metrics_viewset.py index a1eb287f..2ad05854 100644 --- a/tests/views/test_site_monthly_metrics_viewset.py +++ b/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 From 3f5f9beb29db1f3fcbb311c32fce5a83871a8bcb Mon Sep 17 00:00:00 2001 From: Bryan Wilson Date: Tue, 8 Jun 2021 16:46:36 -0700 Subject: [PATCH 4/5] Fix previous_months_iterator and its test method If month_for is June and months_back is 6, should return last day of June and 5 previous months--Jan, Feb, Mar, Apr, May, June Handle months_back of zero properly, returning the month_for month --- figures/helpers.py | 8 +++++--- tests/test_helpers.py | 24 ++++++++++-------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/figures/helpers.py b/figures/helpers.py index 0ba57eb9..09e11eba 100644 --- a/figures/helpers.py +++ b/figures/helpers.py @@ -195,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 @@ -205,9 +207,9 @@ 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 - 1)) + start_month = month_for - relativedelta(months=(max(0, months_back - 1))) - for n_months in range(months_back): + 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) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 1425514c..21c1f588 100644 --- a/tests/test_helpers.py +++ b/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 From eaf24559c6d7dff8dc29ccb4e58f819aae0a79f6 Mon Sep 17 00:00:00 2001 From: Bryan Wilson Date: Tue, 8 Jun 2021 16:50:04 -0700 Subject: [PATCH 5/5] Update six version pin for devsite Juniper --- devsite/requirements/juniper_base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devsite/requirements/juniper_base.txt b/devsite/requirements/juniper_base.txt index 9a25c43b..b55f0e30 100644 --- a/devsite/requirements/juniper_base.txt +++ b/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