feat: plugin slots for video and file upload components and alerts by xitij2000 · Pull Request #1523 · openedx/frontend-app-authoring

xitij2000 · 2024-11-20T12:37:40Z

Description

This change adds plugin slots for the file and video upload pages to allow developers to customise those components. It also adds empty slots allowing developers to add additional alerts.

Supporting information

NA

Testing instructions

TBD

Other information

This change is being made to support a feature where a client wants course authors to acknowledge fair use terms before they are allowed to upload files

Private-ref: https://tasks.opencraft.com/browse/BB-9330

openedx-webhooks · 2024-11-20T12:37:45Z

Thanks for the pull request, @xitij2000!

This repository is currently maintained by @bradenmacdonald.

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.

Details

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-11-20T12:48:48Z

Codecov Report

❌ Patch coverage is 98.80952% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.75%. Comparing base (a56faf8) to head (f2825e9).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
...c/files-and-videos/files-page/CourseFilesTable.tsx	98.03%	1 Missing ⚠️
...files-and-videos/videos-page/CourseVideosTable.tsx	98.91%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1523      +/-   ##
==========================================
+ Coverage   94.73%   94.75%   +0.02%     
==========================================
  Files        1206     1225      +19     
  Lines       27014    27375     +361     
  Branches     6066     6166     +100     
==========================================
+ Hits        25592    25940     +348     
- Misses       1351     1364      +13     
  Partials       71       71

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kaustavb12

👍

Left a minor nit. Otherwise LGTM.

I tested this: Tested in local
I read through the code

src/files-and-videos/files-page/FilesPage.jsx

mphilbrick211 · 2025-02-24T22:12:29Z

@xitij2000 is this still in progress?

xitij2000 · 2025-02-25T08:23:34Z

@xitij2000 is this still in progress?

Yes, I realise I forgot to commit and push the docs here. Will do that shortly and then it's ready for review.

mphilbrick211 · 2025-09-16T19:50:47Z

Hi @xitij2000 - can we close this, or is it still needed?

xitij2000 · 2025-09-23T09:50:09Z

Hi @xitij2000 - can we close this, or is it still needed?

I will update it asap. It is still needed.

bradenmacdonald · 2025-09-23T16:21:11Z

@xitij2000 You can ping me for a review when ready.

xitij2000 · 2025-09-24T08:28:35Z

@bradenmacdonald This is ready for review.

xitij2000 · 2025-09-24T08:30:27Z

src/plugin-slots/CourseVideosSlot/index.tsx

+        currentData: uploadingIdsRef.current.uploadData,
+        originalValue: { name, progress },
+        key: `video_${idx}`,
+        edxVideoId: undefined,


Set this to undefined to make TS happy, since it's expected by the function.

xitij2000 · 2025-09-24T08:31:34Z

src/files-and-videos/videos-page/VideosPage.jsx

