-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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: Make courseware URLs more human-readable #28174
Changes from all commits
a9f61b5
dfbcd79
6239b68
3843786
4870e08
195d212
a355dac
cde19a1
d2312fd
f46df34
1aba481
470b87c
0f2f1c8
8e15f6b
081d79c
5bafb69
41122b9
958b994
c4d61d9
8f88489
d0f6450
8f07f17
8fe1ecd
5602d4e
290b61c
2e82262
d786d1f
f5e3324
ead35a8
5082dd6
966a7ce
eb3bbe0
5b73a7e
6488f7b
4e1d655
9582f0f
c233b93
4ef0b73
bef4111
7a33d78
2063d85
3bc7172
ae88a4e
32b3813
e63a7a8
8a78776
98d859f
093eef0
5ed3aae
2851110
27968a0
64d14af
dea27ab
c730754
6f90485
41888c7
5c65970
27d9c89
632c3d3
ad18bce
e4c022b
0199c9e
8ebd3ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
""" | ||
|
||
from django.utils.translation import ngettext | ||
from django.conf import settings | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Am I correct that you're adding the If so, you might want to leave those links unchanged for now, and leave the course_home_api as-is. The old links should be fine to stay on the outline page for now, as they'll redirect to the new ones. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that is why I am adding them. Not 100%. |
||
from rest_framework import serializers | ||
|
||
from lms.djangoapps.course_home_api.dates.v1.serializers import DateSummarySerializer | ||
|
@@ -27,6 +28,7 @@ def get_blocks(self, block): | |
icon = None | ||
num_graded_problems = block.get('num_graded_problems', 0) | ||
scored = block.get('scored') | ||
hash_key = block['hash_key'] | ||
|
||
if num_graded_problems and block_type == 'sequential': | ||
questions = ngettext('({number} Question)', '({number} Questions)', num_graded_problems) | ||
|
@@ -55,6 +57,7 @@ def get_blocks(self, block): | |
'resume_block': block.get('resume_block', False), | ||
'type': block_type, | ||
'has_scheduled_content': block.get('has_scheduled_content'), | ||
'hash_key': hash_key, | ||
}, | ||
} | ||
for child in children: | ||
|
@@ -128,3 +131,4 @@ class OutlineTabSerializer(DatesBannerSerializerMixin, VerifiedModeSerializerMix | |
resume_course = ResumeCourseSerializer() | ||
welcome_message_html = serializers.CharField() | ||
user_has_passing_grade = serializers.BooleanField() | ||
mfe_short_url_is_active = serializers.BooleanField() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,6 +125,15 @@ class CourseLearningSequenceData: | |
learning sequences in Courses vs. Pathways vs. Libraries. Such an object | ||
would likely not have `visibility` as that holds course-specific concepts. | ||
""" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The changes to this file can be replaced with the model persistence in my PR |
||
mapping = {} | ||
|
||
def short_id_mapping(self, hash_key, *args, **kwargs): | ||
usage_key_id = kwargs.get('usage_key', None) | ||
if usage_key_id is None: | ||
return self.mapping[hash_key] | ||
self.mapping[hash_key] = usage_key_id | ||
|
||
usage_key = attr.ib(type=UsageKey) | ||
title = attr.ib(type=str) | ||
visibility = attr.ib(type=VisibilityData, default=VisibilityData()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need the CMS setting, or can we drop this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure, when I didn't have it before some of the cms tests failed, but that was also before I refactored the code.