fix: Change approach to processing course topics (#31200)

Course topics are now created by traversing the entire course structure from top to bottom instead of starting at the sequential level and then moving up or down as needed.

This also introduces a lot of debug logs to pontetially find the reason why under some circumstances new units don't get processed and end up without a discussions topic.
This commit is contained in:
Kshitij Sobti
2022-11-01 09:31:27 +00:00
committed by GitHub
parent 93aaf5ca42
commit 4fc9eb8324
3 changed files with 44 additions and 28 deletions

View File

@@ -51,7 +51,7 @@ object attached.
Finally, the handler for this discussion change signal, takes the information
from the discussion change signal and compares it to the topics in the
database, and does the following;
database, and does the following:
- If it sees discussions contexts (units) without topics it creates new topics
link entries for the new units. This will happen if you create a new unit,

View File

@@ -61,14 +61,16 @@ def update_course_discussion_config(configuration: CourseDiscussionConfiguration
lookup_key = topic_link.usage_key or topic_link.external_id
topic_context = new_topic_map.pop(lookup_key, None)
if topic_context is None:
log.info(f"[DEBUG INF-291] Unit was deleted or discussion disabled: {lookup_key}")
topic_link.enabled_in_context = False
try:
# If the section/subsection/unit a topic is in is deleted, add that context to title.
topic_link.title = "{section}|{subsection}|{unit}".format(**topic_link.context)
except KeyError:
# It's possible the context is if the topic link was created before the context field was added.
# It's possible the context is empty if the link was created before the context field was added.
pass
else:
log.info(f"[DEBUG INF-291] Unit topic already exists, will be updated: {lookup_key}")
topic_link.enabled_in_context = True
topic_link.ordering = topic_context.ordering
topic_link.title = topic_context.title
@@ -78,6 +80,9 @@ def update_course_discussion_config(configuration: CourseDiscussionConfiguration
topic_link.save()
log.info(f"Creating new discussion topic links for {course_key}")
log.info(f"[DEBUG INF-291] Discovered new units with keys: {[str(key) for key in new_topic_map.keys()]}")
log.info(f"[DEBUG INF-291] New unit names: {[topic.title for topic in new_topic_map.values()]}")
DiscussionTopicLink.objects.bulk_create([
DiscussionTopicLink(
context_key=course_key,

View File

@@ -51,37 +51,48 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc
provider_type = discussions_config.provider_type
def iter_discussable_units():
subsections = store.get_items(course_key, qualifiers={"category": "sequential"})
# Start at 99 so that the initial increment starts it at 100.
# This leaves the first 100 slots for the course wide topics, which is only a concern if there are more
# than that many.
idx = 99
for subsection in subsections:
section = store.get_item(subsection.parent)
for unit in subsection.get_children():
# Increment index even for skipped units so that the index is more stable and won't change
# if settings change, only if a unit is added or removed.
idx += 1
# If unit-level visibility is enabled and the unit doesn't have discussion enabled, skip it.
if unit_level_visibility and not getattr(unit, "discussion_enabled", False):
log.info(f"[DEBUG INF-291] Unit-level visibility enabled: {unit_level_visibility}")
for section in course.get_children():
if section.location.block_type != "chapter":
continue
for subsection in section.get_children():
if subsection.location.block_type != "sequential":
continue
# If the unit is in a graded section and graded sections aren't enabled skip it.
if subsection.graded and not enable_graded_units:
continue
# If the unit is an exam, skip it.
if subsection.is_practice_exam or subsection.is_proctored_enabled or subsection.is_time_limited:
continue
yield DiscussionTopicContext(
usage_key=unit.location,
title=unit.display_name,
group_id=None,
ordering=idx,
context={
"section": section.display_name,
"subsection": subsection.display_name,
"unit": unit.display_name,
},
)
for unit in subsection.get_children():
if unit.location.block_type != 'vertical':
continue
log.info(f"[DEBUG INF-291] Processing unit: {unit.location}")
# Increment index even for skipped units so that the index is more stable and won't change
# if settings change, only if a unit is added or removed.
idx += 1
# If unit-level visibility is enabled and the unit doesn't have discussion enabled, skip it.
if unit_level_visibility and not getattr(unit, "discussion_enabled", False):
log.info(f"[DEBUG INF-291] Skipping unit because discussion is disbled: {unit.location}")
continue
# If the unit is in a graded section and graded sections aren't enabled skip it.
if subsection.graded and not enable_graded_units:
log.info(f"[DEBUG INF-291] Skipping unit because it's in a graded subsection: {unit.location}")
continue
# If the unit is an exam, skip it.
if subsection.is_practice_exam or subsection.is_proctored_enabled or subsection.is_time_limited:
log.info(f"[DEBUG INF-291] Skipping unit because it's in an exam: {unit.location}")
continue
log.info(f"[DEBUG INF-291] Topic will be created for unit: {unit.location}")
yield DiscussionTopicContext(
usage_key=unit.location,
title=unit.display_name,
group_id=None,
ordering=idx,
context={
"section": section.display_name,
"subsection": subsection.display_name,
"unit": unit.display_name,
},
)
with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key):
course = store.get_course(course_key)