Skip to content

Commit

Permalink
🛂(backend) use playlist token in deposit apis
Browse files Browse the repository at this point in the history
As we are now using playlist tokens, it has to be allowed on apis.
Also, old routes can't be used anymore, as token resource is now a
playlist.
  • Loading branch information
kernicPanel committed Aug 7, 2023
1 parent ac2d778 commit 507876b
Show file tree
Hide file tree
Showing 15 changed files with 154 additions and 118 deletions.
104 changes: 55 additions & 49 deletions src/backend/marsha/deposit/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@
)


class ObjectFileDepositoryRelatedMixin:
"""
Get the related depository id contained in resource.
It exposes a function used to get the related depository.
"""

def get_related_filedepository_id(self):
"""Get the related file depository ID from the request."""

# The file depository ID in the URL is mandatory.
return self.kwargs.get("filedepository_id")


class FileDepositoryFilter(django_filters.FilterSet):
"""Filter for file depository."""

Expand Down Expand Up @@ -93,7 +107,7 @@ def get_permissions(self):
]
elif self.action in ["retrieve"]:
permission_classes = [
core_permissions.IsTokenResourceRouteObject
core_permissions.IsPlaylistToken
& (
core_permissions.IsTokenInstructor
| core_permissions.IsTokenAdmin
Expand All @@ -104,7 +118,7 @@ def get_permissions(self):
elif self.action in ["update", "partial_update", "destroy"]:
permission_classes = [
(
core_permissions.IsTokenResourceRouteObject
core_permissions.IsPlaylistToken
& (
core_permissions.IsTokenInstructor
| core_permissions.IsTokenAdmin
Expand Down Expand Up @@ -195,57 +209,14 @@ def lti_select(self, request):
}
)

@action(
methods=["get"],
detail=True,
url_path="depositedfiles",
permission_classes=[
core_permissions.IsTokenInstructor
| core_permissions.IsTokenAdmin
| core_permissions.IsTokenStudent
| IsFileDepositoryPlaylistOrOrganizationAdmin
],
)
# pylint: disable=unused-argument
def depositedfiles(self, request, pk=None):
"""Get deposited files from a file_depository.
Calling the endpoint returns a list of deposited files.
Parameters
----------
request : Type[django.http.request.HttpRequest]
The request on the API endpoint
pk : int
The primary key of the file_depository
Returns
-------
Type[rest_framework.response.Response]
HttpResponse carrying deposited files as a JSON object.
"""
file_depository = self.get_object()
queryset = file_depository.deposited_files.all()
if request.resource and any(
x in LTI_ROLES.get(STUDENT) for x in request.resource.roles
):
queryset = queryset.filter(author_id=request.resource.user.get("id"))
filter_set = DepositedFileFilter(request.query_params, queryset=queryset)
page = self.paginate_queryset(filter_set.qs)
serializer = serializers.DepositedFileSerializer(
page,
many=True,
context={"request": self.request},
)
return self.get_paginated_response(serializer.data)


class DepositedFileViewSet(
APIViewMixin,
ObjectPkMixin,
ObjectRelatedMixin,
ObjectFileDepositoryRelatedMixin,
mixins.CreateModelMixin,
mixins.ListModelMixin,
mixins.UpdateModelMixin,
mixins.DestroyModelMixin,
viewsets.GenericViewSet,
Expand All @@ -272,7 +243,7 @@ def get_permissions(self):
| core_permissions.IsTokenStudent
| core_permissions.UserIsAuthenticated
]
elif self.action in ["create", "list", "metadata"]:
elif self.action in ["list", "metadata"]:
permission_classes = [
core_permissions.IsTokenInstructor
| core_permissions.IsTokenAdmin
Expand All @@ -289,9 +260,44 @@ def get_permissions(self):
permission_classes = self.permission_classes
return [permission() for permission in permission_classes]

def list(self, request, *args, **kwargs):
"""Get a list of deposited files.
Calling the endpoint returns a list of deposited files.
Parameters
----------
request : Type[django.http.request.HttpRequest]
The request on the API endpoint
Returns
-------
Type[rest_framework.response.Response]
HttpResponse carrying deposited files as a JSON object.
"""
queryset = self.filter_queryset(self.get_queryset())
if request.resource and any(
x in LTI_ROLES.get(STUDENT) for x in request.resource.roles
):
queryset = queryset.filter(author_id=request.resource.user.get("id"))
filter_set = DepositedFileFilter(request.query_params, queryset=queryset)
page = self.paginate_queryset(filter_set.qs)
serializer = serializers.DepositedFileSerializer(
page,
many=True,
context={"request": self.request},
)
return self.get_paginated_response(serializer.data)

@action(methods=["post"], detail=True, url_path="initiate-upload")
# pylint: disable=unused-argument
def initiate_upload(self, request, pk=None):
def initiate_upload(
self,
request,
pk=None,
filedepository_id=None,
):
"""Get an upload policy for a deposited file.
Calling the endpoint resets the upload state to `pending` and returns an upload policy to
Expand Down
18 changes: 6 additions & 12 deletions src/backend/marsha/deposit/permissions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
"""Custom permission classes for the Deposit app."""
from django.core.exceptions import ObjectDoesNotExist

from rest_framework import permissions

from marsha.core import models
from marsha.deposit.models import FileDepository


def _is_organization_admin(user_id, file_depository_id):
Expand Down Expand Up @@ -59,12 +58,10 @@ def has_permission(self, request, view):
"""
if not request.resource:
return False
try:
return (
str(view.get_related_object().file_depository.id) == request.resource.id
)
except ObjectDoesNotExist:
return False

return FileDepository.objects.filter(
pk=view.get_related_filedepository_id(), playlist_id=request.resource.id
).exists()


class IsFileDepositoryPlaylistOrOrganizationAdmin(permissions.BasePermission):
Expand Down Expand Up @@ -108,10 +105,7 @@ def has_permission(self, request, view):
which exists, and if the current user is an admin for the playlist this file depository
is a part of or admin of the linked organization.
"""
try:
file_depository_id = view.get_related_object().file_depository.id
except (AttributeError, ObjectDoesNotExist):
file_depository_id = request.data.get("file_depository")
file_depository_id = view.get_related_filedepository_id()

if not file_depository_id:
return False
Expand Down
7 changes: 2 additions & 5 deletions src/backend/marsha/deposit/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,10 @@ def create(self, validated_data):
"""
resource = self.context["request"].resource
user = self.context["request"].user
file_depository_id = self.context["request"].data.get("file_depository")
file_depository_id = self.context["view"].get_related_filedepository_id()

if not validated_data.get("file_depository_id"):
if resource:
validated_data["file_depository_id"] = resource.id
elif file_depository_id:
validated_data["file_depository_id"] = file_depository_id
validated_data["file_depository_id"] = file_depository_id

if resource:
validated_data["author_id"] = resource.user.get("id")
Expand Down
22 changes: 13 additions & 9 deletions src/backend/marsha/deposit/tests/api/depositedfiles/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ def test_api_deposited_file_create_student_with_user_fullname(self):
"""

file_depository = FileDepositoryFactory()
jwt_token = StudentLtiTokenFactory(resource=file_depository)
jwt_token = StudentLtiTokenFactory(resource=file_depository.playlist)

response = self.client.post(
"/api/depositedfiles/",
f"/api/filedepositories/{file_depository.id}/depositedfiles/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
data=json.dumps(
Expand Down Expand Up @@ -89,11 +89,13 @@ def test_api_deposited_file_create_student_with_username(self):

file_depository = FileDepositoryFactory()
jwt_token = StudentLtiTokenFactory(
resource=file_depository, user__user_fullname=None, user__username="student"
resource=file_depository.playlist,
user__user_fullname=None,
user__username="student",
)

response = self.client.post(
"/api/depositedfiles/",
f"/api/filedepositories/{file_depository.id}/depositedfiles/",
data=json.dumps(
{
"size": 123,
Expand Down Expand Up @@ -121,11 +123,13 @@ def test_api_deposited_file_create_student_without_username(self):

file_depository = FileDepositoryFactory()
jwt_token = StudentLtiTokenFactory(
resource=file_depository, user__user_fullname=None, user__username=None
resource=file_depository.playlist,
user__user_fullname=None,
user__username=None,
)

response = self.client.post(
"/api/depositedfiles/",
f"/api/filedepositories/{file_depository.id}/depositedfiles/",
data=json.dumps(
{
"size": 123,
Expand Down Expand Up @@ -153,7 +157,7 @@ def test_api_deposited_file_create_user_access_token(self):
jwt_token = UserAccessTokenFactory(user=organization_access.user)

response = self.client.post(
"/api/depositedfiles/",
f"/api/filedepositories/{file_depository.id}/depositedfiles/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
data=json.dumps(
Expand Down Expand Up @@ -197,7 +201,7 @@ def test_api_deposited_file_create_user_access_token_organization_admin(self):
jwt_token = UserAccessTokenFactory(user=organization_access.user)

response = self.client.post(
"/api/depositedfiles/",
f"/api/filedepositories/{file_depository.id}/depositedfiles/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
data=json.dumps(
Expand Down Expand Up @@ -240,7 +244,7 @@ def test_api_deposited_file_create_user_access_token_playlist_admin(self):
jwt_token = UserAccessTokenFactory(user=playlist_access.user)

response = self.client.post(
"/api/depositedfiles/",
f"/api/filedepositories/{file_depository.id}/depositedfiles/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
data=json.dumps(
Expand Down
15 changes: 10 additions & 5 deletions src/backend/marsha/deposit/tests/api/depositedfiles/test_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def test_api_deposited_file_delete_student(self):

self.assertEqual(DepositedFile.objects.count(), 1)
response = self.client.delete(
f"/api/depositedfiles/{deposited_file.id}/",
f"/api/filedepositories/{deposited_file.file_depository.id}"
f"/depositedfiles/{deposited_file.id}/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
)
Expand All @@ -49,7 +50,8 @@ def test_api_deposited_file_delete_instructor(self):

self.assertEqual(DepositedFile.objects.count(), 1)
response = self.client.delete(
f"/api/depositedfiles/{deposited_file.id!s}/",
f"/api/filedepositories/{deposited_file.file_depository.id}"
f"/depositedfiles/{deposited_file.id!s}/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
)
Expand All @@ -66,7 +68,8 @@ def test_api_deposited_file_delete_user_access_token(self):

self.assertEqual(DepositedFile.objects.count(), 1)
response = self.client.delete(
f"/api/depositedfiles/{deposited_file.id}/",
f"/api/filedepositories/{deposited_file.file_depository.id}"
f"/depositedfiles/{deposited_file.id}/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
)
Expand All @@ -82,7 +85,8 @@ def test_api_deposited_file_delete_user_access_token_organization_admin(self):

self.assertEqual(DepositedFile.objects.count(), 1)
response = self.client.delete(
f"/api/depositedfiles/{deposited_file.id!s}/",
f"/api/filedepositories/{deposited_file.file_depository.id}"
f"/depositedfiles/{deposited_file.id!s}/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
)
Expand All @@ -100,7 +104,8 @@ def test_api_deposited_file_delete_user_access_token_playlist_admin(self):

self.assertEqual(DepositedFile.objects.count(), 1)
response = self.client.delete(
f"/api/depositedfiles/{deposited_file.id!s}/",
f"/api/filedepositories/{deposited_file.file_depository.id}"
f"/depositedfiles/{deposited_file.id!s}/",
HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
content_type="application/json",
)
Expand Down
Loading

0 comments on commit 507876b

Please sign in to comment.