Merge pull request #20040 from cpennington/revert-course-list

Revert course list
This commit is contained in:
Calen Pennington
2019-03-22 10:05:31 -04:00
committed by GitHub
10 changed files with 28 additions and 124 deletions

View File

@@ -121,14 +121,7 @@ class Role(models.Model):
"""
Returns True if the user has one of the given roles for the given course
"""
if 'roles' in getattr(user, '_prefetched_objects_cache', {}):
# Don't blow up the prefetch cache
return any(
role.course_id == course_id and role.name in role_names
for role in user.roles.all()
)
else:
return user.roles.filter(course_id=course_id, name__in=role_names).exists()
return Role.objects.filter(course_id=course_id, name__in=role_names, users=user).exists()
class Permission(models.Model):

View File

@@ -1195,19 +1195,13 @@ class CourseEnrollment(models.Model):
if user.is_anonymous:
return None
try:
if 'courseenrollment' in getattr(user, '_prefetched_objects_cache', {}):
for enrollment in user.courseenrollment_set.all():
if enrollment.course_id == course_key:
return enrollment
return None
else:
query = cls.objects
if select_related is not None:
query = query.select_related(*select_related)
return query.get(
user=user,
course_id=course_key
)
query = cls.objects
if select_related is not None:
query = query.select_related(*select_related)
return query.get(
user=user,
course_id=course_key
)
except cls.DoesNotExist:
return None

View File

@@ -1,38 +0,0 @@
SELECT `course_duration_limits_coursedurationlimitconfig`.`id`,
`course_duration_limits_coursedurationlimitconfig`.`change_date`,
`course_duration_limits_coursedurationlimitconfig`.`changed_by_id`,
`course_duration_limits_coursedurationlimitconfig`.`enabled`,
`course_duration_limits_coursedurationlimitconfig`.`site_id`,
`course_duration_limits_coursedurationlimitconfig`.`org`,
`course_duration_limits_coursedurationlimitconfig`.`course_id`,
`course_duration_limits_coursedurationlimitconfig`.`enabled_as_of`,
1 AS `is_active`
FROM `course_duration_limits_coursedurationlimitconfig`
WHERE (
`course_duration_limits_coursedurationlimitconfig`.`id` IN
(
SELECT max(u0.`id`) AS `max`
FROM `course_duration_limits_coursedurationlimitconfig` u0
GROUP BY u0.`site_id`,
u0.`org`,
u0.`course_id`
ORDER BY NULL)
AND ((
`course_duration_limits_coursedurationlimitconfig`.`course_id` IS NULL
AND `course_duration_limits_coursedurationlimitconfig`.`org` IS NULL
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` IS NULL)
OR (
`course_duration_limits_coursedurationlimitconfig`.`course_id` IS NULL
AND `course_duration_limits_coursedurationlimitconfig`.`org` IS NULL
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` = 1)
OR (
`course_duration_limits_coursedurationlimitconfig`.`course_id` IS NULL
AND `course_duration_limits_coursedurationlimitconfig`.`org` = 'edX'
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` IS NULL)
OR (
`course_duration_limits_coursedurationlimitconfig`.`course_id` = 'course-v1:UCSanDiegoX+ALGS201x+2T2017'
AND `course_duration_limits_coursedurationlimitconfig`.`org` IS NULL
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` IS NULL)))
ORDER BY IF(Isnull(`course_duration_limits_coursedurationlimitconfig`.`course_id`),0,1), `course_duration_limits_coursedurationlimitconfig`.`course_id` DESC,
IF(isnull(`course_duration_limits_coursedurationlimitconfig`.`org`),0,1), `course_duration_limits_coursedurationlimitconfig`.`org` DESC,
IF(isnull(`course_duration_limits_coursedurationlimitconfig`.`site_id`),0,1), `course_duration_limits_coursedurationlimitconfig`.`site_id` DESC

View File

@@ -277,7 +277,6 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
})
@override_settings(SEARCH_ENGINE="search.tests.mock_search_engine.MockSearchEngine")
@override_settings(COURSEWARE_INDEX_NAME=TEST_INDEX_NAME)
@ddt.ddt
class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, SearcherMixin):
"""
Tests the search functionality of the courses API.
@@ -356,8 +355,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
self.assertEqual(res.data['pagination']['count'], 3)
self.assertEqual(len(res.data['results']), 1) # Should return a single course
@ddt.data(True, False)
def test_too_many_courses(self, with_username):
def test_too_many_courses(self):
"""
Test that search results are limited to 100 courses, and that they don't
blow up the database.
@@ -389,22 +387,15 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
self.setup_user(self.audit_user)
# These query counts were found empirically
query_counts = [64, 51, 51, 51, 51, 51, 51, 51, 51, 51, 21]
with_username_adjust = [4, 4, 4, 7, 7, 7, 10, 10, 10, 13, 13]
query_counts = [63, 50, 50, 50, 50, 50, 50, 50, 50, 50, 20]
ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])])
self.clear_caches()
for page in range(1, 12):
RequestCache.clear_all_namespaces()
expected_query_count = query_counts[page - 1]
if with_username:
expected_query_count += with_username_adjust[page - 1]
with self.assertNumQueries(expected_query_count):
params = {'page': page, 'page_size': 30}
if with_username:
params['username'] = self.audit_user.username
response = self.verify_response(params=params)
with self.assertNumQueries(query_counts[page - 1]):
response = self.verify_response(params={'page': page, 'page_size': 30})
self.assertIn('results', response.data)
self.assertEqual(response.data['pagination']['count'], 303)

