Skip to content

Commit

Permalink
Merge pull request #7 from open-craft/fox/bb-7424-quick-course-lookup
Browse files Browse the repository at this point in the history
fix: Drop autocomplete of courses for performance.
  • Loading branch information
Kelketek authored May 18, 2023
2 parents d8afe77 + 2119c01 commit c407167
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 132 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ Change Log
Unreleased
**********


[0.4.0] - 2023-05-18
********************

Changed
=======

* Removed course autocomplete for performance reasons. Source courses must now be specified by course key pasted into the source course ID field.

Added
=====

Expand Down
2 changes: 1 addition & 1 deletion section_to_course/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Factors sections from Open edX courses into their own new course.
"""

__version__ = '0.3.0'
__version__ = '0.4.0'
13 changes: 0 additions & 13 deletions section_to_course/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,6 @@ def build_attrs(self, base_attrs, extra_attrs=None):
return attrs


class CourseAutocompleteSelect(ArbitraryAutocompleteSelect):
"""
Widget which will autocomplete course IDs.
"""

def get_url(self):
"""
Get the URL for the course autocomplete function.
"""
return reverse('section_to_course:course_autocomplete')


class SectionAutocompleteSelect(ArbitraryAutocompleteSelect):
"""
Widget which will autocomplete section IDs.
Expand Down Expand Up @@ -159,7 +147,6 @@ class CreateSectionToCourseLink(forms.ModelForm):

source_course_id = forms.CharField(
max_length=127,
widget=CourseAutocompleteSelect('source_course_id'),
)
source_section_id = forms.CharField(
max_length=255,
Expand Down
85 changes: 0 additions & 85 deletions section_to_course/api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,91 +19,6 @@
from xmodule.modulestore.tests.factories import ItemFactory as BlockFactory


class TestCourseAutoCompleteAPI(ModuleStoreTestCase, APITestCase):
"""
Tests for the course autocomplete API.
"""

def test_rejects_unauthenticated(self):
"""
Test that the API rejects unauthenticated users.
"""
response = self.client.get(reverse('section_to_course:course_autocomplete'))
assert response.status_code == status.HTTP_403_FORBIDDEN

def test_rejects_unauthorized(self):
"""
Test that the API rejects unauthorized users.
"""
user = UserFactory.create()
assert self.client.login(username=user.username, password='test')
response = self.client.get(reverse('section_to_course:course_autocomplete'))
assert response.status_code == status.HTTP_403_FORBIDDEN

def create_courses(self):
"""Create some courses."""
CourseFactory(display_name='Demo Course', org='edX', course='DemoX')
CourseFactory(display_name='Basic Questions', org='OpenCraft', course='Tutorials')

def test_handles_blank(self):
"""
Test that blank terms return all courses.
"""
user = UserFactory.create(is_staff=True)
assert self.client.login(username=user.username, password='test')
self.create_courses()
result_data = {
'results': [
{'id': 'course-v1:edX+DemoX+Demo_Course', 'text': 'Demo Course (course-v1:edX+DemoX+Demo_Course)'},
{
'id': 'course-v1:OpenCraft+Tutorials+Basic_Questions',
'text': 'Basic Questions (course-v1:OpenCraft+Tutorials+Basic_Questions)',
},
],
}
response = self.client.get(reverse('section_to_course:course_autocomplete'))
assert response.status_code == status.HTTP_200_OK
assert response.data == result_data
response = self.client.get(reverse('section_to_course:course_autocomplete'))
assert response.status_code == status.HTTP_200_OK
assert response.data == result_data

def test_filters_display_names(self):
"""
Test that blank terms return all courses.
"""
user = UserFactory.create(is_staff=True)
assert self.client.login(username=user.username, password='test')
self.create_courses()
result_data = {
'results': [
{'id': 'course-v1:edX+DemoX+Demo_Course', 'text': 'Demo Course (course-v1:edX+DemoX+Demo_Course)'},
],
}
response = self.client.get(f'{reverse("section_to_course:course_autocomplete")}?term=dem')
assert response.status_code == status.HTTP_200_OK
assert response.data == result_data

def test_filters_keys(self):
"""
Test that blank terms return all courses.
"""
user = UserFactory.create(is_staff=True)
assert self.client.login(username=user.username, password='test')
self.create_courses()
result_data = {
'results': [
{
'id': 'course-v1:OpenCraft+Tutorials+Basic_Questions',
'text': 'Basic Questions (course-v1:OpenCraft+Tutorials+Basic_Questions)',
},
],
}
response = self.client.get(f'{reverse("section_to_course:course_autocomplete")}?term=course-v1%3AOp')
assert response.status_code == status.HTTP_200_OK
assert response.data == result_data


def create_subsections():
"""Create some subsections."""
course = CourseFactory(display_name='Demo Course', org='edX', course='DemoX')
Expand Down
1 change: 0 additions & 1 deletion section_to_course/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

app_name = 'section_to_course'
urlpatterns = [
path('autocomplete/course/', views.CourseAutocomplete.as_view(), name='course_autocomplete'),
path(
'autocomplete/course/<str:course_id>/sections/',
views.SectionAutocomplete.as_view(),
Expand Down
28 changes: 1 addition & 27 deletions section_to_course/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,10 @@
from rest_framework.response import Response
from rest_framework.views import APIView

from ..compat import course_exists, get_course_outline, modulestore
from ..compat import course_exists, get_course_outline
from ..models import SectionToCourseLink


class CourseAutocomplete(APIView):
"""
Autocomplete API endpoint for courses.
"""

permission_classes = [IsAdminUser]

def get(self, request):
"""
Get all courses and match a search term against them.
"""
self.check_permissions(request)
section_courses = set(SectionToCourseLink.objects.values_list('destination_course_id', flat=True))
all_courses = [
{'id': str(course.id), 'text': f'{course.display_name} ({course.id})'}
for course in modulestore().get_courses()
if course.id not in section_courses
]
term = request.GET.get('term', '').lower()
courses = [
entry for entry in all_courses
if entry['id'].lower().startswith(term) or entry['text'].lower().startswith(term)
]
return Response(data={'results': courses}, status=status.HTTP_200_OK)


class SectionAutocomplete(APIView):
"""
Autocomplete API endpoint for course sections.
Expand Down
2 changes: 1 addition & 1 deletion section_to_course/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def get_course(course_key: CourseLocator):
"""
Get a course from the modulestore.
"""
return modulestore().get_course(course_key)
return get_course_outline(course_key)


def modulestore():
Expand Down
6 changes: 2 additions & 4 deletions section_to_course/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,9 @@ def test_create_course(self):
link = SectionToCourseLink.objects.get()
assert link.source_course_id == course.id
assert link.source_section_id == section.location
update_outline_from_modulestore(link.destination_course_id)
dest_course = get_course(link.destination_course_id)
assert dest_course.display_name == 'New Course'
assert dest_course.org == org.short_name
assert dest_course.number == 'NC101'
assert dest_course.location.run == '2023'
assert dest_course.title == 'New Course'
assert dest_course.self_paced
# Should not create a new link if it's already made.
response = self.create_section_to_course_link(course, section, org)
Expand Down

0 comments on commit c407167

Please sign in to comment.