-
Notifications
You must be signed in to change notification settings - Fork 91
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
[FC-0049] feat: Other Tags section added on tags drawer #987
[FC-0049] feat: Other Tags section added on tags drawer #987
Conversation
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:
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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #987 +/- ##
==========================================
- Coverage 92.28% 92.28% -0.01%
==========================================
Files 707 707
Lines 12469 12493 +24
Branches 2671 2679 +8
==========================================
+ Hits 11507 11529 +22
- Misses 925 927 +2
Partials 37 37 ☔ View full report in Codecov by Sentry. |
@ChrisChV I wasn't able to test this. Whenever I try to import a taxonomy, the I also tried importing different kinds of taxonomies. I'm not sure what the issue is, could you confirm that it is working correctly on your end? Edit: Found these in the logs when I restarted the server:
It seems to also be happening when I tried it on master/main for all repos, so it's probably not related to the changes in these PRs? |
@yusuf-musleh It's working using devstack, but I will test this using tutor |
@yusuf-musleh I have tried it and it has worked for me. It takes a little longer due to indexing in meilisearch. Maybe it's a configuration in meilisearch that's missing from your locale? |
@yusuf-musleh That sounds like it could be related to openedx/modular-learning#213 . I also couldn't reproduce it though (using tutor). Let's consider it part of that issue ^ and unrelated to this PR, if you're seeing on master too. |
@yusuf-musleh If you are blocked for the import of the taxonomy, you can test this without import a taxonomy. You can use an already created taxonomy and delete any associated organization. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well. Just one minor thing.
{intl.formatMessage(messages.otherTagsHeader)} | ||
</p> | ||
<p className="h6 text-gray-500"> | ||
{intl.formatMessage(messages.otherTagsDescription)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated here: 5a45416
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Update styles on Other tags description section in tag drawer
@ChrisChV Could you please rebase this or resolve the conflicts? |
…xonomy-sandbox-20240508
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisChV @bradenmacdonald I tried it again today and it worked, not sure what changed, but its definitely not related to the changes here as you mentioned. It's working well!
👍
- I tested this: I followed the testing instructions in the PR
- I read through the code
- I checked for accessibility issues
-
Includes documentation
@bradenmacdonald Done! @brian-smith-tcril @arbrandes @viktorrusakov @xitij2000 Does anyone have time to approve and merge this before Redwood gets cut? Thank you 🙂 |
@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. |
Description
This adds the new "Other tags" Section to tags drawer that contains the taxonomies/tags that the user doesn't have permission to see/edit. It allow to delete that tags.
Supporting information
Testing instructions
edx-platform
: [FC-0049] feat: Update remove object-tag permission edx-platform#34728make requirements
on cms-shell.SampleTaxonomyOrg1
as organizationOther tags
sectionOther tags
section.Other tags
and Save. Verify that theOther tags
section is gone.Other Information
TODO: It's missing to add examples to test this on https://github.com/open-craft/taxonomy-sample-data