From 940b072ae4b3d3adbf67787a2eda271e72d31412 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 25 Jan 2024 17:16:56 +0000 Subject: [PATCH 1/4] feat: Upgrade Python dependency edx-drf-extensions (#34114) make forgiving JWTs the default Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: robrap --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 03247da13a..16f8f8c262 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -462,7 +462,7 @@ edx-django-utils==5.9.0 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==9.1.2 +edx-drf-extensions==10.0.0 # via # -r requirements/edx/kernel.in # edx-completion diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index ab4f2b7fc3..d4f8b503b5 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -742,7 +742,7 @@ edx-django-utils==5.9.0 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==9.1.2 +edx-drf-extensions==10.0.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 6854009230..0742763f6f 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -540,7 +540,7 @@ edx-django-utils==5.9.0 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==9.1.2 +edx-drf-extensions==10.0.0 # via # -r requirements/edx/base.txt # edx-completion diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 59e30a42a4..868491a75d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -568,7 +568,7 @@ edx-django-utils==5.9.0 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==9.1.2 +edx-drf-extensions==10.0.0 # via # -r requirements/edx/base.txt # edx-completion From 4445aa6b1fd36935ee84821b65f652ece7f38516 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 25 Jan 2024 13:07:05 -0500 Subject: [PATCH 2/4] feat: bring in new client id configuration from ebk (#34115) --- requirements/edx/base.txt | 4 ++-- requirements/edx/development.txt | 4 ++-- requirements/edx/doc.txt | 4 ++-- requirements/edx/kernel.in | 4 ++-- requirements/edx/testing.txt | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 16f8f8c262..20a6922895 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -478,7 +478,7 @@ edx-enterprise==4.10.9 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in -edx-event-bus-kafka==5.5.0 +edx-event-bus-kafka==5.6.0 # via -r requirements/edx/kernel.in edx-event-bus-redis==0.3.2 # via -r requirements/edx/kernel.in @@ -770,7 +770,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/kernel.in openedx-django-wiki==2.0.3 # via -r requirements/edx/kernel.in -openedx-events==9.2.0 +openedx-events==9.3.0 # via # -r requirements/edx/kernel.in # edx-event-bus-kafka diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d4f8b503b5..65927c3a8e 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -760,7 +760,7 @@ edx-enterprise==4.10.9 # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-event-bus-kafka==5.5.0 +edx-event-bus-kafka==5.6.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1300,7 +1300,7 @@ openedx-django-wiki==2.0.3 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events==9.2.0 +openedx-events==9.3.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 0742763f6f..154c2d6775 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -556,7 +556,7 @@ edx-enterprise==4.10.9 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt -edx-event-bus-kafka==5.5.0 +edx-event-bus-kafka==5.6.0 # via -r requirements/edx/base.txt edx-event-bus-redis==0.3.2 # via -r requirements/edx/base.txt @@ -912,7 +912,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.0.3 # via -r requirements/edx/base.txt -openedx-events==9.2.0 +openedx-events==9.3.0 # via # -r requirements/edx/base.txt # edx-event-bus-kafka diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 744f1bd632..3139561ff8 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -73,8 +73,8 @@ edx-codejail edx-django-utils>=5.4.0 # Utilities for cache, monitoring, and plugins edx-drf-extensions edx-enterprise -# edx-event-bus-kafka 4.0.0 adds support for configurable consumer API -edx-event-bus-kafka>=4.0.1 # Kafka implementation of event bus +# edx-event-bus-kafka 5.6.0 adds support for putting client ids on event producers/consumers +edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus edx-event-bus-redis edx-milestones edx-name-affirmation diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 868491a75d..21cf769c83 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -584,7 +584,7 @@ edx-enterprise==4.10.9 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt -edx-event-bus-kafka==5.5.0 +edx-event-bus-kafka==5.6.0 # via -r requirements/edx/base.txt edx-event-bus-redis==0.3.2 # via -r requirements/edx/base.txt @@ -972,7 +972,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.0.3 # via -r requirements/edx/base.txt -openedx-events==9.2.0 +openedx-events==9.3.0 # via # -r requirements/edx/base.txt # edx-event-bus-kafka From 7535f9d58135ffa44aa199d472bcc4e7dcc8a5af Mon Sep 17 00:00:00 2001 From: Jillian Date: Fri, 26 Jan 2024 04:42:17 +1030 Subject: [PATCH 3/4] feat: tagging: serialize object permissions to REST API (#34004) --- .../core/djangoapps/content_tagging/api.py | 8 +- .../rest_api/v1/tests/test_views.py | 101 ++++++++++++++++-- .../content_tagging/rest_api/v1/views.py | 4 +- .../core/djangoapps/content_tagging/rules.py | 4 +- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 9 files changed, 106 insertions(+), 21 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 2a64f77b74..70aa7e2150 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -3,8 +3,6 @@ Content Tagging APIs """ from __future__ import annotations -from typing import Iterator - import openedx_tagging.core.tagging.api as oel_tagging from django.db.models import Q, QuerySet, Exists, OuterRef from openedx_tagging.core.tagging.models import Taxonomy @@ -101,7 +99,7 @@ def get_taxonomies_for_org( return oel_tagging.get_taxonomies(enabled=enabled).filter( Exists( TaxonomyOrg.get_relationships( - taxonomy=OuterRef("pk"), + taxonomy=OuterRef("pk"), # type: ignore rel_type=TaxonomyOrg.RelType.OWNER, org_short_name=org_short_name, ) @@ -130,7 +128,7 @@ def get_unassigned_taxonomies(enabled=True) -> QuerySet: def get_content_tags( object_key: ContentKey, taxonomy_id: int | None = None, -) -> Iterator[ContentObjectTag]: +) -> QuerySet: """ Generates a list of content tags for a given object. @@ -147,7 +145,7 @@ def tag_content_object( object_key: ContentKey, taxonomy: Taxonomy, tags: list, -) -> Iterator[ContentObjectTag]: +) -> QuerySet: """ This is the main API to use when you want to add/update/delete tags from a content object (e.g. an XBlock or course). 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 3ff0623eb8..20b1deb661 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 @@ -6,6 +6,7 @@ from __future__ import annotations from urllib.parse import parse_qs, urlparse import json +from unittest.mock import MagicMock import abc import ddt @@ -33,6 +34,7 @@ from openedx.core.djangoapps.content_libraries.api import ( create_library, set_library_user_permissions, ) +from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.content_tagging.models import TaxonomyOrg from openedx.core.djangolib.testing.utils import skip_unless_cms from openedx.core.lib import blockstore_api @@ -192,7 +194,7 @@ class TestTaxonomyObjectsMixin: rel_type=TaxonomyOrg.RelType.OWNER, ) - # Global taxonomy + # Global taxonomy, which contains tags self.t1 = Taxonomy.objects.create(name="t1", enabled=True) TaxonomyOrg.objects.create( taxonomy=self.t1, @@ -203,6 +205,12 @@ class TestTaxonomyObjectsMixin: taxonomy=self.t2, rel_type=TaxonomyOrg.RelType.OWNER, ) + root1 = Tag.objects.create(taxonomy=self.t1, value="ALPHABET") + Tag.objects.create(taxonomy=self.t1, value="android", parent=root1) + Tag.objects.create(taxonomy=self.t1, value="abacus", parent=root1) + Tag.objects.create(taxonomy=self.t1, value="azure", parent=root1) + Tag.objects.create(taxonomy=self.t1, value="aardvark", parent=root1) + Tag.objects.create(taxonomy=self.t1, value="anvil", parent=root1) # OrgA taxonomy self.tA1 = Taxonomy.objects.create(name="tA1", enabled=True) @@ -278,7 +286,8 @@ class TestTaxonomyListCreateViewSet(TestTaxonomyObjectsMixin, APITestCase): expected_taxonomies: list[str], enabled_parameter: bool | None = None, org_parameter: str | None = None, - unassigned_parameter: bool | None = None + unassigned_parameter: bool | None = None, + page_size: int | None = None, ) -> None: """ Helper function to call the list endpoint and check the response @@ -293,6 +302,7 @@ class TestTaxonomyListCreateViewSet(TestTaxonomyObjectsMixin, APITestCase): "enabled": enabled_parameter, "org": org_parameter, "unassigned": unassigned_parameter, + "page_size": page_size, }.items() if v is not None} response = self.client.get(url, query_params, format="json") @@ -304,11 +314,12 @@ class TestTaxonomyListCreateViewSet(TestTaxonomyObjectsMixin, APITestCase): """ Tests that staff users see all taxonomies """ - # Default page_size=10, and so "tBA1" and "tBA2" appear on the second page + # page_size=10, and so "tBA1" and "tBA2" appear on the second page expected_taxonomies = ["ot1", "ot2", "st1", "st2", "t1", "t2", "tA1", "tA2", "tB1", "tB2"] self._test_list_taxonomy( user_attr="staff", expected_taxonomies=expected_taxonomies, + page_size=10, ) @ddt.data( @@ -476,6 +487,29 @@ class TestTaxonomyListCreateViewSet(TestTaxonomyObjectsMixin, APITestCase): if user_attr == "staffA": assert response.data["orgs"] == [self.orgA.short_name] + def test_list_taxonomy_query_count(self): + """ + Test how many queries are used when retrieving taxonomies and permissions + """ + url = TAXONOMY_ORG_LIST_URL + f'?org=${self.orgA.short_name}&enabled=true' + + self.client.force_authenticate(user=self.staff) + with self.assertNumQueries(16): # TODO Why so many queries? + response = self.client.get(url) + + assert response.status_code == 200 + assert response.data["can_add_taxonomy"] + assert len(response.data["results"]) == 2 + for taxonomy in response.data["results"]: + if taxonomy["system_defined"]: + assert not taxonomy["can_change_taxonomy"] + assert not taxonomy["can_delete_taxonomy"] + assert taxonomy["can_tag_object"] + else: + assert taxonomy["can_change_taxonomy"] + assert taxonomy["can_delete_taxonomy"] + assert taxonomy["can_tag_object"] + @ddt.ddt class TestTaxonomyDetailExportMixin(TestTaxonomyObjectsMixin): @@ -787,7 +821,14 @@ class TestTaxonomyDetailViewSet(TestTaxonomyDetailExportMixin, APITestCase): assert response.status_code == expected_status, reason if status.is_success(expected_status): - check_taxonomy(response.data, taxonomy.pk, **(TaxonomySerializer(taxonomy.cast()).data)) + request = MagicMock() + request.user = user + context = {"request": request} + check_taxonomy( + response.data, + taxonomy.pk, + **(TaxonomySerializer(taxonomy.cast(), context=context)).data, + ) @skip_unless_cms @@ -1538,12 +1579,12 @@ class TestObjectTagViewSet(TestObjectTagMixin, APITestCase): # Fetch this object's tags for a single taxonomy expected_tags = [{ - 'editable': True, 'name': 'Multiple Taxonomy', 'taxonomy_id': taxonomy.pk, + 'can_tag_object': True, 'tags': [ - {'value': 'Tag 1', 'lineage': ['Tag 1']}, - {'value': 'Tag 2', 'lineage': ['Tag 2']}, + {'value': 'Tag 1', 'lineage': ['Tag 1'], 'can_delete_objecttag': True}, + {'value': 'Tag 2', 'lineage': ['Tag 2'], 'can_delete_objecttag': True}, ], }] @@ -1560,6 +1601,28 @@ class TestObjectTagViewSet(TestObjectTagMixin, APITestCase): assert status.is_success(response3.status_code) assert response3.data[str(self.courseA)]["taxonomies"] == expected_tags + def test_object_tags_query_count(self): + """ + Test how many queries are used when retrieving object tags and permissions + """ + object_key = self.courseA + object_id = str(object_key) + tagging_api.tag_content_object(object_key=object_key, taxonomy=self.t1, tags=["anvil", "android"]) + expected_tags = [ + {"value": "android", "lineage": ["ALPHABET", "android"], "can_delete_objecttag": True}, + {"value": "anvil", "lineage": ["ALPHABET", "anvil"], "can_delete_objecttag": True}, + ] + + url = OBJECT_TAGS_URL.format(object_id=object_id) + self.client.force_authenticate(user=self.staff) + with self.assertNumQueries(7): # TODO Why so many queries? + response = self.client.get(url) + + assert response.status_code == 200 + assert len(response.data[object_id]["taxonomies"]) == 1 + assert response.data[object_id]["taxonomies"][0]["can_tag_object"] + assert response.data[object_id]["taxonomies"][0]["tags"] == expected_tags + @skip_unless_cms @ddt.ddt @@ -2029,3 +2092,27 @@ class TestImportTagsView(ImportTaxonomyMixin, APITestCase): assert len(tags) == len(self.old_tags) for i, tag in enumerate(tags): assert tag["value"] == self.old_tags[i].value + + +@skip_unless_cms +@ddt.ddt +class TestTaxonomyTagsViewSet(TestTaxonomyObjectsMixin, APITestCase): + """ + Test cases for TaxonomyTagsViewSet retrive action. + """ + def test_taxonomy_tags_query_count(self): + """ + Test how many queries are used when retrieving small taxonomies+tags and permissions + """ + url = f"{TAXONOMY_TAGS_URL}?search_term=an&parent_tag=ALPHABET".format(pk=self.t1.id) + + self.client.force_authenticate(user=self.staff) + with self.assertNumQueries(13): # TODO Why so many queries? + response = self.client.get(url) + + assert response.status_code == status.HTTP_200_OK + assert response.data["can_add_tag"] + assert len(response.data["results"]) == 2 + for taxonomy in response.data["results"]: + assert taxonomy["can_change_tag"] + assert taxonomy["can_delete_tag"] 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 f04256f4a1..151bc09f5d 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -81,11 +81,11 @@ class TaxonomyOrgView(TaxonomyView): serializer.instance = create_taxonomy(**serializer.validated_data, orgs=user_admin_orgs) @action(detail=False, url_path="import", methods=["post"]) - def create_import(self, request: Request, **kwargs) -> Response: + def create_import(self, request: Request, **kwargs) -> Response: # type: ignore """ Creates a new taxonomy with the given orgs and imports the tags from the uploaded file. """ - response = super().create_import(request, **kwargs) + response = super().create_import(request=request, **kwargs) # type: ignore # If creation was successful, set the orgs for the new taxonomy if status.is_success(response.status_code): diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index f3d0b70c30..af6bdbeb94 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -219,7 +219,7 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: Everyone that has permission to edit the object should be able to tag it. """ if not object_id: - raise ValueError("object_id must be provided") + return True try: usage_key = UsageKey.from_string(object_id) if not usage_key.course_key.is_course: @@ -274,7 +274,7 @@ def can_change_taxonomy_tag(user: UserType, tag: oel_tagging.Tag | None = None) return oel_tagging.is_taxonomy_admin(user) and ( not tag or not taxonomy - or (taxonomy and not taxonomy.allow_free_text and not taxonomy.system_defined) + or (bool(taxonomy) and not taxonomy.allow_free_text and not taxonomy.system_defined) ) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 453c040c20..f0b2dd767f 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -103,7 +103,7 @@ libsass==0.10.0 click==8.1.6 # pinning this version to avoid updates while the library is being developed -openedx-learning==0.4.2 +openedx-learning==0.4.4 # 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 20a6922895..4731fcfed6 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -779,7 +779,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.4.2 +openedx-learning==0.4.4 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 65927c3a8e..50b40cf849 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1311,7 +1311,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.4.2 +openedx-learning==0.4.4 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 154c2d6775..677a299aee 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -921,7 +921,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.4.2 +openedx-learning==0.4.4 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 21cf769c83..4358a89d7d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -981,7 +981,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.4.2 +openedx-learning==0.4.4 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 5838d68efc1bd29d9dda593d1d0a1306654e58ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Thu, 25 Jan 2024 13:33:47 -0500 Subject: [PATCH 4/4] feat: Tagging UX refinements - refresh tag count on edit (#34059) * style: drawer-cover color updated for all tagging drawers * feat: Update TagList component when a tag is updated on Manage tags drawer * feat: Refactor TagCount to be able to refresh the count * feat: Sync tag count in units --- .../contentstore/tests/test_contentstore.py | 6 +- cms/djangoapps/contentstore/views/preview.py | 1 + .../contentstore/views/tests/test_block.py | 8 +- .../xblock_storage_handlers/view_handlers.py | 1 + cms/static/js/factories/tag_count.js | 13 ++++ cms/static/js/models/tag_count.js | 13 ++++ cms/static/js/views/course_outline.js | 27 +++++-- cms/static/js/views/pages/container.js | 1 + .../js/views/pages/container_subviews.js | 77 +++++++++++++++++++ cms/static/js/views/tag_count.js | 54 +++++++++++++ cms/static/sass/elements/_drawer.scss | 4 + cms/templates/container.html | 2 +- cms/templates/course_outline.html | 6 +- cms/templates/js/course-outline.underscore | 13 +--- cms/templates/js/tag-count.underscore | 7 ++ cms/templates/studio_xblock_wrapper.html | 23 ++++-- .../content_tagging/rest_api/v1/urls.py | 2 + webpack.common.config.js | 1 + 18 files changed, 221 insertions(+), 38 deletions(-) create mode 100644 cms/static/js/factories/tag_count.js create mode 100644 cms/static/js/models/tag_count.js create mode 100644 cms/static/js/views/tag_count.js create mode 100644 cms/templates/js/tag-count.underscore diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b82605f893..9f90840156 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -49,7 +49,6 @@ from cms.djangoapps.contentstore.utils import ( delete_course, reverse_course_url, reverse_url, - get_taxonomy_tags_widget_url, ) from cms.djangoapps.contentstore.views.component import ADVANCED_COMPONENT_TYPES from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError @@ -1416,15 +1415,12 @@ class ContentStoreTest(ContentStoreTestCase): course.location.course_key ) - taxonomy_tags_widget_url = get_taxonomy_tags_widget_url(course.id) - self.assertContains( resp, - '
'.format( # lint-amnesty, pylint: disable=line-too-long + '
'.format( # lint-amnesty, pylint: disable=line-too-long locator=str(course.location), course_key=str(course.id), assets_url=assets_url, - taxonomy_tags_widget_url=taxonomy_tags_widget_url, ), status_code=200, html=True diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index a897a38ad2..9c9926a5b2 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -315,6 +315,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_reorderable': is_reorderable, 'can_edit': can_edit, 'can_edit_visibility': context.get('can_edit_visibility', is_course), + 'course_authoring_url': settings.COURSE_AUTHORING_MICROFRONTEND_URL, 'is_loading': context.get('is_loading', False), 'is_selected': context.get('is_selected', False), 'selectable': context.get('selectable', False), diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index ac86961b22..c8ac9b89dc 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -288,15 +288,9 @@ class GetItemTest(ItemTest): self.assertEqual(resp.status_code, 200) usage_key = self.response_usage_key(resp) - # Get the preview HTML without tags - mock_get_object_tag_counts.return_value = {} - html, __ = self._get_container_preview(root_usage_key) - self.assertIn("wrapper-xblock", html) - self.assertNotIn('data-testid="tag-count-button"', html) - # Get the preview HTML with tags mock_get_object_tag_counts.return_value = { - str(usage_key): 13 + str(usage_key): 13, } html, __ = self._get_container_preview(root_usage_key) self.assertIn("wrapper-xblock", html) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index eb79181c46..8b122d8c8d 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -1206,6 +1206,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements xblock_info["tags"] = tags if use_tagging_taxonomy_list_page(): xblock_info["taxonomy_tags_widget_url"] = get_taxonomy_tags_widget_url() + xblock_info["course_authoring_url"] = settings.COURSE_AUTHORING_MICROFRONTEND_URL if course_outline: if xblock_info["has_explicit_staff_lock"]: diff --git a/cms/static/js/factories/tag_count.js b/cms/static/js/factories/tag_count.js new file mode 100644 index 0000000000..cadcfa220f --- /dev/null +++ b/cms/static/js/factories/tag_count.js @@ -0,0 +1,13 @@ +import * as TagCountView from 'js/views/tag_count'; +import * as TagCountModel from 'js/models/tag_count'; + +// eslint-disable-next-line no-unused-expressions +'use strict'; +export default function TagCountFactory(TagCountJson, el) { + var model = new TagCountModel(TagCountJson, {parse: true}); + var tagCountView = new TagCountView({el, model}); + tagCountView.setupMessageListener(); + tagCountView.render(); +} + +export {TagCountFactory}; diff --git a/cms/static/js/models/tag_count.js b/cms/static/js/models/tag_count.js new file mode 100644 index 0000000000..7007dfc9dc --- /dev/null +++ b/cms/static/js/models/tag_count.js @@ -0,0 +1,13 @@ +define(['backbone', 'underscore'], function(Backbone, _) { + /** + * Model for Tag count view + */ + var TagCountModel = Backbone.Model.extend({ + defaults: { + content_id: null, + tags_count: 0, + course_authoring_url: null, + }, + }); + return TagCountModel; +}); diff --git a/cms/static/js/views/course_outline.js b/cms/static/js/views/course_outline.js index 4b7107cc4d..04dc985130 100644 --- a/cms/static/js/views/course_outline.js +++ b/cms/static/js/views/course_outline.js @@ -12,11 +12,11 @@ define(['jquery', 'underscore', 'js/views/xblock_outline', 'edx-ui-toolkit/js/ut 'common/js/components/utils/view_utils', 'js/views/utils/xblock_utils', 'js/models/xblock_outline_info', 'js/views/modals/course_outline_modals', 'js/utils/drag_and_drop', 'common/js/components/views/feedback_notification', 'common/js/components/views/feedback_prompt', - 'js/views/utils/tagging_drawer_utils',], + 'js/views/utils/tagging_drawer_utils', 'js/views/tag_count', 'js/models/tag_count'], function( $, _, XBlockOutlineView, StringUtils, ViewUtils, XBlockViewUtils, XBlockOutlineInfo, CourseOutlineModalsFactory, ContentDragger, NotificationView, PromptView, - TaggingDrawerUtils + TaggingDrawerUtils, TagCountView, TagCountModel ) { var CourseOutlineView = XBlockOutlineView.extend({ // takes XBlockOutlineInfo as a model @@ -28,9 +28,28 @@ function( this.makeContentDraggable(this.el); // Show/hide the paste button this.initializePasteButton(this.el); + this.renderTagCount(); return renderResult; }, + renderTagCount: function() { + const contentId = this.model.get('id'); + const tagCountsByUnit = this.model.get('tag_counts_by_unit') + const tagsCount = tagCountsByUnit !== undefined ? tagCountsByUnit[contentId] : 0 + var countModel = new TagCountModel({ + content_id: contentId, + tags_count: tagsCount, + course_authoring_url: this.model.get('course_authoring_url'), + }, {parse: true}); + var tagCountView = new TagCountView({el: this.$('.tag-count'), model: countModel}); + tagCountView.setupMessageListener(); + tagCountView.render(); + this.$('.tag-count').click((event) => { + event.preventDefault(); + this.openManageTagsDrawer(); + }); + }, + shouldExpandChildren: function() { return this.expandedLocators.contains(this.model.get('id')); }, @@ -461,10 +480,8 @@ function( }, openManageTagsDrawer() { - const article = document.querySelector('[data-taxonomy-tags-widget-url]'); - const taxonomyTagsWidgetUrl = $(article).attr('data-taxonomy-tags-widget-url'); + const taxonomyTagsWidgetUrl = this.model.get('taxonomy_tags_widget_url'); const contentId = this.model.get('id'); - TaggingDrawerUtils.openDrawer(taxonomyTagsWidgetUrl, contentId); }, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 783133af21..3268b60e41 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -111,6 +111,7 @@ function($, _, Backbone, gettext, BasePage, el: this.$('.unit-tags'), model: this.model }); + this.tagListView.setupMessageListener(); this.tagListView.render(); this.unitOutlineView = new UnitOutlineView({ diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index 8848abc246..7ea09eff08 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -370,6 +370,83 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H } }, + setupMessageListener: function () { + window.addEventListener( + "message", (event) => { + // Listen any message from Manage tags drawer. + var data = event.data; + var courseAuthoringUrl = this.model.get("course_authoring_url") + if (event.origin == courseAuthoringUrl + && data.includes('[Manage tags drawer] Tags updated:')) { + // This message arrives when there is a change in the tag list. + // The message contains the new list of tags. + let jsonData = data.replace(/\[Manage tags drawer\] Tags updated: /g, ""); + jsonData = JSON.parse(jsonData); + if (jsonData.contentId == this.model.id) { + this.model.set('tags', this.buildTaxonomyTree(jsonData)); + this.render(); + } + } + }, + ); + }, + + buildTaxonomyTree: function(data) { + // TODO We can use this function for the initial request of tags + // and avoid to use two functions (see get_unit_tags on contentstore/views/component.py) + + var taxonomyList = []; + var totalCount = 0; + var actualId = 0; + data.taxonomies.forEach((taxonomy) => { + // Build a tag tree for each taxonomy + var rootTagsValues = []; + var tags = {}; + taxonomy.tags.forEach((tag) => { + // Creates the tags for all the lineage of this tag + for (let i = tag.lineage.length - 1; i >= 0; i--){ + var tagValue = tag.lineage[i] + var tagProcessedBefore = tags.hasOwnProperty(tagValue); + if (!tagProcessedBefore) { + tags[tagValue] = { + id: actualId, + value: tagValue, + children: [], + } + actualId++; + if (i == 0) { + rootTagsValues.push(tagValue); + } + } + if (i !== tag.lineage.length - 1) { + // Add a child into the children list + tags[tagValue].children.push(tags[tag.lineage[i + 1]]) + } + if (tagProcessedBefore) { + // Break this loop if the tag has been processed before, + // we don't need to process lineage again to avoid duplicates. + break; + } + } + }) + + var tagCount = Object.keys(tags).length; + // Add the tree to the taxonomy list + taxonomyList.push({ + id: taxonomy.taxonomyId, + value: taxonomy.name, + tags: rootTagsValues.map(rootValue => tags[rootValue]), + count: tagCount, + }); + totalCount += tagCount; + }); + + return { + count: totalCount, + taxonomies: taxonomyList, + }; + }, + handleKeyDownOnHeader: function(event) { if (event.key === 'Enter' || event.key === ' ') { event.preventDefault(); diff --git a/cms/static/js/views/tag_count.js b/cms/static/js/views/tag_count.js new file mode 100644 index 0000000000..c7ba4d79e5 --- /dev/null +++ b/cms/static/js/views/tag_count.js @@ -0,0 +1,54 @@ +define(['jquery', 'underscore', 'js/views/baseview', 'edx-ui-toolkit/js/utils/html-utils'], +function($, _, BaseView, HtmlUtils) { + 'use strict'; + + /** + * TagCountView displays the tag count of a unit/component + * + * This component is being rendered in this way to allow receiving + * messages from the Manage tags drawer and being able to update the count. + */ + var TagCountView = BaseView.extend({ + // takes TagCountModel as a model + + initialize: function() { + BaseView.prototype.initialize.call(this); + this.template = this.loadTemplate('tag-count'); + }, + + setupMessageListener: function () { + window.addEventListener( + 'message', (event) => { + // Listen any message from Manage tags drawer. + var data = event.data; + var courseAuthoringUrl = this.model.get("course_authoring_url") + if (event.origin == courseAuthoringUrl + && data.includes('[Manage tags drawer] Count updated:')) { + // This message arrives when there is a change in the tag list. + // The message contains the new count of tags. + let jsonData = data.replace(/\[Manage tags drawer\] Count updated: /g, ""); + jsonData = JSON.parse(jsonData); + if (jsonData.contentId == this.model.get("content_id")) { + this.model.set('tags_count', jsonData.count); + this.render(); + } + } + } + ); + }, + + render: function() { + HtmlUtils.setHtml( + this.$el, + HtmlUtils.HTML( + this.template({ + tags_count: this.model.get("tags_count"), + }) + ) + ); + return this; + } + }); + + return TagCountView; +}); diff --git a/cms/static/sass/elements/_drawer.scss b/cms/static/sass/elements/_drawer.scss index c18073be98..96edfe1983 100644 --- a/cms/static/sass/elements/_drawer.scss +++ b/cms/static/sass/elements/_drawer.scss @@ -13,6 +13,10 @@ background: rgba(0, 0, 0, 0.8); } +.drawer-cover.gray-cover { + background: rgba(112, 112, 112, 0.8); +} + .drawer { @extend %ui-depth4; diff --git a/cms/templates/container.html b/cms/templates/container.html index cd9530ed51..d61ce60e91 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -281,5 +281,5 @@ from openedx.core.djangolib.markup import HTML, Text
-
+
diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index e5959867be..fbcb2941b2 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -29,7 +29,7 @@ from django.urls import reverse <%block name="header_extras"> -% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'course-video-sharing-enable', 'summary-configuration-editor']: +% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'course-video-sharing-enable', 'summary-configuration-editor', 'tag-count']: @@ -281,7 +281,7 @@ from django.urls import reverse assets_url = reverse('assets_handler', kwargs={'course_key_string': str(course_locator.course_key)}) %>

