openedx
feat: removes sidebar upgrade notification component
Followup to #1368.
~These components are specific to edx.org and will be moved to a 2U owned plugin. There are some open questions and I'd like to go back and manually test everything once the 2U plugin is updated to no longer import these. Leaving this in draft state for now.~
edx.org has migrated these components to a 2U owned plugin. This code is no longer being used and can be deprecated. While the LockPaywall is also unused we have opted not to remove it as originally intended. See note below
Removed by this PR
- ~LockPaywall: Upgrade ad shown in place of graded unit if content gating is enabled~ No longer included in this PR
- UpgradeNotification: Sidebar upgrade advertisement
- ~UpsellBullets: Only used by Upgrade/Paywall~ Cannot be removed since we're keeping the LockPaywall
Not Removed by this PR
- Other instances of the Upgrade Button
Open Questions
Unsure on what to do with the LockPaywall. Should this actually be removed? It puts us in a weird state where 'nothing' would be displayed if content gating is enabled and that plugin slot isn't configured. I'm thinking this should either be reworked so the shouldDisplayContentGating logic moves into the plugin itself or we don't touch LockPaywall at all here.
Decision: We will not be removing the LockPaywall as part of this effort
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 89.02%. Comparing base (
7efe8f5) to head (397f6a7). Report is 196 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #1410 +/- ##
==========================================
+ Coverage 88.87% 89.02% +0.15%
==========================================
Files 307 306 -1
Lines 5285 5168 -117
Branches 1344 1280 -64
==========================================
- Hits 4697 4601 -96
+ Misses 572 551 -21
Partials 16 16
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@arbrandes as requested this is a follow up to remove unused code as part of https://github.com/openedx/platform-roadmap/issues/332.
I'll leave timing of merging this up to Axim since this has already been removed from edx.org
@arbrandes is there still desire to have this removed as part of the 2U work? I'm clearing out this project on our end and this is the only open task.
@Zacharis278 @arbrandes I would like to get this merged. Is there anything blocking merging it? An open question about LockPaywall ?
@jristau1984 Is there someone from TNL who can advise on getting this PR finished? I'd like to get this unused code removed, and I think it just needs a rebase. I'm happy to help but don't have as much context on what's still needed or not.
CC @arbrandes
Thanks for the pull request, @zacharis278!
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.
:radio_button: 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.
:radio_button: 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
:radio_button: Submit a signed contributor agreement (CLA)
:warning: We ask all contributors to the Open edX project to submit a signed contributor agreement or indicate their institutional affiliation. Please see the CONTRIBUTING file for more information.
If you've signed an agreement in the past, you may need to re-sign. See The New Home of the Open edX Codebase for details.
Once you've signed the CLA, please allow 1 business day for it to be processed.
After this time, you can re-run the CLA check by adding a comment below that you have signed it.
If the CLA check continues to fail, you can tag the @openedx/cla-problems team in a comment for further assistance.
:radio_button: 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.
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
:bulb: As a result it may take up to several weeks or months to complete a review and merge your PR.
Hey @zacharis278, the CLA check for this PR is failing so it looks like you'll need to go back and follow the instructions from the bot about submitting a signed CLA.
@itsjeyd thanks I'm no longer with 2U or contributing to the project. This kinda just got left over, happy to close the PR if the branch isn't useful anymore otherwise someone else will need to pick this up.
Ported over to https://github.com/openedx/frontend-app-learning/pull/1675.
