feat: add an allowlist of for supported blocks in library [FC-0062] #1378

navinkarkera · 2024-10-11T06:58:54Z

Description

Libraries v2 currently don't support editing blocks other than problem,
text and videos. This commit adds a configuration variable called
LIBRARY_SUPPORTED_BLOCKS to setup allowed list of block types users
can paste into libraries. By default it is set to support
'problem,text,video,html`.

Also, parse and display error message from server on paste from clipboard failure.

~~Depends on: openedx/edx-platform#35633~~

Supporting information

Testing instructions

Copy any unsupported block from course which is not part of LIBRARY_SUPPORTED_BLOCKS setting in https://github.com/openedx/frontend-app-authoring/pull/1378/files#diff-ec767e14d91498b5b31a4ce826c4596061eb83a47d3dee00f6cbe1078232a75cR136
Try to paste it in the library, it should display an error message.

openedx-webhooks · 2024-10-11T06:58:59Z

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 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.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

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.

codecov · 2024-10-11T07:11:48Z

Codecov Report

Attention: Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.02%. Comparing base (66b14a5) to head (61e5268).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...rary-authoring/add-content/AddContentContainer.tsx	94.73%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1378   +/-   ##
=======================================
  Coverage   93.01%   93.02%           
=======================================
  Files        1035     1035           
  Lines       19632    19646   +14     
  Branches     4172     4101   -71     
=======================================
+ Hits        18261    18275   +14     
- Misses       1306     1309    +3     
+ Partials       65       62    -3

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

rpenido

LGTM 👍
Thank you for your work, @navinkarkera!

I tested this using the instructions from this pr
I read through the code
I checked for accessibility issues
Includes documentation

rpenido · 2024-10-11T18:39:39Z

src/library-authoring/add-content/AddContentContainer.test.tsx

+    axiosMock.onPost(pasteUrl).reply(() => Promise.reject({
+      customAttributes: {
+        httpErrorStatus: 400,
+        httpErrorResponseData: JSON.stringify({ block_type: errMsg }),
+      },
+    }));


Nit: Did you try the createAxiosError function from the testUtils.tsx?

@rpenido Doesn't seem to match the error response from server when paste fails. We don't really need it now but it is nice to have for other paste related errors.

rpenido · 2024-10-11T18:49:48Z

Is it possible to disable the Open Response block in this PR?

It would be great to keep these states in sync with the backend. Could you just check if this is an easy/fast improvement? If not, I think it is fine to leave this way for now.

bradenmacdonald · 2024-10-11T21:00:46Z

Yes, open response and drag and drop should be disabled as well for now. We only support text, problem and video.

If we have time to get "advanced editors" working, we can turn on Drag and Drop.

