Skip to content
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

feat: [FC-0070] handled edit modals from advanced xblocks #1445

Conversation

PKulkoRaccoonGang
Copy link
Contributor

@PKulkoRaccoonGang PKulkoRaccoonGang commented Oct 29, 2024

🚨 Dependencies:

Description

This PR includes adding new message types, updating message handlers, and implementing a new modal iframe for legacy XBlock editing.

Supporting information

The next PR will be adding post messages for the tagging functionality.

Testing instructions

Screen.Recording.2025-02-17.at.09.56.38.mov

Other information

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: Peter_Kulko/advanced_blocks_edit_and_move_modals

TUTOR_GROVE_NEW_MFES:
  authoring:
    port: 18000
    repository: https://github.com/raccoongang/frontend-app-course-authoring.git
    version: Peter_Kulko/edit-modal-for-advanced-xblocks

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

@PKulkoRaccoonGang PKulkoRaccoonGang requested a review from a team as a code owner October 29, 2024 08:11
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 29, 2024
@openedx-webhooks
Copy link

openedx-webhooks commented Oct 29, 2024

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.


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.

@PKulkoRaccoonGang PKulkoRaccoonGang marked this pull request as draft October 29, 2024 08:11
@mphilbrick211 mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 29, 2024
@PKulkoRaccoonGang PKulkoRaccoonGang self-assigned this Nov 5, 2024
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/edit-modal-for-advanced-xblocks branch 2 times, most recently from f6b4000 to 648e1ae Compare November 6, 2024 17:43
].join(' ');

export const IFRAME_FEATURE_POLICY = (
'microphone *; camera *; midi *; geolocation *; encrypted-media *, clipboard-write *'
Copy link
Contributor

Choose a reason for hiding this comment

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

This has a typo - see openedx/frontend-app-learning#1500

Also, the same code in frontend-app-learning has this important notice which seems like it should be here too:

https://github.com/openedx/frontend-app-learning/blob/0effb3231831ef54eb5951d3bfa85d4975989605/src/courseware/course/sequence/Unit/ContentIFrame.jsx#L12-L23

@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/edit-modal-for-advanced-xblocks branch from 0bec87f to c788c40 Compare February 13, 2025 07:32
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 93.47826% with 3 lines in your changes missing coverage. Please review.

Project coverage is 93.30%. Comparing base (e9130d3) to head (9901401).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/course-unit/data/thunk.js 72.72% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1445      +/-   ##
==========================================
- Coverage   93.30%   93.30%   -0.01%     
==========================================
  Files        1101     1102       +1     
  Lines       21856    21899      +43     
  Branches     4645     4729      +84     
==========================================
+ Hits        20393    20433      +40     
+ Misses       1398     1395       -3     
- Partials       65       71       +6     

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

@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/edit-modal-for-advanced-xblocks branch from 7cf8b21 to fb13bfb Compare February 13, 2025 09:59
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/edit-modal-for-advanced-xblocks branch from fb13bfb to 5699716 Compare February 13, 2025 10:17
@PKulkoRaccoonGang PKulkoRaccoonGang added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 13, 2025
@open-craft-grove
Copy link

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

@PKulkoRaccoonGang PKulkoRaccoonGang marked this pull request as ready for review February 13, 2025 12:00
@PKulkoRaccoonGang 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 Feb 13, 2025
@open-craft-grove
Copy link

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

@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/edit-modal-for-advanced-xblocks branch from 78a2326 to 9901401 Compare February 17, 2025 08:55
@open-craft-grove
Copy link

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

@open-craft-grove
Copy link

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

@PKulkoRaccoonGang
Copy link
Contributor Author

@arbrandes I have not been able to reproduce these issues. Could you please confirm if you are still experiencing them?
#1544 (review)

@open-craft-grove
Copy link

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
Copy link

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

@arbrandes
Copy link
Contributor

@PKulkoRaccoonGang, it seems the sandbox was not configured to deploy with the edx-platform PR branch it's supposed to depend on. Can you confirm?

@PKulkoRaccoonGang 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 Feb 18, 2025
@PKulkoRaccoonGang
Copy link
Contributor Author

PKulkoRaccoonGang commented Feb 18, 2025

@arbrandes Yes, a few days ago I tried to do it from the correct branches (related to this PR), but it didn't work. Then I tried to check if the deployment from the master branches would be successful, the result was still unsuccessful.
It seems that the sandbox worked yesterday. Restarted the sandbox deployment

UPD: Sandbox deployment successful 🚀

@open-craft-grove
Copy link

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

@arbrandes
Copy link
Contributor

I can't access the sandbox to test. I have asked @kaustavb12 to take a figure out if it's a deployment issue.

@PKulkoRaccoonGang
Copy link
Contributor Author

@arbrandes I can log into the Studio in the deployed sandbox. Maybe try to log in via a direct link.
https://app.pr-1445-ba9a65.sandboxes.opencraft.hosting/authoring/course/course-v1:Test+123+2024

Copy link
Contributor

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

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

I could find nothing to object to in the code, and things work well as far as I was able to test them. Approved!

See openedx/edx-platform#35777 (review) for more review comments, as I tested both PRs simultaneously.

@arbrandes arbrandes merged commit 7e4ecff into openedx:master Feb 20, 2025
7 checks passed
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
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants