Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"apikey",
sa.Column("organization_id", sa.Integer(), nullable=False),
sa.Column("user_id", sa.Uuid(), nullable=False),
sa.Column("key", sqlmodel.sql.sqltypes.AutoString(), nullable=False),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("created_at", sa.DateTime(), nullable=False),
sa.Column("key", sqlmodel.sql.sqltypes.AutoString(), nullable=False),
sa.Column("user_id", sa.Uuid(), nullable=False),
sa.Column("organization_id", sa.Integer(), nullable=False),
sa.Column("is_deleted", sa.Boolean(), nullable=False),
sa.Column("inserted_at", sa.DateTime(), nullable=False),
sa.Column("updated_at", sa.DateTime(), nullable=False),
sa.Column("deleted_at", sa.DateTime(), nullable=True),
sa.ForeignKeyConstraint(
["organization_id"], ["organization.id"], ondelete="CASCADE"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
def upgrade():
op.create_table(
"credential",
sa.Column("organization_id", sa.Integer(), nullable=False),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("credential", sa.JSON(), nullable=True),
sa.Column("organization_id", sa.Integer(), nullable=False),
sa.Column("inserted_at", sa.DateTime(), nullable=True),
sa.Column("updated_at", sa.DateTime(), nullable=True),
sa.Column("deleted_at", sa.DateTime(), nullable=True),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,29 @@ def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"organization",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=255), nullable=False),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column(
"inserted_at", sa.DateTime(), nullable=False, server_default=sa.func.now()
),
sa.Column("updated_at", sa.DateTime(), nullable=False),
sa.PrimaryKeyConstraint("id"),
)
op.create_index(op.f("ix_organization_name"), "organization", ["name"], unique=True)
op.create_table(
"project",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("name", sqlmodel.sql.sqltypes.AutoString(length=255), nullable=False),
sa.Column(
"description", sqlmodel.sql.sqltypes.AutoString(length=500), nullable=True
),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("organization_id", sa.Integer(), nullable=False),
sa.Column(
"inserted_at", sa.DateTime(), nullable=False, server_default=sa.func.now()
),
sa.Column("updated_at", sa.DateTime(), nullable=False),
sa.ForeignKeyConstraint(
["organization_id"], ["organization.id"], ondelete="CASCADE"
),
Expand All @@ -47,13 +55,15 @@ def upgrade():
)
op.create_table(
"projectuser",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("project_id", sa.Integer(), nullable=False),
sa.Column("user_id", sa.Uuid(), nullable=False),
sa.Column("is_admin", sa.Boolean(), nullable=False),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("created_at", sa.DateTime(), nullable=False),
sa.Column("updated_at", sa.DateTime(), nullable=False),
sa.Column("is_deleted", sa.Boolean(), nullable=False),
sa.Column(
"inserted_at", sa.DateTime(), nullable=False, server_default=sa.func.now()
),
sa.Column("updated_at", sa.DateTime(), nullable=False),
sa.Column("deleted_at", sa.DateTime(), nullable=True),
sa.ForeignKeyConstraint(["project_id"], ["project.id"], ondelete="CASCADE"),
sa.ForeignKeyConstraint(["user_id"], ["user.id"], ondelete="CASCADE"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ def upgrade():
sa.Column(
"object_store_url", sqlmodel.sql.sqltypes.AutoString(), nullable=False
),
sa.Column("created_at", sa.DateTime(), nullable=False),
sa.Column("inserted_at", sa.DateTime(), nullable=False),
sa.Column("updated_at", sa.DateTime(), nullable=False),
sa.Column("deleted_at", sa.DateTime(), nullable=True),
sa.ForeignKeyConstraint(["owner_id"], ["user.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("id"),
Expand Down
12 changes: 6 additions & 6 deletions backend/app/alembic/versions/e2412789c190_initialize_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,23 @@ def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"user",
sa.Column("email", sqlmodel.sql.sqltypes.AutoString(), nullable=False),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("is_superuser", sa.Boolean(), nullable=False),
sa.Column("full_name", sqlmodel.sql.sqltypes.AutoString(), nullable=True),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("full_name", sqlmodel.sql.sqltypes.AutoString(), nullable=True),
sa.Column("email", sqlmodel.sql.sqltypes.AutoString(), nullable=False),
sa.Column(
"hashed_password", sqlmodel.sql.sqltypes.AutoString(), nullable=False
),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("is_superuser", sa.Boolean(), nullable=False),
sa.PrimaryKeyConstraint("id"),
)
op.create_index(op.f("ix_user_email"), "user", ["email"], unique=True)
op.create_table(
"item",
sa.Column("description", sqlmodel.sql.sqltypes.AutoString(), nullable=True),
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("title", sqlmodel.sql.sqltypes.AutoString(), nullable=False),
sa.Column("owner_id", sa.Integer(), nullable=False),
sa.Column("title", sqlmodel.sql.sqltypes.AutoString(), nullable=False),
sa.Column("description", sqlmodel.sql.sqltypes.AutoString(), nullable=True),
sa.ForeignKeyConstraint(
["owner_id"],
["user.id"],
Expand Down
4 changes: 2 additions & 2 deletions backend/app/api/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ async def http_exception_handler(request: Request, exc: HTTPException):
return JSONResponse(
status_code=exc.status_code,
content=APIResponse.failure_response(exc.detail).model_dump()
| {"detail": exc.detail}, # TEMPORARY: Keep "detail" for backward compatibility
# TEMPORARY: Keep "detail" for backward compatibility
| {"detail": exc.detail},
)


Expand Down Expand Up @@ -194,7 +195,6 @@ def verify_user_project_organization(
select(ProjectUser).where(
ProjectUser.user_id == current_user.id,
ProjectUser.project_id == project_id,
ProjectUser.is_deleted == False,
)
).first()

Expand Down
1 change: 1 addition & 0 deletions backend/app/crud/api_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def delete_api_key(session: Session, api_key_id: int) -> None:

api_key.is_deleted = True
api_key.deleted_at = datetime.utcnow()
api_key.updated_at = datetime.utcnow()

session.add(api_key)
session.commit()
Expand Down
3 changes: 3 additions & 0 deletions backend/app/crud/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ def update(self, document: Document):
)
raise PermissionError(error)

document.updated_at = now()

self.session.add(document)
self.session.commit()
self.session.refresh(document)
Comment on lines +57 to 61
Copy link

Choose a reason for hiding this comment

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

kody code-review Error Handling high

try:
    document.updated_at = now()
    self.session.add(document)
    self.session.commit()
    self.session.refresh(document)
except Exception as e:
    self.session.rollback()
    raise e

Database operations lack error handling, which can lead to crashes and unhandled exceptions.

This issue appears in multiple locations:

  • backend/app/crud/document.py: Lines 57-61
  • backend/app/crud/project.py: Lines 14-16
  • backend/app/crud/project_user.py: Lines 68-68
    Please add try-catch blocks around database operations to handle potential errors gracefully and provide meaningful error messages.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Expand All @@ -63,5 +65,6 @@ def update(self, document: Document):
def delete(self, doc_id: UUID):
document = self.read_one(doc_id)
document.deleted_at = now()
document.updated_at = now()

return self.update(document)
4 changes: 3 additions & 1 deletion backend/app/crud/organization.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from typing import Any, Optional

from datetime import datetime
from sqlmodel import Session, select

from app.models import Organization, OrganizationCreate
Expand All @@ -9,6 +9,8 @@ def create_organization(
*, session: Session, org_create: OrganizationCreate
) -> Organization:
db_org = Organization.model_validate(org_create)
db_org.inserted_at = datetime.utcnow()
db_org.updated_at = datetime.utcnow()
session.add(db_org)
session.commit()
session.refresh(db_org)
Expand Down
4 changes: 3 additions & 1 deletion backend/app/crud/project.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
from typing import List, Optional

from datetime import datetime
from sqlmodel import Session, select

from app.models import Project, ProjectCreate


def create_project(*, session: Session, project_create: ProjectCreate) -> Project:
db_project = Project.model_validate(project_create)
db_project.inserted_at = datetime.utcnow()
db_project.updated_at = datetime.utcnow()
session.add(db_project)
session.commit()
session.refresh(db_project)
Expand Down
7 changes: 5 additions & 2 deletions backend/app/models/api_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from typing import Optional, List
from sqlmodel import SQLModel, Field, Relationship

from app.core.util import now


class APIKeyBase(SQLModel):
organization_id: int = Field(
Expand All @@ -19,12 +21,13 @@ class APIKeyBase(SQLModel):

class APIKeyPublic(APIKeyBase):
id: int
created_at: datetime
inserted_at: datetime = Field(default_factory=now, nullable=False)


class APIKey(APIKeyBase, table=True):
id: int = Field(default=None, primary_key=True)
created_at: datetime = Field(default_factory=datetime.utcnow, nullable=False)
inserted_at: datetime = Field(default_factory=now, nullable=False)
updated_at: datetime = Field(default_factory=now, nullable=False)
is_deleted: bool = Field(default=False, nullable=False)
deleted_at: Optional[datetime] = Field(default=None, nullable=True)

Expand Down
6 changes: 4 additions & 2 deletions backend/app/models/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from sqlmodel import Field, Relationship, SQLModel
from datetime import datetime

from app.core.util import now


class CredsBase(SQLModel):
organization_id: int = Field(foreign_key="organization.id")
Expand All @@ -24,11 +26,11 @@ class Credential(CredsBase, table=True):
id: int = Field(default=None, primary_key=True)
credential: Dict[str, Any] = Field(default=None, sa_column=sa.Column(sa.JSON))
inserted_at: datetime = Field(
default_factory=datetime.utcnow,
default_factory=now,
sa_column=sa.Column(sa.DateTime, default=datetime.utcnow),
)
updated_at: datetime = Field(
default_factory=datetime.utcnow,
default_factory=now,
sa_column=sa.Column(sa.DateTime, onupdate=datetime.utcnow),
)
deleted_at: Optional[datetime] = Field(
Expand Down
6 changes: 4 additions & 2 deletions backend/app/models/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ class Document(SQLModel, table=True):
)
fname: str
object_store_url: str
created_at: datetime = Field(
inserted_at: datetime = Field(
default_factory=now,
)
updated_at: datetime = Field(
default_factory=now,
)
# updated_at: datetime | None
deleted_at: datetime | None

owner: User = Relationship(back_populates="documents")
6 changes: 6 additions & 0 deletions backend/app/models/organization.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from datetime import datetime
from typing import List, TYPE_CHECKING
from sqlmodel import Field, Relationship, SQLModel
from sqlalchemy.orm import relationship

from app.core.util import now

if TYPE_CHECKING:
from .credentials import Credential
Expand Down Expand Up @@ -29,6 +31,8 @@ class OrganizationUpdate(SQLModel):
# Database model for Organization
class Organization(OrganizationBase, table=True):
id: int = Field(default=None, primary_key=True)
inserted_at: datetime = Field(default_factory=now, nullable=False)
updated_at: datetime = Field(default_factory=now, nullable=False)

# Relationship back to Creds
api_keys: list["APIKey"] = Relationship(
Expand All @@ -45,6 +49,8 @@ class Organization(OrganizationBase, table=True):
# Properties to return via API
class OrganizationPublic(OrganizationBase):
id: int
inserted_at: datetime
updated_at: datetime


class OrganizationsPublic(SQLModel):
Expand Down
7 changes: 7 additions & 0 deletions backend/app/models/project.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from datetime import datetime
from typing import Optional
from sqlmodel import Field, Relationship, SQLModel

from app.core.util import now


# Shared properties for a Project
class ProjectBase(SQLModel):
Expand All @@ -25,6 +28,8 @@ class ProjectUpdate(SQLModel):
class Project(ProjectBase, table=True):
id: int = Field(default=None, primary_key=True)
organization_id: int = Field(foreign_key="organization.id", index=True)
inserted_at: datetime = Field(default_factory=now, nullable=False)
updated_at: datetime = Field(default_factory=now, nullable=False)

users: list["ProjectUser"] = Relationship(
back_populates="project", cascade_delete=True
Expand All @@ -37,6 +42,8 @@ class Project(ProjectBase, table=True):
class ProjectPublic(ProjectBase):
id: int
organization_id: int
inserted_at: datetime
updated_at: datetime


class ProjectsPublic(SQLModel):
Expand Down
9 changes: 5 additions & 4 deletions backend/app/models/project_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from typing import Optional, List
from sqlmodel import SQLModel, Field, Relationship

from app.core.util import now


# Shared properties
class ProjectUserBase(SQLModel):
Expand All @@ -19,18 +21,17 @@ class ProjectUserBase(SQLModel):

class ProjectUserPublic(ProjectUserBase):
id: int
created_at: datetime
inserted_at: datetime
updated_at: datetime


# Database model, database table inferred from class name
class ProjectUser(ProjectUserBase, table=True):
id: int = Field(default=None, primary_key=True)
created_at: datetime = Field(default_factory=datetime.utcnow, nullable=False)
updated_at: datetime = Field(default_factory=datetime.utcnow, nullable=False)
inserted_at: datetime = Field(default_factory=now, nullable=False)
updated_at: datetime = Field(default_factory=now, nullable=False)
is_deleted: bool = Field(default=False, nullable=False)
deleted_at: Optional[datetime] = Field(default=None, nullable=True)

# Relationships
project: "Project" = Relationship(back_populates="users")
user: "User" = Relationship(back_populates="projects")
Expand Down
2 changes: 1 addition & 1 deletion backend/app/tests/crud/documents/test_crud_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_delete_marks_deleted(self, document: Document):
assert document.deleted_at is not None

def test_delete_follows_insert(self, document: Document):
assert document.created_at <= document.deleted_at
assert document.inserted_at <= document.deleted_at

def test_cannot_delete_others_documents(self, db: Session):
store = DocumentStore(db)
Expand Down
2 changes: 1 addition & 1 deletion backend/app/tests/crud/documents/test_crud_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def test_sequential_update_is_ordered(
crud = DocumentCrud(db, documents.owner_id)
(a, b) = (crud.update(y) for (_, y) in zip(range(2), documents))

assert a.created_at <= b.created_at
assert a.inserted_at <= b.inserted_at

def test_insert_does_not_delete(
self,
Expand Down