course export with unpublished transcript fix #255
Conversation
edxval/tests/test_api.py
Outdated
| """ | ||
| language_code = 'en' | ||
| video_id = constants.VIDEO_DICT_FISH['edx_video_id'] | ||
| transcript_file_name = u'super-soaker-en.srt' |
| ) | ||
|
|
||
| delegate_fs = OSFS(self.temp_dir) | ||
| file_system = delegate_fs.makedir(constants.EXPORT_IMPORT_COURSE_DIR, recreate=True) |
There was a problem hiding this comment.
why explicit create here?
There was a problem hiding this comment.
it is a temporary course directory that is always made before exporting the transcript.
EXPORT_IMPORT_COURSE_DIR is an OSFS object having a directory something like /tmp/tmpbo814ql3 and in this directory, create_transcript_file will make a static_dir in case of an unpublished transcript.
(static_dir is course/static in this case)
edxval/tests/test_api.py
Outdated
|
|
||
| delegate_fs = OSFS(self.temp_dir) | ||
| file_system = delegate_fs.makedir(constants.EXPORT_IMPORT_COURSE_DIR, recreate=True) | ||
| # static_dir isn't made when the transcript is not published |
There was a problem hiding this comment.
Instead of inline comments, explain the scenario in unit test docstring
There was a problem hiding this comment.
I'll move it into the docstring.
edxval/tests/test_api.py
Outdated
| # Verify transcript file is created. | ||
| self.assertIn(transcript_file_name, file_system.listdir(constants.EXPORT_IMPORT_STATIC_DIR)) | ||
|
|
||
| # Also verify the content of created transcript file. | ||
| expected_transcript_content = File(open(expected_transcript_path, 'rb')).read() |
There was a problem hiding this comment.
remove internal comments
There was a problem hiding this comment.
internal comments have been removed.
edxval/tests/test_api.py
Outdated
| In a course with unpublished transcript, static_dir directory isn't made | ||
| so we don't explicitly make static_dir in this test | ||
| create_transcript_file will make static_dir inside file_system |
There was a problem hiding this comment.
don't mention exact function names here. Just give context that directory will be created if not present etc.
There was a problem hiding this comment.
added an improved docstring.
|
Please update the version in setup.py before merging. |
Ali-D-Akbar
force-pushed
the
aakbar/PROD-1128
branch
from
8531316
to
c3b1425
Compare
Ali-D-Akbar
force-pushed
the
aakbar/PROD-1128
branch
from
2446d4f
to
841a4a7
Compare
PROD-1128
Description
Exporting a course while having a video transcript that is yet to be published causes the export to fail. Exporting works fine in case of a published transcript file.
Fix:
The lack of directories in the file system causes this issue to occur. There is a static subdirectory that needs to be made before creating the transcript file. So if the directory doesn't exist, it must be made in the File System.
Reviewers
Post Review