ADR: Remove MEMBERSHIP from Activity Sphere Seed + Tighten DTO Validation

ADR: Remove MEMBERSHIP from Activity Sphere Seed + Tighten DTO Validation

Status

PROPOSED

AC ↔ D# mapping

AC	Primary decisions	Notes
AC-1	D1 (data-fix migration), D2 (audit row), D10 (pre-flight RAISE)	Single atomic transaction. Pre-flight RAISE if default_activity_type=‘MEMBERSHIP’.
AC-2	D3 (@IsIn on CreateSphereDto)	Each-element validation; rejects single garbage in mixed array.
AC-3	D3 + D4 (mirrored on UpdateSphereDto)	Optional fields still validated when present.
AC-4	D5 (@ApiProperty example fix)	Replace ['SLOT_BASED','MEMBERSHIP','SERVICE'] → ['DINING','SERVICE'].
AC-5	D6 (e2e payload migration)	Preserves AC-24 shrink-conflict coverage from spheres-superadmin.e2e-spec.ts:200.
AC-6	D7 (new negative e2e)	One test per validator; not exhaustive permutation.
AC-7	D8 (sync:contracts)	super-admin.openapi.yaml regen only; client/business unaffected at contract level.
AC-8	D9 (4 touch-points: swagger-api.json + patch script + 2 generated files)	All 4 MEMBERSHIP-carrying files in business repo cleaned: edit swagger-api.json directly + patch-spheres-swagger.js:55, then npm run generate:api regenerates the two activity-type-code* files clean.

---

Decisions

D1 — Data-fix migration via Drizzle --custom

Decision: generate a custom migration (no schema diff exists for a data-only change). File: 0044_remove_membership_from_sphere_seed.sql, produced by pnpm db:generate -- --custom --name=remove_membership_from_sphere_seed (the --name flag is required — without it drizzle-kit assigns a random codename; migrations 0035-0043 confirm the named convention). Latest migration is 0043_pg_trgm_search_indexes (journal idx 43), so 0044 is the next slot.

Rationale:

• Memory rule: drizzle-generated migrations only (never hand-written file).
• --custom produces an empty file in the journal/snapshot, into which we paste the data-fix SQL. Snapshot stays consistent (no schema drift).
• Pure data UPDATE — no DDL — so drizzle-kit has nothing to auto-emit.

Alternative rejected: edit migration 0037 directly. Migrations are immutable once shipped; drizzle journal would reject the change.

D2 — Audit row via system actor UUID

Decision: insert one sphere_audit_log row per affected sphere, with actor_user_id = '00000000-0000-0000-0000-000000000000' (nil UUID, “system” actor).

Rationale:

• sphere_audit_log.actor_user_id is uuid notNull but intentionally not a FK (activities.schema.ts:102-104 comment confirms). Any UUID value is accepted.
• Existing audit convention: every sphere mutation is logged. A raw migration would otherwise create a forensic gap.
• Nil UUID is a stable, documented convention for “system” actor — no schema change, no seeded user row required.

Alternative rejected: add system-user FK + seeded row. Out of scope; introduces schema change for what is otherwise a five-line data fix.

D3 — @IsIn(ActivityTypeEnum.enumValues, { each: true }) on allowedActivityTypes

Decision: add @IsIn decorator with element-wise validation to both CreateSphereDto and UpdateSphereDto for the allowedActivityTypes array field.

Rationale:

• Current @IsString({ each: true }) accepts any string. This is how MEMBERSHIP entered via the seed migration path stayed valid against the API (even though the seed bypasses the API entirely, the validator should reject it for any future user-driven create/update too).
• each: true validates per-element so ['SHOW', 'BANANA'] → 400 (not just the array as a whole).
• Pattern already used in the same DTO for targetApp (@IsIn(['GYM_APP','TICKETS_APP','DINING_APP'])).
• Import note: ActivityTypeEnum is not yet imported anywhere in the spheres lib; import it from @tktspace/shared/data-access-db — the alias already used by services/spheres.service.ts:15, so no new lib dependency is introduced.

