feat: [FC-0070] implement move xblock modal #1422

ihor-romaniuk · 2024-10-22T10:56:41Z

🚨 Dependencies:

🔗 frontend-app-authoring [MERGED]
🔗 edx-platform [MERGED]

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: romaniuk/implement-move-xblock-modal

TUTOR_GROVE_NEW_MFES:
  authoring:
    port: 18000
    repository: https://github.com/raccoongang/frontend-app-course-authoring.git
    version: romaniuk/implement-move-xblock-modal

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

Description

This PR introduces the ability to move an xBlock from one unit to another within a course using a modal window. The feature enables users to select a target unit for the xBlock and confirm the move.

move-xblock.mov

Related Pull Requests

feat: [FC-0070] add message events to the unit page container edx-platform#35693 - events and listeners to trigger xBlock move

Testing instructions

сreate a new course.
create several units.
open the course unit page.
add some xBlocks to the unit.
move xBlock to another unit: click three dots icon (xBlock action menu) -> select "Move" -> select new location.

openedx-webhooks · 2024-10-22T10:56:46Z

Thanks for the pull request, @ihor-romaniuk!

What's next?

Please work through the following steps to get your changes 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.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

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.

src/course-unit/data/thunk.js

open-craft-grove · 2024-11-08T20:04:45Z

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

PKulkoRaccoonGang

[inform]: Regarding the failing tests. Apparently after merging one of the last PRs we had problems with tests for the library functionality. I have already fixed this in a new PR
#1493

src/course-unit/CourseUnit.test.jsx

src/course-unit/move-modal/index.tsx

src/course-unit/CourseUnit.test.jsx

src/course-unit/move-modal/moveModal.test.tsx

src/course-unit/move-modal/utils.ts

codecov · 2024-11-11T17:26:32Z

Codecov Report

Attention: Patch coverage is 95.49839% with 14 lines in your changes missing coverage. Please review.

Project coverage is 93.11%. Comparing base (ba48a27) to head (a06651c).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/course-unit/data/thunk.js	82.14%	5 Missing ⚠️
src/course-unit/hooks.jsx	72.22%	5 Missing ⚠️
src/course-unit/context/iFrameContext.tsx	82.35%	3 Missing ⚠️
src/course-unit/move-modal/hooks.tsx	98.93%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1422      +/-   ##
==========================================
+ Coverage   93.04%   93.11%   +0.06%     
==========================================
  Files        1048     1060      +12     
  Lines       20544    20889     +345     
  Branches     4363     4531     +168     
==========================================
+ Hits        19116    19451     +335     
- Misses       1358     1369      +11     
+ Partials       70       69       -1

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

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

open-craft-grove · 2024-11-11T17:46:34Z

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

open-craft-grove · 2024-11-11T18:15:32Z

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

src/course-unit/data/api.js

src/course-unit/move-modal/components/EmptyMessage.tsx

open-craft-grove · 2024-11-13T11:47:49Z

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

open-craft-grove · 2024-11-13T15:17:08Z

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

open-craft-grove · 2024-11-15T08:18:32Z

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

src/course-unit/context/iFrameContext.tsx

src/course-unit/move-modal/components/CategoryIndicator.tsx

src/course-unit/move-modal/components/EmptyMessage.tsx

src/course-unit/move-modal/components/ModalLoader.tsx

src/course-unit/move-modal/hooks.tsx

src/course-unit/move-modal/index.scss

src/course-unit/move-modal/moveModal.test.tsx

PKulkoRaccoonGang

Looks nice! 👍
Thanks!

open-craft-grove · 2024-11-15T11:47:44Z

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

arbrandes

Can't find anything to object to except for the console logging. Code looks good and works well. Once the calls to console() are removed, we can merge.

arbrandes · 2024-11-14T20:13:27Z

src/course-unit/context/hooks.tsx

+
+import { IframeContext, IframeContextType } from './iFrameContext';
+
+// eslint-disable-next-line import/prefer-default-export


Not a blocker, but the latest frontend-build makes it so that we don't need this line anymore.

After update current branch to master branch and reinstall dependencies, still catch the error ESLint: Prefer default export on a file with single export.(import/ prefer-default-export)

