Skip to content

feat(discussion): Implement discussion moderation features#110

Draft
mraman-2U wants to merge 9 commits intorelease-ulmofrom
mraman/bulkdelete-ban-edx
Draft

feat(discussion): Implement discussion moderation features#110
mraman-2U wants to merge 9 commits intorelease-ulmofrom
mraman/bulkdelete-ban-edx

Conversation

@mraman-2U
Copy link
Member

This pull request introduces new features and improvements to the discussion moderation system, focusing on user banning, bulk delete and ban operations, and enhanced permission checks. It adds new serializers and API logic for banning users, ensures banned users are excluded from discussion stats, and introduces robust email notifications for ban escalations. Permission logic for moderation actions is clarified and strengthened, and the bulk delete task is updated to support banning users with retry logic.

Key changes:

User Banning and Moderation Logic

  • Added checks in create_thread and create_comment to prevent banned users from posting in course discussions, raising a PermissionDenied error if the user is banned. [1] [2]
  • Excluded banned users from course discussion user stats by filtering them out of the stats response in get_course_discussion_user_stats.

Serializers and API Enhancements

  • Introduced BulkDeleteBanRequestSerializer, BanUserRequestSerializer, and BannedUserSerializer to support bulk delete and ban actions, standalone bans, and listing banned users, with validation and user normalization logic.
  • Improved context generation in serializers to include course_id and ensure staff/moderator privilege checks include course staff.

Permissions and Access Control

  • Clarified and expanded the can_take_action_on_spam function and IsAllowedToBulkDelete permission class to grant bulk delete and ban permissions to global staff, course staff, instructors, and forum moderators/administrators, while restricting access for students and community TAs.

Email Notifications for Moderation

  • Added a new emails.py module to send escalation emails when a user is banned, using ACE (Automated Communications Engine) if available, with fallback to Django email. The email includes details about the ban, moderator, course, and content deleted.

Bulk Delete and Ban Task Improvements

  • Updated the delete_course_post_for_user Celery task to support optional user banning, ban scope, moderator, and reason parameters. Added retry logic with exponential backoff for transient errors.
  • Ensured database transactions are used in tasks for consistency.

Minor Improvements

  • Adjusted spam URL filtering regex to improve matching in filter_spam_urls_from_html.

These changes collectively strengthen discussion moderation, provide better auditability, and improve the experience for moderators and staff

Description

Describe what this pull request changes, and why. Include implications for people using this change.
Design decisions and their rationales should be documented in the repo (docstring / ADR), per
OEP-19, and can be
linked here.

Useful information to include:

  • Which edX user roles will this change impact? Common user roles are "Learner", "Course Author",
    "Developer", and "Operator".
  • Include screenshots for changes to the UI (ideally, both "before" and "after" screenshots, if applicable).
  • Provide links to the description of corresponding configuration changes. Remember to correctly annotate these
    changes.

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

Please provide detailed step-by-step instructions for testing this change.

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

Other information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.
  • If your database migration can't be rolled back easily.

@mraman-2U mraman-2U force-pushed the mraman/bulkdelete-ban-edx branch from 92504d2 to 182c1b6 Compare February 2, 2026 17:32
Copy link

@jcapphelix jcapphelix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes @mraman-2U

Also, there will be other set of PRs for edx-internal repos for new added variables ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contains whitespace-only changes, can be discarded if not required for linting purpose

Comment on lines +94 to +97
requester.id in moderator_user_ids or
requester.id in ta_user_ids or
requester.id in course_staff_user_ids or
is_global_staff

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, we may not need to implement it

all_privileged_ids = ( moderator_user_ids | ta_user_ids | course_staff_user_ids) 
has_moderation_privilege = requester.id in all_privileged_ids or is_global_staff 

Possible change, not required strictly

Comment on lines +267 to +271
course_id = (
view.kwargs.get("course_id") or
request.query_params.get("course_id") or
(request.data.get("course_id") if hasattr(request, 'data') else None)
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these many safety nets ? won't view.kwargs.get be enough ?

site_key = get_captcha_site_key_by_platform(get_platform_from_request())
url = (f"https://recaptchaenterprise.googleapis.com/v1/projects/{settings.RECAPTCHA_PROJECT_ID}/assessments"
f"?key={settings.RECAPTCHA_PRIVATE_KEY}")
url = ("https://recaptchaenterprise.googleapis.com/v1/projects/{}/assessments"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linting related changes ?

If otherwise, changes can be removed. Keep changes as is if linting related issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants