Skip to content

V120: Document Creator Category → Type taxonomy migration - #545

Merged
ddon merged 4 commits into
BeamLabEU:devfrom
timujinne:feature/v120-doc-categories
May 16, 2026
Merged

V120: Document Creator Category → Type taxonomy migration#545
ddon merged 4 commits into
BeamLabEU:devfrom
timujinne:feature/v120-doc-categories

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

V120 — Document Creator Category → Type taxonomy

Adds migration V120, the schema/DB foundation for the Document
Creator's Category → Type hierarchy (module-side work is a separate PR
on phoenix_kit_document_creator).

What V120 does

  • Creates phoenix_kit_doc_categories and phoenix_kit_doc_types
    tables (UUIDv7 PKs, status soft-delete, position ordering).
  • Adds nullable category_uuid / type_uuid FK columns to
    phoenix_kit_doc_templates and phoenix_kit_doc_documents
    (ON DELETE SET NULL); doc_types.category_uuid is
    ON DELETE CASCADE.
  • Data migration: each distinct legacy template.category string
    becomes a Category row; templates/documents are repointed via FK;
    the legacy category string columns on templates and presets are
    dropped.
  • Bumps @current_version to 120; idempotent up/down.

Notes

  • Uses uuid_generate_v7() for data-migrated rows, per house
    convention (consistent with the schema's UUIDv7 autogenerate).
  • No CHANGELOG.md / @version changes — maintainer-owned.

🤖 Generated with Claude Code

Ubuntu Timdev Managerand others added 3 commits May 16, 2026 13:50
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@timujinnetimujinne left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #545 Review — V120: Document Creator Category → Type taxonomy migration

V120 adds the schema foundation for the Document Creator's Category → Type hierarchy: two new tables, nullable FK columns on templates/documents, a data migration of legacy category strings, and the @current_version bump. The migration is well-structured, idempotent in the common single-schema case, correctly restores the ⚡ LATEST marker (V119 had lost it), and touches no maintainer-owned files. Two correctness issues should be addressed before merge.

Critical

None.

Major

v120.ex:33, v120.ex:53uuid primary key has no default. Both create table blocks declare add(:uuid, :uuid, primary_key: true) with no column default. Every other Document Creator table (v86.ex:22,42,90, v117.ex:54,103) declares the PK as add(:uuid, :uuid, primary_key: true, default: fragment("uuid_generate_v7()")). The Ecto schemas use autogenerate: true so app-level inserts work, but any raw INSERT (including future migrations) hits a NOT NULL violation on uuid. The migration's own data-migration block sidesteps this by computing new_uuid := uuid_generate_v7() explicitly — proof the table has no DB-level default. Add default: fragment("uuid_generate_v7()") to both PK columns for consistency and safety.

v120.ex:62-79, v120.ex:89-95information_schema.columns guards omit table_schema. The DO $$ existence checks filter only on table_name/column_name. PhoenixKit supports multi-prefix (multi-tenant) installs — that is what the prefix parameter is for — and V110, V112, V116, V119 all qualify these guards with WHERE table_schema = '#{schema}'. Without that filter, in a multi-schema install the guard sees the column (or its absence) in any schema: the ADD COLUMN and the data-migration IF EXISTS (... column = 'category') block can be skipped or run for the wrong prefix, silently leaving a tenant un-migrated. Add a schema binding and include table_schema = '#{schema}' in all three guards.

Minor

Data migration uses initcap, spec specifies an explicit map. Spec §4 step 4 says "financial" → "Financial", "technical" → "Technical", others "capitalized". initcap(legacy) matches for those two, but a multi-word legacy value like legal review becomes Legal Review rather than Legal review. Harmless if legacy values are single-word; otherwise consider an explicit CASE. Non-blocking.

v120.ex (down) — category re-added as varchar(255), original was unbounded VARCHAR. V117 created the column as VARCHAR (no length) on both tables (v117.ex:40, v117.ex:106). Down-migrations are best-effort and 255 is ample, but for an exact reversal use unbounded varchar.

Nitpick

  • postgres.ex V120 doc section mixes ### heading with #-prefixed comment lines, whereas V119/V118 use plain Markdown bullet lists under the heading. Cosmetic.
  • The down/0 path relies on Postgres auto-dropping the V117 phoenix_kit_doc_templates_category_index when category is dropped — acceptable, though a strict reversal would restore that index.

VERDICT: REQUEST CHANGES — the two Major items are genuine correctness risks (the table_schema one breaks multi-prefix installs). Both are small, localized fixes.

🤖 Review generated with Claude Code

…, exact legacy mapping
- Add `default: fragment("uuid_generate_v7()")` to both PK columns in
phoenix_kit_doc_categories and phoenix_kit_doc_types tables.
- Add `schema` binding (matching v119 pattern) and `AND table_schema = '#{schema}'`
to all three information_schema.columns guards in up/0 and the one in down/0.
- Replace initcap(legacy) with an explicit CASE map (financial/technical) falling
back to capitalize-first-letter for unknown values.
- Change down/0 ADD COLUMN varchar(255) to unbounded varchar on both templates
and presets, matching the V117 original definition.
- Align V120 doc section in postgres.ex to plain Markdown bullets (V118/V119 style).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@timujinne

Copy link
Copy Markdown
ContributorAuthor

Review fixes applied — 766f8f7c

Both Major items and both Minor items from the review are addressed:

  • PK defaultsphoenix_kit_doc_categories / phoenix_kit_doc_types PKs now declare default: fragment("uuid_generate_v7()").
  • Multi-prefix guards — a schema binding was added to up/0 and down/0; all five information_schema.columns checks now qualify on table_schema.
  • Legacy mappinginitcap replaced with an explicit CASE (financial→Financial, technical→Technical, else first-letter-only).
  • Rollbackdown/0 re-adds category as unbounded varchar.

Re-reviewed and verified — mix compile clean. Verdict updated to APPROVE.

🤖 via Claude Code

@ddon
ddon merged commit 1ae382b into BeamLabEU:devMay 16, 2026
ddon pushed a commit that referenced this pull request May 16, 2026
Trim the speculative "if presets should participate" line from the
moduledoc Note — that decision belongs in the PR #545 review doc, not
in durable migration documentation. Factual notes (presets excluded
from the taxonomy, scope-index recreation) stay.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
timujinne pushed a commit to timujinne/phoenix_kit that referenced this pull request May 17, 2026
…edup
Post-merge fixes to the V120 migration (unreleased — no host app has run
migration 120, so editing in place rather than via a new version):
- Dropping phoenix_kit_doc_template_presets.category also dropped the V117
composite index (scope_type, scope_id, category). up/0 now recreates it on
(scope_type, scope_id); down/0 restores the original 3-column form.
- Data migration grouped legacy values by exact string, so case variants
('Financial'/'financial') produced duplicate Category rows. Now groups by
lower(category) and repoints templates case-insensitively.
- Dropped the redundant standalone [:category_uuid] index on doc_types
(covered by the [:category_uuid, :position] composite).
- Moduledoc notes presets do not join the taxonomy; their legacy category
strings are discarded (open question for the module developer).
Adds the PR BeamLabEU#545 review notes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon pushed a commit that referenced this pull request May 18, 2026
Complete the 1.7.112 entry to cover all unreleased work since v1.7.111:
PR #548 (table_default sort/DnD, bulk_actions_bar, empty_state,
sort_selector, form_section/form_actions, Reorder/Values/Format utils),
PR #544 (MediaBrowser folder management overhaul), PR #545 (V120
document-creator taxonomy), PR #547/#542 (i18n manifests), and this
session's post-merge cleanup. Re-dated to 2026-05-18.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@timujinne
timujinne deleted the feature/v120-doc-categories branch June 25, 2026 08:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@timujinne@ddon