View File

@@ -25,10 +25,9 @@ from courseware.model_data import FieldDataCache
from courseware.module_render import get_module
from course_modes.models import CourseMode
from django.conf import settings
from django.db.models import Prefetch, prefetch_related_objects
from django.db.models import Prefetch
from django.urls import reverse
from django.http import Http404, QueryDict
from django_chunked_iterator import batch_iterator, iterator
from enrollment.api import get_course_enrollment_details
from edxmako.shortcuts import render_to_string
from fs.errors import ResourceNotFound
@@ -454,52 +453,31 @@ def get_course_syllabus_section(course, section_key):
raise KeyError("Invalid about key " + str(section_key))
def get_courses(user, org=None, filter_=None, batch_size=100):
def get_courses(user, org=None, filter_=None):
"""
Return a LazySequence of courses available, optionally filtered by org code (case-insensitive).
"""
courses_qset = branding.get_visible_courses(
courses = branding.get_visible_courses(
org=org,
filter_=filter_,
).select_related(
'image_set'
).prefetch_related(
Prefetch(
'modes',
queryset=CourseMode.objects.exclude(mode_slug__in=CourseMode.CREDIT_MODES),
to_attr='selectable_modes',
),
).select_related(
'image_set'
)
permission_name = configuration_helpers.get_value(
'COURSE_CATALOG_VISIBILITY_PERMISSION',
settings.COURSE_CATALOG_VISIBILITY_PERMISSION
)
if user.is_authenticated:
prefetch_related_objects([user], 'roles', 'experimentdata_set')
def with_prefetched_users():
for courses in batch_iterator(courses_qset, batch_size=batch_size):
prefetch_related_objects(
[user],
Prefetch(
'courseenrollment_set',
queryset=CourseEnrollment.objects.filter(
user=user,
course__in=courses
).select_related('schedule')
)
)
for course in courses:
if has_access(user, permission_name, course):
yield course
course_iterator = with_prefetched_users()
else:
course_iterator = (c for c in iterator(courses_qset) if has_access(user, permission_name, c))
return LazySequence(
course_iterator,
est_len=courses_qset.count()
(c for c in courses if has_access(user, permission_name, c)),
est_len=courses.count()
)

View File

@@ -12,24 +12,14 @@ def is_in_holdback(user):
"""
in_holdback = False
if user and user.is_authenticated:
if 'experimentdata' in getattr(user, '_prefetched_objects_cache', {}):
for experiment_data in user.experimentdata_set.all():
if (
experiment_data.experiment_id == EXPERIMENT_ID and
experiment_data.key == EXPERIMENT_DATA_HOLDBACK_KEY
):
in_holdback = experiment_data.value == 'True'
break
else:
try:
holdback_value = ExperimentData.objects.get(
user=user,
experiment_id=EXPERIMENT_ID,
key=EXPERIMENT_DATA_HOLDBACK_KEY,
).value
except ExperimentData.DoesNotExist:
pass
else:
in_holdback = holdback_value == 'True'
try:
holdback_value = ExperimentData.objects.get(
user=user,
experiment_id=EXPERIMENT_ID,
key=EXPERIMENT_DATA_HOLDBACK_KEY,
).value
in_holdback = holdback_value == 'True'
except ExperimentData.DoesNotExist:
pass
return in_holdback

View File

@@ -37,7 +37,6 @@ celery==3.1.25 # Asynchronous task execution library
defusedxml
Django==1.11.20 # Web application framework
django-babel-underscore # underscore template extractor for django-babel (internationalization utilities)
django-chunked-iterator # Allow more control over how large queries are executed
django-config-models>=0.2.2 # Configuration models for Django allowing config management with auditing
django-cors-headers==2.1.0 # Used to allow to configure CORS headers for cross-domain requests
django-countries==4.6.1 # Country data for Django forms and model fields

View File

@@ -68,7 +68,6 @@ defusedxml==0.5.0
django-appconf==1.0.3 # via django-statici18n
django-babel-underscore==0.5.2
django-babel==0.6.2 # via django-babel-underscore
django-chunked-iterator==0.6.1
django-classy-tags==0.8.0 # via django-sekizai
django-config-models==0.2.2
django-cors-headers==2.1.0

View File

@@ -89,7 +89,6 @@ distlib==0.2.8
django-appconf==1.0.3
django-babel-underscore==0.5.2
django-babel==0.6.2
django-chunked-iterator==0.6.1
django-classy-tags==0.8.0
django-config-models==0.2.2
django-cors-headers==2.1.0

View File

@@ -87,7 +87,6 @@ distlib==0.2.8 # via caniusepython3
django-appconf==1.0.3
django-babel-underscore==0.5.2
django-babel==0.6.2
django-chunked-iterator==0.6.1
django-classy-tags==0.8.0
django-config-models==0.2.2
django-cors-headers==2.1.0