Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add plan_only param to api and create /tags/import/plan endpoint [FC-0036] #130

Merged
Show file tree
Hide file tree
Changes from 1 commit
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.4.0"
__version__ = "0.4.1"
22 changes: 14 additions & 8 deletions openedx_tagging/core/tagging/import_export/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ def import_tags(
file: BinaryIO,
parser_format: ParserFormat,
replace=False,
) -> bool:
plan_only=False,
) -> tuple[bool, TagImportTask, TagImportPlan | None]:
"""
Execute the necessary actions to import the tags from `file`

Expand All @@ -73,6 +74,8 @@ def import_tags(
Ex. Given a taxonomy with `tag_1`, `tag_2` and `tag_3`. If there is only `tag_1`
in the file (regardless of action), then `tag_2` and `tag_3` will be deleted
if `replace=True`

Set `plan_only` to True to only generate the actions and not execute them.
"""
_import_validations(taxonomy)

Expand All @@ -97,7 +100,7 @@ def import_tags(
# Check if there are errors in the parse
if errors:
task.handle_parser_errors(errors)
return False
return False, task, None

task.log_parser_end()

Expand All @@ -109,16 +112,19 @@ def import_tags(

if tag_import_plan.errors:
task.handle_plan_errors()
return False
return False, task, tag_import_plan

if not plan_only:
task.log_start_execute()
tag_import_plan.execute(task)

task.log_start_execute()
tag_import_plan.execute(task)
task.end_success()
return True
except Exception as exception:

return True, task, tag_import_plan
except Exception as exception: # pylint: disable=broad-exception-caught
# Log any exception
task.log_exception(exception)
return False
return False, task, None


def get_last_import_status(taxonomy: Taxonomy) -> TagImportTaskState:
Expand Down
5 changes: 4 additions & 1 deletion openedx_tagging/core/tagging/import_export/tasks.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
"""
Import and export celery tasks
"""
from __future__ import annotations

from io import BytesIO

from celery import shared_task # type: ignore[import]

import openedx_tagging.core.tagging.import_export.api as import_export_api

from ..models import Taxonomy
from .import_plan import TagImportPlan, TagImportTask
from .parsers import ParserFormat


Expand All @@ -17,7 +20,7 @@ def import_tags_task(
file: BytesIO,
parser_format: ParserFormat,
replace=False,
) -> bool:
) -> tuple[bool, TagImportTask, TagImportPlan | None]:
"""
Runs import on a celery task
"""
Expand Down
41 changes: 40 additions & 1 deletion openedx_tagging/core/tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

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
from openedx_tagging.core.tagging.models import ObjectTag, Tag, TagImportTask, Taxonomy


class TaxonomyListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method
Expand Down Expand Up @@ -257,3 +257,42 @@ class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint:
"""
taxonomy_name = serializers.CharField(required=True)
taxonomy_description = serializers.CharField(default="")


class TagImportTaskSerializer(serializers.ModelSerializer):
"""
Serializer for the TagImportTask model.
"""
class Meta:
model = TagImportTask
fields = [
"id",
"log",
"status",
"creation_date",
]


class TaxonomyImportPlanResponseSerializer(serializers.Serializer):
"""
Serializer for the response of the Taxonomy Import Plan request
"""
task = TagImportTaskSerializer()
plan = serializers.SerializerMethodField()
error = serializers.CharField(required=False, allow_null=True)

def get_plan(self, obj):
"""
Returns the plan of the import
"""
plan = obj.get("plan", None)
if plan:
return plan.plan()

return None

def update(self, instance, validated_data):
raise RuntimeError('`update()` is not supported by the TagImportTask serializer.')

def create(self, validated_data):
raise RuntimeError('`create()` is not supported by the TagImportTask serializer.')
50 changes: 43 additions & 7 deletions openedx_tagging/core/tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
update_tag_in_taxonomy,
)
from ...data import TagDataQuerySet
from ...import_export.api import export_tags, get_last_import_log, import_tags
from ...import_export.api import export_tags, import_tags
from ...import_export.parsers import ParserFormat
from ...models import Taxonomy
from ...rules import ObjectTagPermissionItem
Expand All @@ -42,6 +42,7 @@
TaxonomyExportQueryParamsSerializer,
TaxonomyImportBodySerializer,
TaxonomyImportNewBodySerializer,
TaxonomyImportPlanResponseSerializer,
TaxonomyListQueryParamsSerializer,
TaxonomySerializer,
TaxonomyTagCreateBodySerializer,
Expand Down Expand Up @@ -192,6 +193,17 @@ class TaxonomyView(ModelViewSet):
* 200 - Success
* 400 - Bad request
* 403 - Permission denied

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

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

# System taxonomies use negative numbers for their primary keys
Expand Down Expand Up @@ -279,15 +291,14 @@ def create_import(self, request: Request, **_kwargs) -> Response:

taxonomy = create_taxonomy(taxonomy_name, taxonomy_description)
try:
import_success = import_tags(taxonomy, file, parser_format)
import_success, task, _plan = 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)
taxonomy.delete()
return Response(import_error, status=status.HTTP_400_BAD_REQUEST)
return Response(task.log, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
return Response(str(e), status=status.HTTP_400_BAD_REQUEST)

Expand All @@ -305,17 +316,42 @@ def update_import(self, request: Request, **_kwargs) -> Response:

taxonomy = self.get_object()
try:
import_success = import_tags(taxonomy, file, parser_format, replace=True)
import_success, task, _plan = import_tags(taxonomy, file, parser_format, replace=True)

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)
return Response(task.log, 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/plan", methods=["put"])
def plan_update_import(self, request: Request, **_kwargs) -> Response:
"""
Plan import tags from the uploaded file to an already created taxonomy,
overwriting any existing tags.
"""
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, task, plan = import_tags(taxonomy, file, parser_format, replace=True, plan_only=True)

if import_success:
serializer = TaxonomyImportPlanResponseSerializer({"task": task, "plan": plan})
return Response(serializer.data)
else:
serializer = TaxonomyImportPlanResponseSerializer({"task": task, "plan": plan, "error": task.log})
return Response(serializer.data, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
serializer = TaxonomyImportPlanResponseSerializer({"error": str(e)})
return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST)


@view_auth_classes
class ObjectTagView(
Expand Down
35 changes: 26 additions & 9 deletions tests/openedx_tagging/core/tagging/import_export/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,53 +91,65 @@ def test_import_export_validations(self) -> None:

def test_with_python_error(self) -> None:
self.file.close()
assert not import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert not result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.ERROR
assert log == task.log
assert "ValueError('I/O operation on closed file.')" in log

def test_with_parser_error(self) -> None:
assert not import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.invalid_parser_file,
self.parser_format,
)
assert not result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.ERROR
assert log == task.log
assert "Starting to load data from file" in log
assert "Invalid '.json' format" in log

def test_with_plan_errors(self) -> None:
assert not import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.invalid_plan_file,
self.parser_format,
)
assert not result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.ERROR
assert log == task.log
assert "Starting to load data from file" in log
assert "Load data finished" in log
assert "Starting plan actions" in log
assert "Plan finished" in log
assert "Conflict with 'create'" in log

def test_valid(self) -> None:
assert import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
replace=True,
)
assert result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.SUCCESS
assert log == task.log
assert "Starting to load data from file" in log
assert "Load data finished" in log
assert "Starting plan actions" in log
Expand All @@ -146,33 +158,37 @@ def test_valid(self) -> None:
assert "Execution finished" in log

def test_start_task_after_error(self) -> None:
assert not import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.invalid_parser_file,
self.parser_format,
)
assert import_export_api.import_tags(
assert not result
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert result

def test_start_task_after_success(self) -> None:
assert import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert result

# Opening again the file
json_data = {"tags": self.tags}
self.file = BytesIO(json.dumps(json_data).encode())

assert import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert result

def test_import_with_export_output(self) -> None:
for parser_format in ParserFormat:
Expand All @@ -183,11 +199,12 @@ def test_import_with_export_output(self) -> None:
file = BytesIO(output.encode())
new_taxonomy = Taxonomy(name="New taxonomy")
new_taxonomy.save()
assert import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
new_taxonomy,
file,
parser_format,
)
assert result
old_tags = self.taxonomy.tag_set.all()
assert len(old_tags) == new_taxonomy.tag_set.count()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ def test_import_tags_task(self):
replace = True

with patch('openedx_tagging.core.tagging.import_export.api.import_tags') as mock_import_tags:
mock_import_tags.return_value = True
mock_import_tags.return_value = (True, None, None)

result = import_export_tasks.import_tags_task(self.taxonomy, file, parser_format, replace)
result, _result_task, _result_plan = import_export_tasks.import_tags_task(
self.taxonomy, file, parser_format, replace
)

self.assertTrue(result)
mock_import_tags.assert_called_once_with(self.taxonomy, file, parser_format, replace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,13 @@ def open_template_file(self, template_file):
@ddt.unpack
def test_import_template(self, template_file, parser_format):
with self.open_template_file(template_file) as import_file:
assert import_api.import_tags(
result, _task, _plan = import_api.import_tags(
self.taxonomy,
import_file,
parser_format,
replace=True,
), import_api.get_last_import_log(self.taxonomy)
)
assert result, import_api.get_last_import_log(self.taxonomy)

assert pretty_format_tags(get_tags(self.taxonomy), external_id=True) == [
'Electronic instruments (ELECTRIC) (None) (children: 2)',
Expand Down
Loading
Loading