From 79f161436e53392389653434cfda98858c8bd552 Mon Sep 17 00:00:00 2001 From: Igor Monadical Date: Fri, 12 Sep 2025 13:07:58 -0400 Subject: [PATCH] chore: meeting user id removal and room id requirement (#635) * chore: remove meeting user id and make meeting room id required * meeting room_id optional * orphaned meeting room ids DATA migration * ci fix * fix meeting_room_id_fkey downgrade * fix migration rollback * fix: put index back (meeting room id) * fix: put index back (meeting room id) * fix: put index back (meeting room id) * remove noop migrations --------- Co-authored-by: Igor Loskutov --- ...da2ee_remove_user_id_from_meeting_table.py | 36 ++++++++++++++++++ ...lean_up_orphaned_room_id_references_in_.py | 32 ++++++++++++++++ ..._make_meeting_room_id_required_and_add_.py | 38 +++++++++++++++++++ ...make_meeting_room_id_nullable_but_keep_.py | 34 +++++++++++++++++ server/reflector/db/meetings.py | 34 +++++------------ server/reflector/views/rooms.py | 1 - server/tests/test_cleanup.py | 2 - 7 files changed, 150 insertions(+), 27 deletions(-) create mode 100644 server/migrations/versions/0ce521cda2ee_remove_user_id_from_meeting_table.py create mode 100644 server/migrations/versions/2ae3db106d4e_clean_up_orphaned_room_id_references_in_.py create mode 100644 server/migrations/versions/6dec9fb5b46c_make_meeting_room_id_required_and_add_.py create mode 100644 server/migrations/versions/def1b5867d4c_make_meeting_room_id_nullable_but_keep_.py diff --git a/server/migrations/versions/0ce521cda2ee_remove_user_id_from_meeting_table.py b/server/migrations/versions/0ce521cda2ee_remove_user_id_from_meeting_table.py new file mode 100644 index 00000000..2e76e8a6 --- /dev/null +++ b/server/migrations/versions/0ce521cda2ee_remove_user_id_from_meeting_table.py @@ -0,0 +1,36 @@ +"""remove user_id from meeting table + +Revision ID: 0ce521cda2ee +Revises: 6dec9fb5b46c +Create Date: 2025-09-10 12:40:55.688899 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "0ce521cda2ee" +down_revision: Union[str, None] = "6dec9fb5b46c" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("meeting", schema=None) as batch_op: + batch_op.drop_column("user_id") + + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("meeting", schema=None) as batch_op: + batch_op.add_column( + sa.Column("user_id", sa.VARCHAR(), autoincrement=False, nullable=True) + ) + + # ### end Alembic commands ### diff --git a/server/migrations/versions/2ae3db106d4e_clean_up_orphaned_room_id_references_in_.py b/server/migrations/versions/2ae3db106d4e_clean_up_orphaned_room_id_references_in_.py new file mode 100644 index 00000000..c091ab49 --- /dev/null +++ b/server/migrations/versions/2ae3db106d4e_clean_up_orphaned_room_id_references_in_.py @@ -0,0 +1,32 @@ +"""clean up orphaned room_id references in meeting table + +Revision ID: 2ae3db106d4e +Revises: def1b5867d4c +Create Date: 2025-09-11 10:35:15.759967 + +""" + +from typing import Sequence, Union + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "2ae3db106d4e" +down_revision: Union[str, None] = "def1b5867d4c" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # Set room_id to NULL for meetings that reference non-existent rooms + op.execute(""" + UPDATE meeting + SET room_id = NULL + WHERE room_id IS NOT NULL + AND room_id NOT IN (SELECT id FROM room WHERE id IS NOT NULL) + """) + + +def downgrade() -> None: + # Cannot restore orphaned references - no operation needed + pass diff --git a/server/migrations/versions/6dec9fb5b46c_make_meeting_room_id_required_and_add_.py b/server/migrations/versions/6dec9fb5b46c_make_meeting_room_id_required_and_add_.py new file mode 100644 index 00000000..20828c65 --- /dev/null +++ b/server/migrations/versions/6dec9fb5b46c_make_meeting_room_id_required_and_add_.py @@ -0,0 +1,38 @@ +"""make meeting room_id required and add foreign key + +Revision ID: 6dec9fb5b46c +Revises: 61882a919591 +Create Date: 2025-09-10 10:47:06.006819 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "6dec9fb5b46c" +down_revision: Union[str, None] = "61882a919591" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("meeting", schema=None) as batch_op: + batch_op.alter_column("room_id", existing_type=sa.VARCHAR(), nullable=False) + batch_op.create_foreign_key( + None, "room", ["room_id"], ["id"], ondelete="CASCADE" + ) + + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("meeting", schema=None) as batch_op: + batch_op.drop_constraint("meeting_room_id_fkey", type_="foreignkey") + batch_op.alter_column("room_id", existing_type=sa.VARCHAR(), nullable=True) + + # ### end Alembic commands ### diff --git a/server/migrations/versions/def1b5867d4c_make_meeting_room_id_nullable_but_keep_.py b/server/migrations/versions/def1b5867d4c_make_meeting_room_id_nullable_but_keep_.py new file mode 100644 index 00000000..982bea27 --- /dev/null +++ b/server/migrations/versions/def1b5867d4c_make_meeting_room_id_nullable_but_keep_.py @@ -0,0 +1,34 @@ +"""make meeting room_id nullable but keep foreign key + +Revision ID: def1b5867d4c +Revises: 0ce521cda2ee +Create Date: 2025-09-11 09:42:18.697264 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "def1b5867d4c" +down_revision: Union[str, None] = "0ce521cda2ee" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("meeting", schema=None) as batch_op: + batch_op.alter_column("room_id", existing_type=sa.VARCHAR(), nullable=True) + + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("meeting", schema=None) as batch_op: + batch_op.alter_column("room_id", existing_type=sa.VARCHAR(), nullable=False) + + # ### end Alembic commands ### diff --git a/server/reflector/db/meetings.py b/server/reflector/db/meetings.py index bb7366b1..c3821241 100644 --- a/server/reflector/db/meetings.py +++ b/server/reflector/db/meetings.py @@ -17,8 +17,12 @@ meetings = sa.Table( sa.Column("host_room_url", sa.String), sa.Column("start_date", sa.DateTime(timezone=True)), sa.Column("end_date", sa.DateTime(timezone=True)), - sa.Column("user_id", sa.String), - sa.Column("room_id", sa.String), + sa.Column( + "room_id", + sa.String, + sa.ForeignKey("room.id", ondelete="CASCADE"), + nullable=True, + ), sa.Column("is_locked", sa.Boolean, nullable=False, server_default=sa.false()), sa.Column("room_mode", sa.String, nullable=False, server_default="normal"), sa.Column("recording_type", sa.String, nullable=False, server_default="cloud"), @@ -80,8 +84,7 @@ class Meeting(BaseModel): host_room_url: str start_date: datetime end_date: datetime - user_id: str | None = None - room_id: str | None = None + room_id: str | None is_locked: bool = False room_mode: Literal["normal", "group"] = "normal" recording_type: Literal["none", "local", "cloud"] = "cloud" @@ -100,12 +103,8 @@ class MeetingController: host_room_url: str, start_date: datetime, end_date: datetime, - user_id: str, room: Room, ): - """ - Create a new meeting - """ meeting = Meeting( id=id, room_name=room_name, @@ -113,7 +112,6 @@ class MeetingController: host_room_url=host_room_url, start_date=start_date, end_date=end_date, - user_id=user_id, room_id=room.id, is_locked=room.is_locked, room_mode=room.room_mode, @@ -125,19 +123,13 @@ class MeetingController: return meeting async def get_all_active(self) -> list[Meeting]: - """ - Get active meetings. - """ query = meetings.select().where(meetings.c.is_active) return await get_database().fetch_all(query) async def get_by_room_name( self, room_name: str, - ) -> Meeting: - """ - Get a meeting by room name. - """ + ) -> Meeting | None: query = meetings.select().where(meetings.c.room_name == room_name) result = await get_database().fetch_one(query) if not result: @@ -145,10 +137,7 @@ class MeetingController: return Meeting(**result) - async def get_active(self, room: Room, current_time: datetime) -> Meeting: - """ - Get latest active meeting for a room. - """ + async def get_active(self, room: Room, current_time: datetime) -> Meeting | None: end_date = getattr(meetings.c, "end_date") query = ( meetings.select() @@ -168,9 +157,6 @@ class MeetingController: return Meeting(**result) async def get_by_id(self, meeting_id: str, **kwargs) -> Meeting | None: - """ - Get a meeting by id - """ query = meetings.select().where(meetings.c.id == meeting_id) result = await get_database().fetch_one(query) if not result: @@ -201,7 +187,7 @@ class MeetingConsentController: result = await get_database().fetch_one(query) if result is None: return None - return MeetingConsent(**result) if result else None + return MeetingConsent(**result) async def upsert(self, consent: MeetingConsent) -> MeetingConsent: """Create new consent or update existing one for authenticated users""" diff --git a/server/reflector/views/rooms.py b/server/reflector/views/rooms.py index 38b611d6..546c1dd3 100644 --- a/server/reflector/views/rooms.py +++ b/server/reflector/views/rooms.py @@ -209,7 +209,6 @@ async def rooms_create_meeting( host_room_url=whereby_meeting["hostRoomUrl"], start_date=parse_datetime_with_timezone(whereby_meeting["startDate"]), end_date=parse_datetime_with_timezone(whereby_meeting["endDate"]), - user_id=user_id, room=room, ) except (asyncpg.exceptions.UniqueViolationError, sqlite3.IntegrityError): diff --git a/server/tests/test_cleanup.py b/server/tests/test_cleanup.py index 3c5149ae..2cb8614c 100644 --- a/server/tests/test_cleanup.py +++ b/server/tests/test_cleanup.py @@ -105,7 +105,6 @@ async def test_cleanup_deletes_associated_meeting_and_recording(): host_room_url="https://example.com/meeting-host", start_date=old_date, end_date=old_date + timedelta(hours=1), - user_id=None, room_id=None, ) ) @@ -241,7 +240,6 @@ async def test_meeting_consent_cascade_delete(): host_room_url="https://example.com/cascade-test-host", start_date=datetime.now(timezone.utc), end_date=datetime.now(timezone.utc) + timedelta(hours=1), - user_id="test-user", room_id=None, ) )