Skip to content

Commit

Permalink
Merge branch 'master' into jill/bump-openedx-learning
Browse files Browse the repository at this point in the history
  • Loading branch information
ChrisChV authored Sep 26, 2024
2 parents 63776ff + 5d744c1 commit e39e4d6
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 48 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Unit tests for course index outline.
"""
from django.conf import settings
from django.test import RequestFactory
from django.urls import reverse
from rest_framework import status
Expand Down Expand Up @@ -62,7 +63,7 @@ def test_course_index_response(self):
"advance_settings_url": f"/settings/advanced/{self.course.id}"
},
"discussions_incontext_feedback_url": "",
"discussions_incontext_learnmore_url": "",
"discussions_incontext_learnmore_url": settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL,
"is_custom_relative_dates_active": True,
"initial_state": None,
"initial_user_clipboard": {
Expand Down Expand Up @@ -103,7 +104,7 @@ def test_course_index_response_with_show_locators(self):
"advance_settings_url": f"/settings/advanced/{self.course.id}"
},
"discussions_incontext_feedback_url": "",
"discussions_incontext_learnmore_url": "",
"discussions_incontext_learnmore_url": settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL,
"is_custom_relative_dates_active": False,
"initial_state": {
"expanded_locators": [
Expand Down
3 changes: 3 additions & 0 deletions cms/djangoapps/contentstore/views/transcripts_ajax.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,9 @@ def _get_item(request, data):
Returns the item.
"""
usage_key = UsageKey.from_string(data.get('locator'))
if not usage_key.context_key.is_course:
# TODO: implement transcript support for learning core / content libraries.
raise TranscriptsRequestValidationException(_('Transcripts are not yet supported in content libraries.'))
# This is placed before has_course_author_access() to validate the location,
# because has_course_author_access() raises r if location is invalid.
item = modulestore().get_item(usage_key)
Expand Down
8 changes: 7 additions & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2813,8 +2813,14 @@

BRAZE_COURSE_ENROLLMENT_CANVAS_ID = ''

######################## Discussion Forum settings ########################

# Feedback link in upgraded discussion notification alert
DISCUSSIONS_INCONTEXT_FEEDBACK_URL = ''
DISCUSSIONS_INCONTEXT_LEARNMORE_URL = ''

# Learn More link in upgraded discussion notification alert
# pylint: disable=line-too-long
DISCUSSIONS_INCONTEXT_LEARNMORE_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/manage_discussions/discussions.html"

#### django-simple-history##
# disable indexing on date field its coming django-simple-history.
Expand Down
10 changes: 10 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,16 @@
epub_exclude_files = ['search.html']


# -- Read the Docs Specific Configuration
# Define the canonical URL if you are using a custom domain on Read the Docs
html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "")

# Tell Jinja2 templates the build is running on Read the Docs
if os.environ.get("READTHEDOCS", "") == "True":
if "html_context" not in globals():
html_context = {}
html_context["READTHEDOCS"] = True

# -- Extension configuration -------------------------------------------------

# -- Options for intersphinx extension ---------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/content/search/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def setUp(self):
"context_key": "lib:org1:lib",
"org": "org1",
"breadcrumbs": [{"display_name": "Library"}],
"content": {"problem_types": [], "capa_content": " "},
"content": {"problem_types": [], "capa_content": ""},
"type": "library_block",
"access_id": lib_access.id,
"last_published": None,
Expand All @@ -157,7 +157,7 @@ def setUp(self):
"context_key": "lib:org1:lib",
"org": "org1",
"breadcrumbs": [{"display_name": "Library"}],
"content": {"problem_types": [], "capa_content": " "},
"content": {"problem_types": [], "capa_content": ""},
"type": "library_block",
"access_id": lib_access.id,
"last_published": None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def test_create_delete_library_block(self, meilisearch_client):
"context_key": "lib:orgA:lib_a",
"org": "orgA",
"breadcrumbs": [{"display_name": "Library Org A"}],
"content": {"problem_types": [], "capa_content": " "},
"content": {"problem_types": [], "capa_content": ""},
"access_id": lib_access.id,
"last_published": None,
"created": created_date.timestamp(),
Expand Down
6 changes: 5 additions & 1 deletion xmodule/capa_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,11 +616,15 @@ def index_dictionary(self):
"",
capa_content
)
# Strip out all other tags, leaving their content. But we want spaces between adjacent tags, so that
# <choice correct="true"><div>Option A</div></choice><choice correct="false"><div>Option B</div></choice>
# becomes "Option A Option B" not "Option AOption B" (these will appear in search results)
capa_content = re.sub(r"</(\w+)><([^>]+)>", r"</\1> <\2>", capa_content)
capa_content = re.sub(
r"(\s|&nbsp;|//)+",
" ",
nh3.clean(capa_content, tags=set())
)
).strip()

