feat: listing CRUD through the service layer - #126
Merged
Conversation
Block 2, PR 2 of 4. One write path -- directory.services.upsert_listing -- for every population route (spec §7.1); the tenant admin is its first caller. - Create / update / unchanged: match on (tenant, listing_type, slug) or on id; a projected diff decides update vs unchanged; an empty patch emits nothing. listing.created carries the full projection, listing.updated carries the JSON Patch, both tagged with the type key. - directory/patch.py: project() renders a Listing into the §4.1 nested shape (not the model); diff() walks it -- per-key scalars, whole-array replace for categories and contact.social, whole-object replace for external_profiles/attributes, per-key for custom_fields. id, status, visibility, tier, owner, timestamps, listing_type and the search vector are absent from the projection, so they never appear in a patch. - directory/normalize.py: trim, "" -> None, lowercase email, E.164 phone, ISO country, 6dp coords -- so a byte-identical re-import is "unchanged". - visibility, tier and status are rejected outright: 422 when the key is present, even set to null. Visibility moves only through set_listing_visibility, which emits listing.published / listing.unpublished only when the published boundary is crossed (draft<->hidden emits nothing). - field_schema.validate_custom_fields: coerces per the type schema, rejects unknown keys, enforces required on create/owner/API (off for CSV), explicit null clears a key. - Idempotency: the key lives in audit.CommandLog. A replay of an applied write returns the original event id (409); a replay of an unchanged write returns no event id (200). The command-log rows are written and committed independently of the command transaction, so a service that raises mid-apply still leaves a row with a NULL outcome (spec §11.2) -- TransactionTestCase covers this. Callers must not wrap upsert_listing in their own transaction; block 5 splits the single row into received/concluded without a migration. - Admin: a Listing form is built per type from its field schema (cf_<key> widgets); the edit view sends the full field set it owns with explicit null for a blanked field, so blanking clears it. New vs edit is EDITOR rank. A create on an existing slug is rejected with a link to that listing, not silently upserted. 58 new tests. Full suite 182, green on Postgres. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L5qqrLcxpVZRis5jbN5pKD Signed-off-by: Matthew Wren <info@origindev.com>
The command-log rows only survive a rolled-back command because they commit independently of it (spec §11.2). A caller that wraps upsert_listing in transaction.atomic() silently pulls those writes into their transaction and loses the guarantee -- so now the call raises MustNotBeInTransaction when transaction.get_connection().in_atomic_block is true. CSV import (block 3) must therefore loop over independent calls, not wrap a batch. ATOMIC_REQUESTS is set to False explicitly in the DATABASES config (with a comment): it is the default, but request-wrapped transactions would break every listing write path, and relying on the default leaves that footgun with no signal. A plain TestCase runs each test in a transaction, so the tests that exercise upsert_listing move to TransactionTestCase; the admin-view tests split into an access class (TestCase, GET only) and a write class (TransactionTestCase). New tests: a call inside the test's transaction, and one inside an explicit atomic(), both raise. Full suite 184, green on Postgres. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L5qqrLcxpVZRis5jbN5pKD Signed-off-by: Matthew Wren <info@origindev.com>
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
1 participant
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.
upsert_listing as the single write path for all four population routes. RFC 6902 patch generation over a fixed §4.1 projection, with normalisation so an identical re-import emits nothing. tier, status and visibility rejected with 422. set_listing_visibility emits published/unpublished only when the published boundary is crossed. Idempotency via the command log, which commits independently of the command transaction and is guarded by a raise if a caller has one open. ATOMIC_REQUESTS pinned False with the reason. Per-type admin forms built from the field schema, sending explicit null so a blanked field clears.