diff --git a/syncmaster/db/migrations/versions/2023-11-23_0002_create_group_table.py b/syncmaster/db/migrations/versions/2023-11-23_0002_create_group_table.py index 1906ac40..2b08c35d 100644 --- a/syncmaster/db/migrations/versions/2023-11-23_0002_create_group_table.py +++ b/syncmaster/db/migrations/versions/2023-11-23_0002_create_group_table.py @@ -27,7 +27,6 @@ def upgrade(): sa.Column("owner_id", sa.BigInteger(), nullable=False), sa.Column("created_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False), sa.Column("updated_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False), - sa.Column("is_deleted", sa.Boolean(), nullable=False), sa.Column( "search_vector", postgresql.TSVECTOR(), diff --git a/syncmaster/db/migrations/versions/2023-11-23_0003_create_queue_table.py b/syncmaster/db/migrations/versions/2023-11-23_0003_create_queue_table.py index 8c3b7a12..78fe16a4 100644 --- a/syncmaster/db/migrations/versions/2023-11-23_0003_create_queue_table.py +++ b/syncmaster/db/migrations/versions/2023-11-23_0003_create_queue_table.py @@ -28,7 +28,6 @@ def upgrade(): sa.Column("description", sa.String(length=512), nullable=False), sa.Column("created_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False), sa.Column("updated_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False), - sa.Column("is_deleted", sa.Boolean(), nullable=False), sa.Column( "search_vector", postgresql.TSVECTOR(), diff --git a/syncmaster/db/migrations/versions/2023-11-23_0007_create_transfer_table.py b/syncmaster/db/migrations/versions/2023-11-23_0007_create_transfer_table.py index 63990771..bc98cc66 100644 --- a/syncmaster/db/migrations/versions/2023-11-23_0007_create_transfer_table.py +++ b/syncmaster/db/migrations/versions/2023-11-23_0007_create_transfer_table.py @@ -47,7 +47,6 @@ def upgrade(): sa.Column("is_scheduled", sa.Boolean(), nullable=False), sa.Column("schedule", sa.String(length=32), nullable=False), sa.Column("queue_id", sa.BigInteger(), nullable=False), - sa.Column("is_deleted", sa.Boolean(), nullable=False), sa.Column("created_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False), sa.Column("updated_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False), sa.ForeignKeyConstraint( diff --git a/syncmaster/db/models/group.py b/syncmaster/db/models/group.py index bc5e4706..5a019b7d 100644 --- a/syncmaster/db/models/group.py +++ b/syncmaster/db/models/group.py @@ -10,7 +10,7 @@ from sqlalchemy.orm import Mapped, mapped_column, relationship from sqlalchemy_utils import ChoiceType -from syncmaster.db.mixins import DeletableMixin, TimestampMixin +from syncmaster.db.mixins import TimestampMixin from syncmaster.db.models.base import Base from syncmaster.db.models.user import User @@ -69,7 +69,7 @@ def roles_at_least(cls, role: str | GroupMemberRole) -> list[str]: return [r.value for r, level in cls.role_hierarchy().items() if level >= required_level] -class Group(Base, TimestampMixin, DeletableMixin): +class Group(Base, TimestampMixin): id: Mapped[int] = mapped_column(BigInteger, primary_key=True) name: Mapped[str] = mapped_column(String(256), nullable=False, unique=True) description: Mapped[str] = mapped_column(String(512), nullable=False, default="") diff --git a/syncmaster/db/models/queue.py b/syncmaster/db/models/queue.py index a71a841f..00104fc4 100644 --- a/syncmaster/db/models/queue.py +++ b/syncmaster/db/models/queue.py @@ -8,7 +8,7 @@ from sqlalchemy.dialects.postgresql import TSVECTOR from sqlalchemy.orm import Mapped, mapped_column, relationship -from syncmaster.db.mixins import DeletableMixin, ResourceMixin, TimestampMixin +from syncmaster.db.mixins import ResourceMixin, TimestampMixin from syncmaster.db.models.base import Base if TYPE_CHECKING: @@ -16,7 +16,7 @@ from syncmaster.db.models.transfer import Transfer -class Queue(Base, ResourceMixin, TimestampMixin, DeletableMixin): +class Queue(Base, ResourceMixin, TimestampMixin): name: Mapped[str] = mapped_column(String(128), nullable=False) slug: Mapped[str] = mapped_column(String(256), nullable=False, unique=True) diff --git a/syncmaster/db/models/transfer.py b/syncmaster/db/models/transfer.py index 4af312bc..c2029871 100644 --- a/syncmaster/db/models/transfer.py +++ b/syncmaster/db/models/transfer.py @@ -17,7 +17,7 @@ from sqlalchemy.dialects.postgresql import TSVECTOR from sqlalchemy.orm import Mapped, declared_attr, mapped_column, relationship -from syncmaster.db.mixins import DeletableMixin, ResourceMixin, TimestampMixin +from syncmaster.db.mixins import ResourceMixin, TimestampMixin from syncmaster.db.models.base import Base from syncmaster.db.models.group import Group @@ -29,7 +29,6 @@ class Transfer( Base, ResourceMixin, - DeletableMixin, TimestampMixin, ): source_connection_id: Mapped[int] = mapped_column( diff --git a/syncmaster/db/repositories/group.py b/syncmaster/db/repositories/group.py index 3ebedf65..2c82b684 100644 --- a/syncmaster/db/repositories/group.py +++ b/syncmaster/db/repositories/group.py @@ -32,7 +32,7 @@ async def paginate_all( page_size: int, search_query: str | None = None, ) -> Pagination: - stmt = select(Group).where(Group.is_deleted.is_(False)) + stmt = select(Group) if search_query: stmt = self._construct_vector_search(stmt, search_query) @@ -73,7 +73,6 @@ async def paginate_for_user( select(Group) .where( Group.owner_id == current_user_id, - Group.is_deleted.is_(False), ) .order_by(Group.name) ) @@ -104,7 +103,6 @@ async def paginate_for_user( .join(Group, UserGroup.group_id == Group.id) .where( UserGroup.user_id == current_user_id, - Group.is_deleted.is_(False), ) .order_by(Group.name) ) @@ -154,7 +152,7 @@ async def read_by_id( self, group_id: int, ) -> Group: - stmt = select(Group).where(Group.id == group_id, Group.is_deleted.is_(False)) + stmt = select(Group).where(Group.id == group_id) try: result: ScalarResult[Group] = await self._session.scalars(stmt) return result.one() @@ -186,7 +184,7 @@ async def update( description: str, owner_id: int, ) -> Group: - args = [Group.id == group_id, Group.is_deleted.is_(False)] + args = [Group.id == group_id] try: return await self._update( *args, @@ -278,7 +276,7 @@ async def get_member_paginate( async def delete(self, group_id: int) -> None: try: await self._delete(group_id) - except EntityNotFoundError as e: + except (EntityNotFoundError, NoResultFound) as e: raise GroupNotFoundError from e async def add_user( diff --git a/syncmaster/db/repositories/queue.py b/syncmaster/db/repositories/queue.py index d6e961ef..9b056a42 100644 --- a/syncmaster/db/repositories/queue.py +++ b/syncmaster/db/repositories/queue.py @@ -38,7 +38,7 @@ async def read_by_id( ) -> Queue: stmt = ( select(Queue) - .where(Queue.id == queue_id, Queue.is_deleted.is_(False)) + .where(Queue.id == queue_id) .options(selectinload(Queue.transfers)) .options(selectinload(Queue.group)) ) @@ -56,7 +56,6 @@ async def paginate( search_query: str | None = None, ): stmt = select(Queue).where( - Queue.is_deleted.is_(False), Queue.group_id == group_id, ) if search_query: @@ -77,7 +76,6 @@ async def update( queue = await self.read_by_id(queue_id=queue_id) return await self._update( Queue.id == queue_id, - Queue.is_deleted.is_(False), description=queue_data.description or queue.description, ) except IntegrityError as e: diff --git a/syncmaster/db/repositories/transfer.py b/syncmaster/db/repositories/transfer.py index 5552fe06..72b5a72a 100644 --- a/syncmaster/db/repositories/transfer.py +++ b/syncmaster/db/repositories/transfer.py @@ -41,7 +41,6 @@ async def paginate( is_scheduled: bool | None = None, ) -> Pagination: stmt = select(Transfer).where( - Transfer.is_deleted.is_(False), Transfer.group_id == group_id, ) @@ -97,7 +96,6 @@ async def read_by_id( select(Transfer) .where( Transfer.id == transfer_id, - Transfer.is_deleted.is_(False), ) .options(selectinload(Transfer.queue)) ) @@ -172,7 +170,6 @@ async def update( strategy_params[key] = transfer.strategy_params[key] return await self._update( Transfer.id == transfer.id, - Transfer.is_deleted.is_(False), name=name or transfer.name, description=description or transfer.description, strategy_params=strategy_params, diff --git a/syncmaster/scheduler/transfer_job_manager.py b/syncmaster/scheduler/transfer_job_manager.py index 1c432a16..98c54edf 100644 --- a/syncmaster/scheduler/transfer_job_manager.py +++ b/syncmaster/scheduler/transfer_job_manager.py @@ -26,7 +26,7 @@ def update_jobs(self, transfers: list[Transfer]) -> None: job_id = str(transfer.id) existing_job = self.scheduler.get_job(job_id) - if not transfer.is_scheduled or transfer.is_deleted: + if not transfer.is_scheduled: if existing_job: self.scheduler.remove_job(job_id) continue diff --git a/tests/test_unit/test_connections/test_copy_connection.py b/tests/test_unit/test_connections/test_copy_connection.py index e8ad780d..efcf4240 100644 --- a/tests/test_unit/test_connections/test_copy_connection.py +++ b/tests/test_unit/test_connections/test_copy_connection.py @@ -44,7 +44,6 @@ async def test_maintainer_plus_can_copy_connection_with_delete_source( "user": "user", "password": "password", } - assert not current.is_deleted query_copy_not_exist = select(Connection).filter( Connection.group_id == empty_group.id, @@ -73,7 +72,6 @@ async def test_maintainer_plus_can_copy_connection_with_delete_source( } assert result.status_code == 200 - await session.refresh(group_connection.connection) query_prev_row = select(Connection).where(Connection.id == curr_id) result_prev_row = await session.scalars(query_prev_row) origin = result_prev_row.one() @@ -100,8 +98,6 @@ async def test_maintainer_plus_can_copy_connection_with_delete_source( "user": "user", "password": "password", } - assert origin.is_deleted == is_delete_source - assert not new.is_deleted assert not creds_new @@ -132,7 +128,6 @@ async def test_superuser_can_copy_connection( "user": "user", "password": "password", } - assert not current.is_deleted query_copy_not_exist = select(Connection).filter( Connection.group_id == empty_group.id, @@ -189,8 +184,6 @@ async def test_superuser_can_copy_connection( "user": "user", "password": "password", } - assert origin.is_deleted == is_delete_source - assert not new.is_deleted assert not creds_new @@ -280,7 +273,7 @@ async def test_not_in_both_groups_user_can_not_copy_connection( query_current_row = select(Connection).where(Connection.id == group_connection.id) result_current_row = await session.scalars(query_current_row) - current = result_current_row.one() + result_current_row.one() query_current_creds = select(AuthData).where(AuthData.connection_id == group_connection.id) result_current_creds = await session.scalars(query_current_creds) @@ -291,7 +284,6 @@ async def test_not_in_both_groups_user_can_not_copy_connection( "user": "user", "password": "password", } - assert not current.is_deleted query_copy_not_exist = select(Connection).filter( Connection.group_id == empty_group.id, @@ -338,7 +330,7 @@ async def test_groupless_user_can_not_copy_connection( query_current_row = select(Connection).where(Connection.id == group_connection.id) result_current_row = await session.scalars(query_current_row) - current = result_current_row.one() + result_current_row.one() query_current_creds = select(AuthData).where(AuthData.connection_id == group_connection.id) result_current_creds = await session.scalars(query_current_creds) @@ -349,7 +341,6 @@ async def test_groupless_user_can_not_copy_connection( "user": "user", "password": "password", } - assert not current.is_deleted query_copy_not_exist = select(Connection).filter( Connection.group_id == empty_group.id, diff --git a/tests/test_unit/test_groups/test_delete_group_by_id.py b/tests/test_unit/test_groups/test_delete_group_by_id.py index f4584fbe..d722fdc4 100644 --- a/tests/test_unit/test_groups/test_delete_group_by_id.py +++ b/tests/test_unit/test_groups/test_delete_group_by_id.py @@ -15,8 +15,7 @@ async def test_only_superuser_can_delete_group( session: AsyncSession, ): # Arrange - group = await session.get(Group, empty_group.group.id) - assert not group.is_deleted + await session.get(Group, empty_group.group.id) # Act result = await client.delete( @@ -33,9 +32,6 @@ async def test_only_superuser_can_delete_group( "message": "Group was deleted", } - await session.refresh(group) - assert group.is_deleted - async def test_not_superuser_cannot_delete_group( client: AsyncClient, diff --git a/tests/test_unit/test_queue/test_delete_queue.py b/tests/test_unit/test_queue/test_delete_queue.py index 618581cc..7a240094 100644 --- a/tests/test_unit/test_queue/test_delete_queue.py +++ b/tests/test_unit/test_queue/test_delete_queue.py @@ -31,11 +31,6 @@ async def test_maintainer_plus_can_delete_queue( } assert result.status_code == 200 - queue = await session.get(Queue, group_queue.id) - await session.refresh(queue) - - assert queue.is_deleted - async def test_superuser_can_delete_queue( client: AsyncClient, @@ -57,11 +52,6 @@ async def test_superuser_can_delete_queue( } assert result.status_code == 200 - queue = await session.get(Queue, group_queue.id) - await session.refresh(queue) - - assert queue.is_deleted - async def test_groupless_user_cannot_delete_queue( client: AsyncClient, diff --git a/tests/test_unit/test_scheduler/test_transfer_job_manager.py b/tests/test_unit/test_scheduler/test_transfer_job_manager.py index 9e56846d..b637e8e7 100644 --- a/tests/test_unit/test_scheduler/test_transfer_job_manager.py +++ b/tests/test_unit/test_scheduler/test_transfer_job_manager.py @@ -45,8 +45,6 @@ async def test_modifying_existing_jobs(transfer_job_manager: TransferJobManager, @pytest.mark.parametrize( "transfer_attr, expected_state, is_existing_job", [ - ("is_deleted", True, True), - ("is_deleted", True, False), ("is_scheduled", False, True), ("is_scheduled", False, False), ], diff --git a/tests/test_unit/test_transfers/test_delete_transfer.py b/tests/test_unit/test_transfers/test_delete_transfer.py index 9ffd971f..b259433f 100644 --- a/tests/test_unit/test_transfers/test_delete_transfer.py +++ b/tests/test_unit/test_transfers/test_delete_transfer.py @@ -16,9 +16,7 @@ async def test_maintainer_plus_can_delete_group_transfer( ): # Arrange user = group_transfer.owner_group.get_member_of_role(role_maintainer_plus) - transfer = await session.get(Transfer, group_transfer.id) - - assert not transfer.is_deleted + await session.get(Transfer, group_transfer.id) # Act result = await client.delete( @@ -33,8 +31,6 @@ async def test_maintainer_plus_can_delete_group_transfer( "status_code": 200, "message": "Transfer was deleted", } - await session.refresh(transfer) - assert transfer.is_deleted async def test_superuser_can_delete_transfer( @@ -56,8 +52,6 @@ async def test_superuser_can_delete_transfer( "status_code": 200, "message": "Transfer was deleted", } - await session.refresh(group_transfer.transfer) - assert group_transfer.is_deleted async def test_groupless_user_cannot_delete_transfer( @@ -106,8 +100,6 @@ async def test_developer_or_below_cannot_delete_transfer( }, } assert result.status_code == 403 - await session.refresh(group_transfer.transfer) - assert not group_transfer.is_deleted async def test_group_member_cannot_delete_other_group_transfer(