D4 — @IsIn(ActivityTypeEnum.enumValues) on defaultActivityType

Decision: add same @IsIn decorator on the single-value field.

Rationale:

• Symmetry with D3; closes the same hole.
• Semantic check defaultActivityType ∈ allowedActivityTypes already exists in service layer (spheres.service.ts:77,139 — validateDefaultInAllowed). With @IsIn it becomes layered: format → request 400; semantic → service 400 with domain-specific error code.

D5 — @ApiProperty.example fix in sphere.dto.ts:57

Decision: replace ['SLOT_BASED', 'MEMBERSHIP', 'SERVICE'] with ['DINING', 'SERVICE'] in the example field. Also add enum: ActivityTypeEnum.enumValues, enumName: 'ActivityType', isArray: true so the regenerated contract emits a named enum reference instead of type: string.

Rationale:

• Example flows directly into super-admin.openapi.yaml via sync:contracts. The legacy value was documenting itself as valid.
• enumName: 'ActivityType' follows the OpenAPI authoring rule in CLAUDE.md (“Every named enum carries enumName: ‘X’ alongside enum: X”) — its absence is documented as a defect in the same checklist.

D6 — e2e payload migration (AC-24 preservation)

Decision: in spheres-superadmin.e2e-spec.ts:200, replace ['MEMBERSHIP', 'SERVICE'] with ['DINING', 'SERVICE']. The test still triggers the AC-24 shrink-conflict path because the assertion is about the conflict mechanism, not the specific types.

Rationale: without this fix, applying D3 turns the existing positive setup into a 400 and the AC-24 coverage silently disappears. The critic explicitly flagged this risk.

D7 — New negative e2e for @IsIn

Decision: one new test: POST /api/superadmin/spheres with allowedActivityTypes: ['SHOW', 'BANANA'] → 400 + response body shape matches class-validator standard error.

Rationale: D3 is the new behaviour; without a test it can regress silently. One element-wise case is enough; we don’t need to enumerate every combination.

D8 — pnpm run sync:contracts regen of super-admin.openapi.yaml

Decision: after DTO edits land, run the script; commit diff alongside backend code.

Rationale: code-first contract model (CLAUDE.md). YAML is a snapshot of live OpenAPI; hand-editing would drift.

D9 — Business client cleanup (swagger-api.json + patch script + regen)

Decision: in tktspace-business, fix all 4 MEMBERSHIP touch-points + regenerate. The flow is NOT “regen auto-cleans” — manual edits to two input files are required.

Step 1 — Edit tools/gen/swagger-api.json (committed file, NOT auto-generated, NOT auto-fetched):

• Locate components.schemas.ActivityTypeCode.enum (currently lines ~9316-9325, value: ["SHOW", "MOVIE", "SLOT_BASED", "DINING", "SERVICE", "MEMBERSHIP"]).
• Remove "MEMBERSHIP" from that array.

Step 2 — Edit tools/gen/patch-spheres-swagger.js:55:

• Remove 'MEMBERSHIP' from the hardcoded ActivityTypeCode.enum array, leaving ['SHOW', 'MOVIE', 'SLOT_BASED', 'DINING', 'SERVICE'].
• Why even though ensure() is absent-only? If someone later deletes ActivityTypeCode from swagger-api.json and re-runs node tools/gen/patch-spheres-swagger.js, the stale value would be re-injected. Keeping the patch script consistent is durability insurance.

Step 3 — Regenerate: npm run generate:api (= ng-openapi-gen -c tools/gen/api-gateway.json, see package.json:19). The patch script is NOT invoked by this command — it’s a separate manual step. ng-openapi-gen now reads the cleaned swagger-api.json and produces core/api/models/activity-type-code.ts + activity-type-code-array.ts without MEMBERSHIP.

