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

feat: cherry pick mobile apis #29

Merged
merged 24 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e193555
feat: [AXIM-44] Adapt Delete Account to Bearer Authorization
Sep 8, 2023
a677b68
fix: (review) Add comment to Delete Account view
KyryloKireiev Sep 25, 2023
0a5110c
fix: [AXIM-50] Fix count items in pagination
Sep 12, 2023
9060663
feat: list courses details by keys
yusuf-musleh Jun 26, 2023
b7f4923
fix: [FC-0031] Add limit the number of returned results for mobile_se…
KyryloKireiev Nov 1, 2023
1100983
feat: [AXIM-6] Add DefaultPagination for UserCourseEnrollmentsList v3
KyryloKireiev Sep 12, 2023
51f0ebe
docs: add docstring for the paginator property override
GlugovGrGlib Nov 20, 2023
b739c00
fix: remove trailing whitespace failing quality check
GlugovGrGlib Nov 20, 2023
25577e2
feat: add tracking event for following post
Oct 4, 2023
8b94c2c
feat: added edit_by_label and closed_by_label in threads response (#3…
muhammadadeeltajamul Apr 19, 2023
da93542
feat: [AXIM-20] Add profile_image to API CommentViewSet
KyryloKireiev Sep 13, 2023
7798746
refactor: [FC-0031] Move get_profile_image method to api
KyryloKireiev Nov 8, 2023
e0b4d50
fix: remove duplicate implementation and correct the docstring
GlugovGrGlib Nov 13, 2023
f37ddd3
feat: [AXIM-26] Extended BlocksInCourseView API
KyryloKireiev Sep 15, 2023
3bb8ebc
fix: [FC-0031] Add parameters description, refactor list method
KyryloKireiev Oct 30, 2023
462c780
refactor: [FC-0031] Use serializer instead of custom function
KyryloKireiev Dec 15, 2023
fd3d355
docs: [FC-0031] Update docstring
GlugovGrGlib Dec 19, 2023
4d179d6
feat: [FC-0031] Add optional field 'is_enrolled' to course detail view
KyryloKireiev Oct 31, 2023
5c1fff6
fix: [FC-0031] Restrict access to is_enrolled field
GlugovGrGlib Dec 15, 2023
8a43fac
feat(mobile_api): Add course access object to mobile course info API …
GlugovGrGlib Apr 25, 2024
fb0f16a
fix: discussion tab should be None if discussion tab is disabled (#33…
muhammadadeeltajamul Dec 11, 2023
79756e8
feat: Added upgrade deadline in blocks api (#34750)
jawad-khan May 9, 2024
37b7549
feat: Add course price in mobile enrollment api (#34255)
jawad-khan Feb 19, 2024
83ead13
temp: fix TestDiscussionCourseEnrollmentSerializer.test_discussion_ta…
OmarIthawi Jun 4, 2024
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
14 changes: 10 additions & 4 deletions lms/djangoapps/branding/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers


def get_visible_courses(org=None, filter_=None, active_only=False):
def get_visible_courses(org=None, filter_=None, active_only=False, course_keys=None):
"""
Yield the CourseOverviews that should be visible in this branded
instance.
Expand All @@ -25,6 +25,8 @@ def get_visible_courses(org=None, filter_=None, active_only=False):
filter_ (dict): Optional parameter that allows custom filtering by
fields on the course.
active_only (bool): Optional parameter that enables fetching active courses only.
course_keys (list[str]): Optional parameter that allows for selecting which
courses to fetch the `CourseOverviews` for
"""
# Import is placed here to avoid model import at project startup.
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
Expand All @@ -36,12 +38,16 @@ def get_visible_courses(org=None, filter_=None, active_only=False):
if org:
# Check the current site's orgs to make sure the org's courses should be displayed
if not current_site_orgs or org in current_site_orgs:
courses = CourseOverview.get_all_courses(orgs=[org], filter_=filter_, active_only=active_only)
courses = CourseOverview.get_all_courses(
orgs=[org], filter_=filter_, active_only=active_only, course_keys=course_keys
)
elif current_site_orgs:
# Only display courses that should be displayed on this site
courses = CourseOverview.get_all_courses(orgs=current_site_orgs, filter_=filter_, active_only=active_only)
courses = CourseOverview.get_all_courses(
orgs=current_site_orgs, filter_=filter_, active_only=active_only, course_keys=course_keys
)
else:
courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only)
courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only, course_keys=course_keys)

courses = courses.order_by('id')

Expand Down
25 changes: 21 additions & 4 deletions lms/djangoapps/course_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def course_detail(request, username, course_key):
return overview


def _filter_by_search(course_queryset, search_term):
def _filter_by_search(course_queryset, search_term, mobile_search=False):
"""
Filters a course queryset by the specified search term.
"""
Expand All @@ -101,6 +101,13 @@ def _filter_by_search(course_queryset, search_term):

search_courses_ids = {course['data']['id'] for course in search_courses['results']}

if mobile_search is True:
course_limit = getattr(settings, 'MOBILE_SEARCH_COURSE_LIMIT', 100)
courses = [course for course in course_queryset[:course_limit] if str(course.id) in search_courses_ids]
return LazySequence(
iter(courses),
est_len=len(courses)
)
return LazySequence(
(
course for course in course_queryset
Expand All @@ -116,7 +123,9 @@ def list_courses(request,
filter_=None,
search_term=None,
permissions=None,
active_only=False):
active_only=False,
course_keys=None,
mobile_search=False):
"""
Yield all available courses.

Expand Down Expand Up @@ -146,13 +155,21 @@ def list_courses(request,
If specified, it filters visible `CourseOverview` objects by
checking if each permission specified is granted for the username.
active_only (bool): Optional parameter that enables fetching active courses only.
course_keys (list[str]):
If specified, it filters visible `CourseOverview` objects by
the course keys (ids) provided
mobile_search (bool):
Optional parameter that limits the number of returned courses
to MOBILE_SEARCH_COURSE_LIMIT.

Return value:
Yield `CourseOverview` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions, active_only=active_only)
course_qs = _filter_by_search(course_qs, search_term)
course_qs = get_courses(
user, org=org, filter_=filter_, permissions=permissions, active_only=active_only, course_keys=course_keys
)
course_qs = _filter_by_search(course_qs, search_term, mobile_search)
return course_qs


Expand Down
16 changes: 16 additions & 0 deletions lms/djangoapps/course_api/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
mobile = ExtendedNullBooleanField(required=False)
active_only = ExtendedNullBooleanField(required=False)
permissions = MultiValueField(required=False)
course_keys = MultiValueField(required=False)
mobile_search = ExtendedNullBooleanField(required=False)

def clean(self):
"""
Expand All @@ -80,6 +82,20 @@ def clean(self):

return cleaned_data

def clean_course_keys(self):
"""
Ensure valid course_keys were provided.
"""
course_keys = self.cleaned_data['course_keys']
if course_keys:
for course_key in course_keys:
try:
CourseKey.from_string(course_key)
except InvalidKeyError:
raise ValidationError(f"'{str(course_key)}' is not a valid course key.") # lint-amnesty, pylint: disable=raise-missing-from

return course_keys


class CourseIdListGetForm(UsernameValidatorMixin, Form):
"""
Expand Down
15 changes: 15 additions & 0 deletions lms/djangoapps/course_api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import urllib

from common.djangoapps.student.models import CourseEnrollment
from django.contrib.auth import get_user_model
from django.urls import reverse
from edx_django_utils import monitoring as monitoring_utils
from rest_framework import serializers
Expand Down Expand Up @@ -164,10 +166,23 @@ def to_representation(self, instance):
"""
Get the `certificate_available_date` in response
if the `certificates.auto_certificate_generation` waffle switch is enabled

Get the 'is_enrolled' in response if 'username' is in query params,
user is staff, superuser, or user is authenticated and
the has the same 'username' as the 'username' in the query params.
"""
response = super().to_representation(instance)
if can_show_certificate_available_date_field(instance):
response['certificate_available_date'] = instance.certificate_available_date

requested_username = self.context['request'].query_params.get('username', None)
if requested_username:
user = self.context['request'].user
if ((user.is_authenticated and user.username == requested_username)
or user.is_staff or user.is_superuser):
User = get_user_model()
requested_user = User.objects.get(username=requested_username)
response['is_enrolled'] = CourseEnrollment.is_enrolled(requested_user, instance.id)
return response


Expand Down
37 changes: 36 additions & 1 deletion lms/djangoapps/course_api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ def _make_api_call(self,
specified_user,
org=None,
filter_=None,
permissions=None):
permissions=None,
course_keys=None):
"""
Call the list_courses api endpoint to get information about
`specified_user` on behalf of `requesting_user`.
Expand All @@ -121,6 +122,7 @@ def _make_api_call(self,
org=org,
filter_=filter_,
permissions=permissions,
course_keys=course_keys,
)

def verify_courses(self, courses):
Expand Down Expand Up @@ -244,6 +246,39 @@ def test_permissions(self):

self.assertEqual({c.id for c in filtered_courses}, {self.course.id})

def test_filter_by_keys(self):
"""
Verify that courses are filtered by the provided course keys.
"""

# Create alternative courses to be included in the `course_keys` filter.
alternative_course_1 = self.create_course(course='alternative-course-1')
alternative_course_2 = self.create_course(course='alternative-course-2')

# No filtering.
unfiltered_expected_courses = [
self.course,
alternative_course_1,
alternative_course_2,
]
unfiltered_courses = self._make_api_call(self.honor_user, self.honor_user)
assert {course.id for course in unfiltered_courses} == {course.id for course in unfiltered_expected_courses}

# With filtering.
filtered_expected_courses = [
alternative_course_1,
alternative_course_2,
]
filtered_courses = self._make_api_call(
self.honor_user,
self.honor_user,
course_keys={
alternative_course_1.id,
alternative_course_2.id
}
)
assert {course.id for course in filtered_courses} == {course.id for course in filtered_expected_courses}


class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
"""
Expand Down
10 changes: 10 additions & 0 deletions lms/djangoapps/course_api/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ def set_up_data(self, user):
'filter_': None,
'permissions': set(),
'active_only': None,
'course_keys': set(),
'mobile_search': None,
}

def test_basic(self):
Expand Down Expand Up @@ -100,6 +102,14 @@ def test_filter(self, param_field_name, param_field_value):

self.assert_valid(self.cleaned_data)

def test_invalid_course_keys(self):
"""
Verify form checks for validity of course keys provided
"""

self.form_data['course_keys'] = 'course-v1:edX+DemoX+Demo_Course,invalid_course_key'
self.assert_error('course_keys', "'invalid_course_key' is not a valid course key.")


class TestCourseIdListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring
FORM_CLASS = CourseIdListGetForm
Expand Down
43 changes: 43 additions & 0 deletions lms/djangoapps/course_api/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,49 @@ def test_basic(self):
)
self.assertDictEqual(result, self.expected_data)

@mock.patch('lms.djangoapps.course_api.serializers.CourseEnrollment.is_enrolled', return_value=True)
def test_is_enrolled_field_true(self, mock_is_enrolled):
course = self.create_course()
result = self._get_result_with_query_param(course)
assert result['is_enrolled'] is True
mock_is_enrolled.assert_called_once()

@mock.patch('lms.djangoapps.course_api.serializers.CourseEnrollment.is_enrolled', return_value=False)
def test_is_enrolled_field_false(self, mock_is_enrolled):
course = self.create_course()
result = self._get_result_with_query_param(course)
assert result['is_enrolled'] is False
mock_is_enrolled.assert_called_once()

def test_is_enrolled_field_anonymous_user(self):
course = self.create_course()
result = self._get_anonymous_result(course)
self.assertNotIn('is_enrolled', result)

def _get_anonymous_request(self):
return Request(self.request_factory.get('/'))

def _get_anonymous_result(self, course):
course_overview = CourseOverview.get_from_id(course.id)
return self.serializer_class(course_overview, context={'request': self._get_anonymous_request()}).data

def _get_result_with_query_param(self, course):
"""
Return the CourseSerializer for the specified course with 'username' in query params.
"""
course_overview = CourseOverview.get_from_id(course.id)
return self.serializer_class(course_overview, context={'request': self._get_request_with_query_param()}).data

def _get_request_with_query_param(self, user=None):
"""
Build a Request object for the specified user with 'username' in query params.
"""
if user is None:
user = self.honor_user
request = Request(self.request_factory.get('/', {'username': user.username}))
request.user = user
return request


class TestCourseKeySerializer(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring

Expand Down
40 changes: 40 additions & 0 deletions lms/djangoapps/course_api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,46 @@ def test_too_many_courses(self):
assert len(response.data['results']) == (30 if (page < 11) else 3)
assert [c['id'] for c in response.data['results']] == ordered_course_ids[((page - 1) * 30):(page * 30)]

def test_count_item_pagination_with_search_term(self):
"""
Test count items in pagination for api courses list - class CourseListView
"""
# Create 15 new courses, courses have the word "new" in the title
[self.create_and_index_course(f"numb_{number}", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned
response = self.verify_response(params={"search_term": "new"})
self.assertEqual(response.status_code, 200)
# We don't have 'count' 15 because 'mobile_search' param is None
# And LazySequence contains all courses
self.assertEqual(response.json()["pagination"]["count"], 18)

def test_count_item_pagination_with_search_term_and_filter(self):
"""
Test count items in pagination for api courses list
with search_term and filter by organisation -
class CourseListView
"""
# Create 25 new courses with two different organisations
[self.create_and_index_course("Org_N", f"new_{number}") for number in range(10)] # pylint: disable=expression-not-assigned
[self.create_and_index_course("Org_X", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned
response = self.verify_response(params={"org": "Org_X", "search_term": "new"})
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json()["pagination"]["count"], 15)

def test_count_item_pagination_with_search_term_and_mobile_search(self):
"""
Test count items in pagination for api courses list
with search_term and 'mobile_search' is True
"""
# Create 25 new courses with two different words in titles
[self.create_and_index_course("Org_N", f"old_{number}") for number in range(10)] # pylint: disable=expression-not-assigned
[self.create_and_index_course("Org_N", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned
response = self.verify_response(
params={"search_term": "new", "mobile_search": True}
)
self.assertEqual(response.status_code, 200)
# We have 'count' 15 because 'mobile_search' param is true
self.assertEqual(response.json()["pagination"]["count"], 15)


class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase):
"""
Expand Down
12 changes: 11 additions & 1 deletion lms/djangoapps/course_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,14 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
date are returned. This is different from search_term because this filtering is done on
CourseOverview and not ElasticSearch.

course_keys (optional):
If specified, it fetches the `CourseOverview` objects for the
the specified course keys

mobile_search (bool):
Optional parameter that limits the number of returned courses
to MOBILE_SEARCH_COURSE_LIMIT.

**Returns**

* 200 on success, with a list of course discovery objects as returned
Expand Down Expand Up @@ -343,7 +351,9 @@ def get_queryset(self):
filter_=form.cleaned_data['filter_'],
search_term=form.cleaned_data['search_term'],
permissions=form.cleaned_data['permissions'],
active_only=form.cleaned_data.get('active_only', False)
active_only=form.cleaned_data.get('active_only', False),
course_keys=form.cleaned_data['course_keys'],
mobile_search=form.cleaned_data.get('mobile_search', False),
)


Expand Down
5 changes: 3 additions & 2 deletions lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ def get_course_syllabus_section(course, section_key):


@function_trace('get_courses')
def get_courses(user, org=None, filter_=None, permissions=None, active_only=False):
def get_courses(user, org=None, filter_=None, permissions=None, active_only=False, course_keys=None):
"""
Return a LazySequence of courses available, optionally filtered by org code
(case-insensitive) or a set of permissions to be satisfied for the specified
Expand All @@ -763,7 +763,8 @@ def get_courses(user, org=None, filter_=None, permissions=None, active_only=Fals
courses = branding.get_visible_courses(
org=org,
filter_=filter_,
active_only=active_only
active_only=active_only,
course_keys=course_keys
).prefetch_related(
'modes',
).select_related(
Expand Down
Loading
Loading