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: [FC-0031] Add new endpoint BlocksInfoInCourseView #33296

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
36 changes: 36 additions & 0 deletions lms/djangoapps/mobile_api/course_info/serializers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"""
Course Info serializers
"""
from rest_framework import serializers

from openedx.core.djangoapps.content.course_overviews.models import CourseOverview


class CourseInfoOverviewSerializer(serializers.ModelSerializer):
"""
Serializer for serialize additional fields in BlocksInfoInCourseView.
"""

name = serializers.CharField(source='display_name')
number = serializers.CharField(source='display_number_with_default')
org = serializers.CharField(source='display_org_with_default')
is_self_paced = serializers.BooleanField(source='self_paced')
media = serializers.SerializerMethodField()

class Meta:
model = CourseOverview
fields = (
'name',
'number',
'org',
'start',
'start_display',
'start_type',
'end',
'is_self_paced',
'media',
)

@staticmethod
def get_media(obj):
return {'image': obj.image_urls}
47 changes: 47 additions & 0 deletions lms/djangoapps/mobile_api/course_info/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import
from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin
from lms.djangoapps.mobile_api.utils import API_V1, API_V05
from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView
from openedx.features.course_experience import ENABLE_COURSE_GOALS
from xmodule.html_block import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -255,3 +256,49 @@ def test_flag_disabled(self, mock_logger):
'For this mobile request, user activity is not enabled for this user {} and course {}'.format(
str(self.user.id), str(self.course.id))
)


@ddt.ddt
class TestBlocksInfoInCourseView(TestBlocksInCourseView): # lint-amnesty, pylint: disable=test-inherits-tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test cases for all the response types mentioned in response status codes returned by this view.

Copy link
Member

Choose a reason for hiding this comment

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

Just because we are extending TestBlocksInCourseView we decided and agreed to focus only on testing the changes in response (new fields as media, course dates, certificate) exist in response

"""
Test class for BlocksInfoInCourseView
"""

def setUp(self):
super().setUp()
self.url = reverse('blocks_info_in_course', kwargs={
'api_version': 'v3',
})

@patch('lms.djangoapps.mobile_api.course_info.views.certificate_downloadable_status')
def test_additional_info_response(self, mock_certificate_downloadable_status):
certificate_url = 'https://test_certificate_url'
mock_certificate_downloadable_status.return_value = {
'is_downloadable': True,
'download_url': certificate_url,
}

expected_image_urls = {
'image':
{
'large': '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg',
'raw': '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg',
'small': '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg'
}
}

response = self.verify_response(url=self.url)

assert response.status_code == 200
assert response.data['id'] == str(self.course.id)
assert response.data['name'] == self.course.display_name
assert response.data['number'] == self.course.display_number_with_default
assert response.data['org'] == self.course.display_org_with_default
assert response.data['start'] == self.course.start.strftime('%Y-%m-%dT%H:%M:%SZ')
assert response.data['start_display'] == 'July 17, 2015'
assert response.data['start_type'] == 'timestamp'
assert response.data['end'] == self.course.end
assert response.data['media'] == expected_image_urls
assert response.data['certificate'] == {'url': certificate_url}
assert response.data['is_self_paced'] is False
mock_certificate_downloadable_status.assert_called_once()
3 changes: 2 additions & 1 deletion lms/djangoapps/mobile_api/course_info/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.conf import settings
from django.urls import path, re_path

from .views import CourseHandoutsList, CourseUpdatesList, CourseGoalsRecordUserActivity
from .views import CourseHandoutsList, CourseUpdatesList, CourseGoalsRecordUserActivity, BlocksInfoInCourseView

urlpatterns = [
re_path(
Expand All @@ -20,4 +20,5 @@
name='course-updates-list'
),
path('record_user_activity', CourseGoalsRecordUserActivity.as_view(), name='record_user_activity'),
path('blocks/', BlocksInfoInCourseView.as_view(), name="blocks_info_in_course"),
]
148 changes: 148 additions & 0 deletions lms/djangoapps/mobile_api/course_info/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@
from rest_framework.views import APIView

from common.djangoapps.static_replace import make_static_urls_absolute
from lms.djangoapps.certificates.api import certificate_downloadable_status
from lms.djangoapps.courseware.courses import get_course_info_section_block
from lms.djangoapps.course_goals.models import UserActivity
from lms.djangoapps.course_api.blocks.views import BlocksInCourseView
from lms.djangoapps.mobile_api.course_info.serializers import CourseInfoOverviewSerializer
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.lib.api.view_utils import view_auth_classes
from openedx.core.lib.xblock_utils import get_course_update_items
from openedx.features.course_experience import ENABLE_COURSE_GOALS
from ..decorators import mobile_course_access, mobile_view
Expand Down Expand Up @@ -166,3 +171,146 @@ def post(self, request, *args, **kwargs):
# Populate user activity for tracking progress towards a user's course goals
UserActivity.record_user_activity(user, course_key)
return Response(status=(200))


