feat: configuration to block unsupported types while pasting in library [FC-0062] #35633
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @navinkarkera! What's next?Please work through the following steps to get your changes 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. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
Thanks for the pull request, @navinkarkera! What's next?Please work through the following steps to get your changes 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. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Adds setting to stop authors from pasting unsupported block types in libraries
navinkarkera
force-pushed
the
navin/configure-unsupported-block-types
branch
from
37cef00 to
3459209
Compare
navinkarkera
changed the title
navinkarkera
changed the title
| @@ -815,6 +815,8 @@ def validate_can_add_block_to_library( | |||
| block_type=block_type, library_type=content_library.type, | |||
| ) | |||
| ) | |||
| if block_type in settings.LIBRARY_UNSUPPORTED_BLOCKS: | |||
| raise IncompatibleTypesError(_('Libraries do not support this type of content yet.')) | |||
There was a problem hiding this comment.
Should we include block_type in the message?
There was a problem hiding this comment.
I think the current message is enough as we are using the Toast to show the message.
There was a problem hiding this comment.
LGTM 👍
Thank you for your work, @navinkarkera!
- I tested this using the instructions from feat: add an allowlist of for supported blocks in library [FC-0062] frontend-app-authoring#1378
- I read through the code
-
I checked for accessibility issues - Includes documentation
| @@ -815,6 +815,8 @@ def validate_can_add_block_to_library( | |||
| block_type=block_type, library_type=content_library.type, | |||
| ) | |||
| ) | |||
| if block_type in settings.LIBRARY_UNSUPPORTED_BLOCKS: | |||
| raise IncompatibleTypesError(_('Libraries do not support this type of content yet.')) | |||
There was a problem hiding this comment.
I think the current message is enough as we are using the Toast to show the message.
|
I think this should be an allowlist, not a denylist. Anything other than Problem, Text, or Video is not going to work in libraries at the moment. But the reason for that is mostly that the frontend doesn't yet support the editors for those types, so I am almost inclined to have this check only on the frontend. [If you're using the backend only through APIs and OLX, almost any type of XBlock should work fine in content libraries v2. It's just the new frontend that doesn't support editors/previews for most XBlocks yet.] |
|
I agree with braden's inclination. It feels weird that in the future when a new type is supported by the frontend it'll need a PR for both the frontend and the backend. |
@bradenmacdonald Yes, if the allowed blocks are fixed than it is better to have an allowlist. My impression was that only few blocks are not supported.
Makes sense, I'll close this PR and move this check in frontend.
@DanielVZ96 It won't need a PR in both frontend and backend as it is configured only in backend, frontend is only displaying the server error message. But it does make sense to move this check to frontend as it is limited by frontend's ability to edit them. |
Description
Adds setting to stop authors from pasting unsupported block types in libraries
Supporting information
Private-ref: https://tasks.opencraft.com/browse/FAL-3860Testing instructions
See openedx/frontend-app-authoring#1378 for test instructions.
Deadline
ASAP