Admin Portal v1 (Week 1): platform-admin gate, org management, users tab (Issue #1254)

[Veekshitha11]

Combined PR for the full Week 1 Admin Portal build (features/admin-portal/v1/plan.md), covering all four milestones: M1 (platform-admin gate), M2 (/admin shell + dashboard), M3 (organizations: list/create/edit), M4 (users tab: invite/role/remove/cancel-invite), plus a local-dev SES fallback and a full end-to-end lifecycle test. Org/user deactivation was scoped out of this PR -- the is_active enforcement added early on was removed rather than left half-wired, since no endpoint ever set it (see PR review discussion). Reintegrated onto current main -- originally built on an older base, then reconciled with two upstream changes that landed in the meantime: RBAC v2's role collapse (#1414, no behavior change to this work -- aliases preserved) and a cascade-delete fix (#1428) that independently fixed the same dashboard/chart-deletion bug this PR had flagged as a fast-follow; removal-impact messaging updated accordingly (orphaned, not deleted). Full regression suite green -- see commit messages for milestone-by-milestone detail.

Summary by CodeRabbit

• New Features

  • Added a platform-admin portal for viewing platform health, statistics, organizations, and organization details.
  • Added organization creation and editing.
  • Added organization user management, including invitations, role changes, removal-impact previews, and membership deletion.
  • Added platform-admin status to current-user and login responses.
  • Added development email fallback logging when email credentials are unavailable.

• Bug Fixes

  • Improved invitation scoping and acceptance across organizations.
  • Authentication failures now return consistent HTTP 401 responses.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 24387ba7-eddd-4382-a5f5-28f65278a957

📥 Commits

Reviewing files that changed from the base of the PR and between eca9865 and 20720fe.

📒 Files selected for processing (4)

• ddpui/api/admin_api.py
• ddpui/core/admin/admin_service.py
• ddpui/schemas/admin_schema.py
• ddpui/tests/api_tests/test_admin_api.py

💤 Files with no reviewable changes (2)

• ddpui/core/admin/admin_service.py
• ddpui/api/admin_api.py

---

Walkthrough

Adds a platform-admin portal for cross-organization administration, scoped membership and invitation management, organization and membership deactivation, authentication enforcement, invitation targeting, and development-safe email behavior.

Changes

Admin portal lifecycle

Layer / File(s)	Summary
State fields and access enforcement ddpui/models/*, ddpui/migrations/*, ddpui/auth.py, ddpui/api/user_org_api.py	Adds organization and membership activation state, invitation target-org tracking, platform-admin authorization, deactivation checks, and platform-admin identity exposure.
Scoped invitation and membership core ddpui/core/orguserfunctions.py, ddpui/api/user_org_api.py, ddpui/tests/api_tests/test_user_org_api.py	Moves invitation and deletion logic to explicit target-org helpers and preserves legacy invitation acceptance fallback.
Admin service contracts and operations ddpui/schemas/admin_schema.py, ddpui/core/admin/*	Defines admin schemas and errors, then implements organization lifecycle, invitation, membership, and removal-impact operations.
Admin portal API and routing ddpui/api/admin_api.py, ddpui/routes.py	Adds platform-admin identity, health, statistics, organization, Users-tab, invitation, and membership routes under /api/v1/admin/.
Lifecycle and integration validation ddpui/tests/api_tests/*, ddpui/tests/core/*, ddpui/utils/awsses.py, ddpui/tests/utils/test_awsses.py	Covers admin lifecycle behavior, authentication enforcement, scoped operations, invitation compatibility, and development SES fallback behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: ⚪ Minimal · up to 20720

The PR adds the Week 1 admin portal capabilities and related lifecycle behavior; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

• DalgoT4D/DDP_backend#1080: Both changes modify JWT middleware token handling and authentication behavior.
• DalgoT4D/DDP_backend#1081: Both changes modify invitation acceptance and organization-user flows.
• DalgoT4D/DDP_backend#1428: Both changes cover preserving content while clearing creator references during membership removal.

Suggested reviewers: fatchat

Sequence Diagram(s)

sequenceDiagram
  participant PlatformAdmin
  participant admin_router
  participant admin_service
  participant Database
  PlatformAdmin->>admin_router: request organization or user operation
  admin_router->>admin_service: authorize and execute scoped action
  admin_service->>Database: read or update organization state
  Database-->>admin_service: return persisted data
  admin_service-->>admin_router: return service result
  admin_router-->>PlatformAdmin: return admin schema response

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 95.29% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: Admin Portal v1, platform-admin authorization, organization management, and user administration.

✨ Finishing Touches 💡 1

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch feature/admin-portal-m4-users

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

ddpui/core/orguserfunctions.py (2)

200-219: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Email lookup uses exact match, not __iexact/normalized — inconsistent with sibling functions.

user__email=payload.email (line 200-202) and invited_email=payload.email (line 217-219) do exact-string matching, unlike invite_user_to_org (user__email__iexact=invited_email, line 255) and change_orguser_role_in_org (user__email__iexact=request_email, line 358) in this same file. Since stored User.email is always lowercased on creation, a caller passing a differently-cased email (e.g. from a user-supplied DeleteOrgUserPayload.email via the single-org delete_orguser_v1 wrapper) would incorrectly get "user does not belong to the org" and skip invitation cleanup for an existing member.

🐛 Proposed fix

+    normalized_email = payload.email.lower().strip()
     orguser_to_delete = OrgUser.objects.filter(
-        org=target_org, user__email=payload.email
+        org=target_org, user__email__iexact=normalized_email
     ).first()
     ...
     # remove the pending invitations for this email in the target org
     Invitation.objects.filter(
-        invited_in_org=target_org, invited_email=payload.email
+        invited_in_org=target_org, invited_email__iexact=normalized_email
     ).delete()

🤖 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 `@ddpui/core/orguserfunctions.py` around lines 200 - 219, Update the member
lookup in the surrounding delete-orguser flow to use case-insensitive email
matching, consistent with invite_user_to_org and change_orguser_role_in_org.
Apply the same case-insensitive matching to the Invitation cleanup query so
differently-cased payload.email values identify the existing user and remove
pending invitations.

---

283-297: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Resend branch returns InvitationSchema under a NewInvitationSchema endpoint. post_organization_user_invite_v1 declares response=NewInvitationSchema, but this branch returns from_invitation(invitation), and from_invitation() builds InvitationSchema fields (invited_by, invited_on, invite_code, invited_new_role_slug). This can break response serialization; return the same NewInvitationSchema shape as the other invite paths.

🤖 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 `@ddpui/core/orguserfunctions.py` around lines 283 - 297, Update the
existing-invitation branch in post_organization_user_invite_v1 to return a
NewInvitationSchema-shaped result instead of from_invitation(invitation), which
produces InvitationSchema fields. Match the response construction used by the
other invite paths while preserving the resend email, timestamp update, logging,
and existing-invitation behavior.

🧹 Nitpick comments (2)

ddpui/utils/awsses.py (1)

16-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated credential-check logic between awsses.py and aws_client.py.

_ses_available() re-implements the exact SES_ACCESS_KEY_ID/SES_SECRET_ACCESS_KEY check already performed inside AWSClient._initialize_boto_session. If that env-var pair ever changes in aws_client.py, this check silently drifts and the DEBUG fallback could misbehave with no test catching it.

♻️ Suggested refactor: single source of truth

-def _ses_available() -> bool:
-    """True when real SES credentials are configured (the same pair AWSClient needs)."""
-    return bool(os.getenv("SES_ACCESS_KEY_ID") and os.getenv("SES_SECRET_ACCESS_KEY"))
+def _ses_available() -> bool:
+    """True when real SES credentials are configured."""
+    return AWSClient.has_credentials("ses")

Add a small has_credentials(cls, service_name) classmethod to AWSClient that checks the same env vars it already resolves per-service.

🤖 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 `@ddpui/utils/awsses.py` around lines 16 - 23, Remove the duplicated
environment-variable check from _ses_available and add or reuse
AWSClient.has_credentials("ses") as the single credential-availability source.
Implement has_credentials as a classmethod using the same per-service credential
resolution as AWSClient._initialize_boto_session, then have _ses_available
delegate to it while preserving the existing boolean behavior.

ddpui/api/admin_api.py (1)

344-361: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Sentinel id=0 for the "already-existing-user" invite branch is easy to misuse, and untested.

When the invitee already has a platform account, the endpoint returns a stub AdminInvitationSchema(id=0, ...) instead of a real invitation. Callers must special-case id == 0 to distinguish "added directly" from "pending invite created" — an implicit contract that's easy to miss on the frontend. No test in test_admin_api.py covers this branch either.

Consider adding an explicit discriminator (e.g. already_member: bool) instead of overloading id.

🤖 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 `@ddpui/api/admin_api.py` around lines 344 - 361, The existing-user invitation
response in the admin invitation flow should expose an explicit discriminator
instead of relying on the sentinel id=0. Add and populate an `already_member`
boolean on `AdminInvitationSchema`, set it for the no-invitation branch around
the `invitation is None` check, preserve the normal invitation behavior, and add
coverage in `test_admin_api.py` for this branch.

🤖 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 `@ddpui/api/admin_api.py`:
- Around line 80-89: Eliminate the per-organization queries in get_admin_orgs
and _admin_org_response by annotating each organization with its user count and
selecting or prefetching the related plan data in the queryset. Update
_admin_org_response to use those loaded values instead of calling
OrgUser.objects.filter(...).count() or org.base_plan(), while preserving the
existing AdminOrgSchema fields and response behavior.
- Around line 373-384: Update put_admin_org_user_role and
change_orguser_role_in_org to return a typed or sentinel permission error
instead of relying on the literal "Insufficient permissions" message. Determine
the HTTP status from that error type/sentinel, while preserving the existing 403
response for permission failures and 400 for other errors.
- Around line 124-151: Update post_admin_org and the
create_organization/create_org_plan flow so Airbyte workspace provisioning is
not left orphaned when plan creation fails: either move the remote provisioning
outside the outer transaction or explicitly delete the created workspace before
raising the plan error. Preserve atomic database behavior and the existing
cleanup on provisioning failure, while shortening the HTTP transaction scope
where possible.

---

Outside diff comments:
In `@ddpui/core/orguserfunctions.py`:
- Around line 200-219: Update the member lookup in the surrounding
delete-orguser flow to use case-insensitive email matching, consistent with
invite_user_to_org and change_orguser_role_in_org. Apply the same
case-insensitive matching to the Invitation cleanup query so differently-cased
payload.email values identify the existing user and remove pending invitations.
- Around line 283-297: Update the existing-invitation branch in
post_organization_user_invite_v1 to return a NewInvitationSchema-shaped result
instead of from_invitation(invitation), which produces InvitationSchema fields.
Match the response construction used by the other invite paths while preserving
the resend email, timestamp update, logging, and existing-invitation behavior.

---

Nitpick comments:
In `@ddpui/api/admin_api.py`:
- Around line 344-361: The existing-user invitation response in the admin
invitation flow should expose an explicit discriminator instead of relying on
the sentinel id=0. Add and populate an `already_member` boolean on
`AdminInvitationSchema`, set it for the no-invitation branch around the
`invitation is None` check, preserve the normal invitation behavior, and add
coverage in `test_admin_api.py` for this branch.

In `@ddpui/utils/awsses.py`:
- Around line 16-23: Remove the duplicated environment-variable check from
_ses_available and add or reuse AWSClient.has_credentials("ses") as the single
credential-availability source. Implement has_credentials as a classmethod using
the same per-service credential resolution as
AWSClient._initialize_boto_session, then have _ses_available delegate to it
while preserving the existing boolean behavior.

🪄 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: CHILL

Plan: Pro

Run ID: 2d0adb6d-d5ef-4a4c-bb3d-af2fdf618ba9

📥 Commits

Reviewing files that changed from the base of the PR and between f03abed and 5222d79.

📒 Files selected for processing (15)

• ddpui/api/admin_api.py
• ddpui/api/user_org_api.py
• ddpui/auth.py
• ddpui/core/orguserfunctions.py
• ddpui/migrations/0169_org_is_active.py
• ddpui/migrations/0170_orguser_is_active.py
• ddpui/migrations/0171_invitation_invited_in_org.py
• ddpui/models/org.py
• ddpui/models/org_user.py
• ddpui/routes.py
• ddpui/tests/api_tests/test_admin_api.py
• ddpui/tests/api_tests/test_user_org_api.py
• ddpui/tests/core/test_auth.py
• ddpui/tests/utils/test_awsses.py
• ddpui/utils/awsses.py

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n '"Insufficient permissions' ddpui/core/orguserfunctions.py

Repository: DalgoT4D/DDP_backend

Length of output: 337

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- ddpui/api/admin_api.py (around lines 360-390) ---'
sed -n '360,390p' ddpui/api/admin_api.py

echo
echo '--- ddpui/core/orguserfunctions.py (around lines 240-370) ---'
sed -n '240,370p' ddpui/core/orguserfunctions.py

echo
echo '--- tests / references to "Insufficient permissions" ---'
rg -n '"Insufficient permissions' ddpui tests || true

Repository: DalgoT4D/DDP_backend

Length of output: 8518

---

Avoid deriving the status code from the literal error string. change_orguser_role_in_org() returns "Insufficient permissions" for permission failures today, but HttpError(403 if error == "Insufficient permissions" else 400, error) makes the response depend on exact wording; a small message change would turn a permission error into a 400. Use a typed/sentinel error instead.

🤖 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 `@ddpui/api/admin_api.py` around lines 373 - 384, Update
put_admin_org_user_role and change_orguser_role_in_org to return a typed or
sentinel permission error instead of relying on the literal "Insufficient
permissions" message. Determine the HTTP status from that error type/sentinel,
while preserving the existing 403 response for permission failures and 400 for
other errors.

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@ddpui/core/admin/admin_service.py`:
- Around line 97-101: Update the organization partial-update logic at
ddpui/core/admin/admin_service.py:97-101 to build update_fields from the
provided name and viz_url arguments, call org.save(update_fields=update_fields)
only when non-empty, and remove the misleading exclusion comment. At
ddpui/core/admin/admin_service.py:106-107, save org_plans with
update_fields=["base_plan"]; at ddpui/core/admin/admin_service.py:117-118, save
org with update_fields=["is_active"].
- Around line 77-79: Update the create-organization flow around create_org_plan
so that when plan_error is present, the newly created org is deleted before
returning the error. Preserve the existing successful path and return values.

🪄 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: CHILL

Plan: Pro

Run ID: e38d0f47-3b8c-449d-9d46-ef8e14650c3d

📥 Commits

Reviewing files that changed from the base of the PR and between 5222d79 and 44f76ff.

📒 Files selected for processing (4)

• ddpui/api/admin_api.py
• ddpui/core/admin/__init__.py
• ddpui/core/admin/admin_service.py
• ddpui/core/orguserfunctions.py

🚧 Files skipped from review as they are similar to previous changes (2)

• ddpui/api/admin_api.py
• ddpui/core/orguserfunctions.py

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Roll back the created organization if plan creation fails.

If create_org_plan fails, the Org record is currently left in the database. This contradicts the API layer's assumption that the transaction is fully rolled back upon failure. Furthermore, since orgfunctions.create_organization enforces unique organization names, the orphaned record will permanently block any future attempts to create an organization with the same name.

Add org.delete() to clean up the database state.

🐛 Proposed fix

     _, plan_error = orgfunctions.create_org_plan(create_payload, org)
     if plan_error:
+        org.delete()
         return None, plan_error

📝 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.

Suggested change

	_, plan_error = orgfunctions.create_org_plan(create_payload, org)
	if plan_error:
	return None, plan_error
	_, plan_error = orgfunctions.create_org_plan(create_payload, org)
	if plan_error:
	org.delete()
	return None, plan_error

🤖 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 `@ddpui/core/admin/admin_service.py` around lines 77 - 79, Update the
create-organization flow around create_org_plan so that when plan_error is
present, the newly created org is deleted before returning the error. Preserve
the existing successful path and return values.

[coderabbitai]

🧹 Nitpick comments (1)

ddpui/tests/api_tests/test_admin_api.py (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the duplicated Redis-mocking helper into a shared test fixture. All three files independently define an identical pair-of-patches helper (RedisClient.get_instance + set_roles_and_permissions_in_redis), each docstring noting no shared helper exists.

• ddpui/tests/api_tests/test_admin_api.py#L882-887: replace _mock_auth_redis() with an import from a shared conftest/test-util fixture.
• ddpui/tests/core/test_admin_auth.py#L39-44: replace _mock_auth_redis() with the same shared fixture.
• ddpui/tests/core/test_admin_service.py#L37-42: replace _mock_redis() with the same shared fixture.

🤖 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 `@ddpui/tests/api_tests/test_admin_api.py` at line 1, Extract the duplicated
Redis patch helper into one shared test fixture or test utility, covering
RedisClient.get_instance and set_roles_and_permissions_in_redis. Replace
_mock_auth_redis in test_admin_api.py and test_admin_auth.py, and _mock_redis in
test_admin_service.py, with imports and usage of that shared fixture; remove the
local helper definitions and outdated docstrings.

🤖 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.

Nitpick comments:
In `@ddpui/tests/api_tests/test_admin_api.py`:
- Line 1: Extract the duplicated Redis patch helper into one shared test fixture
or test utility, covering RedisClient.get_instance and
set_roles_and_permissions_in_redis. Replace _mock_auth_redis in
test_admin_api.py and test_admin_auth.py, and _mock_redis in
test_admin_service.py, with imports and usage of that shared fixture; remove the
local helper definitions and outdated docstrings.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 92841ff9-32bf-4433-83d1-a7ad40cf2a5a

📥 Commits

Reviewing files that changed from the base of the PR and between 44f76ff and 7dd3ae8.

📒 Files selected for processing (9)

• ddpui/api/admin_api.py
• ddpui/auth.py
• ddpui/core/admin/admin_service.py
• ddpui/core/admin/exceptions.py
• ddpui/schemas/admin_schema.py
• ddpui/settings.py
• ddpui/tests/api_tests/test_admin_api.py
• ddpui/tests/core/test_admin_auth.py
• ddpui/tests/core/test_admin_service.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

ddpui/auth.py (1)

161-189: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Skip per-org deactivation checks for platform admins.

When no x-dalgo-org header is present, OrgUser.objects.filter(user=request.user).first() can resolve to any OrgUser row; for @platform_admin_required routes, platform_admin_required never gets a chance to grant cross-org/Dalgo-ops authority because the middleware has already raised 403 for an unrelated deactivated org/membership. Gate these checks with UserAttributes.is_platform_admin or route platform-admin traffic through an exempt credential path.

🤖 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 `@ddpui/auth.py` around lines 161 - 189, The org and membership deactivation
checks in the token-authentication flow must not block platform admins when no
x-dalgo-org header is provided. Use the existing
UserAttributes.is_platform_admin signal to bypass the orguser.org.is_active and
orguser.is_active checks for platform-admin users, while preserving both 403
checks for non-admins and requests scoped to a specific organization.

🤖 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.

Outside diff comments:
In `@ddpui/auth.py`:
- Around line 161-189: The org and membership deactivation checks in the
token-authentication flow must not block platform admins when no x-dalgo-org
header is provided. Use the existing UserAttributes.is_platform_admin signal to
bypass the orguser.org.is_active and orguser.is_active checks for platform-admin
users, while preserving both 403 checks for non-admins and requests scoped to a
specific organization.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 98c48419-42ce-4e81-be23-1bc9811bbea8

📥 Commits

Reviewing files that changed from the base of the PR and between 7dd3ae8 and eca9865.

📒 Files selected for processing (9)

• ddpui/api/admin_api.py
• ddpui/auth.py
• ddpui/core/admin/admin_service.py
• ddpui/core/admin/exceptions.py
• ddpui/routes.py
• ddpui/schemas/admin_schema.py
• ddpui/tests/api_tests/test_admin_api.py
• ddpui/tests/core/test_admin_service.py
• ddpui/tests/core/test_auth.py

💤 Files with no reviewable changes (1)

• ddpui/core/admin/exceptions.py

🚧 Files skipped from review as they are similar to previous changes (1)

• ddpui/tests/core/test_auth.py

[siddhant3030]

Review — two blocking items before merge:

1. Deactivation is enforceable but not actionable. The PR description says M3/M4 include org/user deactivate, and the enforcement layer is all here (Org.is_active, OrgUser.is_active, migrations 0169/0170, the middleware 403s) — but no endpoint ever sets either flag, and neither AdminOrgSchema nor AdminOrgUserSchema exposes is_active, so the portal can't set or display active status (the get_admin_org_users docstring even promises "per-org Status"). If deactivation was deliberately deferred to a follow-up PR, please update the PR body and docstring; otherwise the endpoints are missing.

2. Orphaned Airbyte workspace when plan creation fails. In post_admin_org, @transaction.atomic rolls back the Org row when create_org_plan fails, but create_organization has already provisioned a real Airbyte workspace — an external side effect the DB rollback can't undo, leaving a leaked workspace with no Org pointing at it. Either delete the workspace in the plan-failure path, or log the workspace ID loudly for cleanup.

[siddhant3030]

we dont need this. @Veekshitha11

[Veekshitha11]

Removed /admin/ping and AdminPingSchema. It was only there to exercise the platform-admin gate in tests, so I repointed those 3 tests at /admin/currentuser (a real endpoint with the same gate) instead of dropping the coverage.

[siddhant3030]

can we add only 2 lines of comments here? we dont need this long comment for every functions

[Veekshitha11]

Done — trimmed every function docstring in this file (and admin_api.py, which had the same style) down to 1-2 lines.

[siddhant3030]

why do we need this?

[Veekshitha11]

Good question — this is needed because a platform admin invites cross-org: the admin isn't a member of the target org, so invited_by.org (the admin's own org) isn't the target org. Without invited_in_org, accept/cancel and the per-org pending-invites list would resolve to the wrong org. Trimmed the comment block down per your other note.

[siddhant3030]

we dont need this.

[Veekshitha11]

Removed, along with the /admin/ping endpoint that used it.

[siddhant3030]

is this extending current create org schema?

[Veekshitha11]

It wasn't before — now it does. Made AdminCreateOrgSchema(CreateOrgSchema) a real subclass overriding just base_plan/can_upgrade_plan/subscription_duration/superset_included with admin-friendly defaults, so admin_service.create_org can pass the payload straight through instead of manually rebuilding a CreateOrgSchema.

[siddhant3030]

what is this?

[siddhant3030]

and why do we need it?

[Veekshitha11]

This is a local-dev convenience: _ses_available() checks whether real SES credentials are configured. When they're not (and DEBUG=True), send_text_message logs the email instead of trying to call real SES, so invite/signup/password-reset flows work on a laptop without needing AWS SES set up. It's off in staging/prod since DEBUG is off there, so it can't mask a real misconfiguration. There's a dedicated test for it. Shortened the docstrings per your other comment.

[siddhant3030]

same question here?

[Veekshitha11]

Same answer as above — send_text_message is the one function that uses this local-dev fallback (the other send_* helpers always hit real SES). Docstring trimmed.

[Veekshitha11]

Addressed both:

1. Deactivation — removed is_active entirely rather than building the missing endpoint (migration 0172_remove_org_is_active_orguser_is_active), since it wasn't part of what shipped. Also caught a stale get_admin_org_users docstring still promising "per-org Status" and fixed it. Updating the PR description to drop the stale deactivate mentions.
2. Orphaned Airbyte workspace — admin_service.create_org now calls airbyte_service.delete_workspace(...) when create_org_plan fails, before raising, so the workspace doesn't outlive the rolled-back Org row. A delete failure is logged loudly rather than silently swallowed.

Replied inline on the rest of the comments too.