fix: review comments

Author: shravani-sonata-gottapu

integrated_channels/moodle/client.py

@@ -17,16 +17,107 @@
 from integrated_channels.utils import generate_formatted_log, stringify_and_store_api_record
 
 MOODLE_ERROR_STATUS_MAP = {
+    # Duplicate/Conflict errors (409)
     "shortnametaken": 409,
     "courseidnumbertaken": 409,
+    "morethanonerecordinfetch": 409,
+    "duplicateemail": 409,
+    "duplicateidnumber": 409,
+    "duplicateusername": 409,
+
+    # Not Found errors (404)
     "cannotfindcourse": 404,
     "cannotfinduser": 404,
+    "invalidrecord": 404,
+    "usernamenotfound": 404,
+    "coursenotfound": 404,
+    "usernotfound": 404,
+    "invalidcourseid": 404,
+    "invaliduserid": 404,
+    "modulenotfound": 404,
+    "activitynotfound": 404,
+
+    # Bad Request errors (400)
     "missingparam": 400,
+    "codingerror": 400,
+    "invalidparameter": 400,
+    "invalidparametervalue": 400,
+    "requiredparametermissing": 400,
+    "invalidresponse": 400,
+    "invaliddata": 400,
+
+    # Authentication errors (401)
+    "invalidlogin": 401,
+    "invalidtoken": 401,
+    "sessiontimeout": 401,
+    "requiresauthentication": 401,
+
+    # Authorization/Permission errors (403)
+    "accessexception": 403,
+    "errorcatcontextnotvalid": 403,
+    "usernotfullysetup": 403,
+    "nopermission": 403,
+    "nopermissions": 403,
+    "requirecapability": 403,
+    "contextupdatefailed": 403,
+    "notpermittedtoaccesscourse": 403,
+
+    # Server/Database errors (503)
+    "dbconnectionfailed": 503,
+    "dmlreadexception": 503,
+    "dmlwriteexception": 503,
+    "dmlexception": 503,
+    "databaseerror": 503,
+
+    # Service/Method errors (400)
+    "invalidservicename": 400,
+    "invalidfunctionname": 400,
+    "servicerequireslogin": 401,
+
+    # Enrollment errors (400)
+    "cannotenrol": 400,
+    "enrolnotpermitted": 403,
+    "alreadyenrolled": 409,
+
+    # Grade/Completion errors (400)
+    "invalidcompletion": 400,
+    "invalidgrade": 400,
+    "itemidnotfound": 404,
 }
 
 LOGGER = logging.getLogger(__name__)
 
 
+def _effective_error_status_code(status_code, error_code=None):
+    """
+    Map Moodle error codes to correct HTTP status codes.
+    Moodle returns 200 response status code on failed transmission jobs which is the
+    historical and default behavior of Moodle web services. This function corrects
+    the status code based on the error_code in the response body.
+    Args:
+        status_code: The HTTP status code from the response
+        error_code: The error code from Moodle's response body
+    Returns:
+        int: The effective status code (mapped from MOODLE_ERROR_STATUS_MAP,
+             or 555 for unknown semantic errors, or original status_code)
+    """
+    # First check if we have a mapped status for this error code
+    if error_code and error_code in MOODLE_ERROR_STATUS_MAP:
+        return MOODLE_ERROR_STATUS_MAP[error_code]
+
+    # If status code indicates error (>=400), keep it
+    if status_code and status_code >= 400:
+        return status_code
+
+    # If we get here, Moodle returned a 2xx/3xx status with an unknown error code
+    # This is a semantic error - the HTTP status says success but there's an error
+    if error_code:
+        return 555  # Semantic error for unknown Moodle failure
+
+    # No error code and good status - this shouldn't happen in error path
+    return status_code if status_code else 500
+
+
 class MoodleClientError(ClientError):
     """
     Indicate a problem when interacting with Moodle.
@@ -102,14 +193,16 @@ def inner(self, *args, **kwargs):
             self.token = self._get_access_token()  # pylint: disable=protected-access
             response = method(self, *args, **kwargs)
         elif error_code:
+            status_code = _effective_error_status_code(response.status_code, error_code)
+
             raise MoodleClientError(
                 'Moodle API Client Task "{method}" failed with error code '
                 '"{code}" and message: "{msg}" '.format(
                     method=method.__name__,
                     code=error_code,
                     msg=body.get('message'),
                 ),
-                response.status_code,
+                status_code,
                 moodle_error={
                     'mapped_status': mapped_status,
                     'error_code': error_code,

tests/test_integrated_channels/test_moodle/test_client.py

@@ -501,3 +501,31 @@ def test_missing_param_error_handling(self):
             'A required parameter is missing.',
             context.exception.message
         )
+
+    def test_moodle_errorcode_200_response_maps_to_correct_status(self):
+        """
+        Test that when Moodle returns HTTP 200 with an errorcode,
+        the client maps it to the correct HTTP status code using
+        MOODLE_ERROR_STATUS_MAP.
+        """
+        # pylint: disable=protected-access
+
+        client = MoodleAPIClient(self.enterprise_config)
+
+        mock_response = unittest.mock.Mock(spec=Response)
+        mock_response.json.return_value = {
+            "errorcode": "missingparam",
+            "message": "A required parameter is missing."
+        }
+        mock_response.status_code = 200
+
+        client._post = unittest.mock.MagicMock(return_value=mock_response)
+
+        with self.assertRaises(MoodleClientError) as context:
+            client._wrapped_post(SERIALIZED_DATA)
+
+        self.assertEqual(context.exception.status_code, 400)
+        self.assertEqual(
+            context.exception.moodle_error.get("error_code"),
+            "missingparam"
+        )