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 count in components [FC-0036] #33928

Conversation

ChrisChV
Copy link
Contributor

@ChrisChV ChrisChV commented Dec 14, 2023

Description

This adds the tag count button on components:

image

Supporting information

Testing instructions

  • Enable contentstore.enable_copy_paste_units and new_studio_mfe.use_tagging_taxonomy_list_page flags.
  • Run the taxonomy-sample-data.
  • Go to a unit and verify that the tag count button appears on the components.
  • Create a new component and verify that the tag count button doesn't appear.
  • Add tags using "Manage tags".
  • Refresh the page and verify that all the components has the tag count button.
  • Disable the new_studio_mfe.use_tagging_taxonomy_list_page flag.
  • Refresh the page and verify that the tag count buttons are gone.

Other information

  • The count update is missing if the tags are edited in the manage tags drawer.

@openedx-webhooks
Copy link

openedx-webhooks commented Dec 14, 2023

Thanks for the pull request, @ChrisChV! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Dec 14, 2023
@ChrisChV ChrisChV marked this pull request as draft December 14, 2023 15:54
@ChrisChV ChrisChV marked this pull request as ready for review December 14, 2023 18:18
@bradenmacdonald
Copy link
Contributor

@ChrisChV Do you think you could update this PR so that it uses ?count_implicit from my new PR on both the outline page and the unit page, so that the counts are consistent with the count widget that you made?

Wrong:
Screenshot 2023-12-14 at 10 19 56 AM

Right:
Screenshot 2023-12-14 at 10 21 07 AM

No need to include a version bump, as long as the PRs work together and the counts will be correct once both PRs are merged.

@ChrisChV
Copy link
Contributor Author

ChrisChV commented Dec 15, 2023

No need to include a version bump, as long as the PRs work together and the counts will be correct once both PRs are merged.

@bradenmacdonald I have used the python API, not the REST API. I need the change in the function

TypeError: get_object_tag_counts() got an unexpected keyword argument 'count_implicit'

But I can wait until your PR is ready to bump the version here

Edit: Same for outline page

Edit 2: Updated in 2f99115

@@ -779,7 +779,7 @@ openedx-filters==1.6.0
# via
# -r requirements/edx/kernel.in
# lti-consumer-xblock
openedx-learning==0.3.6
openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@braden/count-implicit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: bump version

Copy link
Contributor

@yusuf-musleh yusuf-musleh left a comment

Choose a reason for hiding this comment

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

@ChrisChV Overall code looks good, and works as expected, except however I found an issue with double counting implicit tags.

@bradenmacdonald bradenmacdonald merged commit 90879d1 into openedx:master Jan 12, 2024
64 checks passed
@bradenmacdonald bradenmacdonald deleted the chris/FAL-3573-tag-count-in-component branch January 12, 2024 17:35
@openedx-webhooks
Copy link

@ChrisChV 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

andrey-canon pushed a commit to nelc/edx-platform that referenced this pull request May 16, 2024
andrey-canon added a commit to nelc/edx-platform that referenced this pull request May 16, 2024
feat: Tag count in components on Studio Unit page(openedx#33928)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Tagging] Show tag count for each tagged component on the Unit Page
5 participants