Merge pull request #1881 from openedx/hamza/ENT-5039

[ENT-5039] - Moodle - Inactivate courses instead of true delete

CHANGELOG.rst

@@ -15,6 +15,10 @@ Change Log
 
 Unreleased
 ----------
+[4.5.4]
+-------
+feat: inactive moodle course instead of true delete
+
 [4.5.3]
 -------
 feat: added dry run mode for content metadata transmission

enterprise/__init__.py

@@ -2,4 +2,4 @@
 Your project description goes here.
 """
 
-__version__ = "4.5.3"
+__version__ = "4.5.4"

integrated_channels/moodle/client.py

@@ -355,7 +355,13 @@ def create_content_metadata(self, serialized_data):
         more_than_one_course = serialized_data.get('courses[1][shortname]')
         serialized_data['wsfunction'] = 'core_course_create_courses'
         try:
-            response = self._wrapped_post(serialized_data)
+            moodle_course_id = self._get_course_id(serialized_data['courses[0][idnumber]'])
+            # Course already exists but is hidden - make it visible
+            if moodle_course_id:
+                serialized_data['courses[0][visible]'] = 1
+                return self.update_content_metadata(serialized_data)
+            else:  # create a new course
+                response = self._wrapped_post(serialized_data)
         except MoodleClientError as error:
             # treat duplicate as successful, but only if its a single course
             # set chunk size settings to 1 if youre seeing a lot of these errors
@@ -397,11 +403,9 @@ def delete_content_metadata(self, serialized_data):
             rsp._content = bytearray('{"result": "Course not found."}', 'utf-8')  # pylint: disable=protected-access
             return rsp
         moodle_course_id = parsed_response['courses'][0]['id']
-        params = {
-            'wsfunction': 'core_course_delete_courses',
-            'courseids[]': moodle_course_id
-        }
-        response = self._wrapped_post(params)
+        serialized_data['wsfunction'] = 'core_course_update_courses'
+        serialized_data['courses[0][id]'] = moodle_course_id
+        response = self._wrapped_post(serialized_data)
         return response.status_code, response.text
 
     def create_assessment_reporting(self, user_id, payload):

integrated_channels/moodle/exporters/content_metadata.py

@@ -114,3 +114,10 @@ def transform_end(self, content_metadata_item):
         if end_date:
             return int(parse(end_date).replace(tzinfo=timezone.utc).timestamp())
         return None
+
+    def _apply_delete_transformation(self, metadata):
+        """
+        Specific transformations required for "deleting/hiding" a course on a Moodle.
+        """
+        metadata['visible'] = 0  # hide a course on moodle - instead of true delete
+        return metadata

tests/test_integrated_channels/test_moodle/test_client.py

@@ -70,6 +70,7 @@ def setUp(self):
         self.user_email = 'testemail@example.com'
         self.moodle_course_id = random.randint(1, 1000)
         self._get_courses_response = bytearray('{{"courses": [{{"id": {}}}]}}'.format(self.moodle_course_id), 'utf-8')
+        self._get_courses_response_empty = bytearray('{}', 'utf-8')
         self.empty_get_courses_response = bytearray('{"courses": []}', 'utf-8')
         self.moodle_module_id = random.randint(1, 1000)
         self.moodle_module_name = 'module'
@@ -129,6 +130,11 @@ def test_successful_create_content_metadata(self):
 
         client = MoodleAPIClient(self.enterprise_config)
         client._post = unittest.mock.MagicMock(name='_post', return_value=SUCCESSFUL_RESPONSE)  # pylint: disable=protected-access
+        client._get_courses = unittest.mock.MagicMock(name='_get_courses')  # pylint: disable=protected-access
+        mock_response = Response()
+        mock_response.status_code = 200
+        mock_response._content = self._get_courses_response_empty  # pylint: disable=protected-access
+        client._get_courses.return_value = mock_response  # pylint: disable=protected-access
         client.create_content_metadata(SERIALIZED_DATA)
         client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access
 
@@ -142,6 +148,11 @@ def test_duplicate_shortname_create_content_metadata(self):
 
         client = MoodleAPIClient(self.enterprise_config)
         client._post = unittest.mock.MagicMock(name='_post', return_value=SHORTNAMETAKEN_RESPONSE)  # pylint: disable=protected-access
+        client._get_courses = unittest.mock.MagicMock(name='_get_courses')  # pylint: disable=protected-access
+        mock_response = Response()
+        mock_response.status_code = 200
+        mock_response._content = self._get_courses_response_empty  # pylint: disable=protected-access
+        client._get_courses.return_value = mock_response  # pylint: disable=protected-access
         client.create_content_metadata(SERIALIZED_DATA)
         client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access
 
@@ -152,9 +163,13 @@ def test_duplicate_courseidnumber_create_content_metadata(self):
         """
         expected_data = SERIALIZED_DATA.copy()
         expected_data['wsfunction'] = 'core_course_create_courses'
-
         client = MoodleAPIClient(self.enterprise_config)
         client._post = unittest.mock.MagicMock(name='_post', return_value=COURSEIDNUMBERTAKEN_RESPONSE)  # pylint: disable=protected-access
+        client._get_courses = unittest.mock.MagicMock(name='_get_courses')  # pylint: disable=protected-access
+        mock_response = Response()
+        mock_response.status_code = 200
+        mock_response._content = self._get_courses_response_empty  # pylint: disable=protected-access
+        client._get_courses.return_value = mock_response  # pylint: disable=protected-access
         client.create_content_metadata(SERIALIZED_DATA)
         client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access
 
