Преглед изворни кода

feat: implement commission enpoints and refactor auth tests

clovis пре 1 недеља
родитељ
комит
39360e8322

+ 12 - 2
.idea/pyLspTools.xml

@@ -1,6 +1,16 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <?xml version="1.0" encoding="UTF-8"?>
 <project version="4">
 <project version="4">
-  <component name="RuffConfiguration">
-    <option name="enabled" value="true" />
+  <component name="PyToolsState">
+    <option name="tools">
+      <map>
+        <entry key="ruff">
+          <value>
+            <ToolEntry>
+              <option name="enabled" value="true" />
+            </ToolEntry>
+          </value>
+        </entry>
+      </map>
+    </option>
   </component>
   </component>
 </project>
 </project>

+ 79 - 8
Readme.md

@@ -1,13 +1,38 @@
 # bdlg-2023-server
 # bdlg-2023-server
 
 
-This repository host an API that can parse gsheet planning for brass dans la garonne event and manage creating automatic
-SMS notification for volunteer.
+This repository hosts an API that helps volunteer managers plan festival
+activities on a schedule, associate volunteers to time slots, and send SMS
+reminders during the event. Volunteer/slot data can also be bootstrapped
+from a Google Sheet import.
+
+## Core concepts
+
+- **Organization** — the top-level tenant. Every user belongs to zero or
+  more organizations (multi-org membership is supported). Every project
+  belongs to exactly one organization, which defines who can access it.
+- **Global roles** — `user` (default) or `super_admin`. Only `super_admin`
+  can create/edit organizations and manage organization membership.
+- **Organization roles** (per user, per organization):
+    - `org_admin` — full control over the organization's projects and can
+      change member roles within their own organization.
+    - `respo_benevole` — manages volunteer allocation across the whole
+      project: slots, templates, volunteers, groups, and SMS.
+    - `respo_commission` — read access to the whole project plan; write
+      access limited to templates/slots belonging to the commission(s) they
+      are a member of. Cannot manage volunteers, groups, or SMS directly.
+- **Commission** (Pôle) — represents a team/area within a project (e.g.
+  "Bar", "Accueil"). A commission's contact info is derived from its
+  members' own profile (`name` + `phone_number`) rather than stored as
+  free text — see `SlotTemplate.responsible_override` for the manual
+  exception case.
+- **Volunteer group** — a saved, searchable set of volunteers within a
+  project, usable for bulk slot assignment and group SMS.
 
 
 ## Getting started
 ## Getting started
 
 
 For running the application
 For running the application
 
 
-```bash 
+```bash
 git clone ...
 git clone ...
 cd ...
 cd ...
 python -m venv venv
 python -m venv venv
@@ -24,34 +49,80 @@ init.sh
 uvicorn app.main:app --reload
 uvicorn app.main:app --reload
 ```
 ```
 
 
