diff --git a/app/api/v1/auth.py b/app/api/v1/auth.py index a44b86d..ce6ac79 100644 --- a/app/api/v1/auth.py +++ b/app/api/v1/auth.py @@ -3,6 +3,7 @@ from __future__ import annotations from pydantic import BaseModel, EmailStr from fastapi import APIRouter, Depends, HTTPException, status +from sqlalchemy import select from sqlalchemy.exc import IntegrityError from app.api.deps import get_auth_service, get_user_repository @@ -19,7 +20,7 @@ class RegisterRequest(BaseModel): email: EmailStr password: str name: str - organization_name: str + organization_name: str | None = None router = APIRouter(prefix="/auth", tags=["auth"]) @@ -37,21 +38,33 @@ async def register_user( if existing is not None: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="User already exists") - organization = Organization(name=payload.organization_name) - repo.session.add(organization) - await repo.session.flush() + organization: Organization | None = None + if payload.organization_name: + existing_org = await repo.session.scalar( + select(Organization).where(Organization.name == payload.organization_name) + ) + if existing_org is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Organization already exists", + ) + + organization = Organization(name=payload.organization_name) + repo.session.add(organization) + await repo.session.flush() user_data = UserCreate(email=payload.email, password=payload.password, name=payload.name) hashed_password = password_hasher.hash(payload.password) try: user = await repo.create(data=user_data, hashed_password=hashed_password) - membership = OrganizationMember( - organization_id=organization.id, - user_id=user.id, - role=OrganizationRole.OWNER, - ) - repo.session.add(membership) + if organization is not None: + membership = OrganizationMember( + organization_id=organization.id, + user_id=user.id, + role=OrganizationRole.OWNER, + ) + repo.session.add(membership) await repo.session.commit() except IntegrityError as exc: await repo.session.rollback() diff --git a/app/api/v1/organizations.py b/app/api/v1/organizations.py index 52d2dbc..70e679b 100644 --- a/app/api/v1/organizations.py +++ b/app/api/v1/organizations.py @@ -1,16 +1,36 @@ """Organization-related API endpoints.""" from __future__ import annotations -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException, status +from pydantic import BaseModel, EmailStr -from app.api.deps import get_current_user, get_organization_repository +from app.api.deps import ( + get_current_user, + get_organization_context, + get_organization_repository, + get_organization_service, + get_user_repository, +) from app.models.organization import OrganizationRead +from app.models.organization_member import OrganizationMemberRead, OrganizationRole from app.models.user import User from app.repositories.org_repo import OrganizationRepository +from app.repositories.user_repo import UserRepository +from app.services.organization_service import ( + OrganizationContext, + OrganizationForbiddenError, + OrganizationMemberAlreadyExistsError, + OrganizationService, +) router = APIRouter(prefix="/organizations", tags=["organizations"]) +class AddMemberPayload(BaseModel): + email: EmailStr + role: OrganizationRole = OrganizationRole.MEMBER + + @router.get("/me", response_model=list[OrganizationRead]) async def list_user_organizations( current_user: User = Depends(get_current_user), @@ -20,3 +40,26 @@ async def list_user_organizations( organizations = await repo.list_for_user(current_user.id) return [OrganizationRead.model_validate(org) for org in organizations] + + +@router.post("/members", response_model=OrganizationMemberRead, status_code=status.HTTP_201_CREATED) +async def add_member_to_organization( + payload: AddMemberPayload, + context: OrganizationContext = Depends(get_organization_context), + service: OrganizationService = Depends(get_organization_service), + user_repo: UserRepository = Depends(get_user_repository), +) -> OrganizationMemberRead: + """Allow owners/admins to add existing users to their organization.""" + + user = await user_repo.get_by_email(payload.email) + if user is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found") + + try: + membership = await service.add_member(context=context, user_id=user.id, role=payload.role) + except OrganizationMemberAlreadyExistsError as exc: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc + except OrganizationForbiddenError as exc: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(exc)) from exc + + return OrganizationMemberRead.model_validate(membership) diff --git a/app/models/activity.py b/app/models/activity.py index 89daa10..b3d16dd 100644 --- a/app/models/activity.py +++ b/app/models/activity.py @@ -11,7 +11,7 @@ from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.types import JSON as GenericJSON, TypeDecorator from sqlalchemy.orm import Mapped, mapped_column, relationship -from app.models.base import Base +from app.models.base import Base, enum_values class ActivityType(StrEnum): @@ -46,7 +46,9 @@ class Activity(Base): author_id: Mapped[int | None] = mapped_column( ForeignKey("users.id", ondelete="SET NULL"), nullable=True ) - type: Mapped[ActivityType] = mapped_column(SqlEnum(ActivityType, name="activity_type"), nullable=False) + type: Mapped[ActivityType] = mapped_column( + SqlEnum(ActivityType, name="activity_type", values_callable=enum_values), nullable=False + ) payload: Mapped[dict[str, Any]] = mapped_column( JSONBCompat().with_variant(GenericJSON(), "sqlite"), nullable=False, diff --git a/app/models/base.py b/app/models/base.py index 6e65077..3eb91e7 100644 --- a/app/models/base.py +++ b/app/models/base.py @@ -1,6 +1,13 @@ """Declarative base for SQLAlchemy models.""" +from __future__ import annotations + +from enum import StrEnum +from typing import TypeVar + from sqlalchemy.orm import DeclarativeBase, declared_attr +EnumT = TypeVar("EnumT", bound=StrEnum) + class Base(DeclarativeBase): """Base class that configures naming conventions.""" @@ -8,3 +15,9 @@ class Base(DeclarativeBase): @declared_attr.directive def __tablename__(cls) -> str: # type: ignore[misc] return cls.__name__.lower() + + +def enum_values(enum_cls: type[EnumT]) -> list[str]: + """Return enum member values to keep DB representation stable.""" + + return [member.value for member in enum_cls] diff --git a/app/models/deal.py b/app/models/deal.py index 628ea8e..b142589 100644 --- a/app/models/deal.py +++ b/app/models/deal.py @@ -9,7 +9,7 @@ from pydantic import BaseModel, ConfigDict from sqlalchemy import DateTime, Enum as SqlEnum, ForeignKey, Integer, Numeric, String, func from sqlalchemy.orm import Mapped, mapped_column, relationship -from app.models.base import Base +from app.models.base import Base, enum_values class DealStatus(StrEnum): @@ -39,10 +39,14 @@ class Deal(Base): amount: Mapped[Decimal | None] = mapped_column(Numeric(12, 2), nullable=True) currency: Mapped[str | None] = mapped_column(String(8), nullable=True) status: Mapped[DealStatus] = mapped_column( - SqlEnum(DealStatus, name="deal_status"), nullable=False, default=DealStatus.NEW + SqlEnum(DealStatus, name="deal_status", values_callable=enum_values), + nullable=False, + default=DealStatus.NEW, ) stage: Mapped[DealStage] = mapped_column( - SqlEnum(DealStage, name="deal_stage"), nullable=False, default=DealStage.QUALIFICATION + SqlEnum(DealStage, name="deal_stage", values_callable=enum_values), + nullable=False, + default=DealStage.QUALIFICATION, ) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), server_default=func.now(), nullable=False diff --git a/app/models/organization_member.py b/app/models/organization_member.py index ec434d3..ab67c21 100644 --- a/app/models/organization_member.py +++ b/app/models/organization_member.py @@ -8,7 +8,7 @@ from pydantic import BaseModel, ConfigDict from sqlalchemy import DateTime, Enum as SqlEnum, ForeignKey, Integer, UniqueConstraint, func from sqlalchemy.orm import Mapped, mapped_column, relationship -from app.models.base import Base +from app.models.base import Base, enum_values class OrganizationRole(StrEnum): @@ -30,7 +30,11 @@ class OrganizationMember(Base): organization_id: Mapped[int] = mapped_column(ForeignKey("organizations.id", ondelete="CASCADE")) user_id: Mapped[int] = mapped_column(ForeignKey("users.id", ondelete="CASCADE")) role: Mapped[OrganizationRole] = mapped_column( - SqlEnum(OrganizationRole, name="organization_role"), + SqlEnum( + OrganizationRole, + name="organization_role", + values_callable=enum_values, + ), nullable=False, default=OrganizationRole.MEMBER, ) diff --git a/app/services/organization_service.py b/app/services/organization_service.py index c2ddafc..6d8354a 100644 --- a/app/services/organization_service.py +++ b/app/services/organization_service.py @@ -24,6 +24,10 @@ class OrganizationForbiddenError(OrganizationServiceError): """Raised when a user does not have enough privileges.""" +class OrganizationMemberAlreadyExistsError(OrganizationServiceError): + """Raised when attempting to add a duplicate organization member.""" + + @dataclass(slots=True, frozen=True) class OrganizationContext: """Resolved organization and membership information for a request.""" @@ -84,4 +88,29 @@ class OrganizationService: """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 + raise OrganizationForbiddenError("Members can only modify their own records") + + async def add_member( + self, + *, + context: OrganizationContext, + user_id: int, + role: OrganizationRole, + ) -> OrganizationMember: + """Add a user to the current organization enforced by permissions.""" + + self.ensure_can_manage_settings(context) + + existing = await self._repository.get_membership(context.organization_id, user_id) + if existing is not None: + raise OrganizationMemberAlreadyExistsError("User already belongs to this organization") + + membership = OrganizationMember( + organization_id=context.organization_id, + user_id=user_id, + role=role, + ) + self._repository.session.add(membership) + await self._repository.session.commit() + await self._repository.session.refresh(membership) + return membership \ No newline at end of file diff --git a/test_db_filling.sql b/test_db_filling.sql new file mode 100644 index 0000000..1f458d3 --- /dev/null +++ b/test_db_filling.sql @@ -0,0 +1,65 @@ +TRUNCATE TABLE activities CASCADE; +TRUNCATE TABLE contacts CASCADE; +TRUNCATE TABLE deals CASCADE; +TRUNCATE TABLE organization_members CASCADE; +TRUNCATE TABLE organizations CASCADE; +TRUNCATE TABLE tasks CASCADE; +TRUNCATE TABLE users CASCADE; + +-- Пользователи +INSERT INTO users (id, email, hashed_password, name, is_active, created_at) +VALUES + (1, 'owner@example.com', 'pbkdf2_sha256$260000$demo$Tk5YEtPJj6..', 'Alice Owner', TRUE, now()), + (2, 'manager@example.com', 'pbkdf2_sha256$260000$demo$Tk5YEtPJj6..', 'Bob Manager', TRUE, now()), + (3, 'member@example.com', 'pbkdf2_sha256$260000$demo$Tk5YEtPJj6..', 'Carol Member', TRUE, now()); + +-- Организации +INSERT INTO organizations (id, name, created_at) +VALUES + (1, 'Acme Corp', now()), + (2, 'Beta LLC', now()); + +-- Участники организаций +INSERT INTO organization_members (id, organization_id, user_id, role, created_at) +VALUES + (1, 1, 1, 'owner', now()), + (2, 1, 2, 'manager', now()), + (3, 1, 3, 'member', now()), + (4, 2, 2, 'owner', now()); + +-- Контакты (в рамках орг. 1) +INSERT INTO contacts (id, organization_id, owner_id, name, email, phone, created_at) +VALUES + (1, 1, 2, 'John Doe', 'john.doe@acme.com', '+1-202-555-0101', now()), + (2, 1, 3, 'Jane Smith', 'jane.smith@acme.com', '+1-202-555-0102', now()); + +-- Сделки +INSERT INTO deals ( + id, organization_id, contact_id, owner_id, title, amount, currency, + status, stage, created_at, updated_at +) VALUES + (1, 1, 1, 2, 'Website Redesign', 15000.00, 'USD', 'in_progress', 'proposal', now(), now()), + (2, 1, 2, 3, 'Support Contract', 5000.00, 'USD', 'new', 'qualification', now(), now()); + +-- Задачи +INSERT INTO tasks ( + id, deal_id, title, description, due_date, is_done, created_at +) VALUES + (1, 1, 'Prepare proposal', 'Draft technical scope', now() + interval '5 days', FALSE, now()), + (2, 2, 'Call client', 'Discuss onboarding plan', now() + interval '3 days', FALSE, now()); + +-- Активности +INSERT INTO activities ( + id, deal_id, author_id, type, payload, created_at +) VALUES + (1, 1, 2, 'comment', '{"text": "Kickoff meeting scheduled"}', now()), + (2, 1, 2, 'status_changed', '{"old_status": "new", "new_status": "in_progress"}', now()), + (3, 2, 3, 'task_created', '{"task_id": 2, "title": "Call client"}', now()); + +SELECT setval('users_id_seq', COALESCE((SELECT MAX(id) FROM users), 0), (SELECT MAX(id) FROM users) IS NOT NULL); +SELECT setval('organizations_id_seq', COALESCE((SELECT MAX(id) FROM organizations), 0), (SELECT MAX(id) FROM organizations) IS NOT NULL); +SELECT setval('organization_members_id_seq', COALESCE((SELECT MAX(id) FROM organization_members), 0), (SELECT MAX(id) FROM organization_members) IS NOT NULL); +SELECT setval('contacts_id_seq', COALESCE((SELECT MAX(id) FROM contacts), 0), (SELECT MAX(id) FROM contacts) IS NOT NULL); +SELECT setval('deals_id_seq', COALESCE((SELECT MAX(id) FROM deals), 0), (SELECT MAX(id) FROM deals) IS NOT NULL); +SELECT setval('tasks_id_seq', COALESCE((SELECT MAX(id) FROM tasks), 0), (SELECT MAX(id) FROM tasks) IS NOT NULL); +SELECT setval('activities_id_seq', COALESCE((SELECT MAX(id) FROM activities), 0), (SELECT MAX(id) FROM activities) IS NOT NULL); \ No newline at end of file diff --git a/tests/api/v1/test_auth.py b/tests/api/v1/test_auth.py index 37eb090..2369747 100644 --- a/tests/api/v1/test_auth.py +++ b/tests/api/v1/test_auth.py @@ -51,6 +51,57 @@ async def test_register_user_creates_organization_membership( assert membership.role == OrganizationRole.OWNER +@pytest.mark.asyncio +async def test_register_user_without_organization_succeeds( + session_factory: async_sessionmaker[AsyncSession], + client: AsyncClient, +) -> None: + payload = { + "email": "solo-user@example.com", + "password": "StrongPass123!", + "name": "Solo User", + } + + response = await client.post("/api/v1/auth/register", json=payload) + + assert response.status_code == 201 + + async with session_factory() as session: + user = await session.scalar(select(User).where(User.email == payload["email"])) + assert user is not None + + membership = await session.scalar( + select(OrganizationMember).where(OrganizationMember.user_id == user.id) + ) + assert membership is None + + +@pytest.mark.asyncio +async def test_register_fails_when_organization_exists( + client: AsyncClient, +) -> None: + payload = { + "email": "owner-one@example.com", + "password": "StrongPass123!", + "name": "Owner One", + "organization_name": "Duplicate Org", + } + response = await client.post("/api/v1/auth/register", json=payload) + assert response.status_code == 201 + + duplicate_payload = { + "email": "owner-two@example.com", + "password": "StrongPass123!", + "name": "Owner Two", + "organization_name": "Duplicate Org", + } + + duplicate_response = await client.post("/api/v1/auth/register", json=duplicate_payload) + + assert duplicate_response.status_code == 409 + assert duplicate_response.json()["detail"] == "Organization already exists" + + @pytest.mark.asyncio async def test_login_endpoint_returns_token_for_valid_credentials( session_factory: async_sessionmaker[AsyncSession], diff --git a/tests/api/v1/test_organizations.py b/tests/api/v1/test_organizations.py index 13a97c3..c078ddd 100644 --- a/tests/api/v1/test_organizations.py +++ b/tests/api/v1/test_organizations.py @@ -7,6 +7,7 @@ from typing import AsyncGenerator, Sequence, cast import pytest import pytest_asyncio from httpx import ASGITransport, AsyncClient +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from sqlalchemy.schema import Table @@ -101,3 +102,188 @@ async def test_list_user_organizations_returns_memberships( async def test_list_user_organizations_requires_token(client: AsyncClient) -> None: response = await client.get("/api/v1/organizations/me") assert response.status_code == 401 + + +@pytest.mark.asyncio +async def test_owner_can_add_member_to_organization( + session_factory: async_sessionmaker[AsyncSession], + client: AsyncClient, +) -> None: + async with session_factory() as session: + owner = User(email="owner-add@example.com", hashed_password="hashed", name="Owner", is_active=True) + invitee = User(email="new-member@example.com", hashed_password="hashed", name="Member", is_active=True) + session.add_all([owner, invitee]) + await session.flush() + + organization = Organization(name="Membership LLC") + session.add(organization) + await session.flush() + + membership = OrganizationMember( + organization_id=organization.id, + user_id=owner.id, + role=OrganizationRole.OWNER, + ) + session.add(membership) + await session.commit() + + token = jwt_service.create_access_token( + subject=str(owner.id), + expires_delta=timedelta(minutes=30), + claims={"email": owner.email}, + ) + + response = await client.post( + "/api/v1/organizations/members", + headers={ + "Authorization": f"Bearer {token}", + "X-Organization-Id": str(organization.id), + }, + json={"email": invitee.email, "role": OrganizationRole.MANAGER.value}, + ) + + assert response.status_code == 201 + payload = response.json() + assert payload["organization_id"] == organization.id + assert payload["user_id"] == invitee.id + assert payload["role"] == OrganizationRole.MANAGER.value + + async with session_factory() as session: + new_membership = await session.scalar( + select(OrganizationMember).where( + OrganizationMember.organization_id == organization.id, + OrganizationMember.user_id == invitee.id, + ) + ) + assert new_membership is not None + assert new_membership.role == OrganizationRole.MANAGER + + +@pytest.mark.asyncio +async def test_add_member_requires_existing_user( + session_factory: async_sessionmaker[AsyncSession], + client: AsyncClient, +) -> None: + async with session_factory() as session: + owner = User(email="owner-missing@example.com", hashed_password="hashed", name="Owner", is_active=True) + session.add(owner) + await session.flush() + + organization = Organization(name="Missing LLC") + session.add(organization) + await session.flush() + + membership = OrganizationMember( + organization_id=organization.id, + user_id=owner.id, + role=OrganizationRole.OWNER, + ) + session.add(membership) + await session.commit() + + token = jwt_service.create_access_token( + subject=str(owner.id), + expires_delta=timedelta(minutes=30), + claims={"email": owner.email}, + ) + + response = await client.post( + "/api/v1/organizations/members", + headers={ + "Authorization": f"Bearer {token}", + "X-Organization-Id": str(organization.id), + }, + json={"email": "ghost@example.com"}, + ) + + assert response.status_code == 404 + assert response.json()["detail"] == "User not found" + + +@pytest.mark.asyncio +async def test_member_role_cannot_add_users( + session_factory: async_sessionmaker[AsyncSession], + client: AsyncClient, +) -> None: + async with session_factory() as session: + member_user = User(email="member@example.com", hashed_password="hashed", name="Member", is_active=True) + invitee = User(email="invitee@example.com", hashed_password="hashed", name="Invitee", is_active=True) + session.add_all([member_user, invitee]) + await session.flush() + + organization = Organization(name="Members Only LLC") + session.add(organization) + await session.flush() + + membership = OrganizationMember( + organization_id=organization.id, + user_id=member_user.id, + role=OrganizationRole.MEMBER, + ) + session.add(membership) + await session.commit() + + token = jwt_service.create_access_token( + subject=str(member_user.id), + expires_delta=timedelta(minutes=30), + claims={"email": member_user.email}, + ) + + response = await client.post( + "/api/v1/organizations/members", + headers={ + "Authorization": f"Bearer {token}", + "X-Organization-Id": str(organization.id), + }, + json={"email": invitee.email}, + ) + + assert response.status_code == 403 + assert response.json()["detail"] == "Only owner/admin can modify organization settings" + + +@pytest.mark.asyncio +async def test_cannot_add_duplicate_member( + session_factory: async_sessionmaker[AsyncSession], + client: AsyncClient, +) -> None: + async with session_factory() as session: + owner = User(email="dup-owner@example.com", hashed_password="hashed", name="Owner", is_active=True) + invitee = User(email="dup-member@example.com", hashed_password="hashed", name="Invitee", is_active=True) + session.add_all([owner, invitee]) + await session.flush() + + organization = Organization(name="Duplicate LLC") + session.add(organization) + await session.flush() + + owner_membership = OrganizationMember( + organization_id=organization.id, + user_id=owner.id, + role=OrganizationRole.OWNER, + ) + invitee_membership = OrganizationMember( + organization_id=organization.id, + user_id=invitee.id, + role=OrganizationRole.MEMBER, + ) + session.add_all([owner_membership, invitee_membership]) + await session.commit() + + token = jwt_service.create_access_token( + subject=str(owner.id), + expires_delta=timedelta(minutes=30), + claims={"email": owner.email}, + ) + + response = await client.post( + "/api/v1/organizations/members", + headers={ + "Authorization": f"Bearer {token}", + "X-Organization-Id": str(organization.id), + }, + json={"email": invitee.email}, + ) + + assert response.status_code == 409 + assert response.json()["detail"] == "User already belongs to this organization" diff --git a/tests/models/test_enums.py b/tests/models/test_enums.py new file mode 100644 index 0000000..9c4021f --- /dev/null +++ b/tests/models/test_enums.py @@ -0,0 +1,32 @@ +"""Regression tests ensuring Enum mappings store lowercase values.""" +from __future__ import annotations + +from enum import StrEnum + +from app.models.activity import Activity, ActivityType +from app.models.deal import Deal, DealStage, DealStatus +from app.models.organization_member import OrganizationMember, OrganizationRole + + +def _values(enum_cls: type[StrEnum]) -> list[str]: + return [member.value for member in enum_cls] + + +def test_organization_role_column_uses_value_strings() -> None: + role_type = OrganizationMember.__table__.c.role.type # noqa: SLF001 - runtime inspection + assert role_type.enums == _values(OrganizationRole) + + +def test_deal_status_column_uses_value_strings() -> None: + status_type = Deal.__table__.c.status.type # noqa: SLF001 - runtime inspection + assert status_type.enums == _values(DealStatus) + + +def test_deal_stage_column_uses_value_strings() -> None: + stage_type = Deal.__table__.c.stage.type # noqa: SLF001 - runtime inspection + assert stage_type.enums == _values(DealStage) + + +def test_activity_type_column_uses_value_strings() -> None: + activity_type = Activity.__table__.c.type.type # noqa: SLF001 - runtime inspection + assert activity_type.enums == _values(ActivityType)