The 4 MEMBERSHIP touch-points (verified — all enumerated):

1. tools/gen/swagger-api.json — generator input (Step 1 fix)
2. tools/gen/patch-spheres-swagger.js:55 — patch script source (Step 2 fix)
3. client/src/app/core/api/models/activity-type-code.ts — generated output (cleaned by Step 3)
4. client/src/app/core/api/models/activity-type-code-array.ts — generated output (cleaned by Step 3)

Why the patch script exists at all (out of scope): historical workaround for sphere endpoints not surfacing in the business contract early in activity-spheres rollout. Retiring it entirely (once business.openapi.yaml carries proper sphere schemas) is a separate cleanup ticket — this ADR only touches the MEMBERSHIP fix.

Note: web (tktspace-web) and mobile (tktspace-mobile-app) do not consume sphere DTOs, so no regen needed there.

Verification of AC-8: grep -rn MEMBERSHIP tktspace-business/client/src/app/core/api/models/ tktspace-business/tools/gen/ returns zero matches.

D10 — Pre-flight RAISE on default_activity_type = 'MEMBERSHIP'

Decision: the migration starts with a DO $$ block that RAISEs EXCEPTION if any sphere has default_activity_type = 'MEMBERSHIP'.

Rationale:

• The validator (@IsIn) was absent, so although seed migrations only put MEMBERSHIP into allowed_activity_types (not default_activity_type), nothing in the schema prevented a manual UPDATE from setting it as default. Pre-flight makes the assumption (“expected zero”) explicit and refuses to proceed otherwise — fail-loud is cheaper than silent data corruption.
• PROD action item: run SELECT count(*) FROM activities.spheres WHERE default_activity_type = 'MEMBERSHIP' manually before deployment. If non-zero, escalate to product for replacement type before merging.

---

Out of scope (explicitly deferred)

• DB-level enum conversion (text[] → activity_type[]) — tracked separately. Closes the SQL-path drift this ADR does not close.
• MEMBERSHIP as a first-class activity type — rejected. It’s a pass type (wallet.schema.ts), modelled and sold through a different flow. Adding it to ActivityTypeEnum would create a second source of truth.
• Backfill audit row for original 0037 seed insert — historical, no operational value.
• Seed-lint CI guard — not worth the infrastructure for one cleanup; the type-safety follow-up covers the same surface more robustly.

---

Risks (post-mitigation)

• None blocking. Data fix is on a small table (5 sphere rows in prod); audit trail preserved; rollback trivial.
• Backward compat: generated client regen drops MEMBERSHIP from the ActivityTypeCode enum. Any consumer hand-coded against that value would break — grep confirms no hand-references exist in tktspace-business/client/src/app/ (the stale file is only imported within core/api/, never referenced from app code).

---

Verification

1. Run pnpm db:generate -- --custom --name=remove_membership_from_sphere_seed to produce 0044_remove_membership_from_sphere_seed.sql (empty file + journal entry; snapshot copied unchanged — no schema diff). Paste the migration body from the spec into the generated file.
2. Apply locally; verify SELECT allowed_activity_types FROM activities.spheres WHERE code='SPORT' no longer contains MEMBERSHIP.
3. Verify one new row in activities.sphere_audit_log with system actor.
4. Apply DTO edits; run pnpm run sync:contracts; diff super-admin.openapi.yaml.
5. Run e2e (pnpm e2e:api) — confirm modified AC-24 test passes and new negative test passes.
6. In tktspace-business: apply D9 Steps 1-2 (edit tools/gen/swagger-api.json + tools/gen/patch-spheres-swagger.js:55 to remove MEMBERSHIP), then D9 Step 3 (npm run generate:api). Verify: grep -rn MEMBERSHIP client/src/app/core/api/models/ tools/gen/ returns zero matches.

STATUS: READY_FOR_REVIEW