feat(comment-replies): add single threading on comments

Author: zzacharo

.gitignore

@@ -69,4 +69,7 @@ target/
 # node modules
 **/*/node_modules
 node_modules
-.python-version
+.python-version
+
+# Claude
+Claude.md

invenio_requests/alembic/2025112501_add_parent_id_and_threading_constraints.py

@@ -0,0 +1,80 @@
+#
+# This file is part of Invenio.
+# Copyright (C) 2025 CERN.
+#
+# Invenio is free software; you can redistribute it and/or modify it
+# under the terms of the MIT License; see LICENSE file for more details.
+
+"""Add parent_id column and parent-child constraints to request_events table."""
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy_utils import UUIDType
+
+# revision identifiers, used by Alembic.
+revision = "2025112501"
+down_revision = "1759321170"
+branch_labels = ()
+depends_on = None
+
+
+def upgrade():
+    """Upgrade database."""
+    # Add parent_id column for parent-child relationships
+    op.add_column(
+        "request_events",
+        sa.Column("parent_id", UUIDType(), nullable=True),
+    )
+
+    # Add index on parent_id for efficient queries
+    op.create_index(
+        op.f("ix_request_events_parent_id"),
+        "request_events",
+        ["parent_id"],
+        unique=False,
+    )
+
+    # Add unique constraint on (id, request_id)
+    # This ensures each event ID is unique within a request
+    op.create_unique_constraint(
+        "uq_request_events_id_request",
+        "request_events",
+        ["id", "request_id"],
+    )
+
+    # Add foreign key constraint for parent-child relationship
+    # Ensures a child's parent is in the same request
+    op.create_foreign_key(
+        "fk_request_events_parent_id_request_id",
+        "request_events",
+        "request_events",
+        ["parent_id", "request_id"],
+        ["id", "request_id"],
+        ondelete="CASCADE",
+    )
+
+
+def downgrade():
+    """Downgrade database."""
+    # Drop foreign key constraint
+    op.drop_constraint(
+        "fk_request_events_parent_id_request_id",
+        "request_events",
+        type_="foreignkey",
+    )
+
+    # Drop unique constraint
+    op.drop_constraint(
+        "uq_request_events_id_request",
+        "request_events",
+        type_="unique",
+    )
+
+    # Drop index
+    op.drop_index(
+        op.f("ix_request_events_parent_id"),
+        table_name="request_events",
+    )
+
+    # Drop parent_id column
+    op.drop_column("request_events", "parent_id")

invenio_requests/config.py

@@ -132,3 +132,10 @@
 
 REQUESTS_REVIEWERS_MAX_NUMBER = 15
 """Maximum number of reviewers allowed for a request."""
+
+REQUESTS_COMMENT_PREVIEW_LIMIT = 5
+"""Number of most recent child comments to inline in parent's search index.
+
+This limits the size of indexed documents when comments have many replies.
+Additional replies can be loaded via pagination.
+"""

invenio_requests/customizations/event_types.py

@@ -46,6 +46,13 @@ def payload_schema():
     payload_required = False
     """Require the event payload."""
 
+    allow_children = False
+    """Allow this event type to have children (parent-child relationships).
+
+    If True, events of this type can have a parent_id and children.
+    If False, attempting to set a parent_id will raise an error.
+    """
+
     def __init__(self, payload=None):
         """Constructor."""
         self.payload = payload or {}
@@ -115,6 +122,8 @@ class LogEventType(EventType):
     """Log event type."""
 
     type_id = "L"
+    allow_children = True
+    """Allow log events to preserve parent-child structure (e.g., deleted comments)."""
 
     def payload_schema():
         """Return payload schema as a dictionary."""
@@ -162,6 +171,9 @@ class CommentEventType(EventType):
 
     type_id = "C"
 
+    allow_children = True
+    """Comments support parent-child relationships (replies to comments)."""
+
     def payload_schema():
         """Return payload schema as a dictionary."""
         # we need to import here because of circular imports

invenio_requests/errors.py

@@ -47,3 +47,36 @@ def __init__(self, action, reason=None):
         """
         reason = reason or "Could not execute the action"
         super().__init__(action, reason)
+
+
+class ChildrenNotSupportedError(Exception):
+    """Exception raised when children are attempted on an event type that doesn't support it."""
+
+    def __init__(self, event_type, message=None):
+        """Constructor.
+
+        :param event_type: The event type that doesn't support children.
+        :param message: Optional custom error message.
+        """
+        self.event_type = event_type
+        self.message = message or (
+            f"Event type '{event_type}' does not support children. "
+            "Only event types with allow_children=True can have parent-child relationships."
+        )
+        super().__init__(self.message)
+
+
+class NestedChildrenNotAllowedError(Exception):
+    """Exception raised when attempting to create nested children (reply to a reply)."""
+
+    def __init__(self, message=None):
+        """Constructor.
+
+        :param message: Optional custom error message.
+        """
+        self.message = message or (
+            "Nested children are not allowed. "
+            "You cannot reply to a comment that is already a reply. "
+            "Only one level of parent-child relationships is supported."
+        )
+        super().__init__(self.message)

invenio_requests/records/api.py

@@ -10,14 +10,19 @@
 
 from enum import Enum
 from functools import partial
+from werkzeug.utils import cached_property
 
 from invenio_records.dumpers import SearchDumper
 from invenio_records.systemfields import ConstantField, DictField, ModelField
 from invenio_records_resources.records.api import Record
 from invenio_records_resources.records.systemfields import IndexField
 
 from ..customizations import RequestState as State
-from .dumpers import CalculatedFieldDumperExt, GrantTokensDumperExt
+from .dumpers import (
+    CalculatedFieldDumperExt,
+    GrantTokensDumperExt,
+    ParentChildDumperExt,
+)
 from .models import RequestEventModel, RequestMetadata
 from .systemfields import (
     EntityReferenceField,
@@ -51,6 +56,13 @@ class RequestEvent(Record):
 
     model_cls = RequestEventModel
 
+    dumper = SearchDumper(
+        extensions=[
+            ParentChildDumperExt(),
+        ]
+    )
+    """Search dumper with parent-child relationship extension."""
+
     # Systemfields
     metadata = None
 
@@ -83,6 +95,36 @@ class RequestEvent(Record):
     created_by = EntityReferenceField("created_by", check_referenced)
     """Who created the event."""
 
+    parent_id = ModelField("parent_id")
+    """The parent event ID for parent-child relationships."""
+
+    parent_child = DictField("parent_child")
+    """OpenSearch join relationship for parent-child queries."""
+
+    @cached_property
+    def parent(self):
+        """Get the parent event as a RequestEvent instance.
+
+        Returns None if this is a top-level event or if the event type
+        doesn't support children.
+        """
+        if not self.model or not self.model.parent:
+            return None
+
+        # Use the SQLAlchemy relationship to get parent (efficient)
+        parent_model = self.model.parent
+        return type(self)(parent_model.data, model=parent_model)
+
+    def pre_commit(self):
+        """Hook called before committing the record.
+
+        Validates that children are allowed for this event type.
+        """
+        from .validators import validate_children_allowed
+
+        validate_children_allowed(self)
+        super().pre_commit()
+
 
 class Request(Record):
     """A generic request record."""

invenio_requests/records/dumpers/__init__.py

@@ -9,8 +9,10 @@
 
 from .calculated import CalculatedFieldDumperExt
 from .granttokens import GrantTokensDumperExt
+from .join import ParentChildDumperExt
 
 __all__ = (
     "CalculatedFieldDumperExt",
     "GrantTokensDumperExt",
+    "ParentChildDumperExt",
 )

invenio_requests/records/dumpers/join.py

@@ -0,0 +1,48 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2025 CERN.
+#
+# Invenio-Requests is free software; you can redistribute it and/or modify
+# it under the terms of the MIT License; see LICENSE file for more details.
+
+"""Search dumper for OpenSearch join relationships."""
+
+from invenio_records.dumpers import SearchDumperExt
+
+
+class ParentChildDumperExt(SearchDumperExt):
+    """Search dumper extension for OpenSearch join relationships.
+
+    This dumper sets the join relationship field for parent-child documents:
+    - Parent events: {"name": "parent"}
+    - Child events (replies): {"name": "child", "parent": parent_id}
+
+    It also handles routing to ensure child documents are indexed on the
+    same shard as their parent (required for join queries to work).
+    """
+
+    def dump(self, record, data):
+        """Dump the join relationship data.
+
+        Sets the parent_child field based on whether the record
+        is a parent event or a child (reply).
+        """
+        if record.parent_id:
+            # This is a child event (reply)
+            data["parent_child"] = {
+                "name": "child",
+                "parent": str(record.parent_id)
+            }
+        else:
+            # This is a parent event
+            data["parent_child"] = {
+                "name": "parent"
+            }
+
+    def load(self, data, record_cls):
+        """Load the data.
+
+        The join relationship is only used in the search index,
+        not in the record data, so we remove it when loading.
+        """
+        data.pop("parent_child", None)

invenio_requests/records/mappings/os-v1/requestevents/requestevent-v1.0.0.json

@@ -47,6 +47,15 @@
       "payload": {
         "type": "object",
         "enabled": false
+      },
+      "parent_id": {
+        "type": "keyword"
+      },
+      "parent_child": {
+        "type": "join",
+        "relations": {
+          "parent": "child"
+        }
       }
     }
   }

invenio_requests/records/mappings/os-v2/requestevents/requestevent-v1.0.0.json

@@ -47,6 +47,15 @@
       "payload": {
         "type": "object",
         "enabled": false
+      },
+      "parent_id": {
+        "type": "keyword"
+      },
+      "parent_child": {
+        "type": "join",
+        "relations": {
+          "parent": "child"
+        }
       }
     }
   }