plane icon indicating copy to clipboard operation
plane copied to clipboard

[WEB-5312] migration: reply for work item comments

Open sangeethailango opened this issue 1 month ago • 2 comments

Description

  • Added management command to create the description for all the issue comments
  • Added description and parent_id field in IssueComment
  • Added sync logic in the IssueComment module to sync the IssueComment and Description.

Summary by CodeRabbit

  • New Features

    • Issue comments now support threaded replies and maintain a linked rich-text description that stays synchronized on create/update.
  • Tests

    • Added comprehensive unit tests covering creation, updates, legacy-data handling, and HTML stripping for issue comments and their descriptions.
  • Chores

    • Added a migration/utility to backfill existing comments into the new description records.

sangeethailango avatar Nov 06 '25 09:11 sangeethailango

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

makeplane[bot] avatar Nov 06 '25 09:11 makeplane[bot]

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Adds nullable OneToOne description and nullable self-referential parent to IssueComment; makes IssueComment inherit ChangeTrackerMixin; updates save() to compute/strip comment HTML and create/update linked Description within transactions; adds a backfill management command, migration, and unit tests covering sync and legacy cases.

Changes

Cohort / File(s) Summary
Model updates
apps/api/plane/db/models/issue.py
Imported Description and ChangeTrackerMixin. IssueComment now extends ChangeTrackerMixin, ProjectBaseModel. Added nullable description OneToOneField to db.Description (related_name=issue_comment_description) and nullable/blank parent ForeignKey to self (related_name=parent_issue_comment). Added TRACKED_FIELDS = ["comment_stripped","comment_json","comment_html"]. Updated save() to compute comment_stripped, create a Description on create or when missing, and detect/update existing Description when tracked fields change; all writes wrapped in transaction.atomic().
Backfill command
apps/api/plane/db/management/commands/copy_issue_comment_to_description.py
New management command batching IssueComment rows with null description_id (500 per batch), mapping IssueComment fields to Description fields, bulk_createing Description records within atomic transactions, then bulk_updateing IssueComment rows to set description_id; repeats until none remain.
Migration
apps/api/plane/db/migrations/0109_issuecomment_description_and_parent_id.py
New migration adding description OneToOneField (to db.description, null, on_delete=CASCADE, related_name=issue_comment_description) and parent ForeignKey (to db.issuecomment, null, blank, on_delete=CASCADE, related_name=parent_issue_comment). Depends on 0108_alter_issueactivity_issue_comment.
Tests
apps/api/plane/tests/unit/models/test_issue_comment_modal.py
New unit tests verifying Description creation on IssueComment create, synchronization of description fields on updates (html/json/stripped), selective-field updates, no-op when unchanged, recovery when Description is missing (legacy), and HTML stripping/empty-html behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant IssueComment
    participant Description
    participant DB

    rect rgba(200,220,255,0.12)
    note over Client,IssueComment: Create IssueComment
    Client->>IssueComment: create(comment_html, comment_json)
    IssueComment->>IssueComment: save() — compute comment_stripped, detect create
    IssueComment->>Description: build Description from comment fields
    Description->>DB: INSERT
    DB-->>Description: id
    IssueComment->>IssueComment: set description_id
    IssueComment->>DB: INSERT IssueComment (atomic)
    DB-->>Client: created IssueComment
    end

    rect rgba(220,255,220,0.12)
    note over Client,IssueComment: Update IssueComment
    Client->>IssueComment: update(comment_html/json)
    IssueComment->>IssueComment: save() — detect changes in TRACKED_FIELDS
    alt linked Description exists
        IssueComment->>Description: UPDATE description_html/json/stripped
        Description->>DB: UPDATE (atomic)
    else linked Description missing (legacy)
        IssueComment->>Description: CREATE new Description
        Description->>DB: INSERT
        DB-->>Description: id
        IssueComment->>IssueComment: set description_id
    end
    IssueComment->>DB: UPDATE IssueComment (atomic)
    DB-->>Client: updated IssueComment
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review IssueComment.save() for correct detection of tracked-field changes, comment_stripped computation, and transaction boundaries.
  • Verify backfill command batching, field mapping, bulk_create usage, and subsequent bulk_update correctness.
  • Confirm migration field attributes (nullability, on_delete, related_name) match model and tests.
  • Inspect unit tests for coverage of legacy/missing Description cases and HTML stripping behavior.

Poem

🐇 I hopped through fields and stitched each thread,
Copied html to description, and nudged the dead,
Parents linked, old gaps now mend,
I sync, I backfill, then happily send.
🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the main changes but lacks Type of Change selection, test scenarios, and references sections required by the template. Add missing sections: select appropriate type of change (likely Feature), describe test scenarios performed, and link the related issue WEB-5312.
Title check ❓ Inconclusive The title '[WEB-5312] migration: work item comments' is vague and generic—it uses non-descriptive terms ('work item comments', 'migration') that don't clearly convey the specific change. It doesn't distinguish this from other potential migration PRs. Revise the title to be more specific about the core change, e.g., '[WEB-5312] Add description field sync to IssueComment model' or '[WEB-5312] Migrate issue comments to description records'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 86.67% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch chore-description-in-issue-comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 06 '25 09:11 coderabbitai[bot]