From de3decbcd00a912e6967acc9cda2409451ccf66a Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 12 Mar 2024 12:49:49 -0500 Subject: [PATCH 01/19] refactor: Extract generate tags on csv to a function api --- .../core/djangoapps/content_tagging/api.py | 62 ++++++++++++++- .../content_tagging/helpers/__init__.py | 0 .../objecttag_export_helpers.py | 2 +- .../test_objecttag_export_helpers.py | 6 +- .../rest_api/v1/tests/test_views.py | 2 +- .../content_tagging/rest_api/v1/views.py | 78 +++---------------- 6 files changed, 78 insertions(+), 72 deletions(-) create mode 100644 openedx/core/djangoapps/content_tagging/helpers/__init__.py rename openedx/core/djangoapps/content_tagging/{rest_api/v1 => helpers}/objecttag_export_helpers.py (98%) rename openedx/core/djangoapps/content_tagging/{rest_api/v1/tests => helpers}/test_objecttag_export_helpers.py (98%) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index c167eba3a629..f548f929c37f 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -4,17 +4,27 @@ from __future__ import annotations from itertools import groupby +import csv +from typing import Iterator +from opaque_keys.edx.keys import UsageKey import openedx_tagging.core.tagging.api as oel_tagging +from openedx_tagging.core.tagging import rules as oel_tagging_rules from django.db.models import Exists, OuterRef, Q, QuerySet from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy from organizations.models import Organization +from rest_framework.exceptions import PermissionDenied, ValidationError +from .helpers.objecttag_export_helpers import build_object_tree_with_objecttags, iterate_with_level from .models import TaxonomyOrg from .types import ContentKey, ObjectTagByObjectIdDict, TagValuesByTaxonomyExportIdDict, TaxonomyDict -from .utils import check_taxonomy_context_key_org, get_context_key_from_key +from .utils import ( + check_taxonomy_context_key_org, + get_context_key_from_key, + get_content_key_from_string, +) def create_taxonomy( @@ -198,6 +208,56 @@ def set_object_tags( ) +def generate_csv(object_id, user, buffer) -> Iterator[str]: + """ + Returns a CSV string with tags and taxonomies of all blocks of `content_key` + """ + content_key = get_content_key_from_string(object_id) + + if isinstance(content_key, UsageKey): + raise ValidationError("The object_id must be a CourseKey or a LibraryLocatorV2.") + + # Check if the user has permission to view object tags for this object_id + if not user.has_perm( + "oel_tagging.view_objecttag", + # The obj arg expects a model, but we are passing an object + oel_tagging_rules.ObjectTagPermissionItem(taxonomy=None, object_id=object_id), # type: ignore[arg-type] + ): + raise PermissionDenied( + "You do not have permission to view object tags for this object_id." + ) + + all_object_tags, taxonomies = get_all_object_tags(content_key) + tagged_content = build_object_tree_with_objecttags(content_key, all_object_tags) + + header = {"name": "Name", "type": "Type", "id": "ID"} + + # Prepare the header for the taxonomies + for taxonomy_id, taxonomy in taxonomies.items(): + header[f"taxonomy_{taxonomy_id}"] = taxonomy.export_id + + csv_writer = csv.DictWriter(buffer, fieldnames=header.keys(), quoting=csv.QUOTE_NONNUMERIC) + yield csv_writer.writerow(header) + + # Iterate over the blocks and yield the rows + for item, level in iterate_with_level(tagged_content): + block_data = { + "name": level * " " + item.display_name, + "type": item.category, + "id": item.block_id, + } + + # Add the tags for each taxonomy + for taxonomy_id in taxonomies: + if taxonomy_id in item.object_tags: + block_data[f"taxonomy_{taxonomy_id}"] = ", ".join([ + object_tag.value + for object_tag in item.object_tags[taxonomy_id] + ]) + + yield csv_writer.writerow(block_data) + + # Expose the oel_tagging APIs get_taxonomy = oel_tagging.get_taxonomy diff --git a/openedx/core/djangoapps/content_tagging/helpers/__init__.py b/openedx/core/djangoapps/content_tagging/helpers/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py similarity index 98% rename from openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py rename to openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py index e99017f432e8..901e8c827c1c 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py @@ -15,7 +15,7 @@ from openedx.core.djangoapps.content_libraries.api import LibraryXBlockMetadata from xmodule.modulestore.django import modulestore -from ...types import ObjectTagByObjectIdDict, ObjectTagByTaxonomyIdDict +from ..types import ObjectTagByObjectIdDict, ObjectTagByTaxonomyIdDict @define diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py similarity index 98% rename from openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py rename to openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py index 61132a69a227..629c7769895d 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py @@ -7,9 +7,9 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory -from .... import api -from ....tests.test_api import TestGetAllObjectTagsMixin -from ..objecttag_export_helpers import TaggedContent, build_object_tree_with_objecttags, iterate_with_level +from .. import api +from ..tests.test_api import TestGetAllObjectTagsMixin +from .objecttag_export_helpers import TaggedContent, build_object_tree_with_objecttags, iterate_with_level class TaggedCourseMixin(TestGetAllObjectTagsMixin, ModuleStoreTestCase): # type: ignore[misc] diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index a01981e60008..13221482211a 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -37,7 +37,7 @@ from openedx.core.djangolib.testing.utils import skip_unless_cms -from .test_objecttag_export_helpers import TaggedCourseMixin +from ....helpers.test_objecttag_export_helpers import TaggedCourseMixin User = get_user_model() diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index d9bf18cd88e0..13d2bd9ba8f3 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -3,14 +3,10 @@ """ from __future__ import annotations -import csv -from typing import Iterator - from django.db.models import Count from django.http import StreamingHttpResponse -from opaque_keys.edx.keys import UsageKey -from openedx_tagging.core.tagging import rules as oel_tagging_rules from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView +from openedx_tagging.core.tagging import rules as oel_tagging_rules from rest_framework import status from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied, ValidationError @@ -20,7 +16,7 @@ from ...api import ( create_taxonomy, - get_all_object_tags, + generate_csv, get_taxonomies, get_taxonomies_for_org, get_taxonomy, @@ -28,9 +24,7 @@ set_taxonomy_orgs ) from ...rules import get_admin_orgs -from ...utils import get_content_key_from_string from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend -from .objecttag_export_helpers import build_object_tree_with_objecttags, iterate_with_level from .serializers import TaxonomyOrgListQueryParamsSerializer, TaxonomyOrgSerializer, TaxonomyUpdateOrgBodySerializer @@ -167,66 +161,18 @@ class Echo(object): def write(self, value): return value - def _generate_csv_rows() -> Iterator[str]: - """ - Receives the blocks, tags and taxonomies and returns a CSV string - """ - - header = {"name": "Name", "type": "Type", "id": "ID"} - - # Prepare the header for the taxonomies - for taxonomy_id, taxonomy in taxonomies.items(): - header[f"taxonomy_{taxonomy_id}"] = taxonomy.export_id - - csv_writer = csv.DictWriter(pseudo_buffer, fieldnames=header.keys(), quoting=csv.QUOTE_NONNUMERIC) - yield csv_writer.writerow(header) - - # Iterate over the blocks and yield the rows - for item, level in iterate_with_level(tagged_content): - block_data = { - "name": level * " " + item.display_name, - "type": item.category, - "id": item.block_id, - } - - # Add the tags for each taxonomy - for taxonomy_id in taxonomies: - if taxonomy_id in item.object_tags: - block_data[f"taxonomy_{taxonomy_id}"] = ", ".join([ - object_tag.value - for object_tag in item.object_tags[taxonomy_id] - ]) - - yield csv_writer.writerow(block_data) - object_id: str = kwargs.get('context_id', None) + pseudo_buffer = Echo() try: - content_key = get_content_key_from_string(object_id) - - if isinstance(content_key, UsageKey): - raise ValidationError("The object_id must be a CourseKey or a LibraryLocatorV2.") - - # Check if the user has permission to view object tags for this object_id - if not self.request.user.has_perm( - "oel_tagging.view_objecttag", - # The obj arg expects a model, but we are passing an object - oel_tagging_rules.ObjectTagPermissionItem(taxonomy=None, object_id=object_id), # type: ignore[arg-type] - ): - raise PermissionDenied( - "You do not have permission to view object tags for this object_id." - ) - - all_object_tags, taxonomies = get_all_object_tags(content_key) - tagged_content = build_object_tree_with_objecttags(content_key, all_object_tags) - + return StreamingHttpResponse( + streaming_content=generate_csv( + object_id, + self.request.user, + pseudo_buffer, + ), + content_type="text/csv", + headers={'Content-Disposition': f'attachment; filename="{object_id}_tags.csv"'}, + ) except ValueError as e: raise ValidationError from e - - pseudo_buffer = Echo() - - return StreamingHttpResponse( - streaming_content=_generate_csv_rows(), - content_type="text/csv", - headers={'Content-Disposition': f'attachment; filename="{object_id}_tags.csv"'}, - ) From 8056ea4a820e5b9c6fd39986629afd5da0e94dfb Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 13 Mar 2024 12:23:50 -0500 Subject: [PATCH 02/19] feat: Add tags.csv to export tarball --- .../contentstore/views/import_export.py | 6 + .../core/djangoapps/content_tagging/api.py | 33 +-- .../core/djangoapps/content_tagging/auth.py | 14 ++ .../helpers/test_objecttag_export_helpers.py | 176 +++++++++++++- .../rest_api/v1/tests/test_views.py | 2 +- .../content_tagging/rest_api/v1/views.py | 11 +- .../content_tagging/tests/test_api.py | 230 +++++------------- xmodule/modulestore/xml_exporter.py | 3 + 8 files changed, 280 insertions(+), 195 deletions(-) create mode 100644 openedx/core/djangoapps/content_tagging/auth.py diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 22310faa1ae2..e30865297d59 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -39,6 +39,7 @@ from common.djangoapps.util.views import ensure_valid_course_key from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order +from openedx.core.djangoapps.content_tagging.auth import has_view_object_tags_access from ..storage import course_import_export_storage from ..tasks import CourseExportTask, CourseImportTask, export_olx, import_olx from ..toggles import use_new_export_page, use_new_import_page @@ -343,6 +344,11 @@ def export_handler(request, course_key_string): requested_format = request.GET.get('_accept', request.META.get('HTTP_ACCEPT', 'text/html')) if request.method == 'POST': + if not has_view_object_tags_access(request.user, str(course_key)): + raise PermissionDenied( + "You do not have permission to view object tags for this object_id." + ) + export_olx.delay(request.user.id, course_key_string, request.LANGUAGE_CODE) return JsonResponse({'ExportStatus': 1}) elif 'text/html' in requested_format: diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index f548f929c37f..e28541ea2121 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -2,6 +2,7 @@ Content Tagging APIs """ from __future__ import annotations +import io from itertools import groupby import csv @@ -9,13 +10,11 @@ from opaque_keys.edx.keys import UsageKey import openedx_tagging.core.tagging.api as oel_tagging -from openedx_tagging.core.tagging import rules as oel_tagging_rules from django.db.models import Exists, OuterRef, Q, QuerySet from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy from organizations.models import Organization -from rest_framework.exceptions import PermissionDenied, ValidationError from .helpers.objecttag_export_helpers import build_object_tree_with_objecttags, iterate_with_level from .models import TaxonomyOrg @@ -208,24 +207,14 @@ def set_object_tags( ) -def generate_csv(object_id, user, buffer) -> Iterator[str]: +def generate_csv_rows(object_id, buffer) -> Iterator[str]: """ - Returns a CSV string with tags and taxonomies of all blocks of `content_key` + Returns a CSV string with tags and taxonomies of all blocks of `object_id` """ content_key = get_content_key_from_string(object_id) if isinstance(content_key, UsageKey): - raise ValidationError("The object_id must be a CourseKey or a LibraryLocatorV2.") - - # Check if the user has permission to view object tags for this object_id - if not user.has_perm( - "oel_tagging.view_objecttag", - # The obj arg expects a model, but we are passing an object - oel_tagging_rules.ObjectTagPermissionItem(taxonomy=None, object_id=object_id), # type: ignore[arg-type] - ): - raise PermissionDenied( - "You do not have permission to view object tags for this object_id." - ) + raise ValueError("The object_id must be a CourseKey or a LibraryLocatorV2.") all_object_tags, taxonomies = get_all_object_tags(content_key) tagged_content = build_object_tree_with_objecttags(content_key, all_object_tags) @@ -258,6 +247,20 @@ def generate_csv(object_id, user, buffer) -> Iterator[str]: yield csv_writer.writerow(block_data) +def export_tags_in_csv_file(object_id, file_dir, file_name) -> None: + """ + Writes a CSV file with tags and taxonomies of all blocks of `object_id` + """ + buffer = io.StringIO() + for _ in generate_csv_rows(object_id, buffer): + # The generate_csv_rows function is a generator, + # we don't need to do anything with the result here + pass + + with file_dir.open(file_name, 'w') as csv_file: + buffer.seek(0) + csv_file.write(buffer.read()) + # Expose the oel_tagging APIs get_taxonomy = oel_tagging.get_taxonomy diff --git a/openedx/core/djangoapps/content_tagging/auth.py b/openedx/core/djangoapps/content_tagging/auth.py new file mode 100644 index 000000000000..73040111ab53 --- /dev/null +++ b/openedx/core/djangoapps/content_tagging/auth.py @@ -0,0 +1,14 @@ +""" +Functions to validate the access in content tagging actions +""" + + +from openedx_tagging.core.tagging import rules as oel_tagging_rules + + +def has_view_object_tags_access(user, object_id): + return user.has_perm( + "oel_tagging.view_objecttag", + # The obj arg expects a model, but we are passing an object + oel_tagging_rules.ObjectTagPermissionItem(taxonomy=None, object_id=object_id), # type: ignore[arg-type] + ) diff --git a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py index 629c7769895d..b8bc94974264 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py @@ -1,6 +1,7 @@ """ Test the objecttag_export_helpers module """ +import time from unittest.mock import patch from openedx.core.djangoapps.content_libraries import api as library_api @@ -8,8 +9,181 @@ from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory from .. import api -from ..tests.test_api import TestGetAllObjectTagsMixin from .objecttag_export_helpers import TaggedContent, build_object_tree_with_objecttags, iterate_with_level +from openedx_tagging.core.tagging.models import ObjectTag, Tag +from organizations.models import Organization + + +class TestGetAllObjectTagsMixin: + """ + Set up data to test get_all_object_tags functions + """ + + def setUp(self): + super().setUp() + + self.orgA = Organization.objects.create(name="Organization A", short_name="orgA") + self.taxonomy_1 = api.create_taxonomy(name="Taxonomy 1") + api.set_taxonomy_orgs(self.taxonomy_1, all_orgs=True) + Tag.objects.create( + taxonomy=self.taxonomy_1, + value="Tag 1.1", + ) + Tag.objects.create( + taxonomy=self.taxonomy_1, + value="Tag 1.2", + ) + + self.taxonomy_2 = api.create_taxonomy(name="Taxonomy 2") + api.set_taxonomy_orgs(self.taxonomy_2, all_orgs=True) + + Tag.objects.create( + taxonomy=self.taxonomy_2, + value="Tag 2.1", + ) + Tag.objects.create( + taxonomy=self.taxonomy_2, + value="Tag 2.2", + ) + + api.tag_object( + object_id="course-v1:orgA+test_course+test_run", + taxonomy=self.taxonomy_1, + tags=['Tag 1.1'], + ) + self.course_tags = api.get_object_tags("course-v1:orgA+test_course+test_run") + + # Tag blocks + api.tag_object( + object_id="block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + taxonomy=self.taxonomy_1, + tags=['Tag 1.1', 'Tag 1.2'], + ) + self.sequential_tags1 = api.get_object_tags( + "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + taxonomy_id=self.taxonomy_1.id, + + ) + api.tag_object( + object_id="block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + taxonomy=self.taxonomy_2, + tags=['Tag 2.1'], + ) + self.sequential_tags2 = api.get_object_tags( + "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + taxonomy_id=self.taxonomy_2.id, + ) + api.tag_object( + object_id="block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", + taxonomy=self.taxonomy_2, + tags=['Tag 2.2'], + ) + self.vertical1_tags = api.get_object_tags( + "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1" + ) + api.tag_object( + object_id="block-v1:orgA+test_course+test_run+type@html+block@test_html", + taxonomy=self.taxonomy_2, + tags=['Tag 2.1'], + ) + self.html_tags = api.get_object_tags("block-v1:orgA+test_course+test_run+type@html+block@test_html") + + # Create "deleted" object tags, which will be omitted from the results. + for object_id in ( + "course-v1:orgA+test_course+test_run", + "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", + "block-v1:orgA+test_course+test_run+type@html+block@test_html", + ): + ObjectTag.objects.create( + object_id=str(object_id), + taxonomy=None, + tag=None, + _value="deleted tag", + _name="deleted taxonomy", + ) + + self.expected_course_objecttags = { + "course-v1:orgA+test_course+test_run": { + self.taxonomy_1.id: list(self.course_tags), + }, + "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential": { + self.taxonomy_1.id: list(self.sequential_tags1), + self.taxonomy_2.id: list(self.sequential_tags2), + }, + "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1": { + self.taxonomy_2.id: list(self.vertical1_tags), + }, + "block-v1:orgA+test_course+test_run+type@html+block@test_html": { + self.taxonomy_2.id: list(self.html_tags), + }, + } + + # Library tags and library contents need a unique block_id that is persisted along test runs + self.block_suffix = str(round(time.time() * 1000)) + + api.tag_object( + object_id=f"lib:orgA:lib_{self.block_suffix}", + taxonomy=self.taxonomy_2, + tags=['Tag 2.1'], + ) + self.library_tags = api.get_object_tags(f"lib:orgA:lib_{self.block_suffix}") + + api.tag_object( + object_id=f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", + taxonomy=self.taxonomy_1, + tags=['Tag 1.1'], + ) + self.problem1_tags = api.get_object_tags( + f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}" + ) + + api.tag_object( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy=self.taxonomy_1, + tags=['Tag 1.2'], + ) + self.library_html_tags1 = api.get_object_tags( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy_id=self.taxonomy_1.id, + ) + + api.tag_object( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy=self.taxonomy_2, + tags=['Tag 2.2'], + ) + self.library_html_tags2 = api.get_object_tags( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy_id=self.taxonomy_2.id, + ) + + # Create "deleted" object tags, which will be omitted from the results. + for object_id in ( + f"lib:orgA:lib_{self.block_suffix}", + f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", + f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + ): + ObjectTag.objects.create( + object_id=object_id, + taxonomy=None, + tag=None, + _value="deleted tag", + _name="deleted taxonomy", + ) + + self.expected_library_objecttags = { + f"lib:orgA:lib_{self.block_suffix}": { + self.taxonomy_2.id: list(self.library_tags), + }, + f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}": { + self.taxonomy_1.id: list(self.problem1_tags), + }, + f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}": { + self.taxonomy_1.id: list(self.library_html_tags1), + self.taxonomy_2.id: list(self.library_html_tags2), + }, + } class TaggedCourseMixin(TestGetAllObjectTagsMixin, ModuleStoreTestCase): # type: ignore[misc] diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 13221482211a..5586635c309e 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1884,7 +1884,7 @@ def test_export_course_invalid_id(self) -> None: url = OBJECT_TAGS_EXPORT_URL.format(object_id="invalid") self.client.force_authenticate(user=self.staff) response = self.client.get(url) - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == status.HTTP_404_NOT_FOUND @skip_unless_cms diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 13d2bd9ba8f3..fc14037ef16f 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -13,10 +13,11 @@ from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView +from ...auth import has_view_object_tags_access from ...api import ( create_taxonomy, - generate_csv, + generate_csv_rows, get_taxonomies, get_taxonomies_for_org, get_taxonomy, @@ -164,11 +165,15 @@ def write(self, value): object_id: str = kwargs.get('context_id', None) pseudo_buffer = Echo() + if not has_view_object_tags_access(self.request.user, object_id): + raise PermissionDenied( + "You do not have permission to view object tags for this object_id." + ) + try: return StreamingHttpResponse( - streaming_content=generate_csv( + streaming_content=generate_csv_rows( object_id, - self.request.user, pseudo_buffer, ), content_type="text/csv", diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 31e6802393c0..794555092b61 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -1,13 +1,16 @@ """Tests for the Tagging models""" -import time - +import io +import os +import tempfile import ddt from django.test.testcases import TestCase +from fs.osfs import OSFS from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx_tagging.core.tagging.models import ObjectTag, Tag +from openedx_tagging.core.tagging.models import Tag from organizations.models import Organization +from ..helpers.test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin from .. import api @@ -238,178 +241,6 @@ def test_get_tags(self): assert result[0]["depth"] == 0 -class TestGetAllObjectTagsMixin: - """ - Set up data to test get_all_object_tags functions - """ - - def setUp(self): - super().setUp() - - self.orgA = Organization.objects.create(name="Organization A", short_name="orgA") - self.taxonomy_1 = api.create_taxonomy(name="Taxonomy 1") - api.set_taxonomy_orgs(self.taxonomy_1, all_orgs=True) - Tag.objects.create( - taxonomy=self.taxonomy_1, - value="Tag 1.1", - ) - Tag.objects.create( - taxonomy=self.taxonomy_1, - value="Tag 1.2", - ) - - self.taxonomy_2 = api.create_taxonomy(name="Taxonomy 2") - api.set_taxonomy_orgs(self.taxonomy_2, all_orgs=True) - - Tag.objects.create( - taxonomy=self.taxonomy_2, - value="Tag 2.1", - ) - Tag.objects.create( - taxonomy=self.taxonomy_2, - value="Tag 2.2", - ) - - api.tag_object( - object_id="course-v1:orgA+test_course+test_run", - taxonomy=self.taxonomy_1, - tags=['Tag 1.1'], - ) - self.course_tags = api.get_object_tags("course-v1:orgA+test_course+test_run") - - # Tag blocks - api.tag_object( - object_id="block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - taxonomy=self.taxonomy_1, - tags=['Tag 1.1', 'Tag 1.2'], - ) - self.sequential_tags1 = api.get_object_tags( - "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - taxonomy_id=self.taxonomy_1.id, - - ) - api.tag_object( - object_id="block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - taxonomy=self.taxonomy_2, - tags=['Tag 2.1'], - ) - self.sequential_tags2 = api.get_object_tags( - "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - taxonomy_id=self.taxonomy_2.id, - ) - api.tag_object( - object_id="block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", - taxonomy=self.taxonomy_2, - tags=['Tag 2.2'], - ) - self.vertical1_tags = api.get_object_tags( - "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1" - ) - api.tag_object( - object_id="block-v1:orgA+test_course+test_run+type@html+block@test_html", - taxonomy=self.taxonomy_2, - tags=['Tag 2.1'], - ) - self.html_tags = api.get_object_tags("block-v1:orgA+test_course+test_run+type@html+block@test_html") - - # Create "deleted" object tags, which will be omitted from the results. - for object_id in ( - "course-v1:orgA+test_course+test_run", - "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", - "block-v1:orgA+test_course+test_run+type@html+block@test_html", - ): - ObjectTag.objects.create( - object_id=str(object_id), - taxonomy=None, - tag=None, - _value="deleted tag", - _name="deleted taxonomy", - ) - - self.expected_course_objecttags = { - "course-v1:orgA+test_course+test_run": { - self.taxonomy_1.id: list(self.course_tags), - }, - "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential": { - self.taxonomy_1.id: list(self.sequential_tags1), - self.taxonomy_2.id: list(self.sequential_tags2), - }, - "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1": { - self.taxonomy_2.id: list(self.vertical1_tags), - }, - "block-v1:orgA+test_course+test_run+type@html+block@test_html": { - self.taxonomy_2.id: list(self.html_tags), - }, - } - - # Library tags and library contents need a unique block_id that is persisted along test runs - self.block_suffix = str(round(time.time() * 1000)) - - api.tag_object( - object_id=f"lib:orgA:lib_{self.block_suffix}", - taxonomy=self.taxonomy_2, - tags=['Tag 2.1'], - ) - self.library_tags = api.get_object_tags(f"lib:orgA:lib_{self.block_suffix}") - - api.tag_object( - object_id=f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", - taxonomy=self.taxonomy_1, - tags=['Tag 1.1'], - ) - self.problem1_tags = api.get_object_tags( - f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}" - ) - - api.tag_object( - object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", - taxonomy=self.taxonomy_1, - tags=['Tag 1.2'], - ) - self.library_html_tags1 = api.get_object_tags( - object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", - taxonomy_id=self.taxonomy_1.id, - ) - - api.tag_object( - object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", - taxonomy=self.taxonomy_2, - tags=['Tag 2.2'], - ) - self.library_html_tags2 = api.get_object_tags( - object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", - taxonomy_id=self.taxonomy_2.id, - ) - - # Create "deleted" object tags, which will be omitted from the results. - for object_id in ( - f"lib:orgA:lib_{self.block_suffix}", - f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", - f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", - ): - ObjectTag.objects.create( - object_id=object_id, - taxonomy=None, - tag=None, - _value="deleted tag", - _name="deleted taxonomy", - ) - - self.expected_library_objecttags = { - f"lib:orgA:lib_{self.block_suffix}": { - self.taxonomy_2.id: list(self.library_tags), - }, - f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}": { - self.taxonomy_1.id: list(self.problem1_tags), - }, - f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}": { - self.taxonomy_1.id: list(self.library_html_tags1), - self.taxonomy_2.id: list(self.library_html_tags2), - }, - } - - class TestGetAllObjectTags(TestGetAllObjectTagsMixin, TestCase): """ Test get_all_object_tags api function @@ -444,3 +275,52 @@ def test_get_library_object_tags(self): self.taxonomy_1.id: self.taxonomy_1, self.taxonomy_2.id: self.taxonomy_2, } + + +class TestExportTags(TaggedCourseMixin): + """ + Tests for export functions + """ + def setUp(self): + super().setUp() + self.expected_csv = ( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\r\n' + '" test sequential","sequential","block-v1:orgA+test_course+test_run+type@sequential+block@test_' + 'sequential","Tag 1.1, Tag 1.2","Tag 2.1"\r\n' + '" test vertical1","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@test_' + 'vertical1","","Tag 2.2"\r\n' + '" test vertical2","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@test_' + 'vertical2","",""\r\n' + '" test html","html","block-v1:orgA+test_course+test_run+type@html+block@test_html","","Tag 2.1"\r\n' + '" untagged sequential","sequential","block-v1:orgA+test_course+test_run+type@sequential+block@untagged_' + 'sequential","",""\r\n' + '" untagged vertical","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@untagged_' + 'vertical","",""\r\n' + ) + + def test_generate_csv_rows(self) -> None: + buffer = io.StringIO() + list(api.generate_csv_rows(str(self.course.id), buffer)) + buffer.seek(0) + csv_content = buffer.getvalue() + assert csv_content == self.expected_csv + + def test_export_tags_in_csv_file(self) -> None: + file_dir_name = tempfile.mkdtemp() + file_dir = OSFS(file_dir_name) + file_name = 'tags.csv' + + api.export_tags_in_csv_file(str(self.course.id), file_dir, file_name) + + file_path = os.path.join(file_dir_name, file_name) + + self.assertTrue(os.path.exists(file_path)) + + with open(file_path, 'r') as f: + content = f.read() + + cleaned_content = content.replace('\r\n', '\n') + cleaned_expected_csv = self.expected_csv.replace('\r\n', '\n') + + self.assertEqual(cleaned_content, cleaned_expected_csv) diff --git a/xmodule/modulestore/xml_exporter.py b/xmodule/modulestore/xml_exporter.py index 6f7b142a6714..6a1e453a57aa 100644 --- a/xmodule/modulestore/xml_exporter.py +++ b/xmodule/modulestore/xml_exporter.py @@ -12,6 +12,7 @@ from fs.osfs import OSFS from opaque_keys.edx.locator import CourseLocator, LibraryLocator from xblock.fields import Reference, ReferenceList, ReferenceValueDict, Scope +from openedx.core.djangoapps.content_tagging.api import export_tags_in_csv_file from xmodule.assetstore import AssetMetadata from xmodule.contentstore.content import StaticContent @@ -282,6 +283,8 @@ def process_extra(self, root, courselike, root_courselike_dir, xml_centric_cours _export_drafts(self.modulestore, self.courselike_key, export_fs, xml_centric_courselike_key) + export_tags_in_csv_file(str(self.courselike_key), export_fs, 'tags.csv') + class LibraryExportManager(ExportManager): """ From 0c74ae2ac4c41da294f83e69b7dfbacf30c41da1 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 14 Mar 2024 17:06:39 -0500 Subject: [PATCH 03/19] feat: Verify tag count to generate tags.csv --- .../commands/tests/test_export_olx.py | 18 ++++++++++++++++-- .../rest_api/v1/tests/test_views.py | 2 +- xmodule/modulestore/xml_exporter.py | 13 +++++++++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py b/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py index c84d7b6fa8cb..4d6990c69408 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py @@ -17,6 +17,8 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from openedx.core.djangoapps.content_tagging.helpers.test_objecttag_export_helpers import TaggedCourseMixin + class TestArgParsingCourseExportOlx(unittest.TestCase): """ @@ -31,7 +33,7 @@ def test_no_args(self): call_command('export_olx') -class TestCourseExportOlx(ModuleStoreTestCase): +class TestCourseExportOlx(TaggedCourseMixin, ModuleStoreTestCase): """ Test exporting OLX content from a course or library. """ @@ -61,7 +63,7 @@ def create_dummy_course(self, store_type): ) return course.id - def check_export_file(self, tar_file, course_key): + def check_export_file(self, tar_file, course_key, withTags=False): """Check content of export file.""" names = tar_file.getnames() dirname = "{0.org}-{0.course}-{0.run}".format(course_key) @@ -71,6 +73,10 @@ def check_export_file(self, tar_file, course_key): self.assertIn(f"{dirname}/about/overview.html", names) self.assertIn(f"{dirname}/assets/assets.xml", names) self.assertIn(f"{dirname}/policies", names) + if withTags: + self.assertIn(f"{dirname}/tags.csv", names) + else: + self.assertNotIn(f"{dirname}/tags.csv", names) def test_export_course(self): test_course_key = self.create_dummy_course(ModuleStoreEnum.Type.split) @@ -98,3 +104,11 @@ def __init__(self, bytes_io): output = output_wrapper.bytes_io.read() with tarfile.open(fileobj=BytesIO(output), mode="r:gz") as tar_file: self.check_export_file(tar_file, test_course_key) + + def test_export_course_with_tags(self): + tmp_dir = path(mkdtemp()) + self.addCleanup(shutil.rmtree, tmp_dir) + filename = tmp_dir / 'test.tar.gz' + call_command('export_olx', '--output', filename, str(self.course.id)) + with tarfile.open(filename) as tar_file: + self.check_export_file(tar_file, self.course.id, withTags=True) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 5586635c309e..34544237c8cf 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1884,7 +1884,7 @@ def test_export_course_invalid_id(self) -> None: url = OBJECT_TAGS_EXPORT_URL.format(object_id="invalid") self.client.force_authenticate(user=self.staff) response = self.client.get(url) - assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.status_code == status.HTTP_403_FORBIDDEN @skip_unless_cms diff --git a/xmodule/modulestore/xml_exporter.py b/xmodule/modulestore/xml_exporter.py index 6a1e453a57aa..29141936732d 100644 --- a/xmodule/modulestore/xml_exporter.py +++ b/xmodule/modulestore/xml_exporter.py @@ -12,7 +12,10 @@ from fs.osfs import OSFS from opaque_keys.edx.locator import CourseLocator, LibraryLocator from xblock.fields import Reference, ReferenceList, ReferenceValueDict, Scope -from openedx.core.djangoapps.content_tagging.api import export_tags_in_csv_file +from openedx.core.djangoapps.content_tagging.api import ( + export_tags_in_csv_file, + get_object_tag_counts +) from xmodule.assetstore import AssetMetadata from xmodule.contentstore.content import StaticContent @@ -283,7 +286,13 @@ def process_extra(self, root, courselike, root_courselike_dir, xml_centric_cours _export_drafts(self.modulestore, self.courselike_key, export_fs, xml_centric_courselike_key) - export_tags_in_csv_file(str(self.courselike_key), export_fs, 'tags.csv') + courselike_key_str = str(self.courselike_key) + block_id_pattern = f"{courselike_key_str.replace('course-v1:', 'block-v1:', 1)}*" + + tags_count = get_object_tag_counts(block_id_pattern) + + if tags_count: + export_tags_in_csv_file(courselike_key_str, export_fs, 'tags.csv') class LibraryExportManager(ExportManager): From 82dab6262b5bcacc851b7ba25fbd5e81d2b6bb34 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 19 Mar 2024 13:48:51 -0500 Subject: [PATCH 04/19] feat: Add new fields to csv exporter --- .../core/djangoapps/content_tagging/api.py | 9 +++++- .../content_tagging/tests/test_api.py | 30 ++++++++----------- xmodule/modulestore/xml_exporter.py | 1 + 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index e28541ea2121..f7bf95c88ab1 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -230,10 +230,17 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]: # Iterate over the blocks and yield the rows for item, level in iterate_with_level(tagged_content): + block_key = get_content_key_from_string(item.block_id) + block_id = None + if isinstance(block_key, CourseKey): + block_id = block_key + else: + block_id = block_key.block_id + block_data = { "name": level * " " + item.display_name, "type": item.category, - "id": item.block_id, + "id": block_id, } # Add the tags for each taxonomy diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 794555092b61..2debbbbe0a1f 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -284,19 +284,14 @@ class TestExportTags(TaggedCourseMixin): def setUp(self): super().setUp() self.expected_csv = ( - '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' - '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\r\n' - '" test sequential","sequential","block-v1:orgA+test_course+test_run+type@sequential+block@test_' - 'sequential","Tag 1.1, Tag 1.2","Tag 2.1"\r\n' - '" test vertical1","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@test_' - 'vertical1","","Tag 2.2"\r\n' - '" test vertical2","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@test_' - 'vertical2","",""\r\n' - '" test html","html","block-v1:orgA+test_course+test_run+type@html+block@test_html","","Tag 2.1"\r\n' - '" untagged sequential","sequential","block-v1:orgA+test_course+test_run+type@sequential+block@untagged_' - 'sequential","",""\r\n' - '" untagged vertical","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@untagged_' - 'vertical","",""\r\n' + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\n' + '" test sequential","sequential","test_sequential","Tag 1.1, Tag 1.2","Tag 2.1"\n' + '" test vertical1","vertical","test_vertical1","","Tag 2.2"\n' + '" test vertical2","vertical","test_vertical2","",""\n' + '" test html","html","test_html","","Tag 2.1"\n' + '" untagged sequential","sequential","untagged_sequential","",""\n' + '" untagged vertical","vertical","untagged_vertical","",""\n' ) def test_generate_csv_rows(self) -> None: @@ -304,7 +299,9 @@ def test_generate_csv_rows(self) -> None: list(api.generate_csv_rows(str(self.course.id), buffer)) buffer.seek(0) csv_content = buffer.getvalue() - assert csv_content == self.expected_csv + cleaned_content = csv_content.replace('\r\n', '\n') + + assert cleaned_content == self.expected_csv def test_export_tags_in_csv_file(self) -> None: file_dir_name = tempfile.mkdtemp() @@ -320,7 +317,4 @@ def test_export_tags_in_csv_file(self) -> None: with open(file_path, 'r') as f: content = f.read() - cleaned_content = content.replace('\r\n', '\n') - cleaned_expected_csv = self.expected_csv.replace('\r\n', '\n') - - self.assertEqual(cleaned_content, cleaned_expected_csv) + self.assertEqual(content, self.expected_csv) diff --git a/xmodule/modulestore/xml_exporter.py b/xmodule/modulestore/xml_exporter.py index 29141936732d..a48c84ac20c1 100644 --- a/xmodule/modulestore/xml_exporter.py +++ b/xmodule/modulestore/xml_exporter.py @@ -291,6 +291,7 @@ def process_extra(self, root, courselike, root_courselike_dir, xml_centric_cours tags_count = get_object_tag_counts(block_id_pattern) + # TODO Verify with course, sections and subsections if tags_count: export_tags_in_csv_file(courselike_key_str, export_fs, 'tags.csv') From 2f931b374db0b038af79d29b104af598272ea6d4 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 19 Mar 2024 14:44:39 -0500 Subject: [PATCH 05/19] test: Fix tests and types on csv exporting --- .../core/djangoapps/content_tagging/api.py | 7 +------ .../helpers/test_objecttag_export_helpers.py | 11 +++++++++++ .../rest_api/v1/tests/test_views.py | 18 +----------------- .../content_tagging/tests/test_api.py | 19 ++++--------------- 4 files changed, 17 insertions(+), 38 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index f7bf95c88ab1..86e62d347c2c 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -231,16 +231,11 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]: # Iterate over the blocks and yield the rows for item, level in iterate_with_level(tagged_content): block_key = get_content_key_from_string(item.block_id) - block_id = None - if isinstance(block_key, CourseKey): - block_id = block_key - else: - block_id = block_key.block_id block_data = { "name": level * " " + item.display_name, "type": item.category, - "id": block_id, + "id": getattr(block_key, 'block_id', item.block_id), } # Add the tags for each taxonomy diff --git a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py index b8bc94974264..e0bae7dac5a5 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py @@ -405,6 +405,17 @@ def setUp(self): (tagged_library_html, 1), ] + self.expected_csv = ( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\r\n' + '" test sequential","sequential","test_sequential","Tag 1.1, Tag 1.2","Tag 2.1"\r\n' + '" test vertical1","vertical","test_vertical1","","Tag 2.2"\r\n' + '" test vertical2","vertical","test_vertical2","",""\r\n' + '" test html","html","test_html","","Tag 2.1"\r\n' + '" untagged sequential","sequential","untagged_sequential","",""\r\n' + '" untagged vertical","vertical","untagged_vertical","",""\r\n' + ) + class TestContentTagChildrenExport(TaggedCourseMixin): # type: ignore[misc] """ diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 34544237c8cf..08078f6aef7a 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1850,24 +1850,8 @@ def test_export_course(self, user_attr) -> None: assert response.status_code == status.HTTP_200_OK assert response.headers['Content-Type'] == 'text/csv' - expected_csv = ( - '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' - '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\r\n' - '" test sequential","sequential","block-v1:orgA+test_course+test_run+type@sequential+block@test_' - 'sequential","Tag 1.1, Tag 1.2","Tag 2.1"\r\n' - '" test vertical1","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@test_' - 'vertical1","","Tag 2.2"\r\n' - '" test vertical2","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@test_' - 'vertical2","",""\r\n' - '" test html","html","block-v1:orgA+test_course+test_run+type@html+block@test_html","","Tag 2.1"\r\n' - '" untagged sequential","sequential","block-v1:orgA+test_course+test_run+type@sequential+block@untagged_' - 'sequential","",""\r\n' - '" untagged vertical","vertical","block-v1:orgA+test_course+test_run+type@vertical+block@untagged_' - 'vertical","",""\r\n' - ) - zip_content = BytesIO(b"".join(response.streaming_content)).getvalue() # type: ignore[attr-defined] - assert zip_content == expected_csv.encode() + assert zip_content == self.expected_csv.encode() def test_export_course_anoymous_forbidden(self) -> None: url = OBJECT_TAGS_EXPORT_URL.format(object_id=str(self.course.id)) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 2debbbbe0a1f..e14bf4137418 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -281,27 +281,14 @@ class TestExportTags(TaggedCourseMixin): """ Tests for export functions """ - def setUp(self): - super().setUp() - self.expected_csv = ( - '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\n' - '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\n' - '" test sequential","sequential","test_sequential","Tag 1.1, Tag 1.2","Tag 2.1"\n' - '" test vertical1","vertical","test_vertical1","","Tag 2.2"\n' - '" test vertical2","vertical","test_vertical2","",""\n' - '" test html","html","test_html","","Tag 2.1"\n' - '" untagged sequential","sequential","untagged_sequential","",""\n' - '" untagged vertical","vertical","untagged_vertical","",""\n' - ) def test_generate_csv_rows(self) -> None: buffer = io.StringIO() list(api.generate_csv_rows(str(self.course.id), buffer)) buffer.seek(0) csv_content = buffer.getvalue() - cleaned_content = csv_content.replace('\r\n', '\n') - assert cleaned_content == self.expected_csv + assert csv_content == self.expected_csv def test_export_tags_in_csv_file(self) -> None: file_dir_name = tempfile.mkdtemp() @@ -317,4 +304,6 @@ def test_export_tags_in_csv_file(self) -> None: with open(file_path, 'r') as f: content = f.read() - self.assertEqual(content, self.expected_csv) + cleaned_content = content.replace('\r\n', '\n') + cleaned_expected_csv = self.expected_csv.replace('\r\n', '\n') + self.assertEqual(cleaned_content, cleaned_expected_csv) From 07db758e5cbb7bd7c808d7b04bd3a2d5727e4ce9 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 26 Mar 2024 11:48:24 -0500 Subject: [PATCH 06/19] feat: Import tags on course and blocks --- .../core/djangoapps/content_tagging/api.py | 65 +++++++++++++++++++ requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 2 +- xmodule/modulestore/xml_exporter.py | 4 +- xmodule/modulestore/xml_importer.py | 24 +++++++ 9 files changed, 97 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 86e62d347c2c..932822b46072 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -263,6 +263,71 @@ def export_tags_in_csv_file(object_id, file_dir, file_name) -> None: buffer.seek(0) csv_file.write(buffer.read()) + +def set_exported_object_tags( + content_key: ContentKey, + exported_tags: TagValuesByTaxonomyExportIdDict, +) -> None: + """ + Sets the tags for the given exported content object. + """ + content_key_str = str(content_key) + + # Clear all tags related with the content. + oel_tagging.delete_object_tags(content_key_str) + + for taxonomy_export_id, tags_values in exported_tags.items(): + if not tags_values: + continue + + taxonomy = oel_tagging.get_taxonomy_by_export_id(taxonomy_export_id) + tags_values = tags_values.split(',') + oel_tagging.tag_object( + object_id=content_key_str, + taxonomy=taxonomy, + tags=tags_values, + create_invalid=True, + taxonomy_export_id=taxonomy_export_id, + ) + + +def import_course_tags_from_csv(csv_path, course_id) -> None: + """ + Import tags from a csv file generated on export. + """ + # Open csv file and extract the tags + try: + with open(csv_path, 'r') as csv_file: + csv_reader = csv.DictReader(csv_file) + tags_in_blocks = list(csv_reader) + except FileNotFoundError: + return + + def get_exported_tags(block) -> TagValuesByTaxonomyExportIdDict: + """ + Returns a map with taxonomy export_id and tags for this block. + """ + result = {} + for key, value in block.items(): + if key in ['Type', 'Name', 'ID']: + continue + result[key] = value + return result + + course_key = get_content_key_from_string(str(course_id)) + + for block in tags_in_blocks: + exported_tags = get_exported_tags(block) + block_type = block.get('Type') + block_id = block.get('ID') + + if block_type == 'course': + set_exported_object_tags(course_key, exported_tags) + else: + block_key = course_key.make_usage_key(block_type, block_id) + set_exported_object_tags(block_key, exported_tags) + + # Expose the oel_tagging APIs get_taxonomy = oel_tagging.get_taxonomy diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 9f753c6c10ce..b329daa27645 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -108,7 +108,7 @@ libsass==0.10.0 click==8.1.6 # pinning this version to avoid updates while the library is being developed -openedx-learning==0.6.3 +openedx-learning==0.8.0 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. openai<=0.28.1 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 19dea21e9ac0..920b758816e5 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -788,7 +788,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.6.3 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 72267883cfe6..5600f16adc2b 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1309,7 +1309,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.6.3 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 5812455cf2e3..7504a3aac922 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -922,7 +922,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.6.3 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 1f2b7a6c5bfb..03ef8064d29c 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -117,7 +117,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) -openedx-learning # Open edX Learning core (experimental) +git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses#egg=openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy openedx-django-wiki openedx-blockstore diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 5555717a0411..cdb98ddeab7f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -979,7 +979,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.6.3 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/xmodule/modulestore/xml_exporter.py b/xmodule/modulestore/xml_exporter.py index a48c84ac20c1..84986e2c1530 100644 --- a/xmodule/modulestore/xml_exporter.py +++ b/xmodule/modulestore/xml_exporter.py @@ -290,9 +290,9 @@ def process_extra(self, root, courselike, root_courselike_dir, xml_centric_cours block_id_pattern = f"{courselike_key_str.replace('course-v1:', 'block-v1:', 1)}*" tags_count = get_object_tag_counts(block_id_pattern) + course_tags_count = get_object_tag_counts(courselike_key_str) - # TODO Verify with course, sections and subsections - if tags_count: + if tags_count or course_tags_count: export_tags_in_csv_file(courselike_key_str, export_fs, 'tags.csv') diff --git a/xmodule/modulestore/xml_importer.py b/xmodule/modulestore/xml_importer.py index 3c1345b44e61..2dbde11164f6 100644 --- a/xmodule/modulestore/xml_importer.py +++ b/xmodule/modulestore/xml_importer.py @@ -52,6 +52,7 @@ from xmodule.tabs import CourseTabList from xmodule.util.misc import escape_invalid_characters from xmodule.x_module import XModuleMixin +from openedx.core.djangoapps.content_tagging.api import import_course_tags_from_csv from .inheritance import own_metadata from .store_utilities import rewrite_nonportable_content_links @@ -476,6 +477,13 @@ def import_drafts(self, courselike, courselike_key, data_path, dest_id): """ raise NotImplementedError + @abstractmethod + def import_tags(self, data_path, dest_id): + """ + To be overloaded with a method that adds tags to already imported blocks + """ + raise NotImplementedError + def recursive_build(self, source_courselike, courselike, courselike_key, dest_id): """ Recursively imports all child blocks from the temporary modulestore into the @@ -574,6 +582,9 @@ def run_imports(self): # Import all draft items into the courselike. courselike = self.import_drafts(courselike, courselike_key, data_path, dest_id) + with self.store.bulk_operations(dest_id): + self.import_tags(data_path, dest_id) + yield courselike @@ -695,6 +706,13 @@ def import_drafts(self, courselike, courselike_key, data_path, dest_id): # Fetch the course to return the most recent course version. return self.store.get_course(courselike.id.replace(branch=None, version_guid=None)) + def import_tags(self, data_path, dest_id): + """ + Imports tags into course blocks. + """ + csv_path = path(data_path) / 'tags.csv' + import_course_tags_from_csv(csv_path, dest_id) + class LibraryImportManager(ImportManager): """ @@ -766,6 +784,12 @@ def import_drafts(self, courselike, courselike_key, data_path, dest_id): """ return courselike + def import_tags(self, data_path, dest_id): + """ + Imports tags into library blocks + """ + # TODO Implement function + def import_course_from_xml(*args, **kwargs): """ From f7e3260ce5f60c298dfec5e3bbd4ce498ed29b8b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 26 Mar 2024 13:51:16 -0500 Subject: [PATCH 07/19] test: Fixing tests and adding test for import --- .../core/djangoapps/content_tagging/api.py | 25 +++---- .../helpers/test_objecttag_export_helpers.py | 12 +-- .../rest_api/v1/tests/test_views.py | 1 + .../content_tagging/tests/test_api.py | 73 ++++++++++++++++++- .../content_tagging/tests/test_tasks.py | 2 + xmodule/modulestore/xml_importer.py | 8 +- 6 files changed, 98 insertions(+), 23 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 365378eb6976..1a77e564c26d 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -276,12 +276,11 @@ def set_exported_object_tags( # Clear all tags related with the content. oel_tagging.delete_object_tags(content_key_str) - for taxonomy_export_id, tags_values in exported_tags.items(): + for taxonomy_export_id, tags_values in exported_tags.items(): if not tags_values: continue taxonomy = oel_tagging.get_taxonomy_by_export_id(taxonomy_export_id) - tags_values = tags_values.split(',') oel_tagging.tag_object( object_id=content_key_str, taxonomy=taxonomy, @@ -296,12 +295,9 @@ def import_course_tags_from_csv(csv_path, course_id) -> None: Import tags from a csv file generated on export. """ # Open csv file and extract the tags - try: - with open(csv_path, 'r') as csv_file: - csv_reader = csv.DictReader(csv_file) - tags_in_blocks = list(csv_reader) - except FileNotFoundError: - return + with open(csv_path, 'r') as csv_file: + csv_reader = csv.DictReader(csv_file) + tags_in_blocks = list(csv_reader) def get_exported_tags(block) -> TagValuesByTaxonomyExportIdDict: """ @@ -309,17 +305,20 @@ def get_exported_tags(block) -> TagValuesByTaxonomyExportIdDict: """ result = {} for key, value in block.items(): - if key in ['Type', 'Name', 'ID']: + if key in ['Type', 'Name', 'ID'] or not value: continue - result[key] = value + result[key] = value.split(',') return result - course_key = get_content_key_from_string(str(course_id)) + course_key = CourseKey.from_string(str(course_id)) for block in tags_in_blocks: exported_tags = get_exported_tags(block) - block_type = block.get('Type') - block_id = block.get('ID') + block_type = block.get('Type', '') + block_id = block.get('ID', '') + + if not block_type or not block_id: + raise ValueError(f"Invalid format of csv in: '{block}'.") if block_type == 'course': set_exported_object_tags(course_key, exported_tags) diff --git a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py index e0bae7dac5a5..eb944d22584b 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py @@ -25,11 +25,11 @@ def setUp(self): self.orgA = Organization.objects.create(name="Organization A", short_name="orgA") self.taxonomy_1 = api.create_taxonomy(name="Taxonomy 1") api.set_taxonomy_orgs(self.taxonomy_1, all_orgs=True) - Tag.objects.create( + self.tag_1_1 = Tag.objects.create( taxonomy=self.taxonomy_1, value="Tag 1.1", ) - Tag.objects.create( + self.tag_1_2 = Tag.objects.create( taxonomy=self.taxonomy_1, value="Tag 1.2", ) @@ -37,11 +37,11 @@ def setUp(self): self.taxonomy_2 = api.create_taxonomy(name="Taxonomy 2") api.set_taxonomy_orgs(self.taxonomy_2, all_orgs=True) - Tag.objects.create( + self.tag_2_1 = Tag.objects.create( taxonomy=self.taxonomy_2, value="Tag 2.1", ) - Tag.objects.create( + self.tag_2_2 = Tag.objects.create( taxonomy=self.taxonomy_2, value="Tag 2.2", ) @@ -100,7 +100,7 @@ def setUp(self): taxonomy=None, tag=None, _value="deleted tag", - _name="deleted taxonomy", + _export_id="deleted_taxonomy", ) self.expected_course_objecttags = { @@ -169,7 +169,7 @@ def setUp(self): taxonomy=None, tag=None, _value="deleted tag", - _name="deleted taxonomy", + _export_id="deleted_taxonomy", ) self.expected_library_objecttags = { diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 08078f6aef7a..fc34fae4dfe9 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1755,6 +1755,7 @@ def test_get_tags(self): # Fetch this object's tags for a single taxonomy expected_tags = [{ 'name': 'Multiple Taxonomy', + 'export_id': '13-multiple-taxonomy', 'taxonomy_id': taxonomy.pk, 'can_tag_object': True, 'tags': [ diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index e14bf4137418..50588c258134 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -277,10 +277,19 @@ def test_get_library_object_tags(self): } -class TestExportTags(TaggedCourseMixin): +class TestExportImportTags(TaggedCourseMixin): """ - Tests for export functions + Tests for export/import functions """ + def _create_csv_file(self, content): + """ + Create a csv file and returns the path and name + """ + file_dir_name = tempfile.mkdtemp() + file_name = f'{file_dir_name}/tags.csv' + with open(file_name, 'w') as csv_file: + csv_file.write(content) + return file_name def test_generate_csv_rows(self) -> None: buffer = io.StringIO() @@ -307,3 +316,63 @@ def test_export_tags_in_csv_file(self) -> None: cleaned_content = content.replace('\r\n', '\n') cleaned_expected_csv = self.expected_csv.replace('\r\n', '\n') self.assertEqual(cleaned_content, cleaned_expected_csv) + + def test_import_tags_invalid_format(self) -> None: + csv_path = self._create_csv_file('invalid format, Invalid\r\ntest1, test2') + with self.assertRaises(ValueError) as exc: + api.import_course_tags_from_csv(csv_path, self.course.id) + assert "Invalid format of csv in" in str(exc.exception) + + def test_import_tags_valid_taxonomy_and_tags(self) -> None: + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\r\n' + ) + api.import_course_tags_from_csv(csv_path, self.course.id) + object_tags = list(api.get_object_tags(self.course.id)) + assert len(object_tags) == 1 + + object_tag = object_tags[0] + assert object_tag.tag == self.tag_1_1 + assert object_tag.taxonomy == self.taxonomy_1 + + def test_import_tags_invalid_tag(self) -> None: + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.11",""\r\n' + ) + api.import_course_tags_from_csv(csv_path, self.course.id) + object_tags = list(api.get_object_tags(self.course.id)) + assert len(object_tags) == 0 + + object_tags = list(api.get_object_tags( + self.course.id, + include_deleted=True, + )) + assert len(object_tags) == 1 + + object_tag = object_tags[0] + assert object_tag.tag is None + assert object_tag.value == 'Tag 1.11' + assert object_tag.taxonomy == self.taxonomy_1 + + def test_import_tags_invalid_taxonomy(self) -> None: + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1-1"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.11"\r\n' + ) + api.import_course_tags_from_csv(csv_path, self.course.id) + object_tags = list(api.get_object_tags(self.course.id)) + assert len(object_tags) == 0 + + object_tags = list(api.get_object_tags( + self.course.id, + include_deleted=True, + )) + assert len(object_tags) == 1 + + object_tag = object_tags[0] + assert object_tag.tag is None + assert object_tag.value == 'Tag 1.11' + assert object_tag.taxonomy is None + assert object_tag.export_id == '1-taxonomy-1-1' diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index 5e3a49dc79f9..ed0cf2c06025 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -38,6 +38,7 @@ def setUp(self): create the taxonomy, simulating the effect of the following migrations: 1. openedx_tagging.core.tagging.migrations.0012_language_taxonomy 2. content_tagging.migrations.0007_system_defined_org_2 + 3. openedx_tagging.core.tagging.migrations.0015_taxonomy_export_id """ super().setUp() Taxonomy.objects.get_or_create(id=-1, defaults={ @@ -47,6 +48,7 @@ def setUp(self): "allow_multiple": False, "allow_free_text": False, "visible_to_authors": True, + "export_id": "-1_languages", "_taxonomy_class": "openedx_tagging.core.tagging.models.system_defined.LanguageTaxonomy", }) TaxonomyOrg.objects.get_or_create(taxonomy_id=-1, defaults={"org": None}) diff --git a/xmodule/modulestore/xml_importer.py b/xmodule/modulestore/xml_importer.py index 2dbde11164f6..0b821f793e2f 100644 --- a/xmodule/modulestore/xml_importer.py +++ b/xmodule/modulestore/xml_importer.py @@ -583,8 +583,12 @@ def run_imports(self): courselike = self.import_drafts(courselike, courselike_key, data_path, dest_id) with self.store.bulk_operations(dest_id): - self.import_tags(data_path, dest_id) - + try: + self.import_tags(data_path, dest_id) + except FileNotFoundError: + logging.info(f'Course import {dest_id}: No tags.csv file present.') + except ValueError as e: + logging.info(f'Course import {dest_id}: {str(e)}') yield courselike From abf7b063e0855cbe97a23a20e13a48de52007e62 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 26 Mar 2024 14:15:57 -0500 Subject: [PATCH 08/19] fix: Tests and lint --- openedx/core/djangoapps/content/search/documents.py | 10 +++++----- openedx/core/djangoapps/content_tagging/api.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index fc1388f82455..c875238c8a0f 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -169,11 +169,11 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: # ... other levels added as needed } for obj_tag in all_tags: - # Add the taxonomy name: - if obj_tag.name not in result[Fields.tags_taxonomy]: - result[Fields.tags_taxonomy].append(obj_tag.name) - # Taxonomy name plus each level of tags, in a list: - parts = [obj_tag.name] + obj_tag.get_lineage() # e.g. ["Location", "North America", "Canada", "Vancouver"] + # Add the taxonomy export_id: + if obj_tag.taxonomy.name not in result[Fields.tags_taxonomy]: + result[Fields.tags_taxonomy].append(obj_tag.taxonomy.name) + # Taxonomy export_id plus each level of tags, in a list: + parts = [obj_tag.taxonomy.name] + obj_tag.get_lineage() # e.g. ["Location", "North America", "Canada", "Vancouver"] parts = [part.replace(" > ", " _ ") for part in parts] # Escape our separator. # Now we build each level (tags.level0, tags.level1, etc.) as applicable. # We have a hard-coded limit of 4 levels of tags for now (see Fields.tags above). diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 1a77e564c26d..8629a9ff006a 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -276,7 +276,7 @@ def set_exported_object_tags( # Clear all tags related with the content. oel_tagging.delete_object_tags(content_key_str) - for taxonomy_export_id, tags_values in exported_tags.items(): + for taxonomy_export_id, tags_values in exported_tags.items(): if not tags_values: continue From 97571af282f80585badf6a1d82202250f5980258 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 26 Mar 2024 14:18:40 -0500 Subject: [PATCH 09/19] style: on comments --- openedx/core/djangoapps/content/search/documents.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index c875238c8a0f..937f5abb1ac8 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -169,10 +169,10 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: # ... other levels added as needed } for obj_tag in all_tags: - # Add the taxonomy export_id: + # Add the taxonomy name: if obj_tag.taxonomy.name not in result[Fields.tags_taxonomy]: result[Fields.tags_taxonomy].append(obj_tag.taxonomy.name) - # Taxonomy export_id plus each level of tags, in a list: + # Taxonomy name plus each level of tags, in a list: parts = [obj_tag.taxonomy.name] + obj_tag.get_lineage() # e.g. ["Location", "North America", "Canada", "Vancouver"] parts = [part.replace(" > ", " _ ") for part in parts] # Escape our separator. # Now we build each level (tags.level0, tags.level1, etc.) as applicable. From 9bba25cb1ea44704438f440695febf104622bcff Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 26 Mar 2024 14:51:15 -0500 Subject: [PATCH 10/19] style: Nit on comment --- openedx/core/djangoapps/content/search/documents.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 937f5abb1ac8..1c7824d04b90 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -172,8 +172,8 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: # Add the taxonomy name: if obj_tag.taxonomy.name not in result[Fields.tags_taxonomy]: result[Fields.tags_taxonomy].append(obj_tag.taxonomy.name) - # Taxonomy name plus each level of tags, in a list: - parts = [obj_tag.taxonomy.name] + obj_tag.get_lineage() # e.g. ["Location", "North America", "Canada", "Vancouver"] + # Taxonomy name plus each level of tags, in a list: # e.g. ["Location", "North America", "Canada", "Vancouver"] + parts = [obj_tag.taxonomy.name] + obj_tag.get_lineage() parts = [part.replace(" > ", " _ ") for part in parts] # Escape our separator. # Now we build each level (tags.level0, tags.level1, etc.) as applicable. # We have a hard-coded limit of 4 levels of tags for now (see Fields.tags above). From 476cf953ae9b41823f3af9748a2fff64f44c655a Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 27 Mar 2024 17:15:04 -0500 Subject: [PATCH 11/19] refactor: Change export/import tags separator to ';' --- openedx/core/djangoapps/content_tagging/api.py | 5 +++-- .../content_tagging/helpers/test_objecttag_export_helpers.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 8629a9ff006a..f94c42fcb64a 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -14,6 +14,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy +from openedx_tagging.core.tagging.models.utils import TAGS_CSV_SEPARATOR from organizations.models import Organization from .helpers.objecttag_export_helpers import build_object_tree_with_objecttags, iterate_with_level @@ -241,7 +242,7 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]: # Add the tags for each taxonomy for taxonomy_id in taxonomies: if taxonomy_id in item.object_tags: - block_data[f"taxonomy_{taxonomy_id}"] = ", ".join([ + block_data[f"taxonomy_{taxonomy_id}"] = f"{TAGS_CSV_SEPARATOR} ".join([ object_tag.value for object_tag in item.object_tags[taxonomy_id] ]) @@ -307,7 +308,7 @@ def get_exported_tags(block) -> TagValuesByTaxonomyExportIdDict: for key, value in block.items(): if key in ['Type', 'Name', 'ID'] or not value: continue - result[key] = value.split(',') + result[key] = value.split(TAGS_CSV_SEPARATOR) return result course_key = CourseKey.from_string(str(course_id)) diff --git a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py index eb944d22584b..8f9c4ba63279 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py @@ -408,7 +408,7 @@ def setUp(self): self.expected_csv = ( '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1",""\r\n' - '" test sequential","sequential","test_sequential","Tag 1.1, Tag 1.2","Tag 2.1"\r\n' + '" test sequential","sequential","test_sequential","Tag 1.1; Tag 1.2","Tag 2.1"\r\n' '" test vertical1","vertical","test_vertical1","","Tag 2.2"\r\n' '" test vertical2","vertical","test_vertical2","",""\r\n' '" test html","html","test_html","","Tag 2.1"\r\n' From 70d1d64599922aef4468ec55e487339b0d9ddbd6 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 1 Apr 2024 11:36:23 -0500 Subject: [PATCH 12/19] chore: bump version --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 8b5a7187e8ac..622e9588ed54 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -794,7 +794,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses +openedx-learning==0.8.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index e7cc585b6b2f..c5b72ee02cc9 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1319,7 +1319,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses +openedx-learning==0.8.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 60b632e0dad3..599f2b51369a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -932,7 +932,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses +openedx-learning==0.8.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 79b02cf1310c..0c2f110c84b9 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -118,7 +118,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) -git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses#egg=openedx-learning # Open edX Learning core (experimental) +openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy openedx-django-wiki openedx-blockstore diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 13c24dcd61d8..1d965c8e2d87 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -987,7 +987,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@chris/FAL-3604-import-export-courses +openedx-learning==0.8.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 31eab30fa69e0914376ebc739c4b439a16029866 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 1 Apr 2024 12:44:44 -0500 Subject: [PATCH 13/19] fix: Bug in `get_all_object_tags` When you add a new tags in a course, the previous tags are not exported How repodruce: - Import a course - Add new tags to the course or blocks - Export a course --- .../core/djangoapps/content_tagging/api.py | 7 +-- .../content_tagging/tests/test_api.py | 46 ++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index f94c42fcb64a..5a6bcfd14b3c 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -165,14 +165,15 @@ def get_all_object_tags( all_object_tags = list(ObjectTag.objects.filter( Q(object_id__startswith=block_id_prefix) | Q(object_id=content_key), Q(tag__isnull=False, tag__taxonomy__isnull=False), - ).select_related("tag__taxonomy")) + ).select_related("tag__taxonomy").order_by("object_id")) grouped_object_tags: ObjectTagByObjectIdDict = {} taxonomies: TaxonomyDict = {} for object_id, block_tags in groupby(all_object_tags, lambda x: x.object_id): grouped_object_tags[object_id] = {} - for taxonomy_id, taxonomy_tags in groupby(block_tags, lambda x: x.tag.taxonomy_id if x.tag else 0): + block_tags_sorted = sorted(block_tags, key=lambda x: x.tag.taxonomy_id if x.tag else 0) + for taxonomy_id, taxonomy_tags in groupby(block_tags_sorted, lambda x: x.tag.taxonomy_id if x.tag else 0): object_tags_list = list(taxonomy_tags) grouped_object_tags[object_id][taxonomy_id] = object_tags_list @@ -181,7 +182,7 @@ def get_all_object_tags( assert object_tags_list[0].tag.taxonomy taxonomies[taxonomy_id] = object_tags_list[0].tag.taxonomy - return grouped_object_tags, taxonomies + return grouped_object_tags, dict(sorted(taxonomies.items())) def set_object_tags( diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 50588c258134..b051117f045f 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -8,7 +8,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx_tagging.core.tagging.models import Tag +from openedx_tagging.core.tagging.models import Tag, ObjectTag from organizations.models import Organization from ..helpers.test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin @@ -261,6 +261,50 @@ def test_get_course_object_tags(self): self.taxonomy_2.id: self.taxonomy_2, } + def test_get_course_object_tags_with_add_tags(self): + """ + This test checks for an issue in get_all_object_tags: + If new tags are added to those already added previously, + the previous tags are lost. + This happens because the new tags will overwrite the old ones + in the result. + """ + # Tag in a new taxonomy + ObjectTag.objects.create( + object_id="block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", + taxonomy=self.taxonomy_1, + tag=self.tag_1_1, + ) + # Tag in a already tagged taxonomy + ObjectTag.objects.create( + object_id="block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", + taxonomy=self.taxonomy_2, + tag=self.tag_2_1, + ) + + with self.assertNumQueries(1): + object_tags, taxonomies = api.get_all_object_tags( + CourseKey.from_string("course-v1:orgA+test_course+test_run") + ) + + # Add new object tags to the expected result + self.expected_course_objecttags["block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1"] = { + self.taxonomy_1.id: list(api.get_object_tags( + "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", + taxonomy_id=self.taxonomy_1.id, + )), + self.taxonomy_2.id: list(api.get_object_tags( + "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1", + taxonomy_id=self.taxonomy_2.id, + )), + } + + assert object_tags == self.expected_course_objecttags + assert taxonomies == { + self.taxonomy_1.id: self.taxonomy_1, + self.taxonomy_2.id: self.taxonomy_2, + } + def test_get_library_object_tags(self): """ Test the get_all_object_tags function using a library From 4534de0bb8aacf4ee3cb0fe1465ec6212c32ee40 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 1 Apr 2024 13:18:30 -0500 Subject: [PATCH 14/19] style: Nits --- openedx/core/djangoapps/content_tagging/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 5a6bcfd14b3c..02fe7e4ede10 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -172,7 +172,7 @@ def get_all_object_tags( for object_id, block_tags in groupby(all_object_tags, lambda x: x.object_id): grouped_object_tags[object_id] = {} - block_tags_sorted = sorted(block_tags, key=lambda x: x.tag.taxonomy_id if x.tag else 0) + block_tags_sorted = sorted(block_tags, key=lambda x: x.tag.taxonomy_id if x.tag else 0) # type: ignore for taxonomy_id, taxonomy_tags in groupby(block_tags_sorted, lambda x: x.tag.taxonomy_id if x.tag else 0): object_tags_list = list(taxonomy_tags) grouped_object_tags[object_id][taxonomy_id] = object_tags_list From 5435985955916bd0f9bf89dd7bce96916bca3a46 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 2 Apr 2024 14:37:28 -0500 Subject: [PATCH 15/19] style: Fix types checks --- openedx/core/djangoapps/content_tagging/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index cc933ecf16e1..1a6662c52911 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -290,13 +290,13 @@ def set_exported_object_tags( if not tags_values: continue - taxonomy = oel_tagging.get_taxonomy_by_export_id(taxonomy_export_id) + taxonomy = oel_tagging.get_taxonomy_by_export_id(str(taxonomy_export_id)) oel_tagging.tag_object( object_id=content_key_str, taxonomy=taxonomy, tags=tags_values, create_invalid=True, - taxonomy_export_id=taxonomy_export_id, + taxonomy_export_id=str(taxonomy_export_id), ) From 8e8a5955568560c32051fef1c0314e708e5407c9 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 4 Apr 2024 14:24:24 -0500 Subject: [PATCH 16/19] refactor: Move test_objecttag_export_helpers.py to test directory --- .../djangoapps/content_tagging/rest_api/v1/tests/test_views.py | 3 +-- openedx/core/djangoapps/content_tagging/tests/test_api.py | 2 +- .../{helpers => tests}/test_objecttag_export_helpers.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) rename openedx/core/djangoapps/content_tagging/{helpers => tests}/test_objecttag_export_helpers.py (99%) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index aca76279229e..ebe453f5ca06 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -37,8 +37,7 @@ from openedx.core.djangoapps.content_tagging.utils import rules_cache from openedx.core.djangolib.testing.utils import skip_unless_cms - -from ....helpers.test_objecttag_export_helpers import TaggedCourseMixin +from ....tests.test_objecttag_export_helpers import TaggedCourseMixin User = get_user_model() diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 2bf6965f8a67..1bc80b73727a 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -9,7 +9,7 @@ from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag from organizations.models import Organization -from ..helpers.test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin +from .test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin from .. import api from ..utils import rules_cache diff --git a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/tests/test_objecttag_export_helpers.py similarity index 99% rename from openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py rename to openedx/core/djangoapps/content_tagging/tests/test_objecttag_export_helpers.py index 28027f774071..d3306844ac40 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_objecttag_export_helpers.py @@ -9,7 +9,7 @@ from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory from .. import api -from .objecttag_export_helpers import TaggedContent, build_object_tree_with_objecttags, iterate_with_level +from ..helpers.objecttag_export_helpers import TaggedContent, build_object_tree_with_objecttags, iterate_with_level from openedx_tagging.core.tagging.models import ObjectTag from organizations.models import Organization From bda8da23add07c08ceb9a9dbe4c1f131e9ec2be7 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 4 Apr 2024 14:34:28 -0500 Subject: [PATCH 17/19] fix: Fix tests --- .../contentstore/management/commands/tests/test_export_olx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py b/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py index 4d6990c69408..0484eaa324cc 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py @@ -17,7 +17,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from openedx.core.djangoapps.content_tagging.helpers.test_objecttag_export_helpers import TaggedCourseMixin +from openedx.core.djangoapps.content_tagging.tests.test_objecttag_export_helpers import TaggedCourseMixin class TestArgParsingCourseExportOlx(unittest.TestCase): From a271fde7be93ebbb3f36b927f867e1e38d2c74f6 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 8 Apr 2024 12:10:31 -0500 Subject: [PATCH 18/19] style: Nits on tests --- .../management/commands/tests/test_export_olx.py | 6 +++--- cms/djangoapps/contentstore/views/import_export.py | 5 ----- xmodule/modulestore/xml_importer.py | 3 ++- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py b/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py index 0484eaa324cc..8f5505ede5c1 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py @@ -63,7 +63,7 @@ def create_dummy_course(self, store_type): ) return course.id - def check_export_file(self, tar_file, course_key, withTags=False): + def check_export_file(self, tar_file, course_key, with_tags=False): """Check content of export file.""" names = tar_file.getnames() dirname = "{0.org}-{0.course}-{0.run}".format(course_key) @@ -73,7 +73,7 @@ def check_export_file(self, tar_file, course_key, withTags=False): self.assertIn(f"{dirname}/about/overview.html", names) self.assertIn(f"{dirname}/assets/assets.xml", names) self.assertIn(f"{dirname}/policies", names) - if withTags: + if with_tags: self.assertIn(f"{dirname}/tags.csv", names) else: self.assertNotIn(f"{dirname}/tags.csv", names) @@ -111,4 +111,4 @@ def test_export_course_with_tags(self): filename = tmp_dir / 'test.tar.gz' call_command('export_olx', '--output', filename, str(self.course.id)) with tarfile.open(filename) as tar_file: - self.check_export_file(tar_file, self.course.id, withTags=True) + self.check_export_file(tar_file, self.course.id, with_tags=True) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index e30865297d59..2543e2d8bbf2 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -344,11 +344,6 @@ def export_handler(request, course_key_string): requested_format = request.GET.get('_accept', request.META.get('HTTP_ACCEPT', 'text/html')) if request.method == 'POST': - if not has_view_object_tags_access(request.user, str(course_key)): - raise PermissionDenied( - "You do not have permission to view object tags for this object_id." - ) - export_olx.delay(request.user.id, course_key_string, request.LANGUAGE_CODE) return JsonResponse({'ExportStatus': 1}) elif 'text/html' in requested_format: diff --git a/xmodule/modulestore/xml_importer.py b/xmodule/modulestore/xml_importer.py index 0b821f793e2f..5b880b4ade2f 100644 --- a/xmodule/modulestore/xml_importer.py +++ b/xmodule/modulestore/xml_importer.py @@ -792,7 +792,8 @@ def import_tags(self, data_path, dest_id): """ Imports tags into library blocks """ - # TODO Implement function + # We don't support tags in v1 libraries, and v2 libraries don't have + # an import/export format defined yet. No action needed here for now. def import_course_from_xml(*args, **kwargs): From c80f6f7117cbcb872eb81f6b58dfb4219b744a6e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 8 Apr 2024 12:38:52 -0500 Subject: [PATCH 19/19] style: Nit on pylint --- cms/djangoapps/contentstore/views/import_export.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 2543e2d8bbf2..22310faa1ae2 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -39,7 +39,6 @@ from common.djangoapps.util.views import ensure_valid_course_key from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from openedx.core.djangoapps.content_tagging.auth import has_view_object_tags_access from ..storage import course_import_export_storage from ..tasks import CourseExportTask, CourseImportTask, export_olx, import_olx from ..toggles import use_new_export_page, use_new_import_page