feat: Created the plugin slot for UpgradeToCompleteAlert#1909
Conversation
|
@michaelroytman do you have permissions to run the workflows? I do not. Thanks for the review! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1909 +/- ##
=======================================
Coverage 91.29% 91.30%
=======================================
Files 343 344 +1
Lines 5770 5774 +4
Branches 1388 1351 -37
=======================================
+ Hits 5268 5272 +4
Misses 483 483
Partials 19 19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
arbrandes
left a comment
arbrandes
left a comment
There was a problem hiding this comment.
Thanks for this! Took a first pass, asked for a couple of changes.
| }: BannerDatesUpgradeSlotProps) => ( | ||
| <PluginSlot | ||
| id="org.openedx.frontend.learning.banner_dates_upgrade.v1" | ||
| idAliases={['dates_upgrade_banner_slot']} |
There was a problem hiding this comment.
Aliases exist to support a deprecation window for slots that are being renamed. Since this is a new slot, it shouldn't have an alias.
|
|
||
| const config = { | ||
| pluginSlots: { | ||
| dates_upgrade_banner_slot: { |
There was a problem hiding this comment.
Let's use the full id, here.
There was a problem hiding this comment.
Fixed Thanks!
| { | ||
| op: PLUGIN_OPERATIONS.Insert, | ||
| widget: { | ||
| id: 'org.openedx.frontend.learning.banner_dates_upgrade.v1', |
There was a problem hiding this comment.
This is the widget id: it's arbitrary, and ideally it'll be different from the slot id where it's used.
There was a problem hiding this comment.
Fixed Thanks!
mshet-sonata-pixel
force-pushed
the
SUBS-173-pr47
branch
from
cdad964 to
6929dc0
Compare
arbrandes
left a comment
arbrandes
left a comment
There was a problem hiding this comment.
Thank you for addressing my comments.
Now we have to fix the coverage regression. There are two ways to do that: either you remove the default content in the slot (not just from the slot, but from the codebase), or you restore some of the removed tests (such as the deleted analytics test (DatesTab.test.jsx:257-281).
mshet-sonata-pixel
force-pushed
the
SUBS-173-pr47
branch
from
6929dc0 to
8e952f9
Compare
4 participants
Creating a PluginSlot UpgradeToCompleteAlert at the location of the - https://github.com/openedx/frontend-app-learning/blob/master/src/course-home/suggested-schedule-messaging/UpgradeToCompleteAlert.jsx , The PluginSlot should render nothing by default. We will be injecting the Alert as a plugin instead.
This will need to be reviewed by 2U owning teams as well as the open source community. It should get support from open source.
The below is the Screeshot-
