Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Tag List on Unit page [FC-0036] #33645

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4a95e8d
feat: Tag List component structure
ChrisChV Nov 1, 2023
d8e327c
feat: Build view to pass tags to UI
ChrisChV Nov 2, 2023
e36cb84
feat: Building Tag list component on unit page
ChrisChV Nov 3, 2023
b383efe
refactor: Extract tag list as a separated component
ChrisChV Nov 6, 2023
a9f2afc
feat: Render tag children
ChrisChV Nov 7, 2023
e19ff55
feat: Manage tags button added on component menu
ChrisChV Nov 7, 2023
7305ed7
chore: Lint
ChrisChV Nov 7, 2023
d79cf97
style: Nits
ChrisChV Nov 7, 2023
b423507
style: Adding comments in get_unit_tags
ChrisChV Nov 8, 2023
3db38e8
Merge branch 'master' into chris/FAL-3527-tags-on-unit-page
ChrisChV Nov 9, 2023
b96fd33
feat: Extract open and close tagging drawer to utility functions
ChrisChV Nov 9, 2023
a005b34
feat: Open manage tag drawer
ChrisChV Nov 9, 2023
694d164
Merge branch 'master' into chris/FAL-3527-tags-on-unit-page
ChrisChV Nov 13, 2023
6d70b80
fix: Bug with multiple children on the same parent tag
ChrisChV Nov 13, 2023
91c03c1
fix: caret up/down for children tags
ChrisChV Nov 13, 2023
76232ba
style: Typo on comments
ChrisChV Nov 14, 2023
7aa790d
style: Nits on UI
ChrisChV Nov 15, 2023
f19b1e8
Merge branch 'master' into chris/FAL-3527-tags-on-unit-page
ChrisChV Nov 15, 2023
aa12f5c
style: Nit on open taxonomy and tag
ChrisChV Nov 15, 2023
29f3b9c
feat: Add a11y support to tag list component
ChrisChV Nov 16, 2023
d3c7603
style: Nit
ChrisChV Nov 16, 2023
aade38f
Merge branch 'master' into chris/FAL-3527-tags-on-unit-page
ChrisChV Nov 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Common utility functions useful throughout the contentstore
"""
from __future__ import annotations
import configparser
import logging
from collections import defaultdict
Expand Down Expand Up @@ -442,7 +443,7 @@ def get_taxonomy_list_url():
return taxonomy_list_url


def get_taxonomy_tags_widget_url(course_locator) -> str:
def get_taxonomy_tags_widget_url(course_locator=None) -> str | None:
"""
Gets course authoring microfrontend URL for taxonomy tags drawer widget view.

Expand All @@ -451,7 +452,9 @@ def get_taxonomy_tags_widget_url(course_locator) -> str:
taxonomy_tags_widget_url = None
# Uses the same waffle flag as taxonomy list page
if use_tagging_taxonomy_list_page():
mfe_base_url = get_course_authoring_url(course_locator)
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
Expand Down
97 changes: 94 additions & 3 deletions cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@
from common.djangoapps.student.auth import has_course_author_access
from common.djangoapps.xblock_django.api import authorable_xblocks, disabled_xblocks
from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag
from cms.djangoapps.contentstore.toggles import use_new_problem_editor
from cms.djangoapps.contentstore.toggles import (
use_new_problem_editor,
use_tagging_taxonomy_list_page,
)
from openedx.core.lib.xblock_utils import get_aside_from_xblock, is_xblock_aside
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
from openedx.core.djangoapps.content_staging import api as content_staging_api
from openedx.core.djangoapps.content_tagging.api import get_content_tags
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order
from ..toggles import use_new_unit_page
Expand Down Expand Up @@ -61,7 +65,7 @@
"editor-mode-button", "upload-dialog",
"add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu",
"add-xblock-component-support-legend", "add-xblock-component-support-level", "add-xblock-component-menu-problem",
"xblock-string-field-editor", "xblock-access-editor", "publish-xblock", "publish-history",
"xblock-string-field-editor", "xblock-access-editor", "publish-xblock", "publish-history", "tag-list",
"unit-outline", "container-message", "container-access", "license-selector", "copy-clipboard-button",
"edit-title-button",
]
Expand Down Expand Up @@ -178,9 +182,14 @@ def container_handler(request, usage_key_string):
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()
unit_tags = None
if show_unit_tags and is_unit_page:
unit_tags = get_unit_tags(usage_key)

