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 <braden@opencraft.com>
Co-authored-by: Yusuf Musleh <yusuf@opencraft.com>
This commit is contained in:
Rômulo Penido
2024-05-09 16:27:05 +03:00
committed by GitHub
parent 7f6133c940
commit b42da7429f
17 changed files with 84 additions and 76 deletions

View File

@@ -5,11 +5,11 @@ API Serializers for unit page
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 @@ class ChildVerticalContainerSerializer(serializers.Serializer):
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)

View File

@@ -150,6 +150,6 @@ class CourseIndexViewTest(CourseTestCase, PermissionAccessMixin):
"""
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)

View File

@@ -8,14 +8,12 @@ from django.test import override_settings
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 @@ class HomePageViewTest(CourseTestCase):
"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 @@ class HomePageViewTest(CourseTestCase):
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'])

View File

@@ -8,7 +8,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag
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 @@ class ContainerVerticalViewTest(BaseXBlockContainer):
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 @@ class ContainerVerticalViewTest(BaseXBlockContainer):
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.

View File

@@ -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()

View File

@@ -58,6 +58,7 @@ from common.djangoapps.util.date_utils import get_default_time_display
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 @@ from cms.djangoapps.contentstore.toggles import (
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)

View File

@@ -29,7 +29,7 @@ from openedx.core.lib.xblock_utils import (
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.

View File

@@ -17,7 +17,6 @@ from openedx_events.content_authoring.data import DuplicatedXBlockData
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 @@ from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
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 @@ class GetItemTest(ItemTest):
),
)
@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()

View File

@@ -717,7 +717,7 @@ class TestCourseOutline(CourseTestCase):
"""
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))

View File

@@ -33,7 +33,6 @@ from xblock.core import XBlock
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.student.auth import (
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)

View File

@@ -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,

View File

@@ -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'),

View File

@@ -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

View File

@@ -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
};
},

View File

@@ -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>

View File

@@ -7,14 +7,15 @@ from lms.lib.utils import is_unit
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"

View File

@@ -1,12 +1,14 @@
"""
Toggles for content tagging
"""
from edx_toggles.toggles import WaffleFlag
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
# .. toggle_name: content_tagging.auto
# .. toggle_implementation: WaffleSwitch
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Setting this enables automatic tagging of content
# .. toggle_type: feature_flag
@@ -15,3 +17,21 @@ from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
# .. toggle_creation_date: 2023-08-30
# .. toggle_tickets: https://github.com/openedx/modular-learning/issues/79
CONTENT_TAGGING_AUTO = CourseWaffleFlag('content_tagging.auto', __name__)
# .. toggle_name: content_tagging.disabled
# .. toggle_implementation: WaffleFlag
# .. toggle_default: False
# .. toggle_description: Setting this disables the tagging feature
# .. toggle_type: feature_flag
# .. toggle_category: admin
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2024-04-30
DISABLE_TAGGING_FEATURE = WaffleFlag('content_tagging.disabled', __name__)
def is_tagging_feature_disabled():
"""
Returns a boolean if tagging feature list page is disabled
"""
return DISABLE_TAGGING_FEATURE.is_enabled()