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 MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ include LICENSE.txt
include README.rst
include requirements/base.in
recursive-include openedx_learning *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg *.py
recursive-include openedx_tagging *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg *.py *.yaml
recursive-include openedx_tagging *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg *.py *.yaml *.json *.csv
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.2.1"
__version__ = "0.2.2"
12 changes: 7 additions & 5 deletions openedx_tagging/core/tagging/import_export/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def _parse_tags(
row = cls.inital_row
for tag in tags_data:
has_error = False
tag_data = {}

# Verify the required fields
for req_field in cls.required_fields:
Expand All @@ -140,20 +141,21 @@ def _parse_tags(
)
)
has_error = True
else:
tag_data[req_field] = tag[req_field]

tag["index"] = row
tag_data["index"] = row
row += 1

# Skip parse if there is an error
if has_error:
continue

# Updating any empty optional field to None
# Optional fields default to None
for opt_field in cls.optional_fields:
if opt_field in tag and not tag.get(opt_field):
tag[opt_field] = None
tag_data[opt_field] = tag.get(opt_field) or None
Copy link
Contributor

Choose a reason for hiding this comment

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

If tag_data[opt_field] is falsy (i.e. 0), we are overriding with None.

I don't think this is a problem because our optional_fields only have the external_id, but we may have some issues using this parser for other things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rpenido Yeah, I was preserving the previous behavior with that change, since the CSV parser will send us empty strings for parent_id, when what we really want is None.
I think that if we ever need the parser to do something different, then there will be a clear use case, and the code and tests can be modified then.


tags.append(TagItem(**tag))
tags.append(TagItem(**tag_data))

return tags, errors

Expand Down
30 changes: 30 additions & 0 deletions openedx_tagging/core/tagging/import_export/template.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
id,value,parent_id,comments
WINDS,Wind instruments,,"This is an example Tag Import file, in CSV format."
PERCUSS,Percussion instruments,,"Only the 'id' and 'value' fields are required. They can be anything you like, but they must must be unique within the taxonomy. Existing tags matching 'id' will be updated on import."
ELECTRIC,Electronic instruments,,"Top-level tags have no 'parent_id', and you can have as many top-level tags as you wish."
STRINGS,String instruments,,"All other fields (like these 'comments') are ignored on import, and will not be included in subsequent tag exports."
BELLS,Idiophone,PERCUSS,"Providing a 'parent_id' creates a tag hierarchy."
DRUMS,Membranophone,PERCUSS,"The 'parent_id' must match an 'id' found earlier in the import file."
CAJÓN,Cajón,DRUMS,"Tag values may contain unicode characters."
PYLE,Pyle Stringed Jam Cajón,CAJÓN,"A tag hierarchy may contain as many as 3 levels. This tag is at level 4, and so it will not be shown to users."
THERAMIN,Theramin,ELECTRIC,"A tag hierarchy may contain uneven levels. Here, the Electronic branch has only 2 levels, while Percussion has 3."
CHORD,Chordophone,PERCUSS,
BRASS,Brass,WINDS,
WOODS,Woodwinds,WINDS,
FLUTE,Flute,WOODS,
PLUCK,Plucked strings,STRINGS,
MANDOLIN,Mandolin,PLUCK,
HARP,Harp,PLUCK,
BANJO,Banjo,PLUCK,
BOW,Bowed strings,STRINGS,
VIOLIN,Violin,BOW,
CELLO,Cello,BOW,
CLARINET,Clarinet,WOODS,
OBOE,Oboe,WOODS,
TRUMPET,Trumpet,BRASS,
TUBA,Tuba,BRASS,
SYNTH,Synthesizer,ELECTRIC,
CELESTA,Celesta,BELLS,
HI-HAT,Hi-hat,BELLS,
TABLA,Tabla,DRUMS,
PIANO,Piano,CHORD,
158 changes: 158 additions & 0 deletions openedx_tagging/core/tagging/import_export/template.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
{
"tags": [
{
"id": "WINDS",
"value": "Wind instruments",
"parent_id": "",
"comments": "This is an example Tag Import file, in JSON format."
},
{
"id": "PERCUSS",
"value": "Percussion instruments",
"parent_id": "",
"comments": "Only the 'id' and 'value' fields are required. They can be anything you like, but they must must be unique within the taxonomy. Existing tags matching 'id' will be updated on import."
},
{
"id": "ELECTRIC",
"value": "Electronic instruments",
"parent_id": "",
"comments": "Top-level tags have no 'parent_id', and you can have as many top-level tags as you wish."
},
{
"id": "STRINGS",
"value": "String instruments",
"parent_id": "",
"comments": "All other fields (like these 'comments') are ignored on import, and will not be included in subsequent tag exports."
},
{
"id": "BELLS",
"value": "Idiophone",
"parent_id": "PERCUSS",
"comments": "Providing a 'parent_id' creates a tag hierarchy."
},
{
"id": "DRUMS",
"value": "Membranophone",
"parent_id": "PERCUSS",
"comments": "The 'parent_id' must match an 'id' found earlier in the import file."
},
{
"id": "CAJÓN",
"value": "Cajón",
"parent_id": "DRUMS",
"comments": "Tag values may contain unicode characters."
},
{
"id": "PYLE",
"value": "Pyle Stringed Jam Cajón",
"parent_id": "CAJÓN",
"comments": "A tag hierarchy may contain as many as 3 levels. This tag is at level 4, and so it will not be shown to users."
},
{
"id": "THERAMIN",
"value": "Theramin",
"parent_id": "ELECTRIC",
"comments": "A tag hierarchy may contain uneven levels. Here, the Electronic branch has only 2 levels, while Percussion has 3."
},
{
"id": "CHORD",
"value": "Chordophone",
"parent_id": "PERCUSS"
},
{
"id": "BRASS",
"value": "Brass",
"parent_id": "WINDS"
},
{
"id": "WOODS",
"value": "Woodwinds",
"parent_id": "WINDS"
},
{
"id": "FLUTE",
"value": "Flute",
"parent_id": "WOODS"
},
{
"id": "PLUCK",
"value": "Plucked strings",
"parent_id": "STRINGS"
},
{
"id": "MANDOLIN",
"value": "Mandolin",
"parent_id": "PLUCK"
},
{
"id": "HARP",
"value": "Harp",
"parent_id": "PLUCK"
},
{
"id": "BANJO",
"value": "Banjo",
"parent_id": "PLUCK"
},
{
"id": "BOW",
"value": "Bowed strings",
"parent_id": "STRINGS"
},
{
"id": "VIOLIN",
"value": "Violin",
"parent_id": "BOW"
},
{
"id": "CELLO",
"value": "Cello",
"parent_id": "BOW"
},
{
"id": "CLARINET",
"value": "Clarinet",
"parent_id": "WOODS"
},
{
"id": "OBOE",
"value": "Oboe",
"parent_id": "WOODS"
},
{
"id": "TRUMPET",
"value": "Trumpet",
"parent_id": "BRASS"
},
{
"id": "TUBA",
"value": "Tuba",
"parent_id": "BRASS"
},
{
"id": "SYNTH",
"value": "Synthesizer",
"parent_id": "ELECTRIC"
},
{
"id": "CELESTA",
"value": "Celesta",
"parent_id": "BELLS"
},
{
"id": "HI-HAT",
"value": "Hi-hat",
"parent_id": "BELLS"
},
{
"id": "TABLA",
"value": "Tabla",
"parent_id": "DRUMS"
},
{
"id": "PIANO",
"value": "Piano",
"parent_id": "CHORD"
}
]
}
7 changes: 6 additions & 1 deletion openedx_tagging/core/tagging/rest_api/v1/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.urls.conf import include, path
from rest_framework.routers import DefaultRouter

