-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[WEB-4716] chore: created new description model #7597
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Description and DescriptionVersion Django models (JSON/HTML/binary/stripped fields, UUID PKs, audit FKs to users/project/workspace) with save-time HTML stripping, a migration creating Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant Model as Description / DescriptionVersion
participant Stripper as strip_tags
participant ORM as Django ORM
participant DB as Database
Caller->>Model: save()
activate Model
Model->>Stripper: strip_tags(description_html)
Stripper-->>Model: description_stripped
Model->>ORM: validate & prepare fields
ORM->>DB: INSERT / UPDATE
DB-->>ORM: OK
ORM-->>Model: saved
deactivate Model
Model-->>Caller: return instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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.
Pull Request Overview
This PR introduces new Django models for managing descriptions and their versions, providing a structured way to store and track changes to issue descriptions across different formats (JSON, HTML, binary, and stripped text).
- Adds Description and DescriptionVersion models with support for multiple content formats
- Implements automatic HTML tag stripping in the save method for both models
- Includes database migration to create the necessary tables and relationships
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/api/plane/db/models/description.py | Creates new Description and DescriptionVersion models with workspace relationships and content processing |
| apps/api/plane/db/models/init.py | Exports the new models for use throughout the application |
| apps/api/plane/db/migrations/0101_description_descriptionversion.py | Database migration to create tables for the new models with proper foreign key relationships |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
🧹 Nitpick comments (5)
apps/api/plane/db/models/__init__.py (1)
87-87: Expose Description models here is fine; watch for potential confusion with IssueDescriptionVersion.Importing and re-exporting Description and DescriptionVersion is consistent with the rest of this module. Small nit: we now have both IssueDescriptionVersion and DescriptionVersion in the same namespace, which are distinct but similarly named. This is OK, but be mindful in call sites to import the intended one explicitly to avoid mistakes.
apps/api/plane/db/models/description.py (2)
19-24: Consider adding a composite index to match likely query patterns.Common access patterns will likely filter by workspace and order by created_at. A composite index helps.
Suggested Meta addition:
class Meta: verbose_name = "Description" verbose_name_plural = "Descriptions" db_table = "descriptions" ordering = ("-created_at",) + indexes = [ + models.Index(fields=["workspace", "created_at"], name="descr_ws_created_idx"), + ]If you want, I can update the model and generate the migration for this index.
51-56: Add the same composite index here for parity and query performance.class Meta: verbose_name = "Description Version" verbose_name_plural = "Description Versions" db_table = "description_versions" ordering = ("-created_at",) + indexes = [ + models.Index(fields=["workspace", "created_at"], name="descrv_ws_created_idx"), + models.Index(fields=["description", "created_at"], name="descrv_desc_created_idx"), + ]apps/api/plane/db/migrations/0101_description_descriptionversion.py (2)
88-94: Optionally add indexes aligned with expected access patterns.If you adopt the indexes suggested in the model Meta, regenerate this migration to include them. This will speed queries like “all descriptions in a workspace sorted by created_at” and “versions for a description sorted by created_at.”
I can add the indexes to the models and generate an updated migration if you’d like.
115-124: Minor: primary_key already implies unique + index; redundant flags on id can be avoided (not a blocker).The UUIDField includes primary_key=True which makes it unique and indexed by default. The duplicated unique/db_index often comes from a shared base; no action required unless you are touching the base model.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/api/plane/db/migrations/0101_description_descriptionversion.py(1 hunks)apps/api/plane/db/models/__init__.py(1 hunks)apps/api/plane/db/models/description.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/api/plane/db/models/description.py (1)
15-17: Ignore duplicate workspace field suggestion – this override is intentionalThe
workspacefield onDescription(WorkspaceBaseModel)is deliberately overridden to customize itsrelated_name="descriptions". Django allows subclasses of an abstract base model to redefine inherited fields for precisely this purpose. No removal is necessary.Likely an incorrect or invalid review comment.
apps/api/plane/db/migrations/0101_description_descriptionversion.py (1)
140-146: FK string targets look correct; ensure app label/model name resolution matches runtime.to="db.description" and to="db.workspace" align with Django’s migration conventions. No issues expected.
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: 1
♻️ Duplicate comments (3)
apps/api/plane/db/models/description.py (3)
32-35: Docstring should reflect version-tracking purposeClarify that this model stores historical versions linked to Description.
class DescriptionVersion(WorkspaceBaseModel): - """ - DescriptionVersion is a model that stores the description of an issue. - """ + """ + DescriptionVersion stores historical versions of a Description for auditing + and change tracking over time. + """
22-29: Optional: DRY the save logic via a small mixinBoth models duplicate identical save logic. A simple abstract mixin keeps it in one place.
Alternative to the two save() patches above:
@@ -from django.db import models +from django.db import models from django.utils.html import strip_tags from .workspace import WorkspaceBaseModel +class DescriptionHtmlStripMixin(models.Model): + class Meta: + abstract = True + + # Override these in subclasses if field names differ + _html_field = "description_html" + _stripped_field = "description_stripped" + + def save(self, *args, **kwargs): + update_fields = kwargs.get("update_fields") + if update_fields is None or self._html_field in update_fields: + html = getattr(self, self._html_field, "") or "" + plain = strip_tags(html).strip() + setattr(self, self._stripped_field, plain or None) + if update_fields is not None: + s = set(update_fields) + s.add(self._stripped_field) + kwargs["update_fields"] = s + super().save(*args, **kwargs) + @@ -class Description(WorkspaceBaseModel): +class Description(DescriptionHtmlStripMixin, WorkspaceBaseModel): @@ - def save(self, *args, **kwargs): - # Strip the html tags using html parser - self.description_stripped = ( - None - if (self.description_html == "" or self.description_html is None) - else strip_tags(self.description_html) - ) - super(Description, self).save(*args, **kwargs) + # save() inherited from DescriptionHtmlStripMixin @@ -class DescriptionVersion(WorkspaceBaseModel): +class DescriptionVersion(DescriptionHtmlStripMixin, WorkspaceBaseModel): @@ - def save(self, *args, **kwargs): - # Strip the html tags using html parser - self.description_stripped = ( - None - if (self.description_html == "" or self.description_html is None) - else strip_tags(self.description_html) - ) - super(DescriptionVersion, self).save(*args, **kwargs) + # save() inherited from DescriptionHtmlStripMixinAlso applies to: 51-58
51-58: Mirror robust save(update_fields=...) logic for DescriptionVersionSame issues as Description.save. Normalize blanks to None and ensure derived field is persisted on partial updates, recomputing only when HTML changes.
def save(self, *args, **kwargs): - # Strip the html tags using html parser - self.description_stripped = ( - None - if (self.description_html == "" or self.description_html is None) - else strip_tags(self.description_html) - ) - super(DescriptionVersion, self).save(*args, **kwargs) + update_fields = kwargs.get("update_fields") + if update_fields is None or "description_html" in update_fields: + plain = strip_tags(self.description_html or "").strip() + self.description_stripped = plain or None + if update_fields is not None: + s = set(update_fields) + s.add("description_stripped") + kwargs["update_fields"] = s + super(DescriptionVersion, self).save(*args, **kwargs)
🧹 Nitpick comments (2)
apps/api/plane/db/models/description.py (2)
14-14: Mark derived field as non-editable
description_strippedis computed; settingeditable=Falseavoids accidental admin/form edits.- description_stripped = models.TextField(blank=True, null=True) + description_stripped = models.TextField(blank=True, null=True, editable=False)Apply the same change to DescriptionVersion.
Also applies to: 43-43
"
12-12: Consider default "" for HTML instead of "Using an empty string as the default avoids sentinel HTML. Your new save() logic will already normalize blanks, but this change improves data cleanliness.
- description_html = models.TextField(blank=True, default="<p></p>") + description_html = models.TextField(blank=True, default="")Apply the same change to DescriptionVersion.
Also applies to: 41-41
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/plane/db/migrations/0101_description_descriptionversion.py(1 hunks)apps/api/plane/db/models/description.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/db/migrations/0101_description_descriptionversion.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
Description
this pull request creates a new description and description version model.
Type of Change
Summary by CodeRabbit