feat(admin): add organization update API and schemas#1338
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds admin organization management functionality with three protected endpoints for listing, fetching, and updating organizations. It includes a request schema with validation, API endpoint implementations with slug uniqueness enforcement and audit logging, and router registration under the admin path. ChangesAdmin Organization Management API
Sequence DiagramsequenceDiagram
participant Admin as Admin Client
participant API as Admin API
participant DB as Database
participant Log as AuditLog
Admin->>API: GET /api/admin/organizations?name=...
API->>DB: Query organizations (filtered)
DB-->>API: Organization list
API-->>Admin: {count, data}
Admin->>API: GET /api/admin/organizations/{id}
API->>DB: Fetch organization
DB-->>API: Organization or empty
API-->>Admin: {id, name, slug} or 404
Admin->>API: PUT /api/admin/organizations/{id} (payload)
API->>DB: Load organization by id
DB-->>API: Organization or empty
API->>DB: Check for existing org with slug
DB-->>API: Other org with slug or empty
API->>DB: Update name and slug
DB-->>API: Saved organization
API->>Log: Create AdminAuditLog record
Log-->>API: Audit record created
API-->>Admin: {message, id, name, slug}
🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. 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: 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/api/admin_portal/organizations_api.py`:
- Around line 64-75: The slug uniqueness check is race-prone because it lacks a
DB constraint and the exists() pre-check is separate from the save; add a
uniqueness constraint to the Org model (Org.slug -> unique=True or a
UniqueConstraint in ddpui/models/org.py and generate a migration), then in
organizations_api.py wrap the write in a transaction.atomic block around
org.save() and catch django.db.IntegrityError (or DatabaseError) to translate it
into HttpError(409, "Slug already exists"); you can keep the existing_slug
pre-check for early feedback but must still handle IntegrityError on org.save()
to ensure atomicity and return 409 on conflict.
In `@ddpui/schemas/admin_org_schema.py`:
- Around line 7-8: The schema currently allows empty or whitespace-only strings
for the name and slug fields; update the AdminOrgSchema (or the schema class
that defines name and slug) to enforce non-blank values by either changing the
field types to Pydantic constrained strings (e.g., use
constr(strip_whitespace=True, min_length=1) for name and slug) or adding a
`@validator` for ("name","slug") that strips the value and raises ValueError if
the result is empty, ensuring both fields cannot be blank/whitespace-only.
🪄 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: 4471907f-6ea2-41f6-81fb-39b87ed0abc3
📒 Files selected for processing (4)
ddpui/api/admin_portal/organizations_api.pyddpui/models/airbyte.pyddpui/routes.pyddpui/schemas/admin_org_schema.py
| existing_slug = ( | ||
| Org.objects.filter(slug=payload.slug) | ||
| .exclude(id=org_id) | ||
| .exists() | ||
| ) | ||
|
|
||
| if existing_slug: | ||
| raise HttpError(400, "Slug already exists") | ||
|
|
||
| org.name = payload.name | ||
| org.slug = payload.slug | ||
| org.save() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Org model field definitions (name/slug) =="
fd -i 'org.py' | xargs -r rg -n -C2 'class\s+Org|name\s*=|slug\s*='
echo
echo "== Uniqueness constraints involving slug =="
rg -n -C2 'slug|UniqueConstraint|unique_together|unique\s*=\s*True' --type pyRepository: DalgoT4D/DDP_backend
Length of output: 50377
Slug uniqueness check is non-atomic and race-prone.
The Org.slug field has no database-level uniqueness constraint (unique=True or UniqueConstraint). The pre-check with .exists() and the write with .save() are separate operations, allowing concurrent requests to both pass validation and create duplicate slugs.
Add a database constraint and wrap the write in atomic transaction handling with IntegrityError catching. Also update the HTTP status code to 409 (Conflict) to properly reflect the error semantics.
💡 Suggested hardening
+from django.db import IntegrityError, transaction
from ninja import Router
from ninja.errors import HttpError
@@
existing_slug = (
Org.objects.filter(slug=payload.slug)
.exclude(id=org_id)
.exists()
)
if existing_slug:
- raise HttpError(400, "Slug already exists")
+ raise HttpError(409, "Slug already exists")
org.name = payload.name
org.slug = payload.slug
- org.save()
+ try:
+ with transaction.atomic():
+ org.save(update_fields=["name", "slug"])
+ except IntegrityError:
+ raise HttpError(409, "Slug already exists")Also add to the Org model (ddpui/models/org.py):
class Org(models.Model):
name = models.CharField(max_length=50)
- slug = models.CharField(max_length=20, null=True)
+ slug = models.CharField(max_length=20, null=True, unique=True)🤖 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_portal/organizations_api.py` around lines 64 - 75, The slug
uniqueness check is race-prone because it lacks a DB constraint and the exists()
pre-check is separate from the save; add a uniqueness constraint to the Org
model (Org.slug -> unique=True or a UniqueConstraint in ddpui/models/org.py and
generate a migration), then in organizations_api.py wrap the write in a
transaction.atomic block around org.save() and catch django.db.IntegrityError
(or DatabaseError) to translate it into HttpError(409, "Slug already exists");
you can keep the existing_slug pre-check for early feedback but must still
handle IntegrityError on org.save() to ensure atomicity and return 409 on
conflict.
There was a problem hiding this comment.
Thanks for the review. I’ve noted the race-condition concern around slug uniqueness validation. Since this PR focuses on introducing the initial admin update APIs, I’d prefer to keep database-level uniqueness constraints and transactional handling as a follow-up improvement to avoid expanding the migration scope significantly in this change set.
There was a problem hiding this comment.
@sachin9919, understood — thanks for the context. Deferring database-level constraints and transactional handling to a follow-up is a reasonable call to keep this PR focused.
Would you like me to open a GitHub issue to track the follow-up work? It would cover:
- Adding
unique=True(or aUniqueConstraint) toOrg.slugalong with the corresponding migration. - Wrapping
org.save()intransaction.atomic()and catchingIntegrityErrorto returnHTTP 409on slug conflicts in the update endpoint.
There was a problem hiding this comment.
Updated the schema validation to reject whitespace-only values for name and slug using field_validator, ensuring inputs are properly stripped and validated before update operations.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'admin_org_schema.py' | xargs -r cat -n
Length of output: 683
@sachin9919, the implementation looks solid. A few notes from reviewing the updated file:
-
Correct layering —
Field(..., min_length=1)rejects empty strings at Pydantic's field level, andmust_not_be_blankthen catches whitespace-only inputs (e.g." "). The two guards complement each other well. -
Silent normalisation — because the validator returns
value.strip(), inputs like" admin "will be silently saved as"admin". This is usually desirable, but worth being aware of (and potentially documenting) so callers know the API normalises values rather than rejecting them. -
Classmethod pattern —
@field_validator+@classmethodis the correct Pydantic v2 pattern; no issues there.
The race-condition / atomicity concern from the earlier review thread is still tracked as a follow-up. Would you still like me to open a GitHub issue to track:
- Adding
unique=TruetoOrg.slugwith a migration. - Wrapping
org.save()intransaction.atomic()and catchingIntegrityError→HTTP 409?
There was a problem hiding this comment.
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/models/admin_audit_log.py`:
- Around line 14-17: Change the org ForeignKey on the AdminAuditLog model from
on_delete=models.CASCADE to on_delete=models.SET_NULL and add null=True,
blank=True so audit records persist when an Org is deleted; update the
AdminAuditLog.__str__ method to guard against self.org being None (e.g., check
if self.org is None and use a fallback string or "deleted-org" instead of
accessing self.org.slug directly) to avoid AttributeError after an Org is
removed.
- Around line 27-30: The created_at DateTimeField in the model (created_at =
models.DateTimeField(...)) is using the undocumented auto_created=True; change
it to use auto_now_add=True so the timestamp is set once on creation, remove the
explicit default=timezone.now (not needed with auto_now_add), and add
editable=False to prevent edits; update the created_at field definition
accordingly in the admin_audit_log model.
🪄 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: 675ce16e-1b97-4a57-93ea-2c048a888184
📒 Files selected for processing (5)
ddpui/api/admin_portal/organizations_api.pyddpui/migrations/0160_alter_syncstats_sync_type_adminauditlog.pyddpui/models/__init__.pyddpui/models/admin_audit_log.pyddpui/schemas/admin_org_schema.py
✅ Files skipped from review due to trivial changes (1)
- ddpui/models/init.py
🚧 Files skipped from review as they are similar to previous changes (1)
- ddpui/api/admin_portal/organizations_api.py
| org = models.ForeignKey( | ||
| Org, | ||
| on_delete=models.CASCADE, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n ddpui/models/admin_audit_log.pyRepository: DalgoT4D/DDP_backend
Length of output: 1104
🏁 Script executed:
# Search for AdminAuditLog usage
rg "AdminAuditLog" --type py -B 2 -A 2
# Look for migrations related to this model
find . -path "*/migrations/*" -name "*.py" -exec grep -l "AdminAuditLog" {} \;Repository: DalgoT4D/DDP_backend
Length of output: 2128
🏁 Script executed:
# Check the migration
cat -n ddpui/migrations/0160_alter_syncstats_sync_type_adminauditlog.py | head -80
# Check the usage in organizations_api.py
rg "AdminAuditLog" ddpui/api/admin_portal/organizations_api.py -B 5 -A 10Repository: DalgoT4D/DDP_backend
Length of output: 2815
🏁 Script executed:
# Search for org deletion logic
rg "\.delete\(\)" --type py -B 3 -A 3 | grep -A 5 -B 5 "org" | head -50
# Search for audit-related comments or requirements
rg "audit|compliance|retention" --type py -i | head -20
# Check if Org model has CASCADE dependencies elsewhere
rg "on_delete=models\.CASCADE" --type py | grep -i orgRepository: DalgoT4D/DDP_backend
Length of output: 8626
Use SET_NULL instead of CASCADE to preserve audit logs when organizations are deleted
Audit logs should survive organization deletion to maintain auditability and compliance. Currently, deleting an org cascades and removes all its audit records. Change on_delete=models.CASCADE to on_delete=models.SET_NULL with null=True, blank=True.
Additionally, update the __str__ method to handle self.org is None, as it currently accesses self.org.slug unconditionally and will fail with AttributeError if the org is deleted.
🤖 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/models/admin_audit_log.py` around lines 14 - 17, Change the org
ForeignKey on the AdminAuditLog model from on_delete=models.CASCADE to
on_delete=models.SET_NULL and add null=True, blank=True so audit records persist
when an Org is deleted; update the AdminAuditLog.__str__ method to guard against
self.org being None (e.g., check if self.org is None and use a fallback string
or "deleted-org" instead of accessing self.org.slug directly) to avoid
AttributeError after an Org is removed.
2 participants
Overview
This PR extends the Admin Organization APIs by adding support for updating organizations and introducing reusable admin schemas for request validation.
Features
PUT /api/admin/v1/organizations/{id}endpointTechnical Details
OrgmodelAPI Endpoint
Update Organization
PUT /api/admin/v1/organizations/{id}Example request:
{ "name": "Updated Org", "slug": "updated-org" }Impact
This improves the Admin Portal backend foundation by enabling organization update workflows and reusable schema-driven validation for future admin APIs.
Summary by CodeRabbit
Release Notes