+### Bootstrapping the first organization / super_admin
+
+New databases start with no organizations and no `super_admin`. After
+running migrations, promote the first user manually (there is currently
+no API endpoint for this, by design — it's a one-time operational step):
+
+```sql
+UPDATE user_model
+SET global_role = 'SUPER_ADMIN'
+WHERE email = '<you>';
+```
+
+From there, that user can create organizations and add members via the
+`/organizations` endpoints.
+
 ## Debug
 ## Debug
 
 
-```bash 
+```bash
 python -m app.debug
 python -m app.debug
 ```
 ```
 
 
 Run tests
 Run tests
 
 
-```bash 
+```bash
 pytest
 pytest
 pytest app\test\test_volunteer.py
 pytest app\test\test_volunteer.py
 ```
 ```
 
 
 Run coverage tests
 Run coverage tests
 
 
-```bash 
+```bash
 coverage run -m pytest
 coverage run -m pytest
 coverage html
 coverage html
 ```
 ```
 
 
+## Database migrations
+
+```bash
+# create migration
+alembic revision --autogenerate -m "migration_name"
+# apply all migrations
+alembic upgrade head
+```
+
+Autogenerated migrations are a starting point, not a final draft —
+review them by hand, especially for:
+
+- new `NOT NULL` columns on tables that may already have data (add
+  nullable, backfill, then tighten in the same migration)
+- new Postgres ENUM types referenced outside a `create_table` block
+  (need an explicit `create_type=False` + manual `CREATE TYPE`/`DROP TYPE`)
+- FK `ondelete` behavior vs. the SQLAlchemy relationship's own cascade
+  config (see `passive_deletes=True` on `Organization.projects` — without
+  it, `session.delete(org)` tries to null out `Project.organization_id`
+  in Python before the DB's `ON DELETE CASCADE` ever runs, which fails
+  since that column is `NOT NULL`)
+
 ## Update requirements
 ## Update requirements
 
 
 ```bash
 ```bash
 poetry lock
 poetry lock
 poetry export -f requirements.txt --output requirements.txt --without-hashes
 poetry export -f requirements.txt --output requirements.txt --without-hashes
-oetry export -f requirements.txt --output requirements-dev.txt --without-hashes --with dev
+poetry export -f requirements.txt --output requirements-dev.txt --without-hashes --with dev
 ```
 ```
 
 
+## Update frontend API types
+
+```bash
+npm run update-schema
+```
+
+Regenerate after any backend route or schema change — see the frontend
+repo's README for details.
+
 ## Credit
 ## Credit
 
 
-FastAPI project generated using https://github.com/rafsaf/minimal-fastapi-postgres-template
+FastAPI project generated using https://github.com/rafsaf/minimal-fastapi-postgres-template

+ 6 - 4
app/api/api.py

@@ -2,16 +2,17 @@ from fastapi import APIRouter
 
 
 from app.api.endpoints import (
 from app.api.endpoints import (
     auth,
     auth,
+    commissions,
+    organizations,
     project,
     project,
     slots,
     slots,
-    users,
-    volunteers,
     sms,
     sms,
+    sms_sender,
     tags,
     tags,
     templates,
     templates,
-    sms_sender,
-    organizations,
+    users,
     volunteer_groups,
     volunteer_groups,
+    volunteers,
 )
 )
 
 
 api_router = APIRouter()
 api_router = APIRouter()
@@ -19,6 +20,7 @@ api_router.include_router(auth.router, prefix="/auth", tags=["auth"])
 api_router.include_router(users.router, prefix="/users", tags=["users"])
 api_router.include_router(users.router, prefix="/users", tags=["users"])
 api_router.include_router(organizations.router, prefix="/organizations", tags=["organization"])
 api_router.include_router(organizations.router, prefix="/organizations", tags=["organization"])
 api_router.include_router(project.router, tags=["project"])
 api_router.include_router(project.router, tags=["project"])
+api_router.include_router(commissions.router, tags=["commissions"])
 api_router.include_router(slots.router, tags=["slot"])
 api_router.include_router(slots.router, tags=["slot"])
 api_router.include_router(tags.router, tags=["tag"])
 api_router.include_router(tags.router, tags=["tag"])
 api_router.include_router(volunteers.router, tags=["volunteer"])
 api_router.include_router(volunteers.router, tags=["volunteer"])

+ 145 - 0
app/api/endpoints/commissions.py

@@ -0,0 +1,145 @@
+from uuid import UUID
+
+from fastapi import APIRouter, Depends, HTTPException
+from sqlalchemy import select
+from sqlalchemy.orm import Session
+
+from app.api import deps
+from app.api.utils import assert_project_exists_or_404
+from app.models import Commission, CommissionMember, OrgRole, User
+from app.schemas.requests import (
+    CommissionCreateRequest,
+    CommissionMembershipRequest,
+    CommissionUpdateRequest,
+)
+from app.schemas.responses import CommissionResponse
+
+router = APIRouter(prefix="/project/{project_id}", tags=["commissions"])
+
+READ_COMMISSIONS = (OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION)
+MANAGE_COMMISSIONS = (OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE)
+
+
+def _get_commission_or_404(session: Session, project_id: UUID, commission_id: UUID) -> Commission:
+    commission = session.get(Commission, commission_id)
+    if commission is None or commission.project_id != str(project_id):
+        raise HTTPException(status_code=404, detail="Commission not found")
+    return commission
+
+
+@router.get("/commissions", response_model=list[CommissionResponse])
+async def list_project_commissions(
+    project_id: UUID,
+    current_user: User = Depends(deps.require_org_role(*READ_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """List all commissions for a specific project"""
+    assert_project_exists_or_404(session, project_id)
+    results = session.execute(select(Commission).where(Commission.project_id == project_id))
+    return results.scalars().all()
+
+
+@router.post("/commission", response_model=CommissionResponse)
+async def create_commission(
+    project_id: UUID,
+    new_commission: CommissionCreateRequest,
+    current_user: User = Depends(deps.require_org_role(*MANAGE_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """Create a new commission in the project"""
+    assert_project_exists_or_404(session, project_id)
+    commission = Commission(project_id=project_id, **new_commission.model_dump())
+    session.add(commission)
+    session.commit()
+    session.refresh(commission)
+    return commission
+
+
+@router.get("/commission/{commission_id}", response_model=CommissionResponse)
+async def get_commission(
+    project_id: UUID,
+    commission_id: UUID,
+    current_user: User = Depends(deps.require_org_role(*READ_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """Get a single commission"""
+    return _get_commission_or_404(session, project_id, commission_id)
+
+
+@router.patch("/commission/{commission_id}", response_model=CommissionResponse)
+async def update_commission(
+    project_id: UUID,
+    commission_id: UUID,
+    edit_commission: CommissionUpdateRequest,
+    current_user: User = Depends(deps.require_org_role(*MANAGE_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """Edit a commission's details"""
+    commission = _get_commission_or_404(session, project_id, commission_id)
+    commission.name = edit_commission.name
+    session.commit()
+    session.refresh(commission)
+    return commission
+
+
+@router.delete("/commission/{commission_id}")
+async def delete_commission(
+    project_id: UUID,
+    commission_id: UUID,
+    current_user: User = Depends(deps.require_org_role(*MANAGE_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """Delete a commission (automatically deletes CommissionMember links via cascade)"""
+    commission = _get_commission_or_404(session, project_id, commission_id)
+    session.delete(commission)
+    session.commit()
+
+
+@router.post("/commission/{commission_id}/members", response_model=CommissionResponse)
+async def add_members_to_commission(
+    project_id: UUID,
+    commission_id: UUID,
+    payload: CommissionMembershipRequest,
+    current_user: User = Depends(deps.require_org_role(*MANAGE_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """Bulk-add users as members to a commission (idempotent)"""
+    commission = _get_commission_or_404(session, project_id, commission_id)
+
+    # Verify users exist
+    valid_users = session.scalars(select(User).where(User.id.in_(payload.user_ids))).all()
+    if len(valid_users) != len(payload.user_ids):
+        raise HTTPException(status_code=400, detail="One or more invalid user IDs")
+
+    existing_user_ids = {str(m.user_id) for m in commission.members}
+
+    for user in valid_users:
+        if str(user.id) in existing_user_ids:
+            continue
+        new_member = CommissionMember(commission_id=commission.id, user_id=user.id)
+        commission.members.append(new_member)
+
+    session.commit()
+    session.refresh(commission)
+    return commission
+
+
+@router.delete("/commission/{commission_id}/member/{user_id}", response_model=CommissionResponse)
+async def remove_member_from_commission(
+    project_id: UUID,
+    commission_id: UUID,
+    user_id: UUID,
+    current_user: User = Depends(deps.require_org_role(*MANAGE_COMMISSIONS)),
+    session: Session = Depends(deps.get_session),
+):
+    """Remove a single user from a commission"""
+    commission = _get_commission_or_404(session, project_id, commission_id)
+
+    member_to_remove = next((m for m in commission.members if str(m.user_id) == str(user_id)), None)
+
+    if member_to_remove:
+        session.delete(member_to_remove)
+        session.commit()
+        session.refresh(commission)
+
+    return commission

+ 8 - 0
app/models.py

@@ -140,6 +140,10 @@ class Project(Base):
 class Commission(Base):
 class Commission(Base):
     __tablename__ = "commissions"
     __tablename__ = "commissions"
     id: Mapped[UUID] = uid_column()
     id: Mapped[UUID] = uid_column()
+    created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now())
+    updated_at: Mapped[datetime] = mapped_column(
+        DateTime(timezone=True), default=datetime.now, onupdate=func.now()
+    )
     project_id: Mapped[UUID] = mapped_column(ForeignKey("projects.id", ondelete="CASCADE"))
     project_id: Mapped[UUID] = mapped_column(ForeignKey("projects.id", ondelete="CASCADE"))
     project: Mapped["Project"] = relationship(back_populates="commissions")
     project: Mapped["Project"] = relationship(back_populates="commissions")
     name: Mapped[str] = mapped_column(String(128), nullable=False)  # e.g. "Bar", "Accueil"
     name: Mapped[str] = mapped_column(String(128), nullable=False)  # e.g. "Bar", "Accueil"
@@ -154,6 +158,10 @@ class Commission(Base):
         """Read-only, derived from members' own profile — never stored redundantly."""
         """Read-only, derived from members' own profile — never stored redundantly."""
         return [{"name": m.user.name, "phone_number": m.user.phone_number} for m in self.members]
         return [{"name": m.user.name, "phone_number": m.user.phone_number} for m in self.members]
 
 
+    @hybrid_property
+    def templates_id(self) -> list[UUID]:
+        return [t.id for t in self.templates]
+
 
 
 class CommissionMember(Base):
 class CommissionMember(Base):
     __tablename__ = "commission_members"
     __tablename__ = "commission_members"

+ 12 - 0
app/schemas/requests.py

@@ -103,6 +103,18 @@ class ProjectSMSBatchRequest(BaseRequest):
     )
     )
 
 
 
 
+class CommissionCreateRequest(BaseModel):
+    name: str
+
+
+class CommissionUpdateRequest(BaseModel):
+    name: str
+
+
+class CommissionMembershipRequest(BaseModel):
+    user_ids: list[UUID4]
+
+
 class VolunteerCreateRequest(BaseRequest):
 class VolunteerCreateRequest(BaseRequest):
     name: str
     name: str
     surname: str | None = Field(default="")
     surname: str | None = Field(default="")

+ 9 - 13
app/schemas/responses.py

@@ -1,7 +1,7 @@
 from datetime import datetime
 from datetime import datetime
 from enum import Enum
 from enum import Enum
 
 
-from pydantic import BaseModel, ConfigDict, EmailStr
+from pydantic import UUID4, BaseModel, ConfigDict, EmailStr
 
 
 
 
 class BaseResponse(BaseModel):
 class BaseResponse(BaseModel):
@@ -9,7 +9,7 @@ class BaseResponse(BaseModel):
 
 
 
 
 class BaseObjectResponse(BaseResponse):
 class BaseObjectResponse(BaseResponse):
-    id: str
+    id: UUID4
     created_at: datetime
     created_at: datetime
     updated_at: datetime
     updated_at: datetime
 
 
@@ -73,17 +73,13 @@ class CommissionContactResponse(BaseResponse):
 class CommissionResponse(BaseObjectResponse):
 class CommissionResponse(BaseObjectResponse):
     name: str
     name: str
     contacts: list[CommissionContactResponse] = []
     contacts: list[CommissionContactResponse] = []
-    templates_id: list[str] = []
-
-    # Commission model doesn't currently expose a hybrid `templates_id` --
-    # add one alongside `contacts` on the model (mirrors the pattern already
-    # used by SlotTemplate.tags_id / Volunteer.slots_id) if this is needed.
+    templates_id: list[UUID4] = []
 
 
 
 
 class VolunteerGroupResponse(BaseObjectResponse):
 class VolunteerGroupResponse(BaseObjectResponse):
     name: str
     name: str
     color: str | None = None
     color: str | None = None
-    volunteers_id: list[str] = []
+    volunteers_id: list[UUID4] = []
 
 
 
 
 class VolunteerResponse(BaseObjectResponse):
 class VolunteerResponse(BaseObjectResponse):
@@ -92,7 +88,7 @@ class VolunteerResponse(BaseObjectResponse):
     email: str
     email: str
     phone_number: str
     phone_number: str
     automatic_sms: bool
     automatic_sms: bool
-    slots_id: list[str] = []
+    slots_id: list[UUID4] = []
     groups_id: list[str] = []
     groups_id: list[str] = []
     comment: str
     comment: str
 
 
@@ -103,11 +99,11 @@ class SlotResponse(BaseObjectResponse):
     ending_time: datetime
     ending_time: datetime
     required_volunteers: int
     required_volunteers: int
     volunteers_id: list[str] = []
     volunteers_id: list[str] = []
-    template_id: str | None
+    template_id: UUID4 | None
 
 
 
 
 class SMSResponse(BaseObjectResponse):
 class SMSResponse(BaseObjectResponse):
-    volunteer_id: str | None
+    volunteer_id: UUID4 | None
     content: str
     content: str
     phone_number: str
     phone_number: str
     sending_time: datetime
     sending_time: datetime
@@ -120,7 +116,7 @@ class TemplateResponse(BaseObjectResponse):
     place: str
     place: str
     responsible_override: str | None = None
     responsible_override: str | None = None
     commission_id: str | None = None
     commission_id: str | None = None
-    tags_id: list[str] = []
+    tags_id: list[UUID4] = []
     comment: str
     comment: str
 
 
 
 
@@ -142,7 +138,7 @@ class ProjectResponse(BaseObjectResponse):
 
 
 
 
 class ProjectListResponse(BaseResponse):
 class ProjectListResponse(BaseResponse):
-    id: str
+    id: UUID4
     created_at: datetime
     created_at: datetime
     updated_at: datetime
     updated_at: datetime
     name: str
     name: str

+ 96 - 0
app/tests/shared_access.py

@@ -0,0 +1,96 @@
+from typing import Any
+from uuid import uuid4
+
+import pytest
+from httpx import AsyncClient
+
+from app.main import app
+from app.models import OrgRole
+
+# Type alias for our route definition: (Method, Endpoint Name, Path Params, JSON Payload)
+RouteSpec = tuple[str, str, dict[str, Any], dict[str, Any] | None]
+
+
+class SharedProjectAccessTests:
+    """
+    A reusable interface for testing cross-cutting concerns (Auth, Permissions, 404s)
+    across project-scoped API endpoints.
+
+    HOW TO USE:
+    1. Inherit from this class.
+    2. Apply `@pytest.mark.parametrize("route_spec", YOUR_ROUTES, ids=your_id_formatter)`
+       to your child class.
+    3. Override the `resolved_route` fixture to map static placeholders (e.g., "PROJECT_ID")
+       to actual database fixture objects at execution time.
+    """
+
+    @pytest.fixture
+    def resolved_route(self, *args, **kwargs) -> RouteSpec:
+        """
+        INTERFACE FIXTURE: Child classes MUST override this fixture.
+
+        This fixture is responsible for taking a static `route_spec` and swapping
+        out string placeholders for real UUIDs generated by database fixtures.
+
+        Returns:
+            Tuple containing:
+            - HTTP Method (str)
+            - FastAPI endpoint name (str)
+            - Path parameters (dict)
+            - JSON payload (dict or None)
+        """
+        raise NotImplementedError(
+            "Child classes inheriting from SharedProjectAccessTests must implement "
+            "the `resolved_route` fixture to inject real database IDs."
+        )
+
+    async def test_requires_auth(self, client: AsyncClient, resolved_route: RouteSpec):
+        """Ensures the endpoint returns 401 Unauthorized if no token is provided."""
+        method, endpoint, kwargs, payload = resolved_route
+        url = app.url_path_for(endpoint, **kwargs)
+        req = getattr(client, method.lower())
+
+        response = await req(url, json=payload) if payload else await req(url)
+        assert response.status_code == 401, f"{response.status_code} != 401"
+
+    async def test_forbidden_other_org(
+        self, client: AsyncClient, other_org_user, resolved_route: RouteSpec
+    ):
+        """Ensures a user from a DIFFERENT organization receives a 403 Forbidden."""
+        method, endpoint, kwargs, payload = resolved_route
+        if "project_id" not in kwargs:
+            pytest.skip("Not a project-scoped route. Skipping 403 check.")
+        url = app.url_path_for(endpoint, **kwargs)
+        _, headers = other_org_user
+        req = getattr(client, method.lower())
+
+        response = (
+            await req(url, json=payload, headers=headers)
+            if payload
+            else await req(url, headers=headers)
+        )
+        assert response.status_code == 403, f"{response.status_code} != 403"
+
+    async def test_project_not_found(
+        self, client: AsyncClient, make_org_user, resolved_route: RouteSpec
+    ):
+        """Ensures that if the project does not exist, the API returns a 404."""
+        method, endpoint, kwargs, payload = resolved_route
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+
+        # Inject a non-existent project_id
+        kwargs_copy = kwargs.copy()
+        if "project_id" in kwargs_copy:
+            kwargs_copy["project_id"] = str(uuid4())
+        else:
+            pytest.skip("Not a project-scoped route. Skipping 403 check.")
+
+        url = app.url_path_for(endpoint, **kwargs_copy)
+        req = getattr(client, method.lower())
+
+        response = (
+            await req(url, json=payload, headers=headers)
+            if payload
+            else await req(url, headers=headers)
+        )
+        assert response.status_code == 404

+ 343 - 0
app/tests/test_commissions.py

@@ -0,0 +1,343 @@
+import uuid
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import select
+from sqlalchemy.orm import Session
+
+from app.core.session import session as session_maker
+from app.main import app
+from app.models import Commission, CommissionMember, Organization, OrgRole, Project, User
+from app.tests.shared_access import SharedProjectAccessTests
+
+pytestmark = pytest.mark.asyncio
+
+MANAGE_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE]
+FORBIDDEN_MANAGE_ROLES = [OrgRole.RESPO_COMMISSION]
+READ_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
+
+
+@pytest.fixture
+def two_users() -> tuple[User, User]:
+    with session_maker() as db:
+        u1 = User(
+            email="u1@test.com",
+            hashed_password="hashed1",
+            name="Arthur Pendragon",
+            phone_number="0601010101",
+        )
+        u2 = User(
+            email="u2@test.com",
+            hashed_password="hashed2",
+            name="Merlin Enchanteur",
+            phone_number="0602020202",
+        )
+        db.add_all([u1, u2])
+        db.commit()
+        db.refresh(u1)
+        db.refresh(u2)
+        return u1, u2
+
+
+@pytest.fixture
+def default_commission(default_project: Project) -> Commission:
+    with session_maker() as db:
+        commission = Commission(project_id=default_project.id, name="Logistique")
+        db.add(commission)
+        db.commit()
+        db.refresh(commission)
+        return commission
+
+
+COMMISSION_ROUTES = [
+    ("GET", "list_project_commissions", {"project_id": "PROJECT"}, None),
+    ("POST", "create_commission", {"project_id": "PROJECT"}, {"name": "Test"}),
+    ("GET", "get_commission", {"project_id": "PROJECT", "commission_id": "COMMISSION"}, None),
+    (
+        "PATCH",
+        "update_commission",
+        {"project_id": "PROJECT", "commission_id": "COMMISSION"},
+        {"name": "Updated"},
+    ),
+    ("DELETE", "delete_commission", {"project_id": "PROJECT", "commission_id": "COMMISSION"}, None),
+    (
+        "POST",
+        "add_members_to_commission",
+        {"project_id": "PROJECT", "commission_id": "COMMISSION"},
+        {"user_ids": ["USER"]},
+    ),
+    (
+        "DELETE",
+        "remove_member_from_commission",
+        {
+            "project_id": "PROJECT",
+            "commission_id": "COMMISSION",
+            "user_id": "USER",
+        },
+        None,
+    ),
+]
+
+
+class TestCommissionCrossCutting(SharedProjectAccessTests):
+    @pytest.fixture(params=COMMISSION_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project, default_commission):
+        method, endpoint, kwargs, payload = request.param
+        resolved_kwargs = {
+            k: (
+                default_project.id
+                if v == "PROJECT"
+                else default_commission.id
+                if v == "COMMISSION"
+                else v
+            )
+            for k, v in kwargs.items()
+        }
+
+        return method, endpoint, resolved_kwargs, payload
+
+
+class TestListCommissions:
+    @pytest.mark.parametrize("role", READ_ROLES)
+    async def test_role_access(
+        self,
+        client: AsyncClient,
+        default_project: Project,
+        default_commission: Commission,
+        make_org_user,
+        role,
+    ):
+        _, headers = make_org_user(role=role)
+        response = await client.get(
+            app.url_path_for("list_project_commissions", project_id=default_project.id),
+            headers=headers,
+        )
+        assert response.status_code == 200
+        data = response.json()
+        assert len(data) == 1
+        assert data[0]["name"] == "Logistique"
+
+
+class TestCreateCommission:
+    @pytest.mark.parametrize("role", MANAGE_ROLES)
+    async def test_role_access(
+        self, client: AsyncClient, default_project: Project, make_org_user, session: Session, role
+    ):
+        _, headers = make_org_user(role=role)
+        response = await client.post(
+            app.url_path_for("create_commission", project_id=default_project.id),
+            headers=headers,
+            json={"name": "Accueil"},
+        )
+        assert response.status_code == 200
+        commission = (
+            session.execute(select(Commission).where(Commission.name == "Accueil"))
+            .scalars()
+            .first()
+        )
+        assert commission is not None
+        assert commission.project_id == str(default_project.id)
+
+    @pytest.mark.parametrize("role", FORBIDDEN_MANAGE_ROLES)
+    async def test_forbidden_manage_roles(
+        self, client: AsyncClient, default_project: Project, make_org_user, role
+    ):
+        _, headers = make_org_user(role=role)
+        response = await client.post(
+            app.url_path_for("create_commission", project_id=default_project.id),
+            headers=headers,
+            json={"name": "Nope"},
+        )
+        assert response.status_code == 403
+
+
+class TestGetCommission:
+    async def test_commission_not_found(
+        self, client: AsyncClient, make_org_user, default_project: Project
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        response = await client.get(
+            app.url_path_for(
+                "get_commission", project_id=default_project.id, commission_id=uuid.uuid4()
+            ),
+            headers=headers,
+        )
+        assert response.status_code == 404
+
+    async def test_commission_from_other_project_not_found(
+        self, client: AsyncClient, default_commission: Commission, make_org_user, session: Session
+    ):
+        other_org = Organization(id=str(uuid.uuid4()), name="Other Org")
+        session.add(other_org)
+        session.commit()
+        other_project = Project(name="Other Project", is_public=False, organization_id=other_org.id)
+        session.add(other_project)
+        session.commit()
+
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        response = await client.get(
+            app.url_path_for(
+                "get_commission", project_id=other_project.id, commission_id=default_commission.id
+            ),
+            headers=headers,
+        )
+        # Even if they have the org admin role, the project_id URL mismatch means 403 or 404
+        assert response.status_code in (403, 404)
+
+
+class TestUpdateCommission:
+    async def test_updates_name(
+        self,
+        client: AsyncClient,
+        default_project: Project,
+        default_commission: Commission,
+        make_org_user,
+        session: Session,
+    ):
+        _, headers = make_org_user(role=OrgRole.RESPO_BENEVOLE)
+        response = await client.patch(
+            app.url_path_for(
+                "update_commission",
+                project_id=default_project.id,
+                commission_id=default_commission.id,
+            ),
+            headers=headers,
+            json={"name": "Securite Renamed"},
+        )
+        assert response.status_code == 200
+        commission = session.get(Commission, default_commission.id)
+        assert commission.name == "Securite Renamed"
+
+
+class TestDeleteCommission:
+    async def test_deletes_commission_and_members_links_but_not_users(
+        self,
+        client: AsyncClient,
+        default_project: Project,
+        default_commission: Commission,
+        two_users,
+        make_org_user,
+        session: Session,
+    ):
+        u1, _ = two_users
+        commission = session.get(Commission, default_commission.id)
+        commission.members.append(CommissionMember(user_id=u1.id))
+        session.commit()
+
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        response = await client.delete(
+            app.url_path_for(
+                "delete_commission",
+                project_id=default_project.id,
+                commission_id=default_commission.id,
+            ),
+            headers=headers,
+        )
+
+        assert response.status_code == 200
+        assert session.get(Commission, default_commission.id) is None
+        # User should still exist
+        assert session.get(User, u1.id) is not None
+        # Association row should be cascaded
+        assoc = (
+            session.execute(
+                select(CommissionMember).where(
+                    CommissionMember.commission_id == default_commission.id
+                )
+            )
+            .scalars()
+            .all()
+        )
+        assert len(assoc) == 0
+
+
+class TestCommissionMembership:
+    async def test_add_users_to_commission(
+        self,
+        client: AsyncClient,
+        default_project: Project,
+        default_commission: Commission,
+        two_users,
+        make_org_user,
+    ):
+        u1, u2 = two_users
+        _, headers = make_org_user(role=OrgRole.RESPO_BENEVOLE)
+        response = await client.post(
+            app.url_path_for(
+                "add_members_to_commission",
+                project_id=default_project.id,
+                commission_id=default_commission.id,
+            ),
+            headers=headers,
+            json={"user_ids": [str(u1.id), str(u2.id)]},
+        )
+        assert response.status_code == 200
+        data = response.json()
+        assert len(data["contacts"]) == 2
+        names = [contact["name"] for contact in data["contacts"]]
+        assert "Arthur Pendragon" in names
+        assert "Merlin Enchanteur" in names
+
+    async def test_add_is_idempotent(
+        self,
+        client: AsyncClient,
+        default_project: Project,
+        default_commission: Commission,
+        two_users,
+        make_org_user,
+    ):
+        u1, _ = two_users
+        _, headers = make_org_user(role=OrgRole.RESPO_BENEVOLE)
+
+        await client.post(
+            app.url_path_for(
+                "add_members_to_commission",
+                project_id=default_project.id,
+                commission_id=default_commission.id,
+            ),
+            headers=headers,
+            json={"user_ids": [str(u1.id)]},
+        )
+
+        response = await client.post(
+            app.url_path_for(
+                "add_members_to_commission",
+                project_id=default_project.id,
+                commission_id=default_commission.id,
+            ),
+            headers=headers,
+            json={"user_ids": [str(u1.id)]},
+        )
+        assert response.status_code == 200
+        assert len(response.json()["contacts"]) == 1
+
+    async def test_remove_member_from_commission(
+        self,
+        client: AsyncClient,
+        default_project: Project,
+        default_commission: Commission,
+        two_users,
+        make_org_user,
+        session: Session,
+    ):
+        u1, u2 = two_users
+        commission = session.get(Commission, default_commission.id)
+        commission.members.append(CommissionMember(user_id=u1.id))
+        commission.members.append(CommissionMember(user_id=u2.id))
+        session.commit()
+
+        _, headers = make_org_user(role=OrgRole.RESPO_BENEVOLE)
+        response = await client.delete(
+            app.url_path_for(
+                "remove_member_from_commission",
+                project_id=default_project.id,
+                commission_id=default_commission.id,
+                user_id=u1.id,
+            ),
+            headers=headers,
+        )
+
+        assert response.status_code == 200
+        data = response.json()
+        assert len(data["contacts"]) == 1
+        assert data["contacts"][0]["name"] == u2.name

+ 25 - 88
app/tests/test_project.py → app/tests/test_projects.py

@@ -6,19 +6,39 @@ from sqlalchemy import select
 from sqlalchemy.orm import Session
 from sqlalchemy.orm import Session
 
 
 from app.main import app
 from app.main import app
-from app.models import GlobalRole, Organization, OrgRole, Project, Slot, Volunteer
+from app.models import GlobalRole, OrgRole, Project, Slot, Volunteer
 from app.tests.conftest import default_organization_id, default_project_id, default_project_name
 from app.tests.conftest import default_organization_id, default_project_id, default_project_name
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
 ALL_ORG_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
 ALL_ORG_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
 
 
+PROJECT_ROUTES = [
+    ("GET", "list_project", {}, None),
+    (
+        "POST",
+        "create_project",
+        {},
+        {
+            "name": "Test",
+            "organization_id": default_organization_id,
+            "is_public": False,
+        },
+    ),
+    ("POST", "update_project", {"project_id": default_project_id}, {"title": "Updated"}),
+    ("POST", "create_sms_batch", {"project_id": default_project_id}, {"template": "Hello"}),
+    ("DELETE", "delete_project", {"project_id": default_project_id}, None),
+]
+
+
+class TestProjectCrossCutting(SharedProjectAccessTests):
+    @pytest.fixture(params=PROJECT_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project):
+        return request.param
 
 
-class TestListProject:
-    async def test_requires_auth(self, client: AsyncClient):
-        response = await client.get(app.url_path_for("list_project"))
-        assert response.status_code == 401
 
 
+class TestListProject:
     @pytest.mark.parametrize("role", ALL_ORG_ROLES)
     @pytest.mark.parametrize("role", ALL_ORG_ROLES)
     async def test_org_member_sees_their_project(
     async def test_org_member_sees_their_project(
         self, client: AsyncClient, default_project: Project, make_org_user, role
         self, client: AsyncClient, default_project: Project, make_org_user, role
@@ -53,11 +73,6 @@ class TestListProject:
 
 
 
 
 class TestGetProject:
 class TestGetProject:
-    async def test_requires_auth(self, client: AsyncClient, default_project: Project):
-        response = await client.get(app.url_path_for("get_project", project_id=default_project_id))
-        print(response.text)
-        assert response.status_code == 401
-
     @pytest.mark.parametrize(
     @pytest.mark.parametrize(
         "role, expected_status",
         "role, expected_status",
         [
         [
@@ -88,16 +103,6 @@ class TestGetProject:
         )
         )
         assert response.status_code == status
         assert response.status_code == status
 
 
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.delete(
-            app.url_path_for("get_project", project_id=default_project.id),
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestGetPublicProject:
 class TestGetPublicProject:
     async def test_missing_or_private_returns_404(
     async def test_missing_or_private_returns_404(
@@ -131,16 +136,6 @@ class TestListPublicProject:
 
 
 
 
 class TestCreateProject:
 class TestCreateProject:
-    async def test_requires_auth(
-        self, client: AsyncClient, default_organization: Organization, session: Session
-    ):
-        response = await client.post(
-            app.url_path_for("create_project"),
-            json={"name": "Coucou", "organization_id": default_organization_id},
-        )
-        assert response.status_code == 401
-        assert session.execute(select(Project)).scalars().first() is None
-
     @pytest.mark.parametrize(
     @pytest.mark.parametrize(
         "role, expected_status",
         "role, expected_status",
         [
         [
@@ -199,17 +194,6 @@ class TestCreateProject:
         )
         )
         assert response.status_code == 400
         assert response.status_code == 400
 
 
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("create_project"),
-            json={"name": "Coucou", "organization_id": default_organization_id},
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestUpdateProject:
 class TestUpdateProject:
     @pytest.mark.parametrize(
     @pytest.mark.parametrize(
@@ -242,27 +226,6 @@ class TestUpdateProject:
             project = session.get(Project, default_project_id)
             project = session.get(Project, default_project_id)
             assert project.name == "Coucou"
             assert project.name == "Coucou"
 
 
-    async def test_requires_auth(
-        self, client: AsyncClient, default_project: Project, session: Session
-    ):
-        response = await client.post(
-            app.url_path_for("update_project", project_id=default_project_id),
-            json={"name": "Coucou 2"},
-        )
-        assert response.status_code == 401
-        assert session.get(Project, default_project_id).name == default_project_name
-
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("update_project", project_id=default_project_id),
-            json={"name": "Coucou 2"},
-            headers=headers,
-        )
-        assert response.status_code == 403
-
     async def test_validation_error(
     async def test_validation_error(
         self, client: AsyncClient, default_public_project: Project, make_org_user
         self, client: AsyncClient, default_public_project: Project, make_org_user
     ):
     ):
@@ -324,22 +287,6 @@ class TestDeleteProject:
         else:
         else:
             assert project is not None
             assert project is not None
 
 
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.delete(
-            app.url_path_for("delete_project", project_id=default_project_id)
-        )
-        assert response.status_code == 401
-
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("delete_project", project_id=default_project_id),
-            headers=headers,
-        )
-        assert response.status_code == 403
-
     async def test_cascades_to_volunteers_and_slots(
     async def test_cascades_to_volunteers_and_slots(
         self, client: AsyncClient, default_public_project: Project, make_org_user, session: Session
         self, client: AsyncClient, default_public_project: Project, make_org_user, session: Session
     ):
     ):
@@ -360,13 +307,3 @@ class TestDeleteProject:
             .first()
             .first()
             is None
             is None
         )
         )
-
-    async def test_nonexistent_project_returns_404(self, client: AsyncClient, make_org_user):
-        """require_org_role looks the project up before the handler runs, so a
-        missing project now 404s -- this differs from the pre-refactor behavior
-        where DELETE on a nonexistent id silently returned 200."""
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        response = await client.delete(
-            app.url_path_for("delete_project", project_id=uuid.uuid4()), headers=headers
-        )
-        assert response.status_code == 404

+ 33 - 121
app/tests/test_slot.py → app/tests/test_slots.py

@@ -22,6 +22,7 @@ from app.tests.conftest import (
     default_template_id,
     default_template_id,
     default_volunteer_id,
     default_volunteer_id,
 )
 )
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
@@ -72,21 +73,40 @@ def other_commission_template(
     return template
     return template
 
 
 
 
-class TestListProjectSlots:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.get(
-            app.url_path_for("list_project_slots", project_id=default_project_id)
-        )
-        assert response.status_code == 401
+t1 = datetime(2000, 1, 1, tzinfo=UTC)
+SLOT_ROUTES = [
+    ("GET", "list_project_slots", {"project_id": "PROJECT"}, None),
+    (
+        "POST",
+        "create_slot",
+        {"project_id": "PROJECT"},
+        {
+            "title": "Test",
+            "starting_time": t1.isoformat(),
+            "ending_time": (t1 + timedelta(days=1)).isoformat(),
+        },
+    ),
+    (
+        "POST",
+        "update_slot",
+        {"project_id": "PROJECT", "slot_id": default_slot_id},
+        {"title": "Updated"},
+    ),
+    ("DELETE", "delete_slot", {"project_id": "PROJECT", "slot_id": default_slot_id}, None),
+]
+
+
+class TestSlotCrossCuttingAccess(SharedProjectAccessTests):
+    @pytest.fixture(params=SLOT_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project):
+        method, endpoint, kwargs, payload = request.param
+        resolved_kwargs = {
+            k: (default_project.id if v == "PROJECT" else v) for k, v in kwargs.items()
+        }
+        return method, endpoint, resolved_kwargs, payload
 
 
-    async def test_project_not_found(
-        self, client: AsyncClient, default_user_headers: dict, default_user_org_membership
-    ):
-        response = await client.get(
-            app.url_path_for("list_project_slots", project_id=uuid4()), headers=default_user_headers
-        )
-        assert response.status_code == 404
 
 
+class TestListProjectSlots:
     async def test_invalid_project_id(
     async def test_invalid_project_id(
         self, client: AsyncClient, default_user_headers: dict, default_user_org_membership
         self, client: AsyncClient, default_user_headers: dict, default_user_org_membership
     ):
     ):
@@ -147,26 +167,6 @@ class TestListProjectSlots:
 
 
 
 
 class TestCreateSlot:
 class TestCreateSlot:
-    async def test_requires_auth(self, client: AsyncClient):
-        response = await client.post(app.url_path_for("create_slot", project_id=default_project_id))
-        assert response.status_code == 401
-
-    async def test_project_not_found(
-        self, client: AsyncClient, default_user_headers: dict, default_user_org_membership
-    ):
-        starting_time = datetime(1900, 1, 1)
-        payload = {
-            "title": "être mort",
-            "starting_time": starting_time.isoformat(),
-            "ending_time": (starting_time + timedelta(minutes=60)).isoformat(),
-        }
-        response = await client.post(
-            app.url_path_for("create_slot", project_id=uuid4()),
-            json=payload,
-            headers=default_user_headers,
-        )
-        assert response.status_code == 404
-
     async def test_org_admin_creates(
     async def test_org_admin_creates(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,
@@ -297,50 +297,8 @@ class TestCreateSlot:
         )
         )
         assert response.status_code == 403
         assert response.status_code == 403
 
 
-    async def test_cannot_create_other_org_slot(
-        self,
-        client: AsyncClient,
-        default_public_project: Project,
-        session: Session,
-        commission_and_member,
-        other_org_user,
-    ):
-        """An admin of a project belonging to a DIFFERENT organization cannot access the project."""
-        _, headers = other_org_user
-        starting_time = datetime(1900, 1, 1)
-        response = await client.post(
-            app.url_path_for("create_slot", project_id=default_project_id),
-            json={
-                "title": "Scene shift",
-                "starting_time": starting_time.isoformat(),
-                "ending_time": (starting_time + timedelta(minutes=60)).isoformat(),
-            },
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestUpdateSlot:
 class TestUpdateSlot:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.post(
-            app.url_path_for("update_slot", project_id=default_project_id, slot_id=default_slot_id)
-        )
-        assert response.status_code == 401
-
-    async def test_project_not_found(
-        self,
-        client: AsyncClient,
-        default_public_project: Project,
-        default_user_headers: dict,
-        default_user_org_membership,
-    ):
-        response = await client.post(
-            app.url_path_for("update_slot", project_id=uuid4(), slot_id=default_slot_id),
-            json={"title": "x"},
-            headers=default_user_headers,
-        )
-        assert response.status_code == 404
-
     async def test_slot_not_found(
     async def test_slot_not_found(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,
@@ -577,54 +535,8 @@ class TestUpdateSlot:
         )
         )
         assert response.status_code == 403
         assert response.status_code == 403
 
 
-    async def test_cannot_update_other_org_slot(
-        self,
-        client: AsyncClient,
-        default_project: Project,
-        session: Session,
-        commission_and_member,
-        other_org_user,
-    ):
-        """An admin of a project belonging to a DIFFERENT organization cannot access the project."""
-        _, headers = other_org_user
-        slot = Slot(
-            project_id=default_project.id,
-            title="Bar shift",
-            starting_time=datetime.now(),
-            ending_time=datetime.now() + timedelta(hours=1),
-        )
-        session.add(slot)
-        session.commit()
-
-        response = await client.post(
-            app.url_path_for("update_slot", project_id=default_project_id, slot_id=slot.id),
-            json={"title": "Bar shift renamed"},
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestDeleteSlot:
 class TestDeleteSlot:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.delete(
-            app.url_path_for("delete_slot", project_id=default_project_id, slot_id=default_slot_id)
-        )
-        assert response.status_code == 401
-
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.delete(
-            app.url_path_for(
-                "delete_slot",
-                project_id=default_project_id,
-                slot_id=default_slot_id,
-            ),
-            headers=headers,
-        )
-        assert response.status_code == 403
-
     async def test_invalid_slot_id(
     async def test_invalid_slot_id(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,

+ 24 - 93
app/tests/test_sms.py

@@ -9,26 +9,38 @@ from sqlalchemy.orm import Session
 from app.main import app
 from app.main import app
 from app.models import OrgRole, Project, Sms, Volunteer
 from app.models import OrgRole, Project, Sms, Volunteer
 from app.tests.conftest import default_project_id, default_sms_id, default_volunteer_id
 from app.tests.conftest import default_project_id, default_sms_id, default_volunteer_id
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
 ALL_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
 ALL_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
 
 
+route_kwarg = {"project_id": default_project_id}
+route_kwargs = {**route_kwarg, "sms_id": default_sms_id}
+SMS_ROUTES = [
+    ("GET", "list_project_sms", route_kwarg, None),
+    (
+        "POST",
+        "create_sms",
+        route_kwarg,
+        {
+            "phone_number": "Test",
+            "content": "Coucou",
+        },
+    ),
+    ("POST", "update_sms", route_kwargs, {"title": "Updated"}),
+    ("DELETE", "delete_sms", route_kwargs, None),
+]
 
 
-class TestListProjectSms:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.get(
-            app.url_path_for("list_project_sms", project_id=default_project_id)
-        )
-        assert response.status_code == 401
 
 
-    async def test_project_not_found(self, client: AsyncClient, make_org_user):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        response = await client.get(
-            app.url_path_for("list_project_sms", project_id=uuid.uuid4()), headers=headers
-        )
-        assert response.status_code == 404
+class TestSMSCrossCutting(SharedProjectAccessTests):
+    @pytest.fixture(params=SMS_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project):
+
+        return request.param
+
 
 
+class TestListProjectSms:
     async def test_invalid_project_id_format(self, client: AsyncClient, make_org_user):
     async def test_invalid_project_id_format(self, client: AsyncClient, make_org_user):
         _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
         _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
         response = await client.get(
         response = await client.get(
@@ -49,29 +61,8 @@ class TestListProjectSms:
         assert len(data) == 1
         assert len(data) == 1
         assert data[0]["id"] == default_sms_id
         assert data[0]["id"] == default_sms_id
 
 
-    async def test_no_membership_forbidden(
-        self, client: AsyncClient, default_public_project: Project, make_org_user
-    ):
-        _, headers = make_org_user(role=None)
-        response = await client.get(
-            app.url_path_for("list_project_sms", project_id=default_project_id), headers=headers
-        )
-        assert response.status_code == 403
-
 
 
 class TestCreateSms:
 class TestCreateSms:
-    async def test_requires_auth(self, client: AsyncClient):
-        response = await client.post(app.url_path_for("create_sms", project_id=default_project_id))
-        assert response.status_code == 401
-
-    async def test_project_not_found(self, client: AsyncClient, make_org_user):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        payload = {"phone_number": "06 75 75 75 75 ", "content": "sms_content"}
-        response = await client.post(
-            app.url_path_for("create_sms", project_id=uuid.uuid4()), json=payload, headers=headers
-        )
-        assert response.status_code == 404
-
     @pytest.mark.parametrize("role", ALL_ROLES)
     @pytest.mark.parametrize("role", ALL_ROLES)
     async def test_all_roles_can_create(
     async def test_all_roles_can_create(
         self,
         self,
@@ -131,24 +122,8 @@ class TestCreateSms:
         )
         )
         assert response.status_code == 422
         assert response.status_code == 422
 
 
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("create_sms", project_id=default_project_id),
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestUpdateSms:
 class TestUpdateSms:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.post(
-            app.url_path_for("update_sms", project_id=default_project_id, sms_id=default_sms_id)
-        )
-        assert response.status_code == 401
-
     async def test_invalid_payload(
     async def test_invalid_payload(
         self, client: AsyncClient, default_public_project: Project, make_org_user
         self, client: AsyncClient, default_public_project: Project, make_org_user
     ):
     ):
@@ -160,23 +135,6 @@ class TestUpdateSms:
         )
         )
         assert response.status_code == 422
         assert response.status_code == 422
 
 
-    async def test_project_not_found(
-        self, client: AsyncClient, default_public_project: Project, make_org_user
-    ):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        payload = {
-            "volunteer_id": default_volunteer_id,
-            "phone_number": "06 75 75 75 75 ",
-            "content": "sms_content",
-            "sending_time": datetime(2024, 5, 17, tzinfo=UTC).isoformat(),
-        }
-        response = await client.post(
-            app.url_path_for("update_sms", project_id=uuid.uuid4(), sms_id=default_sms_id),
-            json=payload,
-            headers=headers,
-        )
-        assert response.status_code == 404
-
     async def test_sms_not_found(
     async def test_sms_not_found(
         self, client: AsyncClient, default_public_project: Project, make_org_user
         self, client: AsyncClient, default_public_project: Project, make_org_user
     ):
     ):
@@ -243,35 +201,8 @@ class TestUpdateSms:
         )
         )
         assert response.status_code == 403
         assert response.status_code == 403
 
 
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("update_sms", project_id=default_project_id, sms_id=default_sms_id),
-            json={"content": "hijacked"},
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestDeleteSms:
 class TestDeleteSms:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.delete(
-            app.url_path_for("delete_sms", project_id=default_project_id, sms_id=default_sms_id)
-        )
-        assert response.status_code == 401
-
-    async def test_forbidden_other_org(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.delete(
-            app.url_path_for("delete_sms", project_id=default_project_id, sms_id=default_sms_id),
-            headers=headers,
-        )
-        assert response.status_code == 403
-
     async def test_invalid_sms_id_format(
     async def test_invalid_sms_id_format(
         self, client: AsyncClient, default_public_project: Project, make_org_user
         self, client: AsyncClient, default_public_project: Project, make_org_user
     ):
     ):

+ 17 - 41
app/tests/test_tag.py → app/tests/test_tags.py

@@ -9,19 +9,30 @@ from sqlalchemy.orm import Session
 from app.main import app
 from app.main import app
 from app.models import OrgRole, Project, Slot, SlotTag, SlotTemplate
 from app.models import OrgRole, Project, Slot, SlotTag, SlotTemplate
 from app.tests.conftest import default_organization_id, default_project_id, default_tag_id
 from app.tests.conftest import default_organization_id, default_project_id, default_tag_id
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
 ALL_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
 ALL_ROLES = [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
 
 
+route_kwargs = {"project_id": default_project_id}
+route_kwargs_2 = {**route_kwargs, "tag_id": default_tag_id}
+TAG_ROUTES = [
+    ("GET", "list_project_tags", route_kwargs, None),
+    ("POST", "create_tag", route_kwargs, {"title": "Test"}),
+    ("GET", "list_tagged_slot", route_kwargs_2, None),
+    ("POST", "update_tag", route_kwargs_2, {"title": "Updated"}),
+    ("DELETE", "delete_tag", route_kwargs_2, None),
+]
 
 
-class TestListProjectTags:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.get(
-            app.url_path_for("list_project_tags", project_id=default_project_id)
-        )
-        assert response.status_code == 401
 
 
+class TestSlotCrossCuttingAccess(SharedProjectAccessTests):
+    @pytest.fixture(params=TAG_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project):
+        return request.param
+
+
+class TestListProjectTags:
     @pytest.mark.parametrize("role", ALL_ROLES)
     @pytest.mark.parametrize("role", ALL_ROLES)
     async def test_all_roles_can_read(
     async def test_all_roles_can_read(
         self,
         self,
@@ -57,21 +68,8 @@ class TestListProjectTags:
         )
         )
         assert response.status_code == 403
         assert response.status_code == 403
 
 
-    async def test_other_org_member_forbidden(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.get(
-            app.url_path_for("list_project_tags", project_id=default_project_id), headers=headers
-        )
-        assert response.status_code == 403
-
 
 
 class TestCreateTag:
 class TestCreateTag:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.post(app.url_path_for("create_tag", project_id=default_project_id))
-        assert response.status_code == 401
-
     async def test_invalid_project_id_format(
     async def test_invalid_project_id_format(
         self, client: AsyncClient, default_public_project: Project, make_org_user
         self, client: AsyncClient, default_public_project: Project, make_org_user
     ):
     ):
@@ -81,17 +79,6 @@ class TestCreateTag:
         )
         )
         assert response.status_code == 422
         assert response.status_code == 422
 
 
-    async def test_project_not_found(
-        self, client: AsyncClient, default_public_project: Project, make_org_user
-    ):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        response = await client.post(
-            app.url_path_for("create_tag", project_id=uuid.uuid4()),
-            json={"title": "1st tag"},
-            headers=headers,
-        )
-        assert response.status_code == 404
-
     async def test_invalid_payload(
     async def test_invalid_payload(
         self, client: AsyncClient, default_public_project: Project, make_org_user
         self, client: AsyncClient, default_public_project: Project, make_org_user
     ):
     ):
@@ -189,17 +176,6 @@ class TestCreateTag:
         )
         )
         assert response.status_code == 400
         assert response.status_code == 400
 
 
-    async def test_other_org_member_forbidden(
-        self, client: AsyncClient, default_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("create_tag", project_id=default_project_id),
-            json={"title": "hijacked"},
-            headers=headers,
-        )
-        assert response.status_code == 403
-
 
 
 class TestUpdateTag:
 class TestUpdateTag:
     async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
     async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):

+ 22 - 95
app/tests/test_template.py → app/tests/test_templates.py

@@ -6,7 +6,13 @@ from sqlalchemy.orm import Session
 
 
 from app.main import app
 from app.main import app
 from app.models import Commission, CommissionMember, OrgRole, Project, Slot, SlotTemplate
 from app.models import Commission, CommissionMember, OrgRole, Project, Slot, SlotTemplate
-from app.tests.conftest import default_slot_id, default_tag_id, default_template_id
+from app.tests.conftest import (
+    default_project_id,
+    default_slot_id,
+    default_tag_id,
+    default_template_id,
+)
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
@@ -35,25 +41,23 @@ def other_commission(session: Session, default_public_project: Project):
     return commission
     return commission
 
 
 
 
-class TestCreateTemplate:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.post(
-            app.url_path_for("create_template", project_id=default_public_project.id),
-            json={"title": "1st template"},
-        )
-        assert response.status_code == 401
+route_kwargs = {"project_id": default_project_id}
+route_kwargs_2 = {**route_kwargs, "template_id": default_template_id}
+TEMPLATE_ROUTES = [
+    ("GET", "list_project_templates", route_kwargs, None),
+    ("POST", "create_template", route_kwargs, {"title": "Test"}),
+    ("POST", "update_template", route_kwargs_2, {"title": "Updated"}),
+    ("DELETE", "delete_template", route_kwargs_2, None),
+]
 
 
-    async def test_other_org_member_forbidden(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for("create_template", project_id=default_public_project.id),
-            json={"title": "1st template"},
-            headers=headers,
-        )
-        assert response.status_code == 403
 
 
+class TestSlotCrossCuttingAccess(SharedProjectAccessTests):
+    @pytest.fixture(params=TEMPLATE_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project):
+        return request.param
+
+
+class TestCreateTemplate:
     async def test_validation_error(
     async def test_validation_error(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,
@@ -68,19 +72,6 @@ class TestCreateTemplate:
         )
         )
         assert response.status_code == 422
         assert response.status_code == 422
 
 
-    async def test_project_not_found(
-        self,
-        client: AsyncClient,
-        default_user_headers: dict,
-        default_user_org_membership,
-    ):
-        response = await client.post(
-            app.url_path_for("create_template", project_id=uuid4()),
-            json={"title": "1st template"},
-            headers=default_user_headers,
-        )
-        assert response.status_code == 404
-
     @pytest.mark.parametrize(
     @pytest.mark.parametrize(
         "payload",
         "payload",
         [
         [
@@ -173,32 +164,6 @@ class TestCreateTemplate:
 
 
 
 
 class TestUpdateTemplate:
 class TestUpdateTemplate:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.post(
-            app.url_path_for(
-                "update_template",
-                project_id=default_public_project.id,
-                template_id=default_template_id,
-            ),
-            json={"title": "x"},
-        )
-        assert response.status_code == 401
-
-    async def test_other_org_member_forbidden(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        _, headers = other_org_user
-        response = await client.post(
-            app.url_path_for(
-                "update_template",
-                project_id=default_public_project.id,
-                template_id=default_template_id,
-            ),
-            json={"title": "x"},
-            headers=headers,
-        )
-        assert response.status_code == 403
-
     async def test_invalid_template_id_format(
     async def test_invalid_template_id_format(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,
@@ -231,21 +196,6 @@ class TestUpdateTemplate:
         )
         )
         assert response.status_code == 404
         assert response.status_code == 404
 
 
-    async def test_project_not_found(
-        self,
-        client: AsyncClient,
-        default_user_headers: dict,
-        default_user_org_membership,
-    ):
-        response = await client.post(
-            app.url_path_for(
-                "update_template", project_id=uuid4(), template_id=default_template_id
-            ),
-            json={"title": "1st template"},
-            headers=default_user_headers,
-        )
-        assert response.status_code == 404
-
     @pytest.mark.parametrize(
     @pytest.mark.parametrize(
         "code,payload",
         "code,payload",
         [
         [
@@ -413,29 +363,6 @@ class TestUpdateTemplate:
 
 
 
 
 class TestDeleteTemplate:
 class TestDeleteTemplate:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.delete(
-            app.url_path_for(
-                "delete_template",
-                project_id=default_public_project.id,
-                template_id=default_template_id,
-            )
-        )
-        assert response.status_code == 401
-
-    async def test_other_org_member_forbidden(
-        self, client: AsyncClient, default_public_project: Project, other_org_user
-    ):
-        response = await client.delete(
-            app.url_path_for(
-                "delete_template",
-                project_id=default_public_project.id,
-                template_id=default_template_id,
-            ),
-            headers=other_org_user[1],
-        )
-        assert response.status_code == 403
-
     async def test_invalid_ids(
     async def test_invalid_ids(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,

+ 36 - 0
app/tests/test_volunteer_groups.py

@@ -9,6 +9,8 @@ from sqlalchemy.orm import Session
 from app.core.session import session as session_maker
 from app.core.session import session as session_maker
 from app.main import app
 from app.main import app
 from app.models import Organization, OrgRole, Project, Slot, Volunteer, VolunteerGroup
 from app.models import Organization, OrgRole, Project, Slot, Volunteer, VolunteerGroup
+from app.tests.conftest import default_project_id, default_slot_id
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
@@ -52,6 +54,40 @@ def default_group(default_project: Project) -> VolunteerGroup:
         return group
         return group
 
 
 
 
+route_kwargs = {"project_id": default_project_id}
+route_kwargs_2 = {**route_kwargs, "group_id": "GROUP"}
+VOLUNTEER_GROUP_ROUTES = [
+    ("GET", "list_project_groups", route_kwargs, None),
+    ("POST", "create_group", route_kwargs, {"name": "Test"}),
+    ("GET", "get_group", route_kwargs_2, None),
+    ("PATCH", "update_group", route_kwargs_2, {"name": "Updated"}),
+    ("DELETE", "delete_group", route_kwargs_2, None),
+    ("POST", "add_volunteers_to_group", route_kwargs_2, {"volunteer_ids": []}),
+    ("DELETE", "remove_volunteer_from_group", {**route_kwargs_2, "volunteer_id": "VOL"}, None),
+    ("POST", "add_group_to_slot", {**route_kwargs_2, "slot_id": "SLOT"}, None),
+    ("POST", "send_sms_to_group", route_kwargs_2, {"content": "coucou"}),
+]
+
+
+class TestVolunteerGroupCrossCuttingAccess(SharedProjectAccessTests):
+    @pytest.fixture(params=VOLUNTEER_GROUP_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_group, two_volunteers):
+        method, endpoint, kwargs, payload = request.param
+        resolved_kwargs = {
+            k: (
+                default_group.id
+                if v == "GROUP"
+                else two_volunteers[0].id
+                if v == "VOL"
+                else default_slot_id
+                if v == "SLOT"
+                else v
+            )
+            for k, v in kwargs.items()
+        }
+        return method, endpoint, resolved_kwargs, payload
+
+
 class TestListGroups:
 class TestListGroups:
     @pytest.mark.parametrize("role", MANAGE_ROLES)
     @pytest.mark.parametrize("role", MANAGE_ROLES)
     async def test_role_access(
     async def test_role_access(

+ 16 - 81
app/tests/test_volunteer.py → app/tests/test_volunteers.py

@@ -8,38 +8,30 @@ from sqlalchemy.orm import Session
 from app.main import app
 from app.main import app
 from app.models import OrgRole, Project, Slot, Sms, Volunteer
 from app.models import OrgRole, Project, Slot, Sms, Volunteer
 from app.tests.conftest import default_project_id, default_slot_id, default_volunteer_id
 from app.tests.conftest import default_project_id, default_slot_id, default_volunteer_id
+from app.tests.shared_access import SharedProjectAccessTests
 
 
 pytestmark = pytest.mark.asyncio
 pytestmark = pytest.mark.asyncio
 
 
 WRITE_FORBIDDEN_ROLES = [OrgRole.RESPO_COMMISSION, None]
 WRITE_FORBIDDEN_ROLES = [OrgRole.RESPO_COMMISSION, None]
 
 
 
 
-class TestListVolunteer:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.get(
-            app.url_path_for("list_project_volunteers", project_id=default_project_id),
-        )
-        assert response.status_code == 401
+route_kwargs = {"project_id": default_project_id}
+route_kwargs_2 = {**route_kwargs, "volunteer_id": default_volunteer_id}
+VOLUNTEER_ROUTES = [
+    ("GET", "list_project_volunteers", route_kwargs, None),
+    ("POST", "create_volunteer", route_kwargs, {"name": "Test", "email": "a@free.fr"}),
+    ("POST", "update_volunteer", route_kwargs_2, {"name": "Updated"}),
+    ("DELETE", "delete_volunteer", route_kwargs_2, None),
+]
 
 
-    @pytest.mark.parametrize(
-        "project_id,status_code",
-        [(uuid.uuid4(), 404), ("pas un uuid valid", 422)],
-    )
-    async def test_read_list_fails(
-        self,
-        client: AsyncClient,
-        default_public_project: Project,
-        make_org_user,
-        project_id,
-        status_code,
-    ):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        response = await client.get(
-            app.url_path_for("list_project_volunteers", project_id=project_id),
-            headers=headers,
-        )
-        assert response.status_code == status_code
 
 
+class TestVolunteerCrossCuttingAccess(SharedProjectAccessTests):
+    @pytest.fixture(params=VOLUNTEER_ROUTES, ids=lambda x: f"{x[0]}-{x[1]}")
+    def resolved_route(self, request, default_project):
+        return request.param
+
+
+class TestListVolunteer:
     @pytest.mark.parametrize(
     @pytest.mark.parametrize(
         "role", [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
         "role", [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
     )
     )
@@ -79,30 +71,6 @@ class TestListVolunteer:
 
 
 
 
 class TestCreateVolunteer:
 class TestCreateVolunteer:
-    async def test_requires_auth(self, client: AsyncClient):
-        response = await client.post(
-            app.url_path_for("create_volunteer", project_id=default_project_id)
-        )
-        assert response.status_code == 401
-
-    async def test_invalid_project_id(
-        self,
-        client: AsyncClient,
-        make_org_user,
-    ):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        payload = {
-            "name": "Lancelot",
-            "email": "lancelot@dulac.fr",
-            "phone_number": "03 14 15 92 65",
-        }
-        response = await client.post(
-            app.url_path_for("create_volunteer", project_id=uuid.uuid4()),
-            json=payload,
-            headers=headers,
-        )
-        assert response.status_code == 404
-
     async def test_invalid_payload(
     async def test_invalid_payload(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,
@@ -225,31 +193,6 @@ class TestCreateVolunteer:
 
 
 
 
 class TestUpdateVolunteer:
 class TestUpdateVolunteer:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.post(
-            app.url_path_for(
-                "update_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
-            )
-        )
-        assert response.status_code == 401
-
-    async def test_invalid_project_id(
-        self,
-        client: AsyncClient,
-        default_public_project: Project,
-        make_org_user,
-    ):
-        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
-        payload = {"name": "Lancelot", "email": "l@dulac.fr", "phone_number": "0314159265"}
-        response = await client.post(
-            app.url_path_for(
-                "update_volunteer", project_id=uuid.uuid4(), volunteer_id=default_volunteer_id
-            ),
-            json=payload,
-            headers=headers,
-        )
-        assert response.status_code == 404
-
     async def test_invalid_volunteer_id(
     async def test_invalid_volunteer_id(
         self,
         self,
         client: AsyncClient,
         client: AsyncClient,
@@ -345,14 +288,6 @@ class TestUpdateVolunteer:
 
 
 
 
 class TestDeleteVolunteer:
 class TestDeleteVolunteer:
-    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
-        response = await client.delete(
-            app.url_path_for(
-                "delete_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
-            )
-        )
-        assert response.status_code == 401
-
     @pytest.mark.parametrize("role", WRITE_FORBIDDEN_ROLES)
     @pytest.mark.parametrize("role", WRITE_FORBIDDEN_ROLES)
     async def test_read_only_roles_forbidden(
     async def test_read_only_roles_forbidden(
         self,
         self,

+ 1 - 1
pyproject.toml

@@ -2,7 +2,7 @@
 authors = ["clovis jaquin <clovis@jaquin.fr>"]
 authors = ["clovis jaquin <clovis@jaquin.fr>"]
 description = "FastAPI project that can parse gsheet planning for brass dans la garonne event and manage creating automatic SMS notification for volunteer"
 description = "FastAPI project that can parse gsheet planning for brass dans la garonne event and manage creating automatic SMS notification for volunteer"
 name = "bdlg-2023"
 name = "bdlg-2023"
-version = "0.2.3"
+version = "1.0.1"
 
 
 [tool.poetry.dependencies]
 [tool.poetry.dependencies]
 fastapi = "0.116.*"
 fastapi = "0.116.*"