-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[WEB-2854] chore: trigger issue_description_version task on issue create and update #6202
Conversation
WalkthroughThe changes introduce a new asynchronous task, 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 (
|
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
apiserver/plane/bgtasks/issue_description_version_task.py (1)
43-45
: Add type annotations to function parameters for clarityAdding type annotations to the parameters of
issue_description_version_task
enhances code readability and helps with static type checking.Apply this diff to add type annotations:
@shared_task def issue_description_version_task( - updated_issue, issue_id, user_id, is_creating + updated_issue: str, issue_id: str, user_id: str, is_creating: bool ) -> Optional[bool]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apiserver/plane/app/views/issue/base.py
(3 hunks)apiserver/plane/bgtasks/issue_description_version_task.py
(1 hunks)apiserver/plane/db/models/issue.py
(1 hunks)apiserver/plane/settings/common.py
(1 hunks)
🔇 Additional comments (4)
apiserver/plane/bgtasks/issue_description_version_task.py (1)
1-84
: Overall implementation is solid and handles exceptions appropriately
The task issue_description_version_task
correctly processes issue description versions with proper error handling and transactional integrity.
apiserver/plane/db/models/issue.py (1)
807-813
: Enhancement of log_issue_description_version
with additional context
Including parameters like workspace_id
, project_id
, and user IDs provides better context and traceability for issue description versions. The implementation aligns well with the existing model fields.
apiserver/plane/app/views/issue/base.py (2)
61-61
: Import statement correctly brings in the new task
The import of issue_description_version_task
ensures the task is available for dispatching.
432-438
: Triggering the versioning task after issue creation
Invoking issue_description_version_task
after creating an issue ensures that the initial description version is logged appropriately.
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
apiserver/plane/bgtasks/issue_description_version_task.py (1)
54-58
: Early return should indicate skip reasonReturn a boolean to indicate the skip was intentional, maintaining consistency with the function's return type.
Apply this diff:
if ( current_issue.get("description_html") == issue.description_html and not is_creating ): - return + return True # Skip processing as description hasn't changed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/issue_description_version_task.py
(1 hunks)
🔇 Additional comments (3)
apiserver/plane/bgtasks/issue_description_version_task.py (3)
14-15
:
Ensure should_update_existing_version
always returns a boolean value
The function implicitly returns None
when version
is None
, which contradicts the boolean return type annotation.
Apply this diff:
def should_update_existing_version(
version: IssueDescriptionVersion, user_id: str, max_time_difference: int = 600
) -> bool:
if not version:
- return
+ return False
76-84
: 🛠️ Refactor suggestion
Enhance error logging and return values
Improve error logging with more context and return False to indicate failure.
Apply this diff:
except Issue.DoesNotExist:
- # Issue no longer exists, skip processing
- return
+ log_exception(f"Issue {issue_id} not found while processing description version")
+ return False
except json.JSONDecodeError as e:
- log_exception(f"Invalid JSON for updated_issue: {e}")
- return
+ log_exception(
+ f"Invalid JSON for issue {issue_id}: {str(e)}",
+ extra={"updated_issue": updated_issue}
+ )
+ return False
except Exception as e:
- log_exception(f"Error processing issue description version: {e}")
- return
+ log_exception(
+ f"Error processing description version for issue {issue_id}: {str(e)}",
+ extra={"user_id": user_id, "is_creating": is_creating}
+ )
+ return False
Likely invalid or redundant comment.
42-43
: Verify Celery task registration
Let's ensure the task is properly registered in the Celery configuration.
✅ Verification successful
Let me generate additional verification scripts to check the Celery configuration and task registration more thoroughly.
Based on the verification results, I can now provide the final response:
Celery task integration is properly configured
The task is correctly integrated with Celery as evidenced by:
- Task is properly decorated with
@shared_task
- Task is imported and used correctly in
plane/app/views/issue/base.py
- Celery app is configured with autodiscover_tasks() which automatically registers tasks from all Django apps
- The task is being called correctly using
.delay()
method in the views
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Celery task registration and configuration
# Check if the task is imported in Celery configuration
rg -l "issue_description_version_task" | grep -E "celery.*\.py|tasks\.py|__init__\.py"
# Check for other references to ensure proper integration
rg "issue_description_version_task" -A 3 -B 3
Length of output: 2483
Script:
#!/bin/bash
# Check Celery configuration files for task registration
fd "celery.py|__init__.py" apiserver/plane/bgtasks/ -x cat {}
# Check if the task is properly decorated and configured
ast-grep --pattern 'from celery.decorators import shared_task'
ast-grep --pattern '@shared_task
def issue_description_version_task($$$)'
# Check for celery app configuration
fd "celery.py" apiserver/plane/ -x cat {}
Length of output: 2178
Description
issue_description_version
task to execute automatically during issue creation and update operations.Type of Change
References
[WEB-2854]
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores