Merge branch 'master' into replace_django_fernet

CHANGELOG.rst

@@ -16,10 +16,23 @@ Change Log
 Unreleased
 ----------
 
-[4.0.12]
+[4.0.15]
 feat: Replace deprecated `django-fernet-fields` with its forked djfernet.
+--------
+[4.0.14]
+--------
+bug: swapping grades api grade_percent return value type from string to float
+
+[4.0.13]
+--------
+fix: more flexible default site 
+
+[4.0.12]
+--------
+fix: allow sub directories in moodle base URLs
 
 [4.0.11]
+--------
 feat: upgrade django-simple-history to 3.1.1
 
 [4.0.10]

enterprise/__init__.py

@@ -2,6 +2,6 @@
 Your project description goes here.
 """
 
-__version__ = "4.0.12"
+__version__ = "4.0.15"
 
 default_app_config = "enterprise.apps.EnterpriseConfig"

enterprise/models.py

@@ -165,7 +165,7 @@ def get_default_site():
     In stage it should be 'courses.stage.edx.org'.
     """
     value = settings.LMS_BASE
-    site, __ = Site.objects.get_or_create(domain=value, name=value)
+    site, __ = Site.objects.get_or_create(domain=value, defaults={"name": value})
     return site.id
 
 

integrated_channels/integrated_channel/exporters/learner_data.py

@@ -332,6 +332,21 @@ def get_grades_summary(
                     f' Passed timestamp: {passed_timestamp}'
                 ))
 
+        # In the past we have been inconsistent about the format/source/typing of the grade_percent value.
+        # Initial investigations have lead us to believe that grade percents from the source are seemingly more
+        # accurate now, so we're enforcing float typing. To account for the chance that old documentation is
+        # right and grade percents are reported as letter grade values, the float conversion is wrapped in a try/except
+        # in order to both not blow up and also log said instance of letter grade values.
+        try:
+            if grade_percent is not None:
+                grade_percent = float(grade_percent)
+        except ValueError as exc:
+            LOGGER.error(generate_formatted_log(
+                channel_name, enterprise_customer_uuid, lms_user_id, course_id,
+                f'Grade percent is not a valid float: {grade_percent}. Failed with exc: {exc}'
+            ))
+            grade_percent = None
+
         return completed_date_from_api, grade_from_api, is_passing_from_api, grade_percent, passed_timestamp
 
     def get_incomplete_content_count(self, enterprise_enrollment, channel_name):

integrated_channels/moodle/client.py

@@ -109,7 +109,7 @@ class MoodleAPIClient(IntegratedChannelApiClient):
         - Customer's Moodle service short name
     """
 
-    MOODLE_API_PATH = '/webservice/rest/server.php'
+    MOODLE_API_PATH = 'webservice/rest/server.php'
 
     def __init__(self, enterprise_configuration):
         """
@@ -122,6 +122,7 @@ def __init__(self, enterprise_configuration):
         super().__init__(enterprise_configuration)
         self.config = apps.get_app_config('moodle')
         self.token = enterprise_configuration.token or self._get_access_token()
