From 103993d448f13a3dd7936e04b13d67e77e31a08a Mon Sep 17 00:00:00 2001
From: John Baldwin <jlbaldwin@gmail.com>
Date: Fri, 26 Jun 2020 00:17:25 -0400
Subject: [PATCH 1/2] Fixes CourseDailyMetricsSerializer when average_progress
 is 1.00

Updated CourseDailyMetricsSerializer.average_progress `max_digits` to
allow 3 digits. Was previously 2 digits which failed the serializer
validation when the `average_progress` value was 1.00.

Also updated the `average_progress` field definition to set min and max
values. Unfortunately, this validation is not being tested. Did some
initial timeboxed investigation, but stopped at time limit of 1 hour.
Note here that we need to investigation and make sure validation works.

Updated the serializer tests
---
 figures/serializers.py    | 14 +++++++++++---
 tests/test_serializers.py | 21 +++++++++++++++++++--
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/figures/serializers.py b/figures/serializers.py
index cae6e155..5d4ce394 100644
--- a/figures/serializers.py
+++ b/figures/serializers.py
@@ -170,8 +170,16 @@ class Meta:
 
 class CourseDailyMetricsSerializer(serializers.ModelSerializer):
     """Provides summary data about a specific course
+
+    [john@appsembler] min_value and max_value validation added to average_progress,
+    however, unit tests show these are ignored. I timeboxed an hour to go through
+    the DRF 3.6.3 source code, and try to trace why then stopped due to time
+    constraints.
     """
-    average_progress = serializers.DecimalField(max_digits=2, decimal_places=2)
+    average_progress = serializers.DecimalField(max_digits=3,
+                                                decimal_places=2,
+                                                min_value=0.00,
+                                                max_value=1.00)
 
     class Meta:
         model = CourseDailyMetrics
@@ -290,9 +298,9 @@ def get_staff(self, obj):
     def get_metrics(self, obj):
         qs = CourseDailyMetrics.objects.filter(course_id=str(obj.id))
         if qs:
-            return CourseDailyMetricsSerializer(qs.latest('date_for')).data
+            return CourseDailyMetricsSerializer(qs.order_by('-date_for')[0]).data
         else:
-            return []
+            return None
 
 
 def get_course_history_metric(site, course_id, func, date_for, months_back):
diff --git a/tests/test_serializers.py b/tests/test_serializers.py
index 6aaeb64f..f1c657f0 100644
--- a/tests/test_serializers.py
+++ b/tests/test_serializers.py
@@ -12,6 +12,7 @@
 from django.contrib.sites.models import Site
 from django.db import models
 from django.utils.timezone import utc
+from rest_framework.exceptions import ValidationError
 
 from student.models import CourseEnrollment
 
@@ -240,6 +241,22 @@ def test_has_fields(self):
             else:
                 assert data[field_name] == db_field
 
+    @pytest.mark.parametrize('average_progress', [0, 0.00, 0.5, 1.0, 1.00])
+    def test_average_progress_valid(self, average_progress):
+        obj = CourseDailyMetricsFactory(average_progress=average_progress)
+        serializer = CourseDailyMetricsSerializer(instance=obj)
+        check_val = Decimal(average_progress).quantize(Decimal('.00'))
+        data = serializer.data
+        assert data['average_progress'] == unicode(check_val)
+
+    @pytest.mark.xfail
+    @pytest.mark.parametrize('average_progress', [-1.0, 9.0])
+    def test_average_progress_not_valid(self, average_progress):
+        obj = CourseDailyMetricsFactory(average_progress=average_progress)
+        serializer = CourseDailyMetricsSerializer(instance=obj)
+        with pytest.raises(ValidationError):
+            data = serializer.data
+
 
 @pytest.mark.django_db
 class TestSiteDailyMetricsSerializer(object):
@@ -353,8 +370,8 @@ def test_get_metrics_with_cdm_records(self):
         [CourseDailyMetricsFactory(site=self.site,
                                    course_id=self.course_overview.id,
                                    date_for=date) for date in dates]
-        assert self.serializer.get_metrics(
-            self.course_overview)['date_for'] == dates[-1]
+        data = self.serializer.get_metrics(self.course_overview)
+        assert data['date_for'] == dates[-1]
 
 
 class TestGeneralUserDataSerializer(object):

From 70ec51eb85d366b715428f5452a7cbd8019abb1d Mon Sep 17 00:00:00 2001
From: John Baldwin <jlbaldwin@gmail.com>
Date: Fri, 26 Jun 2020 00:41:29 -0400
Subject: [PATCH 2/2] Test GeneralCourseDataSerializer for get_metrics is None

---
 tests/test_serializers.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/test_serializers.py b/tests/test_serializers.py
index f1c657f0..b8c0792a 100644
--- a/tests/test_serializers.py
+++ b/tests/test_serializers.py
@@ -373,6 +373,10 @@ def test_get_metrics_with_cdm_records(self):
         data = self.serializer.get_metrics(self.course_overview)
         assert data['date_for'] == dates[-1]
 
+    def test_get_metrics_with_no_cdm_records(self):
+        data = self.serializer.get_metrics(self.course_overview)
+        assert not data
+
 
 class TestGeneralUserDataSerializer(object):
     '''Tests the UserIndexSerializer serializer class