feat: Enable taxonomy/tagging feature in MFE by default (#34633)

* feat: make tagging feature enabled by default * fix: use the correct flag for tagging enabled * fix: make compatible with other changes from master * fix: more compatibility fixes * fix: show tag counts at all levels of the outline, not just units * chore: typo * test: fix counts in test suite now that tagging is on by default --------- Co-authored-by: Braden MacDonald <[email protected]> Co-authored-by: Yusuf Musleh <[email protected]>
openedx · May 9, 2024 · b9e5a2f · b9e5a2f
1 parent 463169b
commit b9e5a2f
Show file tree

Hide file tree

Showing 17 changed files with 84 additions and 76 deletions.
diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py
@@ -5,11 +5,11 @@
 from django.urls import reverse
 from rest_framework import serializers
 
-from cms.djangoapps.contentstore.toggles import use_tagging_taxonomy_list_page
 from cms.djangoapps.contentstore.helpers import (
     xblock_studio_url,
     xblock_type_display_name,
 )
+from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 
 
 class MessageValidation(serializers.Serializer):
@@ -122,7 +122,7 @@ def get_actions(self, obj):  # pylint: disable=unused-argument
         Method to get actions for each child xlock of the unit.
         """
 
-        can_manage_tags = use_tagging_taxonomy_list_page()
+        can_manage_tags = not is_tagging_feature_disabled()
         xblock = obj["xblock"]
         is_course = xblock.scope_ids.usage_id.context_key.is_course
         xblock_url = xblock_studio_url(xblock)

diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py
@@ -150,6 +150,6 @@ def test_number_of_calls_to_db(self):
         """
         Test to check number of queries made to mysql and mongo
         """
-        with self.assertNumQueries(29, table_ignorelist=WAFFLE_TABLES):
+        with self.assertNumQueries(32, table_ignorelist=WAFFLE_TABLES):
             with check_mongo_calls(3):
                 self.client.get(self.url)
diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py
@@ -8,14 +8,12 @@
 from django.urls import reverse
 from edx_toggles.toggles.testutils import (
     override_waffle_switch,
-    override_waffle_flag,
 )
 from rest_framework import status
 
 from cms.djangoapps.contentstore.tests.utils import CourseTestCase
 from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase
 from cms.djangoapps.contentstore.views.course import ENABLE_GLOBAL_STAFF_OPTIMIZATION
-from cms.djangoapps.contentstore.toggles import ENABLE_TAGGING_TAXONOMY_LIST_PAGE
 from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -52,9 +50,9 @@ def test_home_page_courses_response(self):
             "in_process_course_actions": [],
             "libraries": [],
             "libraries_enabled": True,
-            "taxonomies_enabled": False,
+            "taxonomies_enabled": True,
             "library_authoring_mfe_url": settings.LIBRARY_AUTHORING_MICROFRONTEND_URL,
-            "taxonomy_list_mfe_url": None,
+            "taxonomy_list_mfe_url": 'http://course-authoring-mfe/taxonomies',
             "optimization_enabled": False,
             "redirect_to_library_authoring_mfe": False,
             "request_course_creator_url": "/request_course_creator",
@@ -72,7 +70,6 @@ def test_home_page_courses_response(self):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertDictEqual(expected_response, response.data)
 
-    @override_waffle_flag(ENABLE_TAGGING_TAXONOMY_LIST_PAGE, True)
     def test_taxonomy_list_link(self):
         response = self.client.get(self.url)
         self.assertTrue(response.data['taxonomies_enabled'])

diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py
@@ -8,7 +8,7 @@
 from xblock.validation import ValidationMessage
 
 from cms.djangoapps.contentstore.tests.utils import CourseTestCase
-from cms.djangoapps.contentstore.toggles import ENABLE_TAGGING_TAXONOMY_LIST_PAGE
+from openedx.core.djangoapps.content_tagging.toggles import DISABLE_TAGGING_FEATURE
 from xmodule.partitions.partitions import (
     ENROLLMENT_TRACK_PARTITION_ID,
     Group,
@@ -164,7 +164,6 @@ def test_xblock_is_published(self):
         response = self.client.get(url)
         self.assertTrue(response.data["is_published"])
 
-    @override_waffle_flag(ENABLE_TAGGING_TAXONOMY_LIST_PAGE, True)
     def test_children_content(self):
         """
         Check that returns valid response with children of vertical container.
@@ -238,7 +237,7 @@ def test_not_valid_usage_key_string(self):
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
-    @override_waffle_flag(ENABLE_TAGGING_TAXONOMY_LIST_PAGE, False)
+    @override_waffle_flag(DISABLE_TAGGING_FEATURE, True)
     def test_actions_with_turned_off_taxonomy_flag(self):
         """
         Check that action manage_tags for each child item has the same value as taxonomy flag.

diff --git a/cms/djangoapps/contentstore/toggles.py b/cms/djangoapps/contentstore/toggles.py
@@ -581,21 +581,3 @@ def default_enable_flexible_peer_openassessments(course_key):
     level to opt in/out of rolling forward this feature.
     """
     return DEFAULT_ENABLE_FLEXIBLE_PEER_OPENASSESSMENTS.is_enabled(course_key)
-
-
-# .. toggle_name: new_studio_mfe.use_tagging_taxonomy_list_page
-# .. toggle_implementation: WaffleFlag
-# .. toggle_default: False
-# .. toggle_description: This flag enables the use of the taxonomy list page.
-# .. toggle_use_cases: temporary
-# .. toggle_creation_date: 2023-10-06
-# .. toggle_target_removal_date: TBA
-# .. toggle_warning:
-ENABLE_TAGGING_TAXONOMY_LIST_PAGE = WaffleFlag('new_studio_mfe.use_tagging_taxonomy_list_page', __name__)
-
-
-def use_tagging_taxonomy_list_page():
-    """
-    Returns a boolean if taxonomy list page is enabled
-    """
-    return ENABLE_TAGGING_TAXONOMY_LIST_PAGE.is_enabled()
diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py
@@ -58,6 +58,7 @@
 from common.djangoapps.xblock_django.api import deprecated_xblocks
 from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
 from openedx.core import toggles as core_toggles
+from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course
 from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND
 from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
@@ -91,7 +92,6 @@
     use_new_video_editor,
     use_new_video_uploads_page,
     use_new_custom_pages,
-    use_tagging_taxonomy_list_page,
 )
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
@@ -489,16 +489,19 @@ def get_custom_pages_url(course_locator) -> str:
     return custom_pages_url
 
 
