-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(discussion): Bulk delete with Ban at course or org level #37689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a comprehensive discussion moderation system that enables moderators to ban users at course or organization level during bulk delete operations. The feature addresses crypto spam attacks and harassment by providing tools to ban users from discussions, with proper audit logging and notification mechanisms.
Key Changes:
- New Database Models: Three models (
DiscussionBan,DiscussionBanException,DiscussionModerationLog) with proper indexing and constraints for tracking bans, exceptions, and audit logs - REST API Endpoints: Bulk delete/ban, banned user listing, and unban functionality with comprehensive permission checks
- Email Notifications: ACE-based escalation emails sent to partner support when users are banned
- Configuration: New Django settings and waffle flags to control ban feature availability and email notifications
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 32 comments.
Show a summary per file
| File | Description |
|---|---|
lms/envs/common.py |
Added discussion moderation settings (email toggle, escalation email, reason max length) |
lms/envs/devstack.py |
Disabled ban emails in local dev; contains unrelated port changes that should be reverted |
lms/envs/test.py |
Disabled ban emails in tests to prevent spam |
lms/djangoapps/discussion/toggles.py |
Added ENABLE_DISCUSSION_BAN waffle flag for feature rollout control |
lms/djangoapps/discussion/models.py |
Core data models with validation, unique constraints, and ban checking logic |
lms/djangoapps/discussion/migrations/0001_initial.py |
Database schema creation with indexes and constraints |
lms/djangoapps/discussion/admin.py |
Django admin with read-only audit access; has XSS vulnerability in user links |
lms/djangoapps/discussion/rest_api/views.py |
ViewSet with bulk-delete-ban, banned-users list, and unban endpoints |
lms/djangoapps/discussion/rest_api/serializers.py |
Request/response serializers with validation |
lms/djangoapps/discussion/rest_api/permissions.py |
Enhanced permission checking for moderation actions |
lms/djangoapps/discussion/rest_api/tasks.py |
Enhanced Celery task for async deletion with ban creation |
lms/djangoapps/discussion/rest_api/emails.py |
Ban escalation email sending via ACE or Django mail |
lms/djangoapps/discussion/rest_api/urls.py |
URL routing for new moderation endpoints |
lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/* |
ACE email templates (HTML and text) |
lms/djangoapps/discussion/templates/discussion/ban_escalation_email.txt |
Fallback text email template |
lms/djangoapps/discussion/tests/test_models.py |
Comprehensive model tests (369 lines) |
lms/djangoapps/discussion/rest_api/tests/test_moderation_*.py |
Test suites for views, tasks, serializers, permissions, and emails (1000+ lines total) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lms/djangoapps/discussion/rest_api/tests/test_moderation_tasks.py
Outdated
Show resolved
Hide resolved
lms/djangoapps/discussion/rest_api/tests/test_moderation_permissions.py
Outdated
Show resolved
Hide resolved
lms/djangoapps/discussion/rest_api/tests/test_moderation_serializers.py
Outdated
Show resolved
Hide resolved
lms/djangoapps/discussion/rest_api/tests/test_moderation_tasks.py
Outdated
Show resolved
Hide resolved
lms/djangoapps/discussion/rest_api/tests/test_moderation_views.py
Outdated
Show resolved
Hide resolved
eb88620 to
d895d3f
Compare
d895d3f to
45b2499
Compare
This pull request introduces a new Django admin interface and database schema for discussion moderation, including user bans, ban exceptions, and moderation audit logs. The changes add three new models with appropriate admin configurations, permissions, and indexing for efficient querying and compliance/audit needs.
Discussion Moderation Models and Database Schema:
DiscussionBan,DiscussionBanException, andDiscussionModerationLog, with fields and relationships to support user bans at both course and organization levels, exceptions to bans, and audit logging of moderation actions. The migration includes indexes and constraints for data integrity and query performance.Django Admin Configuration and Permissions:
DiscussionBan,DiscussionBanException,DiscussionModerationLog), using a mixin to enforce read-only access for non-superusers and full access for superusers, with audit logs being read-only for all users. Admin interfaces provide search, filtering, and linked references for ease of use and support compliance requirements.…user bansDescription
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:
"Developer", and "Operator".
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.