From 8c326501bf79a55a6b6306e72c8a13a6217d118d Mon Sep 17 00:00:00 2001 From: Artem Kashaev Date: Thu, 27 Nov 2025 15:38:47 +0500 Subject: [PATCH] Merge branch 'organizations' (cherry-picked) --- app/api/deps.py | 27 +++++- app/api/v1/activities/views.py | 20 +++-- app/api/v1/analytics/views.py | 17 +++- app/api/v1/contacts/views.py | 14 ++- app/api/v1/deals/views.py | 23 +++-- app/api/v1/tasks/views.py | 14 ++- app/repositories/org_repo.py | 13 +++ app/services/organization_service.py | 87 ++++++++++++++++++ tests/services/test_organization_service.py | 99 +++++++++++++++++++++ 9 files changed, 292 insertions(+), 22 deletions(-) create mode 100644 app/services/organization_service.py create mode 100644 tests/services/test_organization_service.py diff --git a/app/api/deps.py b/app/api/deps.py index e33cc97..53f6660 100644 --- a/app/api/deps.py +++ b/app/api/deps.py @@ -2,7 +2,7 @@ from collections.abc import AsyncGenerator import jwt -from fastapi import Depends, HTTPException, status +from fastapi import Depends, Header, HTTPException, status from fastapi.security import OAuth2PasswordBearer from sqlalchemy.ext.asyncio import AsyncSession @@ -14,6 +14,12 @@ from app.repositories.deal_repo import DealRepository from app.repositories.org_repo import OrganizationRepository from app.repositories.user_repo import UserRepository from app.services.auth_service import AuthService +from app.services.organization_service import ( + OrganizationAccessDeniedError, + OrganizationContext, + OrganizationContextMissingError, + OrganizationService, +) from app.services.user_service import UserService oauth2_scheme = OAuth2PasswordBearer(tokenUrl=f"{settings.api_v1_prefix}/auth/token") @@ -51,6 +57,12 @@ def get_auth_service( ) +def get_organization_service( + repo: OrganizationRepository = Depends(get_organization_repository), +) -> OrganizationService: + return OrganizationService(repository=repo) + + async def get_current_user( token: str = Depends(oauth2_scheme), repo: UserRepository = Depends(get_user_repository), @@ -72,3 +84,16 @@ async def get_current_user( if user is None: raise credentials_exception return user + + +async def get_organization_context( + x_organization_id: int | None = Header(default=None, alias="X-Organization-Id"), + current_user: User = Depends(get_current_user), + service: OrganizationService = Depends(get_organization_service), +) -> OrganizationContext: + try: + return await service.get_context(user_id=current_user.id, organization_id=x_organization_id) + except OrganizationContextMissingError as exc: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc + except OrganizationAccessDeniedError as exc: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc diff --git a/app/api/v1/activities/views.py b/app/api/v1/activities/views.py index dcf594a..03c73de 100644 --- a/app/api/v1/activities/views.py +++ b/app/api/v1/activities/views.py @@ -1,7 +1,10 @@ """Activity timeline API stubs.""" from __future__ import annotations -from fastapi import APIRouter, status +from fastapi import APIRouter, Depends, status + +from app.api.deps import get_organization_context +from app.services.organization_service import OrganizationContext from .models import ActivityCommentPayload @@ -13,14 +16,21 @@ def _stub(endpoint: str) -> dict[str, str]: @router.get("/", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def list_activities(deal_id: int) -> dict[str, str]: +async def list_activities( + deal_id: int, + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for listing deal activities.""" - _ = deal_id + _ = (deal_id, context) return _stub("GET /deals/{deal_id}/activities") @router.post("/", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def create_activity_comment(deal_id: int, payload: ActivityCommentPayload) -> dict[str, str]: +async def create_activity_comment( + deal_id: int, + payload: ActivityCommentPayload, + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for adding a comment activity to a deal.""" - _ = (deal_id, payload) + _ = (deal_id, payload, context) return _stub("POST /deals/{deal_id}/activities") diff --git a/app/api/v1/analytics/views.py b/app/api/v1/analytics/views.py index de3ea27..08d5383 100644 --- a/app/api/v1/analytics/views.py +++ b/app/api/v1/analytics/views.py @@ -1,7 +1,10 @@ """Analytics API stubs (deal summary and funnel).""" from __future__ import annotations -from fastapi import APIRouter, Query, status +from fastapi import APIRouter, Depends, Query, status + +from app.api.deps import get_organization_context +from app.services.organization_service import OrganizationContext router = APIRouter(prefix="/analytics", tags=["analytics"]) @@ -11,13 +14,19 @@ def _stub(endpoint: str) -> dict[str, str]: @router.get("/deals/summary", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def deals_summary(days: int = Query(30, ge=1, le=180)) -> dict[str, str]: +async def deals_summary( + days: int = Query(30, ge=1, le=180), + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for aggregated deal statistics.""" - _ = days + _ = (days, context) return _stub("GET /analytics/deals/summary") @router.get("/deals/funnel", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def deals_funnel() -> dict[str, str]: +async def deals_funnel( + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for funnel analytics.""" + _ = context return _stub("GET /analytics/deals/funnel") diff --git a/app/api/v1/contacts/views.py b/app/api/v1/contacts/views.py index 19dd0b1..8fa8529 100644 --- a/app/api/v1/contacts/views.py +++ b/app/api/v1/contacts/views.py @@ -1,7 +1,10 @@ """Contact API stubs required by the spec.""" from __future__ import annotations -from fastapi import APIRouter, Query, status +from fastapi import APIRouter, Depends, Query, status + +from app.api.deps import get_organization_context +from app.services.organization_service import OrganizationContext from .models import ContactCreatePayload @@ -18,13 +21,18 @@ async def list_contacts( page_size: int = Query(20, ge=1, le=100), search: str | None = None, owner_id: int | None = None, + context: OrganizationContext = Depends(get_organization_context), ) -> dict[str, str]: """Placeholder list endpoint supporting the required filters.""" + _ = context return _stub("GET /contacts") @router.post("/", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def create_contact(payload: ContactCreatePayload) -> dict[str, str]: +async def create_contact( + payload: ContactCreatePayload, + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for creating a contact within the current organization.""" - _ = payload + _ = (payload, context) return _stub("POST /contacts") diff --git a/app/api/v1/deals/views.py b/app/api/v1/deals/views.py index c3d8ffc..e10b22d 100644 --- a/app/api/v1/deals/views.py +++ b/app/api/v1/deals/views.py @@ -3,7 +3,10 @@ from __future__ import annotations from decimal import Decimal -from fastapi import APIRouter, Query, status +from fastapi import APIRouter, Depends, Query, status + +from app.api.deps import get_organization_context +from app.services.organization_service import OrganizationContext from .models import DealCreatePayload, DealUpdatePayload @@ -25,21 +28,29 @@ async def list_deals( owner_id: int | None = None, order_by: str | None = None, order: str | None = Query(default=None, pattern="^(asc|desc)$"), + context: OrganizationContext = Depends(get_organization_context), ) -> dict[str, str]: """Placeholder for deal filtering endpoint.""" - _ = (status_filter,) + _ = (status_filter, context) return _stub("GET /deals") @router.post("/", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def create_deal(payload: DealCreatePayload) -> dict[str, str]: +async def create_deal( + payload: DealCreatePayload, + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for creating a new deal.""" - _ = payload + _ = (payload, context) return _stub("POST /deals") @router.patch("/{deal_id}", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def update_deal(deal_id: int, payload: DealUpdatePayload) -> dict[str, str]: +async def update_deal( + deal_id: int, + payload: DealUpdatePayload, + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for modifying deal status or stage.""" - _ = (deal_id, payload) + _ = (deal_id, payload, context) return _stub("PATCH /deals/{deal_id}") diff --git a/app/api/v1/tasks/views.py b/app/api/v1/tasks/views.py index 4955ad7..9ed6f92 100644 --- a/app/api/v1/tasks/views.py +++ b/app/api/v1/tasks/views.py @@ -3,7 +3,10 @@ from __future__ import annotations from datetime import date -from fastapi import APIRouter, Query, status +from fastapi import APIRouter, Depends, Query, status + +from app.api.deps import get_organization_context +from app.services.organization_service import OrganizationContext from .models import TaskCreatePayload @@ -20,13 +23,18 @@ async def list_tasks( only_open: bool = False, due_before: date | None = Query(default=None), due_after: date | None = Query(default=None), + context: OrganizationContext = Depends(get_organization_context), ) -> dict[str, str]: """Placeholder for task filtering endpoint.""" + _ = context return _stub("GET /tasks") @router.post("/", status_code=status.HTTP_501_NOT_IMPLEMENTED) -async def create_task(payload: TaskCreatePayload) -> dict[str, str]: +async def create_task( + payload: TaskCreatePayload, + context: OrganizationContext = Depends(get_organization_context), +) -> dict[str, str]: """Placeholder for creating a task linked to a deal.""" - _ = payload + _ = (payload, context) return _stub("POST /tasks") diff --git a/app/repositories/org_repo.py b/app/repositories/org_repo.py index 86b7e65..c78c947 100644 --- a/app/repositories/org_repo.py +++ b/app/repositories/org_repo.py @@ -4,6 +4,7 @@ from __future__ import annotations from collections.abc import Sequence from sqlalchemy import select +from sqlalchemy.orm import selectinload from sqlalchemy.ext.asyncio import AsyncSession from app.models.organization import Organization, OrganizationCreate @@ -42,6 +43,18 @@ class OrganizationRepository: result = await self._session.scalars(stmt) return result.unique().all() + async def get_membership(self, organization_id: int, user_id: int) -> OrganizationMember | None: + stmt = ( + select(OrganizationMember) + .where( + OrganizationMember.organization_id == organization_id, + OrganizationMember.user_id == user_id, + ) + .options(selectinload(OrganizationMember.organization)) + ) + result = await self._session.scalars(stmt) + return result.first() + async def create(self, data: OrganizationCreate) -> Organization: organization = Organization(name=data.name) self._session.add(organization) diff --git a/app/services/organization_service.py b/app/services/organization_service.py new file mode 100644 index 0000000..c2ddafc --- /dev/null +++ b/app/services/organization_service.py @@ -0,0 +1,87 @@ +"""Organization-related business rules.""" +from __future__ import annotations + +from dataclasses import dataclass + +from app.models.organization import Organization +from app.models.organization_member import OrganizationMember, OrganizationRole +from app.repositories.org_repo import OrganizationRepository + + +class OrganizationServiceError(Exception): + """Base class for organization service errors.""" + + +class OrganizationContextMissingError(OrganizationServiceError): + """Raised when the request lacks organization context.""" + + +class OrganizationAccessDeniedError(OrganizationServiceError): + """Raised when a user tries to work with a foreign organization.""" + + +class OrganizationForbiddenError(OrganizationServiceError): + """Raised when a user does not have enough privileges.""" + + +@dataclass(slots=True, frozen=True) +class OrganizationContext: + """Resolved organization and membership information for a request.""" + + organization: Organization + membership: OrganizationMember + + @property + def organization_id(self) -> int: + return self.organization.id + + @property + def role(self) -> OrganizationRole: + return self.membership.role + + @property + def user_id(self) -> int: + return self.membership.user_id + + +class OrganizationService: + """Encapsulates organization-specific policies.""" + + def __init__(self, repository: OrganizationRepository) -> None: + self._repository = repository + + async def get_context(self, *, user_id: int, organization_id: int | None) -> OrganizationContext: + """Resolve request context ensuring the user belongs to the given organization.""" + + if organization_id is None: + raise OrganizationContextMissingError("X-Organization-Id header is required") + + membership = await self._repository.get_membership(organization_id, user_id) + if membership is None or membership.organization is None: + raise OrganizationAccessDeniedError("Organization not found") + + return OrganizationContext(organization=membership.organization, membership=membership) + + def ensure_entity_in_context(self, *, entity_organization_id: int, context: OrganizationContext) -> None: + """Make sure a resource belongs to the current organization.""" + + if entity_organization_id != context.organization_id: + raise OrganizationAccessDeniedError("Resource belongs to another organization") + + def ensure_can_manage_settings(self, context: OrganizationContext) -> None: + """Allow only owner/admin to change organization-level settings.""" + + if context.role not in {OrganizationRole.OWNER, OrganizationRole.ADMIN}: + raise OrganizationForbiddenError("Only owner/admin can modify organization settings") + + def ensure_can_manage_entity(self, context: OrganizationContext) -> None: + """Managers/admins/owners may manage entities; members are restricted.""" + + if context.role == OrganizationRole.MEMBER: + raise OrganizationForbiddenError("Members cannot manage shared entities") + + def ensure_member_owns_entity(self, *, context: OrganizationContext, owner_id: int) -> None: + """Members can only mutate entities they own (contacts/deals/tasks).""" + + if context.role == OrganizationRole.MEMBER and owner_id != context.user_id: + raise OrganizationForbiddenError("Members can only modify their own records") \ No newline at end of file diff --git a/tests/services/test_organization_service.py b/tests/services/test_organization_service.py new file mode 100644 index 0000000..a1c3322 --- /dev/null +++ b/tests/services/test_organization_service.py @@ -0,0 +1,99 @@ +"""Unit tests for OrganizationService.""" +from __future__ import annotations + +from unittest.mock import MagicMock + +import pytest # type: ignore[import-not-found] +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.organization import Organization +from app.models.organization_member import OrganizationMember, OrganizationRole +from app.repositories.org_repo import OrganizationRepository +from app.services.organization_service import ( + OrganizationAccessDeniedError, + OrganizationContext, + OrganizationContextMissingError, + OrganizationForbiddenError, + OrganizationService, +) + + +class StubOrganizationRepository(OrganizationRepository): + """Simple in-memory stand-in for OrganizationRepository.""" + + def __init__(self, membership: OrganizationMember | None) -> None: + super().__init__(session=MagicMock(spec=AsyncSession)) + self._membership = membership + + async def get_membership(self, organization_id: int, user_id: int) -> OrganizationMember | None: # pragma: no cover - helper + if ( + self._membership + and self._membership.organization_id == organization_id + and self._membership.user_id == user_id + ): + return self._membership + return None + + +def make_membership(role: OrganizationRole, *, organization_id: int = 1, user_id: int = 10) -> OrganizationMember: + organization = Organization(name="Acme Inc") + organization.id = organization_id + membership = OrganizationMember( + organization_id=organization_id, + user_id=user_id, + role=role, + ) + membership.organization = organization + return membership + + +@pytest.mark.asyncio +async def test_get_context_success() -> None: + membership = make_membership(OrganizationRole.MANAGER) + service = OrganizationService(StubOrganizationRepository(membership)) + + context = await service.get_context(user_id=membership.user_id, organization_id=membership.organization_id) + + assert context.organization_id == membership.organization_id + assert context.role == OrganizationRole.MANAGER + + +@pytest.mark.asyncio +async def test_get_context_missing_header() -> None: + service = OrganizationService(StubOrganizationRepository(None)) + + with pytest.raises(OrganizationContextMissingError): + await service.get_context(user_id=1, organization_id=None) + + +@pytest.mark.asyncio +async def test_get_context_access_denied() -> None: + service = OrganizationService(StubOrganizationRepository(None)) + + with pytest.raises(OrganizationAccessDeniedError): + await service.get_context(user_id=1, organization_id=99) + + +def test_ensure_can_manage_settings_blocks_manager() -> None: + membership = make_membership(OrganizationRole.MANAGER) + organization = membership.organization + assert organization is not None + context = OrganizationContext(organization=organization, membership=membership) + service = OrganizationService(StubOrganizationRepository(membership)) + + with pytest.raises(OrganizationForbiddenError): + service.ensure_can_manage_settings(context) + + +def test_member_must_own_entity() -> None: + membership = make_membership(OrganizationRole.MEMBER) + organization = membership.organization + assert organization is not None + context = OrganizationContext(organization=organization, membership=membership) + service = OrganizationService(StubOrganizationRepository(membership)) + + with pytest.raises(OrganizationForbiddenError): + service.ensure_member_owns_entity(context=context, owner_id=999) + + # Same owner should pass silently. + service.ensure_member_owns_entity(context=context, owner_id=membership.user_id) \ No newline at end of file