+        self.api_url = urljoin(self.enterprise_configuration.moodle_base_url, self.MOODLE_API_PATH)
 
     def _post(self, additional_params):
         """
@@ -137,10 +138,7 @@ def _post(self, additional_params):
         params.update(additional_params)
 
         response = requests.post(
-            url=urljoin(
-                self.enterprise_configuration.moodle_base_url,
-                self.MOODLE_API_PATH,
-            ),
+            url=self.api_url,
             data=params,
             headers=headers
         )
@@ -166,7 +164,7 @@ def _get_access_token(self):
         response = requests.post(
             urljoin(
                 self.enterprise_configuration.moodle_base_url,
-                '/login/token.php',
+                'login/token.php',
             ),
             params=querystring,
             headers={
@@ -236,10 +234,7 @@ def _get_course_contents(self, course_id):
             'moodlewsrestformat': 'json'
         }
         response = requests.get(
-            urljoin(
-                self.enterprise_configuration.moodle_base_url,
-                self.MOODLE_API_PATH,
-            ),
+            self.api_url,
             params=params
         )
         return response
@@ -286,10 +281,7 @@ def _get_courses(self, key):
             'moodlewsrestformat': 'json'
         }
         response = requests.get(
-            urljoin(
-                self.enterprise_configuration.moodle_base_url,
-                self.MOODLE_API_PATH,
-            ),
+            self.api_url,
             params=params
         )
         return response

tests/test_enterprise/management/test_manufacture_data.py

@@ -72,7 +72,7 @@ def call_command(command_name, *args, **options):
     def get_actions(parser):
         # Parser actions and actions from sub-parser choices.
         for opt in parser._actions:  # pylint: disable=protected-access
-            if isinstance(opt, _SubParsersAction):  # pylint: disable=protected-access
+            if isinstance(opt, _SubParsersAction):
                 for sub_opt in opt.choices.values():
                     yield from get_actions(sub_opt)
             else:

tests/test_integrated_channels/test_integrated_channel/test_exporters/test_learner_data.py

@@ -657,6 +657,71 @@ def test_learner_data_self_paced_course_with_funky_certificate(
             assert report.completed_timestamp == expected_completion
             assert report.grade == expected_grade
 
+    @ddt.data(
+        ('A-', None),
+        ('0.72', 72.0),
+    )
+    @ddt.unpack
+    @mock.patch('enterprise.models.CourseEnrollment')
+    @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.get_course_certificate')
+    @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.get_single_user_grade')
+    @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.get_persistent_grade')
+    @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.get_course_details')
+    @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.get_completion_summary')
+    @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.is_course_completed')
+    @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
+    def test_learner_data_grade_typing(
+            self,
+            grade_value,
+            expected_grade,
+            mock_course_catalog_api,
+            mock_is_course_completed,
+            mock_get_completion_summary,
+            mock_get_course_details,
+            mock_get_persistent_grade,
+            mock_get_single_user_grade,
+            mock_get_course_certificate,
+            mock_course_enrollment_class
+    ):
+        factories.EnterpriseCourseEnrollmentFactory(
+            enterprise_customer_user=self.enterprise_customer_user,
+            course_id=self.course_id,
+        )
+        # Return a mock certificate with a grade value to be transformed by the exporter
+        certificate = {
+            'username': self.user,
+            'course_id': self.course_id,
+            'certificate_type': 'professional',
+            'status': 'downloadable',
+            'is_passing': True,
+            'grade': grade_value,
+        }
+
+        mock_course_catalog_api.return_value.get_course_id.return_value = self.course_key
+        mock_get_completion_summary.return_value = {'complete_count': 1, 'incomplete_count': 0, 'locked_count': 0}
+        mock_is_course_completed.return_value = True
+        mock_get_course_certificate.return_value = certificate
+        mock_get_persistent_grade.return_value = mock_persistent_course_grade(
+            user_id='a-user-id',
+            course_id=self.course_id,
+            passed_timestamp=self.NOW_TIMESTAMP,
+        )
+        mock_get_course_details.return_value = mock_course_overview(pacing='self', end=None)
+        mock_get_single_user_grade.return_value = mock_single_learner_grade(
+            passing=True,
+        )
+        mock_course_enrollment_class.objects.get.return_value.mode = 'verified'
+
+        # Collect the learner data, with time set to NOW
+        with freeze_time(self.NOW):
+            self.config = factories.Degreed2EnterpriseCustomerConfigurationFactory(
+                enterprise_customer=self.enterprise_customer,
+            )
+            self.exporter = self.config.get_learner_data_exporter('test')
+            learner_data = list(self.exporter.export())
+
+        assert learner_data[0].grade == expected_grade
+
     @mock.patch('enterprise.models.EnrollmentApiClient')
     @mock.patch('integrated_channels.integrated_channel.exporters.learner_data.GradesApiClient')
     @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')

tests/test_integrated_channels/test_moodle/test_client.py

@@ -4,7 +4,6 @@
 
 import random
 import unittest
-from urllib.parse import urljoin
 
 import pytest
 import responses
@@ -64,11 +63,11 @@ class TestMoodleApiClient(unittest.TestCase):
     def setUp(self):
         super().setUp()
         self.moodle_base_url = 'http://testing/'
+        self.custom_moodle_base_url = 'http://testing/subdir/'
         self.token = 'token'
         self.password = 'pass'
         self.user = 'user'
         self.user_email = '[email protected]'
-        self.moodle_api_path = '/webservice/rest/server.php'
         self.moodle_course_id = random.randint(1, 1000)
         self._get_courses_response = bytearray('{{"courses": [{{"id": {}}}]}}'.format(self.moodle_course_id), 'utf-8')
         self.empty_get_courses_response = bytearray('{"courses": []}', 'utf-8')
@@ -84,14 +83,21 @@ def setUp(self):
             token=self.token,
         )
         self.enterprise_custom_config = factories.MoodleEnterpriseCustomerConfigurationFactory(
-            moodle_base_url=self.moodle_base_url,
+            moodle_base_url=self.custom_moodle_base_url,
             username=self.user,
             password=self.password,
             token=self.token,
             grade_scale=10,
             grade_assignment_name='edX Grade Test'
         )
 
+    def test_api_url(self):
+        moodle_api_client = MoodleAPIClient(self.enterprise_config)
+        moodle_api_client_with_sub_dir = MoodleAPIClient(self.enterprise_custom_config)
+
+        assert moodle_api_client.api_url == 'http://testing/webservice/rest/server.php'
+        assert moodle_api_client_with_sub_dir.api_url == 'http://testing/subdir/webservice/rest/server.php'
+
     def test_moodle_config_is_set(self):
         """
         Test global_moodle_config is setup.
@@ -107,7 +113,7 @@ def test_get_course_id(self):
         client = MoodleAPIClient(self.enterprise_config)
         responses.add(
             responses.GET,
-            urljoin(self.enterprise_config.moodle_base_url, self.moodle_api_path),
+            client.api_url,
             json={'courses': [{'id': 2}]},
             status=200
         )
@@ -233,10 +239,8 @@ def test_delete_content_metadata_no_course_found(self):
     def test_course_completion_with_no_course(self):
         """Test that we properly raise exceptions if the client receives a 404 from Moodle"""
         with responses.RequestsMock() as rsps:
-            moodle_api_path = urljoin(
-                self.enterprise_config.moodle_base_url,
-                self.moodle_api_path,
-            )
+            client = MoodleAPIClient(self.enterprise_config)
+            moodle_api_path = client.api_url
             moodle_get_courses_query = 'wstoken={}&wsfunction=core_course_get_courses_by_field&field=idnumber' \
                                        '&value={}&moodlewsrestformat=json'.format(self.token, self.moodle_course_id)
             request_url = '{}?{}'.format(moodle_api_path, moodle_get_courses_query)
@@ -246,7 +250,7 @@ def test_course_completion_with_no_course(self):
                 body=b'{}',
                 status=200
             )
-            client = MoodleAPIClient(self.enterprise_config)
+
             with pytest.raises(ClientError) as client_error:
                 client.create_course_completion(self.user_email, self.learner_data_payload)