openedx
[OldMongo FC-0004] Remove support for children in Old Mongo
Description
First part with the straightforward, non-controversial tests: https://github.com/openedx/edx-platform/pull/31224 Removed support for children in Old Mongo Updated related tests
Useful information to include: https://github.com/openedx/public-engineering/issues/80
Thanks for the pull request, @UvgenGen! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
- supporting documentation
- Open edX discussion forum threads
- timeline information ("this must be merged by XX date", and why that is)
- partner information ("this is a course on edx.org")
- any other information that can help Product understand the context for the PR
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
Please let us know once your PR is ready for our review and all tests are green.
@ormsbee I've opened a separate pull request with the straightforward tests:
https://github.com/openedx/edx-platform/pull/31224
This pr includes some of tests that have been commented on about ddt, it's fixed.
@UvgenGen - hi there! Just following-up on this - looks like Dave requested changes, so I'm just following up :)
@mphilbrick211 Hi! I'll rebased it after merging this PR https://github.com/openedx/edx-platform/pull/31480
Thanks @UvgenGen! @ormsbee flagging this for you, though I know you are traveling this week.
Thanks @UvgenGen! @ormsbee flagging this for you, though I know you are traveling this week.
Friendly ping on this @ormsbee :)
@mphilbrick211: Thank you for the reminder. FWIW, I did start re-reading through this today, but it's taking me a while. Aiming to have a review submitted by end of day Wed.
@UvgenGen: Is this ready for another review? (I just want to make sure you're not waiting on me.)
@ormsbee I addressed all comments except last one. I left a question to your comment:
Can we mock the data at the Mongo layer–i.e. exercise Old Mongo read logic on the root CourseBlock by mocking what it gets back from the database? CourseOverview compatibility is one of the things that has to keep working, or it will cause operational errors the next time someone adds something to CourseOverviews, which may be months from now.
Q:
We investigate possibility to mock draft course data, but in witch case CourseOverview can be regenerated if we removed all create/update methods for draft modulestore?
PR updated except this issue. I will wait for an answer, thanks)
We investigate possibility to mock draft course data, but in witch case CourseOverview can be regenerated if we removed all create/update methods for draft modulestore?
I'm not entirely sure if I understand this question, so I'll reiterate my priorities:
- It is required that we still be able to regenerate CourseOverviews from Old Mongo courses, which requires being able to read the root CourseBlock from those courses. Breaking this could potentially break a lot of other functionality that we don't want to deal with yet.
- If we can ensure that CourseOverviews will continue to work by mocking the data that comes back from MongoDB in situations where Old Mongo CourseOverviews are generated, then let's do that. If this works, we can remove the create/update methods on the old modulestore.
Does that make sense?
@ormsbee we are not found any test that regenerate course overview, so we planning add a new one. If it's possible to regenerate course overview we will follow you advice.
@ormsbee we added new course overview test for Old Mongo:
openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py::CourseOverviewTestCase::test_mongo_course_overview_generation
Please take a look)
Hi @UvgenGen @ormsbee! Looks like tests need to be run on this PR. Is there an update on getting this merged?
@mphilbrick211: Just wanted to merge the last of the BD-13 PRs before this one.
@kaustavb12, @Agrendalath: @glugovgrglib mentioned that new failures occurred on this PR after https://github.com/openedx/edx-platform/pull/31472 was merged. Do you have any insight on this?
@jristau1984: Assuming that the above issue gets settled, would it be okay to merge this on Monday?
@ormsbee, https://github.com/openedx/edx-platform/pull/31472 should not cause SyntaxError: invalid syntax errors. I don't have any context on this PR, but it seems that a force push + 581f07c47d0644b446c3e7751b7100992fbafb2c resolved these failures?
@Agrendalath: I think it's more a question of whether that https://github.com/openedx/edx-platform/commit/581f07c47d0644b446c3e7751b7100992fbafb2c should have been necessary–i.e. why did the calls to block.render[STUDENT_VIEW] break after #31472 merged?
@ormsbee, I ran lms/djangoapps/courseware/tests/test_video_mongo.py::TestVideoNonYouTube::test_video_constructor and checked the generated context. It looks like this.
master branch before https://github.com/openedx/edx-platform/pull/31472:
(('video.html',
{'autoadvance_enabled': False,
'branding_info': None,
'bumper_metadata': 'null',
'cdn_eval': False,
'cdn_exp_group': None,
'display_name': 'A Name',
'download_video_link': 'example.mp4',
'handout': None,
'hide_downloads': False,
'id': 'i4x-org_0-course_0-video-A_Name',
'is_embed': False,
'license': None,
'metadata': '{"autoAdvance": false, "autohideHtml5": false, "autoplay": '
'false, "captionDataDir": null, "completionEnabled": false, '
'"completionPercentage": 0.95, "duration": null, "end": 3610.0, '
'"generalSpeed": 1.0, "lmsRootURL": "http://localhost:8000", '
'"poster": null, "prioritizeHls": false, '
'"publishCompletionUrl": '
'"i4x://org.0/course_0/video/A_Name/publish_completion", '
'"recordedYoutubeIsAvailable": true, "savedVideoPosition": 0.0, '
'"saveStateEnabled": true, "saveStateUrl": '
'"i4x://org.0/course_0/video/A_Name/xmodule_handler/save_user_state", '
'"showCaptions": "true", "sources": ["example.mp4", '
'"example.webm"], "speed": null, "start": 3603.0, "streams": '
'"1.00:3_yD_cEKoCk", "transcriptAvailableTranslationsUrl": '
'"i4x://org.0/course_0/video/A_Name/transcriptavailable_translations", '
'"transcriptLanguage": "en", "transcriptLanguages": {"en": '
'"English"}, "transcriptTranslationUrl": '
'"i4x://org.0/course_0/video/A_Name/transcripttranslation/__lang__", '
'"ytApiUrl": "https://www.youtube.com/iframe_api", '
'"ytMetadataEndpoint": "", "ytTestTimeout": 1500}',
'poster': 'null',
'public_video_url': None,
'track': None,
'transcript_download_format': 'srt',
'transcript_download_formats_list': [{'display_name': 'SubRip (.srt) file',
'value': 'srt'},
{'display_name': 'Text (.txt) file',
'value': 'txt'}]}),
{})
master branch:
(('video.html',
{'autoadvance_enabled': False,
'branding_info': None,
'bumper_metadata': 'null',
'cdn_eval': False,
'cdn_exp_group': None,
'display_name': 'A Name',
'download_video_link': 'example.mp4',
'handout': None,
'hide_downloads': False,
'id': 'i4x-org_0-course_0-video-A_Name',
'is_embed': False,
'license': None,
'metadata': '{"autoAdvance": false, "autohideHtml5": false, "autoplay": '
'false, "captionDataDir": null, "completionEnabled": false, '
'"completionPercentage": 0.95, "duration": null, "end": 3610.0, '
'"generalSpeed": 1.0, "lmsRootURL": "http://localhost:8000", '
'"poster": null, "prioritizeHls": false, '
'"publishCompletionUrl": '
'"/courses/org.0/course_0/Run_0/xblock/i4x:;_;_org.0;_course_0;_video;_A_Name/handler/publish_completion", '
'"recordedYoutubeIsAvailable": true, "savedVideoPosition": 0.0, '
'"saveStateEnabled": true, "saveStateUrl": '
'"/courses/org.0/course_0/Run_0/xblock/i4x:;_;_org.0;_course_0;_video;_A_Name/handler/xmodule_handler/save_user_state", '
'"showCaptions": "true", "sources": ["example.mp4", '
'"example.webm"], "speed": null, "start": 3603.0, "streams": '
'"1.00:3_yD_cEKoCk", "transcriptAvailableTranslationsUrl": '
'"/courses/org.0/course_0/Run_0/xblock/i4x:;_;_org.0;_course_0;_video;_A_Name/handler/transcript/available_translations", '
'"transcriptLanguage": "en", "transcriptLanguages": {"en": '
'"English"}, "transcriptTranslationUrl": '
'"/courses/org.0/course_0/Run_0/xblock/i4x:;_;_org.0;_course_0;_video;_A_Name/handler/transcript/translation/__lang__", '
'"ytApiUrl": "https://www.youtube.com/iframe_api", '
'"ytMetadataEndpoint": "", "ytTestTimeout": 1500}',
'poster': 'null',
'public_video_url': None,
'track': None,
'transcript_download_format': 'srt',
'transcript_download_formats_list': [{'display_name': 'SubRip (.srt) file',
'value': 'srt'},
{'display_name': 'Text (.txt) file',
'value': 'txt'}]}),
{})
Current version (from this PR):
(('video.html',
{'autoadvance_enabled': False,
'branding_info': None,
'bumper_metadata': 'null',
'cdn_eval': False,
'cdn_exp_group': None,
'display_name': 'A Name',
'download_video_link': 'example.mp4',
'handout': None,
'hide_downloads': False,
'id': 'A_Name',
'is_embed': False,
'license': None,
'metadata': '{"autoAdvance": false, "autohideHtml5": false, "autoplay": '
'false, "captionDataDir": null, "completionEnabled": false, '
'"completionPercentage": 0.95, "duration": null, "end": 3610.0, '
'"generalSpeed": 1.0, "lmsRootURL": "http://localhost:8000", '
'"poster": null, "prioritizeHls": false, '
'"publishCompletionUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/publish_completion", '
'"recordedYoutubeIsAvailable": true, "savedVideoPosition": 0.0, '
'"saveStateEnabled": true, "saveStateUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/xmodule_handler/save_user_state", '
'"showCaptions": "true", "sources": ["example.mp4", '
'"example.webm"], "speed": null, "start": 3603.0, "streams": '
'"1.00:3_yD_cEKoCk", "transcriptAvailableTranslationsUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/transcript/available_translations", '
'"transcriptLanguage": "en", "transcriptLanguages": {"en": '
'"English"}, "transcriptTranslationUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/transcript/translation/__lang__", '
'"ytApiUrl": "https://www.youtube.com/iframe_api", '
'"ytMetadataEndpoint": "", "ytTestTimeout": 1500}',
'poster': 'null',
'public_video_url': None,
'track': None,
'transcript_download_format': 'srt',
'transcript_download_formats_list': [{'display_name': 'SubRip (.srt) file',
'value': 'srt'},
{'display_name': 'Text (.txt) file',
'value': 'txt'}]}),
{})
Version from before https://github.com/openedx/edx-platform/commit/581f07c47d0644b446c3e7751b7100992fbafb2c:
(('video.html',
{'autoadvance_enabled': False,
'branding_info': None,
'bumper_metadata': 'null',
'cdn_eval': False,
'cdn_exp_group': None,
'display_name': 'A Name',
'download_video_link': 'example.mp4',
'handout': None,
'hide_downloads': False,
'id': 'A_Name',
'is_embed': False,
'license': None,
'metadata': '{"autoAdvance": false, "autohideHtml5": false, "autoplay": '
'false, "captionDataDir": null, "completionEnabled": false, '
'"completionPercentage": 0.95, "duration": null, "end": 3610.0, '
'"generalSpeed": 1.0, "lmsRootURL": "http://localhost:8000", '
'"poster": null, "prioritizeHls": false, '
'"publishCompletionUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/publish_completion", '
'"recordedYoutubeIsAvailable": true, "savedVideoPosition": 0.0, '
'"saveStateEnabled": true, "saveStateUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/xmodule_handler/save_user_state", '
'"showCaptions": "true", "sources": ["example.mp4", '
'"example.webm"], "speed": null, "start": 3603.0, "streams": '
'"1.00:3_yD_cEKoCk", "transcriptAvailableTranslationsUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/transcript/available_translations", '
'"transcriptLanguage": "en", "transcriptLanguages": {"en": '
'"English"}, "transcriptTranslationUrl": '
'"/courses/course-v1:org.0+course_0+Run_0/xblock/block-v1:org.0+course_0+Run_0+type@video+block@A_Name/handler/transcript/translation/__lang__", '
'"ytApiUrl": "https://www.youtube.com/iframe_api", '
'"ytMetadataEndpoint": "", "ytTestTimeout": 1500}',
'poster': 'null',
'public_video_url': None,
'track': None,
'transcript_download_format': 'srt',
'transcript_download_formats_list': [{'display_name': 'SubRip (.srt) file',
'value': 'srt'},
{'display_name': 'Text (.txt) file',
'value': 'txt'}]}),
{})
<div class="aside-block"
data-success-class="fa fa-check-square-o fa-lg pass"
data-failure-class="fa fa-times fa-lg fail"
data-error-class="fa fa-exclamation-triangle fa-lg error"
data-unknown-class="fa fa-question-circle fa-lg unknown"
data-local-resource-url="/static/xblock/resources/acid/public/test_data.json"
>
<button>Acid Aside for block-v1:org.0+course_0+Run_0+type@video+block@A_Name</button>
<div>
<p>JS init function run:
<span class="js-init-run">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</p>
<p>Resource Url Test:
<span class="local-resource-test">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</p>
<table class='storage-tests'>
<tr>
<th>Scope</th>
<th>Server-side<br>handler_url<br>returned</th>
<th>Server-side<br>handler_url<br>succeeded</th>
<th>Client-side<br>handler_url<br>returned</th>
<th>Client-side<br>handler_url<br>succeeded</th>
</tr>
<tr class="scope-storage-test scope-user_state "
data-handler-url="/courses/course-v1:org.0+course_0+Run_0/xblock/aside-usage-v2:block-v1$:org.0+course_0+Run_0+type@video+block@A_Name::acid_aside/handler/check_storage/SUFFIX1465?QUERY=1465&SCOPE=user_state"
data-scope="user_state"
data-value="1465"
>
<td>user_state</td>
<td>
<span class="server-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="server-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
</tr>
<tr class="scope-storage-test scope-preferences alt"
data-handler-url="/courses/course-v1:org.0+course_0+Run_0/xblock/aside-usage-v2:block-v1$:org.0+course_0+Run_0+type@video+block@A_Name::acid_aside/handler/check_storage/SUFFIX7096?QUERY=7096&SCOPE=preferences"
data-scope="preferences"
data-value="7096"
>
<td>preferences</td>
<td>
<span class="server-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="server-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
</tr>
<tr class="scope-storage-test scope-user_info "
data-handler-url="/courses/course-v1:org.0+course_0+Run_0/xblock/aside-usage-v2:block-v1$:org.0+course_0+Run_0+type@video+block@A_Name::acid_aside/handler/check_storage/SUFFIX4376?QUERY=4376&SCOPE=user_info"
data-scope="user_info"
data-value="4376"
>
<td>user_info</td>
<td>
<span class="server-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="server-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
</tr>
<tr class="scope-storage-test scope-user_state_summary alt"
data-handler-url="/courses/course-v1:org.0+course_0+Run_0/xblock/aside-usage-v2:block-v1$:org.0+course_0+Run_0+type@video+block@A_Name::acid_aside/handler/check_storage/SUFFIX2238?QUERY=2238&SCOPE=user_state_summary"
data-scope="user_state_summary"
data-value="2238"
>
<td>user_state_summary</td>
<td>
<span class="server-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="server-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-returned">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
<td>
<span class="client-storage-test-succeeded">
<i class="fa fa-question-circle fa-lg unknown"></i>
</span>
</td>
</tr>
</table>
<p>Sees Acid DOM: <span class="acid-dom"><i class="fa fa-question-circle fa-lg unknown"></i></span></p>
</div>
</div>
It seems that https://github.com/openedx/edx-platform/pull/31472 changed the URLs in this context, but it didn't trigger any test failures. Therefore, the behavior change of self.block.render(STUDENT_VIEW) might be caused by this PR (https://github.com/openedx/edx-platform/pull/31134).
@ormsbee there are several production issues in-flight right now, I will have to wait to merge this until the impact from those issues dies down a bit.
@ormsbee PR is rebased but I have an error with JS tests. Looks like this pipeline needs to be restarted
@UvgenGen 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.
2U Release Notice: This PR has been deployed to the edX production environment.