Skip to content

Commit

Permalink
add soft delete to projects
Browse files Browse the repository at this point in the history
  • Loading branch information
habibasseiss committed Nov 26, 2024
1 parent 1225bed commit bf384f0
Show file tree
Hide file tree
Showing 4 changed files with 261 additions and 25 deletions.
3 changes: 3 additions & 0 deletions app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ class Project:
created_at: Mapped[datetime] = mapped_column(
init=False, server_default=func.now()
)
deleted_at: Mapped[datetime | None] = mapped_column(
init=False, nullable=True, default=None
)
organization_id: Mapped[uuid.UUID | None] = mapped_column(
ForeignKey('organizations.id'), nullable=False
)
Expand Down
39 changes: 38 additions & 1 deletion app/routers/projects.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging
from datetime import datetime
from http import HTTPStatus
from typing import Annotated
from uuid import UUID
Expand All @@ -13,6 +15,8 @@
from app.services.upload_service import delete_file_from_s3, upload_file_to_s3
from app.settings import Settings

logger = logging.getLogger(__name__)

settings = Settings.model_validate({})


Expand Down Expand Up @@ -51,6 +55,7 @@ def get_project(
Project.organization_id == organization_id,
Project.id == project_id,
Project.organization.has(Organization.users.contains(user)),
Project.deleted_at.is_(None),
)
)
if not project:
Expand All @@ -67,7 +72,10 @@ def list_organization_projects(
):
organization = get_organization(session, user, organization_id)
projects = session.scalars(
select(Project).where(Project.organization_id == organization.id)
select(Project).where(
Project.organization_id == organization.id,
Project.deleted_at.is_(None),
)
).all()
return {'projects': projects}

Expand Down Expand Up @@ -128,6 +136,35 @@ def delete_project(
user: CurrentUser,
):
project = get_project(session, user, organization_id, project_id)
project.deleted_at = datetime.utcnow()
session.commit()


@router.delete('/{project_id}/hard', status_code=HTTPStatus.NO_CONTENT)
async def hard_delete_project(
organization_id: UUID,
project_id: UUID,
session: DbSession,
user: CurrentUser,
):
project = get_project(session, user, organization_id, project_id)

# Delete files from S3
for file in project.files:
try:
await delete_file_from_s3(file.path)
except HTTPException as e:
# Log the error but don't stop the deletion process
logger.error(
f'Failed to delete file {file.path} from S3: {str(e)}'
)
except Exception as e:
# Log any other unexpected errors
logger.error(
f'Unexpected error deleting file {file.path} from S3: {str(e)}'
)