${_("Course Outline")}

-
+
@@ -323,5 +323,5 @@ from django.urls import reverse
-
+
diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index be5e147754..c62979bced 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -7,7 +7,8 @@ var hasPartitionGroups = xblockInfo.get('has_partition_group_components'); var userPartitionInfo = xblockInfo.get('user_partition_info'); var selectedGroupsLabel = userPartitionInfo['selected_groups_label']; var selectedPartitionIndex = userPartitionInfo['selected_partition_index']; -var tagsCount = (xblockInfo.get('tag_counts_by_unit') || {})[xblockInfo.get('id')] || 0; +var xblockId = xblockInfo.get('id') +var tagsCount = (xblockInfo.get('tag_counts_by_unit') || {})[xblockId] || 0; var statusMessages = []; var messageType; @@ -171,14 +172,8 @@ if (is_proctored_exam) { <% } %> - <% if (xblockInfo.isVertical() && typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage && tagsCount > 0) { %> -
  • - - - <%- tagsCount %> - <%- gettext('Manage Tags') %> - -
    • + <% if (xblockInfo.isVertical() && typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage) { %> +
  • <% } %> <% if (typeof enableCopyPasteUnits !== "undefined" && enableCopyPasteUnits) { %> diff --git a/cms/templates/js/tag-count.underscore b/cms/templates/js/tag-count.underscore new file mode 100644 index 0000000000..253323109f --- /dev/null +++ b/cms/templates/js/tag-count.underscore @@ -0,0 +1,7 @@ +<% if (tags_count && tags_count > 0) { %> + +<% } %> diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 0d6cafc95d..9ae3a3a5dd 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -29,6 +29,9 @@ block_is_unit = is_unit(xblock) + +<%static:webpack entry="js/factories/tag_count"> + TagCountFactory({ + tags_count: "${tags_count | n, js_escaped_string}", + content_id: "${xblock.location | n, js_escaped_string}", + course_authoring_url: "${course_authoring_url | n, js_escaped_string}", + }, + $('li.tag-count[data-locator="${xblock.location | n, js_escaped_string}"]') + ); + + % if not is_root: % if is_reorderable:
  • @@ -99,14 +112,8 @@ block_is_unit = is_unit(xblock)
      % if not is_root: % if can_edit: - % if use_tagging and tags_count: -
    • - -
      • + % if use_tagging: +
    • % endif % if not show_inline:
    • diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py index 53addf281c..ad7fd8005c 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py @@ -3,6 +3,7 @@ Taxonomies API v1 URLs. """ from rest_framework.routers import DefaultRouter +from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagCountsView from django.urls.conf import path, include @@ -16,6 +17,7 @@ from . import views router = DefaultRouter() router.register("taxonomies", views.TaxonomyOrgView, basename="taxonomy") router.register("object_tags", views.ObjectTagOrgView, basename="object_tag") +router.register("object_tag_counts", ObjectTagCountsView, basename="object_tag_counts") urlpatterns = [ path( diff --git a/webpack.common.config.js b/webpack.common.config.js index 239cfb4f2e..075bfcf827 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -84,6 +84,7 @@ module.exports = Merge.smart({ 'js/factories/xblock_validation': './cms/static/js/factories/xblock_validation.js', 'js/factories/edit_tabs': './cms/static/js/factories/edit_tabs.js', 'js/sock': './cms/static/js/sock.js', + 'js/factories/tag_count': './cms/static/js/factories/tag_count.js', // LMS SingleSupportForm: './lms/static/support/jsx/single_support_form.jsx',