-
Notifications
You must be signed in to change notification settings - Fork 10
Add projects-by-org endpoint and pagination for organizations list #727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
abd1fc8
84ce2e8
c22fbe8
76b4d3a
24641d6
5d75b34
8e14322
0d88fa5
3309d6c
22f7665
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. |
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,32 @@ def test_read_projects(db: Session, superuser_token_headers: dict[str, str]) -> | |
| assert isinstance(response_data["data"], list) | ||
|
|
||
|
|
||
| # Test pagination has_more metadata for projects | ||
| def test_read_projects_has_more( | ||
| db: Session, superuser_token_headers: dict[str, str] | ||
| ) -> None: | ||
| create_test_project(db) | ||
| create_test_project(db) | ||
|
|
||
| response = client.get( | ||
| f"{settings.API_V1_STR}/projects/?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 | ||
|
|
||
| response = client.get( | ||
| f"{settings.API_V1_STR}/projects/?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 | ||
|
|
||
|
|
||
| # Test updating a project | ||
| def test_update_project( | ||
| db: Session, test_project: Project, superuser_token_headers: dict[str, str] | ||
|
|
@@ -94,3 +120,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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace hardcoded missing-org ID and fix unused Using 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: (ARG001) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| # 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 == 403 | ||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert HTTP status codes, not only error text. These tests currently pass on message match alone; they should also lock in 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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