[US-11.6 / OBT-231] Scope console lists to the caller's managed projects - #90
Open
Conversation
Non-admin users receive only the data tied to the projects they manage (ProjectUserAccess.role == "manager"), instead of the full dataset: organizations linked to those projects, the languages of those projects, and the phases attached to them. Platform admins are unaffected and continue to see everything. Adds get_managed_project_ids and per-domain "by projects" services (list_organizations_by_projects, list_languages_by_projects, list_phases_by_projects and list_phases_with_deps_by_projects), replacing the earlier organization-based scoping to align with the 2026-07 per-project manager model. Wires them into the organizations, languages and phases list endpoints.
levigtri
force-pushed
the
levigft/obt-231-us-116-scope-the-organizations-list-for-non-admin-users
branch
from
July 4, 2026 20:34
454360f to
0f9fc13
Compare
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesAdds active-state and creator metadata to languages, an admin-only deactivation flow, project-scoped organization/language/phase listings, role-aware API routing, and validation preventing projects from using inactive languages. Language lifecycle and persistence
Managed-project listing services
Role-aware API endpoints
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant get_managed_project_ids
participant ListingService
participant AsyncSession
User->>API: Request organization, language, or phase listing
API->>get_managed_project_ids: Resolve managed projects for non-admin
get_managed_project_ids->>AsyncSession: Query manager access
AsyncSession-->>get_managed_project_ids: Managed project IDs
API->>ListingService: Query scoped records
ListingService->>AsyncSession: Execute project-scoped query
AsyncSession-->>ListingService: Matching records
ListingService-->>API: Return records
API-->>User: Return serialized response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…-a-language-instead-of-permanently-deleting' into fix/obt-231-90 # Conflicts: # app/api/languages.py
list_languages_by_projects is the manager's list path and returned languages regardless of is_active, so a manager kept seeing a language after a platform admin deactivated it. Filter on is_active, and honour include_inactive only on the admin branch of the list route. This is why the branch now sits on top of US-2.1 (#97): is_active is introduced there, so the filter cannot be expressed on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
levigtri
changed the base branch from
main
to
levigft/obt-200-us-21-deactivate-a-language-instead-of-permanently-deleting
July 16, 2026 20:43
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/languages.py`:
- Line 7: Move the platform-admin branching and managed-project lookup from the
language router into a language service operation that performs role-aware
language listing. Update the router to only parse validated request data, inject
dependencies, call that service operation, and map its result into the response;
remove its direct use of get_managed_project_ids and related business
orchestration.
In `@app/core/org_scope.py`:
- Around line 12-19: Move get_managed_project_ids from app/core/org_scope.py
into the appropriate app/services module, preserving its database query
behavior. Update app/api/organizations.py at lines 7 and 32-33 and
app/api/phases.py at lines 6, 40-44, and 54-55 to import and invoke the
service-layer helper; remove the core-layer definition and direct callers.
In `@app/services/language/deactivate_language.py`:
- Around line 9-17: Add concise docstrings to the public functions
deactivate_language, create_language, list_languages, and
list_languages_by_projects. In app/services/language/deactivate_language.py
lines 9-17, document platform-admin-only deactivation; in
app/services/language/create_language.py lines 8-14, document creation and
creator attribution; in app/services/language/list_languages.py lines 7-10,
document default inactive filtering and include_inactive; and in
app/services/language/list_languages_by_projects.py lines 8-18, document
managed-project and active-language filtering.
In `@app/services/org/list_organizations_by_projects.py`:
- Around line 8-10: Add concise docstrings to the public functions
list_organizations_by_projects in
app/services/org/list_organizations_by_projects.py (lines 8-10),
list_phases_by_projects in app/services/phase/list_phases_by_projects.py (lines
8-12) describing project and optional single-project filtering, and the
project-scoped phase/dependency function at lines 30-33 describing its results.
In `@app/services/project/create_project.py`:
- Around line 19-23: Protect language validation and the subsequent project
write in both create_project.py (lines 19-33) and update_project.py (lines
19-30) with the same transaction/row lock, or enforce the active-language
invariant atomically at write time. Ensure create_project cannot insert and
update_project cannot assign a language_id after another transaction deactivates
the language.
🪄 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: 3a04dc45-3d81-4171-bb55-4d99459d6cc7
📒 Files selected for processing (23)
alembic/versions/20260704_0003_add_language_is_active.pyalembic/versions/20260704_0004_add_language_created_by.pyapp/api/languages.pyapp/api/organizations.pyapp/api/phases.pyapp/core/org_scope.pyapp/db/models/language.pyapp/models/language.pyapp/services/language/__init__.pyapp/services/language/create_language.pyapp/services/language/deactivate_language.pyapp/services/language/list_languages.pyapp/services/language/list_languages_by_projects.pyapp/services/org/__init__.pyapp/services/org/list_organizations_by_projects.pyapp/services/phase/__init__.pyapp/services/phase/list_phases_by_projects.pyapp/services/project/create_project.pyapp/services/project/update_project.pytests/baker.pytests/test_console_scoping.pytests/test_language_service.pytests/test_project_service.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🤖 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/languages.py`:
- Line 7: Move the platform-admin branching and managed-project lookup from the
language router into a language service operation that performs role-aware
language listing. Update the router to only parse validated request data, inject
dependencies, call that service operation, and map its result into the response;
remove its direct use of get_managed_project_ids and related business
orchestration.
In `@app/core/org_scope.py`:
- Around line 12-19: Move get_managed_project_ids from app/core/org_scope.py
into the appropriate app/services module, preserving its database query
behavior. Update app/api/organizations.py at lines 7 and 32-33 and
app/api/phases.py at lines 6, 40-44, and 54-55 to import and invoke the
service-layer helper; remove the core-layer definition and direct callers.
In `@app/services/language/deactivate_language.py`:
- Around line 9-17: Add concise docstrings to the public functions
deactivate_language, create_language, list_languages, and
list_languages_by_projects. In app/services/language/deactivate_language.py
lines 9-17, document platform-admin-only deactivation; in
app/services/language/create_language.py lines 8-14, document creation and
creator attribution; in app/services/language/list_languages.py lines 7-10,
document default inactive filtering and include_inactive; and in
app/services/language/list_languages_by_projects.py lines 8-18, document
managed-project and active-language filtering.
In `@app/services/org/list_organizations_by_projects.py`:
- Around line 8-10: Add concise docstrings to the public functions
list_organizations_by_projects in
app/services/org/list_organizations_by_projects.py (lines 8-10),
list_phases_by_projects in app/services/phase/list_phases_by_projects.py (lines
8-12) describing project and optional single-project filtering, and the
project-scoped phase/dependency function at lines 30-33 describing its results.
In `@app/services/project/create_project.py`:
- Around line 19-23: Protect language validation and the subsequent project
write in both create_project.py (lines 19-33) and update_project.py (lines
19-30) with the same transaction/row lock, or enforce the active-language
invariant atomically at write time. Ensure create_project cannot insert and
update_project cannot assign a language_id after another transaction deactivates
the language.
🪄 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: 3a04dc45-3d81-4171-bb55-4d99459d6cc7
📒 Files selected for processing (23)
alembic/versions/20260704_0003_add_language_is_active.pyalembic/versions/20260704_0004_add_language_created_by.pyapp/api/languages.pyapp/api/organizations.pyapp/api/phases.pyapp/core/org_scope.pyapp/db/models/language.pyapp/models/language.pyapp/services/language/__init__.pyapp/services/language/create_language.pyapp/services/language/deactivate_language.pyapp/services/language/list_languages.pyapp/services/language/list_languages_by_projects.pyapp/services/org/__init__.pyapp/services/org/list_organizations_by_projects.pyapp/services/phase/__init__.pyapp/services/phase/list_phases_by_projects.pyapp/services/project/create_project.pyapp/services/project/update_project.pytests/baker.pytests/test_console_scoping.pytests/test_language_service.pytests/test_project_service.py
🛑 Comments failed to post (5)
app/api/languages.py (1)
7-7: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move listing-scope orchestration into a language service.
This router now branches on platform-admin status and calls the database-backed
get_managed_project_ids; expose one service operation for role-aware language listing instead, leaving the router to dependency injection, request parsing, and response mapping.As per coding guidelines,
app/apimust “parse and validate input, call service functions,” and must not contain “business rules, orchestration, model creation, or direct database access in routers.”Also applies to: 21-25
🤖 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/api/languages.py` at line 7, Move the platform-admin branching and managed-project lookup from the language router into a language service operation that performs role-aware language listing. Update the router to only parse validated request data, inject dependencies, call that service operation, and map its result into the response; remove its direct use of get_managed_project_ids and related business orchestration.Source: Coding guidelines
app/core/org_scope.py (1)
12-19: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep project-scope lookup in the service layer.
The new database-backed authorization helper is defined in
app/coreand consumed directly by API routers. Move it intoapp/servicesand update each caller.
app/core/org_scope.py#L12-L19: relocateget_managed_project_ids.app/api/organizations.py#L7-L7: import the helper from its service module.app/api/organizations.py#L32-L33: call the service-layer helper.app/api/phases.py#L6-L6: import the helper from its service module.app/api/phases.py#L40-L44: call the service-layer helper for phase listing.app/api/phases.py#L54-L55: call the service-layer helper for dependency listing.As per coding guidelines,
app/coreis reserved for core infrastructure,app/apiis an HTTP access layer, and all database access must live inapp/services/.📍 Affects 3 files
app/core/org_scope.py#L12-L19(this comment)app/api/organizations.py#L7-L7app/api/organizations.py#L32-L33app/api/phases.py#L6-L6app/api/phases.py#L40-L44app/api/phases.py#L54-L55🤖 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/core/org_scope.py` around lines 12 - 19, Move get_managed_project_ids from app/core/org_scope.py into the appropriate app/services module, preserving its database query behavior. Update app/api/organizations.py at lines 7 and 32-33 and app/api/phases.py at lines 6, 40-44, and 54-55 to import and invoke the service-layer helper; remove the core-layer definition and direct callers.Source: Coding guidelines
app/services/language/deactivate_language.py (1)
9-17: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add concise docstrings to the public language services.
app/services/language/deactivate_language.py#L9-L17: document the platform-admin-only deactivation behavior.app/services/language/create_language.py#L8-L14: document creation and creator attribution semantics.app/services/language/list_languages.py#L7-L10: document default inactive filtering andinclude_inactive.app/services/language/list_languages_by_projects.py#L8-L18: document managed-project and active-language filtering.As per coding guidelines,
app/services/**/*.pyrequires “concise docstrings on public service functions.”📍 Affects 4 files
app/services/language/deactivate_language.py#L9-L17(this comment)app/services/language/create_language.py#L8-L14app/services/language/list_languages.py#L7-L10app/services/language/list_languages_by_projects.py#L8-L18🤖 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/language/deactivate_language.py` around lines 9 - 17, Add concise docstrings to the public functions deactivate_language, create_language, list_languages, and list_languages_by_projects. In app/services/language/deactivate_language.py lines 9-17, document platform-admin-only deactivation; in app/services/language/create_language.py lines 8-14, document creation and creator attribution; in app/services/language/list_languages.py lines 7-10, document default inactive filtering and include_inactive; and in app/services/language/list_languages_by_projects.py lines 8-18, document managed-project and active-language filtering.Source: Coding guidelines
app/services/org/list_organizations_by_projects.py (1)
8-10: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add concise docstrings to the new public service functions.
app/services/org/list_organizations_by_projects.py#L8-L10: document the project-scoped organization query.app/services/phase/list_phases_by_projects.py#L8-L12: document project and optional single-project filtering.app/services/phase/list_phases_by_projects.py#L30-L33: document project-scoped phase and dependency results.As per coding guidelines, public functions in
app/services/**/*.pyrequire concise docstrings.📍 Affects 2 files
app/services/org/list_organizations_by_projects.py#L8-L10(this comment)app/services/phase/list_phases_by_projects.py#L8-L12app/services/phase/list_phases_by_projects.py#L30-L33🤖 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/org/list_organizations_by_projects.py` around lines 8 - 10, Add concise docstrings to the public functions list_organizations_by_projects in app/services/org/list_organizations_by_projects.py (lines 8-10), list_phases_by_projects in app/services/phase/list_phases_by_projects.py (lines 8-12) describing project and optional single-project filtering, and the project-scoped phase/dependency function at lines 30-33 describing its results.Source: Coding guidelines
app/services/project/create_project.py (1)
19-23: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Files ==\n' git ls-files 'app/services/project/*' 'app/services/language/*' 'app/models/*' 'app/core/*' | sort printf '\n== Outlines ==\n' for f in app/services/project/create_project.py app/services/project/update_project.py app/services/language/*.py app/models/*.py app/core/exceptions.py; do if [ -f "$f" ]; then echo "--- $f" ast-grep outline "$f" --view expanded || true fi done printf '\n== Relevant content ==\n' for f in app/services/project/create_project.py app/services/project/update_project.py app/services/language/*.py app/models/*.py app/core/exceptions.py; do if [ -f "$f" ]; then echo "--- $f" nl -ba "$f" | sed -n '1,240p' fi doneRepository: shemaobt/tripod-api
Length of output: 50377
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in \ app/services/project/create_project.py \ app/services/project/update_project.py \ app/services/language/get_language_by_id.py \ app/services/language/deactivate_language.py \ app/core/database.py \ app/models/language.py \ app/models/project.py do echo "--- $f" nl -ba "$f" | sed -n '1,220p' echo done printf '\n== Search for row locking / isolation / select_for_update ==\n' rg -n "select_for_update|FOR UPDATE|with_for_update|isolation_level|begin\(|commit\(|flush\(" app/services app/core app/models -g '!**/__pycache__/**'Repository: shemaobt/tripod-api
Length of output: 241
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path paths = [ "app/services/project/create_project.py", "app/services/project/update_project.py", "app/services/language/get_language_by_id.py", "app/services/language/deactivate_language.py", "app/core/database.py", "app/models/language.py", "app/models/project.py", ] for p in paths: path = Path(p) print(f"--- {p}") if not path.exists(): print("MISSING") print() continue for i, line in enumerate(path.read_text().splitlines(), 1): if i > 220: print("... (truncated)") break print(f"{i:4d}: {line}") print() PY printf '\n== Search for row locking / transaction boundaries ==\n' rg -n "select_for_update|with_for_update|FOR UPDATE|begin\(|commit\(|flush\(" app/services app/core app/models -g '!**/__pycache__/**'Repository: shemaobt/tripod-api
Length of output: 19707
Lock language validation to the project write
create_projectandupdate_projectreadLanguage.is_activewithout any row lock, then write the project later. Another transaction can deactivate the language in between, letting an inactivelanguage_idslip through.
app/services/project/create_project.py#L19-L33: validate and insert under the same lock/transaction, or enforce the active-language invariant at write time.app/services/project/update_project.py#L19-L30: apply the same protection before assigningproject.language_id.📍 Affects 2 files
app/services/project/create_project.py#L19-L23(this comment)app/services/project/update_project.py#L19-L24🤖 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/create_project.py` around lines 19 - 23, Protect language validation and the subsequent project write in both create_project.py (lines 19-33) and update_project.py (lines 19-30) with the same transaction/row lock, or enforce the active-language invariant atomically at write time. Ensure create_project cannot insert and update_project cannot assign a language_id after another transaction deactivates the language.
4 tasks
joaocarvoli
requested changes
Jul 17, 2026
| deps_result = await db.execute( | ||
| select(PhaseDependency).where( | ||
| PhaseDependency.phase_id.in_(phase_ids), | ||
| PhaseDependency.depends_on_id.in_(phase_ids), |
Member
There was a problem hiding this comment.
Just make sure here if dropping the edges that point outside the managed projects is really what we want. If a manager has "Review" attached to his project but "Draft" is not, the admin gets Review: [Draft] and the manager gets Review: [], so on the Console the phase appears with no prerequisite at all. The PR is for scope the visibility, not for change the dependencies graph — could you verify that against the owner's rule and if it is wrong drop the depends_on_id filter?
| db_session, [managed.id], project_id=other.id | ||
| ) | ||
|
|
||
| assert result == [] |
Member
There was a problem hiding this comment.
list_phases_with_deps_by_projects is the only one of the four scoped endpoints with no test here, and it is the one with more logic (the deps map + the filter of the other comment). Could you add one? I think it would already answer the question there.
| return list(result.scalars().unique().all()) | ||
|
|
||
|
|
||
| async def list_phases_with_deps_by_projects( |
Member
There was a problem hiding this comment.
Two public functions on the same file here — the rest of app/services/phase/ is one function per file named after it, and list_all_phases_with_deps.py is exactly the sibling of this one. If the one function per file is really the majority on the codebase, please move list_phases_with_deps_by_projects to its own file.
2 tasks
# Conflicts: # app/api/languages.py
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[US-11.6 / OBT-231] Scope console lists to the caller's managed projects
Summary
Non-admin users previously received the full dataset from the console list endpoints. This scopes
GET /api/organizations,/api/languages,/api/phasesand/api/phases/with-dependenciesto the projects the caller manages (ProjectUserAccess.role == "manager"), aligning the scoping with the 2026-07 per-project manager model. Platform admins are unaffected and continue to see everything.Changes
1. Add the managed-projects helper
app/core/org_scope.py— newget_managed_project_ids(db, user_id)returning the project IDs where the user'sProjectUserAccess.role == "manager". (Reused by US-11.8's console guard and project scoping.)2. Add per-domain "by projects" services
app/services/org/list_organizations_by_projects.py,app/services/language/list_languages_by_projects.py,app/services/phase/list_phases_by_projects.py(new) — resolve, from a set of project IDs, the organizations linked viaproject_organization_access, the distinct languages (Project.language_id), and the phases attached viaproject_phases(pluslist_phases_with_deps_by_projects). Re-exported from each domain__init__.py. Replaces the earlier organization-based services.3. Scope the list endpoints by managed projects
app/api/organizations.py,app/api/languages.py,app/api/phases.py— each list route branches onuser.is_platform_admin; otherwise resolvesget_managed_project_idsand delegates to the matching "by projects" service.4. Keep inactive languages out of the manager list (rule change 2026-07-16)
app/services/language/list_languages_by_projects.pyfilters onLanguage.is_active. Without it, the manager path bypassed the active-only filter thatlist_languagesalready applies, so a manager kept seeing a language after a platform admin deactivated it — the owner's rule is that a manager must never see an inactive language.app/api/languages.pycorrespondingly honoursinclude_inactiveonly on the platform-admin branch, so the parameter is unreachable for a manager.This is the change that forced the stack:
Language.is_activearrives with #97, andlist_languages_by_projectsis owned by this PR, so neither branch could carry the fix alone.Type of Change
Testing
tests/test_console_scoping.py::test_list_languages_by_projects_excludes_inactive(new) — a manager managing a project whose language was deactivated receives only the active one. Verified to fail without the filter (assert ['lca', 'lci'] == ['lca']) and pass with it.tests/test_console_scoping.pyand the org/language/phase service tests pass.ruff check+ruff format --checkclean.Summary by CodeRabbit
Merge reconciliation with Journeys (#132/#135)
Once journeys land,
list_phases_by_projects/list_phases_with_deps_by_projectsmust derive the manager's phases from the journeys of their managed projects (full phase set per journey,sort_orderordering; link-based fallback only for projects without a journey) instead ofproject_phasesrows — otherwise managers only see phases that already had a status change. See the "Merge reconciliation" section on #135 and the stacked reconciliation PR #136 (merge it last); the fix is already validated on the local integration branch.