-def get_taxonomy_list_url():
+def get_taxonomy_list_url() -> str | None:
     """
     Gets course authoring microfrontend URL for taxonomy list page view.
     """
-    taxonomy_list_url = None
-    if use_tagging_taxonomy_list_page():
-        mfe_base_url = settings.COURSE_AUTHORING_MICROFRONTEND_URL
-        if mfe_base_url:
-            taxonomy_list_url = f'{mfe_base_url}/taxonomies'
-    return taxonomy_list_url
+    if is_tagging_feature_disabled():
+        return None
+
+    mfe_base_url = settings.COURSE_AUTHORING_MICROFRONTEND_URL
+
+    if not mfe_base_url:
+        return None
+
+    return f'{mfe_base_url}/taxonomies'
 
 
 def get_taxonomy_tags_widget_url(course_locator=None) -> str | None:
@@ -507,15 +510,18 @@ def get_taxonomy_tags_widget_url(course_locator=None) -> str | None:
 
     The `content_id` needs to be appended to the end of the URL when using it.
     """
-    taxonomy_tags_widget_url = None
-    # Uses the same waffle flag as taxonomy list page
-    if use_tagging_taxonomy_list_page():
+    if is_tagging_feature_disabled():
+        return None
+
+    if course_locator:
+        mfe_base_url = get_course_authoring_url(course_locator)
+    else:
         mfe_base_url = settings.COURSE_AUTHORING_MICROFRONTEND_URL
-        if course_locator:
-            mfe_base_url = get_course_authoring_url(course_locator)
-        if mfe_base_url:
-            taxonomy_tags_widget_url = f'{mfe_base_url}/tagging/components/widget/'
-    return taxonomy_tags_widget_url
+
+    if not mfe_base_url:
+        return None
+
+    return f'{mfe_base_url}/tagging/components/widget/'
 
 
 def course_import_olx_validation_is_enabled():
@@ -1688,7 +1694,7 @@ def get_home_context(request, no_course=False):
         'archived_courses': archived_courses,
         'in_process_course_actions': in_process_course_actions,
         'libraries_enabled': LIBRARIES_ENABLED,
-        'taxonomies_enabled': use_tagging_taxonomy_list_page(),
+        'taxonomies_enabled': not is_tagging_feature_disabled(),
         'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(),
         'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL,
         'taxonomy_list_mfe_url': get_taxonomy_list_url(),
@@ -1984,7 +1990,7 @@ def get_container_handler_context(request, usage_key, course, xblock):  # pylint
     prev_url = quote_plus(prev_url) if prev_url else None
     next_url = quote_plus(next_url) if next_url else None
 
-    show_unit_tags = use_tagging_taxonomy_list_page()
+    show_unit_tags = not is_tagging_feature_disabled()
     unit_tags = None
     if show_unit_tags and is_unit_page:
         unit_tags = get_unit_tags(usage_key)

diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py
@@ -29,7 +29,7 @@
 from xmodule.modulestore.django import (
     modulestore,
 )  # lint-amnesty, pylint: disable=wrong-import-order
-from cms.djangoapps.contentstore.toggles import use_tagging_taxonomy_list_page
+from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 
 from xmodule.x_module import (
     AUTHOR_VIEW,
@@ -242,7 +242,7 @@ def xblock_view_handler(request, usage_key_string, view_name):
 
             # Fetch tags of children components
             tags_count_map = {}
-            if use_tagging_taxonomy_list_page():
+            if not is_tagging_feature_disabled():
                 tags_count_map = get_children_tags_count(xblock)
 
             # Set up the context to be passed to each XBlock's render method.

diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py
@@ -17,7 +17,6 @@
 from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
 from openedx_events.tests.utils import OpenEdxEventsTestMixin
 from edx_proctoring.exceptions import ProctoredExamNotFoundException
-from edx_toggles.toggles.testutils import override_waffle_flag
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.asides import AsideUsageKeyV2
 from opaque_keys.edx.keys import CourseKey, UsageKey
@@ -85,7 +84,6 @@
     add_container_page_publishing_info,
     create_xblock_info,
 )
-from cms.djangoapps.contentstore.toggles import ENABLE_TAGGING_TAXONOMY_LIST_PAGE
 
 
 class AsideTest(XBlockAside):
@@ -272,7 +270,6 @@ def test_get_container_nested_container_fragment(self):
             ),
         )
 
-    @override_waffle_flag(ENABLE_TAGGING_TAXONOMY_LIST_PAGE, True)
     @patch("cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers.get_object_tag_counts")
     def test_tag_count_in_container_fragment(self, mock_get_object_tag_counts):
         root_usage_key = self._create_vertical()

diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py
@@ -717,7 +717,7 @@ def test_number_of_calls_to_db(self):
         """
         Test to check number of queries made to mysql and mongo
         """
-        with self.assertNumQueries(26, table_ignorelist=WAFFLE_TABLES):
+        with self.assertNumQueries(29, table_ignorelist=WAFFLE_TABLES):
             with check_mongo_calls(3):
                 self.client.get_html(reverse_course_url('course_handler', self.course.id))
 

diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
@@ -33,7 +33,6 @@
 from xblock.fields import Scope
 
 from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG
-from cms.djangoapps.contentstore.toggles import use_tagging_taxonomy_list_page
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig
 from common.djangoapps.static_replace import replace_static_urls
@@ -44,6 +43,7 @@
 from common.djangoapps.util.date_utils import get_default_time_display
 from common.djangoapps.util.json_request import JsonResponse, expect_json
 from openedx.core.djangoapps.bookmarks import api as bookmarks_api
+from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
 from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
 from openedx.core.lib.gating import api as gating_api
@@ -1217,7 +1217,10 @@ def create_xblock_info(  # lint-amnesty, pylint: disable=too-many-statements
             xblock_info["ancestor_has_staff_lock"] = False
         if tags is not None:
             xblock_info["tags"] = tags
-        if use_tagging_taxonomy_list_page():
+
+        # Don't show the "Manage Tags" option and tag counts if the DISABLE_TAGGING_FEATURE waffle is true
+        xblock_info["is_tagging_feature_disabled"] = is_tagging_feature_disabled()
+        if not is_tagging_feature_disabled():
             xblock_info["taxonomy_tags_widget_url"] = get_taxonomy_tags_widget_url()
             xblock_info["course_authoring_url"] = settings.COURSE_AUTHORING_MICROFRONTEND_URL
 
@@ -1240,9 +1243,7 @@ def create_xblock_info(  # lint-amnesty, pylint: disable=too-many-statements
             else:
                 xblock_info["hide_from_toc_message"] = False
 
-            # If the ENABLE_TAGGING_TAXONOMY_LIST_PAGE feature flag is enabled, we show the "Manage Tags" options
-            if use_tagging_taxonomy_list_page():
-                xblock_info["use_tagging_taxonomy_list_page"] = True
+            if not is_tagging_feature_disabled():
                 xblock_info["course_tags_count"] = _get_course_tags_count(course.id)
                 xblock_info["tag_counts_by_block"] = _get_course_block_tags(xblock.location.context_key)
 

diff --git a/cms/static/js/views/course_outline.js b/cms/static/js/views/course_outline.js
@@ -33,13 +33,16 @@ function(
         },
 
         renderTagCount: function() {
+            if (this.model.get('is_tagging_feature_disabled')) {
+                return; // Tagging feature is disabled; don't initialize the tag count view.
+            }
             const contentId = this.model.get('id');
-            const tagCountsByBlock = this.model.get('tag_counts_by_block')
             // Skip the course block since that is handled elsewhere in course_manage_tags
             if (contentId.includes('@course')) {
-                return
+                return;
             }
-            const tagsCount = tagCountsByBlock !== undefined ? tagCountsByBlock[contentId] : 0
+            const tagCountsByBlock = this.model.get('tag_counts_by_block');
+            const tagsCount = tagCountsByBlock !== undefined ? tagCountsByBlock[contentId] : 0;
             const tagCountElem = this.$(`.tag-count[data-locator="${contentId}"]`);
             var countModel = new TagCountModel({
                 content_id: contentId,

diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js
@@ -107,12 +107,14 @@ function($, _, Backbone, gettext, BasePage,
                 });
                 this.viewLiveActions.render();
 
-                this.tagListView = new ContainerSubviews.TagList({
-                    el: this.$('.unit-tags'),
-                    model: this.model
-                });
-                this.tagListView.setupMessageListener();
-                this.tagListView.render();
+                if (!this.model.get('is_tagging_feature_disabled')) {
+                    this.tagListView = new ContainerSubviews.TagList({
+                        el: this.$('.unit-tags'),
+                        model: this.model
+                    });
+                    this.tagListView.setupMessageListener();
+                    this.tagListView.render();
+                }
 
                 this.unitOutlineView = new UnitOutlineView({
                     el: this.$('.wrapper-unit-overview'),

diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js
@@ -138,7 +138,7 @@ function($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView, ViewUtils,
             }
 
             // if tagging enabled
-            if (this.model.get('use_tagging_taxonomy_list_page')) {
+            if (!this.model.get('is_tagging_feature_disabled')) {
                 this.courseManageTagsView = new CourseManageTagsView({
                     el: this.$('.status-manage-tags'),
                     model: this.model

diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js
@@ -115,8 +115,8 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, XBlockStringFieldE
                 hideFromTOCMessage: this.model.get('hide_from_toc_message'),
                 enableHideFromTOC: this.model.get('hide_from_toc'),
                 course: course,
-                enableCopyPasteUnits: this.model.get("enable_copy_paste_units"), // ENABLE_COPY_PASTE_UNITS waffle flag
-                useTaggingTaxonomyListPage: this.model.get("use_tagging_taxonomy_list_page"), // ENABLE_TAGGING_TAXONOMY_LIST_PAGE waffle flag
+                enableCopyPasteUnits: this.model.get('enable_copy_paste_units'), // ENABLE_COPY_PASTE_UNITS waffle flag
+                isTaggingFeatureDisabled: this.model.get('is_tagging_feature_disabled'), // DISABLE_TAGGING_FEATURE waffle flag
             };
         },
 

diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore
@@ -187,7 +187,7 @@ if (is_proctored_exam) {
                     </li>
                 <% } %>
 
-                <% if (typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage) { %>
+                <% if (!isTaggingFeatureDisabled) { %>
                     <li class="action-item tag-count" data-locator="<%- xblockId %>"></li>
                 <% } %>
 
@@ -210,7 +210,7 @@ if (is_proctored_exam) {
                                             <a class="configure-button" href="#" role="button"><%- gettext('Configure') %></a>
                                         </li>
                                     <% } %>
-                                    <% if (typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage) { %>
+                                    <% if (!isTaggingFeatureDisabled) { %>
                                         <li class="nav-item">
                                             <a class="manage-tags-button" href="#" role="button"><%- gettext('Manage Tags') %></a>
                                         </li>

diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html
@@ -7,14 +7,15 @@
 from openedx.core.djangolib.js_utils import (
     dump_js_escaped_json, js_escaped_string
 )
-from cms.djangoapps.contentstore.toggles import use_new_text_editor, use_new_problem_editor, use_new_video_editor, use_video_gallery_flow, use_tagging_taxonomy_list_page
+from cms.djangoapps.contentstore.toggles import use_new_text_editor, use_new_problem_editor, use_new_video_editor, use_video_gallery_flow
+from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 %>
 <%
 use_new_editor_text = use_new_text_editor()
 use_new_editor_video = use_new_video_editor()
 use_new_editor_problem = use_new_problem_editor()
 use_new_video_gallery_flow = use_video_gallery_flow()
-use_tagging = use_tagging_taxonomy_list_page()
+use_tagging = not is_tagging_feature_disabled()
 xblock_url = xblock_studio_url(xblock)
 show_inline = xblock.has_children and not xblock_url
 section_class = "level-nesting" if show_inline else "level-element"