Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Open edX Learning ("Learning Core").
"""
__version__ = "0.3.2"
__version__ = "0.3.3"
29 changes: 29 additions & 0 deletions openedx_tagging/core/tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from rest_framework.reverse import reverse

from openedx_tagging.core.tagging.data import TagData
from openedx_tagging.core.tagging.import_export.parsers import ParserFormat
from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy


Expand Down Expand Up @@ -216,3 +217,31 @@ class TaxonomyTagDeleteBodySerializer(serializers.Serializer): # pylint: disabl
child=serializers.CharField(), required=True
)
with_subtags = serializers.BooleanField(required=False)


class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=abstract-method
"""
Serializer of the body for the Taxonomy Import request
"""
file = serializers.FileField(required=True)

def validate(self, attrs):
"""
Validates the file extension and add parser_format to the data
"""
filename = attrs["file"].name
ext = filename.split('.')[-1]
parser_format = getattr(ParserFormat, ext.upper(), None)
if not parser_format:
raise serializers.ValidationError({"file": f'File type not supported: {ext.lower()}'}, 'file')

attrs['parser_format'] = parser_format
return attrs


class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint: disable=abstract-method
"""
Serializer of the body for the Taxonomy Create and Import request
"""
taxonomy_name = serializers.CharField(required=True)
taxonomy_description = serializers.CharField(default="")
83 changes: 78 additions & 5 deletions openedx_tagging/core/tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from rest_framework.decorators import action
from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError
from rest_framework.generics import ListAPIView, RetrieveUpdateDestroyAPIView
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.viewsets import GenericViewSet, ModelViewSet

Expand All @@ -26,7 +27,7 @@
update_tag_in_taxonomy,
)
from ...data import TagDataQuerySet
from ...import_export.api import export_tags
from ...import_export.api import export_tags, get_last_import_log, import_tags
from ...import_export.parsers import ParserFormat
from ...models import ObjectTag, Taxonomy
from ...rules import ObjectTagPermissionItem
Expand All @@ -40,6 +41,8 @@
ObjectTagUpdateQueryParamsSerializer,
TagDataSerializer,
TaxonomyExportQueryParamsSerializer,
TaxonomyImportBodySerializer,
TaxonomyImportNewBodySerializer,
TaxonomyListQueryParamsSerializer,
TaxonomySerializer,
TaxonomyTagCreateBodySerializer,
Expand All @@ -52,7 +55,7 @@
@view_auth_classes
class TaxonomyView(ModelViewSet):
"""
View to list, create, retrieve, update, delete or export Taxonomies.
View to list, create, retrieve, update, delete, export or import Taxonomies.

**List Query Parameters**
* enabled (optional) - Filter by enabled status. Valid values: true,
Expand Down Expand Up @@ -167,7 +170,29 @@ class TaxonomyView(ModelViewSet):
* 400 - Invalid query parameter
* 403 - Permission denied

**Import/Create Taxonomy Example Requests**
POST /tagging/rest_api/v1/taxonomy/import/
{
"taxonomy_name": "Taxonomy Name",
"taxonomy_description": "This is a description",
"file": <file>,
}

**Import/Create Taxonomy Query Returns**
* 200 - Success
* 400 - Bad request
* 403 - Permission denied

**Import/Update Taxonomy Example Requests**
PUT /tagging/rest_api/v1/taxonomy/:pk/tags/import/
{
"file": <file>,
}

**Import/Update Taxonomy Query Returns**
* 200 - Success
* 400 - Bad request
* 403 - Permission denied
"""

lookup_value_regex = r"\d+"
Expand Down Expand Up @@ -215,9 +240,6 @@ def export(self, request, **_kwargs) -> HttpResponse:
Export a taxonomy.
"""
taxonomy = self.get_object()
perm = "oel_tagging.export_taxonomy"
if not request.user.has_perm(perm, taxonomy):
raise PermissionDenied("You do not have permission to export this taxonomy.")
Copy link
Contributor

@bradenmacdonald bradenmacdonald Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that "if you can view the taxonomy, you can export it", but are we still checking that the user has "view" permission here? I don't see it. (Though it looks like it's covered by the tests. I'm just wondering where that check is happening).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query_params = TaxonomyExportQueryParamsSerializer(
data=request.query_params.dict()
)
Expand All @@ -242,6 +264,57 @@ def export(self, request, **_kwargs) -> HttpResponse:

return HttpResponse(tags, content_type=content_type)

@action(detail=False, url_path="import", methods=["post"])
def create_import(self, request: Request, **_kwargs) -> Response:
"""
Creates a new taxonomy and imports the tags from the uploaded file.
"""
body = TaxonomyImportNewBodySerializer(data=request.data)
body.is_valid(raise_exception=True)

taxonomy_name = body.validated_data["taxonomy_name"]
taxonomy_description = body.validated_data["taxonomy_description"]
file = body.validated_data["file"].file
parser_format = body.validated_data["parser_format"]

taxonomy = create_taxonomy(taxonomy_name, taxonomy_description)
try:
import_success = import_tags(taxonomy, file, parser_format)

if import_success:
serializer = self.get_serializer(taxonomy)
return Response(serializer.data, status=status.HTTP_201_CREATED)
else:
import_error = get_last_import_log(taxonomy)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. This works fine in a serial environment, but if we've got multiple imports happening at the same time, this won't necessarily be the import log we're interested in here. Really, the only way to get that is if we have the task ID after import.

But I don't know if it's worth refactoring import_tags to support this, so: could you add a comment here to warn us? Then we'll know where to look if users report problems.

Copy link
Contributor Author

@rpenido rpenido Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not as alarming as I made it sound:
1- It only happens on update. If we do a mass import/create we will have new ids, so no problem.
2- I did another look at the code and realized that we prevent multiple imports of the same taxonomy simultaneously. So, the concurrent problem may not occur at all, right?

taxonomy.delete()
return Response(import_error, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
return Response(str(e), status=status.HTTP_400_BAD_REQUEST)

@action(detail=True, url_path="tags/import", methods=["put"])
def update_import(self, request: Request, **_kwargs) -> Response:
"""
Imports tags from the uploaded file to an already created taxonomy.
"""
body = TaxonomyImportBodySerializer(data=request.data)
body.is_valid(raise_exception=True)

file = body.validated_data["file"].file
parser_format = body.validated_data["parser_format"]

taxonomy = self.get_object()
try:
import_success = import_tags(taxonomy, file, parser_format)

if import_success:
serializer = self.get_serializer(taxonomy)
return Response(serializer.data)
else:
import_error = get_last_import_log(taxonomy)
return Response(import_error, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
return Response(str(e), status=status.HTTP_400_BAD_REQUEST)


@view_auth_classes
class ObjectTagView(
Expand Down
1 change: 0 additions & 1 deletion openedx_tagging/core/tagging/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ def can_change_object_tag(
rules.add_perm("oel_tagging.change_taxonomy", can_change_taxonomy)
rules.add_perm("oel_tagging.delete_taxonomy", can_change_taxonomy)
rules.add_perm("oel_tagging.view_taxonomy", can_view_taxonomy)
rules.add_perm("oel_tagging.export_taxonomy", can_view_taxonomy)

# Tag
rules.add_perm("oel_tagging.add_tag", can_change_tag)
Expand Down
Loading