From c7e5a3b7106edec5f1a01a6a51fb3f6025c48743 Mon Sep 17 00:00:00 2001 From: Thonyk Date: Thu, 2 Jan 2025 20:19:05 +0100 Subject: [PATCH] Fix: apply suggestions --- app/core/groups/groups_type.py | 1 + app/core/models_core.py | 7 --- app/core/schemas_core.py | 3 - app/core/schools/endpoints_schools.py | 40 ++++--------- app/core/users/cruds_users.py | 86 ++++++++++++++------------- 5 files changed, 59 insertions(+), 78 deletions(-) diff --git a/app/core/groups/groups_type.py b/app/core/groups/groups_type.py index b3007cae2..60faf5bf0 100644 --- a/app/core/groups/groups_type.py +++ b/app/core/groups/groups_type.py @@ -65,4 +65,5 @@ def get_account_types_except_externals() -> list[AccountType]: AccountType.staff, AccountType.association, AccountType.demo, + AccountType.other_school_student, ] diff --git a/app/core/models_core.py b/app/core/models_core.py index 760a38c0e..6c8ac8095 100644 --- a/app/core/models_core.py +++ b/app/core/models_core.py @@ -132,13 +132,6 @@ class CoreSchool(Base): name: Mapped[str] = mapped_column(String, index=True, nullable=False, unique=True) email_regex: Mapped[str] = mapped_column(String, nullable=False) - students: Mapped[list["CoreUser"]] = relationship( - "CoreUser", - back_populates="school", - lazy="selectin", - default_factory=list, - ) - class CoreAssociationMembership(Base): __tablename__ = "core_association_membership" diff --git a/app/core/schemas_core.py b/app/core/schemas_core.py index 109f199c4..7f90bb579 100644 --- a/app/core/schemas_core.py +++ b/app/core/schemas_core.py @@ -78,14 +78,11 @@ class CoreUserSimple(CoreUserBase): id: str account_type: AccountType - model_config = ConfigDict(from_attributes=True) class CoreSchool(CoreSchoolSimple): """Schema for school's model similar to core_school table in database""" - model_config = ConfigDict(from_attributes=True) - students: list[CoreUserSimple] = [] diff --git a/app/core/schools/endpoints_schools.py b/app/core/schools/endpoints_schools.py index 475cde0c5..05c51ba83 100644 --- a/app/core/schools/endpoints_schools.py +++ b/app/core/schools/endpoints_schools.py @@ -32,7 +32,7 @@ ) async def read_schools( db: AsyncSession = Depends(get_db), - user=Depends(is_user_an_ecl_member), + user: schemas_core.CoreUser = Depends(is_user_an_ecl_member), ): """ Return all schools from database as a list of dictionaries @@ -50,7 +50,7 @@ async def read_schools( async def read_school( school_id: str, db: AsyncSession = Depends(get_db), - user=Depends(is_user_in(GroupType.admin)), + user: schemas_core.CoreUser = Depends(is_user_in(GroupType.admin)), ): """ Return school with id from database as a dictionary. This includes a list of users being members of the school. @@ -75,7 +75,7 @@ async def create_school( user: schemas_core.CoreUser = Depends(is_user_in(GroupType.admin)), ): """ - Create a new school. + Create a new school and add users to it based on the email regex. **This endpoint is only usable by administrators** """ @@ -110,15 +110,9 @@ async def create_school( ), ) await db.commit() - except ValueError as error: - await db.rollback() - raise HTTPException(status_code=422, detail=str(error)) except IntegrityError: await db.rollback() - raise HTTPException( - status_code=400, - detail="School creation failed due to database integrity error", - ) + raise else: return db_school @@ -131,7 +125,7 @@ async def update_school( school_id: str, school_update: schemas_core.CoreSchoolUpdate, db: AsyncSession = Depends(get_db), - user=Depends(is_user_in(GroupType.admin)), + user: schemas_core.CoreUser = Depends(is_user_in(GroupType.admin)), ): """ Update the name or the description of a school. @@ -180,7 +174,11 @@ async def update_school( account_type=AccountType.other_school_student, ), ) - await db.commit() + try: + await db.commit() + except IntegrityError: + await db.rollback() + raise @router.delete( @@ -190,7 +188,7 @@ async def update_school( async def delete_school( school_id: str, db: AsyncSession = Depends(get_db), - user=Depends(is_user_in(GroupType.admin)), + user: schemas_core.CoreUser = Depends(is_user_in(GroupType.admin)), ): """ Delete school from database. @@ -211,23 +209,11 @@ async def delete_school( if school is None: raise HTTPException(status_code=404, detail="School not found") - users = await cruds_users.get_users(db=db, schools_ids=[school_id]) - for db_user in users: - await cruds_users.update_user( - db, - db_user.id, - schemas_core.CoreUserUpdateAdmin( - school_id=SchoolType.no_school.value, - account_type=AccountType.external, - ), - ) + await cruds_users.remove_users_from_school(db=db, school_id=school_id) await cruds_schools.delete_school(db=db, school_id=school_id) try: await db.commit() except IntegrityError: await db.rollback() - raise HTTPException( - status_code=400, - detail="School deletion failed due to database integrity error", - ) + raise diff --git a/app/core/users/cruds_users.py b/app/core/users/cruds_users.py index 67f9ebced..660bbb066 100644 --- a/app/core/users/cruds_users.py +++ b/app/core/users/cruds_users.py @@ -39,51 +39,55 @@ async def get_users( excluded_groups = excluded_groups or [] schools_ids = schools_ids or None + # We want, for each group that should be included check if + # - at least one of the user's groups match the expected group + included_group_condition = [ + models_core.CoreUser.groups.any( + models_core.CoreGroup.id == group_id, + ) + for group_id in included_groups + ] + included_account_type_condition = or_( + False, + *[ + models_core.CoreUser.account_type == account_type + for account_type in included_account_types + ], + ) + # We want, for each group that should not be included + # check that the following condition is false : + # - at least one of the user's groups match the expected group + excluded_group_condition = [ + not_( + models_core.CoreUser.groups.any( + models_core.CoreGroup.id == group_id, + ), + ) + for group_id in excluded_groups + ] + excluded_account_type_condition = [ + not_( + models_core.CoreUser.account_type == account_type, + ) + for account_type in excluded_account_types + ] + school_condition = ( + or_( + *[models_core.CoreUser.school_id == school_id for school_id in schools_ids], + ) + if schools_ids + else and_(True) + ) + result = await db.execute( select(models_core.CoreUser).where( and_( True, - # We want, for each group that should be included check if - # - at least one of the user's groups match the expected group - *[ - models_core.CoreUser.groups.any( - models_core.CoreGroup.id == group_id, - ) - for group_id in included_groups - ], - or_( - False, - *[ - models_core.CoreUser.account_type == account_type - for account_type in included_account_types - ], - ), - *[ - not_( - models_core.CoreUser.account_type == account_type, - ) - for account_type in excluded_account_types - ], - # We want, for each group that should not be included - # check that the following condition is false : - # - at least one of the user's groups match the expected group - *[ - not_( - models_core.CoreUser.groups.any( - models_core.CoreGroup.id == group_id, - ), - ) - for group_id in excluded_groups - ], - or_( - False, - *[ - models_core.CoreUser.school_id == school_id - for school_id in schools_ids - ], - ) - if schools_ids - else and_(True), + *included_group_condition, + included_account_type_condition, + *excluded_account_type_condition, + *excluded_group_condition, + school_condition, ), ), )