Skip to content

feat: add Proctoring Info Panel plugin slot - #1759

Closed
Anas12091101 wants to merge 2 commits into
openedx:masterfrom
mitodl:anas/proctoring-info-plugin-slot
Closed

feat: add Proctoring Info Panel plugin slot#1759
Anas12091101 wants to merge 2 commits into
openedx:masterfrom
mitodl:anas/proctoring-info-plugin-slot

Conversation

@Anas12091101

@Anas12091101 Anas12091101 commented Jul 9, 2025

Copy link
Copy Markdown
Contributor

Description

This PR introduces a plugin slot to enable overriding the ProctoringInfoPanel component. It also adds a new proctoringReviewRequirementsButtonLink prop in the ProctoringInfoPanel, allowing customization of the "Review instructions and system requirements" link without requiring a full replacement of the ProctoringInfoPanel.

Screenshots

  • Default
    Screenshot 2025-07-09 at 12 24 40 PM

  • Custom
    Screenshot 2025-07-09 at 12 00 13 PM

Testing Instructions

  • Override the plugin slot by pasting the following config in the env.config.jsx file
    import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';
    
    const config = {
     pluginSlots: {
       "org.openedx.frontend.learning.proctoring_info_panel.v1":{
           plugins: [
             {
               op: PLUGIN_OPERATIONS.Insert,
               widget: {
                 id: 'proctoring_info_panel',
                 type: DIRECT_PLUGIN,
                 RenderWidget: () => <h3>Proctoring Info Panel</h3>,
               },
             },
           ],
       }}
    }
    export default config;
  • Ensure that the custom component is rendered in place of the default component.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 9, 2025
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @Anas12091101!

This repository is currently maintained by @openedx/committers-frontend-app-learning.

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.

* `proctoring_info_panel_slot`

### Props:
* `proctoringReviewRequirementsButtonLink`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start, thanks! But it would be ideal if you could add:

  1. An example of how to use this slot, including sample env.config.jsx code.
  2. A screenshot of the slot using the above example.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @arbrandes!

The reason I initially left out the screenshots and env.config.jsx example was because I noticed that some of the recently added plugin slots, like ContentIFrameLoaderSlot were missing them in their README as well.

That said, I’ve now added the example screenshots and config code in 36e6ceb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any slot that is missing documentation is a bug. All slots should have documentation. However some don't and if you have context on the ones that don't we'd love PRs. Some of the ones that don't have docs are because we found them, but we don't know how to use them.

@Anas12091101
Anas12091101 requested a review from arbrandes July 18, 2025 10:52
@itsjeyd itsjeyd moved this from Needs Triage to In Eng Review in Contributions Jul 24, 2025
@itsjeyd itsjeyd added waiting for eng review PR is ready for review. Review and merge it, or suggest changes. needs test run Author's first PR to this repository, awaiting test authorization from Axim labels Jul 24, 2025
@itsjeyd

itsjeyd commented Jul 24, 2025

Copy link
Copy Markdown

Hi @arbrandes @sarina @brian-smith-tcril, do PRs that introduce plugin slots generally require product review?

CC @mphilbrick211

@sarina

sarina commented Jul 24, 2025

Copy link
Copy Markdown
Contributor

@itsjeyd I don't think slots generally require product review, but it's not a bad idea to get some product input on new slots. For example I wonder with respect to this slot - can it only be used for proctoring information? I doubt it. I think the name should be more generic and the example could illustrate how to use it with proctoring info.

@sarina

sarina commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Hi @Anas12091101 - I've approved your test run. I've also invited you to the Open edX organization so your tests will run with each commit. Please visit https://github.com/openedx to accept the invitation.

@sarina sarina added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs test run Author's first PR to this repository, awaiting test authorization from Axim waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Jul 30, 2025
@sarina

sarina commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

@Anas12091101 I'm still waiting on an answer to this question:

can [this slot] only be used for proctoring information?

@brian-smith-tcril

Copy link
Copy Markdown
Contributor

@sarina there's nothing about this slot that would make it only able to be used for proctoring information. That being said, I don't think simply renaming this slot to be more generic is a good solution either. In the OutlineTab component we have (simplified):

<Title />
<Alerts />
<StartOrResumeCourseCard />
<WelcomeMessage />
<CourseHomeSectionOutlineSlot />
<ProctoringInfoPanel />
<WeeklyLearningGoalCard />
<CourseTools />
<CourseOutlineTabNotificationsSlot />
<CourseDates />
<CourseHandouts />

