Skip to content

fix: open mfe modal editor of new duplicated xblock #36641

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.

Sign up for GitHub

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

Merged
ChrisChV merged 6 commits into from
May 20, 2025
Merged

fix: open mfe modal editor of new duplicated xblock #36641

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c5d6f45
fix: open mfe modal editor of new duplicated xblock
DanielVZ96 May 1, 2025
2bf2dfe
feat: send videogallery waffle flag to mfe
DanielVZ96 May 10, 2025
9b5dac2
fix: update waffle flag tests
DanielVZ96 May 10, 2025
f17d0b6
fix: backwards compatibility
DanielVZ96 May 13, 2025
587cb84
chore: empty commit to run
DanielVZ96 May 13, 2025
66cfa86
Merge branch 'master' into dvz/mfe-fix-unit-studio-duplicate
ChrisChV May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions cms/static/js/views/pages/container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ function($, _, Backbone, gettext, BasePage,
this.isLibraryPage = this.model.attributes.category === 'library';
this.isLibraryContentPage = this.model.attributes.category === 'library_content';
this.isSplitTestContentPage = this.model.attributes.category === 'split_test';
this.isVerticalContentPage = this.model.attributes.category === 'vertical';

this.nameEditor = new XBlockStringFieldEditor({
el: this.$('.wrapper-xblock-field'),
model: this.model
Expand Down Expand Up @@ -160,6 +162,9 @@ function($, _, Backbone, gettext, BasePage,
case 'completeManageXBlockAccess':
this.refreshXBlock(xblockElement, false);
break;
case 'completeXBlockDuplicating':
this.refreshXBlock(xblockElement, true, true);
break;
case 'completeXBlockMoving':
xblockWrapper.hide();
break;
Expand Down Expand Up @@ -1138,7 +1143,6 @@ function($, _, Backbone, gettext, BasePage,
onNewXBlock: function(xblockElement, scrollOffset, is_duplicate, data) {
var useNewTextEditor = this.$('.xblock-header-primary').attr('use-new-editor-text'),
useNewVideoEditor = this.$('.xblock-header-primary').attr('use-new-editor-video'),
useVideoGalleryFlow = this.$('.xblock-header-primary').attr("use-video-gallery-flow"),

@bradenmacdonald bradenmacdonald May 1, 2025

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.

Are we sure it's OK to remove this "video gallery flow"? Isn't it something that 2U uses?

@pomegranited pomegranited May 2, 2025

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.

Oh no.. @DanielVZ96 had code in openedx/frontend-app-authoring#1838 (review) which used this alternate video creation flow, but I didn't know it existed, and so I reverted those changes 😢

But you're right, we should avoid breaking anything else, and fix what we removed.. sorry about this!

@DanielVZ96 DanielVZ96 May 2, 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.

@bradenmacdonald you are right. I'll have to refactor this a bit to handle both flows.

@ChrisChV ChrisChV May 13, 2025

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.

@DanielVZ96 @pomegranited CC @bradenmacdonald I'm not sure if this is in scope, but when you open the editor from the Unity legacy page, with new_core_editors.use_video_gallery_flow enabled, the video gallery workflow doesn't open.

@DanielVZ96 DanielVZ96 May 13, 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.

@ChrisChV I think that is in scope. nice find.

@ChrisChV ChrisChV May 14, 2025

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.

I still see the issue: https://www.loom.com/share/e890c58a81e34a34a08c3fa708d1197d

@ChrisChV ChrisChV May 19, 2025

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.

@DanielVZ96 I still can't see the gallery workflow when opening the editor from the legacy unit page. I have rebuilt the assets. Maybe my local is misconfigured, @pomegranited could you test if it works?

@pomegranited pomegranited May 20, 2025

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.

@ChrisChV I didn't think we had to worry about the legacy unit page anymore -- Isn't that right @bradenmacdonald ?

@bradenmacdonald bradenmacdonald May 20, 2025

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.

Not really, but we should at least avoid introducing regressions on it.

@ChrisChV ChrisChV May 20, 2025

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.

Ok, make sense to me, I can confirm that is not a regression 👍

useNewProblemEditor = this.$('.xblock-header-primary').attr('use-new-editor-problem');

// find the block type in the locator if availible
Expand All @@ -1151,15 +1155,7 @@ function($, _, Backbone, gettext, BasePage,
|| (useNewVideoEditor === 'True' && blockType.includes('video'))
|| (useNewProblemEditor === 'True' && blockType.includes('problem')))
){
var destinationUrl;
if (useVideoGalleryFlow === 'True' && blockType.includes('video')) {
destinationUrl = this.$('.xblock-header-primary').attr("authoring_MFE_base_url") + '/course-videos/' + encodeURI(data.locator);
}
else {
destinationUrl = this.$('.xblock-header-primary').attr("authoring_MFE_base_url") + '/' + blockType[1] + '/' + encodeURI(data.locator);
}

if (this.options.isIframeEmbed && this.isSplitTestContentPage) {
if (this.options.isIframeEmbed && (this.isSplitTestContentPage || this.isVerticalContentPage)) {
return this.postMessageToParent({
type: 'handleRedirectToXBlockEditPage',
message: 'Redirect to xBlock edit page',
Expand All @@ -1169,8 +1165,6 @@ function($, _, Backbone, gettext, BasePage,
},
});
}

window.location.href = destinationUrl;
return;
}
if (!this.options.isIframeEmbed) {
Expand Down