feat: [FC-0070] rendering split test content in unit page #1492

ihor-romaniuk · 2024-11-08T17:34:59Z

Description

This PR introduces functionality to display Split Test Content within the new Unit page interface.

The feature enables opening a Split Test Content page within the Unit page’s new interface. This page displays the xBlocks from the specified groups and provides basic configuration options for the groups.

split_test.mov

Related Pull Requests

feat: [FC-0070] add events and style for rendering Split xblock in chromeless template edx-platform#35813 - events to trigger click on View button and styles

Testing instructions

Create a new course.
Create several units.
Open the course unit page.
Add a Content Experiment xBlock to the unit.
Click the “View” button on the Content Experiment xBlock.

Other information

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: romaniuk/children-container-for-rendering-split-test

TUTOR_GROVE_NEW_MFES:
  authoring:
    port: 18000
    repository: https://github.com/raccoongang/frontend-app-course-authoring.git
    version: romaniuk/children-container-for-rendering-split-test

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

openedx-webhooks · 2024-11-08T17:35:04Z

Thanks for the pull request, @ihor-romaniuk!

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.

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.

codecov · 2025-02-26T11:20:55Z

Codecov Report

Attention: Patch coverage is 95.04132% with 6 lines in your changes missing coverage. Please review.

Project coverage is 93.37%. Comparing base (a0a0b9d) to head (eddab6b).
Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
src/course-unit/data/thunk.js	72.72%	2 Missing and 1 partial ⚠️
src/course-unit/add-component/AddComponent.jsx	97.43%	1 Missing ⚠️
src/course-unit/hooks.jsx	90.00%	1 Missing ⚠️
src/course-unit/xblock-container-iframe/index.tsx	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1492      +/-   ##
==========================================
+ Coverage   93.35%   93.37%   +0.01%     
==========================================
  Files        1109     1111       +2     
  Lines       22103    22305     +202     
  Branches     4765     4826      +61     
==========================================
+ Hits        20635    20827     +192     
- Misses       1397     1405       +8     
- Partials       71       73       +2

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

src/course-unit/add-component/AddComponent.jsx

src/course-unit/data/thunk.js

src/course-unit/hooks.jsx

src/course-unit/sidebar/SplitTestSidebarInfo.tsx

open-craft-grove · 2025-02-26T13:21:20Z

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

open-craft-grove · 2025-02-26T19:35:03Z

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

open-craft-grove · 2025-02-26T21:04:09Z

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

open-craft-grove · 2025-02-26T22:18:44Z

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

arbrandes

I haven't reviewed the code in depth, yet, but I noticed a few problems when testing in the sandbox:

It's not possible to add an HTML ("Text") block to a content group in the new split_test page. Clicking on it does nothing and throws no detectable error.
Adding a problem works, but the "Select" button when selecting a problem type initially is presented too far down the page, after a blank space. This initially led me to believe the button just wasn't showing.
An X-Frame-Options issue? After selecting a problem type and saving it, my browser refuses to display the contents with an "The loading of in a frame is denied by “X-Frame-Options“ directive set to “sameorigin“." Refreshing the page fixes it. This is reproducible in this PR's sandbox.
When copying a component with an empty clipboard, the "paste component" button does not show until after refreshing the page. (Pasting does work afterwards, though.)
If you click on the config link inside "This content experiment uses group configuration 'Group config 1' at the top of the page, the group config page is embedded in the split_test page instead of directing the user to the top-level route.

I'm not sure how many of the above issues stem from the new page. If they're pre-existing, or if the issues are meant to be solved in a subsequent PR, we can consider merging it as is. Let me know.

PKulkoRaccoonGang · 2025-03-03T15:30:45Z

src/course-unit/add-component/AddComponent.jsx

-        });
+        handleCreateNewCourseXBlock(
+          { type, boilerplate: moduleName, parentLocator: blockId },
+          ({ courseKey, locator }) => navigate(`/course/${courseKey}/editor/html/${locator}`),


[inform]: This code section was covered by the test in the previous PR

PKulkoRaccoonGang · 2025-03-03T15:34:27Z

src/course-unit/context/iFrameContext.tsx

@@ -5,7 +5,7 @@ import { logError } from '@edx/frontend-platform/logging';

 export interface IframeContextType {
  setIframeRef: (ref: MutableRefObject<HTMLIFrameElement | null>) => void;
-  sendMessageToIframe: (messageType: string, payload: unknown) => void;
+  sendMessageToIframe: (messageType: string, payload: unknown, consumerWindow?: any) => void;


[nit]: Let's try to get rid of any and specify a type for consumerWindow.
For example, it could be consumerWindow?: Window | null.

PKulkoRaccoonGang · 2025-03-03T15:39:40Z

src/course-unit/context/iFrameContext.tsx

@@ -16,11 +16,12 @@ export const IframeProvider: React.FC = ({ children }: { children: ReactNode })
    iframeRef.current = ref.current;
  }, []);

