[WEB-2809] chore: changed the soft deletion logic #6171
Conversation
WalkthroughThe changes in this pull request involve modifications to several files, primarily focusing on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apiserver/plane/bgtasks/deletion_task.py (4)
15-17: Enhance the docstring with detailed informationThe docstring for
soft_delete_related_objectsis minimal. Providing more details about the function's purpose, parameters, and behavior would improve code readability and maintainability.Consider expanding the docstring as follows:
def soft_delete_related_objects(app_label, model_name, instance_pk, using=None): """ Soft delete a model instance and its related objects recursively. Args: app_label (str): The app label of the model. model_name (str): The name of the model class. instance_pk (int): The primary key of the instance to soft delete. using (str, optional): The database alias to use. Defaults to None. Returns: None """
21-21: Line exceeds maximum length (95 > 88 characters)The comment on line 21 exceeds the recommended maximum line length of 88 characters, which may affect readability. Consider splitting the comment into multiple lines.
Apply this diff to fix the line length:
- # Get the instance using all_objects to ensure we can get even if it's already soft deleted + # Get the instance using all_objects to ensure we can get even if it's already + # soft deleted🧰 Tools
🪛 Ruff (0.8.0)
21-21: Line too long (95 > 88)
(E501)
100-100: Use logging instead of print statements for error handlingUsing
loggingmodule to handle logging more appropriately.To address this, first import the
loggingmodule and set up a logger at the top of the file:import logging logger = logging.getLogger(__name__)Then, replace the
- print(f"Error handling relation {related_name}: {str(e)}") + logger.error(f"Error handling relation {related_name}: {str(e)}")
71-83: Refactor duplicated code into a helper functionThe code for soft-deleting related objects and recursively calling
soft_delete_related_objectsis duplicated in lines 71-83 and 88-98. Refactoring this into a helper function will reduce code duplication and improve maintainability.Consider adding a helper function:
def soft_delete_and_recurse(related_obj, using=None): if hasattr(related_obj, "deleted_at") and not related_obj.deleted_at: related_obj.deleted_at = timezone.now() related_obj.save() # Recursively handle related objects soft_delete_related_objects( related_obj._meta.app_label, related_obj._meta.model_name, related_obj.pk, using, )Then, replace the duplicated code with calls to this helper function:
First occurrence (lines 71-83):
- if related_obj: - if hasattr(related_obj, "deleted_at"): - if not related_obj.deleted_at: - related_obj.deleted_at = timezone.now() - related_obj.save() - # Recursively handle related objects - soft_delete_related_objects( - related_obj._meta.app_label, - related_obj._meta.model_name, - related_obj.pk, - using, - ) + if related_obj: + soft_delete_and_recurse(related_obj, using)Second occurrence (lines 88-98):
- for related_obj in related_queryset: - if hasattr(related_obj, "deleted_at"): - if not related_obj.deleted_at: - related_obj.deleted_at = timezone.now() - related_obj.save() - # Recursively handle related objects - soft_delete_related_objects( - related_obj._meta.app_label, - related_obj._meta.model_name, - related_obj.pk, - using, - ) + for related_obj in related_queryset: + soft_delete_and_recurse(related_obj, using)Also applies to: 88-98
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
apiserver/plane/app/views/issue/base.py(0 hunks)apiserver/plane/bgtasks/deletion_task.py(1 hunks)apiserver/plane/db/models/__init__.py(0 hunks)
💤 Files with no reviewable changes (2)
- apiserver/plane/app/views/issue/base.py
- apiserver/plane/db/models/init.py
🧰 Additional context used
🪛 Ruff (0.8.0)
apiserver/plane/bgtasks/deletion_task.py
21-21: Line too long (95 > 88)
(E501)
Description
Type of Change
References
WEB-2809
Summary by CodeRabbit
New Features
updated_at.Bug Fixes
IssueListEndpoint.Refactor
Chores