# Fetch the XBlock info for use by the container page. Note that it includes information
# about the block's ancestors and siblings for use by the Unit Outline.
xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page)
xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page, tags=unit_tags)

if is_unit_page:
add_container_page_publishing_info(xblock, xblock_info)
Expand Down Expand Up @@ -216,6 +225,7 @@ def container_handler(request, usage_key_string):
'draft_preview_link': preview_lms_link,
'published_preview_link': lms_link,
'templates': CONTAINER_TEMPLATES,
'show_unit_tags': show_unit_tags,
# Status of the user's clipboard, exactly as would be returned from the "GET clipboard" REST API.
'user_clipboard': user_clipboard,
})
Expand Down Expand Up @@ -598,3 +608,84 @@ def component_handler(request, usage_key_string, handler, suffix=''):
)

return webob_to_django_response(resp)


def get_unit_tags(usage_key):
"""
Get the tags of a Unit and build a json to be read by the UI

Note: When migrating the `TagList` subview from `container_subview.js` to the course-authoring MFE,
this function can be simplified to use the REST API of openedx-learning,
which already provides this grouping + sorting logic.
"""
# Get content tags from content tagging API
content_tags = get_content_tags(usage_key)
Copy link
Member

Choose a reason for hiding this comment

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

@ChrisChV I'm wondering if there is a way to make use of @bradenmacdonald 's updated implementation in the openedx-learning repo that added the lineage information to the object tags and takes care of the grouping under taxonomies as well, rather than re-implementing it here as well we could potentially utilize that as a single source of truth.

Here is a sample of how the data looks like:

{
    "block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb0b": {
        "taxonomies": [
            {
                "name": "LightCastSkillsTaxonomy",
                "taxonomy_id": 15,
                "editable": true,
                "tags": [
                    {
                        "value": "Administrative Functions",
                        "lineage": [
                            "Administration",
                            "Administrative Support",
                            "Administrative Functions"
                        ]
                    }
                ]
            },
            {
                "name": "OpenCanadaTaxonomy",
                "taxonomy_id": 14,
                "editable": true,
                "tags": [
                    {
                        "value": "Verbal Ability",
                        "lineage": [
                            "Abilities",
                            "Cognitive Abilities",
                            "Communication Abilities",
                            "Verbal Ability"
                        ]
                    }
                ]
            },
            ...
        ]
    }
}

Though I think to make use of that, it seems there needs to be some changes made to the openedx-learning repo, namely extracting the logic out of the ObjectTagsByTaxonomySerializer and exposing a python API that can be used in this repo. Since currently It is only available through the rest api.

Any thoughts on this?

Copy link
Contributor

@bradenmacdonald bradenmacdonald Nov 8, 2023

Choose a reason for hiding this comment

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

My thinking was that it's OK to have this duplicated here, because eventually this will be re-implemented in the course-authoring MFE and fetch data using REST (that's out of scope for us now, but at some point ALL of these Studio pages will be using the new MFE only). When that happens, it can use the logic in openedx-learning.

We should definitely put in a comment stating that when this is moved into an MFE it should be simplified to use the REST API which already provides this grouping + sorting logic. But for now since it's already implemented this way, I think it's fine to leave it like this and get it done sooner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok @bradenmacdonald, that makes sense 👍

@yusuf-musleh Adding another comment, In addition to grouping by taxonomy, it is also necessary to group by parents, to be able to assemble each level of the component.

Copy link
Member

Choose a reason for hiding this comment

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

@bradenmacdonald I see, yup, that make sense to me as well!

@ChrisChV Sounds good!