-  const sendMessageToIframe = useCallback((messageType: string, payload: any) => {
+  const sendMessageToIframe = useCallback((messageType: string, payload: any, consumerWindow?: any) => {


[curious]: Since we have a full understanding of what's in the payload, should we type the payload structure (type: string, payload: {}, description?: string)?

window.parent.postMessage( { type: '<TYPE_NAME>', payload: { ... }, description: <SOME_TEXT> }, document.referrer );

We don't pass description to messages to iframe, so I think it's not necessary to add it here.
We pass this value from legacy to MFE but not the other way around.

PKulkoRaccoonGang · 2025-03-03T15:44:23Z

src/course-unit/xblock-container-iframe/index.scss

@@ -0,0 +1,4 @@
+.xblock-container-iframe {
+    width: calc(100% + ($spacer * .3125));
+    margin: 0 (-($spacer * .3125));


[optional]: Since this MFE will be moving to the use of Paragon Design tokens, it would be great to already envisage the use of CSS variables in such mathematical constructions.

Suggested change

margin: 0 (-($spacer * .3125));

margin: 0 (($spacer * .3125) * -1);

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

ihor-romaniuk force-pushed the romaniuk/children-container-for-rendering-split-test branch from 3b37da2 to 8e41ceb Compare November 12, 2024 15:25

ihor-romaniuk mentioned this pull request Nov 12, 2024

feat: [FC-0070] add events and style for rendering Split xblock in chromeless template openedx/edx-platform#35813

Open

ihor-romaniuk added the FC Relates to an Axim Funded Contribution project label Nov 12, 2024

ihor-romaniuk mentioned this pull request Nov 28, 2024

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

Merged

ihor-romaniuk force-pushed the romaniuk/children-container-for-rendering-split-test branch 3 times, most recently from 15e0b50 to d22e2e0 Compare February 26, 2025 09:54

ihor-romaniuk marked this pull request as ready for review February 26, 2025 09:54

ihor-romaniuk requested review from PKulkoRaccoonGang, arbrandes and bradenmacdonald February 26, 2025 09:55

feat: [FC-0070] rendering split test content in unit page

d863730

ihor-romaniuk force-pushed the romaniuk/children-container-for-rendering-split-test branch from d22e2e0 to d863730 Compare February 26, 2025 11:09

PKulkoRaccoonGang assigned ihor-romaniuk Feb 26, 2025

PKulkoRaccoonGang added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 26, 2025

PKulkoRaccoonGang suggested changes Feb 26, 2025

View reviewed changes

ihor-romaniuk force-pushed the romaniuk/children-container-for-rendering-split-test branch from 4e4eb4f to 94889ce Compare February 26, 2025 18:53

ihor-romaniuk force-pushed the romaniuk/children-container-for-rendering-split-test branch from 94889ce to beae096 Compare February 26, 2025 19:46

fix: after review

eddab6b

ihor-romaniuk force-pushed the romaniuk/children-container-for-rendering-split-test branch from beae096 to eddab6b Compare February 26, 2025 21:40

arbrandes requested changes Mar 3, 2025

View reviewed changes

PKulkoRaccoonGang reviewed Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [FC-0070] rendering split test content in unit page #1492

feat: [FC-0070] rendering split test content in unit page #1492

ihor-romaniuk commented Nov 8, 2024 •

edited

Loading

openedx-webhooks commented Nov 8, 2024 •

edited

Loading

codecov bot commented Feb 26, 2025 •

edited

Loading

open-craft-grove commented Feb 26, 2025

open-craft-grove commented Feb 26, 2025

open-craft-grove commented Feb 26, 2025

open-craft-grove commented Feb 26, 2025

arbrandes left a comment

PKulkoRaccoonGang Mar 3, 2025

PKulkoRaccoonGang Mar 3, 2025

ihor-romaniuk Mar 4, 2025

PKulkoRaccoonGang Mar 3, 2025 •

edited

Loading

ihor-romaniuk Mar 4, 2025

PKulkoRaccoonGang Mar 3, 2025

ihor-romaniuk Mar 4, 2025

	margin: 0 (-($spacer * .3125));
	margin: 0 (($spacer * .3125) * -1);

feat: [FC-0070] rendering split test content in unit page #1492

Are you sure you want to change the base?

feat: [FC-0070] rendering split test content in unit page #1492

Conversation

ihor-romaniuk commented Nov 8, 2024 • edited Loading

Description

Related Pull Requests

Testing instructions

Other information

openedx-webhooks commented Nov 8, 2024 • edited Loading

codecov bot commented Feb 26, 2025 • edited Loading

Codecov Report

open-craft-grove commented Feb 26, 2025

open-craft-grove commented Feb 26, 2025

open-craft-grove commented Feb 26, 2025

open-craft-grove commented Feb 26, 2025

arbrandes left a comment

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 3, 2025

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 3, 2025

Choose a reason for hiding this comment

ihor-romaniuk Mar 4, 2025

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

ihor-romaniuk Mar 4, 2025

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 3, 2025

Choose a reason for hiding this comment

ihor-romaniuk Mar 4, 2025

Choose a reason for hiding this comment

ihor-romaniuk commented Nov 8, 2024 •

edited

Loading

openedx-webhooks commented Nov 8, 2024 •

edited

Loading

codecov bot commented Feb 26, 2025 •

edited

Loading

PKulkoRaccoonGang Mar 3, 2025 •

edited

Loading