-  const handleDownloadFile = (selectedRows) => dispatch(fetchVideoDownload({ selectedRows, courseId }));
-  const handleUsagePaths = (video) => dispatch(getUsagePaths({ video, courseId }));
-  const handleFileOrder = ({ newFileIdOrder, sortType }) => {
-    dispatch(updateVideoOrder(courseId, newFileIdOrder, sortType));


While there is a sortType param here, the updateVideoOrder function doesn't seem to take this param, so it's passed pointlessly.

bradenmacdonald

@xitij2000 so sorry for the delay. I think this looks good; just needs some updates to the READMEs. Could you please make those fixes and rebase this, then I can merge it?

src/plugin-slots/CourseOutlinePageAlertsSlot/README.md

src/plugin-slots/CourseVideosSlot/README.md

src/plugin-slots/EditFileErrorAlertsSlot/README.md

src/plugin-slots/CourseFilesSlot/README.md

bradenmacdonald · 2025-10-16T16:23:49Z

Thanks for those changes. There's a new test failure to fix.

xitij2000 · 2025-10-17T12:42:58Z

Thanks for those changes. There's a new test failure to fix.

Fixed!

bradenmacdonald

OK, I still think it would make more sense to have separate slots for the videos and files page, but this is good with me.

xitij2000 · 2025-10-18T05:38:09Z

@bradenmacdonald I can make that change, it will just need some minor refactoring.

bradenmacdonald · 2025-10-21T23:59:09Z

OK, I think that would be a good idea. Then I'm happy to merge this.

xitij2000 · 2025-10-22T07:26:40Z

OK, I think that would be a good idea. Then I'm happy to merge this.

I've split the slots and also renamed them. I've removed 'error' from their name since they can be used for other kinds of alerts as well.

bradenmacdonald · 2025-10-22T19:35:39Z

@xitij2000 I think you forgot to push that change? I don't see any new commits, and I still see the combined slot.

This change add plugin slots for the file and video upload components, and the alerts components on those pages.

xitij2000 · 2025-10-23T08:18:04Z

@xitij2000 I think you forgot to push that change? I don't see any new commits, and I still see the combined slot.

Odd, I think the push failed, I've pushed again with all fixes.

bradenmacdonald

Thanks for those changes, and for refactoring the plugins to TypeScript.

itsjeyd · 2025-10-29T09:23:37Z

@xitij2000 @bradenmacdonald Just to confirm, will this PR work without the changes from openedx/openedx-platform#35895 (which are currently in product review)?

xitij2000 · 2025-10-29T09:32:58Z

@itsjeyd Yes, those changes are required for the plugins that are intended to go in these slots, but not for the slots themselves.

xitij2000 requested a review from a team as a code owner November 20, 2024 12:37

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

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from d1a51a4 to 694edb1 Compare November 20, 2024 12:38

kaustavb12 approved these changes Nov 28, 2024

View reviewed changes

src/files-and-videos/files-page/FilesPage.jsx Show resolved Hide resolved

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch 2 times, most recently from 4e9d9df to 9c79a41 Compare December 19, 2024 08:21

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from 9c79a41 to efafa3c Compare February 26, 2025 08:06

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from efafa3c to b42f8bc Compare March 12, 2025 12:42

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from b42f8bc to bedef36 Compare June 11, 2025 12:00

openedx-webhooks added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Jun 11, 2025

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from bedef36 to abb9a6f Compare June 25, 2025 09:24

mphilbrick211 moved this from In Eng Review to Waiting on Author in Contributions Jul 16, 2025

mphilbrick211 added the inactive PR author has been unresponsive for several months label Sep 16, 2025

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch 8 times, most recently from d709e67 to 7769d81 Compare September 23, 2025 12:29

xitij2000 commented Sep 24, 2025

View reviewed changes

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from 7769d81 to 994468d Compare September 26, 2025 05:46

bradenmacdonald reviewed Oct 15, 2025

View reviewed changes

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from 33c0789 to 40b9ec7 Compare October 16, 2025 09:11

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch 2 times, most recently from 734b4c9 to f0461f1 Compare October 17, 2025 12:32

bradenmacdonald approved these changes Oct 17, 2025

View reviewed changes

feat: Add slots for video and file upload components and alerts

f2825e9

This change add plugin slots for the file and video upload components, and the alerts components on those pages.

xitij2000 force-pushed the kshitij/plugin-slots/files-and-alerts branch from e48366d to f2825e9 Compare October 23, 2025 04:43

bradenmacdonald approved these changes Oct 24, 2025

View reviewed changes

bradenmacdonald merged commit 15a728d into master Oct 24, 2025
7 checks passed

bradenmacdonald deleted the kshitij/plugin-slots/files-and-alerts branch October 24, 2025 21:39

github-project-automation bot moved this from Waiting on Author to Done in Contributions Oct 24, 2025

openedx-webhooks removed the inactive PR author has been unresponsive for several months label Oct 24, 2025

Conversation

xitij2000 commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Other information

Uh oh!

openedx-webhooks commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kaustavb12 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mphilbrick211 commented Feb 24, 2025

Uh oh!

xitij2000 commented Feb 25, 2025

Uh oh!

mphilbrick211 commented Sep 16, 2025

Uh oh!

xitij2000 commented Sep 23, 2025

Uh oh!

bradenmacdonald commented Sep 23, 2025

Uh oh!

xitij2000 commented Sep 24, 2025

Uh oh!

xitij2000 Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

xitij2000 Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bradenmacdonald commented Oct 16, 2025

Uh oh!

xitij2000 commented Oct 17, 2025

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

xitij2000 commented Oct 18, 2025

Uh oh!

bradenmacdonald commented Oct 21, 2025

Uh oh!

xitij2000 commented Oct 22, 2025

Uh oh!

bradenmacdonald commented Oct 22, 2025

Uh oh!

xitij2000 commented Oct 23, 2025

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

itsjeyd commented Oct 29, 2025

Uh oh!

xitij2000 commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

xitij2000 commented Nov 20, 2024 •

edited

Loading

openedx-webhooks commented Nov 20, 2024 •

edited

Loading

codecov bot commented Nov 20, 2024 •

edited

Loading

kaustavb12 left a comment •

edited

Loading