Skip to content

Create explicit Locations. #1938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cahrens merged 1 commit into from
Dec 13, 2013
Merged

Create explicit Locations. #1938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/mongo/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ def _get_course_for_item(self, location, depth=0):
# @hack! We need to find the course location however, we don't
# know the 'name' parameter in this context, so we have
# to assume there's only one item in this query even though we are not specifying a name
course_search_location = ['i4x', location.org, location.course, 'course', None]
course_search_location = Location('i4x', location.org, location.course, 'course', None)
courses = self.get_items(course_search_location, depth=depth)

# make sure we found exactly one match on this above course search
Expand Down
6 changes: 3 additions & 3 deletions common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,15 @@ def test_get_instance(self):
)

def test_get_items(self):
modules = self.store.get_items(['i4x', None, None, 'course', None], IMPORT_COURSEID)
modules = self.store.get_items(Location('i4x', None, None, 'course', None), IMPORT_COURSEID)

@dmitchell dmitchell Dec 13, 2013

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.

Because there's an index on '_id.category' but not the combo of it w/ tag, it's better to do these as
Location(None, None, None, 'course', None), but that's arguably out of scope for this PR.

@cahrens cahrens Dec 13, 2013

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.

Since it is a test, I don't think performance is a concern.

assert_equals(len(modules), 1)
assert_equals(modules[0].location.course, self.import_course)

modules = self.store.get_items(['i4x', None, None, 'course', None], XML_COURSEID1)
modules = self.store.get_items(Location('i4x', None, None, 'course', None), XML_COURSEID1)
assert_equals(len(modules), 1)
assert_equals(modules[0].location.course, 'toy')

modules = self.store.get_items(['i4x', None, None, 'course', None], XML_COURSEID2)
modules = self.store.get_items(Location('i4x', None, None, 'course', None), XML_COURSEID2)
assert_equals(len(modules), 1)
assert_equals(modules[0].location.course, 'simple')

Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/tests/test_module_render.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_module_render_with_jump_to_id(self):
module = render.get_module(
self.mock_user,
mock_request,
['i4x', 'edX', 'toy', 'html', 'toyjumpto'],
Location('i4x', 'edX', 'toy', 'html', 'toyjumpto'),
field_data_cache,
self.course_id
)
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def jump_to_id(request, course_id, module_id):
course_location = CourseDescriptor.id_to_location(course_id)

items = modulestore().get_items(
['i4x', course_location.org, course_location.course, None, module_id],
Location('i4x', course_location.org, course_location.course, None, module_id),
course_id=course_id
)

Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/django_comment_client/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import pystache_custom as pystache

from xmodule.modulestore.django import modulestore
from xmodule.modulestore import Location
from django.utils.timezone import UTC

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -56,7 +57,7 @@ def has_forum_access(uname, course_id, rolename):

def _get_discussion_modules(course):
all_modules = modulestore().get_items(
['i4x', course.location.org, course.location.course, 'discussion', None],
Location('i4x', course.location.org, course.location.course, 'discussion', None),
course_id=course.id
)

Expand Down
5 changes: 3 additions & 2 deletions lms/djangoapps/open_ended_grading/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

from xmodule.modulestore.django import modulestore
from xmodule.modulestore import search
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import NoPathToItem

from django.http import HttpResponse, Http404, HttpResponseRedirect
from edxmako.shortcuts import render_to_string
Expand Down Expand Up @@ -92,7 +93,7 @@ def find_peer_grading_module(course):
# Get the course id and split it.
course_id_parts = course.id.split("/")
# Get the peer grading modules currently in the course. Explicitly specify the course id to avoid issues with different runs.
items = modulestore().get_items(['i4x', course_id_parts[0], course_id_parts[1], 'peergrading', None],
items = modulestore().get_items(Location('i4x', course_id_parts[0], course_id_parts[1], 'peergrading', None),
course_id=course.id)
#See if any of the modules are centralized modules (ie display info from multiple problems)
items = [i for i in items if not getattr(i, "use_for_single_location", True)]
Expand Down