Skip to content

Commit

Permalink
fix: [EDXOLDMNG-271] fix config creation without singnatories
Browse files Browse the repository at this point in the history
  • Loading branch information
NiedielnitsevIvan committed Jan 3, 2023
1 parent aa77ef9 commit 9b9ac0e
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 23 deletions.
39 changes: 24 additions & 15 deletions credentials/apps/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
"""
import ast
import logging
from copy import copy
from pathlib import Path
from typing import Dict, Generator, List, Union
from typing import Dict, Generator, List, Optional, Union

from django.core.exceptions import ObjectDoesNotExist
from django.core.files.uploadedfile import InMemoryUploadedFile
Expand Down Expand Up @@ -321,29 +322,35 @@ class SignatoryListField(serializers.ListField):
Populate signatories images in dict files instead of strings with their filenames.
"""

def to_internal_value(self, data: str) -> Dict[str, Union[str, InMemoryUploadedFile]]:
def to_representation(self, data):
return super().to_representation(data.all())

def to_internal_value(self, data: List[str]) -> List[Optional[Dict[str, Union[str, InMemoryUploadedFile]]]]:
"""
Converts a list of dict that is a string to a list of dict.
Converts a list of dict that are a string to a list of dicts.
"""
data_ = self.populate_signatories(data, self.context["request"].FILES) # pylint: disable=no-member
return super().to_internal_value(data_)
try:
jsonified_data = list(map(ast.literal_eval, copy(data)))
except ValueError:
return []
else:
_data = self.populate_signatories(
jsonified_data, self.context["request"].FILES # pylint: disable=no-member
)
return super().to_internal_value(_data)

@staticmethod
def populate_signatories(
signatories_data: List[str],
signatories_data: List[Dict[str, str]],
files: MultiValueDict,
) -> Generator[Dict[str, Union[str, InMemoryUploadedFile]], None, None]:
"""
Populates a list of signature data with files.
"""
for signatory_data in signatories_data:
signatory = ast.literal_eval(signatory_data)
if signatory_file := files.get(signatory.get("image"), None):
signatory["image"] = signatory_file
yield signatory

def to_representation(self, data):
return super().to_representation(data.all())
if signatory_file := files.get(signatory_data.get("image"), None):
signatory_data["image"] = signatory_file
yield signatory_data


class CourseCertificateSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -393,8 +400,10 @@ def create(self, validated_data) -> CourseCertificate:
},
)

cert.signatories.clear()
for signatory_data in validated_data.get("signatories", []):
signatories_data = validated_data.get("signatories", [])
if signatories_data:
cert.signatories.clear()
for signatory_data in signatories_data:
signatory = Signatory.objects.create(**signatory_data)
cert.signatories.add(signatory)

Expand Down
8 changes: 4 additions & 4 deletions credentials/apps/api/v2/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,18 +340,18 @@ class SignatoryListFieldTests(SiteMixin, TestCase):
def test_populate_signatories(self):
png_image = create_image("png")
signatories_data = [
"""{
{
"image": "/asset-v1:edX+DemoX+Demo_Course+type@asset+block@images_course_image.png",
"name": "signatory 1",
"organization": "edX",
"title": "title",
}""",
"""{
},
{
"image": "/asset-v1:edX+DemoX+Demo_Course+type@asset+block@images_course_image+1.png",
"name": "signatory 2",
"organization": "edX",
"title": "title",
}""",
},
]
files = {
"/asset-v1:edX+DemoX+Demo_Course+type@asset+block@images_course_image.png": png_image,
Expand Down
16 changes: 16 additions & 0 deletions credentials/apps/api/v2/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,22 @@ def test_post_for_staff(self):
response = self.client.post(self.url, data=self.valid_data)
self.assertEqual(response.status_code, 201)

def test_post_for_staff_with_blank_signatories(self):
self.authenticate_user(self.superuser)
data = {
"course_id": "course-v1:edX+DemoX+Demo_Course",
"certificate_type": "honor",
"title": "Name of the certificate",
# NOTE: In tests, parsers do not work properly, unlike real queries.
# It is in this form that data comes from the studio when creating
# a certificate configuration without signatures.
# This is the case for this test.
"signatories": ["image", "name", "organization", "title"],
"is_active": True,
}
response = self.client.post(self.url, data=data, files={})
self.assertEqual(response.status_code, 201)

def test_delete_for_anonymous(self):
data = {
"course_id": self.certificate.course_id,
Expand Down
19 changes: 15 additions & 4 deletions credentials/apps/api/v2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
from django.db.models import Q
from django.shortcuts import get_object_or_404
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from rest_framework import generics, mixins, permissions, status, viewsets
from rest_framework import mixins, permissions, status, viewsets
from rest_framework.exceptions import Throttled
from rest_framework.parsers import FormParser, MultiPartParser
from rest_framework.parsers import FormParser, JSONParser, MultiPartParser
from rest_framework.response import Response
from rest_framework.throttling import ScopedRateThrottle
from rest_framework.views import APIView, exception_handler
Expand Down Expand Up @@ -286,14 +286,19 @@ def _replace_username_for_all_models(self, current_username, new_username, repla

class CourseCertificateViewSet(
mixins.CreateModelMixin,
generics.RetrieveDestroyAPIView,
mixins.DestroyModelMixin,
mixins.RetrieveModelMixin,
mixins.UpdateModelMixin,
viewsets.GenericViewSet,
):
queryset = CourseCertificate.objects.all()
serializer_class = CourseCertificateSerializer
permission_classes = (IsAdminUserOrReadOnly,)
parser_classes = (MultiPartParser, FormParser)
parser_classes = (
JSONParser,
MultiPartParser,
FormParser,
)
lookup_field = "course_id"

def get_object(self) -> CourseCertificate:
Expand All @@ -305,6 +310,12 @@ def get_object(self) -> CourseCertificate:
certificate_type=data.get("certificate_type"),
)

def delete(self, request, *args, **kwargs):
return self.destroy(request, *args, **kwargs)

def get(self, request, *args, **kwargs):
return self.retrieve(request, *args, **kwargs)

def perform_destroy(self, instance: CourseCertificate) -> None:
instance.signatories.clear()
instance.delete()

0 comments on commit 9b9ac0e

Please sign in to comment.