Skip to content

Commit

Permalink
feat: reapply forum v2 changes (#36002)
Browse files Browse the repository at this point in the history
* feat: Reapply "Integrate Forum V2 into edx-platform"

This reverts commit 818aa34.

* 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: openedx/forum#137

* chore: bump openedx-forum to 0.1.5

This should fix an issue with index creation on edX.org.
  • Loading branch information
regisb authored and jawad-khan committed Dec 17, 2024
1 parent f38adaa commit fe162fa
Show file tree
Hide file tree
Showing 26 changed files with 1,900 additions and 431 deletions.
398 changes: 288 additions & 110 deletions lms/djangoapps/discussion/django_comment_client/base/tests.py

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,6 @@ def create_thread(request, course_id, commentable_id):
params['context'] = ThreadContext.STANDALONE
else:
params['context'] = ThreadContext.COURSE

thread = cc.Thread(**params)

# Divide the thread if required
Expand Down
147 changes: 105 additions & 42 deletions lms/djangoapps/discussion/django_comment_client/tests/group_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,51 +60,76 @@ class CohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
Provides test cases to verify that views pass the correct `group_id` to
the comments service when requesting content in cohorted discussions.
"""
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
def call_view(self, mock_is_forum_v2_enabled, mock_request, commentable_id, user, group_id, pass_group_id=True):
"""
Call the view for the implementing test class, constructing a request
from the parameters.
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass

def test_cohorted_topic_student_without_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.student, '', pass_group_id=False)
def test_cohorted_topic_student_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.student, '', pass_group_id=False)
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)

def test_cohorted_topic_student_none_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.student, "")
def test_cohorted_topic_student_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.student, "")
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)

def test_cohorted_topic_student_with_own_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.student, self.student_cohort.id)
def test_cohorted_topic_student_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.student, self.student_cohort.id)
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)

def test_cohorted_topic_student_with_other_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.student, self.moderator_cohort.id)
def test_cohorted_topic_student_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"cohorted_topic",
self.student,
self.moderator_cohort.id
)
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)

def test_cohorted_topic_moderator_without_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.moderator, '', pass_group_id=False)
def test_cohorted_topic_moderator_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"cohorted_topic",
self.moderator,
'',
pass_group_id=False
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_cohorted_topic_moderator_none_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.moderator, "")
def test_cohorted_topic_moderator_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(mock_is_forum_v2_enabled, mock_request, "cohorted_topic", self.moderator, "")
self._assert_comments_service_called_without_group_id(mock_request)

def test_cohorted_topic_moderator_with_own_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.moderator, self.moderator_cohort.id)
def test_cohorted_topic_moderator_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"cohorted_topic",
self.moderator,
self.moderator_cohort.id
)
self._assert_comments_service_called_with_group_id(mock_request, self.moderator_cohort.id)

def test_cohorted_topic_moderator_with_other_group_id(self, mock_request):
self.call_view(mock_request, "cohorted_topic", self.moderator, self.student_cohort.id)
def test_cohorted_topic_moderator_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"cohorted_topic",
self.moderator,
self.student_cohort.id
)
self._assert_comments_service_called_with_group_id(mock_request, self.student_cohort.id)

def test_cohorted_topic_moderator_with_invalid_group_id(self, mock_request):
def test_cohorted_topic_moderator_with_invalid_group_id(self, mock_is_forum_v2_enabled, mock_request):
invalid_id = self.student_cohort.id + self.moderator_cohort.id
response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return
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
assert response.status_code == 500

def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_request):
def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_is_forum_v2_enabled, mock_request):
CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.AUDIT)
CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
Expand All @@ -115,7 +140,7 @@ def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_request):
})

invalid_id = -1000
response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return
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
assert response.status_code == 500


Expand All @@ -124,57 +149,95 @@ class NonCohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
Provides test cases to verify that views pass the correct `group_id` to
the comments service when requesting content in non-cohorted discussions.
"""
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
def call_view(self, mock_is_forum_v2_enabled, mock_request, commentable_id, user, group_id, pass_group_id=True):
"""
Call the view for the implementing test class, constructing a request
from the parameters.
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass

