-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[WEB-5312] migration: reply for work item comments #8072
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: preview
Are you sure you want to change the base?
Changes from 10 commits
df965a7
0987943
ac85e59
69120b8
8a72d2a
ea1e95c
f18e120
24f8010
0daf524
c65d2ff
6095f86
a86753e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Django imports | ||
| from django.core.management.base import BaseCommand | ||
| from django.db import transaction | ||
|
|
||
| # Module imports | ||
| from plane.db.models import Description | ||
| from plane.db.models import IssueComment | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = "Create Description records for existing IssueComment" | ||
|
|
||
| def handle(self, *args, **kwargs): | ||
| batch_size = 500 | ||
|
|
||
| while True: | ||
| comments = list( | ||
| IssueComment.objects.filter(description_id__isnull=True).order_by("created_at")[:batch_size] | ||
| ) | ||
|
|
||
| if not comments: | ||
| break | ||
|
|
||
| with transaction.atomic(): | ||
| descriptions = [ | ||
| Description( | ||
| created_at=comment.created_at, | ||
| updated_at=comment.updated_at, | ||
| description_json=comment.comment_json, | ||
| description_html=comment.comment_html, | ||
| description_stripped=comment.comment_stripped, | ||
| project_id=comment.project_id, | ||
| created_by_id=comment.created_by_id, | ||
| updated_by_id=comment.updated_by_id, | ||
| workspace_id=comment.workspace_id, | ||
| ) | ||
| for comment in comments | ||
| ] | ||
|
|
||
| created_descriptions = Description.objects.bulk_create(descriptions) | ||
|
|
||
| comments_to_update = [] | ||
| for comment, description in zip(comments, created_descriptions): | ||
| comment.description_id = description.id | ||
| comments_to_update.append(comment) | ||
|
|
||
| IssueComment.objects.bulk_update(comments_to_update, ["description_id"]) | ||
|
|
||
| self.stdout.write(self.style.SUCCESS("Successfully Copied IssueComment to Description")) | ||
|
Comment on lines
+13
to
+49
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Generated by Django 4.2.22 on 2025-11-06 08:28 | ||
|
|
||
| from django.db import migrations, models | ||
| import django.db.models.deletion | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ('db', '0108_alter_issueactivity_issue_comment'), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name='issuecomment', | ||
| name='description', | ||
| field=models.OneToOneField(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='issue_comment_description', to='db.description'), | ||
| ), | ||
| migrations.AddField( | ||
| model_name='issuecomment', | ||
| name='parent', | ||
| field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='parent_issue_comment', to='db.issuecomment'), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |||||||||
| from plane.utils.exception_logger import log_exception | ||||||||||
| from .project import ProjectBaseModel | ||||||||||
| from plane.utils.uuid import convert_uuid_to_integer | ||||||||||
| from .description import Description | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def get_default_properties(): | ||||||||||
|
|
@@ -446,6 +447,9 @@ class IssueComment(ProjectBaseModel): | |||||||||
| comment_stripped = models.TextField(verbose_name="Comment", blank=True) | ||||||||||
| comment_json = models.JSONField(blank=True, default=dict) | ||||||||||
| comment_html = models.TextField(blank=True, default="<p></p>") | ||||||||||
| description = models.OneToOneField( | ||||||||||
| "db.Description", on_delete=models.CASCADE, related_name="issue_comment_description", null=True | ||||||||||
| ) | ||||||||||
| attachments = ArrayField(models.URLField(), size=10, blank=True, default=list) | ||||||||||
| issue = models.ForeignKey(Issue, on_delete=models.CASCADE, related_name="issue_comments") | ||||||||||
| # System can also create comment | ||||||||||
|
|
@@ -463,10 +467,53 @@ class IssueComment(ProjectBaseModel): | |||||||||
| external_source = models.CharField(max_length=255, null=True, blank=True) | ||||||||||
| external_id = models.CharField(max_length=255, blank=True, null=True) | ||||||||||
| edited_at = models.DateTimeField(null=True, blank=True) | ||||||||||
| parent = models.ForeignKey( | ||||||||||
| "self", on_delete=models.CASCADE, null=True, blank=True, related_name="parent_issue_comment" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| def save(self, *args, **kwargs): | ||||||||||
| self.comment_stripped = strip_tags(self.comment_html) if self.comment_html != "" else "" | ||||||||||
| return super(IssueComment, self).save(*args, **kwargs) | ||||||||||
| is_creating = self._state.adding or not self.pk | ||||||||||
|
||||||||||
| is_creating = self._state.adding or not self.pk | |
| is_creating = self._state.adding |
Copilot
AI
Nov 19, 2025
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.
Potential N+1 query issue: Fetching the old comment instance individually inside the save method will cause a database query for every update. Consider tracking field changes using Django's built-in mechanisms or caching the old values before the save, especially since this will be called for every single comment update operation.
Copilot
AI
Nov 19, 2025
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.
[nitpick] Code duplication: The description_defaults dictionary (lines 483-491) is created regardless of whether it's needed. In update scenarios where no fields changed, this dictionary is unnecessarily constructed. Consider moving this creation inside the conditional blocks where it's actually used to avoid unnecessary computation.
Copilot
AI
Nov 19, 2025
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.
[nitpick] Inefficient update detection: The code compares entire JSON objects using != (line 507), which can be inefficient for large JSON structures. Additionally, this comparison might give false positives if the JSON has the same semantic content but different key ordering. Consider using a more robust comparison method or hashing the JSON for comparison.
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Nov 19, 2025
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.
Inconsistent update pattern: When updating an existing comment without a description (lines 516-518), the code uses IssueComment.objects.filter().update() to set description_id, which bypasses the model's save method and won't trigger signals. This is inconsistent with the creation flow (line 497) which uses super().save(). Consider using the same pattern for both paths.
| IssueComment.objects.filter(pk=self.pk).update(description_id=description.id) | |
| self.description_id = description.id | |
| self.description_id = description.id | |
| super(IssueComment, self).save(update_fields=["description_id"]) |
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.
[nitpick] Missing error handling in migration command: If the bulk_create or bulk_update operations fail, the transaction will roll back but there's no error reporting. Consider adding try-except blocks to log specific errors and provide meaningful feedback when the migration encounters issues, especially for debugging in production environments.