Libraries v2 currently don't support editing blocks other than problem, text and videos. This commit adds a configuration variable called `LIBRARY_SUPPORTED_BLOCKS` to setup allowed list of block types users can paste into libraries. By default it is set to support 'problem,text,video,html`.

navinkarkera · 2024-10-12T11:01:26Z

@rpenido @bradenmacdonald I have moved the block type check here and also disabled/enabled add block buttons based on LIBRARY_SUPPORTED_BLOCKS env setting.

rpenido

Good work @navinkarkera! 👍

src/index.jsx

src/setupTest.js

Co-authored-by: Rômulo Penido <[email protected]>

ChrisChV

Looks good, only one nit and I will merge it 👍

ChrisChV · 2024-10-14T23:49:20Z

src/index.jsx

@@ -133,6 +133,7 @@ initialize({
        ENABLE_CHECKLIST_QUALITY: process.env.ENABLE_CHECKLIST_QUALITY || 'true',
        ENABLE_GRADING_METHOD_IN_PROBLEMS: process.env.ENABLE_GRADING_METHOD_IN_PROBLEMS === 'true',
        LIBRARY_MODE: process.env.LIBRARY_MODE || 'v1 only',
+        LIBRARY_SUPPORTED_BLOCKS: (process.env.LIBRARY_SUPPORTED_BLOCKS || 'problem,video,html').split(','),


@navinkarkera Could you add this in .env, .env.development and .env.test?

@ChrisChV Done. 61e5268

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 11, 2024

feat: show error msg from server on paste

96f922f

navinkarkera force-pushed the navin/paste_error_msg branch from b72bec2 to 96f922f Compare October 11, 2024 06:59

navinkarkera mentioned this pull request Oct 11, 2024

feat: configuration to block unsupported types while pasting in library [FC-0062] openedx/edx-platform#35633

Closed

navinkarkera changed the title ~~feat: show error msg from server on paste~~ feat: show error msg from server on paste [FC-0062] Oct 11, 2024

navinkarkera marked this pull request as ready for review October 11, 2024 07:10

navinkarkera requested a review from a team as a code owner October 11, 2024 07:10

test: err msg from server on paste

926da3c

rpenido approved these changes Oct 11, 2024

View reviewed changes

navinkarkera changed the title ~~feat: show error msg from server on paste [FC-0062]~~ feat: add an allowlist of for supported blocks in library [FC-0062] Oct 12, 2024

feat: enable add button for blocks based on setting

7afe814

rpenido approved these changes Oct 12, 2024

View reviewed changes

src/index.jsx Outdated Show resolved Hide resolved

src/setupTest.js Outdated Show resolved Hide resolved

refactor: list of blocks

9cc69ce

Co-authored-by: Rômulo Penido <[email protected]>

ChrisChV approved these changes Oct 14, 2024

View reviewed changes

chore: add LIBRARY_SUPPORT_BLOCKS to env files

61e5268

ChrisChV merged commit 7fb4600 into openedx:master Oct 15, 2024
8 checks passed

ChrisChV deleted the navin/paste_error_msg branch October 15, 2024 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add an allowlist of for supported blocks in library [FC-0062] #1378

feat: add an allowlist of for supported blocks in library [FC-0062] #1378

navinkarkera commented Oct 11, 2024 •

edited

Loading

openedx-webhooks commented Oct 11, 2024 •

edited

Loading

codecov bot commented Oct 11, 2024 •

edited

Loading

rpenido left a comment

rpenido Oct 11, 2024

navinkarkera Oct 12, 2024

rpenido commented Oct 11, 2024

bradenmacdonald commented Oct 11, 2024

navinkarkera commented Oct 12, 2024

rpenido left a comment

ChrisChV left a comment •

edited

Loading

ChrisChV Oct 14, 2024

navinkarkera Oct 15, 2024

feat: add an allowlist of for supported blocks in library [FC-0062] #1378

feat: add an allowlist of for supported blocks in library [FC-0062] #1378

Conversation

navinkarkera commented Oct 11, 2024 • edited Loading

Description

Supporting information

Testing instructions

openedx-webhooks commented Oct 11, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Oct 11, 2024 • edited Loading

Codecov Report

rpenido left a comment

Choose a reason for hiding this comment

rpenido Oct 11, 2024

Choose a reason for hiding this comment

navinkarkera Oct 12, 2024

Choose a reason for hiding this comment

rpenido commented Oct 11, 2024

bradenmacdonald commented Oct 11, 2024

navinkarkera commented Oct 12, 2024

rpenido left a comment

Choose a reason for hiding this comment

ChrisChV left a comment • edited Loading

Choose a reason for hiding this comment

ChrisChV Oct 14, 2024

Choose a reason for hiding this comment

navinkarkera Oct 15, 2024

Choose a reason for hiding this comment

navinkarkera commented Oct 11, 2024 •

edited

Loading

openedx-webhooks commented Oct 11, 2024 •

edited

Loading

codecov bot commented Oct 11, 2024 •

edited

Loading

ChrisChV left a comment •

edited

Loading