def test_non_cohorted_topic_student_without_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.student, '', pass_group_id=False)
def test_non_cohorted_topic_student_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"non_cohorted_topic",
self.student,
'',
pass_group_id=False
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_student_none_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.student, '')
def test_non_cohorted_topic_student_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(mock_is_forum_v2_enabled, mock_request, "non_cohorted_topic", self.student, '')
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_student_with_own_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.student, self.student_cohort.id)
def test_non_cohorted_topic_student_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"non_cohorted_topic",
self.student,
self.student_cohort.id
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_student_with_other_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.student, self.moderator_cohort.id)
def test_non_cohorted_topic_student_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"non_cohorted_topic",
self.student,
self.moderator_cohort.id
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_moderator_without_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.moderator, '', pass_group_id=False)
def test_non_cohorted_topic_moderator_without_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"non_cohorted_topic",
self.moderator,
"",
pass_group_id=False,
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_moderator_none_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.moderator, '')
def test_non_cohorted_topic_moderator_none_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(mock_is_forum_v2_enabled, mock_request, "non_cohorted_topic", self.moderator, '')
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_moderator_with_own_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.moderator, self.moderator_cohort.id)
def test_non_cohorted_topic_moderator_with_own_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"non_cohorted_topic",
self.moderator,
self.moderator_cohort.id,
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_moderator_with_other_group_id(self, mock_request):
self.call_view(mock_request, "non_cohorted_topic", self.moderator, self.student_cohort.id)
def test_non_cohorted_topic_moderator_with_other_group_id(self, mock_is_forum_v2_enabled, mock_request):
self.call_view(
mock_is_forum_v2_enabled,
mock_request,
"non_cohorted_topic",
self.moderator,
self.student_cohort.id,
)
self._assert_comments_service_called_without_group_id(mock_request)

def test_non_cohorted_topic_moderator_with_invalid_group_id(self, mock_request):
def test_non_cohorted_topic_moderator_with_invalid_group_id(self, mock_is_forum_v2_enabled, mock_request):
invalid_id = self.student_cohort.id + self.moderator_cohort.id
self.call_view(mock_request, "non_cohorted_topic", self.moderator, invalid_id)
self.call_view(mock_is_forum_v2_enabled, mock_request, "non_cohorted_topic", self.moderator, invalid_id)
self._assert_comments_service_called_without_group_id(mock_request)

def test_team_discussion_id_not_cohorted(self, mock_request):
def test_team_discussion_id_not_cohorted(self, mock_is_forum_v2_enabled, mock_request):
team = CourseTeamFactory(
course_id=self.course.id,
topic_id='topic-id'
)

team.add_user(self.student)
self.call_view(mock_request, team.discussion_topic_id, self.student, '')
self.call_view(mock_is_forum_v2_enabled, mock_request, team.discussion_topic_id, self.student, '')

self._assert_comments_service_called_without_group_id(mock_request)
7 changes: 4 additions & 3 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def _get_course(course_key: CourseKey, user: User, check_tab: bool = True) -> Co
return course


def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
def _get_thread_and_context(request, thread_id, retrieve_kwargs=None, course_id=None):
"""
Retrieve the given thread and build a serializer context for it, returning
both. This function also enforces access control for the thread (checking
Expand All @@ -213,7 +213,7 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
retrieve_kwargs["with_responses"] = False
if "mark_as_read" not in retrieve_kwargs:
retrieve_kwargs["mark_as_read"] = False
cc_thread = Thread(id=thread_id).retrieve(**retrieve_kwargs)
cc_thread = Thread(id=thread_id).retrieve(course_id=course_id, **retrieve_kwargs)
course_key = CourseKey.from_string(cc_thread["course_id"])
course = _get_course(course_key, request.user)
context = get_context(course, request, cc_thread)
Expand Down Expand Up @@ -1645,7 +1645,8 @@ def get_thread(request, thread_id, requested_fields=None, course_id=None):
retrieve_kwargs={
"with_responses": True,
"user_id": str(request.user.id),
}
},
course_id=course_id,
)
if course_id and course_id != cc_thread.course_id:
raise ThreadNotFoundError("Thread not found.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def send_response_on_followed_post_notification(self):

while has_more_subscribers:

subscribers = Subscription.fetch(self.thread.id, query_params={'page': page})
subscribers = Subscription.fetch(self.thread.id, self.course.id, query_params={'page': page})
if page <= subscribers.num_pages:
for subscriber in subscribers.collection:
# Check if the subscriber is not the thread creator or response creator
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/discussion/rest_api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def get_context(course, request, thread=None):
moderator_user_ids = get_moderator_users_list(course.id)
ta_user_ids = get_course_ta_users_list(course.id)
requester = request.user
cc_requester = CommentClientUser.from_django_user(requester).retrieve()
cc_requester = CommentClientUser.from_django_user(requester).retrieve(course_id=course.id)
cc_requester["course_id"] = course.id
course_discussion_settings = CourseDiscussionSettings.get(course.id)
is_global_staff = GlobalStaff().has_user(requester)
Expand Down
Loading

0 comments on commit fe162fa

Please sign in to comment.