-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[backport] fix: render library v2 assets with whitespace (#35974) #36060
[backport] fix: render library v2 assets with whitespace (#35974) #36060
Conversation
Assets that contain whitespace fail to be rendered when uploaded to library v2
Thanks for the pull request, @DanielVZ96! This repository is currently maintained by @openedx/wg-maintenance-edx-platform. 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.
|
Sandbox deployment successful 🚀 |
Sandbox deployment successful 🚀 |
@DanielVZ96 I tested this in my local and I got this error. Can you check if it's not just me? CC @bradenmacdonald |
@ChrisChV @DanielVZ96 I'm having a different issue - the image asset is working just fine, on |
@bradenmacdonald I got the same error as @ChrisChV for some reason the edx-platform sumac.master branch is missing this endpoint. also in order to test this PR you need to use the text editor file upload feature to upload the images and test that they render correctly. but it seems like the text editor no longer has that functionality or it needs to be activated somehow. |
It was never backported to Sumac, so that feature doesn't currently exist. Can you not test the bug using the "Advanced" file upload, which is what I showed in my screenshot? If the bug only affects the visual editor and not regular library asset uploads in general, then we don't need to backport it for now. |
Yes, that endpoint was added after The Sumac release was cut. Perhaps you are using the wrong branch of frontend-app-authoring then, because the corresponding code is not in the |
But if the bug only affects the text editor file upload feature, and assets with a space in their name work perfectly well otherwise, then why are we fixing it in edx-platform? It sounds like it's a bug in the frontend editor in that case. |
@bradenmacdonald testing edx-platform sumac.master with frontend-app-authoring sumac.master the assets with whitespace aren't loading for me in the cms |
@DanielVZ96 Can you show me a screenshot and specific testing instructions? |
@bradenmacdonald here's a video:
https://www.loom.com/share/2e64c23b2e07457caa51e11580c6dc7d?sid=c8765bdd-73c0-494c-a4ee-bd456a56a87e I also tested with this PR's code and it's still failing with the advanced image upload, but with the new mfe editor image upload (i was able to find it through studio), it works correctly: https://www.loom.com/share/21945849fa4345c6b100c0ff4d80abda?sid=19a4b424-678c-4e26-8863-40cee9919492 |
I think we can close this for now and consider fixing it as part of backporting openedx/modular-learning#246 . |
Assets that contain whitespace fail to be rendered when uploaded to library v2
Description
Assets that contain whitespace fail to be rendered when uploaded to library v2.
Backport of: #35974
Deadline
ASAP