from . import views
from . import views, views_import

router = DefaultRouter()
router.register("taxonomies", views.TaxonomyView, basename="taxonomy")
Expand All @@ -18,4 +18,9 @@
views.TaxonomyTagsView.as_view(),
name="taxonomy-tags",
),
path(
"import/template.<str:file_ext>",
views_import.TemplateView.as_view(),
name="taxonomy-import-template",
),
]
51 changes: 51 additions & 0 deletions openedx_tagging/core/tagging/rest_api/v1/views_import.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""
Taxonomy Import views
"""
from __future__ import annotations

import os

from django.http import FileResponse, Http404
from rest_framework.request import Request
from rest_framework.views import APIView


class TemplateView(APIView):
"""
View which serves the static Taxonomy Import template files.

**Example Requests**
GET /tagging/rest_api/v1/import/template.csv
GET /tagging/rest_api/v1/import/template.json

**Query Returns**
* 200 - Success
* 404 - Template file not found
* 405 - Method not allowed
"""
http_method_names = ['get']

template_dir = os.path.join(
os.path.dirname(__file__),
"../../import_export/",
)
allowed_ext_to_content_type = {
"csv": "text/csv",
"json": "application/json",
}

def get(self, request: Request, file_ext: str, *args, **kwargs) -> FileResponse:
"""
Downloads the requested file as an attachment,
or raises 404 if not found.
"""
content_type = self.allowed_ext_to_content_type.get(file_ext)
if not content_type:
raise Http404

filename = f"template.{file_ext}"
content_disposition = f'attachment; filename="{filename}"'
fh = open(os.path.join(self.template_dir, filename), "rb")
response = FileResponse(fh, content_type=content_type)
response['Content-Disposition'] = content_disposition
return response
11 changes: 11 additions & 0 deletions tests/openedx_tagging/core/tagging/import_export/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ def test_load_data_errors(self) -> None:
)

@ddt.data(
(
{"tags": [
{"id": "tag_1", "value": "Tag 1", "comment": "This field is ignored."}, # Valid
]},
[]
),
(
{"tags": [
{"id": "tag_1", "value": "Tag 1"}, # Valid
Expand Down Expand Up @@ -209,6 +215,11 @@ class TestCSVParser(TestImportExportMixin, TestCase):
# Valid
"id,value\n",
[]
),
(
# Valid
"id,value,ignored\n",
[]
)
)
@ddt.unpack
Expand Down
Loading