so renaming this would mean we'd have

<Title />
<Alerts />
<StartOrResumeCourseCard />
<WelcomeMessage />
<CourseHomeSectionOutlineSlot />
- <ProctoringInfoPanel />
+ <SomeGenericSlotInTheMiddleOfItAll /> 
<WeeklyLearningGoalCard />
<CourseTools />
<CourseOutlineTabNotificationsSlot />
<CourseDates />
<CourseHandouts />

We also already have the CourseHomeSectionOutlineSlot in there, so this seems to be mostly following that pattern.

I'm not sure what the best path forward for this is.

@codecov

codecov Bot commented Jul 30, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.47%. Comparing base (b050535) to head (36e6ceb).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1759      +/-   ##
==========================================
+ Coverage   90.46%   90.47%   +0.01%     
==========================================
  Files         344      345       +1     
  Lines        5812     5819       +7     
  Branches     1360     1402      +42     
==========================================
+ Hits         5258     5265       +7     
+ Misses        537      535       -2     
- Partials       17       19       +2     

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

@sarina

sarina commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

CourseHomeSectionOutlineInfoPanel? 😬

I don't like naming this slot something so prescriptive when it could be used for different things, but yeah, I see it's just in the middle. Is this slot even necessary? If it's in the middle of the CourseHomeSectionOutlineSlot, couldn't operators just put a Proctoring Info Panel into that slot along with their other content?

@brian-smith-tcril

Copy link
Copy Markdown
Contributor

If it's in the middle of the CourseHomeSectionOutlineSlot, couldn't operators just put a Proctoring Info Panel into that slot along with their other content?

They could, but that wouldn't remove the default ProctoringInfoPanel content.

@brian-smith-tcril

brian-smith-tcril commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

So it seems like there's a macro issue here and a micro issue here.

The macro issue is that site operators don't have enough flexibility to customize the OutlineTab/Course About Sidebar.

The micro issue is that the "Review instructions and system requirements" button in the ProctoringInfoPanel is hardcoded to link to https://support.edx.org/hc/en-us/sections/115004169247-Taking-Timed-and-Proctored-Exams and there's no way for site operators to override that.

This PR solves the micro issue by wrapping the ProctoringInfoPanel in a slot (I noticed that the URL isn't being exposed as a plugin prop in this PR but I assume that's just an oversight).

If the main use case for this slot is changing that URL, then keeping it as ProctoringInfoPanelSlot makes sense to me.

@sarina

sarina commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

We do have a way I thought now of overriding URLs: https://github.com/openedx/frontend-platform?tab=readme-ov-file#overriding-default-external-links

@brian-smith-tcril

Copy link
Copy Markdown
Contributor

We do have a way I thought now of overriding URLs: https://github.com/openedx/frontend-platform?tab=readme-ov-file#overriding-default-external-links

I remembered discussing that but I wasn't aware that it had actually landed! I'd definitely be in favor of setting up that URL to be replaceable using externalLinkUrlOverrides and putting off the slot aspect of this for later.

@Anas12091101

Copy link
Copy Markdown
Contributor Author

Thanks @sarina and @brian-smith-tcril!

The core issue we encountered was the hardcoded link for the "Review instructions and system requirements" button. I initially opened this PR to address it using a slot. However, since externalLinkUrlOverrides provides a more convenient solution, we'd prefer to use that instead.

I noticed that the version of frontend-platform in this MFE is still v8.3.1, whereas externalLinkUrlOverrides was introduced in v8.4.0. Do you know when this MFE will be upgraded to use v8.4.0?

@brian-smith-tcril

Copy link
Copy Markdown
Contributor

@Anas12091101 it's 8.4.0 in the package lock

"node_modules/@edx/frontend-platform": {
"version": "8.4.0",

so you should be able to use that functionality right now. It'd be good to update the pin in package.json to specify we need ^8.4.0 as part of the PR that utilizes the externalLinkUrlOverrides functionality to accurately communicate the version required by the new code.

@Anas12091101

Copy link
Copy Markdown
Contributor Author

Great, I’ll open a separate PR for this. Should I go ahead and close this one for now?

@brian-smith-tcril

Copy link
Copy Markdown
Contributor

Closing in favor of #1775

I'm glad we were able to work through the requirements and figure out a good path forward!

Thank you!

@github-project-automation github-project-automation Bot moved this from In Eng Review to Done in Contributions Aug 4, 2025
@openedx-webhooks openedx-webhooks removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants