* feat: UUIDs as person identifiers
* feat: person uuid oidc claims and apis
* chore: ruff ruff
* fix: adjust how push is triggered
* fix: keep the mypy ignore on the person model import
Reformatting the import into a parenthesized block moved the ignore comment to
the closing paren. mypy reports the simple_history HistoricalPerson and
HistoricalEmail attribute errors against the 'from ... import (' line, so the
comment has to sit there to suppress them.
* refactor: register the anycase_uuid converter in the root URLconf
Registering it in ietf/utils/converters.py made importing that module a side
effect, and Django refuses to register a converter twice, so naming the
converter from a second URLconf was a latent error. Define it there, register it
once in ietf/urls.py before urlpatterns names it.
* fix: create a Person and its primary UUID atomically
A Person with no primary UUID cannot be named to any external system, so the
create and the assign_primary_uuid() that follows it have to succeed or fail
together. Covers all three production creation sites, including the draft
submission one, and wraps the surrounding aliases and nominee email so a
failure part way leaves nothing half-built.
* refactor: give each UUID batch endpoint a single response serializer
The resolved/unknown split needed a PolymorphicProxySerializer, which is an
annotation helper rather than a real serializer, so the endpoints hand-built
dicts and told consumers not to infer the outcome from which fields were
present. Use one entry serializer per endpoint instead, discriminated on status,
with the identifier fields nullable and always present, and actually serialize
responses through it so the schema cannot drift from what is returned.
Drops the ResolvedStatusEnum/UnknownStatusEnum overrides that existed only to
keep the two single-valued status enums apart - there is now one StatusEnum. The
entry fields are not read_only because read_only implies required=False, which
left a generated client treating even status as optional.
Also annotates retrieve with @extend_schema_view rather than overriding it just
to call super().
* refactor: serve the pk-to-UUID conversion from a plain APIView
Routing this lookup through a GenericViewSet forced the handler to be named
create, because that is what SimpleRouter maps POST to on a collection route.
Nothing is created: the view returned 200 while drf-spectacular inferred 201
from the action name, so the schema advertised a status code the endpoint never
sends and a generated client would treat the real response as unexpected.
An APIView.post() returns 200 with no annotation gymnastics. The viewset was
buying nothing else - no retrieve, no mixins, and an empty queryset. api_key
auth is unaffected, since HasApiKey just reads api_key_endpoint off the view.
The URL is unchanged. Its name loses the router's -list suffix, and the schema
test now checks the declared success codes so this cannot drift again.
* feat: carry both UUID claims in one OIDC scope
Splitting the current identifier and the superseded ones across two scopes was
finer-grained than any consumer needs - there is no case for granting one and
not the other, and the prior list is far too short for response size to matter.
Also corrects the scope description, which claimed the prior list included the
identifier in use now. It does not, and datatracker_uuid is where that lives.
* fix: check for exactly one primary UUID, not just one or more
The job logged that every Person has exactly one primary while only looking for
Persons with none. The partial unique constraint should make more than one
impossible, so finding one means the data is grossly inconsistent and worth
reporting - and ensure_primary_uuid() cannot repair that case, since it would be
picking a survivor arbitrarily, so it is reported and skipped rather than
silently 'fixed'. Same change in the base-test-data check.
* fix: let the UUID push enqueue use the default retry policy
Celery's default is three attempts over well under a second, which is cheap
enough on the request path that changed the UUID set and is the difference
between riding out a broker blip or failover and dropping the push on the floor.
The broker-error catch still keeps an outright outage from failing the
datatracker operation.
* chore: add dev API tokens for the person UUID endpoints
Neither endpoint had an APP_API_TOKENS entry in the container config, so every
call to them from a dev environment got a 403.
* test: build Person UUIDs with the factories and read them through the accessors
PersonFactory now makes its Person's primary UUID with PersonUUIDFactory instead
of calling assign_primary_uuid() itself, so all UUID handling in tests goes
through the factories. PersonFactory(primary_uuid=False) covers the
no-UUIDs-at-all case, which no production path can reach, replacing the tests
that created a Person and then deleted its UUID rows.
Tests now assert through Person.primary_uuid and Person.prior_uuids rather than
querying uuids directly, so the accessors are the example to copy. Direct
queries remain only where they are the point: the test proving the accessors
agree with the rows, and setup that deliberately builds inconsistent state.
Also drops the retry kwarg assertion that went with the old retry=False.
* fix: order prior_uuids deterministically
A merge stamps every UUID it moves with the same time, so ordering the prior list
on time alone left the order undefined in exactly the case where there is more
than one prior. Break ties on the UUID, which also makes the claim that
uuid_sets_for() matches this accessor true - it was already ordering on both.
* docs: correct why prior_uuids breaks ties on the uuid
The previous comment justified the tie-break by claiming a merge gives every
UUID it moves the same timestamp. It does not: merge_persons() moves them with a
queryset update that names only person and primary, and PersonUUID.time is a
per-row default with no auto_now, so each keeps its original timestamp.
The tie-break stands on narrower ground - it makes the order total instead of
leaving equal timestamps to the database, and matches the ordering uuid_sets_for()
already used - so only the comment changes.
* test: clear over-zealous concerns about API return values
---------
Co-authored-by: Jennifer Richards <jennifer@staff.ietf.org>
Resolves #11221
Commit history pre-squash
feat: UUIDs as person identifiers
feat: person uuid oidc claims and apis
chore: ruff ruff
fix: adjust how push is triggered
fix: keep the mypy ignore on the person model import
Reformatting the import into a parenthesized block moved the ignore comment to the closing paren. mypy reports the simple_history HistoricalPerson and HistoricalEmail attribute errors against the 'from ... import (' line, so the comment has to sit there to suppress them.
Registering it in ietf/utils/converters.py made importing that module a side effect, and Django refuses to register a converter twice, so naming the converter from a second URLconf was a latent error. Define it there, register it once in ietf/urls.py before urlpatterns names it.
A Person with no primary UUID cannot be named to any external system, so the create and the assign_primary_uuid() that follows it have to succeed or fail together. Covers all three production creation sites, including the draft submission one, and wraps the surrounding aliases and nominee email so a failure part way leaves nothing half-built.
The resolved/unknown split needed a PolymorphicProxySerializer, which is an annotation helper rather than a real serializer, so the endpoints hand-built dicts and told consumers not to infer the outcome from which fields were present. Use one entry serializer per endpoint instead, discriminated on status, with the identifier fields nullable and always present, and actually serialize responses through it so the schema cannot drift from what is returned.
Drops the ResolvedStatusEnum/UnknownStatusEnum overrides that existed only to keep the two single-valued status enums apart - there is now one StatusEnum. The entry fields are not read_only because read_only implies required=False, which left a generated client treating even status as optional.
Also annotates retrieve with @extend_schema_view rather than overriding it just to call super().
Routing this lookup through a GenericViewSet forced the handler to be named create, because that is what SimpleRouter maps POST to on a collection route. Nothing is created: the view returned 200 while drf-spectacular inferred 201 from the action name, so the schema advertised a status code the endpoint never sends and a generated client would treat the real response as unexpected.
An APIView.post() returns 200 with no annotation gymnastics. The viewset was buying nothing else - no retrieve, no mixins, and an empty queryset. api_key auth is unaffected, since HasApiKey just reads api_key_endpoint off the view.
The URL is unchanged. Its name loses the router's -list suffix, and the schema test now checks the declared success codes so this cannot drift again.
Splitting the current identifier and the superseded ones across two scopes was finer-grained than any consumer needs - there is no case for granting one and not the other, and the prior list is far too short for response size to matter.
Also corrects the scope description, which claimed the prior list included the identifier in use now. It does not, and datatracker_uuid is where that lives.
The job logged that every Person has exactly one primary while only looking for Persons with none. The partial unique constraint should make more than one impossible, so finding one means the data is grossly inconsistent and worth reporting - and ensure_primary_uuid() cannot repair that case, since it would be picking a survivor arbitrarily, so it is reported and skipped rather than silently 'fixed'. Same change in the base-test-data check.
Celery's default is three attempts over well under a second, which is cheap enough on the request path that changed the UUID set and is the difference between riding out a broker blip or failover and dropping the push on the floor. The broker-error catch still keeps an outright outage from failing the datatracker operation.
Neither endpoint had an APP_API_TOKENS entry in the container config, so every call to them from a dev environment got a 403.
PersonFactory now makes its Person's primary UUID with PersonUUIDFactory instead of calling assign_primary_uuid() itself, so all UUID handling in tests goes through the factories. PersonFactory(primary_uuid=False) covers the no-UUIDs-at-all case, which no production path can reach, replacing the tests that created a Person and then deleted its UUID rows.
Tests now assert through Person.primary_uuid and Person.prior_uuids rather than querying uuids directly, so the accessors are the example to copy. Direct queries remain only where they are the point: the test proving the accessors agree with the rows, and setup that deliberately builds inconsistent state.
Also drops the retry kwarg assertion that went with the old retry=False.
A merge stamps every UUID it moves with the same time, so ordering the prior list on time alone left the order undefined in exactly the case where there is more than one prior. Break ties on the UUID, which also makes the claim that uuid_sets_for() matches this accessor true - it was already ordering on both.
The previous comment justified the tie-break by claiming a merge gives every UUID it moves the same timestamp. It does not: merge_persons() moves them with a queryset update that names only person and primary, and PersonUUID.time is a per-row default with no auto_now, so each keeps its original timestamp.
The tie-break stands on narrower ground - it makes the order total instead of leaving equal timestamps to the database, and matches the ordering uuid_sets_for() already used - so only the comment changes.