capa_body = {
"capa_content": capa_content,
Expand Down
15 changes: 0 additions & 15 deletions xmodule/partitions/tests/test_partitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,21 +462,6 @@ class TestPartitionService(PartitionServiceBaseClass):
Test getting a user's group out of a partition
"""

def test_get_user_group_id_for_partition(self):
# assign the first group to be returned
user_partition_id = self.user_partition.id
groups = self.user_partition.groups
self.user_partition.scheme.current_group = groups[0]

# get a group assigned to the user
group1_id = self.partition_service.get_user_group_id_for_partition(self.user, user_partition_id)
assert group1_id == groups[0].id

# switch to the second group and verify that it is returned for the user
self.user_partition.scheme.current_group = groups[1]
group2_id = self.partition_service.get_user_group_id_for_partition(self.user, user_partition_id)
assert group2_id == groups[1].id

def test_caching(self):
username = "psvc_cache_user"
user_partition_id = self.user_partition.id
Expand Down
57 changes: 38 additions & 19 deletions xmodule/tests/test_capa_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -3290,7 +3290,7 @@ def test_response_types_ignores_non_response_tags(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['multiplechoiceresponse'],
'content': {'display_name': name, 'capa_content': ' Label Some comment Apple Banana Chocolate Donut '}}
'content': {'display_name': name, 'capa_content': 'Label Some comment Apple Banana Chocolate Donut'}}

def test_response_types_multiple_tags(self):
xml = textwrap.dedent("""
Expand Down Expand Up @@ -3328,7 +3328,7 @@ def test_response_types_multiple_tags(self):
'problem_types': {"optionresponse", "multiplechoiceresponse"},
'content': {
'display_name': name,
'capa_content': " Label Some comment Donut Buggy '1','2' "
'capa_content': "Label Some comment Donut Buggy '1','2'"
},
}
)
Expand Down Expand Up @@ -3369,7 +3369,7 @@ def test_solutions_not_indexed(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': [],
'content': {'display_name': name, 'capa_content': ' '}}
'content': {'display_name': name, 'capa_content': ''}}

