Fix: Use target user enrollment for org-level bulk delete #85

alenkadev · 2026-01-16T12:51:13Z

Description:
Bulk-delete by organization in edX discussions only deletes posts from the currently active course instead of removing all user posts across the entire organization.

Solution:
In views.py incorrectly filtered CourseEnrollment records using request.user (the admin making the request) instead of user (the target user whose posts should be deleted).

jira link- COSMO2-787

Copilot

Pull request overview

This PR fixes a critical bug in the organization-level bulk delete functionality for discussion posts. The bug caused the system to query enrollments for the wrong user when determining which courses to delete posts from.

Changes:

Fixed enrollment query to use target user instead of requesting user for org-level bulk delete operations

Comments suppressed due to low confidence (1)

lms/djangoapps/discussion/rest_api/views.py:1590

The org-level bulk delete functionality (when course_or_org='org') lacks test coverage. This critical path should be tested to ensure the enrollment query correctly uses the target user and not the requesting user. Consider adding a test that verifies posts are deleted from courses where the target user is enrolled, not where the moderator is enrolled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jcapphelix · 2026-01-16T14:14:47Z

lms/djangoapps/discussion/rest_api/views.py

+            course_ids.extend([
+                str(c_id)
+                for c_id in enrollments
+                if c_id.org == org_id
+            ])


Why these lines are changed ?

jcapphelix

Can you check on why some other lines are changed ?

Also there are multiple commits here. We just need one.

Can squash then in 1 when you apply the fixes for other lines ?

Thanks.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-16T14:43:19Z

lms/djangoapps/discussion/rest_api/views.py

-                for c_id in enrollments
-                if c_id.org == org_id
-            ])
+            enrollments = CourseEnrollment.objects.filter(


The organization-level bulk delete functionality (course_or_org='org') lacks test coverage. The existing tests in BulkDeleteUserPostsTest only test basic course-level deletion, permission checks, and the execute parameter, but do not verify that the correct user's enrollments are retrieved for org-level deletion. Consider adding a test that verifies the fixed behavior by creating enrollments for both the target user and admin user in multiple courses within an organization, then asserting that only the target user's enrolled courses are included in the deletion scope.

Copilot

Copilot wasn't able to review any files in this pull request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-16T16:22:08Z

lms/djangoapps/discussion/rest_api/views.py

+        if course_or_org == "org":
+            org_id = CourseKey.from_string(course_id).org
+            enrollments = CourseEnrollment.objects.filter(
+                user=request.user


The same bug that was fixed in BulkDeleteUserPosts (line 1670) is present here. This should filter by user (the target user whose posts are being restored) instead of request.user (the admin making the request). When course_or_org is 'org', this will incorrectly use the admin's enrollments instead of the target user's enrollments.

Suggested change

user=request.user

user=user

Copilot · 2026-01-16T16:22:09Z

lms/djangoapps/discussion/rest_api/views.py

            form.cleaned_data["order_direction"],
            form.cleaned_data["requested_fields"],
            form.cleaned_data["count_flagged"],
+            form.cleaned_data["show_deleted"],


The new show_deleted parameter is added to the function call but there is no test coverage for this new functionality. The test file test_views_v2.py has comprehensive tests for BulkDeleteUserPosts, and similar coverage should be added for the new show_deleted parameter behavior.

Copilot · 2026-01-16T16:22:09Z

lms/djangoapps/discussion/rest_api/views.py

-        post_status = request.GET.get('status', None)
+        order_by = order_by_mapping.get(order_by, "activity")
+        post_status = request.GET.get("status", None)
+        show_deleted = request.GET.get("show_deleted", "false").lower() == "true"


The new show_deleted query parameter handling lacks test coverage. Given that there are comprehensive tests for LearnerThreadView in the test suite, tests should be added to verify this new parameter works correctly.

Copilot · 2026-01-16T16:22:09Z

lms/djangoapps/discussion/rest_api/views.py

            "count_flagged": count_flagged,
            "thread_type": thread_type,
            "sort_key": order_by,
+            "show_deleted": show_deleted,


The show_deleted parameter is being passed to the query but lacks test coverage. Tests should verify the behavior when show_deleted is true vs false.

Copilot · 2026-01-16T16:22:10Z

lms/djangoapps/discussion/rest_api/views.py

            form.cleaned_data["requested_fields"],
-            form.cleaned_data["merge_question_type_responses"]
+            form.cleaned_data["merge_question_type_responses"],
+            form.cleaned_data["show_deleted"],


The new show_deleted parameter for comments lacks test coverage. Tests should be added to verify this parameter works correctly when listing comments by thread.

Copilot · 2026-01-16T16:22:10Z

lms/djangoapps/discussion/rest_api/views.py

        )
+
+
+class RestoreContent(DeveloperErrorViewMixin, APIView):


The new RestoreContent view class lacks test coverage. While the view is not yet exposed via URLs, it should have tests similar to the comprehensive test coverage that exists for BulkDeleteUserPosts.

Copilot · 2026-01-16T16:22:10Z

lms/djangoapps/discussion/rest_api/views.py

+            )
+
+
+class BulkRestoreUserPosts(DeveloperErrorViewMixin, APIView):


