Add extensible course view types for edX platform#8015
Conversation
4194ec0 to
c8d3767
Compare
|
Note: this work has been separated out from https://github.com/edx/edx-platform/pull/7937 where it was originally implemented. |
|
@cahrens @cpennington please review this newly separated out PR to provide extensible tabs. FYI: @antoviaque @bradenmacdonald @ormsbee @dan-f @dianakhuang |
|
@nasthagiri We should chat sometime soon about how this extension point (and future ones) can play nicely with mobile. |
c8d3767 to
b76b138
Compare
|
FYI, the build failure is now down to quality issues. I plan on addressing these later today. |
f9eece7 to
92b1f42
Compare
There was a problem hiding this comment.
We already talked about this, but at least for this feature, it doesn't seem necessary to have a separate class for this.
There was a problem hiding this comment.
As we discussed, it seems simpler to have the plugin's define their own name field, rather than have this complicated logic to wrap each plugin. Unfortunately there's an instanceof check that I need to remove, but that should be fine.
|
@dianakhuang @cpennington I've generalized the logic for how Studio dynamically adds tabs when features are enabled. This should now work for the "Teams" tab which needs to enable itself based upon something other than the existence of a boolean feature flag. |
|
test this please |
There was a problem hiding this comment.
What API does a Feature have to implement?
There was a problem hiding this comment.
I imagine that Feature is going to get fleshed out a bit more later when we start implementing teams using it. I can take it out of this PR if it's too distracting.
There was a problem hiding this comment.
This was my mistake. I intended to remove it when I implemented tabs as a first-class extension point. @dianakhuang please go ahead and take it out.
|
After doing a quick spot-check on devstack, the Instructor tab is not showing up in the LMS. I'm going to investigate. |
|
Huh, maybe I didn't reinstall/run setup.py or something like that, but it seems to be working fine right now. |
f31ebca to
be4dbc9
Compare
cms/djangoapps/contentstore/utils.py
Outdated
There was a problem hiding this comment.
{p['type]': p for p in [OPEN_ENDED_PANEL, NOTES_PANEL]}
|
Done w/ my review pass. |
|
Thanks @cpennington ! I'll get on fixing these. |
There was a problem hiding this comment.
Nit: it looks like there's a lot of duplicated code in PUT vs POST. Is it worth abstracting, perhaps by using a class-based view?
|
Done with this review pass. |
8438288 to
4ec6ecd
Compare
|
FYI @cpennington I had a commit specifically for addressing your feedback, if you wanted to review that before giving us a thumbs up on this PR. |
|
👍 |
4ec6ecd to
77e560a
Compare
77e560a to
dae137f
Compare
|
The one failing test is flaky. On rerun, it passed: https://build.testeng.edx.org/job/edx-platform-specific-tests/84/ |
Add extensible course view types for edX platform
This is the first step to supporting pluggable features in the edX platform. A new Stevedore "course_view_type" extension point is added, that allows any Django app to register a new view for the course. Currently, each course view is shown as a tab in the course, but a client is free to choose whether these are shown as tabs. In particular, on mobile there UX may not be represented as tabs.
To prove out this change, the following tabs have been converted to course views:
See this wiki page for the background:
https://openedx.atlassian.net/wiki/display/AC/Feature+Plug-Ins+for+edX+Platform