Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/app/api/docs/organization/list.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
List all organizations.

Returns paginated list of all organizations in the system.
Returns paginated list of all organizations in the system. The response includes a `has_more` field in `metadata` indicating whether additional pages are available.
3 changes: 3 additions & 0 deletions backend/app/api/docs/projects/list_by_org.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
List all projects for a given organization.

Returns all projects belonging to the specified organization ID. The organization must exist and be active.
11 changes: 8 additions & 3 deletions backend/app/api/routes/organization.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
from typing import List

from fastapi import APIRouter, Depends, HTTPException
from fastapi import APIRouter, Depends, HTTPException, Query
from sqlalchemy import func
from sqlmodel import select

Expand All @@ -27,14 +27,19 @@
response_model=APIResponse[List[OrganizationPublic]],
description=load_description("organization/list.md"),
)
def read_organizations(session: SessionDep, skip: int = 0, limit: int = 100):
def read_organizations(
session: SessionDep,
skip: int = Query(0, ge=0),
limit: int = Query(100, ge=1, le=100),
):
count_statement = select(func.count()).select_from(Organization)
count = session.exec(count_statement).one()

statement = select(Organization).offset(skip).limit(limit)
organizations = session.exec(statement).all()

return APIResponse.success_response(organizations)
has_more = (skip + limit) < count
return APIResponse.success_response(organizations, metadata={"has_more": has_more})
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this kind of support is not there in read projects endpoint as well so why are we only adding it to this one

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

since this is requried for frontend that's why

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

but how do you know that we wont require this for project in future

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

but do u think organization would have lot of projects? .. for consistency I added the has_more metadata in projects endpoint as well



# Create a new organization
Expand Down
17 changes: 17 additions & 0 deletions backend/app/api/routes/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
from app.crud.project import (
create_project,
get_project_by_id,
get_projects_by_organization,
)
from app.crud.organization import validate_organization
from app.utils import APIResponse, load_description

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -112,3 +114,18 @@ def delete_project(session: SessionDep, project_id: int):
f"[delete_project] Project deleted successfully | project_id={project_id}"
)
return APIResponse.success_response(None)


# Get projects by organization
@router.get(
"/organization/{org_id}",
dependencies=[Depends(require_permission(Permission.SUPERUSER))],
response_model=APIResponse[List[ProjectPublic]],
description=load_description("projects/list_by_org.md"),
)
def read_projects_by_organization(
session: SessionDep, org_id: int
) -> APIResponse[List[ProjectPublic]]:
validate_organization(session=session, org_id=org_id)
projects = get_projects_by_organization(session=session, org_id=org_id)
return APIResponse.success_response(projects)
2 changes: 1 addition & 1 deletion backend/app/crud/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ def validate_organization(session: Session, org_id: int) -> Organization:
logger.error(
f"[validate_organization] Organization is not active | 'org_id': {org_id}"
)
raise HTTPException("Organization is not active")
raise HTTPException(status_code=503, detail="Organization is not active")

return organization
28 changes: 28 additions & 0 deletions backend/app/tests/api/routes/test_org.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,31 @@ def test_delete_organization(
headers=superuser_token_headers,
)
assert response.status_code == 404


# Test pagination has_more metadata
def test_read_organizations_has_more(
db: Session, superuser_token_headers: dict[str, str]
) -> None:
# Create 2 orgs and request with limit=1 to trigger has_more=True
create_test_organization(db)
create_test_organization(db)

response = client.get(
f"{settings.API_V1_STR}/organizations/?skip=0&limit=1",
headers=superuser_token_headers,
)
assert response.status_code == 200
response_data = response.json()
assert "metadata" in response_data
assert response_data["metadata"]["has_more"] is True

