Skip to content

fix(meeting): increase Participant role column length to prevent admission failures #96

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.

Sign up for GitHub

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

Merged
aniebietafia merged 2 commits into from
Jun 6, 2026
+45 −1
Merged

fix(meeting): increase Participant role column length to prevent admission failures #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
526f74e
fix(meeting): increase Participant role column length to prevent admi…
aniebietafia Jun 6, 2026
84570a9
Merge pull request #95 from Brints/feat/schedule_meetings
aniebietafia Jun 6, 2026
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
44 changes: 44 additions & 0 deletions alembic/versions/96532f61566e_increase_participant_role_length.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
"""increase_participant_role_length

Revision ID: 96532f61566e
Revises: 91cf98820af0
Create Date: 2026-06-06 22:34:47.445999

"""

from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision: str = "96532f61566e"
down_revision: Union[str, Sequence[str], None] = "91cf98820af0"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
"""Upgrade schema."""
# ### commands auto generated by Alembic - please adjust! ###
op.alter_column(
"participants",
"role",
existing_type=sa.VARCHAR(length=10),
type_=sa.String(length=20),
existing_nullable=False,
)
# ### end Alembic commands ###


def downgrade() -> None:
"""Downgrade schema."""
# ### commands auto generated by Alembic - please adjust! ###
op.alter_column(
"participants",
"role",
existing_type=sa.String(length=20),
type_=sa.VARCHAR(length=10),
existing_nullable=False,
)
# ### end Alembic commands ###
Comment on lines +34 to +44
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

Downgrade may fail or truncate data if roles exceed 10 characters.

The downgrade path does not validate or handle existing role values that exceed 10 characters (e.g., "participant" = 11 characters). This could result in:

  • PostgreSQL: Migration failure with data truncation error
  • MySQL/MariaDB: Silent data truncation and corruption

Since the PR title mentions preventing "admission failures" and the context shows "participant" is a valid role value, existing production data likely contains 11-character role values.

🛡️ Recommended fix to ensure safe downgrade

Add data validation before the schema change:

 def downgrade() -> None:
     """Downgrade schema."""
+    # Check for role values that would be truncated
+    connection = op.get_bind()
+    result = connection.execute(
+        sa.text("SELECT COUNT(*) FROM participants WHERE LENGTH(role) > 10")
+    )
+    count = result.scalar()
+    if count > 0:
+        raise ValueError(
+            f"Cannot downgrade: {count} participant(s) have role values exceeding 10 characters. "
+            "Please update or remove these records before downgrading."
+        )
+    
     # ### commands auto generated by Alembic - please adjust! ###
     op.alter_column(

Alternatively, add a comment documenting the risk:

 def downgrade() -> None:
-    """Downgrade schema."""
+    """Downgrade schema.
+    
+    WARNING: This downgrade will fail if any participant records have role values
+    longer than 10 characters (e.g., 'participant'). Ensure all role values are
+    shortened to <= 10 characters before running this downgrade.
+    """
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@alembic/versions/96532f61566e_increase_participant_role_length.py` around
lines 34 - 44, The downgrade() migration alters participants.role from length 20
to 10 which will fail or silently truncate existing values >10; before calling
op.alter_column in downgrade(), scan the participants table for any role strings
longer than 10 (e.g., SELECT role WHERE char_length(role) > 10), and either (a)
abort the downgrade with a clear error/log message listing offending values, or
(b) perform a safe migration step to normalize/truncate or map those roles to
permitted 10-char values and write those updates back to the table, then proceed
to call op.alter_column("participants", "role", ...). Ensure this logic
references downgrade(), op.alter_column, and the "participants"."role" column so
the migration explicitly handles or prevents data loss.

2 changes: 1 addition & 1 deletion app/modules/meeting/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class Participant(Base):
left_at: Mapped[datetime | None] = mapped_column(
DateTime(timezone=True), nullable=True
)
role: Mapped[str] = mapped_column(String(10), default="guest", nullable=False)
role: Mapped[str] = mapped_column(String(20), default="guest", nullable=False)

__table_args__ = (
UniqueConstraint("room_id", "user_id", name="uq_participant_room_user"),
Expand Down
Loading