Fix consultation delete, get rid of SearchableAssociatedFiles

The `SearchableAssociatedFiles` mixin was ultimately causing more
issues than benefits. Accidental, not necessary complexity.
We are better of getting rid of it. KISS

1) First of all, the observer never quite worked because of using
   associated in a Mixin.
2) Secondly, it's unlikely we'll have many many models with files to
   search in the future, so the benefits of having this mixin are not
   so great.
3) Thirdly, this was also non-trivial for type checking.

Also fixed delete consultation with files / comments by adding
`ondelete='CASCADE')` to the Foreignkey.

src/privatim/__init__.py

@@ -2,11 +2,13 @@
 from fanstatic import Fanstatic
 
 from pyramid.events import BeforeRender
+from sqlalchemy.dialects.postgresql import TSVECTOR
+
 from privatim import helpers
 from privatim.layouts.action_menu import ActionMenuEntry
 from pyramid.config import Configurator
 from pyramid_beaker import session_factory_from_settings
-from sqlalchemy import Column, ForeignKey, String, TIMESTAMP, func
+from sqlalchemy import Column, ForeignKey, String, TIMESTAMP, func, Computed
 from email.headerregistry import Address
 from privatim.mail import PostmarkMailer
 from privatim.orm.uuid_type import UUIDStr as UUIDStrType
@@ -191,63 +193,6 @@ def upgrade(context: 'UpgradeContext'):  # type: ignore[no-untyped-def]
     context.drop_column('user', 'statements')
     context.drop_table('statements')
 
-    # for column in [
-    #     'title',
-    #     'description',
-    #     'recommendation',
-    #     'evaluation_result',
-    #     'decision',
-    # ]:
-    #     if context.has_column('consultations', column):
-    #         col_info = context.get_column_info('consultations', column)
-    #         if col_info and not isinstance(col_info['type'], MarkupText):
-    #             context.operations.alter_column(
-    #                 'consultations',
-    #                 column,
-    #                 type_=MarkupText,
-    #                 existing_type=col_info['type'],
-    #                 nullable=col_info['nullable']
-    #             )
-    #
-    # # Upgrade Meeting model
-    # for column in ['name', 'decisions']:
-    #     if context.has_column('meetings', column):
-    #         col_info = context.get_column_info('meetings', column)
-    #         if col_info and not isinstance(col_info['type'], MarkupText):
-    #             context.operations.alter_column(
-    #                 'meetings',
-    #                 column,
-    #                 type_=MarkupText,
-    #                 existing_type=col_info['type'],
-    #                 nullable=col_info['nullable']
-    #             )
-    # # Upgrade AgendaItem model
-    # for column in ['title', 'description']:
-    #     if context.has_column('agenda_items', column):
-    #         col_info = context.get_column_info('agenda_items', column)
-    #         if col_info and not isinstance(col_info['type'], MarkupText):
-    #             context.operations.alter_column(
-    #                 'agenda_items',
-    #                 column,
-    #                 type_=MarkupText,
-    #                 existing_type=col_info['type'],
-    #                 nullable=col_info['nullable']
-    #             )
-    #
-    # # Upgrade Comment model
-    # if context.has_table('comments'):
-    #     if context.has_column('comments', 'content'):
-    #         col_info = context.get_column_info('comments', 'content')
-    #         if col_info and not isinstance(col_info['type'], MarkupText):
-    #             context.operations.alter_column(
-    #                 'comments',
-    #                 'content',
-    #                 type_=MarkupText,
-    #                 existing_type=col_info['type'],
-    #                 nullable=col_info['nullable']
-    #             )
-    #
-
     if not context.has_column('meetings', 'creator_id'):
         context.add_column(
             'meetings',
@@ -279,4 +224,45 @@ def upgrade(context: 'UpgradeContext'):  # type: ignore[no-untyped-def]
         )
     )
 
+    # New changes for consultations and files
+
+    # 1. Drop the association table
+    context.drop_table('searchable_files_for_consultations_files')
+
+    # 2. Add consultation_id to searchable_files
+    context.add_column(
+        'searchable_files',
+        Column(
+            'consultation_id',
+            UUIDStrType,
+            ForeignKey('consultations.id'),
+            nullable=True,
+        ),
+    )
+
+    # 3. drop previous
+    context.drop_column('consultations', 'searchable_text_de_CH')
+
+    context.add_column(
+        'searchable_files',
+        Column(
+            'searchable_text_de_CH',
+            TSVECTOR,
+            Computed(
+                "to_tsvector('german', COALESCE(extract, ''))",
+                persisted=True
+            ),
+            nullable=True,
+        ),
+
+    )
+
+    # this needs to be added in the second run
+    # context.operations.create_index(
+    #     'idx_searchable_files_searchable_text_de_CH',
+    #     'searchable_files',
+    #     ['searchable_text_de_CH'],
+    #     postgresql_using='gin',
+    # )
+
     context.commit()

src/privatim/cli/helpers.py

@@ -1,8 +1,7 @@
 import click
 from pyramid.paster import bootstrap
-from sqlalchemy import select, func
+from sqlalchemy import select
 
-from privatim.models.associated_file import SearchableAssociatedFiles
 from privatim.models.file import SearchableFile
 from privatim.orm import Base
 
@@ -21,7 +20,7 @@ def print_tsvectors(config_uri: str) -> None:
         seen = set()
         for mapper in Base.registry.mappers:
             cls = mapper.class_
