Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
10 changes: 10 additions & 0 deletions openedx/core/djangoapps/discussions/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from openedx_events.learning.data import CourseDiscussionConfigurationData
from openedx_events.learning.signals import COURSE_DISCUSSIONS_CHANGED

from openedx.core.djangoapps.course_apps.models import CourseAppStatus
from openedx.core.djangoapps.discussions.models import (
DiscussionsConfiguration,
DiscussionTopicLink,
Expand Down Expand Up @@ -108,5 +109,14 @@ def update_course_discussion_config(configuration: CourseDiscussionConfiguration
context_key=course_key,
).update(enabled=configuration.enabled)

# Also update CourseAppStatus to keep the Pages & Resources UI in sync.
# The update_course_apps_status task may run before this handler due to
# the COURSE_PUBLISH_TASK_DELAY countdown, caching a stale enabled value.
Comment on lines +112 to +114
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have added the code here. We have 2 scenarios:

New course: No CourseAppStatus row exists yet. The handler creates one with the correct enabled value.
Old course: A CourseAppStatus row already exists (created by update_course_apps_status on a previous publish, or by initialize_course_app_status on first MFE page load). The handler updates it.

could you add an assertion for each of those scenarios into https://github.com/openedx/openedx-platform/blob/master/openedx/core/djangoapps/discussions/tests/test_handlers.py ?

CourseAppStatus.update_status_for_course_app(
course_key=course_key,
app_id="discussion",
enabled=configuration.enabled,
)


COURSE_DISCUSSIONS_CHANGED.connect(handle_course_discussion_config_update)
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""
Sync DiscussionsConfiguration.enabled and CourseAppStatus.enabled with
CourseOverviewTab.is_hidden.

When a course is imported, the discussion tab's is_hidden value is carried over
from the source course. However, DiscussionsConfiguration.enabled and
CourseAppStatus.enabled default to True and are not updated from the imported
tab state, causing a desync.

This migration reads each discussion tab from CourseOverviewTab (a DB cache of
course tabs) and sets both DiscussionsConfiguration.enabled and
CourseAppStatus.enabled to NOT is_hidden.
"""
Comment on lines +1 to +13
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you clarify that this is a backfill for existing courses, whereas the sync going forward is handled by update_course_discussion_config ?


from django.db import migrations


def sync_enabled_from_course_overview_tab(apps, schema_editor):
CourseOverviewTab = apps.get_model("course_overviews", "CourseOverviewTab")
DiscussionsConfiguration = apps.get_model("discussions", "DiscussionsConfiguration")
CourseAppStatus = apps.get_model("course_apps", "CourseAppStatus")

discussion_tabs = CourseOverviewTab.objects.filter(tab_id="discussion").select_related("course_overview")

for tab in discussion_tabs.iterator():
course_key = tab.course_overview_id
expected_enabled = not tab.is_hidden
DiscussionsConfiguration.objects.filter(
context_key=course_key,
).exclude(
enabled=expected_enabled,
).update(enabled=expected_enabled)
CourseAppStatus.objects.filter(
course_key=course_key,
app_id="discussion",
).exclude(
enabled=expected_enabled,
).update(enabled=expected_enabled)


class Migration(migrations.Migration):

dependencies = [
("discussions", "0018_auto_20230904_1054"),
("course_overviews", "0030_backfill_new_catalog_courseruns"),
("course_apps", "0002_alter_historicalcourseappstatus_options"),
]

operations = [
migrations.RunPython(sync_enabled_from_course_overview_tab, migrations.RunPython.noop),
]
Loading