feat: add a file upload endpoint to the discussions REST API. (#29067)
This commit is contained in:
committed by
GitHub
parent
d0f8d9ea35
commit
21a12a13e5
@@ -3,6 +3,18 @@ Discussion API permission logic
|
||||
"""
|
||||
from typing import Dict, Set, Union
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from rest_framework import permissions
|
||||
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
from common.djangoapps.student.roles import (
|
||||
CourseInstructorRole,
|
||||
CourseStaffRole,
|
||||
GlobalStaff,
|
||||
)
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import (
|
||||
has_discussion_privileges,
|
||||
)
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
|
||||
|
||||
@@ -118,3 +130,21 @@ def can_delete(cc_content, context):
|
||||
Return True if the requester can delete the given content, False otherwise
|
||||
"""
|
||||
return _is_author_or_privileged(cc_content, context)
|
||||
|
||||
|
||||
class IsStaffOrCourseTeamOrEnrolled(permissions.BasePermission):
|
||||
"""
|
||||
Permission that checks to see if the user is allowed to post or
|
||||
comment in the course.
|
||||
"""
|
||||
|
||||
def has_permission(self, request, view):
|
||||
"""Returns true if the user is enrolled or is staff."""
|
||||
course_key = CourseKey.from_string(view.kwargs.get('course_id'))
|
||||
return (
|
||||
GlobalStaff().has_user(request.user) or
|
||||
CourseStaffRole(course_key).has_user(request.user) or
|
||||
CourseInstructorRole(course_key).has_user(request.user) or
|
||||
CourseEnrollment.is_enrolled(request.user, course_key) or
|
||||
has_discussion_privileges(request.user, course_key)
|
||||
)
|
||||
|
||||
@@ -11,10 +11,12 @@ from urllib.parse import urlparse
|
||||
import ddt
|
||||
import httpretty
|
||||
from django.urls import reverse
|
||||
from django.core.files.uploadedfile import SimpleUploadedFile
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from pytz import UTC
|
||||
from rest_framework.parsers import JSONParser
|
||||
from rest_framework.test import APIClient, APITestCase
|
||||
from rest_framework import status
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
@@ -23,6 +25,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, chec
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
|
||||
from common.djangoapps.student.models import get_retired_username_by_username
|
||||
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff
|
||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, SuperuserFactory, UserFactory
|
||||
from common.djangoapps.util.testing import PatchMediaTypeMixin, UrlResetMixin
|
||||
from common.test.utils import disable_signal
|
||||
@@ -133,6 +136,153 @@ class DiscussionAPIViewTestMixin(ForumsEnableMixin, CommentsServiceMockMixin, Ur
|
||||
self.test_basic()
|
||||
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
class UploadFileViewTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestCase):
|
||||
"""
|
||||
Tests for UploadFileView.
|
||||
"""
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.valid_file = {
|
||||
"uploaded_file": SimpleUploadedFile(
|
||||
"test.jpg",
|
||||
b"test content",
|
||||
content_type="image/jpeg",
|
||||
),
|
||||
}
|
||||
self.user = UserFactory.create(password="password")
|
||||
self.course = CourseFactory.create(org='a', course='b', run='c', start=datetime.now(UTC))
|
||||
self.url = reverse("upload_file", kwargs={"course_id": str(self.course.id)})
|
||||
|
||||
def user_login(self):
|
||||
"""
|
||||
Authenticates the test client with the example user.
|
||||
"""
|
||||
self.client.login(username=self.user.username, password="password")
|
||||
|
||||
def enroll_user_in_course(self):
|
||||
"""
|
||||
Makes the example user enrolled to the course.
|
||||
"""
|
||||
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
|
||||
|
||||
def assert_upload_success(self, response):
|
||||
"""
|
||||
Asserts that the upload response was successful and returned the
|
||||
expected contents.
|
||||
"""
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.content_type == "application/json"
|
||||
response_data = json.loads(response.content)
|
||||
assert "location" in response_data
|
||||
|
||||
def test_file_upload_by_unauthenticated_user(self):
|
||||
"""
|
||||
Should fail if an unauthenticated user tries to upload a file.
|
||||
"""
|
||||
response = self.client.post(self.url, self.valid_file)
|
||||
assert response.status_code == status.HTTP_401_UNAUTHORIZED
|
||||
|
||||
def test_file_upload_by_unauthorized_user(self):
|
||||
"""
|
||||
Should fail if the user is not either staff or a student
|
||||
enrolled in the course.
|
||||
"""
|
||||
self.user_login()
|
||||
response = self.client.post(self.url, self.valid_file)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN
|
||||
|
||||
def test_file_upload_by_enrolled_user(self):
|
||||
"""
|
||||
Should succeed when a valid file is uploaded by an authenticated
|
||||
user who's enrolled in the course.
|
||||
"""
|
||||
self.user_login()
|
||||
self.enroll_user_in_course()
|
||||
response = self.client.post(self.url, self.valid_file)
|
||||
self.assert_upload_success(response)
|
||||
|
||||
def test_file_upload_by_global_staff(self):
|
||||
"""
|
||||
Should succeed when a valid file is uploaded by a global staff
|
||||
member.
|
||||
"""
|
||||
self.user_login()
|
||||
GlobalStaff().add_users(self.user)
|
||||
response = self.client.post(self.url, self.valid_file)
|
||||
self.assert_upload_success(response)
|
||||
|
||||
def test_file_upload_by_instructor(self):
|
||||
"""
|
||||
Should succeed when a valid file is uploaded by a course instructor.
|
||||
"""
|
||||
self.user_login()
|
||||
CourseInstructorRole(course_key=self.course.id).add_users(self.user)
|
||||
response = self.client.post(self.url, self.valid_file)
|
||||
self.assert_upload_success(response)
|
||||
|
||||
def test_file_upload_by_course_staff(self):
|
||||
"""
|
||||
Should succeed when a valid file is uploaded by a course staff
|
||||
member.
|
||||
"""
|
||||
self.user_login()
|
||||
CourseStaffRole(course_key=self.course.id).add_users(self.user)
|
||||
response = self.client.post(self.url, self.valid_file)
|
||||
self.assert_upload_success(response)
|
||||
|
||||
def test_file_upload_with_thread_key(self):
|
||||
"""
|
||||
Should contain the given thread_key in the uploaded file name.
|
||||
"""
|
||||
self.user_login()
|
||||
self.enroll_user_in_course()
|
||||
response = self.client.post(self.url, {
|
||||
**self.valid_file,
|
||||
"thread_key": "somethread",
|
||||
})
|
||||
response_data = json.loads(response.content)
|
||||
assert "/somethread/" in response_data["location"]
|
||||
|
||||
def test_file_upload_with_invalid_file(self):
|
||||
"""
|
||||
Should fail if the uploaded file format is not allowed.
|
||||
"""
|
||||
self.user_login()
|
||||
self.enroll_user_in_course()
|
||||
invalid_file = {
|
||||
"uploaded_file": SimpleUploadedFile(
|
||||
"test.txt",
|
||||
b"test content",
|
||||
content_type="text/plain",
|
||||
),
|
||||
}
|
||||
response = self.client.post(self.url, invalid_file)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN
|
||||
|
||||
def test_file_upload_with_invalid_course_id(self):
|
||||
"""
|
||||
Should fail if the course does not exist.
|
||||
"""
|
||||
self.user_login()
|
||||
self.enroll_user_in_course()
|
||||
url = reverse("upload_file", kwargs={"course_id": "d/e/f"})
|
||||
response = self.client.post(url, self.valid_file)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN
|
||||
|
||||
def test_file_upload_with_no_data(self):
|
||||
"""
|
||||
Should fail when the user sends a request missing an
|
||||
`uploaded_file` field.
|
||||
"""
|
||||
self.user_login()
|
||||
self.enroll_user_in_course()
|
||||
response = self.client.post(self.url, data={})
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
|
||||
"""Tests for CourseView"""
|
||||
|
||||
@@ -16,7 +16,8 @@ from lms.djangoapps.discussion.rest_api.views import (
|
||||
CourseView,
|
||||
ReplaceUsernamesView,
|
||||
RetireUserView,
|
||||
ThreadViewSet
|
||||
ThreadViewSet,
|
||||
UploadFileView,
|
||||
)
|
||||
|
||||
ROUTER = SimpleRouter()
|
||||
@@ -31,6 +32,11 @@ urlpatterns = [
|
||||
CourseDiscussionSettingsAPIView.as_view(),
|
||||
name="discussion_course_settings",
|
||||
),
|
||||
url(
|
||||
fr"^v1/courses/{settings.COURSE_ID_PATTERN}/upload$",
|
||||
UploadFileView.as_view(),
|
||||
name="upload_file",
|
||||
),
|
||||
url(
|
||||
r"^v1/courses/{}/roles/(?P<rolename>[A-Za-z0-9+ _-]+)/?$".format(
|
||||
settings.COURSE_ID_PATTERN
|
||||
|
||||
@@ -4,19 +4,24 @@ Discussion API views
|
||||
|
||||
|
||||
import logging
|
||||
import uuid
|
||||
|
||||
from django.core.exceptions import BadRequest
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.exceptions import ValidationError
|
||||
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
||||
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from rest_framework import permissions, status
|
||||
from rest_framework.authentication import SessionAuthentication
|
||||
from rest_framework.exceptions import ParseError, UnsupportedMediaType
|
||||
from rest_framework.parsers import JSONParser
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.views import APIView
|
||||
from rest_framework.viewsets import ViewSet
|
||||
|
||||
from common.djangoapps.util.file import store_uploaded_file
|
||||
from lms.djangoapps.discussion.django_comment_client import settings as cc_settings
|
||||
from lms.djangoapps.course_goals.models import UserActivity
|
||||
from lms.djangoapps.instructor.access import update_forum_role
|
||||
from openedx.core.djangoapps.discussions.serializers import DiscussionSettingsSerializer
|
||||
@@ -24,7 +29,7 @@ from openedx.core.djangoapps.django_comment_common import comment_client
|
||||
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role
|
||||
from openedx.core.djangoapps.user_api.accounts.permissions import CanReplaceUsername, CanRetireUser
|
||||
from openedx.core.djangoapps.user_api.models import UserRetirementStatus
|
||||
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
|
||||
from openedx.core.lib.api.authentication import BearerAuthentication, BearerAuthenticationAllowInactiveUser
|
||||
from openedx.core.lib.api.parsers import MergePatchParser
|
||||
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
|
||||
from xmodule.modulestore.django import modulestore
|
||||
@@ -53,6 +58,7 @@ from ..rest_api.serializers import (
|
||||
DiscussionRolesListSerializer,
|
||||
DiscussionRolesSerializer,
|
||||
)
|
||||
from ..rest_api.permissions import IsStaffOrCourseTeamOrEnrolled
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -94,6 +100,7 @@ class CourseView(DeveloperErrorViewMixin, APIView):
|
||||
* allow_anonymous_to_peers: A boolean which indicating whether posts
|
||||
anonymous to peers are allowed or not.
|
||||
"""
|
||||
|
||||
def get(self, request, course_id):
|
||||
"""Implements the GET method as described in the class docstring."""
|
||||
course_key = CourseKey.from_string(course_id) # TODO: which class is right?
|
||||
@@ -129,6 +136,7 @@ class CourseTopicsView(DeveloperErrorViewMixin, APIView):
|
||||
* non_courseware_topics: The list of topic trees that are not linked to
|
||||
courseware. Items are of the same format as in courseware_topics.
|
||||
"""
|
||||
|
||||
def get(self, request, course_id):
|
||||
"""
|
||||
Implements the GET method as described in the class docstring.
|
||||
@@ -607,6 +615,79 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
|
||||
return Response(update_comment(request, comment_id, request.data))
|
||||
|
||||
|
||||
class UploadFileView(DeveloperErrorViewMixin, APIView):
|
||||
"""
|
||||
**Use Cases**
|
||||
|
||||
Upload a file to be attached to a thread or comment.
|
||||
|
||||
**URL Parameters**
|
||||
|
||||
* course_id:
|
||||
The ID of the course where this thread or comment belongs.
|
||||
|
||||
**POST Upload File Parameters**
|
||||
|
||||
* thread_key:
|
||||
If the upload belongs to a comment, refer to the parent
|
||||
`thread_id`, otherwise it should be `"root"`.
|
||||
|
||||
**Example Requests**:
|
||||
POST /api/discussion/v1/courses/{course_id}/upload/
|
||||
Content-Type: multipart/form-data; boundary=--Boundary
|
||||
|
||||
----Boundary
|
||||
Content-Disposition: form-data; name="thread_key"
|
||||
|
||||
<thread_key>
|
||||
----Boundary
|
||||
Content-Disposition: form-data; name="uploaded_file"; filename="<filename>.<ext>"
|
||||
Content-Type: <mimetype>
|
||||
|
||||
<file_content>
|
||||
----Boundary--
|
||||
|
||||
**Response Values**
|
||||
|
||||
* location: The URL to access the uploaded file.
|
||||
"""
|
||||
|
||||
authentication_classes = (
|
||||
JwtAuthentication,
|
||||
BearerAuthentication,
|
||||
SessionAuthentication,
|
||||
)
|
||||
permission_classes = (
|
||||
permissions.IsAuthenticated,
|
||||
IsStaffOrCourseTeamOrEnrolled,
|
||||
)
|
||||
|
||||
def post(self, request, course_id):
|
||||
"""
|
||||
Handles a file upload.
|
||||
"""
|
||||
thread_key = request.POST.get("thread_key", "root")
|
||||
unique_file_name = f"{course_id}/{thread_key}/{uuid.uuid4()}"
|
||||
try:
|
||||
file_storage, stored_file_name = store_uploaded_file(
|
||||
request, "uploaded_file", cc_settings.ALLOWED_UPLOAD_FILE_TYPES,
|
||||
unique_file_name, max_file_size=cc_settings.MAX_UPLOAD_FILE_SIZE,
|
||||
)
|
||||
except ValueError:
|
||||
raise BadRequest("no `uploaded_file` was provided") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
|
||||
file_absolute_url = file_storage.url(stored_file_name)
|
||||
|
||||
# this is a no-op in production, but is required in development,
|
||||
# since the filesystem storage returns the path without a base_url
|
||||
file_absolute_url = request.build_absolute_uri(file_absolute_url)
|
||||
|
||||
return Response(
|
||||
{"location": file_absolute_url},
|
||||
content_type="application/json",
|
||||
)
|
||||
|
||||
|
||||
class RetireUserView(APIView):
|
||||
"""
|
||||
**Use Cases**
|
||||
|
||||
Reference in New Issue
Block a user