# Request all with large limit to verify has_more=False
response = client.get(
f"{settings.API_V1_STR}/organizations/?skip=0&limit=100",
headers=superuser_token_headers,
)
assert response.status_code == 200
response_data = response.json()
assert "metadata" in response_data
assert response_data["metadata"]["has_more"] is False
46 changes: 46 additions & 0 deletions backend/app/tests/api/routes/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from app.main import app
from app.core.config import settings
from app.models import Project, ProjectCreate
from app.models import Organization, OrganizationCreate
from app.tests.utils.test_data import create_test_organization, create_test_project


Expand Down Expand Up @@ -94,3 +95,48 @@ def test_delete_project(
headers=superuser_token_headers,
)
assert response.status_code == 404


# Test retrieving projects by organization
def test_read_projects_by_organization(
db: Session, superuser_token_headers: dict[str, str]
) -> None:
project = create_test_project(db)
response = client.get(
f"{settings.API_V1_STR}/projects/organization/{project.organization_id}",
headers=superuser_token_headers,
)
assert response.status_code == 200
response_data = response.json()
assert "data" in response_data
assert isinstance(response_data["data"], list)
assert len(response_data["data"]) >= 1
assert any(p["id"] == project.id for p in response_data["data"])


# Test retrieving projects by non-existent organization
def test_read_projects_by_organization_not_found(
db: Session, superuser_token_headers: dict[str, str]
) -> None:
response = client.get(
f"{settings.API_V1_STR}/projects/organization/999999",
headers=superuser_token_headers,
)
assert response.status_code == 404
Comment on lines +143 to +150
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace hardcoded missing-org ID and fix unused db (Line 119).

Using 999999 is brittle; generate a guaranteed non-existent ID from the DB so the test stays deterministic and resolves Ruff ARG001.

Proposed fix
+from app.tests.utils.utils import get_non_existent_id
@@
 def test_read_projects_by_organization_not_found(
     db: Session, superuser_token_headers: dict[str, str]
 ) -> None:
+    non_existent_org_id = get_non_existent_id(db, Organization)
     response = client.get(
-        f"{settings.API_V1_STR}/projects/organization/999999",
+        f"{settings.API_V1_STR}/projects/organization/{non_existent_org_id}",
         headers=superuser_token_headers,
     )
     assert response.status_code == 404
🧰 Tools
🪛 Ruff (0.15.7)

[warning] 119-119: Unused function argument: db

