Skip to content

Commit 4c05137

Browse files
feat!: remove last calls to cs_comments_service (#37376)
This removes the last remaining code that called out to the cs_comments_service. All forums backend logic now uses the v2 API from the forum repo (https://github.com/openedx/forum). This does NOT remove MongoDB support. This also implements the endpoint to retrieve all comments for a user using the new forum backend. This is not actually called from any known frontend code, but it has not been formally deprecated as an endpoint, and therefore needs to be supported. As part of the cleanup, the ENABLE_FORUM_V2 course waffle flag has also been removed, along with all remaining switching logic that used to route between the Python API in the forum repo and service calls to the cs_comments_service Ruby service. Other endpoints affected (switching logic removed): * get course commentable counts * get/update course user stats * update comment/thread/user * delete thread (implementation moved to forum repo) * follow * retire user This is part of the following overall DEPR ticket: https://github.com/openedx/cs_comments_service/issues/437
1 parent e5b497c commit 4c05137

20 files changed

Lines changed: 3687 additions & 4654 deletions

File tree

lms/djangoapps/discussion/django_comment_client/base/tests.py

Lines changed: 0 additions & 636 deletions
This file was deleted.

lms/djangoapps/discussion/django_comment_client/base/tests_v2.py

Lines changed: 410 additions & 0 deletions
Large diffs are not rendered by default.

lms/djangoapps/discussion/django_comment_client/tests/group_id.py

Lines changed: 32 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -60,38 +60,36 @@ class CohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
6060
Provides test cases to verify that views pass the correct `group_id` to
6161
the comments service when requesting content in cohorted discussions.
6262
"""
63-
def call_view(self, mock_is_forum_v2_enabled, mock_request, commentable_id, user, group_id, pass_group_id=True):
63+
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
6464
"""
6565
Call the view for the implementing test class, constructing a request
6666
from the parameters.
6767
"""
6868
pass # lint-amnesty, pylint: disable=unnecessary-pass
6969

70-
def test_cohorted_topic_student_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
71-
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.student, '', pass_group_id=False)
70+
def test_cohorted_topic_student_without_group_id(self, mock_request):
71+
self.call_view(mock_request, "cohorted_topic", self.student, '', pass_group_id=False)
7272
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)
7373

74-
def test_cohorted_topic_student_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
75-
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.student, "")
74+
def test_cohorted_topic_student_none_group_id(self, mock_request):
75+
self.call_view(mock_request, "cohorted_topic", self.student, "")
7676
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)
7777

78-
def test_cohorted_topic_student_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
79-
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.student, self.student_cohort.id)
78+
def test_cohorted_topic_student_with_own_group_id(self, mock_request):
79+
self.call_view(mock_request, "cohorted_topic", self.student, self.student_cohort.id)
8080
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)
8181

82-
def test_cohorted_topic_student_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
82+
def test_cohorted_topic_student_with_other_group_id(self, mock_request):
8383
self.call_view(
84-
mock_is_forum_v2_enabled,
8584
mock_request,
8685
"cohorted_topic",
8786
self.student,
8887
self.moderator_cohort.id
8988
)
9089
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)
9190

92-
def test_cohorted_topic_moderator_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
91+
def test_cohorted_topic_moderator_without_group_id(self, mock_request):
9392
self.call_view(
94-
mock_is_forum_v2_enabled,
9593
mock_request,
9694
"cohorted_topic",
9795
self.moderator,
@@ -100,36 +98,34 @@ def test_cohorted_topic_moderator_without_group_id(self, mock_is_forum_v2_enable
10098
)
10199
self._assert_comments_service_called_without_group_id(mock_request)
102100

103-
def test_cohorted_topic_moderator_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
104-
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.moderator, "")
101+
def test_cohorted_topic_moderator_none_group_id(self, mock_request):
102+
self.call_view(mock_request, "cohorted_topic", self.moderator, "")
105103
self._assert_comments_service_called_without_group_id(mock_request)
106104

107-
def test_cohorted_topic_moderator_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
105+
def test_cohorted_topic_moderator_with_own_group_id(self, mock_request):
108106
self.call_view(
109-
mock_is_forum_v2_enabled,
110107
mock_request,
111108
"cohorted_topic",
112109
self.moderator,
113110
self.moderator_cohort.id
114111
)
115112
self._assert_comments_service_called_with_group_id(mock_request, self.moderator_cohort.id)
116113

117-
def test_cohorted_topic_moderator_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
114+
def test_cohorted_topic_moderator_with_other_group_id(self, mock_request):
118115
self.call_view(
119-
mock_is_forum_v2_enabled,
120116
mock_request,
121117
"cohorted_topic",
122118
self.moderator,
123119
self.student_cohort.id
124120
)
125121
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)
126122

127-
def test_cohorted_topic_moderator_with_invalid_group_id(self, mock_is_forum_v2_enabled, mock_request):
123+
def test_cohorted_topic_moderator_with_invalid_group_id(self, mock_request):
128124
invalid_id = self.student_cohort.id + self.moderator_cohort.id
129-
response = self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return
125+
response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return
130126
assert response.status_code == 500
131127

132-
def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_is_forum_v2_enabled, mock_request):
128+
def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_request):
133129
CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.AUDIT)
134130
CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED)
135131
discussion_settings = CourseDiscussionSettings.get(self.course.id)
@@ -140,7 +136,7 @@ def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_is_forum_v2
140136
})
141137

142138
invalid_id = -1000
143-
response = self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return
139+
response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return
144140
assert response.status_code == 500
145141

146142

@@ -149,16 +145,15 @@ class NonCohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
149145
Provides test cases to verify that views pass the correct `group_id` to
150146
the comments service when requesting content in non-cohorted discussions.
151147
"""
152-
def call_view(self, mock_is_forum_v2_enabled, mock_request, commentable_id, user, group_id, pass_group_id=True):
148+
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
153149
"""
154150
Call the view for the implementing test class, constructing a request
155151
from the parameters.
156152
"""
157153
pass # lint-amnesty, pylint: disable=unnecessary-pass
158154

159-
def test_non_cohorted_topic_student_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
155+
def test_non_cohorted_topic_student_without_group_id(self, mock_request):
160156
self.call_view(
161-
mock_is_forum_v2_enabled,
162157
mock_request,
163158
"non_cohorted_topic",
164159
self.student,
@@ -167,33 +162,30 @@ def test_non_cohorted_topic_student_without_group_id(self, mock_is_forum_v2_enab
167162
)
168163
self._assert_comments_service_called_without_group_id(mock_request)
169164

170-
def test_non_cohorted_topic_student_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
171-
self.call_view(mock_is_forum_v2_enabled, mock_request, "non_cohorted_topic", self.student, '')
165+
def test_non_cohorted_topic_student_none_group_id(self, mock_request):
166+
self.call_view(mock_request, "non_cohorted_topic", self.student, '')
172167
self._assert_comments_service_called_without_group_id(mock_request)
173168

174-
def test_non_cohorted_topic_student_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
169+
def test_non_cohorted_topic_student_with_own_group_id(self, mock_request):
175170
self.call_view(
176-
mock_is_forum_v2_enabled,
177171
mock_request,
178172
"non_cohorted_topic",
179173
self.student,
180174
self.student_cohort.id
181175
)
182176
self._assert_comments_service_called_without_group_id(mock_request)
183177

184-
def test_non_cohorted_topic_student_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
178+
def test_non_cohorted_topic_student_with_other_group_id(self, mock_request):
185179
self.call_view(
186-
mock_is_forum_v2_enabled,
187180
mock_request,
188181
"non_cohorted_topic",
189182
self.student,
190183
self.moderator_cohort.id
191184
)
192185
self._assert_comments_service_called_without_group_id(mock_request)
193186

194-
def test_non_cohorted_topic_moderator_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
187+
def test_non_cohorted_topic_moderator_without_group_id(self, mock_request):
195188
self.call_view(
196-
mock_is_forum_v2_enabled,
197189
mock_request,
198190
"non_cohorted_topic",
199191
self.moderator,
@@ -202,43 +194,41 @@ def test_non_cohorted_topic_moderator_without_group_id(self, mock_is_forum_v2_en
202194
)
203195
self._assert_comments_service_called_without_group_id(mock_request)
204196

205-
def test_non_cohorted_topic_moderator_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
206-
self.call_view(mock_is_forum_v2_enabled, mock_request, "non_cohorted_topic", self.moderator, '')
197+
def test_non_cohorted_topic_moderator_none_group_id(self, mock_request):
198+
self.call_view(mock_request, "non_cohorted_topic", self.moderator, '')
207199
self._assert_comments_service_called_without_group_id(mock_request)
208200

209-
def test_non_cohorted_topic_moderator_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
201+
def test_non_cohorted_topic_moderator_with_own_group_id(self, mock_request):
210202
self.call_view(
211-
mock_is_forum_v2_enabled,
212203
mock_request,
213204
"non_cohorted_topic",
214205
self.moderator,
215206
self.moderator_cohort.id,
216207
)
217208
self._assert_comments_service_called_without_group_id(mock_request)
218209

219-
def test_non_cohorted_topic_moderator_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
210+
def test_non_cohorted_topic_moderator_with_other_group_id(self, mock_request):
220211
self.call_view(
221-
mock_is_forum_v2_enabled,
222212
mock_request,
223213
"non_cohorted_topic",
224214
self.moderator,
225215
self.student_cohort.id,
226216
)
227217
self._assert_comments_service_called_without_group_id(mock_request)
228218

229-
def test_non_cohorted_topic_moderator_with_invalid_group_id(self, mock_is_forum_v2_enabled, mock_request):
219+
def test_non_cohorted_topic_moderator_with_invalid_group_id(self, mock_request):
230220
invalid_id = self.student_cohort.id + self.moderator_cohort.id
231-
self.call_view(mock_is_forum_v2_enabled, mock_request, "non_cohorted_topic", self.moderator, invalid_id)
221+
self.call_view(mock_request, "non_cohorted_topic", self.moderator, invalid_id)
232222
self._assert_comments_service_called_without_group_id(mock_request)
233223

234-
def test_team_discussion_id_not_cohorted(self, mock_is_forum_v2_enabled, mock_request):
224+
def test_team_discussion_id_not_cohorted(self, mock_request):
235225
team = CourseTeamFactory(
236226
course_id=self.course.id,
237227
topic_id='topic-id'
238228
)
239229

240230
team.add_user(self.student)
241-
self.call_view(mock_is_forum_v2_enabled, mock_request, team.discussion_topic_id, self.student, '')
231+
self.call_view(mock_request, team.discussion_topic_id, self.student, '')
242232

243233
self._assert_comments_service_called_without_group_id(mock_request)
244234

lms/djangoapps/discussion/django_comment_client/tests/mixins.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,6 @@ def setUpClassAndForumMock(cls):
1717
"""
1818
cls.mock_forum_api = mock.Mock()
1919

20-
# TODO: Remove this after moving all APIs
21-
cls.flag_v2_patcher = mock.patch(
22-
"openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled"
23-
)
24-
cls.mock_enable_forum_v2 = cls.flag_v2_patcher.start()
25-
cls.mock_enable_forum_v2.return_value = True
26-
2720
patch_targets = [
2821
"openedx.core.djangoapps.django_comment_common.comment_client.thread.forum_api",
2922
"openedx.core.djangoapps.django_comment_common.comment_client.comment.forum_api",
@@ -41,8 +34,6 @@ def setUpClassAndForumMock(cls):
4134
@classmethod
4235
def disposeForumMocks(cls):
4336
"""Stop patches after tests complete."""
44-
cls.flag_v2_patcher.stop()
45-
4637
for patcher in cls.forum_api_patchers:
4738
patcher.stop()
4839

0 commit comments

Comments
 (0)