# Group content tags by taxonomy
taxonomy_dict = {}
for content_tag in content_tags:
taxonomy_id = content_tag.taxonomy_id
# When a taxonomy is deleted, the id here is None.
# In that case the tag is not shown in the UI.
if taxonomy_id:
if taxonomy_id not in taxonomy_dict:
taxonomy_dict[taxonomy_id] = []
taxonomy_dict[taxonomy_id].append(content_tag)

taxonomy_list = []
total_count = 0

def handle_tag(tags, root_ids, tag, child_tag_id=None):
"""
Group each tag by parent to build a tree.
"""
tag_processed_before = tag.id in tags
if not tag_processed_before:
tags[tag.id] = {
'id': tag.id,
'value': tag.value,
'children': [],
}
if child_tag_id:
# Add a child into the children list
tags[tag.id].get('children').append(tags[child_tag_id])
if tag.parent_id is None:
if tag.id not in root_ids:
root_ids.append(tag.id)
elif not tag_processed_before:
# Group all the lineage of this tag.
#
# Skip this if the tag has been processed before,
# we don't need to process lineage again to avoid duplicates.
handle_tag(tags, root_ids, tag.parent, tag.id)

# Build a tag tree for each taxonomy
for content_tag_list in taxonomy_dict.values():
tags = {}
root_ids = []

for content_tag in content_tag_list:
# When a tag is deleted from the taxonomy, the `tag` here is None.
# In that case the tag is not shown in the UI.
if content_tag.tag:
handle_tag(tags, root_ids, content_tag.tag)

taxonomy = content_tag_list[0].taxonomy

if tags:
count = len(tags)
# Add the tree to the taxonomy list
taxonomy_list.append({
'id': taxonomy.id,
'value': taxonomy.name,
'tags': [tags[tag_id] for tag_id in root_ids],
'count': count,
})
total_count += count

unit_tags = {
'count': total_count,
'taxonomies': taxonomy_list,
}

return unit_tags
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
has_children_visible_to_specific_partition_groups,
is_currently_visible_to_students,
is_self_paced,
get_taxonomy_tags_widget_url,
)