def test_indexing_checkboxes(self):
name = "Checkboxes"
Expand All @@ -3390,7 +3390,7 @@ def test_indexing_checkboxes(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['choiceresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_dropdown(self):
name = "Dropdown"
Expand All @@ -3405,7 +3405,7 @@ def test_indexing_dropdown(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['optionresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_multiple_choice(self):
name = "Multiple Choice"
Expand All @@ -3424,7 +3424,7 @@ def test_indexing_multiple_choice(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['multiplechoiceresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_numerical_input(self):
name = "Numerical Input"
Expand All @@ -3446,7 +3446,7 @@ def test_indexing_numerical_input(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['numericalresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_text_input(self):
name = "Text Input"
Expand All @@ -3465,7 +3465,7 @@ def test_indexing_text_input(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['stringresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_non_latin_problem(self):
sample_text_input_problem_xml = textwrap.dedent("""
Expand All @@ -3476,7 +3476,7 @@ def test_indexing_non_latin_problem(self):
""")
name = "Non latin Input"
block = self._create_block(sample_text_input_problem_xml, name=name)
capa_content = " Δοκιμή με μεταβλητές με Ελληνικούς χαρακτήρες μέσα σε python: $FX1_VAL "
capa_content = "Δοκιμή με μεταβλητές με Ελληνικούς χαρακτήρες μέσα σε python: $FX1_VAL"

block_dict = block.index_dictionary()
assert block_dict['content']['capa_content'] == smart_str(capa_content)
Expand All @@ -3503,7 +3503,7 @@ def test_indexing_checkboxes_with_hints_and_feedback(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['choiceresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_dropdown_with_hints_and_feedback(self):
name = "Dropdown with Hints and Feedback"
Expand All @@ -3523,7 +3523,7 @@ def test_indexing_dropdown_with_hints_and_feedback(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['optionresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_multiple_choice_with_hints_and_feedback(self):
name = "Multiple Choice with Hints and Feedback"
Expand All @@ -3543,7 +3543,7 @@ def test_indexing_multiple_choice_with_hints_and_feedback(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['multiplechoiceresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_numerical_input_with_hints_and_feedback(self):
name = "Numerical Input with Hints and Feedback"
Expand All @@ -3561,7 +3561,7 @@ def test_indexing_numerical_input_with_hints_and_feedback(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['numericalresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_text_input_with_hints_and_feedback(self):
name = "Text Input with Hints and Feedback"
Expand All @@ -3579,7 +3579,7 @@ def test_indexing_text_input_with_hints_and_feedback(self):
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['stringresponse'],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ').strip()}}

def test_indexing_problem_with_html_tags(self):
sample_problem_xml = textwrap.dedent("""
Expand All @@ -3598,14 +3598,33 @@ def test_indexing_problem_with_html_tags(self):
""")
name = "Mixed business"
block = self._create_block(sample_problem_xml, name=name)
capa_content = textwrap.dedent("""
This has HTML comment in it.
HTML end.
""")
capa_content = "This has HTML comment in it. HTML end."
assert block.index_dictionary() ==\
{'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': [],
'content': {'display_name': name, 'capa_content': capa_content.replace('\n', ' ')}}
'content': {'display_name': name, 'capa_content': capa_content}}

def test_indexing_problem_with_no_whitespace_between_tags(self):
"""
The new (MFE) visual editor for capa problems renders the OLX without spaces between the tags.
We want to make sure the index description is still readable and has whitespace.
"""
sample_problem_xml = (
"<problem display_name=\"No spaces\">"
"<choiceresponse><div>Question text here.</div><checkboxgroup>"
"<choice correct=\"true\"><div>Option A</div></choice>"
"<choice correct=\"false\"><div>Option B</div></choice>"
"</checkboxgroup></choiceresponse>"
"</problem>"
)
name = "No spaces"
block = self._create_block(sample_problem_xml, name=name)
capa_content = "Question text here. Option A Option B"
assert block.index_dictionary() == {
'content_type': ProblemBlock.INDEX_CONTENT_TYPE,
'problem_types': ['choiceresponse'],
'content': {'display_name': name, 'capa_content': capa_content},
}

def test_invalid_xml_handling(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion xmodule/video_block/transcripts_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ def get_transcript_from_learning_core(video_block, language, output_format, tran
"""
# TODO: Update to use Learning Core data models once static assets support
# has been added.
raise NotImplementedError("Transcripts not supported.")
raise NotFoundError("No transcript - transcripts not supported yet by learning core components.")


def get_transcript(video, lang=None, output_format=Transcript.SRT, youtube_id=None):
Expand Down
18 changes: 12 additions & 6 deletions xmodule/video_block/video_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ def get_html(self, view=STUDENT_VIEW, context=None): # lint-amnesty, pylint: di
'hide_downloads': is_public_view or is_embed,
'id': self.location.html_id(),
'block_id': str(self.location),
'course_id': str(self.location.course_key),
'course_id': str(self.context_key),
'video_id': str(self.edx_video_id),
'user_id': self.get_user_id(),
'is_embed': is_embed,
Expand Down Expand Up @@ -510,8 +510,10 @@ def get_course_video_sharing_override(self):
"""
Return course video sharing options override or None
"""
if not self.context_key.is_course:
return False # Only courses support this feature at all (not libraries)
try:
course = get_course_by_id(self.course_id)
course = get_course_by_id(self.context_key)
return getattr(course, 'video_sharing_options', None)

# In case the course / modulestore does something weird
Expand All @@ -523,11 +525,13 @@ def is_public_sharing_enabled(self):
"""
Is public sharing enabled for this video?
"""
if not self.context_key.is_course:
return False # Only courses support this feature at all (not libraries)
try:
# Video share feature must be enabled for sharing settings to take effect
feature_enabled = PUBLIC_VIDEO_SHARE.is_enabled(self.location.course_key)
feature_enabled = PUBLIC_VIDEO_SHARE.is_enabled(self.context_key)
except Exception as err: # pylint: disable=broad-except
log.exception(f"Error retrieving course for course ID: {self.location.course_key}")
log.exception(f"Error retrieving course for course ID: {self.context_key}")
return False
if not feature_enabled:
return False
Expand All @@ -552,11 +556,13 @@ def is_transcript_feedback_enabled(self):
"""
Is transcript feedback enabled for this video?
"""
if not self.context_key.is_course:
return False # Only courses support this feature at all (not libraries)
try:
# Video transcript feedback must be enabled in order to show the widget
feature_enabled = TRANSCRIPT_FEEDBACK.is_enabled(self.location.course_key)
feature_enabled = TRANSCRIPT_FEEDBACK.is_enabled(self.context_key)
except Exception as err: # pylint: disable=broad-except
log.exception(f"Error retrieving course for course ID: {self.location.course_key}")
log.exception(f"Error retrieving course for course ID: {self.context_key}")
return False
return feature_enabled

Expand Down

0 comments on commit e39e4d6

Please sign in to comment.