(ARG001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/app/tests/api/routes/test_project.py` around lines 118 - 125, In
test_read_projects_by_organization_not_found, avoid the hardcoded 999999 and the
unused db param by computing a guaranteed-nonexistent organization ID from the
database (e.g., use the db Session to query the maximum Organization.id and set
non_existent_id = (max_id or 0) + 1), then call
client.get(f"{settings.API_V1_STR}/projects/organization/{non_existent_id}",
headers=superuser_token_headers) and assert 404; update the function body of
test_read_projects_by_organization_not_found to use this computed id so the db
parameter is used and the test is deterministic.



# Test retrieving projects by inactive organization
def test_read_projects_by_inactive_organization(
db: Session, superuser_token_headers: dict[str, str]
) -> None:
org = create_test_organization(db)
org.is_active = False
db.add(org)
db.commit()
db.refresh(org)

response = client.get(
f"{settings.API_V1_STR}/projects/organization/{org.id}",
headers=superuser_token_headers,
)
assert response.status_code == 503
35 changes: 34 additions & 1 deletion backend/app/tests/crud/test_org.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import pytest
from fastapi import HTTPException
from sqlmodel import Session

from app.crud.organization import create_organization, get_organization_by_id
from app.crud.organization import (
create_organization,
get_organization_by_id,
validate_organization,
)
from app.models import Organization, OrganizationCreate
from app.tests.utils.utils import random_lower_string, get_non_existent_id
from app.tests.utils.test_data import create_test_organization
Expand Down Expand Up @@ -32,3 +38,30 @@ def test_get_non_existent_organization(db: Session) -> None:
organization_id = get_non_existent_id(db, Organization)
fetched_org = get_organization_by_id(session=db, org_id=organization_id)
assert fetched_org is None


def test_validate_organization_success(db: Session) -> None:
"""Test that a valid and active organization passes validation."""
organization = create_test_organization(db)

validated_org = validate_organization(session=db, org_id=organization.id)
assert validated_org.id == organization.id


def test_validate_organization_not_found(db: Session) -> None:
"""Test that validation fails when organization does not exist."""
non_existent_org_id = get_non_existent_id(db, Organization)
with pytest.raises(HTTPException, match="Organization not found"):
validate_organization(session=db, org_id=non_existent_org_id)


def test_validate_organization_inactive(db: Session) -> None:
"""Test that validation fails when organization is inactive."""
organization = create_test_organization(db)
organization.is_active = False
db.add(organization)
db.commit()
db.refresh(organization)

with pytest.raises(HTTPException, match="Organization is not active"):
validate_organization(session=db, org_id=organization.id)
Comment on lines +51 to +67
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Assert HTTP status codes, not only error text.

These tests currently pass on message match alone; they should also lock in 404 and 503 to prevent silent behavior drift.

Proposed test hardening
 def test_validate_organization_not_found(db: Session) -> None:
     """Test that validation fails when organization does not exist."""
     non_existent_org_id = get_non_existent_id(db, Organization)
-    with pytest.raises(HTTPException, match="Organization not found"):
+    with pytest.raises(HTTPException) as exc_info:
         validate_organization(session=db, org_id=non_existent_org_id)
+    assert exc_info.value.status_code == 404
+    assert exc_info.value.detail == "Organization not found"
@@
 def test_validate_organization_inactive(db: Session) -> None:
@@
-    with pytest.raises(HTTPException, match="Organization is not active"):
+    with pytest.raises(HTTPException) as exc_info:
         validate_organization(session=db, org_id=organization.id)
+    assert exc_info.value.status_code == 503
+    assert exc_info.value.detail == "Organization is not active"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_validate_organization_not_found(db: Session) -> None:
"""Test that validation fails when organization does not exist."""
non_existent_org_id = get_non_existent_id(db, Organization)
with pytest.raises(HTTPException, match="Organization not found"):
validate_organization(session=db, org_id=non_existent_org_id)
def test_validate_organization_inactive(db: Session) -> None:
"""Test that validation fails when organization is inactive."""
organization = create_test_organization(db)
organization.is_active = False
db.add(organization)
db.commit()
db.refresh(organization)
with pytest.raises(HTTPException, match="Organization is not active"):
validate_organization(session=db, org_id=organization.id)
def test_validate_organization_not_found(db: Session) -> None:
"""Test that validation fails when organization does not exist."""
non_existent_org_id = get_non_existent_id(db, Organization)
with pytest.raises(HTTPException) as exc_info:
validate_organization(session=db, org_id=non_existent_org_id)
assert exc_info.value.status_code == 404
assert exc_info.value.detail == "Organization not found"
def test_validate_organization_inactive(db: Session) -> None:
"""Test that validation fails when organization is inactive."""
organization = create_test_organization(db)
organization.is_active = False
db.add(organization)
db.commit()
db.refresh(organization)
with pytest.raises(HTTPException) as exc_info:
validate_organization(session=db, org_id=organization.id)
assert exc_info.value.status_code == 503
assert exc_info.value.detail == "Organization is not active"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/app/tests/crud/test_org.py` around lines 47 - 63, The tests
test_validate_organization_not_found and test_validate_organization_inactive
currently only match exception text; update them to also assert the
HTTPException.status_code to prevent silent behavior drift: use pytest.raises as
a context manager to capture the exception from validate_organization and add an
assertion that excinfo.value.status_code == 404 for the "Organization not found"
case and == 503 for the "Organization is not active" case, referring to
validate_organization and HTTPException to locate the code.

Loading