Skip to content
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.

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

fix: return empty list when no courses are found for request #35942

Merged
merged 6 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
86 changes: 83 additions & 3 deletions cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
Unit tests for home page view.
"""
import ddt
import pytz
from collections import OrderedDict
from datetime import datetime, timedelta
from django.conf import settings
from django.test import override_settings
from django.urls import reverse
Expand Down Expand Up @@ -100,12 +102,13 @@ class HomePageCoursesViewTest(CourseTestCase):
def setUp(self):
super().setUp()
self.url = reverse("cms.djangoapps.contentstore:v1:courses")
CourseOverviewFactory.create(
self.course_overview = CourseOverviewFactory.create(
id=self.course.id,
org=self.course.org,
display_name=self.course.display_name,
display_number_with_default=self.course.number,
)
self.non_staff_client, _ = self.create_non_staff_authed_user_client()

def test_home_page_response(self):
"""Check successful response content"""
Expand All @@ -131,6 +134,7 @@ def test_home_page_response(self):
print(response.data)
self.assertDictEqual(expected_response, response.data)

@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
def test_home_page_response_with_api_v2(self):
"""Check successful response content with api v2 modifications.

Expand All @@ -155,12 +159,88 @@ def test_home_page_response_with_api_v2(self):
"in_process_course_actions": [],
}

with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API):
response = self.client.get(self.url)
response = self.client.get(self.url)

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertDictEqual(expected_response, response.data)

@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
@ddt.data(
("active_only", "true", 2, 0),
("archived_only", "true", 0, 1),
("search", "sample", 1, 0),
("search", "demo", 0, 1),
("order", "org", 2, 1),
("order", "display_name", 2, 1),
("order", "number", 2, 1),
("order", "run", 2, 1)
)
@ddt.unpack
def test_filter_and_ordering_courses(
self,
filter_key,
filter_value,
expected_active_length,
expected_archived_length
):
"""Test home page with org filter and ordering for a staff user.

The test creates an active/archived course, and then filters/orders them using the query parameters.
"""
archived_course_key = self.store.make_course_key("demo-org", "demo-number", "demo-run")
CourseOverviewFactory.create(
display_name="Course (Demo)",
id=archived_course_key,
org=archived_course_key.org,
end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC),
)
active_course_key = self.store.make_course_key("sample-org", "sample-number", "sample-run")
CourseOverviewFactory.create(
display_name="Course (Sample)",
id=active_course_key,
org=active_course_key.org,
)

response = self.client.get(self.url, {filter_key: filter_value})

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(len(response.data["archived_courses"]), expected_archived_length)
self.assertEqual(len(response.data["courses"]), expected_active_length)

@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
@ddt.data(
("active_only", "true"),
("archived_only", "true"),
("search", "sample"),
("order", "org"),
)
@ddt.unpack
def test_filter_and_ordering_no_courses_staff(self, filter_key, filter_value):
"""Test home page with org filter and ordering when there are no courses for a staff user."""
self.course_overview.delete()

response = self.client.get(self.url, {filter_key: filter_value})

self.assertEqual(len(response.data["courses"]), 0)
self.assertEqual(response.status_code, status.HTTP_200_OK)

@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
@ddt.data(
("active_only", "true"),
("archived_only", "true"),
("search", "sample"),
("order", "org"),
)
@ddt.unpack
def test_home_page_response_no_courses_non_staff(self, filter_key, filter_value):
"""Test home page with org filter and ordering when there are no courses for a non-staff user."""
self.course_overview.delete()

response = self.non_staff_client.get(self.url)

self.assertEqual(len(response.data["courses"]), 0)
self.assertEqual(response.status_code, status.HTTP_200_OK)

@override_waffle_switch(ENABLE_GLOBAL_STAFF_OPTIMIZATION, True)
def test_org_query_if_passed(self):
"""Test home page when org filter passed as a query param"""
Expand Down
98 changes: 98 additions & 0 deletions cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def setUp(self):
org=archived_course_key.org,
end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC),
)
self.non_staff_client, _ = self.create_non_staff_authed_user_client()

def test_home_page_response(self):
"""Get list of courses available to the logged in user.
Expand Down Expand Up @@ -247,3 +248,100 @@ def test_api_v2_is_disabled(self, mock_modulestore, mock_course_overview):
self.assertEqual(response.status_code, status.HTTP_200_OK)
mock_modulestore().get_course_summaries.assert_called_once()
mock_course_overview.get_all_courses.assert_not_called()

@ddt.data(
("active_only", "true"),
("archived_only", "true"),
("search", "sample"),
("order", "org"),
("page", 1),
)
@ddt.unpack
def test_if_empty_list_of_courses(self, query_param, value):
"""Get list of courses when no courses are available.

