From 86d9b08b5d652333a446f5eeb0080d4158fca4e0 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Date: Thu, 23 Oct 2025 19:48:39 +0500 Subject: [PATCH] feat: remove last cs_comments service references (#37503) This commit removes all remaining references to cs_comments_service except the ForumsConfig model. The only purpose of keeping the model and table around is so that the webapp processes don't start throwing errors during deployment because they're running the old code for a few minutes after the database migration has run. We can drop ForumsConfig and add the drop-table migration after Ulmo is cut. Also bumps the openedx-forum version to 0.3.7 --------- Co-authored-by: Taimoor Ahmed --- .../student/tests/test_admin_views.py | 2 +- .../course_goals/tests/test_user_activity.py | 5 - .../django_comment_client/base/tests_v2.py | 18 ++- .../django_comment_client/tests/test_utils.py | 31 ------ .../django_comment_client/tests/utils.py | 16 +-- lms/djangoapps/discussion/plugins.py | 2 +- .../discussion/rest_api/tests/test_api.py | 3 +- .../discussion/rest_api/tests/test_api_v2.py | 20 +--- .../rest_api/tests/test_serializers_v2.py | 6 +- .../discussion/rest_api/tests/test_utils.py | 3 +- .../discussion/rest_api/tests/test_views.py | 6 +- .../rest_api/tests/test_views_v2.py | 7 +- .../discussion/tests/test_tasks_v2.py | 5 - lms/djangoapps/discussion/tests/test_views.py | 9 +- .../discussion/tests/test_views_v2.py | 31 +++--- lms/urls.py | 2 - .../core/djangoapps/discussions/README.rst | 9 +- .../djangoapps/django_comment_common/admin.py | 10 -- .../comment_client/models.py | 21 +--- .../comment_client/user.py | 2 +- .../comment_client/utils.py | 105 +----------------- 21 files changed, 46 insertions(+), 267 deletions(-) delete mode 100644 openedx/core/djangoapps/django_comment_common/admin.py diff --git a/common/djangoapps/student/tests/test_admin_views.py b/common/djangoapps/student/tests/test_admin_views.py index 968ba330a9..d001befe3b 100644 --- a/common/djangoapps/student/tests/test_admin_views.py +++ b/common/djangoapps/student/tests/test_admin_views.py @@ -200,7 +200,7 @@ class AdminUserPageTest(TestCase): Changing the username is still possible using the database or from the model directly. - However, changing the username might cause issues with the logs and/or the cs_comments_service since it + However, changing the username might cause issues with the logs and/or the forum service since it stores the username in a different database. """ request = Mock() diff --git a/lms/djangoapps/course_goals/tests/test_user_activity.py b/lms/djangoapps/course_goals/tests/test_user_activity.py index 04eb267152..285c538862 100644 --- a/lms/djangoapps/course_goals/tests/test_user_activity.py +++ b/lms/djangoapps/course_goals/tests/test_user_activity.py @@ -20,7 +20,6 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.util.testing import UrlResetMixin from lms.djangoapps.course_goals.models import UserActivity -from openedx.core.djangoapps.django_comment_common.models import ForumsConfig from openedx.features.course_experience import ENABLE_COURSE_GOALS User = get_user_model() @@ -53,10 +52,6 @@ class UserActivityTests(UrlResetMixin, ModuleStoreTestCase): self.request = RequestFactory().get('foo') self.request.user = self.user - config = ForumsConfig.current() - config.enabled = True - config.save() - def test_mfe_tabs_call_user_activity(self): ''' New style tabs call one of two metadata endpoints diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests_v2.py b/lms/djangoapps/discussion/django_comment_client/base/tests_v2.py index 9a5384c28c..7bc84e5038 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests_v2.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests_v2.py @@ -48,7 +48,6 @@ from lms.djangoapps.discussion.django_comment_client.tests.unicode import ( ) from lms.djangoapps.discussion.django_comment_client.tests.utils import ( CohortedTestCase, - ForumsEnableMixin, ) from lms.djangoapps.teams.tests.factories import ( CourseTeamFactory, @@ -397,7 +396,6 @@ class ViewsTestCaseMixin: @disable_signal(views, "comment_flagged") @disable_signal(views, "thread_flagged") class ViewsTestCase( - ForumsEnableMixin, MockForumApiMixin, UrlResetMixin, SharedModuleStoreTestCase, @@ -1019,7 +1017,6 @@ class ViewsTestCase( @disable_signal(views, "comment_endorsed") class ViewPermissionsTestCase( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockForumApiMixin, @@ -1733,7 +1730,7 @@ TEAM_COMMENTABLE_ID = "test-team-discussion" @disable_signal(views, "comment_created") @ddt.ddt class ForumEventTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, MockForumApiMixin + SharedModuleStoreTestCase, MockForumApiMixin ): """ Forum actions are expected to launch analytics events. Test these here. @@ -2018,7 +2015,6 @@ class ForumEventTestCase( @disable_signal(views, "thread_edited") class UpdateThreadUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin, @@ -2084,7 +2080,7 @@ class UpdateThreadUnicodeTestCase( class CreateThreadUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin + SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin ): @classmethod @@ -2136,7 +2132,7 @@ class CreateThreadUnicodeTestCase( @disable_signal(views, "comment_created") class CreateCommentUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin + SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin ): @classmethod @@ -2189,7 +2185,7 @@ class CreateCommentUnicodeTestCase( @disable_signal(views, "comment_edited") class UpdateCommentUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin + SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin ): @classmethod def setUpClass(cls): # pylint: disable=super-method-not-called @@ -2236,7 +2232,7 @@ class UpdateCommentUnicodeTestCase( @disable_signal(views, "comment_created") class CreateSubCommentUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin + SharedModuleStoreTestCase, UnicodeTestMixin, MockForumApiMixin ): """ Make sure comments under a response can handle unicode. @@ -2294,7 +2290,7 @@ class CreateSubCommentUnicodeTestCase( del Thread.commentable_id -class UsersEndpointTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, MockForumApiMixin): +class UsersEndpointTestCase(SharedModuleStoreTestCase, MockForumApiMixin): @classmethod def setUpClass(cls): # pylint: disable=super-method-not-called @@ -2468,7 +2464,7 @@ def _create_and_transform_event(**kwargs): @ddt.ddt -class ForumThreadViewedEventTransformerTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): +class ForumThreadViewedEventTransformerTestCase(UrlResetMixin, ModuleStoreTestCase): """ Test that the ForumThreadViewedEventTransformer transforms events correctly and without raising exceptions. diff --git a/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py b/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py index 8800504c86..742eb23bf5 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py @@ -39,12 +39,10 @@ from openedx.core.djangoapps.discussions.utils import ( ) from openedx.core.djangoapps.django_comment_common.comment_client.utils import ( CommentClientMaintenanceError, - perform_request, ) from openedx.core.djangoapps.django_comment_common.models import ( CourseDiscussionSettings, DiscussionsIdMapping, - ForumsConfig, assign_role ) from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles @@ -1650,35 +1648,6 @@ class GroupModeratorPermissionsTestCase(ModuleStoreTestCase): 'can_report': True} -class ClientConfigurationTestCase(TestCase): - """Simple test cases to ensure enabling/disabling the use of the comment service works as intended.""" - - def test_disabled(self): - """Ensures that an exception is raised when forums are disabled.""" - config = ForumsConfig.current() - config.enabled = False - config.save() - - with pytest.raises(CommentClientMaintenanceError): - perform_request('GET', 'http://www.google.com') - - @patch('requests.request') - def test_enabled(self, mock_request): - """Ensures that requests proceed normally when forums are enabled.""" - config = ForumsConfig.current() - config.enabled = True - config.save() - - response = Mock() - response.status_code = 200 - response.json = lambda: {} - - mock_request.return_value = response - - result = perform_request('GET', 'http://www.google.com') - assert result == {} - - def set_discussion_division_settings( course_key, enable_cohorts=False, always_divide_inline_discussions=False, divided_discussions=[], division_scheme=CourseDiscussionSettings.COHORT diff --git a/lms/djangoapps/discussion/django_comment_client/tests/utils.py b/lms/djangoapps/discussion/django_comment_client/tests/utils.py index 4f5fa72ef3..bc3fdffa11 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/utils.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/utils.py @@ -13,24 +13,12 @@ from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from common.djangoapps.util.testing import UrlResetMixin from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory -from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, ForumsConfig, Role +from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles from openedx.core.lib.teams_config import TeamsConfig -class ForumsEnableMixin: - """ - Ensures that the forums are enabled for a given test class. - """ - def setUp(self): - super().setUp() - - config = ForumsConfig.current() - config.enabled = True - config.save() - - -class CohortedTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase): +class CohortedTestCase(UrlResetMixin, SharedModuleStoreTestCase): """ Sets up a course with a student, a moderator and their cohorts. """ diff --git a/lms/djangoapps/discussion/plugins.py b/lms/djangoapps/discussion/plugins.py index 54898d51c3..e7edbf6f4a 100644 --- a/lms/djangoapps/discussion/plugins.py +++ b/lms/djangoapps/discussion/plugins.py @@ -20,7 +20,7 @@ from openedx.features.lti_course_tab.tab import DiscussionLtiCourseTab class DiscussionTab(TabFragmentViewMixin, EnrolledTab): """ - A tab for the cs_comments_service forums. + A tab for the forums. """ type = 'discussion' diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 116a6f6013..480568d115 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -16,7 +16,6 @@ from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import ( UserFactory ) -from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin from lms.djangoapps.discussion.rest_api.api import get_user_comments from lms.djangoapps.discussion.rest_api.tests.utils import ( ForumMockUtilsMixin, @@ -29,7 +28,7 @@ User = get_user_model() @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class GetUserCommentsTest(ForumsEnableMixin, ForumMockUtilsMixin, SharedModuleStoreTestCase): +class GetUserCommentsTest(ForumMockUtilsMixin, SharedModuleStoreTestCase): """ Tests for get_user_comments. """ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py index 8a9ecbb5e1..900d52017c 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api_v2.py @@ -43,9 +43,6 @@ from common.djangoapps.student.tests.factories import ( ) from common.djangoapps.util.testing import UrlResetMixin from common.test.utils import MockSignalHandlerMixin, disable_signal -from lms.djangoapps.discussion.django_comment_client.tests.utils import ( - ForumsEnableMixin, -) from lms.djangoapps.discussion.tests.utils import ( make_minimal_cs_comment, make_minimal_cs_thread, @@ -183,7 +180,6 @@ def _set_course_discussion_blackout(course, user_id): @disable_signal(api, "thread_voted") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class CreateThreadTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockSignalHandlerMixin, @@ -601,7 +597,6 @@ class CreateThreadTest( new=mock.Mock(), ) class CreateCommentTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockSignalHandlerMixin, @@ -1039,7 +1034,6 @@ class CreateCommentTest( @disable_signal(api, "thread_voted") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class UpdateThreadTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockSignalHandlerMixin, @@ -1699,7 +1693,6 @@ class UpdateThreadTest( @disable_signal(api, "comment_voted") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class UpdateCommentTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockSignalHandlerMixin, @@ -2303,7 +2296,6 @@ class UpdateCommentTest( @disable_signal(api, "thread_deleted") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class DeleteThreadTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockSignalHandlerMixin, @@ -2482,7 +2474,6 @@ class DeleteThreadTest( @disable_signal(api, "comment_deleted") @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class DeleteCommentTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, MockSignalHandlerMixin, @@ -2672,7 +2663,6 @@ class DeleteCommentTest( @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class RetrieveThreadTest( - ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase, ForumMockUtilsMixin, @@ -2829,7 +2819,7 @@ class RetrieveThreadTest( @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class GetThreadListTest( - ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin, SharedModuleStoreTestCase + ForumMockUtilsMixin, UrlResetMixin, SharedModuleStoreTestCase ): """Test for get_thread_list""" @@ -3464,7 +3454,7 @@ class GetThreadListTest( @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class GetCommentListTest( - ForumsEnableMixin, SharedModuleStoreTestCase, ForumMockUtilsMixin + SharedModuleStoreTestCase, ForumMockUtilsMixin ): """Test for get_comment_list""" @@ -4235,7 +4225,7 @@ class CourseTopicsV2Test(ModuleStoreTestCase): @mock.patch.dict("django.conf.settings.FEATURES", {"DISABLE_START_DATES": False}) @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class GetCourseTopicsTest(ForumMockUtilsMixin, ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): +class GetCourseTopicsTest(ForumMockUtilsMixin, UrlResetMixin, ModuleStoreTestCase): """Test for get_course_topics""" @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) def setUp(self): @@ -4673,7 +4663,7 @@ class GetCourseTopicsTest(ForumMockUtilsMixin, ForumsEnableMixin, UrlResetMixin, @override_settings(DISCUSSION_MODERATION_EDIT_REASON_CODES={"test-edit-reason": "Test Edit Reason"}) @override_settings(DISCUSSION_MODERATION_CLOSE_REASON_CODES={"test-close-reason": "Test Close Reason"}) @ddt.ddt -class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase): +class GetCourseTest(UrlResetMixin, SharedModuleStoreTestCase): """Test for get_course""" @classmethod def setUpClass(cls): @@ -4754,7 +4744,7 @@ class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase) @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class GetCourseTestBlackouts(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): +class GetCourseTestBlackouts(UrlResetMixin, ModuleStoreTestCase): """ Tests of get_course for courses that have blackout dates. """ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py index 563c5e80c8..e45e66280c 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers_v2.py @@ -16,7 +16,6 @@ from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.util.testing import UrlResetMixin -from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin from lms.djangoapps.discussion.rest_api.serializers import CommentSerializer, ThreadSerializer, get_context from lms.djangoapps.discussion.rest_api.tests.utils import ( ForumMockUtilsMixin, @@ -36,7 +35,7 @@ from openedx.core.djangoapps.django_comment_common.models import ( @ddt.ddt -class CommentSerializerDeserializationTest(ForumsEnableMixin, ForumMockUtilsMixin, SharedModuleStoreTestCase): +class CommentSerializerDeserializationTest(ForumMockUtilsMixin, SharedModuleStoreTestCase): """Tests for ThreadSerializer deserialization.""" @classmethod def setUpClass(cls): @@ -387,7 +386,6 @@ class CommentSerializerDeserializationTest(ForumsEnableMixin, ForumMockUtilsMixi @ddt.ddt class ThreadSerializerDeserializationTest( - ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin, SharedModuleStoreTestCase @@ -535,7 +533,7 @@ class ThreadSerializerDeserializationTest( @ddt.ddt -class SerializerTestMixin(ForumsEnableMixin, UrlResetMixin, ForumMockUtilsMixin): +class SerializerTestMixin(UrlResetMixin, ForumMockUtilsMixin): """ Test Mixin for Serializer tests """ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_utils.py b/lms/djangoapps/discussion/rest_api/tests/test_utils.py index 49c1e3889d..c6af74d83d 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_utils.py @@ -11,7 +11,6 @@ from pytz import UTC from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory -from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin from lms.djangoapps.discussion.rest_api.tests.utils import CommentsServiceMockMixin from lms.djangoapps.discussion.rest_api.utils import ( discussion_open_for_user, @@ -182,7 +181,7 @@ class TestRemoveEmptySequentials(unittest.TestCase): @ddt.ddt -class TestBlackoutDates(ForumsEnableMixin, CommentsServiceMockMixin, ModuleStoreTestCase): +class TestBlackoutDates(CommentsServiceMockMixin, ModuleStoreTestCase): """ Test for the is_posting_allowed function """ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 18a180b43a..619f5c2e7d 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -21,9 +21,6 @@ from common.djangoapps.student.tests.factories import ( UserFactory ) from common.djangoapps.util.testing import UrlResetMixin -from lms.djangoapps.discussion.django_comment_client.tests.utils import ( - ForumsEnableMixin, -) from lms.djangoapps.discussion.rest_api.tests.utils import ( ForumMockUtilsMixin, make_minimal_cs_comment, @@ -34,7 +31,6 @@ from lms.djangoapps.discussion.rest_api.tests.utils import ( @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class CommentViewSetListByUserTest( - ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin, ModuleStoreTestCase, @@ -70,7 +66,7 @@ class CommentViewSetListByUserTest( def register_mock_endpoints(self): """ - Register cs_comments_service mocks for sample threads and comments. + Register forum service mocks for sample threads and comments. """ self.register_get_threads_response( threads=[ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py b/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py index 40b2ca9154..10251224ad 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views_v2.py @@ -25,7 +25,6 @@ from rest_framework.parsers import JSONParser from rest_framework.test import APIClient, APITestCase from lms.djangoapps.discussion.django_comment_client.tests.utils import ( - ForumsEnableMixin, config_course_discussions, topic_name_to_id, ) @@ -72,7 +71,7 @@ from openedx.core.djangoapps.discussions.config.waffle import ENABLE_NEW_STRUCTU from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage -class DiscussionAPIViewTestMixin(ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin): +class DiscussionAPIViewTestMixin(ForumMockUtilsMixin, UrlResetMixin): """ Mixin for common code in tests of Discussion API views. This includes creation of common structures (e.g. a course, user, and enrollment), logging @@ -1814,7 +1813,7 @@ class LearnerThreadViewAPITest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): @ddt.ddt @httpretty.activate @override_waffle_flag(ENABLE_DISCUSSIONS_MFE, True) -class CourseActivityStatsTest(ForumsEnableMixin, UrlResetMixin, ForumMockUtilsMixin, APITestCase, +class CourseActivityStatsTest(UrlResetMixin, ForumMockUtilsMixin, APITestCase, SharedModuleStoreTestCase): """ Tests for the course stats endpoint @@ -2028,7 +2027,7 @@ class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -class UploadFileViewTest(ForumsEnableMixin, ForumMockUtilsMixin, UrlResetMixin, ModuleStoreTestCase): +class UploadFileViewTest(ForumMockUtilsMixin, UrlResetMixin, ModuleStoreTestCase): """ Tests for UploadFileView. """ diff --git a/lms/djangoapps/discussion/tests/test_tasks_v2.py b/lms/djangoapps/discussion/tests/test_tasks_v2.py index eed2c36f3d..d5b742dc85 100644 --- a/lms/djangoapps/discussion/tests/test_tasks_v2.py +++ b/lms/djangoapps/discussion/tests/test_tasks_v2.py @@ -36,7 +36,6 @@ from openedx.core.djangoapps.ace_common.template_context import ( from openedx.core.djangoapps.content.course_overviews.tests.factories import ( CourseOverviewFactory, ) -from openedx.core.djangoapps.django_comment_common.models import ForumsConfig from openedx.core.djangoapps.django_comment_common.signals import comment_created from openedx.core.djangoapps.site_configuration.tests.factories import ( SiteConfigurationFactory, @@ -102,10 +101,6 @@ class TaskTestCase( CourseEnrollmentFactory(user=cls.thread_author, course_id=cls.course.id) CourseEnrollmentFactory(user=cls.comment_author, course_id=cls.course.id) - config = ForumsConfig.current() - config.enabled = True - config.save() - cls.create_threads_and_comments() @classmethod diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 5f4953d1e6..7f575b454f 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -31,7 +31,6 @@ from lms.djangoapps.discussion import views from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from lms.djangoapps.discussion.django_comment_client.permissions import get_team from lms.djangoapps.discussion.django_comment_client.tests.utils import ( - ForumsEnableMixin, config_course_discussions, topic_name_to_id ) @@ -41,7 +40,6 @@ from openedx.core.djangoapps.course_groups.tests.test_views import CohortViewsTe from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientPaginatedResult from openedx.core.djangoapps.django_comment_common.models import ( CourseDiscussionSettings, - ForumsConfig ) from openedx.core.djangoapps.django_comment_common.utils import ThreadContext from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES @@ -82,9 +80,6 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase): # lint-amnest self.client = Client() assert self.client.login(username=uname, password=password) - config = ForumsConfig.current() - config.enabled = True - config.save() patcher = mock.patch( "openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api.get_course_id_by_thread" ) @@ -317,7 +312,7 @@ class AllowPlusOrMinusOneInt(int): @patch('requests.request', autospec=True) -class CommentsServiceRequestHeadersTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring +class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring CREATE_USER = False @@ -391,7 +386,7 @@ class CommentsServiceRequestHeadersTestCase(ForumsEnableMixin, UrlResetMixin, Mo self.assert_all_calls_have_header(mock_request, "X-Edx-Api-Key", "test_api_key") -class EnrollmentTestCase(ForumsEnableMixin, ModuleStoreTestCase): +class EnrollmentTestCase(ModuleStoreTestCase): """ Tests for the behavior of views depending on if the student is enrolled in the course diff --git a/lms/djangoapps/discussion/tests/test_views_v2.py b/lms/djangoapps/discussion/tests/test_views_v2.py index 1e4f36b8f5..0f268e93eb 100644 --- a/lms/djangoapps/discussion/tests/test_views_v2.py +++ b/lms/djangoapps/discussion/tests/test_views_v2.py @@ -61,7 +61,6 @@ from lms.djangoapps.discussion.django_comment_client.tests.unicode import ( ) from lms.djangoapps.discussion.django_comment_client.tests.utils import ( CohortedTestCase, - ForumsEnableMixin, config_course_discussions, topic_name_to_id, ) @@ -84,7 +83,6 @@ from openedx.core.djangoapps.django_comment_common.comment_client.utils import ( from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_STUDENT, CourseDiscussionSettings, - ForumsConfig, ) from openedx.core.djangoapps.django_comment_common.utils import ( ThreadContext, @@ -317,7 +315,7 @@ class ForumViewsUtilsMixin(MockForumApiMixin): self.set_mock_side_effect("get_user", make_user_callback()) -class SingleThreadTestCase(ForumsEnableMixin, ModuleStoreTestCase, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring +class SingleThreadTestCase(ModuleStoreTestCase, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring CREATE_USER = False @@ -727,7 +725,7 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, GroupIdAssertionMixinV2, For ) -class SingleThreadContentGroupTestCase(ForumsEnableMixin, UrlResetMixin, ContentGroupTestCase, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring +class SingleThreadContentGroupTestCase(UrlResetMixin, ContentGroupTestCase, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) def setUp(self): @@ -897,7 +895,7 @@ class FollowedThreadsDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGr ) -class SingleThreadUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring +class SingleThreadUnicodeTestCase(SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring @classmethod def setUpClass(cls): # pylint: disable=super-method-not-called @@ -934,7 +932,7 @@ class SingleThreadUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, class ForumFormDiscussionContentGroupTestCase( - ForumsEnableMixin, ContentGroupTestCase, ForumViewsUtilsMixin + ContentGroupTestCase, ForumViewsUtilsMixin ): """ Tests `forum_form_discussion api` works with different content groups. @@ -1030,7 +1028,6 @@ class ForumFormDiscussionContentGroupTestCase( class ForumFormDiscussionUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin, @@ -1076,7 +1073,6 @@ class ForumFormDiscussionUnicodeTestCase( class EnterpriseConsentTestCase( EnterpriseTestConsentRequired, - ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin, @@ -1198,7 +1194,7 @@ class InlineDiscussionGroupIdTestCase( # lint-amnesty, pylint: disable=missing- class InlineDiscussionContextTestCase( - ForumsEnableMixin, ModuleStoreTestCase, ForumViewsUtilsMixin + ModuleStoreTestCase, ForumViewsUtilsMixin ): # lint-amnesty, pylint: disable=missing-class-docstring def setUp(self): @@ -1455,7 +1451,7 @@ class UserProfileDiscussionGroupIdTestCase( @ddt.ddt class ForumDiscussionXSSTestCase( - ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin + UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin ): # lint-amnesty, pylint: disable=missing-class-docstring @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) @@ -1535,7 +1531,7 @@ class ForumDiscussionXSSTestCase( class InlineDiscussionTestCase( - ForumsEnableMixin, ModuleStoreTestCase, ForumViewsUtilsMixin + ModuleStoreTestCase, ForumViewsUtilsMixin ): # lint-amnesty, pylint: disable=missing-class-docstring def setUp(self): @@ -1610,7 +1606,7 @@ class InlineDiscussionTestCase( class ForumDiscussionSearchUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin + SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin ): # lint-amnesty, pylint: disable=missing-class-docstring @classmethod @@ -1652,7 +1648,7 @@ class ForumDiscussionSearchUnicodeTestCase( class InlineDiscussionUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin + SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin ): # lint-amnesty, pylint: disable=missing-class-docstring @classmethod @@ -1743,7 +1739,7 @@ class ForumFormDiscussionGroupIdTestCase( class UserProfileTestCase( - ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin + UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin ): # lint-amnesty, pylint: disable=missing-class-docstring TEST_THREAD_TEXT = "userprofile-test-text" @@ -1882,7 +1878,7 @@ class UserProfileTestCase( class ThreadViewedEventTestCase( - EventTestMixin, ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin, + EventTestMixin, UrlResetMixin, ModuleStoreTestCase, ForumViewsUtilsMixin, ): """ Forum thread views are expected to launch analytics events. Test these here. @@ -1979,7 +1975,6 @@ class ThreadViewedEventTestCase( class FollowedThreadsUnicodeTestCase( - ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin @@ -2024,7 +2019,7 @@ class FollowedThreadsUnicodeTestCase( assert response_data['discussion_data'][0]['body'] == text -class UserProfileUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring +class UserProfileUnicodeTestCase(SharedModuleStoreTestCase, UnicodeTestMixin, ForumViewsUtilsMixin): # lint-amnesty, pylint: disable=missing-class-docstring @classmethod def setUpClass(cls): @@ -2059,7 +2054,7 @@ class UserProfileUnicodeTestCase(ForumsEnableMixin, SharedModuleStoreTestCase, U assert response_data['discussion_data'][0]['body'] == text -class ForumMFETestCase(ForumsEnableMixin, SharedModuleStoreTestCase, ModuleStoreTestCase, MockForumApiMixin): # lint-amnesty, pylint: disable=missing-class-docstring +class ForumMFETestCase(SharedModuleStoreTestCase, ModuleStoreTestCase, MockForumApiMixin): # lint-amnesty, pylint: disable=missing-class-docstring """ Tests that the MFE upgrade banner and MFE is shown in the correct situation with the correct UI """ diff --git a/lms/urls.py b/lms/urls.py index 092dcb6be9..490f4727e0 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -41,7 +41,6 @@ from openedx.core.djangoapps.common_views.xblock import xblock_resource from openedx.core.djangoapps.cors_csrf import views as cors_csrf_views from openedx.core.djangoapps.course_groups import views as course_groups_views from openedx.core.djangoapps.debug import views as openedx_debug_views -from openedx.core.djangoapps.django_comment_common.models import ForumsConfig from openedx.core.djangoapps.lang_pref import views as lang_pref_views from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance from openedx.core.djangoapps.password_policy.forms import PasswordPolicyAwareAdminAuthForm @@ -917,7 +916,6 @@ if settings.FEATURES.get('ENABLE_LTI_PROVIDER'): urlpatterns += [ path('config/programs', ConfigurationModelCurrentAPIView.as_view(model=ProgramsApiConfig)), path('config/catalog', ConfigurationModelCurrentAPIView.as_view(model=CatalogIntegration)), - path('config/forums', ConfigurationModelCurrentAPIView.as_view(model=ForumsConfig)), ] if settings.DEBUG: diff --git a/openedx/core/djangoapps/discussions/README.rst b/openedx/core/djangoapps/discussions/README.rst index 5c1127d324..d8a937a07e 100644 --- a/openedx/core/djangoapps/discussions/README.rst +++ b/openedx/core/djangoapps/discussions/README.rst @@ -3,7 +3,7 @@ Discussions This Discussions app is responsible for providing support for configuring discussion tools in the Open edX platform. This includes the in-built forum -tool that uses the `cs_comments_service`, but also other LTI-based tools. +tool, but also other LTI-based tools. Technical Overview @@ -44,10 +44,9 @@ discussion configuration information such as the course key, the provider type, whether in-context discussions are enabled, whether graded units are enabled, when unit level visibility is enabled. Other plugin configuration and a list of discussion contexts for which discussions are enabled. Each discussion -context has a usage key, a title (the units name) an external id -(the cs_comments_service id), it's ordering in the course, and additional -context. It then sends its own signal that has the discussion configuration -object attached. +context has a usage key, a title (the units name) an external id, +its ordering in the course, and additional context. It then sends its own +signal that has the discussion configuration 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 diff --git a/openedx/core/djangoapps/django_comment_common/admin.py b/openedx/core/djangoapps/django_comment_common/admin.py deleted file mode 100644 index 6d2d7a34d4..0000000000 --- a/openedx/core/djangoapps/django_comment_common/admin.py +++ /dev/null @@ -1,10 +0,0 @@ -""" -Admin for managing the connection to the Forums backend service. -""" - - -from django.contrib import admin - -from .models import ForumsConfig - -admin.site.register(ForumsConfig) 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 cbb6b25071..e788ac5e9e 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/models.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/models.py @@ -3,7 +3,7 @@ import logging -from .utils import CommentClientRequestError, extract, perform_request, get_course_key +from .utils import CommentClientRequestError, extract, get_course_key from forum import api as forum_api log = logging.getLogger(__name__) @@ -101,25 +101,6 @@ class Model: def find(cls, id): # pylint: disable=redefined-builtin return cls(id=id) - @classmethod - def retrieve_all(cls, params=None): - """ - Performs a GET request against the resource's listing endpoint. - - Arguments: - params: A dictionary of parameters to be passed as the request's query string. - - Returns: - The parsed JSON response from the backend. - """ - return perform_request( - 'get', - cls.url(action='get_all'), - params, - metric_tags=[f'model_class:{cls.__name__}'], - metric_action='model.retrieve_all', - ) - def _update_from_response(self, response_data): for k, v in response_data.items(): if k in self.accessible_fields: diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/user.py b/openedx/core/djangoapps/django_comment_common/comment_client/user.py index 0bb49c2d53..bd208545ce 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/user.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/user.py @@ -33,7 +33,7 @@ class User(models.Model): def read(self, source): """ - Calls cs_comments_service to mark thread as read for the user + Calls forum service to mark thread as read for the user """ course_id = self.attributes.get("course_id") course_key = utils.get_course_key(course_id) diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py index 26625ed3a7..ccdced767e 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py @@ -3,14 +3,10 @@ import logging -from uuid import uuid4 -import requests -from django.utils.translation import get_language +import requests # pylint: disable=unused-import from opaque_keys.edx.keys import CourseKey -from .settings import SERVICE_HOST as COMMENTS_SERVICE - log = logging.getLogger(__name__) @@ -31,78 +27,6 @@ def extract(dic, keys): return strip_none({k: dic.get(k) for k in keys}) -def perform_request(method, url, data_or_params=None, raw=False, - metric_action=None, metric_tags=None, paged_results=False): - # To avoid dependency conflict - from openedx.core.djangoapps.django_comment_common.models import ForumsConfig - config = ForumsConfig.current() - - if not config.enabled: - raise CommentClientMaintenanceError('service disabled') - - if metric_tags is None: - metric_tags = [] - - metric_tags.append(f'method:{method}') - if metric_action: - metric_tags.append(f'action:{metric_action}') - - if data_or_params is None: - data_or_params = {} - headers = { - 'X-Edx-Api-Key': config.api_key, - 'Accept-Language': get_language(), - } - request_id = uuid4() - request_id_dict = {'request_id': request_id} - - if method in ['post', 'put', 'patch']: - data = data_or_params - params = request_id_dict - else: - data = None - params = data_or_params.copy() - params.update(request_id_dict) - response = requests.request( - method, - url, - data=data, - params=params, - headers=headers, - timeout=config.connection_timeout - ) - - metric_tags.append(f'status_code:{response.status_code}') - status_code = int(response.status_code) - if status_code > 200: - metric_tags.append('result:failure') - else: - metric_tags.append('result:success') - - if 200 < status_code < 500: # lint-amnesty, pylint: disable=no-else-raise - log.info(f'Investigation Log: CommentClientRequestError for request with {method} and params {params}') - raise CommentClientRequestError(response.text, response.status_code) - # Heroku returns a 503 when an application is in maintenance mode - elif status_code == 503: - raise CommentClientMaintenanceError(response.text) - elif status_code == 500: - raise CommentClient500Error(response.text) - else: - if raw: - return response.text - else: - try: - data = response.json() - except ValueError: - raise CommentClientError( # lint-amnesty, pylint: disable=raise-missing-from - "Invalid JSON response for request {request_id}; first 100 characters: '{content}'".format( - request_id=request_id, - content=response.text[:100] - ) - ) - return data - - def clean_forum_params(params): """Convert string booleans to actual booleans and remove None values and empty lists from forum parameters.""" result = {} @@ -160,33 +84,6 @@ class SubscriptionsPaginatedResult: self.corrected_text = corrected_text -def check_forum_heartbeat(): - """ - Check the forum connection via its built-in heartbeat service and create an answer which can be used in the LMS - heartbeat django application. - This function can be connected to the LMS heartbeat checker through the HEARTBEAT_CHECKS variable. - """ - # To avoid dependency conflict - from openedx.core.djangoapps.django_comment_common.models import ForumsConfig - config = ForumsConfig.current() - - if not config.enabled: - # If this check is enabled but forums disabled, don't connect, just report no error - return 'forum', True, 'OK' - - try: - res = requests.get( - '%s/heartbeat' % COMMENTS_SERVICE, - timeout=config.connection_timeout - ).json() - if res['OK']: - return 'forum', True, 'OK' - else: - return 'forum', False, res.get('check', 'Forum heartbeat failed') - except Exception as fail: - return 'forum', False, str(fail) - - def get_course_key(course_id: CourseKey | str | None) -> CourseKey | None: """ Returns a CourseKey if the provided course_id is a valid string representation of a CourseKey.