The new BulkRestoreUserPosts view class lacks test coverage. Similar to BulkDeleteUserPosts which has comprehensive tests in test_views_v2.py, this new functionality should have corresponding tests to verify permissions, parameter handling, and org-level restoration.

Copilot · 2026-01-16T16:22:10Z

lms/djangoapps/discussion/rest_api/views.py

+        )
+
+
+class DeletedContentView(DeveloperErrorViewMixin, APIView):


The new DeletedContentView class lacks test coverage. Tests should verify pagination, content_type filtering, author_id filtering, and error handling for invalid inputs.

Copilot · 2026-01-16T16:22:10Z

lms/djangoapps/discussion/rest_api/views.py

        if execute_task:
            event_data = {
                "triggered_by": request.user.username,
+                "triggered_by_user_id": str(request.user.id),


The new triggered_by_user_id field is added to event_data here but was not present in the original code before this PR. While this is an improvement for audit tracking, ensure that the task delete_course_post_for_user and any downstream event handlers are updated to expect this new field, otherwise it may cause issues with event processing.

Copilot AI review requested due to automatic review settings January 16, 2026 12:51

Copilot started reviewing on behalf of alenkadev January 16, 2026 12:51 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

alenkadev force-pushed the fix/bulk-delete-organization-scope branch from 7874d61 to f4c3292 Compare January 16, 2026 13:21

alenkadev requested a review from Copilot January 16, 2026 13:52

Copilot started reviewing on behalf of alenkadev January 16, 2026 13:52 View session

alenkadev requested review from jcapphelix and removed request for Copilot and jcapphelix January 16, 2026 13:52

jcapphelix reviewed Jan 16, 2026

View reviewed changes

jcapphelix requested changes Jan 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 16, 2026 14:36

alenkadev force-pushed the fix/bulk-delete-organization-scope branch from 5b6d78f to d5421b2 Compare January 16, 2026 14:36

Copilot started reviewing on behalf of alenkadev January 16, 2026 14:37 View session

alenkadev force-pushed the fix/bulk-delete-organization-scope branch 2 times, most recently from adff0fb to fe9e5b0 Compare January 16, 2026 14:42

Copilot AI reviewed Jan 16, 2026

View reviewed changes

alenkadev force-pushed the fix/bulk-delete-organization-scope branch 2 times, most recently from d432c2a to b8df920 Compare January 16, 2026 15:35

alenkadev closed this Jan 16, 2026

alenkadev force-pushed the fix/bulk-delete-organization-scope branch from b8df920 to f348776 Compare January 16, 2026 15:39

Copilot AI review requested due to automatic review settings January 16, 2026 15:39

Copilot AI reviewed Jan 16, 2026

View reviewed changes

alenkadev reopened this Jan 16, 2026

Copilot AI review requested due to automatic review settings January 16, 2026 16:15

Copilot started reviewing on behalf of alenkadev January 16, 2026 16:16 View session

fix: use target user enrollment for org-level bulk delete

e0368ad

Copilot AI reviewed Jan 16, 2026

View reviewed changes

alenkadev force-pushed the fix/bulk-delete-organization-scope branch from 09b4ed0 to e0368ad Compare January 16, 2026 16:22

		)


		class BulkRestoreUserPosts(DeveloperErrorViewMixin, APIView):

		)


		class DeletedContentView(DeveloperErrorViewMixin, APIView):

Fix: Use target user enrollment for org-level bulk delete #85

Are you sure you want to change the base?

Fix: Use target user enrollment for org-level bulk delete #85

Uh oh!

Conversation

alenkadev commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jcapphelix Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

jcapphelix left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alenkadev commented Jan 16, 2026 •

edited

Loading