-            if issubclass(cls, SearchableAssociatedFiles) and cls not in seen:
+            if issubclass(cls, SearchableFile) and cls not in seen:
                 seen.add(cls)
                 click.echo(f"\nProcessing model: {cls.__name__}")
                 stmt = select(cls.searchable_text_de_CH)
@@ -30,57 +29,3 @@ def print_tsvectors(config_uri: str) -> None:
                     click.echo(f"ID: {id}")
                     click.echo(f"TSVector: {tsvector}")
                     click.echo("---")
-
-
-@click.command()
-@click.argument('config_uri')
-def print_text(config_uri: str) -> None:
-    """
-    Iterate over all models inheriting from SearchableAssociatedFiles
-    and print their tsvector of searchable_text.
-    """
-    env = bootstrap(config_uri)
-
-    with env['request'].tm:
-        db = env['request'].dbsession
-        seen = set()
-        for mapper in Base.registry.mappers:
-            cls = mapper.class_
-            if issubclass(cls, SearchableAssociatedFiles) and cls not in seen:
-                seen.add(cls)
-                click.echo(f"\nProcessing model: {cls.__name__}")
-                texts2 = db.execute(select(
-                    func.string_agg(SearchableFile.extract, ' ')).select_from(
-                    cls).join(cls.files).group_by(cls.id)).all()
-                for content in texts2:
-                    click.echo(f"text_contents: {content}")
-                    click.echo("---")
-
-
-@click.command()
-@click.argument('config_uri')
-def reindex(config_uri: str) -> None:
-
-    env = bootstrap(config_uri)
-
-    with env['request'].tm:
-        db = env['request'].dbsession
-        seen = set()
-        for mapper in Base.registry.mappers:
-            cls = mapper.class_
-            if issubclass(cls, SearchableAssociatedFiles) and cls not in seen:
-                seen.add(cls)
-                click.echo(f"\nProcessing model: {cls.__name__}")
-
-                stmt = select(cls)
-                results = db.execute(stmt).scalars().fetchall()
-                for instance in results:
-                    assert isinstance(instance, cls)
-                    name = getattr(instance, 'title', None)
-                    if name is not None:
-                        click.echo(f"\nReindexing model: {cls.__name__} with "
-                                   f"title: {name[:30]}")
-                    else:
-                        click.echo(f"\nReindexing model: {cls.__name__} with")
-
-                    instance.reindex_files()

src/privatim/models/consultation.py

@@ -6,9 +6,7 @@
 from sqlalchemy.orm import Mapped, mapped_column, relationship
 from pyramid.authorization import Allow
 from pyramid.authorization import Authenticated
-from sqlalchemy_utils import observes  # type: ignore[import-untyped]
 
-from privatim.models.associated_file import SearchableAssociatedFiles
 from privatim.models.commentable import Commentable
 from privatim.models.searchable import SearchableMixin
 from privatim.orm import Base
@@ -19,12 +17,12 @@
 
 from typing import TYPE_CHECKING, Iterator
 if TYPE_CHECKING:
+    from sqlalchemy.dialects.postgresql import TSVECTOR
     from privatim.types import ACL
     from sqlalchemy.orm import InstrumentedAttribute
     from privatim.models import User
     from privatim.models.file import SearchableFile
     from privatim.models.comment import Comment
-    from sqlalchemy.dialects.postgresql import TSVECTOR
 
 
 class Status(Base):
@@ -82,9 +80,7 @@ def __repr__(self) -> str:
         return f'<Tag {self.name}>'
 
 
-class Consultation(  # type: ignore[misc]
-    Base, Commentable, SearchableAssociatedFiles, SearchableMixin
-):
+class Consultation(Base, Commentable, SearchableMixin):
     """Vernehmlassung (Verfahren der Stellungnahme zu einer öffentlichen
     Frage)"""
 
@@ -175,15 +171,18 @@ def __init__(
         'Consultation',
         back_populates='previous_version',
         foreign_keys='Consultation.replaced_consultation_id',
-        remote_side='Consultation.id'
+        remote_side='Consultation.id',
+        cascade='all, delete'
     )
     replaced_consultation_id: Mapped[UUIDStrType] = mapped_column(
-        ForeignKey('consultations.id'), nullable=True
+        ForeignKey('consultations.id', ondelete='CASCADE'),
+        nullable=True
     )
     previous_version: Mapped['Consultation | None'] = relationship(
         'Consultation',
         back_populates='replaced_by',
-        foreign_keys=[replaced_consultation_id]
+        foreign_keys=[replaced_consultation_id],
+        cascade='all, delete'
     )
 
     # Querying the latest version is undoubtedly a common operation.
@@ -193,6 +192,12 @@ def __init__(
         Integer, default=1, index=True,
     )
 
+    files: Mapped[list['SearchableFile']] = relationship(
+        'SearchableFile',
+        back_populates='consultation',
+        cascade='all, delete-orphan'
+    )
+
     @classmethod
     def searchable_fields(
         cls,
@@ -201,19 +206,6 @@ def searchable_fields(
             if field is not None:
                 yield field
 
-    @observes('files')
-    def files_observer(self, files: list['SearchableFile']) -> None:
-        """
-        Observer method for the 'files' relationship.
-        While potentially inefficient for large collections, it's typically
-        fine as the number of files is expected to be small (1-5). Consider
-        optimizing if performance issues arise.
-
-        This would optimally be inside SearchableAssociatedFiles, but it's a
-        tricky, type checking wise and otherwise.
-        """
-        self.reindex_files()
-
     def __repr__(self) -> str:
         return (
             f'<Consultation {self.title}, searchable_text_de_CH: '