Django code review standards

# Django code review standards

Follow these standards when writing or changing code in this repository. Code review flags changes that break them.

## Prevent N+1 Queries in Django

Applies to `**/*.py`, `**/templates/**/*.html`.

Review Django ORM access in views, Django REST Framework serializers, and templates for queries that run once per row.

Flag changes that:
- Read a `ForeignKey` or `OneToOneField` relation (`order.customer.name`) on each object in a loop, serializer, or template when the queryset has no matching `select_related()`.
- Iterate a reverse `ForeignKey` or a `ManyToManyField` (`order.items.all()`, `book.authors.all()`) per object without `prefetch_related()` on the parent queryset.
- Call `.count()`, `.exists()`, or `.aggregate()` on a related manager per row for a relation that is not prefetched, instead of annotating the parent queryset with `Count`, `Exists`, `Sum`, or `Subquery`.
- Add nested serializers, `source='relation.field'` fields, or `SerializerMethodField` methods that read relations the view's `get_queryset()` does not load.
- Call `.filter()`, `.exclude()`, or `.order_by()` on a prefetched relation inside a loop, which ignores the prefetch cache and queries again; use `Prefetch(queryset=..., to_attr=...)` instead.
- Call `.get()` or `.filter()` inside a loop over IDs instead of one `filter(pk__in=...)` or `in_bulk()` query.

Do not flag:
- Reading the foreign key column (`order.customer_id`), which does not query.
- Code that handles one object or a small fixed number of objects, such as reading `order.customer` once in a detail view.
- Relations already loaded by a custom manager, a shared `get_queryset()`, or the queryset the caller passes in.

## Scope Django Querysets to the Current User or Tenant

Applies to `**/*.py`.

Review Django views, Django REST Framework viewsets, and serializers for object access that is not scoped to the requesting user or tenant.

Flag changes that:
- Fetch a user- or tenant-owned model by ID with `Model.objects.get()`, `get_object_or_404(Model, ...)`, or `filter(pk=...)` without also filtering on `request.user` or the tenant.
- Set `model = Model` or `queryset = Model.objects.all()` on a `DetailView`, `UpdateView`, `DeleteView`, or DRF viewset for owned data without overriding `get_queryset()` to filter by the requesting user or tenant.
- Load objects directly in DRF custom actions instead of calling `self.get_object()`, which applies `get_queryset()` scoping and `has_object_permission()` checks, or override `get_object()` without calling `self.check_object_permissions()`.
- Rely on `IsAuthenticated`, `LoginRequiredMixin`, or `@login_required` as the only protection for objects that belong to a specific user or tenant.
- Accept related IDs through `PrimaryKeyRelatedField(queryset=Model.objects.all())` or a similar field, including the fields `ModelSerializer` generates for writable foreign keys, which lets a caller attach another tenant's objects.
- Update or delete with `Model.objects.filter(pk=...).update()` or `.delete()` without the ownership filter.

Do not flag:
- Models that are public by design, such as published articles or a product catalog.
- Lookups through a scoped relation or manager, such as `request.user.invoices.get(pk=pk)` or `get_object_or_404(request.user.invoices, pk=pk)`.
- Views limited to staff or superusers, such as Django admin `ModelAdmin` classes or views guarded by `is_staff` checks.

## Enforce Zero-Downtime Migrations in Django

Applies to `**/migrations/*.py`.

Review Django migrations for operations that lock or rewrite large tables or break the release still serving traffic during a deploy.

Flag changes that:
- Add a non-nullable field to an existing table without `db_default`, including `AddField` with a one-off default and `preserve_default=False`; Django drops the column default afterwards, so inserts from the previous release fail.
- Make an existing column non-nullable or change its type with `AlterField` (for example `IntegerField` to `BigIntegerField`) on a large table, which scans or rewrites it under an ACCESS EXCLUSIVE lock.
- Add an index to an existing PostgreSQL table through `AddIndex`, `db_index=True`, or a new `ForeignKey` (indexed by default) instead of `AddIndexConcurrently` in a migration with `atomic = False`.
- Remove or rename a field or model (`RemoveField`, `RenameField`, `DeleteModel`, `RenameModel`) in the same release as the code change, instead of making the column nullable, removing it from model state with `SeparateDatabaseAndState`, and dropping it in a later release.
- Import models directly in `RunPython` instead of `apps.get_model()`, or backfill a large table row by row with `save()` inside the migration's transaction.

Do not flag:
- Migrations that create new tables, or alter tables created earlier in the same pull request.
- New unindexed columns that are nullable or have a constant `db_default` (PostgreSQL 11 or later).
- Increasing a `CharField`'s `max_length`, which PostgreSQL applies without rewriting the table.
- The `AddIndexConcurrently` requirement in projects that do not use PostgreSQL.

## Make Django Writes Atomic and Run Side Effects on Commit

Applies to `**/*.py`.

Review Django code that writes to the database for missing transactions and for side effects that run before the data they depend on is committed.

