mitodl / edx-platform

The Open edX platform, the software that powers edX!
http://open.edx.org/
GNU Affero General Public License v3.0
6 stars 1 forks source link

Add annotations for CourseYoutubeBlockedFlag #270

Open pdpinch opened 3 years ago

pdpinch commented 3 years ago

The CourseYoutubeBlockedFlag video config model isn't documented. We're using it on mitxonline, so it would be nice to capture how it's used.

https://github.com/edx/edx-platform/blob/8aedebcdb29bb16b94786503c12a52b07c73dff5/openedx/core/djangoapps/video_config/models.py#L96-L102

asadiqbal08 commented 3 years ago

Is CourseYoutubeBlockedFlag currently using in mitxonline ? Do we need to document it somewhere in this story ? any link where need to do such ?

pdpinch commented 3 years ago

When we exported courses from edx.org and imported them into mitxonline, we found that the videos played from YouTube on mitxonline, instead of from cloudfront, as they do on edx.org. After a lot of experimentation, we hit on setting CourseYoutubeBlockedFlag for each course so that the YouTube videos wouldn't play, and users were presented with cloudfront.

See, for example, https://courses-qa.mitxonline.mit.edu/admin/video_config/courseyoutubeblockedflag/

The setting isn't documented, and I'm not really sure what it was designed for. I'm hoping that by doing some research in the codebase, and maybe talking to some folks at edX, we can find out and get it documented. I'm not even sure it's the best way to achieve our goals. It is annoying that we need to create a waffle flag for each course run.

asadiqbal08 commented 3 years ago

@pdpinch that is the particular PR basically introduced this change set. The description over this PR pointing these settings are specific to courses (that really annoying to set this flag for every courses). Other side, I found very limited use of this feature only under this block only

Here two things comes in my mind:

1- Should we only need to document this behavior at some specific file/location for our reference ? OR 2- Are we thinking to extend this feature set by introducing a generic environment / setting toggle variable that not be specific to any course (along with this implementation) ?

pdpinch commented 3 years ago

Thanks for finding the PR. I'm a little surprised to see that this flag was added exactly for the reason we're using it: courses that have lost access to their YouTube channels.

I'm sorry I wasn't clear in the issue description. What I'm really looking for is to have the code annotated the way waffle flags are supposed to be annotated (OEP-17), so the documentation shows up in https://edx.readthedocs.io/projects/edx-platform-technical/en/latest/index.html

asadiqbal08 commented 3 years ago

@pdpinch , I get your point but the issue is that, The way, CourseYoutubeBlockedFlag, is implemented in edx platform is bit different than waffle flags annotations. Actually, they separately defined the configurations models for them please see this file that is different from this waffle annotation logic for course level.

I am not sure why it was required earlier to define these models instead of waffle annotation.

Can you redirect me to edx channel where I can ask this question and direction where we can document such implemented stuff ?

pdpinch commented 3 years ago

I would suggest posting to https://discuss.openedx.org/c/development/11 because it seems to get more attention than Slack. On Slack, you might consider #documentation or #video.

I think Regis Behmo and Rob Raposa are the experts on annotations. I don't remember who is expert on video.

asadiqbal08 commented 3 years ago

ref: https://discuss.openedx.org/t/missing-annotations-for-courseyoutubeblockedflag/5952

asadiqbal08 commented 3 years ago

@pdpinch I am thinking to get follow up on early basis over the slack channels as you mentioned but It is requiring me to use edx credentials ( that I may not have), Is that any alternative ?

pdpinch commented 2 years ago

I believe anyone can create an account on openedx.slack.com. It shouldn't require edx credentials. There should be a link somewhere on openedx.org to create an account.

shaidar commented 2 years ago

@pdpinch Does anyone need to ask edx again about this or should we close the issue?