@@ -168,6 +183,11 @@ def test_multi_duplicate_create_content_metadata(self):
 
         client = MoodleAPIClient(self.enterprise_config)
         client._post = unittest.mock.MagicMock(name='_post', return_value=SHORTNAMETAKEN_RESPONSE)  # pylint: disable=protected-access
+        client._get_courses = unittest.mock.MagicMock(name='_get_courses')  # pylint: disable=protected-access
+        mock_response = Response()
+        mock_response.status_code = 200
+        mock_response._content = self._get_courses_response_empty  # pylint: disable=protected-access
+        client._get_courses.return_value = mock_response  # pylint: disable=protected-access
         with self.assertRaises(MoodleClientError):
             client.create_content_metadata(MULTI_SERIALIZED_DATA)
         client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access
@@ -182,6 +202,11 @@ def test_multi_duplicate_courseidnumber_create_content_metadata(self):
 
         client = MoodleAPIClient(self.enterprise_config)
         client._post = unittest.mock.MagicMock(name='_post', return_value=COURSEIDNUMBERTAKEN_RESPONSE)  # pylint: disable=protected-access
+        client._get_courses = unittest.mock.MagicMock(name='_get_courses')  # pylint: disable=protected-access
+        mock_response = Response()
+        mock_response.status_code = 200
+        mock_response._content = self._get_courses_response_empty  # pylint: disable=protected-access
+        client._get_courses.return_value = mock_response  # pylint: disable=protected-access
         with self.assertRaises(MoodleClientError):
             client.create_content_metadata(MULTI_SERIALIZED_DATA)
         client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access
@@ -205,8 +230,12 @@ def test_update_content_metadata(self):
     def test_delete_content_metadata(self):
         """
         Test core logic for formatting a delete request to Moodle.
+        Mark a course visible:0 rather than doing a true delete
         """
-        expected_data = {'wsfunction': 'core_course_delete_courses', 'courseids[]': self.moodle_course_id}
+        expected_data = SERIALIZED_DATA.copy()
+        expected_data['wsfunction'] = 'core_course_update_courses'
+        expected_data['courses[0][visible]'] = 0
+        expected_data['courses[0][id]'] = self.moodle_course_id
 
         client = MoodleAPIClient(self.enterprise_config)
         client._post = unittest.mock.MagicMock(name='_post', return_value=SUCCESSFUL_RESPONSE)  # pylint: disable=protected-access
@@ -217,7 +246,9 @@ def test_delete_content_metadata(self):
         mock_response._content = self._get_courses_response  # pylint: disable=protected-access
 
         client._get_courses.return_value = mock_response  # pylint: disable=protected-access
-        client.delete_content_metadata(SERIALIZED_DATA)
+        serialized_data = SERIALIZED_DATA.copy()
+        serialized_data['courses[0][visible]'] = 0
+        client.delete_content_metadata(serialized_data)
 
         client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access
 
@@ -352,3 +383,28 @@ def test_get_course_final_grade_module_custom_name(self):
 
         # The base transmitter expects the create course completion response to be a tuple of (code, body)
         assert client.get_course_final_grade_module(2) == (1337, 'foobar')
+
+    def test_successful_update_existing_content_metadata(self):
+        """
+        Test core logic of create_content_metadata to ensure
+        if a course already exists(hidden) then client sets its
+        visibility: 1 instead of creating a new course
+        """
+        expected_data = SERIALIZED_DATA.copy()
+        expected_data['wsfunction'] = 'core_course_update_courses'
+        expected_data['courses[0][visible]'] = 1
+        expected_data['courses[0][id]'] = self.moodle_course_id
+
+        client = MoodleAPIClient(self.enterprise_config)
+        client._post = unittest.mock.MagicMock(  # pylint: disable=protected-access
+            name='_post', return_value=SUCCESSFUL_RESPONSE)
+        client._get_courses = unittest.mock.MagicMock(  # pylint: disable=protected-access
+            name='_get_courses')
+        mock_response = Response()
+        mock_response.status_code = 200
+        mock_response._content = self._get_courses_response  # pylint: disable=protected-access
+        client._get_course_id = unittest.mock.MagicMock(name='_get_course_id')  # pylint: disable=protected-access
+        client._get_course_id.return_value = self.moodle_course_id  # pylint: disable=protected-access
+        client._get_courses.return_value = mock_response  # pylint: disable=protected-access
+        client.create_content_metadata(SERIALIZED_DATA)
+        client._post.assert_called_once_with(expected_data)  # pylint: disable=protected-access

tests/test_integrated_channels/test_moodle/test_exporters/test_content_metadata.py

@@ -164,3 +164,16 @@ def test_transform_title(self):
         )
         exporter = MoodleContentMetadataExporter('fake-user', self.config)
         assert exporter.transform_title(content_metadata_item) == expected_title
+
+    @responses.activate
+    def test_apply_delete_transformation(self):
+        """
+        `MoodleContentMetadataExporter``'s ``transform_title`` returns a str
+        featuring the title and partners/organizations
+        """
+        content_metadata_item = {
+            'title': 'edX Demonstration Course'
+        }
+        exporter = MoodleContentMetadataExporter('fake-user', self.config)
+        transformed_metada_data = exporter._apply_delete_transformation(content_metadata_item)  # pylint: disable=protected-access
+        assert transformed_metada_data['visible'] == 0