From deeecdf246dcb7542dd952ff2b3f80013f9afda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Mon, 9 Dec 2024 16:15:54 +0100 Subject: [PATCH 1/2] feat: make it possible to globally disable forum v2 with setting We introduce a setting that allows us to bypass any course waffle flag check. The advantage of such a setting is that we don't need to find the course ID: in some cases, we might not have access to the course ID, and we need to look for it... in forum v2. See discussion here: https://github.com/openedx/forum/issues/137 --- .../djangoapps/discussions/config/waffle.py | 15 ++++++- .../comment_client/models.py | 33 +++++++++++++--- .../comment_client/thread.py | 39 ++++++++++++++++--- 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/openedx/core/djangoapps/discussions/config/waffle.py b/openedx/core/djangoapps/discussions/config/waffle.py index 7b6395bb1c01..05fc24eeb3b7 100644 --- a/openedx/core/djangoapps/discussions/config/waffle.py +++ b/openedx/core/djangoapps/discussions/config/waffle.py @@ -2,6 +2,7 @@ This module contains various configuration settings via waffle switches for the discussions app. """ +from django.conf import settings from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag @@ -57,6 +58,18 @@ def is_forum_v2_enabled(course_id): """ - Returns a boolean if forum V2 is enabled on the course + Returns whether forum V2 is enabled on the course. This is a 2-step check: + + 1. Check value of settings.DISABLE_FORUM_V2: if it exists and is true, this setting overrides any course flag. + 2. Else, check the value of the corresponding course waffle flag. """ + if is_forum_v2_disabled_globally(): + return False return ENABLE_FORUM_V2.is_enabled(course_id) + + +def is_forum_v2_disabled_globally() -> bool: + """ + Return True if DISABLE_FORUM_V2 is defined and true-ish. + """ + return getattr(settings, "DISABLE_FORUM_V2", False) diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/models.py b/openedx/core/djangoapps/django_comment_common/comment_client/models.py index 31256eb64735..094475c81fb5 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/models.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/models.py @@ -2,10 +2,11 @@ import logging +import typing as t from .utils import CommentClientRequestError, extract, perform_request, get_course_key from forum import api as forum_api -from openedx.core.djangoapps.discussions.config.waffle import is_forum_v2_enabled +from openedx.core.djangoapps.discussions.config.waffle import is_forum_v2_enabled, is_forum_v2_disabled_globally log = logging.getLogger(__name__) @@ -72,13 +73,14 @@ def retrieve(self, *args, **kwargs): def _retrieve(self, *args, **kwargs): course_id = self.attributes.get("course_id") or kwargs.get("course_id") - if not course_id: - course_id = forum_api.get_course_id_by_comment(self.id) - course_key = get_course_key(course_id) + if course_id: + use_forumv2 = is_forum_v2_enabled(course_id) + else: + use_forumv2, course_id = is_forum_v2_enabled_for_comment(self.id) response = None - if is_forum_v2_enabled(course_key): + if use_forumv2: if self.type == "comment": - response = forum_api.get_parent_comment(comment_id=self.attributes["id"], course_id=str(course_key)) + response = forum_api.get_parent_comment(comment_id=self.attributes["id"], course_id=course_id) if response is None: raise CommentClientRequestError("Forum v2 API call is missing") else: @@ -369,3 +371,22 @@ def handle_create_thread(self, course_id): context=request_data.get("context", None), ) return response + + +def is_forum_v2_enabled_for_comment(comment_id: str) -> tuple[bool, t.Optional[str]]: + """ + Figure out whether we use forum v2 for a given comment. + + See is_forum_v2_enabled_for_thread. + + Return: + + enabled (bool) + course_id (str or None) + """ + if is_forum_v2_disabled_globally(): + return False, None + + course_id = forum_api.get_course_id_by_comment(comment_id) + course_key = get_course_key(course_id) + return is_forum_v2_enabled(course_key), course_id diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py index 895a28c73993..2a6bb34a80cf 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py @@ -2,12 +2,13 @@ import logging +import typing as t from eventtracking import tracker from . import models, settings, utils from forum import api as forum_api -from openedx.core.djangoapps.discussions.config.waffle import is_forum_v2_enabled +from openedx.core.djangoapps.discussions.config.waffle import is_forum_v2_enabled, is_forum_v2_disabled_globally log = logging.getLogger(__name__) @@ -172,16 +173,17 @@ def _retrieve(self, *args, **kwargs): } request_params = utils.strip_none(request_params) course_id = kwargs.get("course_id") - if not course_id: - course_id = forum_api.get_course_id_by_thread(self.id) - course_key = utils.get_course_key(course_id) - if is_forum_v2_enabled(course_key): + if course_id: + use_forumv2 = is_forum_v2_enabled(course_id) + else: + use_forumv2, course_id = is_forum_v2_enabled_for_thread(self.id) + if use_forumv2: if user_id := request_params.get('user_id'): request_params['user_id'] = str(user_id) response = forum_api.get_thread( thread_id=self.id, params=request_params, - course_id=str(course_key) + course_id=course_id, ) else: response = utils.perform_request( @@ -296,3 +298,28 @@ def _url_for_pin_thread(thread_id): def _url_for_un_pin_thread(thread_id): return f"{settings.PREFIX}/threads/{thread_id}/unpin" + + +def is_forum_v2_enabled_for_thread(thread_id: str) -> tuple[bool, t.Optional[str]]: + """ + Figure out whether we use forum v2 for a given thread. + + This is a complex affair... First, we check the value of the DISABLE_FORUM_V2 + setting, which overrides everything. If this setting does not exist, then we need to + find the course ID that corresponds to the thread ID. Then, we return the value of + the course waffle flag for this course ID. + + Note that to fetch the course ID associated to a thread ID, we need to connect both + to mongodb and mysql. As a consequence, when forum v2 needs adequate connection + strings for both backends. + + Return: + + enabled (bool) + course_id (str or None) + """ + if is_forum_v2_disabled_globally(): + return False, None + course_id = forum_api.get_course_id_by_thread(thread_id) + course_key = utils.get_course_key(course_id) + return is_forum_v2_enabled(course_key), course_id From 0b5b5113c801a511365c9de8fb32c173c4f0c93a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Tue, 10 Dec 2024 16:04:19 +0100 Subject: [PATCH 2/2] chore: bump openedx-forum to 0.1.5 This should fix an issue with index creation on edX.org. --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3767b6811acf..c2aebbb6fa31 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -836,7 +836,7 @@ openedx-filters==1.11.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # ora2 -openedx-forum==0.1.4 +openedx-forum==0.1.5 # via -r requirements/edx/kernel.in openedx-learning==0.18.1 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 7a1fe5de5104..da46c049045b 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1388,7 +1388,7 @@ openedx-filters==1.11.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # ora2 -openedx-forum==0.1.4 +openedx-forum==0.1.5 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 89ba44565989..4efcbcc5b5a0 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1003,7 +1003,7 @@ openedx-filters==1.11.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-forum==0.1.4 +openedx-forum==0.1.5 # via -r requirements/edx/base.txt openedx-learning==0.18.1 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index cbb57628e5e5..1b19a6cbf65e 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1048,7 +1048,7 @@ openedx-filters==1.11.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-forum==0.1.4 +openedx-forum==0.1.5 # via -r requirements/edx/base.txt openedx-learning==0.18.1 # via