@view_auth_classes(is_authenticated=False)
class BlocksInfoInCourseView(BlocksInCourseView):
"""
**Use Case**

This API endpoint is specifically optimized for the course homepage on Mobile Apps.
The endpoint returns the blocks in the course according to the requesting user's access level.
Additionally, response encompasses info fields with information about the course,
including certificate URL, media dictionary with course image URLs, start and end dates for the course.

**Example requests**:

This api works with all versions {api_version}, you can use: v0.5, v1, v2 or v3
Copy link
Contributor

Choose a reason for hiding this comment

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

Why introduce a new version if it can work with previous versions?

Copy link
Member

Choose a reason for hiding this comment

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

Hello @jawad-khan, the version 3 was introduced first of all in PR: #33294
Here we have added version 3 support as per comment https://openedx.atlassian.net/wiki/spaces/COMM/pages/3853910042?focusedCommentId=3856859206 for general consistency in new mobile applications.

Ed's suggestion was to make it more intuitive for using mobile API with new and old apps:

one could assume that any call to the API version n or above would be for the new apps, < n for the legacy apps


GET /api/mobile/{api_version}/course_info/blocks/?course_id=<course_id>
GET /api/mobile/{api_version}/course_info/blocks/?course_id=<course_id>
&username=anjali
&depth=all
&requested_fields=graded,format,student_view_multi_device,lti_url
&block_counts=video
&student_view_data=video
&block_types_filter=problem,html

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add information about the params this view accpets?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I have done that

Copy link
Contributor

Choose a reason for hiding this comment

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

You can refer to the parent class about all params not used here. We are only using course_id and return_type here.

**Parameters:**

username (str): The username of the specified user for whom the course data
is being accessed.
depth (integer, str, None): Optional number of blocks you receive in response
course nesting depth, you can get only sections, sections and subsections,
or provide string 'all' to receive all blocks of the course.
requested_field (list): Optional list of names of additional fields to return for each block.
Supported fields can be found in transformers.SUPPORTED_FIELDS.
block_counts (list): Optional list of names of block types for which an aggregated count
of blocks is returned.
student_view_data (list): Optional list of names of block types for
which student_view_data is returned.
block_types_filter (list): Filter by block types:
'video', 'discussion', 'html', 'chapter', 'sequential', 'vertical'.
return_type (list, dict): Optional list or dictionary of block's fields based on 'return_type'.

**Response example**

Body consists of the following fields, you received this response if you use
'return_type=dict' in query params:

root: (str) The ID of the root node of the requested course block structure.\
blocks: (dict) A dictionary or list, based on the value of the
"return_type" parameter. Maps block usage IDs to a collection of
information about each block. Each block contains the following
fields.

id: (str) The Course's id (Course Run key)
Copy link
Contributor

Choose a reason for hiding this comment

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

what if we call the view with list param, will we still get these fields?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, it's done. Add description to "return_type=list"

Copy link
Contributor

Choose a reason for hiding this comment

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

If we call this view with return_type=list we will not get certificate and all other details processed in this view. Please update view according to the desired behavior.

Copy link
Member

Choose a reason for hiding this comment

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

@jawad-khan, the docstring was updated to explicitly include data about the response when return_type=list is provided

name: (str) The course's name
number: (str) The course's number
org: (str) The course's organisation
start: (str) Date the course begins, in ISO 8601 notation
start_display: (str) Readably formatted start of the course
start_type: (str) Hint describing how `start_display` is set. One of:
* `"string"`: manually set by the course author
* `"timestamp"`: generated from the `start` timestamp
* `"empty"`: no start date is specified
end: (str) Date the course ends, in ISO 8601 notation
media: (dict) An object that contains named media items. Included here:
* course_image: An image to show for the course. Represented
as an object with the following fields:
* uri: The location of the image
certificate: (dict) Information about the user's earned certificate in the course.
Included here:
* uri: The location of the user's certificate
is_self_paced: (bool) Indicates if the course is self paced

Body consists of the following fields, you received this response if you use
'return_type=list' in query params:

id: (str) The Course's id (Course Run key)
block_id: (str) The unique identifier for the block_id
lms_web_url: (str) The URL to the navigational container of the xBlock on the web.
legacy_web_url: (str) Like `lms_web_url`, but always directs to
the "Legacy" frontend experience.
student_view_url: (str) The URL to retrieve the HTML rendering
of this block's student view
type: (str): The type of block. Possible values the names of any
XBlock type in the system, including custom blocks. Examples are
course, chapter, sequential, vertical, html, problem, video, and
discussion.
display_name: (str) The display name of the block.

**Returns**

* 200 on success with above fields.
* 400 if an invalid parameter was sent or the username was not provided
* 401 unauthorized, the provided access token has expired and is no longer valid
for an authenticated request.
* 403 if a user who does not have permission to masquerade as
another user specifies a username other than their own.
* 404 if the course is not available or cannot be seen.
"""

def get_certificate(self, request, course_id):
"""
Returns the information about the user's certificate in the course.

Arguments:
request (Request): The request object.
course_id (str): The identifier of the course.
Returns:
(dict): A dict containing information about location of the user's certificate
or an empty dictionary, if there is no certificate.
"""
if request.user.is_authenticated:
certificate_info = certificate_downloadable_status(request.user, course_id)
if certificate_info['is_downloadable']:
return {
'url': request.build_absolute_uri(
certificate_info['download_url']
),
}
return {}

def list(self, request, **kwargs): # pylint: disable=W0221
"""
REST API endpoint for listing all the blocks information in the course and
information about the course while regarding user access and roles.

Arguments:
request - Django request object
"""

response = super().list(request, kwargs)

if request.GET.get('return_type', 'dict') == 'dict':
course_id = request.query_params.get('course_id', None)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if course_id is None?
Please handle all these cases in tests.

Copy link
Member

Choose a reason for hiding this comment

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

Hi Jawad!
The course_id is a required field and validated in the parent class - here https://github.com/raccoongang/edx-platform/blob/master/lms/djangoapps/course_api/blocks/views.py#L305

Furthermore, all the test cases are covered because TestBlocksInfoInCourseView class is extending TestBlocksInCourseView which has those test cases covered.

course_key = CourseKey.from_string(course_id)
course_overview = CourseOverview.get_from_id(course_key)
course_data = {
'id': course_id,
'certificate': self.get_certificate(request, course_key),
}
course_data.update(CourseInfoOverviewSerializer(course_overview).data)
response.data.update(course_data)
return response
Loading