From ba4bcf115a9e1da3d03ee830b9bf2ca56532fa66 Mon Sep 17 00:00:00 2001
From: Dmytro <98233552+DmytroAlipov@users.noreply.github.com>
Date: Wed, 7 Feb 2024 14:47:18 +0100
Subject: [PATCH] fix: ORA response with attached file (#33676)

The details are described in this discussion: https://discuss.openedx.org/t/ora-grading-returns-error/11482
---
 .../ora_staff_grader/serializers.py           | 10 +++-
 .../tests/test_serializers.py                 | 47 +++++++++++++++----
 2 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/lms/djangoapps/ora_staff_grader/serializers.py b/lms/djangoapps/ora_staff_grader/serializers.py
index 643a4725e1a0..d3ffb46f1933 100644
--- a/lms/djangoapps/ora_staff_grader/serializers.py
+++ b/lms/djangoapps/ora_staff_grader/serializers.py
@@ -4,6 +4,8 @@
 # pylint: disable=abstract-method
 # pylint: disable=missing-function-docstring
 
+from urllib.parse import urljoin
+from django.conf import settings
 from rest_framework import serializers
 
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
@@ -191,11 +193,17 @@ def get_isEnabled(self, obj):
 class UploadedFileSerializer(serializers.Serializer):
     """Serializer for a file uploaded as a part of a response"""
 
-    downloadUrl = serializers.URLField(source="download_url")
+    downloadUrl = serializers.SerializerMethodField(method_name="get_download_url")
     description = serializers.CharField()
     name = serializers.CharField()
     size = serializers.IntegerField()
 
+    def get_download_url(self, obj):
+        """
+        Get the representation for SerializerMethodField `downloadUrl`
+        """
+        return urljoin(settings.LMS_ROOT_URL, obj.get("download_url"))
+
 
 class ResponseSerializer(serializers.Serializer):
     """Serializer for the responseData api construct, which represents the contents of a submitted learner response"""
diff --git a/lms/djangoapps/ora_staff_grader/tests/test_serializers.py b/lms/djangoapps/ora_staff_grader/tests/test_serializers.py
index 19939c0f9b85..d105cd649e89 100644
--- a/lms/djangoapps/ora_staff_grader/tests/test_serializers.py
+++ b/lms/djangoapps/ora_staff_grader/tests/test_serializers.py
@@ -2,8 +2,10 @@
 Tests for ESG Serializers
 """
 from unittest.mock import Mock, MagicMock, patch
+from urllib.parse import urljoin
 
 import ddt
+from django.conf import settings
 from django.test import TestCase
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 
@@ -457,14 +459,37 @@ class TestUploadedFileSerializer(TestCase):
 
     def test_uploaded_file_serializer(self):
         """Base serialization behavior"""
-        input_data = MagicMock(size=89794)
+        input_data = {
+            "download_url": "/test.txt",
+            "description": "Test description",
+            "name": "Test name",
+            "size": 89111,
+        }
         data = UploadedFileSerializer(input_data).data
 
         expected_value = {
-            "downloadUrl": str(input_data.download_url),
-            "description": str(input_data.description),
-            "name": str(input_data.name),
-            "size": input_data.size,
+            "downloadUrl": f'{settings.LMS_ROOT_URL}{input_data["download_url"]}',
+            "description": input_data["description"],
+            "name": input_data["name"],
+            "size": input_data["size"],
+        }
+        assert data == expected_value
+
+    def test_uploaded_file_serializer_with_full_url(self):
+        """Test UploadedFileSerializer with a full download URL"""
+        input_data = {
+            "download_url": f"{settings.LMS_ROOT_URL}/test.txt",
+            "description": "Test description",
+            "name": "Test name",
+            "size": 78222,
+        }
+        data = UploadedFileSerializer(input_data).data
+
+        expected_value = {
+            "downloadUrl": input_data["download_url"],
+            "description": input_data["description"],
+            "name": input_data["name"],
+            "size": input_data["size"],
         }
         assert data == expected_value
 
@@ -484,7 +509,11 @@ def test_response_serializer(self, has_text, has_files):
         """Base serialization behavior"""
         input_data = MagicMock()
         if has_files:
-            input_data.files = [Mock(size=111), Mock(size=222), Mock(size=333)]
+            input_data.files = [
+                {"size": 111, "download_url": "/file1.txt", "description": Mock(), "name": Mock()},
+                {"size": 222, "download_url": "/file2.txt", "description": Mock(), "name": Mock()},
+                {"size": 333, "download_url": "/file3.txt", "description": Mock(), "name": Mock()},
+            ]
         if has_text:
             input_data.text = [Mock(), Mock(), Mock()]
 
@@ -520,12 +549,12 @@ def test_file_list_serializer(self):
             "files": [{
                 "name": Mock(),
                 "description": Mock(),
-                "download_url": Mock(),
+                "download_url": f"{settings.LMS_ROOT_URL}/test-1.png",
                 "size": 12345,
             }, {
                 "name": Mock(),
                 "description": Mock(),
-                "download_url": Mock(),
+                "download_url": "/test-2.png",
                 "size": 54321,
             }],
             "text": "",
@@ -540,7 +569,7 @@ def test_file_list_serializer(self):
 
             assert output_file["name"] == str(input_file["name"])
             assert output_file["description"] == str(input_file["description"])
-            assert output_file["downloadUrl"] == str(input_file["download_url"])
+            assert output_file["downloadUrl"] == urljoin(settings.LMS_ROOT_URL, str(input_file["download_url"]))
             assert output_file["size"] == input_file["size"]