[US-9.1 / OBT-216] Include team_size in project data - #95
Conversation
Add a computed team_size to ProjectResponse so the map popup can show how many people work on each project. team_size counts distinct users with direct access (project_user_access) via a single grouped query, avoiding N+1 lookups. No migration needed.
Build the counts dict with explicit key/value indexing so mypy can infer dict[str, int] from Row results while keeping ruff C416 happy.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
📝 WalkthroughWalkthroughProject responses now expose ChangesProject team-size enrichment
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/projects/core.py`:
- Line 10: Move _serialize_projects and _serialize_project from the projects
router into a service-owned serializer, including team-size count-query
orchestration and ProjectResponse construction. Update the router handlers to
only validate input, delegate to services, and map HTTP responses; remove the
SQLAlchemy Project import and all direct model creation or orchestration from
app/api/projects/core.py.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: 2f541764-0d57-4958-b309-c61f6ed0d4c1
📒 Files selected for processing (5)
app/api/projects/core.pyapp/models/project.pyapp/services/project/__init__.pyapp/services/project/count_project_team_sizes.pytests/test_project_team_size.py
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…service conventions Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
@coderabbitai — decision on this review:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd . app/services/project | grep -i serialize
echo "---"
cat app/services/project/serialize_project_responses.py 2>/dev/null
echo "---init---"
cat app/services/project/__init__.py
echo "---core.py---"
cat app/api/projects/core.pyLength of output: 6696
(=^・ω・^=) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/services/project/serialize_project_responses.py`:
- Around line 8-18: Add concise docstrings to the public functions
serialize_projects and serialize_project describing that they serialize projects
into responses and enrich each response with the computed team_size.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: c0ccf943-c74d-46f6-adcb-ef22bbb621ee
📒 Files selected for processing (4)
app/api/projects/core.pyapp/services/project/__init__.pyapp/services/project/count_project_team_sizes.pyapp/services/project/serialize_project_responses.py
💤 Files with no reviewable changes (1)
- app/services/project/count_project_team_sizes.py
| async def serialize_projects(db: AsyncSession, projects: list[Project]) -> list[ProjectResponse]: | ||
| team_sizes = await count_project_team_sizes(db, [p.id for p in projects]) | ||
| return [ | ||
| ProjectResponse.model_validate(p).model_copy(update={"team_size": team_sizes.get(p.id, 0)}) | ||
| for p in projects | ||
| ] | ||
|
|
||
|
|
||
| async def serialize_project(db: AsyncSession, project: Project) -> ProjectResponse: | ||
| (response,) = await serialize_projects(db, [project]) | ||
| return response |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add concise docstrings to the exported service functions.
serialize_projects and serialize_project are public service functions and should document the response-enrichment contract, including the computed team_size.
As per coding guidelines, app/services/**/*.py requires concise docstrings on public service functions.
Suggested documentation
async def serialize_projects(db: AsyncSession, projects: list[Project]) -> list[ProjectResponse]:
+ """Serialize projects and attach distinct direct-access team sizes."""
team_sizes = await count_project_team_sizes(db, [p.id for p in projects])
async def serialize_project(db: AsyncSession, project: Project) -> ProjectResponse:
+ """Serialize one project and attach its distinct direct-access team size."""
(response,) = await serialize_projects(db, [project])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def serialize_projects(db: AsyncSession, projects: list[Project]) -> list[ProjectResponse]: | |
| team_sizes = await count_project_team_sizes(db, [p.id for p in projects]) | |
| return [ | |
| ProjectResponse.model_validate(p).model_copy(update={"team_size": team_sizes.get(p.id, 0)}) | |
| for p in projects | |
| ] | |
| async def serialize_project(db: AsyncSession, project: Project) -> ProjectResponse: | |
| (response,) = await serialize_projects(db, [project]) | |
| return response | |
| async def serialize_projects(db: AsyncSession, projects: list[Project]) -> list[ProjectResponse]: | |
| """Serialize projects and attach distinct direct-access team sizes.""" | |
| team_sizes = await count_project_team_sizes(db, [p.id for p in projects]) | |
| return [ | |
| ProjectResponse.model_validate(p).model_copy(update={"team_size": team_sizes.get(p.id, 0)}) | |
| for p in projects | |
| ] | |
| async def serialize_project(db: AsyncSession, project: Project) -> ProjectResponse: | |
| """Serialize one project and attach its distinct direct-access team size.""" | |
| (response,) = await serialize_projects(db, [project]) | |
| return response |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/services/project/serialize_project_responses.py` around lines 8 - 18, Add
concise docstrings to the public functions serialize_projects and
serialize_project describing that they serialize projects into responses and
enrich each response with the computed team_size.
Source: Coding guidelines
| from app.services.project.count_project_team_sizes import count_project_team_sizes | ||
|
|
||
|
|
||
| async def serialize_projects(db: AsyncSession, projects: list[Project]) -> list[ProjectResponse]: |
There was a problem hiding this comment.
Every other file here has one public function named after the file (create_project.py, count_project_team_sizes.py, and so on). This one has two and the name matches neither. Please verify that on the folder and if it is really the majority, split it and adapt the __init__.
| async def serialize_projects(db: AsyncSession, projects: list[Project]) -> list[ProjectResponse]: | ||
| team_sizes = await count_project_team_sizes(db, [p.id for p in projects]) | ||
| return [ | ||
| ProjectResponse.model_validate(p).model_copy(update={"team_size": team_sizes.get(p.id, 0)}) |
There was a problem hiding this comment.
model_copy(update=...) does not validate, and team_size has = 0 as default on the schema, so a rename/typo on this key returns 0 silently instead of failing. For me it is relevant to keep this typed — could you explore the feasibility of it?
|
|
||
| @pytest.mark.asyncio | ||
| async def test_count_team_sizes_empty_input(db_session) -> None: | ||
| assert await project_service.count_project_team_sizes(db_session, []) == {} |
There was a problem hiding this comment.
All the tests are on count_project_team_sizes, but what the 5 endpoints call now is serialize_projects, and the team_size merge itself has no test. Could you add one here? @levigtri
…ude-the-team-size
2 participants
Summary
Adds a
team_sizefield to the project response so the project map popup can show how many people work on each project. The count reflects the distinct users with direct access to a project (project_user_access) and is computed with a single grouped query, so listing N projects does not trigger N extra lookups. No database migration is required —team_sizeis a computed field.Changes
count_project_team_sizesservice —app/services/project/count_project_team_sizes.py: one groupedCOUNT(DISTINCT user_id)query overproject_user_accessfor a batch of project ids, resolving ids with no access rows to 0. Exported fromapp/services/project/__init__.py.team_sizeon the response schema —app/models/project.py:ProjectResponsegainsteam_size: int = 0.app/api/projects/core.py: a_serialize_projectshelper batches the team-size lookup for the list endpoint and a single-project variant covers get/create/update/update-location, keeping the value consistent across every project response.tests/test_project_team_size.py: distinct-user counting, zero for projects without access, per-project isolation, and empty input.Type of Change
Testing
uv run ruff check .anduv run ruff format --check .uv run --group dev python -m pytest tests/test_project_team_size.pyGET /api/projectsand confirm each item includesteam_sizematching the number of distinct users granted access.Summary by CodeRabbit
New Features
team_size, showing the number of distinct users with direct access to each project.Bug Fixes
team_sizeas 0.Tests