@bradenmacdonald, FYI. Not sure what went wrong.

@arbrandes This MFE's master branch is still on 14.1.4 not 14.2.0

npm ls @openedx/frontend-build @edx/[email protected] ├─┬ @edx/[email protected] │ └── @openedx/[email protected] deduped ├─┬ @edx/[email protected] │ └── @openedx/[email protected] deduped └── @openedx/[email protected]

Let's update @openedx/frontend-build in the next PRs to avoid delaying this feature, as it will require additional time to refactor the current code (I found 104 matches for import/prefer-default-export).

I've done that refactor in #1519 and it's now merged to master :)

arbrandes · 2024-11-14T20:15:51Z

src/course-unit/context/iFrameContext.tsx

+        iframeWindow.postMessage({ type: messageType, payload }, '*');
+      } catch (error) {
+        // eslint-disable-next-line no-console
+        console.error('Failed to send message to iframe:', error);


This is fine for debugging, but there's a reason for the linting to be there: we don't want to use the console in production code.

Replaced with throw new Error('Failed to send message to iframe.');

The idea to not use console() is to avoid showing ugly messages to the end user. I'm not sure if throwing a new Error() will get managed properly, but I suspect it won't.

How about if we used logError instead? Right now it only works with newRelic, but at least it takes case of both sides of the issue: not showing the message to the end user while still allowing the operator to know about it.

Good point. This place and comment below are replaced with logError events.

arbrandes · 2024-11-14T20:16:51Z

src/course-unit/context/iFrameContext.tsx

+      }
+    } else {
+      // eslint-disable-next-line no-console
+      console.warn('Iframe is not accessible or loaded yet.');


Please remove the console message, as per the previous comment.

Replaced with throw new Error('Iframe is not accessible or loaded yet.');

open-craft-grove · 2024-11-18T17:25:28Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-11-20T10:49:57Z

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

arbrandes

Works for me! Thank you!

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 22, 2024

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 22, 2024

ihor-romaniuk mentioned this pull request Oct 24, 2024

feat: [FC-0070] add message events to the unit page container openedx/edx-platform#35693

Merged

This was referenced Nov 6, 2024

feat: [FC-0070] rendering library content in unit page #1475

Open

feat: [FC-0070] rendering library content in unit page openedx/edx-platform#35785

Open

ihor-romaniuk commented Nov 7, 2024

View reviewed changes

src/course-unit/data/thunk.js Show resolved Hide resolved

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from c917065 to 1a9145e Compare November 8, 2024 17:46

ihor-romaniuk marked this pull request as ready for review November 8, 2024 17:46

ihor-romaniuk requested a review from a team as a code owner November 8, 2024 17:46

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from 1a9145e to de9f97c Compare November 8, 2024 18:08

arbrandes self-requested a review November 8, 2024 18:17

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch 3 times, most recently from 0d617b7 to 062e439 Compare November 8, 2024 18:28

ihor-romaniuk added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Nov 8, 2024

PKulkoRaccoonGang suggested changes Nov 11, 2024

View reviewed changes

PKulkoRaccoonGang assigned ihor-romaniuk Nov 11, 2024

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from 64a02a0 to 54ddf6b Compare November 11, 2024 17:15

PKulkoRaccoonGang 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 Nov 12, 2024

bradenmacdonald reviewed Nov 12, 2024

View reviewed changes

src/course-unit/data/api.js Outdated Show resolved Hide resolved

src/course-unit/data/api.js Outdated Show resolved Hide resolved

src/course-unit/move-modal/components/EmptyMessage.tsx Outdated Show resolved Hide resolved

bradenmacdonald mentioned this pull request Nov 13, 2024

Update workflow for library-sourced content (Authoring MFE - Unit Page) #1464

Open

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from 54ddf6b to 7429b68 Compare November 13, 2024 11:13

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from 7429b68 to 8965b19 Compare November 13, 2024 14:39

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from 8965b19 to dedc765 Compare November 15, 2024 07:34

PKulkoRaccoonGang suggested changes Nov 15, 2024

View reviewed changes

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from dedc765 to 01031d4 Compare November 15, 2024 11:02

PKulkoRaccoonGang approved these changes Nov 15, 2024

View reviewed changes

navinkarkera mentioned this pull request Nov 15, 2024

feat: preview library block changes in course unit [FC-0062] #1506

Merged

PKulkoRaccoonGang mentioned this pull request Nov 15, 2024

feat: [FC-0070] listen to xblock interaction events #1431

Open

arbrandes requested changes Nov 18, 2024

View reviewed changes

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from 01031d4 to c561446 Compare November 18, 2024 16:46

ihor-romaniuk 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 Nov 19, 2024

feat: [FC-0070] implement move xblock modal

a06651c

ihor-romaniuk force-pushed the romaniuk/implement-move-xblock-modal branch from c561446 to a06651c Compare November 20, 2024 10:06

arbrandes approved these changes Nov 21, 2024

View reviewed changes

arbrandes merged commit b5419ac into openedx:master Nov 21, 2024
7 checks passed

ihor-romaniuk deleted the romaniuk/implement-move-xblock-modal branch November 22, 2024 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [FC-0070] implement move xblock modal #1422

feat: [FC-0070] implement move xblock modal #1422

ihor-romaniuk commented Oct 22, 2024 •

edited

Loading

openedx-webhooks commented Oct 22, 2024 •

edited

Loading

open-craft-grove commented Nov 8, 2024

PKulkoRaccoonGang left a comment

codecov bot commented Nov 11, 2024 •

edited

Loading

open-craft-grove commented Nov 11, 2024

open-craft-grove commented Nov 11, 2024

open-craft-grove commented Nov 13, 2024

open-craft-grove commented Nov 13, 2024

open-craft-grove commented Nov 15, 2024

PKulkoRaccoonGang left a comment •

edited

Loading

open-craft-grove commented Nov 15, 2024

arbrandes left a comment

arbrandes Nov 14, 2024

ihor-romaniuk Nov 18, 2024

arbrandes Nov 18, 2024

bradenmacdonald Nov 18, 2024

ihor-romaniuk Nov 20, 2024

bradenmacdonald Nov 20, 2024

arbrandes Nov 14, 2024

ihor-romaniuk Nov 18, 2024 •

edited

Loading

arbrandes Nov 18, 2024

ihor-romaniuk Nov 20, 2024

arbrandes Nov 14, 2024

ihor-romaniuk Nov 18, 2024 •

edited

Loading

open-craft-grove commented Nov 18, 2024

open-craft-grove commented Nov 20, 2024

arbrandes left a comment


		import { IframeContext, IframeContextType } from './iFrameContext';

		// eslint-disable-next-line import/prefer-default-export

feat: [FC-0070] implement move xblock modal #1422

feat: [FC-0070] implement move xblock modal #1422

Conversation

ihor-romaniuk commented Oct 22, 2024 • edited Loading

Description

Related Pull Requests

Testing instructions

openedx-webhooks commented Oct 22, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

open-craft-grove commented Nov 8, 2024

PKulkoRaccoonGang left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 11, 2024 • edited Loading

Codecov Report

open-craft-grove commented Nov 11, 2024

open-craft-grove commented Nov 11, 2024

open-craft-grove commented Nov 13, 2024

open-craft-grove commented Nov 13, 2024

open-craft-grove commented Nov 15, 2024

PKulkoRaccoonGang left a comment • edited Loading

Choose a reason for hiding this comment

open-craft-grove commented Nov 15, 2024

arbrandes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihor-romaniuk Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihor-romaniuk Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

open-craft-grove commented Nov 18, 2024

open-craft-grove commented Nov 20, 2024

arbrandes left a comment

Choose a reason for hiding this comment

ihor-romaniuk commented Oct 22, 2024 •

edited

Loading

openedx-webhooks commented Oct 22, 2024 •

edited

Loading

codecov bot commented Nov 11, 2024 •

edited

Loading

PKulkoRaccoonGang left a comment •

edited

Loading

ihor-romaniuk Nov 18, 2024 •

edited

Loading

ihor-romaniuk Nov 18, 2024 •

edited

Loading