Skip to content

preview_handler changes to support Locators - #2434

Merged
cahrens merged 2 commits into
masterfrom
christina/preview_handler
Feb 5, 2014
Merged

preview_handler changes to support Locators#2434
cahrens merged 2 commits into
masterfrom
christina/preview_handler

Conversation

@cahrens

@cahrens cahrens commented Feb 3, 2014

Copy link
Copy Markdown

@cpennington and @dmitchell Please review.

This is STUD-967, changing the code so that it will be able to handle descriptor.location being a Locator instead of a Location.

The changes ended up being minimal-- it was mostly just thinking through whether or not the usages of descriptor.location will be OK for Locators. I added some comments in the code for particular usages that were not obvious (to me).

@cpennington I am including you because you wrote the partial(replace_static_urls... code that I'm changing. I don't see why the arguments for that should be different from the arguments to replace_static_urls right above. data_dir is not needed for courses not being served out of xml_modulestore, and we already have a course_id defined.

@dmitchell

Copy link
Copy Markdown
Contributor

👍 after rebasing and ensuring tests pass

@cahrens

cahrens commented Feb 5, 2014

Copy link
Copy Markdown
Author

@dmitchell and @cpennington PR is rebased, tests passing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure getting rid of data_dir is the right thing to do?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is passed for data_directory in the call above. I looked at the code that is being called, and it seems that data_directory is only used for XML modulestore (which Studio doesn't use). The code is in /edx-platform/common/djangoapps/static_replace/init.py

I couldn't think of any reason why the arguments should be different in the 2 places. Do you know of a reason why they should differ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I think that should be fine in this code. Another good reason that we should clean up the api for the ContentStore and static replacement, so that we don't have to have all of these special cases depending on whether we're in an XML world or a mongo one.

@cpennington

Copy link
Copy Markdown
Contributor

👍 once my comments are addressed.

cahrens pushed a commit that referenced this pull request Feb 5, 2014
preview_handler changes to support Locators
@cahrens
cahrens merged commit 235d10d into master Feb 5, 2014
@cahrens
cahrens deleted the christina/preview_handler branch February 5, 2014 17:53
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Mar 23, 2018
* Add ORA2 video upload option openedx#2375 (openedx#2417)

* Modified courseware page look and feel openedx#2377 (openedx#2409)

* Modified about page openedx#2379 (openedx#2413)

* Fix biz bugs. openedx#2404 (openedx#2406)

* Fix display width of popup. (openedx#2384)

(cherry picked from commit a43c935a575d15fcf629f0edeb81446c778cda95)

* Validate duplicate url-code.

* Fix bug, when course not found.

* Fix order of course as CourseOverview.

* Remove additional-info count from contract grid. openedx#2419 (openedx#2437)

* Fix order global course. openedx#2420 (openedx#2421)

* Add additional info register. openedx#2419 (openedx#2433)

* fix survey csv character encode problem openedx#2380 (openedx#2434)

* Fix bokchoy for LoginCodeEnabledBizSurveyTest. (openedx#2457)

* Fix register students confirm message. (openedx#2461)

* Add command to check playback_log. openedx#2438 (openedx#2445)

* Fix courseware page lookandfeel (openedx#2446, openedx#2439, openedx#2452, openedx#2453)

* Fix box-shadow of sequence-nav-button. openedx#2453 (openedx#2467)

* Fix bugs. openedx#2462 openedx#2463 (openedx#2464)

* Fix password message in register students page.

* Fix glass pane of processing when register additional item.

* Fix display width of popup. (openedx#2466)

* Fix min-width of sequence-nav. openedx#2468 (openedx#2469)

* Fix isRegistered javascript in about page openedx#2470 (openedx#2471)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants