Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .fides/db_dataset.yml
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,9 @@ dataset:
- name: id
data_categories:
- system.operations
- name: parent_id
data_categories:
- system.operations
- name: updated_at
data_categories:
- system.operations
Expand Down
4 changes: 4 additions & 0 deletions changelog/7864-comment-threading-migration.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: Added
description: Added parent_id column to Comment model for threading support
pr: 7864
labels: ["db-migration"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"""add_comment_parent_id

Revision ID: 1724da7ee981
Revises: 6a42f48c23dd
Create Date: 2026-04-08 19:01:40.083985

"""
import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "1724da7ee981"
down_revision = "6a42f48c23dd"
branch_labels = None
depends_on = None


def upgrade():
op.add_column(
"comment", sa.Column("parent_id", sa.String(length=255), nullable=True)
)
op.create_index("ix_comment_parent_id", "comment", ["parent_id"], unique=False)
op.create_foreign_key(
"comment_parent_id_fkey",
"comment",
"comment",
["parent_id"],
["id"],
ondelete="SET NULL",
)


def downgrade():
op.drop_constraint("comment_parent_id_fkey", "comment", type_="foreignkey")
op.drop_index("ix_comment_parent_id", table_name="comment")
op.drop_column("comment", "parent_id")
31 changes: 29 additions & 2 deletions src/fides/api/models/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,35 @@ class Comment(Base):
user_id = Column(
String, ForeignKey("fidesuser.id", ondelete="SET NULL"), nullable=True
)
parent_id = Column(
String(255),
ForeignKey("comment.id", name="comment_parent_id_fkey", ondelete="SET NULL"),
nullable=True,
)
# Not all users in the system have a username, and users can be deleted.
# Store a non-normalized copy of username for these cases.
username = Column(String, nullable=True)
comment_text = Column(String, nullable=False)
comment_type = Column(EnumColumn(CommentType), nullable=False)

__table_args__ = (Index("ix_comment_parent_id", "parent_id"),)

parent = relationship(
"Comment",
remote_side="Comment.id",
foreign_keys=[parent_id],
back_populates="replies",
uselist=False,
)

replies = relationship(
"Comment",
foreign_keys=[parent_id],
back_populates="parent",
uselist=True,
order_by="Comment.created_at",
)

user = relationship(
"FidesUser",
lazy="selectin",
Expand Down Expand Up @@ -129,8 +152,12 @@ class Comment(Base):
)

def delete(self, db: Session) -> None:
"""Delete the comment and all associated references."""
# Delete attachments associated with this comment
"""Delete the comment, its replies, and all associated references."""
# TODO (ENG-3299): When message_to_subject / reply_from_subject CommentTypes
# are added, prevent deletion of comments with those types to preserve
# correspondence threads.
for reply in self.replies:
reply.delete(db)
AttachmentService(db).delete_for_reference(
self.id, AttachmentReferenceType.comment
)
Expand Down
56 changes: 55 additions & 1 deletion tests/ctl/models/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
AttachmentReference,
AttachmentReferenceType,
)
from fides.api.models.comment import Comment, CommentReference, CommentReferenceType
from fides.api.models.comment import (
Comment,
CommentReference,
CommentReferenceType,
CommentType,
)
from fides.api.models.fides_user import FidesUser


Expand Down Expand Up @@ -395,6 +400,55 @@ def mock_get_s3_client(auth_method, storage_secrets):
attachment.delete(db)


def test_reply_relationship(db, comment):
"""Test that parent/replies relationships load correctly on a threaded comment."""
reply = Comment.create(
db,
data={
"user_id": comment.user_id,
"comment_text": "This is a reply",
"comment_type": CommentType.reply,
"parent_id": comment.id,
},
)
db.refresh(comment)

assert reply.parent_id == comment.id
assert reply.parent.id == comment.id
assert reply in comment.replies

reply.delete(db)


def test_delete_parent_comment_deletes_replies(db, user):
"""Test that deleting a parent comment also deletes its replies via delete()."""
parent = Comment.create(
db,
data={
"user_id": user.id,
"comment_text": "Parent comment",
"comment_type": CommentType.note,
},
)
reply = Comment.create(
db,
data={
"user_id": user.id,
"comment_text": "Reply comment",
"comment_type": CommentType.reply,
"parent_id": parent.id,
},
)
parent_id = parent.id
reply_id = reply.id

parent.delete(db)
db.commit()

assert db.query(Comment).filter_by(id=parent_id).first() is None
assert db.query(Comment).filter_by(id=reply_id).first() is None


def test_comment_to_multiple_attachments_relationship(
s3_client, db, comment, multiple_attachments, monkeypatch
):
Expand Down
Loading