Flag changes that:
- Perform several related writes (`create()`, `save()`, `update()`, `delete()`, `bulk_create()`) that must succeed or fail together without `transaction.atomic`, when `ATOMIC_REQUESTS` does not cover the code path.
- Call Celery `.delay()` or `.apply_async()`, `send_mail()` or `EmailMessage.send()`, or publish webhooks and events inside an atomic block or an `ATOMIC_REQUESTS` view instead of through `transaction.on_commit()`.
- Make slow or irreversible external calls, such as payment charges or third-party HTTP requests, inside `transaction.atomic`, which holds locks during network I/O and cannot be undone by a rollback.
- Read a value, change it in Python, and `save()` it back (balances, stock counts, counters) without an `F()` expression or `select_for_update()` inside `transaction.atomic`.
- Catch `IntegrityError` or `DatabaseError` inside an atomic block without wrapping the failing query in its own nested `transaction.atomic()`, which leaves the outer transaction unusable.

Do not flag:
- A single ORM call such as one `save()`, `QuerySet.update()`, `delete()`, or `bulk_create()`, which Django applies atomically on its own.
- Side effects already deferred with `transaction.on_commit()` or Celery's `delay_on_commit()`.
- Logging and metrics calls, which are harmless if the transaction rolls back.

## Prevent SQL Injection in Django Raw Queries

Applies to `**/*.py`.

Review Django code that runs SQL outside the ORM's query builder for values interpolated into the SQL string.

Flag changes that:
- Build the SQL passed to `Model.objects.raw()`, `RawSQL()`, `cursor.execute()`, or `cursor.executemany()` with f-strings, `%` formatting, `.format()`, or concatenation of variable values instead of the `params` argument.
- Pass interpolated strings to `extra(where=..., select=..., tables=...)` instead of `params` and `select_params`, or add new `extra()` calls, which Django's documentation calls a last resort it aims to deprecate.
- Wrap placeholders in quotes (`'%s'`); the driver quotes parameter values itself, and Django's documentation warns against it.
- Insert table names, column names, or `ORDER BY` clauses from request data without mapping them through a fixed allowlist, since identifiers cannot be bound as parameters.
- Format `LIKE` wildcards or `IN` lists into the SQL instead of putting wildcards in the parameter value and generating one `%s` per list item.
- Pass request data as field names or lookup keys to `filter()`, `exclude()`, `order_by()`, or `values()`, such as `filter(**request.GET.dict())`, which lets callers query any field or relation, including password hashes on related users.

Do not flag:
- Placeholder lists built from a count, such as `", ".join(["%s"] * len(ids))`, with the values passed in `params`.
- Constant SQL fragments or identifiers interpolated from code-defined constants or an allowlist.
- `RunSQL` operations in migrations that contain only static SQL.

## Flag Security Vulnerabilities

Scan for security flaws: hardcoded secrets, SQL injection, XSS, weak auth, exposed data, poor crypto, unsafe deserialization, insecure HTTP calls in production, and more. Ensure all network requests use HTTPS/TLS. Identify attack vectors and security violations to maintain robust defenses.

## Require API Auth, Validation, and Tenant Scoping

Review API handlers and controller endpoints for missing authentication, authorization, tenant scoping, and request validation.

Flag new or modified endpoints that:
- Accept request bodies without runtime validation, such as Zod schemas, DTOs, or framework-native validators.
- Trust user-supplied identifiers for tenant, organization, or account access instead of deriving access from the authenticated session.
- Return sensitive internal errors or use inconsistent HTTP status codes for 400, 401, 403, 404, and 500 cases.
- Mix reads and writes in a way that makes side effects hard to reason about.
- Add public or high-volume endpoints without rate limiting or abuse controls.

## Block Unsafe Database Migrations

Review database migrations for operations that can lock large tables, rewrite full tables, run long backfills, create blocking indexes, or require full table scans. Prefer phased rollouts, concurrent indexes, nullable columns followed by backfills, batched data changes, and explicit rollback or recovery plans for risky schema changes.

## Prevent Database Query Performance Regressions

Flag database access patterns that can degrade production performance, including N+1 queries, unbounded list endpoints, missing pagination, full-table scans, missing indexes on selective filters, per-row writes in loops, and queries that load more columns or rows than needed. Prefer bounded reads, batching, selective indexes, and query shapes that scale with table size.

## Prevent Sensitive Data in Logs

Logging, error reporting, analytics events, and audit trails must not expose sensitive data. Flag code that logs PII, emails, passwords, API keys, auth tokens, session identifiers, payment data, or raw request/response bodies without redaction. Error serialization must preserve enough context to debug while sanitizing secrets and user data.

## Enforce Rollback Steps in Database Migrations

Each database migration file must include a "down" or "rollback" section that reverses the changes. This ensures all database changes can be safely undone if deployment issues occur. Migration files should clearly separate "up" and "down" migrations using appropriate method names or comments.

## Enforce Pragmatic Test Coverage

Be reasonable when evaluating tests. Flag changes when important behavior is untested, but do not require tests for every export, type, config, metadata file, or mechanical change.

Flag PRs that:
- Change core business logic without covering the main success and failure paths.
- Add UI behavior without testing the user-visible interaction or state change.
- Add tests that only assert rendering, existence, snapshots, or that a function returns any value.
- Skip, mute, or weaken existing tests without a clear reason.
- Mock so much of the system that the test no longer validates the behavior the PR claims to change.