Skip to content

feat: [FC-0070] Manage Tags interoperation - #1454

Merged
arbrandes merged 5 commits into
openedx:masterfrom
raccoongang:Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View
Mar 3, 2025
Merged

feat: [FC-0070] Manage Tags interoperation#1454
arbrandes merged 5 commits into
openedx:masterfrom
raccoongang:Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View

Conversation

@peterkulko

@peterkulko peterkulko commented Oct 31, 2024

Copy link
Copy Markdown
Contributor

🚨 Dependencies:

Description

Added interaction between MFE and Legacy tagging functionality for xblocks on the Course unit page.

Useful information to include:

Testing instructions

Screen.Recording.2025-02-24.at.15.35.58.mov

Other information

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View

TUTOR_GROVE_NEW_MFES:
  authoring:
    port: 18000
    repository: https://github.com/raccoongang/frontend-app-course-authoring.git
    version: Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View

TUTOR_GROVE_WAFFLE_FLAGS:
  - name: contentstore.new_studio_mfe.use_new_unit_page
    everyone: true

TUTOR_GROVE_MFE_LMS_COMMON_SETTINGS:
  MFE_CONFIG:
    ENABLE_UNIT_PAGE: true

@peterkulko
peterkulko requested a review from a team as a code owner October 31, 2024 19:14
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 31, 2024
@openedx-webhooks

openedx-webhooks commented Oct 31, 2024

Copy link
Copy Markdown

Thanks for the pull request, @PKulkoRaccoonGang!

This repository is currently maintained by @openedx/2u-tnl.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@peterkulko
peterkulko marked this pull request as draft October 31, 2024 19:14
@peterkulko peterkulko self-assigned this Oct 31, 2024
@codecov

codecov Bot commented Nov 4, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.38%. Comparing base (7e4ecff) to head (27c4533).
Report is 96 commits behind head on master.

Files with missing lines Patch % Lines
src/course-unit/hooks.jsx 92.59% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1454      +/-   ##
==========================================
+ Coverage   93.33%   93.38%   +0.04%     
==========================================
  Files        1108     1109       +1     
  Lines       22045    22145     +100     
  Branches     4674     4692      +18     
==========================================
+ Hits        20575    20679     +104     
+ Misses       1405     1401       -4     
  Partials       65       65              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mphilbrick211 mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Nov 6, 2024
@peterkulko
peterkulko force-pushed the Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View branch from 3e33967 to 487d38f Compare February 23, 2025 23:34
@peterkulko
peterkulko force-pushed the Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View branch 3 times, most recently from 8bf4836 to b87a550 Compare February 24, 2025 11:51
@peterkulko
peterkulko force-pushed the Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View branch from b87a550 to 62af3bd Compare February 24, 2025 13:20
@peterkulko
peterkulko marked this pull request as ready for review February 24, 2025 13:32
@peterkulko peterkulko added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 24, 2025
@open-craft-grove

Copy link
Copy Markdown

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Comment thread src/course-unit/CourseUnit.test.jsx Outdated
Comment on lines +109 to +116
if (queryKey[0] === 'taxonomies') {
return {
data: {
results: [],
},
isSuccess: true,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally instead of mocking react-query we prefer to mock the API requests using the mocks defined in content-tags-drawer/data/api.mocks.ts.

So you can just write something like this at the top of your test file:

mockContentTaxonomyTagsData.applyMock();
mockTaxonomyListData.applyMock();
mockTaxonomyTagsData.applyMock();
mockContentData.applyMock();

If you do want/need to keep it this way though, you should at least replace the 'taxonomies' string with taxonomyQueryKeys.all from here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like the option with mocks from content-tags-drawer the most, but unfortunately I couldn't run the tests successfully. Probably the problem is related to this mock @tanstack/react-query

Added taxonomyQueryKeys.all instead of the taxonomies line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah those mocks won't work if you mock out react-query entirely.

@peterkulko peterkulko Feb 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added taxonomyQueryKeys.all instead of the taxonomies line

Do you mind if we stop here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah that's fine. No need to rewrite it, but in the future be aware of the existing mocks we have (as well as everything in testUtils.tsx as they make writing new tests way simpler.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot!

Comment thread src/content-tags-drawer/data/apiHooks.jsx
@peterkulko
peterkulko force-pushed the Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View branch from 6944a90 to b112c32 Compare February 25, 2025 12:59
@peterkulko
peterkulko force-pushed the Peter_Kulko/manage-tags-interoperation-between-mfe-and-Studio-xBlock-View branch from b112c32 to 27c4533 Compare February 25, 2025 13:16
@open-craft-grove

Copy link
Copy Markdown

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove

Copy link
Copy Markdown

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@arbrandes arbrandes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This works well, and the code looks good. Approved!

@arbrandes

Copy link
Copy Markdown
Contributor

Oh, and thank a bunch for fixing the scrolling issue!

@arbrandes
arbrandes merged commit 411607e into openedx:master Mar 3, 2025
@github-project-automation github-project-automation Bot moved this from Waiting on Author to Done in Contributions Mar 3, 2025
@peterkulko peterkulko added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Apr 24, 2025
Arpit-Nakrani-Networked pushed a commit to Arpit-Nakrani-Networked/frontend-app-authoring that referenced this pull request Jul 13, 2025
* test: Remove support for Node 18

* chore: update code coverage artifact naming
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

create-sandbox open-craft-grove should create a sandbox environment from this PR FC Relates to an Axim Funded Contribution project 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.

6 participants