Conversation
This reverts commit 8f6699d.
There was a problem hiding this comment.
Pull request overview
This PR reverts the soft delete feature (PR #4) from the forum application, returning to hard deletes for threads and comments. The revert removes soft delete functionality that marked content as deleted (is_deleted=True) while keeping it in the database, and restores the previous behavior of permanently removing content from the database.
Key changes:
- Removed soft delete fields (
is_deleted,deleted_at,deleted_by) from models and serializers - Deleted migration file that added soft delete fields to the database
- Removed soft delete and restore methods from both MongoDB and MySQL backends
- Reverted tests to assert hard deletes (checking for
None) instead of soft deletes - Downgraded version from 0.4.0 to 0.3.9
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_views/test_threads.py |
Reverted test assertions to check for hard deletes (None) instead of soft deletes (is_deleted=True) |
tests/test_views/test_comments.py |
Updated test docstrings and assertions to reflect hard delete behavior; removed pylint disable comment |
tests/test_backends/test_mongodb/test_comments.py |
Changed delete method return type assertion from tuple to int |
tests/e2e/test_users.py |
Removed deleted content statistics (deleted_threads, deleted_responses, deleted_replies) from expected response |
forum/views/comments.py |
Updated docstring to remove deleted_by parameter; reordered imports alphabetically |
forum/serializers/contents.py |
Removed soft delete fields (is_deleted, deleted_at, deleted_by) from ContentSerializer |
forum/migrations/0006_comment_deleted_at_comment_deleted_by_and_more.py |
Deleted entire migration file that added soft delete fields |
forum/backends/mysql/models.py |
Removed soft delete fields from Content and CourseStat models; updated comment_count to count all comments |
forum/backends/mysql/api.py |
Removed soft delete methods and logic; removed is_deleted filtering from queries; reordered imports |
forum/backends/mongodb/threads.py |
Removed soft delete methods (restore_thread, get_user_deleted_threads_count, restore_user_deleted_threads) |
forum/backends/mongodb/comments.py |
Simplified delete method signature and implementation to perform hard deletes only |
forum/backends/mongodb/api.py |
Removed soft delete methods; updated update_stats_for_course to only update existing fields; removed is_deleted parameter from queries; refactored imports |
forum/backends/backend.py |
Removed abstract methods for getting deleted content |
forum/api/users.py |
Removed show_deleted parameter from get_user_active_threads |
forum/api/threads.py |
Removed deleted_by parameter from delete_thread; simplified stats update; removed soft delete functions |
forum/api/search.py |
Removed is_deleted parameter from search_threads |
forum/api/comments.py |
Removed deleted_by parameter from delete_comment; updated stats logic to handle hard deletes; removed soft delete functions |
forum/api/__init__.py |
Removed soft delete function exports; reordered imports |
forum/__init__.py |
Downgraded version from 0.4.0 to 0.3.9 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| backend.delete_comment(comment_id) | ||
| author_id = comment["author_id"] | ||
| comment_course_id = comment["course_id"] | ||
|
|
||
| # soft_delete_comment returns (responses_deleted, replies_deleted) | ||
| responses_deleted, replies_deleted = backend.soft_delete_comment( | ||
| comment_id, deleted_by | ||
| ) | ||
|
|
||
| # Update stats based on what was actually deleted | ||
| if responses_deleted > 0: | ||
| # A response (parent comment) was deleted | ||
| backend.update_stats_for_course( | ||
| author_id, | ||
| comment_course_id, | ||
| responses=-responses_deleted, | ||
| deleted_responses=responses_deleted, | ||
| replies=-replies_deleted, | ||
| deleted_replies=replies_deleted, | ||
| ) | ||
| parent_comment_id = data["parent_id"] | ||
| if parent_comment_id: | ||
| backend.update_stats_for_course(author_id, comment_course_id, replies=-1) | ||
| else: | ||
| # Only a reply was deleted (no response) | ||
| backend.update_stats_for_course( | ||
| author_id, | ||
| comment_course_id, | ||
| replies=-replies_deleted, | ||
| deleted_replies=replies_deleted, | ||
| ) | ||
| backend.update_stats_for_course(author_id, comment_course_id, responses=-1) |
There was a problem hiding this comment.
The stats are being updated without checking if the comment is anonymous. Anonymous comments should not update user course statistics. The function should check if the comment is anonymous or anonymous_to_peers before calling update_stats_for_course, similar to how thread deletion handles this check on line 191 of forum/api/threads.py.
| k: course_stat[k] + v | ||
| for k, v in kwargs.items() | ||
| if k in course_stat |
There was a problem hiding this comment.
The logic in update_stats_for_course now only updates fields that already exist in course_stat (with the condition if k in course_stat). This could silently fail to update stats if a field is missing from the course_stat dictionary. The soft delete feature likely added fields like deleted_threads, deleted_responses, and deleted_replies, and this revert is removing those. However, if other new fields are added in the future and they don't exist in existing course_stat records, they won't be initialized properly. Consider using course_stat.get(k, 0) pattern to handle missing fields gracefully.
| k: course_stat[k] + v | |
| for k, v in kwargs.items() | |
| if k in course_stat | |
| k: course_stat.get(k, 0) + v | |
| for k, v in kwargs.items() |
|
|
||
| Note: Soft delete marks the comment as deleted but does NOT decrement | ||
| the parent's child_count (this matches the MongoDB behavior). | ||
| Test creating a new child comment. |
There was a problem hiding this comment.
The docstring for this test says "Test creating a new child comment" but the test is actually testing the deletion of a child comment. The docstring should be updated to "Test deleting a child comment" to accurately reflect what the test does.
| Test creating a new child comment. | |
| Test deleting a child comment. |
feat(discussion): Refactor ban and unban API views, models, and tests test(discussion): remove duplicate MySQL test_discussion_ban_models.py style: fix trailing whitespace and long lines in ban files - Remove trailing whitespace from forum/api/bans.py and forum/views/bans.py - Break long lines to meet 120 character limit - Add mypy ignore_errors for ban-related modules (type annotations needed) style: fix trailing whitespace and long lines in ban files - Remove trailing whitespace from forum/api/bans.py and forum/views/bans.py - Break long lines to meet 120 character limit - Add mypy ignore_errors for ban-related modules (type annotations needed) Revert "style: fix trailing whitespace and long lines in ban files" This reverts commit 9f0b0f5. feat(discussion): Refactor discussion ban tests for consistency and readability feat(ban): Refactor ban API functions and add comprehensive tests feat: soft delete feature (#4) Implements soft delete functionality for discussion threads, responses, and comments using the is_deleted flag instead of permanently deleting records. This enables safe deletion and restoration of discussion content while preserving existing data. Revert "feat: soft delete feature (#4)" (#9) This reverts commit 8f6699d. feat: added soft delete functionality (#10) Implements soft delete functionality for discussion threads, responses, and comments using the is_deleted flag instead of permanently deleting records. This enables safe deletion and restoration of discussion content while preserving existing data. feat(ban): Enhance ban and unban APIs to handle User objects directly refactor(ban): Improve code formatting and readability in ban API feat(ban): Update get_banned_users to exclude org-level bans with course exceptions
Reverts #4