Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Made-with: Cursor
zamanafzal
force-pushed
the
zafzal/coure-outline-plugin
branch
from
bb9c3d9 to
4f6b9f1
Compare
for more information, see https://pre-commit.ci
| ProjectType.LMS: { | ||
| PluginURLs.NAMESPACE: "", | ||
| PluginURLs.REGEX: "", | ||
| PluginURLs.REGEX: "^api/course-outline/v0/", |
There was a problem hiding this comment.
Let's give the API a name that distinguishes it from the open edx built APIs, so the future readers may easily know that this is not an edX API, Instead it is specific to OL.
| PluginURLs.REGEX: "^api/course-outline/v0/", | |
| PluginURLs.REGEX: "^api/ol-course-outline/v0/", |
| # {prefix}s{schema_version}:{course_key}:{content_version} | ||
| # where content_version is derived from `course.course_version`. | ||
| COURSE_OUTLINE_CACHE_KEY_PREFIX = "ol_course_outline_api:outline:v0:" | ||
| COURSE_OUTLINE_CACHE_TIMEOUT_SECONDS = 60 * 60 * 24 # 24 hours |
There was a problem hiding this comment.
This should be configurable and go into settings.
| # Key format is: | ||
| # {prefix}s{schema_version}:{course_key}:{content_version} | ||
| # where content_version is derived from `course.course_version`. | ||
| COURSE_OUTLINE_CACHE_KEY_PREFIX = "ol_course_outline_api:outline:v0:" |
There was a problem hiding this comment.
Should the package name here be dynamic? something like {__package__}? OR Does it even need to be a constant when we are appending it with our own content while creating the cache key? I'd say this does not have to be a constant, rather a variable inside the view.
| @verify_course_exists() | ||
| def get(self, request, course_id): | ||
| try: | ||
| course_key = CourseKey.from_string(course_id) | ||
| except InvalidKeyError as err: | ||
| raise DeveloperErrorViewMixin.api_error( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| developer_message="Invalid course_id", | ||
| ) from err |
There was a problem hiding this comment.
Isn't verify_course_exists and the try/except duplicate? If we are already checking via the decorator, we don't need to check again.
| ) | ||
|
|
||
| @verify_course_exists() | ||
| def get(self, request, course_id): |
|
|
||
| cache.set( | ||
| cache_key, | ||
| dict(response_data), |
There was a problem hiding this comment.
Is this dict converstion needed?
| from ol_openedx_course_outline_api.utils import build_modules_from_blocks | ||
|
|
||
|
|
||
| class CourseOutlineView(DeveloperErrorViewMixin, GenericAPIView): |
There was a problem hiding this comment.
This could be a simple APIView rather than a GenerivAPIView
| """ | ||
|
|
||
| # Block type groupings used when summarizing course content. | ||
| CONTAINER_TYPES = {"course", "chapter", "sequential", "vertical"} |
There was a problem hiding this comment.
nit:
| CONTAINER_TYPES = {"course", "chapter", "sequential", "vertical"} | |
| CONTENT_TYPES = {"course", "chapter", "sequential", "vertical"} |
| cache.set( | ||
| cache_key, | ||
| dict(response_data), | ||
| COURSE_OUTLINE_CACHE_TIMEOUT_SECONDS, | ||
| ) |
There was a problem hiding this comment.
Since the cache is working on the course publish, I don't think we need to keep it timed at all. The course outline/TOC is not going to change without a course publish at all. What do you think?
| cache.set( | |
| cache_key, | |
| dict(response_data), | |
| COURSE_OUTLINE_CACHE_TIMEOUT_SECONDS, | |
| ) | |
| cache.set( | |
| cache_key, | |
| dict(response_data), | |
| None, | |
| ) |
2 participants
What are the relevant tickets?
https://github.com/mitodl/hq/issues/10460
Description (What does it do?)
This PR adds a new Open edX LMS plugin: ol_openedx_course_outline_api.
Exposes a new endpoint: GET /api/course-outline/v0/<course_id>/
Returns a course outline summary per chapter (module id/title), including:
Effort estimation fields (effort_time, effort_activities) from the platform Effort Estimation transformer via the Blocks API
Content counts under each chapter: videos, readings (html), problems, assignments (graded or formatted sequentials), and “app_items” (other leaf blocks)
Excludes staff-only content and content hidden from TOC
Adds 24h caching for the full JSON response with a versioned cache key that also changes when course.course_version changes (so publishing effectively invalidates the cache)
Screenshots (if appropriate):
How can this be tested?
Prereq: Install/enable the plugin in LMS (per repo plugin install guide), restart LMS.
tutor dev exec lms bashpip install -e /openedx/src/open-edx-plugins/src/ol_openedx_course_outline_apiManual verification
As a staff user, request:
GET http://local.openedx.io:8000/api/course-outline/v0/<course_id>/
Example course id: course-v1:edX+DemoX+Demo_Course