From db6fd6f8f4478e91bb531e6c2fa50143e1c2e012 Mon Sep 17 00:00:00 2001 From: Fox Danger Piacenti Date: Wed, 17 May 2023 12:11:14 -0500 Subject: [PATCH 1/2] fix: Drop autocomplete of courses for performance. --- CHANGELOG.rst | 9 +++ section_to_course/__init__.py | 2 +- section_to_course/admin.py | 13 ---- section_to_course/api/tests/test_views.py | 85 ----------------------- section_to_course/api/urls.py | 1 - section_to_course/api/views.py | 28 +------- section_to_course/compat.py | 2 +- section_to_course/tests/test_admin.py | 6 +- 8 files changed, 14 insertions(+), 132 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 50538d5..ea39e9a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,15 @@ Change Log Unreleased ********** + +[0.3.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 ===== diff --git a/section_to_course/__init__.py b/section_to_course/__init__.py index 7bddbce..16b866e 100644 --- a/section_to_course/__init__.py +++ b/section_to_course/__init__.py @@ -2,4 +2,4 @@ Factors sections from Open edX courses into their own new course. """ -__version__ = '0.3.0' +__version__ = '0.4.0' diff --git a/section_to_course/admin.py b/section_to_course/admin.py index 197735f..9f69e7e 100644 --- a/section_to_course/admin.py +++ b/section_to_course/admin.py @@ -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. @@ -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, diff --git a/section_to_course/api/tests/test_views.py b/section_to_course/api/tests/test_views.py index 5a39bc4..53b0ae4 100644 --- a/section_to_course/api/tests/test_views.py +++ b/section_to_course/api/tests/test_views.py @@ -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') diff --git a/section_to_course/api/urls.py b/section_to_course/api/urls.py index 88091b6..3f9bdf9 100644 --- a/section_to_course/api/urls.py +++ b/section_to_course/api/urls.py @@ -8,7 +8,6 @@ app_name = 'section_to_course' urlpatterns = [ - path('autocomplete/course/', views.CourseAutocomplete.as_view(), name='course_autocomplete'), path( 'autocomplete/course//sections/', views.SectionAutocomplete.as_view(), diff --git a/section_to_course/api/views.py b/section_to_course/api/views.py index 9fa8b04..ef97172 100644 --- a/section_to_course/api/views.py +++ b/section_to_course/api/views.py @@ -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. diff --git a/section_to_course/compat.py b/section_to_course/compat.py index ee5aa27..a95f7c4 100644 --- a/section_to_course/compat.py +++ b/section_to_course/compat.py @@ -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(): diff --git a/section_to_course/tests/test_admin.py b/section_to_course/tests/test_admin.py index 116d2dd..ccad190 100644 --- a/section_to_course/tests/test_admin.py +++ b/section_to_course/tests/test_admin.py @@ -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) From 2119c012d10f8a1b8f1a2a5cbc4f751215ad5b4e Mon Sep 17 00:00:00 2001 From: Fox Piacenti Date: Thu, 18 May 2023 11:09:34 -0500 Subject: [PATCH 2/2] docs: Update CHANGELOG.rst Co-authored-by: Piotr Surowiec --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ea39e9a..61654e1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,7 +15,7 @@ Unreleased ********** -[0.3.0] - 2023-05-18 +[0.4.0] - 2023-05-18 ******************** Changed