Expected result:
- An empty list of courses available to the logged in user.
"""
self.active_course.delete()
self.archived_course.delete()

response = self.client.get(self.api_v2_url, {query_param: value})

self.assertEqual(len(response.data['results']['courses']), 0)
self.assertEqual(response.status_code, status.HTTP_200_OK)

@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
@ddt.data(
("active_only", "true", 2, 0),
("archived_only", "true", 0, 1),
("search", "foo", 1, 0),
("search", "demo", 0, 1),
("order", "org", 2, 1),
("order", "display_name", 2, 1),
("order", "number", 2, 1),
("order", "run", 2, 1)
)
@ddt.unpack
def test_filter_and_ordering_courses(
self,
filter_key,
filter_value,
expected_active_length,
expected_archived_length
):
"""Get list of courses when filter and ordering are applied.

This test creates two courses besides the default courses created in the setUp method.
Then filters and orders them based on the filter_key and filter_value passed as query parameters.

Expected result:
- A list of courses available to the logged in user for the specified filter and order.
"""
archived_course_key = self.store.make_course_key("demo-org", "demo-number", "demo-run")
CourseOverviewFactory.create(
display_name="Course (Demo)",
id=archived_course_key,
org=archived_course_key.org,
end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC),
)
active_course_key = self.store.make_course_key("foo-org", "foo-number", "foo-run")
CourseOverviewFactory.create(
display_name="Course (Foo)",
id=active_course_key,
org=active_course_key.org,
)

response = self.client.get(self.api_v2_url, {filter_key: filter_value})

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(
len([course for course in response.data["results"]["courses"] if course["is_active"]]),
expected_active_length
)
self.assertEqual(
len([course for course in response.data["results"]["courses"] if not course["is_active"]]),
expected_archived_length
)

@ddt.data(
("active_only", "true"),
("archived_only", "true"),
("search", "sample"),
("order", "org"),
("page", 1),
)
@ddt.unpack
def test_if_empty_list_of_courses_non_staff(self, query_param, value):
"""Get list of courses when no courses are available for non-staff users.

Expected result:
- An empty list of courses available to the logged in user.
"""
self.active_course.delete()
self.archived_course.delete()

response = self.non_staff_client.get(self.api_v2_url, {query_param: value})

self.assertEqual(len(response.data["results"]["courses"]), 0)
self.assertEqual(response.status_code, status.HTTP_200_OK)
21 changes: 18 additions & 3 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from django.contrib.auth import get_user_model
from django.contrib.auth.decorators import login_required
from django.core.exceptions import FieldError, PermissionDenied, ValidationError as DjangoValidationError
from django.db.models import QuerySet
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound
from django.shortcuts import redirect
from django.urls import reverse
Expand Down Expand Up @@ -575,6 +576,10 @@ def filter_ccx(course_access):

if course_keys:
courses_list = CourseOverview.get_all_courses(filter_={'id__in': course_keys})
else:
# If no course keys are found for the current user, then return without filtering
# or ordering the courses list.
return courses_list, []
Comment on lines +579 to +582
Copy link
Member Author

Choose a reason for hiding this comment

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


search_query, order, active_only, archived_only = get_query_params_if_present(request)
courses_list = get_filtered_and_ordered_courses(
Expand All @@ -588,7 +593,11 @@ def filter_ccx(course_access):
return courses_list, []


def get_courses_by_status(active_only, archived_only, course_overviews):
def get_courses_by_status(
active_only: bool,
archived_only: bool,
course_overviews: QuerySet[CourseOverview]
) -> QuerySet[CourseOverview]:
"""
Return course overviews based on a base queryset filtered by a status.

Expand All @@ -602,7 +611,10 @@ def get_courses_by_status(active_only, archived_only, course_overviews):
return CourseOverview.get_courses_by_status(active_only, archived_only, course_overviews)


def get_courses_by_search_query(search_query, course_overviews):
def get_courses_by_search_query(
search_query: str | None,
course_overviews: QuerySet[CourseOverview]
) -> QuerySet[CourseOverview]:
"""Return course overviews based on a base queryset filtered by a search query.

Args:
Expand All @@ -614,7 +626,10 @@ def get_courses_by_search_query(search_query, course_overviews):
return CourseOverview.get_courses_matching_query(search_query, course_overviews=course_overviews)


def get_courses_order_by(order_query, course_overviews):
def get_courses_order_by(
order_query: str | None,
course_overviews: QuerySet[CourseOverview]
) -> QuerySet[CourseOverview]:
"""Return course overviews based on a base queryset ordered by a query.

Args:
Expand Down
Loading