from .create_xblock import create_xblock
Expand Down Expand Up @@ -1093,6 +1094,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
course=None,
is_concise=False,
summary_configuration=None,
tags=None,
):
"""
Creates the information needed for client-side XBlockInfo.
Expand Down Expand Up @@ -1385,6 +1387,10 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
)
else:
xblock_info["ancestor_has_staff_lock"] = False
if tags is not None:
xblock_info["tags"] = tags
if use_tagging_taxonomy_list_page():
xblock_info["taxonomy_tags_widget_url"] = get_taxonomy_tags_widget_url()

if course_outline:
if xblock_info["has_explicit_staff_lock"]:
Expand Down
4 changes: 4 additions & 0 deletions cms/static/js/models/xblock_info.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ define(
* True if summary configuration is enabled.
*/
summary_configuration_enabled: null,
/**
* List of tags of the unit. This list is managed by the content_tagging module.
*/
tags: null,
},

initialize: function() {
Expand Down
39 changes: 6 additions & 33 deletions cms/static/js/views/course_outline.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
define(['jquery', 'underscore', 'js/views/xblock_outline', 'edx-ui-toolkit/js/utils/string-utils',
'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',],
'common/js/components/views/feedback_notification', 'common/js/components/views/feedback_prompt',
'js/views/utils/tagging_drawer_utils',],
function(
$, _, XBlockOutlineView, StringUtils, ViewUtils, XBlockViewUtils,
XBlockOutlineInfo, CourseOutlineModalsFactory, ContentDragger, NotificationView, PromptView
XBlockOutlineInfo, CourseOutlineModalsFactory, ContentDragger, NotificationView, PromptView,
TaggingDrawerUtils
) {
var CourseOutlineView = XBlockOutlineView.extend({
// takes XBlockOutlineInfo as a model
Expand Down Expand Up @@ -458,41 +460,12 @@ function(
event.stopPropagation();
},

closeManageTagsDrawer(drawer, drawerCover) {
$(drawerCover).css('display', 'none');
$(drawer).empty();
$(drawer).css('display', 'none');
$('body').removeClass('drawer-open');
},

openManageTagsDrawer(event) {
const drawer = document.querySelector("#manage-tags-drawer");
const drawerCover = document.querySelector(".drawer-cover")
openManageTagsDrawer() {
const article = document.querySelector('[data-taxonomy-tags-widget-url]');
const taxonomyTagsWidgetUrl = $(article).attr('data-taxonomy-tags-widget-url');
const contentId = this.model.get('id');

// Add handler to close drawer when dark background is clicked
$(drawerCover).click(function() {
this.closeManageTagsDrawer(drawer, drawerCover);
}.bind(this));

// Add event listen to close drawer when close button is clicked from within the Iframe
window.addEventListener("message", function (event) {
if (event.data === 'closeManageTagsDrawer') {
this.closeManageTagsDrawer(drawer, drawerCover)
}
}.bind(this));

$(drawerCover).css('display', 'block');
// xss-lint: disable=javascript-jquery-html
$(drawer).html(
`<iframe src="${taxonomyTagsWidgetUrl}${contentId}" onload="this.contentWindow.focus()" frameborder="0" style="width: 100%; height: 100%;"></iframe>`
);
$(drawer).css('display', 'block');

// Prevent background from being scrollable when drawer is open
$('body').addClass('drawer-open');
TaggingDrawerUtils.openDrawer(taxonomyTagsWidgetUrl, contentId);
},

addButtonActions: function(element) {
Expand Down
19 changes: 18 additions & 1 deletion cms/static/js/views/pages/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page
'js/models/xblock_info', 'js/views/xblock_string_field_editor', 'js/views/xblock_access_editor',
'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils',
'common/js/components/views/feedback_notification', 'common/js/components/views/feedback_prompt',
'js/views/utils/tagging_drawer_utils',
],
function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent,
EditXBlockModal, MoveXBlockModal, XBlockInfo, XBlockStringFieldEditor, XBlockAccessEditor,
ContainerSubviews, UnitOutlineView, XBlockUtils, NotificationView, PromptView) {
ContainerSubviews, UnitOutlineView, XBlockUtils, NotificationView, PromptView, TaggingDrawerUtils) {
'use strict';

var XBlockContainerPage = BasePage.extend({
Expand All @@ -27,6 +28,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
'click .show-actions-menu-button': 'showXBlockActionsMenu',
'click .new-component-button': 'scrollToNewComponentButtons',
'click .paste-component-button': 'pasteComponent',
'click .tags-button': 'openManageTags',
},

options: {
Expand Down Expand Up @@ -97,6 +99,12 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
});
this.viewLiveActions.render();

this.tagListView = new ContainerSubviews.TagList({
el: this.$('.unit-tags'),
model: this.model
});
this.tagListView.render();

this.unitOutlineView = new UnitOutlineView({
el: this.$('.wrapper-unit-overview'),
model: this.model
Expand Down Expand Up @@ -125,6 +133,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
xblockView = this.xblockView,
loadingElement = this.$('.ui-loading'),
unitLocationTree = this.$('.unit-location'),
unitTags = this.$('.unit-tags'),
hiddenCss = 'is-hidden';

loadingElement.removeClass(hiddenCss);
Expand All @@ -150,6 +159,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
// Refresh the views now that the xblock is visible
self.onXBlockRefresh(xblockView);
unitLocationTree.removeClass(hiddenCss);
unitTags.removeClass(hiddenCss);

// Re-enable Backbone events for any updated DOM elements
self.delegateEvents();
Expand Down Expand Up @@ -409,6 +419,13 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
this.duplicateComponent(this.findXBlockElement(event.target));
},

openManageTags: function(event) {
const taxonomyTagsWidgetUrl = this.model.get('taxonomy_tags_widget_url');
const contentId = this.findXBlockElement(event.target).data('locator');

TaggingDrawerUtils.openDrawer(taxonomyTagsWidgetUrl, contentId);
},

showMoveXBlockModal: function(event) {
var xblockElement = this.findXBlockElement(event.target),
parentXBlockElement = xblockElement.parents('.studio-xblock-wrapper'),
Expand Down
Loading
Loading