# Delete the project and all associated files from database
session.delete(project)
session.commit()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""add deleted_at to projects
Revision ID: 90a4f1ff4dfe
Revises: e1f919c1c0d4
Create Date: 2024-11-26 09:18:11.085301
"""
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision: str = '90a4f1ff4dfe'
down_revision: Union[str, None] = 'e1f919c1c0d4'
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! ###
op.add_column('projects', sa.Column('deleted_at', sa.DateTime(), nullable=True))
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column('projects', 'deleted_at')
# ### end Alembic commands ###
214 changes: 190 additions & 24 deletions tests/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,16 +397,187 @@ async def test_project_deletion_with_files(
for file in files:
session.refresh(file)

# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.NO_CONTENT

# Verify project is marked as deleted but files still exist
db_project = session.get(Project, project.id)
assert db_project is not None
assert db_project.deleted_at is not None

for file in files:
db_file = session.get(File, file.id)
assert db_file is not None


@pytest.mark.asyncio
async def test_project_deletion_handles_s3_error(
client: TestClient,
token: str,
organization: Organization,
project: Project,
session: Session,
):
# Create a test file
file = File( # type: ignore
path=f'projects/{project.id}/test.txt',
size=100,
mime_type='text/plain',
original_filename='test.txt',
project_id=project.id,
)
session.add(file)
session.commit()
session.refresh(file)

# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}',
headers={'Authorization': f'Bearer {token}'},
)

# Expect success
assert response.status_code == HTTPStatus.NO_CONTENT

# Verify project is marked as deleted but file still exists
db_project = session.get(Project, project.id)
assert db_project is not None
assert db_project.deleted_at is not None

db_file = session.get(File, file.id)
assert db_file is not None


def test_soft_delete_project(
client: TestClient,
token: str,
organization: Organization,
project: Project,
session: Session,
):
# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.NO_CONTENT

# Verify project is not in the list
response = client.get(
f'/organizations/{organization.id}/projects',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.OK
data = response.json()
assert len(data['projects']) == 0

# Verify project cannot be accessed directly
response = client.get(
f'/organizations/{organization.id}/projects/{project.id}',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.NOT_FOUND

# Verify project exists in database with deleted_at timestamp
db_project = session.get(Project, project.id)
assert db_project is not None
assert db_project.deleted_at is not None


def test_soft_delete_project_with_files(
client: TestClient,
token: str,
organization: Organization,
project: Project,
session: Session,
):
# Add a file to the project
file = File( # type: ignore
path='test/path',
size=100,
mime_type='text/plain',
original_filename='test.txt',
project_id=project.id,
)
session.add(file)
session.commit()

# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.NO_CONTENT

# Verify project and file still exist in database
db_project = session.get(Project, project.id)
assert db_project is not None
assert db_project.deleted_at is not None

db_file = session.get(File, file.id)
assert db_file is not None


@pytest.mark.asyncio
async def test_hard_delete_project(
client: TestClient,
token: str,
organization: Organization,
project: Project,
session: Session,
):
# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}/hard',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.NO_CONTENT

# Verify project is completely removed from database
db_project = session.get(Project, project.id)
assert db_project is None


@pytest.mark.asyncio
async def test_hard_delete_project_with_files(
client: TestClient,
token: str,
organization: Organization,
project: Project,
session: Session,
):
# Create some test files in the database
files = [
File( # type: ignore
path=f'projects/{project.id}/test{i}.txt',
size=100,
mime_type='text/plain',
original_filename=f'test{i}.txt',
project_id=project.id,
)
for i in range(3)
]
session.add_all(files)
session.commit()
for file in files:
session.refresh(file)

# Mock the S3 deletion
with patch('app.services.upload_service.boto3.client') as mock_s3:
mock_s3_client = mock_s3.return_value
mock_s3_client.delete_object.return_value = {
'ResponseMetadata': {'HTTPStatusCode': HTTPStatus.NO_CONTENT}
}
mock_s3_client: S3Client = mock_s3.return_value
mock_s3_client.delete_object = MagicMock( # type: ignore
return_value={
'ResponseMetadata': {'HTTPStatusCode': HTTPStatus.NO_CONTENT}
}
)

# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}',
f'/organizations/{organization.id}/projects/{project.id}/hard',
headers={'Authorization': f'Bearer {token}'},
)
assert response.status_code == HTTPStatus.NO_CONTENT
Expand All @@ -419,21 +590,18 @@ async def test_project_deletion_with_files(
Key=file.path,
)

# Verify files are deleted from database
db_files = (
session.query(File).filter(File.project_id == project.id).all()
)
assert len(db_files) == 0
# Verify files are deleted from database
for file in files:
db_file = session.get(File, file.id)
assert db_file is None

# Verify project is deleted
db_project = (
session.query(Project).filter(Project.id == project.id).first()
)
assert db_project is None
# Verify project is deleted
db_project = session.get(Project, project.id)
assert db_project is None


@pytest.mark.asyncio
async def test_project_deletion_handles_s3_error(
async def test_hard_delete_project_handles_s3_error(
client: TestClient,
token: str,
organization: Organization,
Expand Down Expand Up @@ -465,7 +633,7 @@ async def test_project_deletion_handles_s3_error(

# Delete the project
response = client.delete(
f'/organizations/{organization.id}/projects/{project.id}',
f'/organizations/{organization.id}/projects/{project.id}/hard',
headers={'Authorization': f'Bearer {token}'},
)

Expand All @@ -478,10 +646,8 @@ async def test_project_deletion_handles_s3_error(
Key=file.path,
)

# Verify database records are deleted
db_file = session.query(File).filter(File.id == file.id).first()
assert db_file is None
db_project = (
session.query(Project).filter(Project.id == project.id).first()
)
assert db_project is None
# Verify database records are deleted even if S3 deletion fails
db_file = session.get(File, file.id)
assert db_file is None
db_project = session.get(Project, project.id)
assert db_project is None

0 comments on commit bf384f0

Please sign in to comment.