feat: enable markdown to olx conversion#2518
Conversation
|
Thanks for the pull request, @Anas12091101! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2518 +/- ##
==========================================
+ Coverage 94.68% 94.71% +0.02%
==========================================
Files 1203 1208 +5
Lines 26865 27113 +248
Branches 6026 6097 +71
==========================================
+ Hits 25438 25679 +241
- Misses 1357 1376 +19
+ Partials 70 58 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
82428e5 to
f3e2511
Compare
| const state = getState(); | ||
| export const switchToAdvancedEditor = (editorRef) => (dispatch, getState) => { | ||
| const { problem } = getState(); | ||
| const editorObject = fetchEditorContent({ format: '' }); |
There was a problem hiding this comment.
Can we move this editorObject down inside the else { clause?
| problemType: PropTypes.string.isRequired, | ||
| editorType: PropTypes.string.isRequired, | ||
| // eslint-disable-next-line react/forbid-prop-types | ||
| editorRef: PropTypes.object.isRequired, |
There was a problem hiding this comment.
This is fine, but I would have preferred we pass the editorRef around using a ProblemEditorContext instead of prop drilling.
208886b to
86c1977
Compare
|
@Anas12091101 Is this ready for another review? Please let me know whenever you'd like. |
|
Yes, it's ready for review |
arslanashraf7
left a comment
There was a problem hiding this comment.
I did the testing of this PR. It mostly looked good to me. I only noticed a few things but most of those are not related to this PR.
What did I test:
- I was able to create a new markdown problem and I was able to convert it to OLX.
- I was able to convert an existing markdown problem to OLX (e.g,. If a problem was created on the master branch first and then opened in this branch).
- I was able to create markdown problems in libraries and their conversion to OLX
- I tested the rendering of both the Markdown problems and the OLX converted problems in LMS/Learning MFE and they worked fine there as well
- I was able to do the runtime testing to of Markdown to OLX conversion (e.g. I created a new problem converted it to markdown, then without saving it, I then converted it to OLX and it worked fine)
Things I noticed
- I think the dialogue message that appears when you convert a problem to markdown might be incorrect because it still says that you can not convert it back to the simple editor/OLX
2. (Not related to this PR) The problem simple editor still opens up when a problem is converted to OLX. e.g. convert any problem to the advanced/OLX editor, save it, and then edit it again - The simple editor will be opened instead of opening the problem in OLX editor.
@bradenmacdonald, could you recommend an updated version of this message? |
@Anas12091101 I think something like this:
|
|
I think we should change the "Convert to OLX" message too:
|
bradenmacdonald
left a comment
There was a problem hiding this comment.
Thanks, looks good! Please just make this one change to TypeScript for the new file, and update those messages, and then I'll approve and merge.
| @@ -0,0 +1,30 @@ | |||
| import React, { createContext, useContext, useMemo } from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
There was a problem hiding this comment.
prop-types is deprecated. Since this is a new file, can you please use TypeScript .tsx and avoid propTypes or defaultProps?

Description
This PR enables the conversion from Markdown to OLX. The reverse conversion (OLX to Markdown) has been deferred for now, as it would require implementing a dedicated OLX-to-Markdown converter, which may take additional time. We plan to address that in a future update to ensure this change is merged before the Ulmo cut.
Useful information to include:
In Course
https://github.com/user-attachments/assets/cafee2fe-5445-4ec5-b90a-8b2d9b54d9f3
In Library
https://github.com/user-attachments/assets/98f45954-2d11-4dec-abcc-cc75d537882a
Supporting information
#1920
https://github.com/mitodl/hq/issues/8729
Testing instructions
Other information
Include anything else that will help reviewers and consumers understand the change.
Best Practices Checklist
We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:
.ts,.tsx).propTypes,defaultProps, andinjectIntlpatterns are not used in any new or modified code.src/testUtils.tsx(specificallyinitializeMocks)apiHooks.tsin this repo for examples.messages.tsfiles have adescriptionfor translators to use.../. To import from parent folders, use@src, e.g.import { initializeMocks } from '@src/testUtils';instead offrom '../../../../testUtils'