From 7cea8ab318573c93ceef30c1eae7c96b63b433e3 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sun, 8 Nov 2026 20:29:31 +0100 Subject: [PATCH] feat: move zjazd tags to the material's meeting The one-way migration 20261108140000_move_zjazd_tags_to_meetings gives every material tagged zjazd-N without a meeting meeting N (the lowest when it has several), never overwrites a meeting_id, logs tags naming a missing meeting or disagreeing with meeting_id at Warn, and deletes the exact zjazd-N tags; their links go through ON DELETE CASCADE. Rollback is a no-op: the backup is the pg_dump taken before migrate. --- plugin_test.go | 4 +- registry.gen.go | 1 + ...61108140000_move_zjazd_tags_to_meetings.go | 111 ++++++++++++++++ updates/move_zjazd_tags_internal_test.go | 85 ++++++++++++ zjazd_migration_postgres_test.go | 121 ++++++++++++++++++ 5 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 updates/20261108140000_move_zjazd_tags_to_meetings.go create mode 100644 updates/move_zjazd_tags_internal_test.go create mode 100644 zjazd_migration_postgres_test.go diff --git a/plugin_test.go b/plugin_test.go index 3bebadc..d78bd63 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -79,8 +79,8 @@ func TestPluginActivates(t *testing.T) { if n := len(p.Models()); n != 14 { t.Errorf("Models = %d, want 14", n) } - if n := len(p.Migrations()); n != 20 { - t.Errorf("Migrations = %d, want 20", n) + if n := len(p.Migrations()); n != 21 { + t.Errorf("Migrations = %d, want 21", n) } if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:seed" }) { t.Error("bm:seed is not registered") diff --git a/registry.gen.go b/registry.gen.go index 013ef19..55d9021 100644 --- a/registry.gen.go +++ b/registry.gen.go @@ -52,6 +52,7 @@ func generatedMigrations() []*gormigrate.Migration { updates.CreateChecklistItems(), updates.AddPrivacyToMaterials(), updates.CreateMaterialFileTexts(), + updates.MoveZjazdTagsToMeetings(), } } diff --git a/updates/20261108140000_move_zjazd_tags_to_meetings.go b/updates/20261108140000_move_zjazd_tags_to_meetings.go new file mode 100644 index 0000000..62af68d --- /dev/null +++ b/updates/20261108140000_move_zjazd_tags_to_meetings.go @@ -0,0 +1,111 @@ +package updates + +import ( + "git.golem15.com/golem15/summercms/modules/diary" + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// The slug pattern is capped at six digits so the CAST to INTEGER can never +// overflow, and it is anchored so a topic slug such as zjazd-3-24-10 is not +// taken for a zjazd. None of the statements holds a semicolon (execAll). +const ( + moveZjazdReportSQL = `SELECT mt.material_id, t.slug, m.meeting_id, me.id AS tag_meeting_id, + COUNT(*) OVER (PARTITION BY mt.material_id) AS zjazd_tags +FROM jz_bm_material_tag mt +JOIN jz_bm_tags t ON t.id = mt.tag_id +JOIN jz_bm_materials m ON m.id = mt.material_id +LEFT JOIN jz_bm_meetings me ON me.number = CAST(substring(t.slug FROM '^zjazd-([0-9]{1,6})$') AS INTEGER) +WHERE t.slug ~ '^zjazd-[0-9]{1,6}$' +ORDER BY mt.material_id, t.slug` + + moveZjazdAssignSQL = `WITH tagged AS ( + SELECT DISTINCT ON (mt.material_id) mt.material_id, me.id AS meeting_id + FROM jz_bm_material_tag mt + JOIN jz_bm_tags t ON t.id = mt.tag_id + JOIN jz_bm_meetings me ON me.number = CAST(substring(t.slug FROM '^zjazd-([0-9]{1,6})$') AS INTEGER) + WHERE t.slug ~ '^zjazd-[0-9]{1,6}$' + ORDER BY mt.material_id, me.number +) +UPDATE jz_bm_materials m SET meeting_id = tagged.meeting_id +FROM tagged WHERE m.id = tagged.material_id AND m.meeting_id IS NULL` + + moveZjazdDeleteSQL = `DELETE FROM jz_bm_tags WHERE slug ~ '^zjazd-[0-9]{1,6}$'` +) + +// zjazdTagLink is one link between a material and a zjazd-N tag, with the +// material's current meeting and the meeting the tag names (nil when there +// is no meeting with that number). +type zjazdTagLink struct { + MaterialID int64 `gorm:"column:material_id"` + Slug string `gorm:"column:slug"` + MeetingID *int64 `gorm:"column:meeting_id"` + TagMeetingID *int64 `gorm:"column:tag_meeting_id"` + ZjazdTags int64 `gorm:"column:zjazd_tags"` +} + +// MoveZjazdTagsToMeetings returns the +// 20261108140000_move_zjazd_tags_to_meetings gormigrate entry: the zjazd of +// a material is its meeting_id, so every material tagged zjazd-N that has no +// meeting gets meeting N, and the zjazd-N tags are deleted (their +// jz_bm_material_tag rows go with them through ON DELETE CASCADE). An +// existing meeting_id is never overwritten. A tag naming a missing meeting, +// a tag that disagrees with meeting_id and a material with several zjazd +// tags are logged at Warn; the material then keeps its meeting, gets none, +// or gets the lowest zjazd. +// +// The migration is one-way, so Rollback does nothing: the deleted tags and +// their links can only be restored from the pg_dump --data-only of +// jz_bm_tags and jz_bm_material_tag taken before migrate (DEPLOY.md), and +// the meeting_id values it set stay, because they are correct. +func MoveZjazdTagsToMeetings() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20261108140000_move_zjazd_tags_to_meetings", + Migrate: func(tx *gorm.DB) error { + log := diary.For("jz.bm") + var links []zjazdTagLink + if err := tx.Raw(moveZjazdReportSQL).Find(&links).Error; err != nil { + return err + } + for _, l := range links { + args := []any{ + "material_id", l.MaterialID, + "slug", l.Slug, + "meeting_id", optionalID(l.MeetingID), + "tag_meeting_id", optionalID(l.TagMeetingID), + } + switch { + case l.TagMeetingID == nil: + log.Warn("zjazd tag has no meeting; the tag is removed", args...) + case l.MeetingID != nil && *l.MeetingID != *l.TagMeetingID: + log.Warn("zjazd tag disagrees with meeting_id; meeting_id is kept", args...) + } + if l.ZjazdTags > 1 && l.MeetingID == nil { + log.Warn("material has several zjazd tags; the lowest zjazd is used", args...) + } + } + assigned := tx.Exec(moveZjazdAssignSQL) + if assigned.Error != nil { + return assigned.Error + } + log.Info("zjazd tags moved to meeting_id", "materials", assigned.RowsAffected) + deleted := tx.Exec(moveZjazdDeleteSQL) + if deleted.Error != nil { + return deleted.Error + } + log.Info("zjazd tags deleted", "tags", deleted.RowsAffected) + return nil + }, + Rollback: func(tx *gorm.DB) error { + return nil + }, + } +} + +// optionalID is the logged form of a nullable id: the number, or nil. +func optionalID(id *int64) any { + if id == nil { + return nil + } + return *id +} diff --git a/updates/move_zjazd_tags_internal_test.go b/updates/move_zjazd_tags_internal_test.go new file mode 100644 index 0000000..25cac22 --- /dev/null +++ b/updates/move_zjazd_tags_internal_test.go @@ -0,0 +1,85 @@ +package updates + +import ( + "slices" + "strings" + "testing" + + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +const zjazdSlugPattern = `^zjazd-[0-9]{1,6}$` + +func TestMoveZjazdTagsSQL(t *testing.T) { + statements := map[string]string{ + "report": moveZjazdReportSQL, + "assign": moveZjazdAssignSQL, + "delete": moveZjazdDeleteSQL, + } + for name, sql := range statements { + if strings.Contains(sql, ";") { + t.Errorf("%s statement holds a semicolon", name) + } + if !strings.Contains(sql, zjazdSlugPattern) { + t.Errorf("%s statement does not match slugs by %s", name, zjazdSlugPattern) + } + } + for _, want := range []string{"m.meeting_id IS NULL", "DISTINCT ON (mt.material_id)", "ORDER BY mt.material_id, me.number"} { + if !strings.Contains(moveZjazdAssignSQL, want) { + t.Errorf("assign statement lacks %q", want) + } + } + if !strings.HasPrefix(moveZjazdDeleteSQL, "DELETE FROM jz_bm_tags WHERE ") || strings.Count(moveZjazdDeleteSQL, "jz_bm_") != 1 { + t.Errorf("delete statement must target jz_bm_tags only: %s", moveZjazdDeleteSQL) + } + if !strings.HasPrefix(moveZjazdReportSQL, "SELECT ") { + t.Errorf("report statement must only read: %s", moveZjazdReportSQL) + } +} + +// recordingDB is a gorm DryRun postgres handle that needs no database and +// records the SQL of every Raw, Exec and Find it is given. +func recordingDB(t *testing.T) (*gorm.DB, *[]string) { + t.Helper() + db, err := gorm.Open(postgres.New(postgres.Config{DSN: "host=127.0.0.1"}), &gorm.Config{DryRun: true, DisableAutomaticPing: true}) + if err != nil { + t.Fatal(err) + } + var recorded []string + record := func(tx *gorm.DB) { recorded = append(recorded, tx.Statement.SQL.String()) } + if err := db.Callback().Raw().After("gorm:raw").Register("test:record_raw", record); err != nil { + t.Fatal(err) + } + if err := db.Callback().Query().After("gorm:query").Register("test:record_query", record); err != nil { + t.Fatal(err) + } + if err := db.Callback().Row().After("gorm:row").Register("test:record_row", record); err != nil { + t.Fatal(err) + } + return db, &recorded +} + +func TestMoveZjazdTagsDryRun(t *testing.T) { + m := MoveZjazdTagsToMeetings() + if m.ID != "20261108140000_move_zjazd_tags_to_meetings" { + t.Errorf("ID = %q", m.ID) + } + + db, recorded := recordingDB(t) + if err := m.Migrate(db); err != nil { + t.Fatalf("Migrate: %v", err) + } + want := []string{moveZjazdReportSQL, moveZjazdAssignSQL, moveZjazdDeleteSQL} + if !slices.Equal(*recorded, want) { + t.Errorf("Migrate ran %d statements in this order:\n%s\nwant report, assign, delete", len(*recorded), strings.Join(*recorded, "\n---\n")) + } + + db, recorded = recordingDB(t) + if err := m.Rollback(db); err != nil { + t.Errorf("Rollback: %v, want nil (one-way migration)", err) + } + if len(*recorded) != 0 { + t.Errorf("Rollback ran %v, want nothing", *recorded) + } +} diff --git a/zjazd_migration_postgres_test.go b/zjazd_migration_postgres_test.go new file mode 100644 index 0000000..d94dd3f --- /dev/null +++ b/zjazd_migration_postgres_test.go @@ -0,0 +1,121 @@ +package bm_test + +import ( + "database/sql" + "testing" + + "git.golem15.com/jakub/sm-bm-plugin/updates" + "gorm.io/gorm" +) + +// TestMoveZjazdTagsAgainstDatabase pins the data behaviour of the one-way +// zjazd migration (D-12): a material with no meeting gets the meeting its +// zjazd-N tag names (the lowest when it has several), an existing meeting +// is kept, a tag naming a missing meeting assigns nothing, only exact +// zjazd-N slugs are deleted and their links go with them, and a second run +// changes nothing. +func TestMoveZjazdTagsAgainstDatabase(t *testing.T) { + _, _, gdb := migrated(t) + + insertID := func(query string, args ...any) int64 { + t.Helper() + var id int64 + if err := gdb.Raw(query+" RETURNING id", args...).Scan(&id).Error; err != nil { + t.Fatalf("%s: %v", query, err) + } + return id + } + meeting := map[int]int64{} + for _, n := range []int{2, 3, 4} { + meeting[n] = insertID(`INSERT INTO jz_bm_meetings (number, starts_on, ends_on) VALUES (?, '2026-10-01', '2026-10-02')`, n) + } + tag := map[string]int64{} + for _, slug := range []string{"zjazd-3", "zjazd-4", "zjazd-5", "zjazd-3-24-10", "globalizacja"} { + tag[slug] = insertID(`INSERT INTO jz_bm_tags (name, slug) VALUES (?, ?)`, slug, slug) + } + material := func(title string, meetingID *int64, slugs ...string) int64 { + t.Helper() + id := insertID(`INSERT INTO jz_bm_materials (title, type, meeting_id) VALUES (?, 'notatka', ?)`, title, meetingID) + for _, s := range slugs { + if err := gdb.Exec(`INSERT INTO jz_bm_material_tag (material_id, tag_id) VALUES (?, ?)`, id, tag[s]).Error; err != nil { + t.Fatal(err) + } + } + return id + } + four := meeting[4] + a := material("A", nil, "zjazd-3", "globalizacja") + b := material("B", &four, "zjazd-3") + c := material("C", nil, "zjazd-5") + d := material("D", nil, "zjazd-4", "zjazd-3") + e := material("E", nil, "zjazd-3-24-10") + + run := func() { + t.Helper() + if err := gdb.Transaction(func(tx *gorm.DB) error { + return updates.MoveZjazdTagsToMeetings().Migrate(tx) + }); err != nil { + t.Fatalf("Migrate: %v", err) + } + } + meetingOf := func(id int64) sql.NullInt64 { + t.Helper() + var m sql.NullInt64 + if err := gdb.Raw(`SELECT meeting_id FROM jz_bm_materials WHERE id = ?`, id).Scan(&m).Error; err != nil { + t.Fatal(err) + } + return m + } + check := func(round string) { + t.Helper() + for _, w := range []struct { + name string + id int64 + want sql.NullInt64 + }{ + {"A (zjazd-3, no meeting)", a, sql.NullInt64{Int64: meeting[3], Valid: true}}, + {"B (meeting 4, tag zjazd-3)", b, sql.NullInt64{Int64: meeting[4], Valid: true}}, + {"C (zjazd-5, no meeting 5)", c, sql.NullInt64{}}, + {"D (zjazd-4 and zjazd-3)", d, sql.NullInt64{Int64: meeting[3], Valid: true}}, + {"E (zjazd-3-24-10, a topic)", e, sql.NullInt64{}}, + } { + if got := meetingOf(w.id); got != w.want { + t.Errorf("%s: material %s meeting_id = %+v, want %+v", round, w.name, got, w.want) + } + } + var zjazd int64 + if err := gdb.Raw(`SELECT COUNT(*) FROM jz_bm_tags WHERE slug ~ '^zjazd-[0-9]+$'`).Scan(&zjazd).Error; err != nil { + t.Fatal(err) + } + if zjazd != 0 { + t.Errorf("%s: %d zjazd-N tags remain", round, zjazd) + } + var kept []string + if err := gdb.Raw(`SELECT slug FROM jz_bm_tags ORDER BY slug`).Scan(&kept).Error; err != nil { + t.Fatal(err) + } + if len(kept) != 2 || kept[0] != "globalizacja" || kept[1] != "zjazd-3-24-10" { + t.Errorf("%s: tags left %v, want [globalizacja zjazd-3-24-10]", round, kept) + } + var links int64 + if err := gdb.Raw(`SELECT COUNT(*) FROM jz_bm_material_tag WHERE (material_id = ? AND tag_id = ?) OR (material_id = ? AND tag_id = ?)`, + a, tag["globalizacja"], e, tag["zjazd-3-24-10"]).Scan(&links).Error; err != nil { + t.Fatal(err) + } + if links != 2 { + t.Errorf("%s: topic links left %d, want A-globalizacja and E-zjazd-3-24-10", round, links) + } + var orphans int64 + if err := gdb.Raw(`SELECT COUNT(*) FROM jz_bm_material_tag mt LEFT JOIN jz_bm_tags t ON t.id = mt.tag_id WHERE t.id IS NULL`).Scan(&orphans).Error; err != nil { + t.Fatal(err) + } + if orphans != 0 || count(t, gdb, "jz_bm_material_tag") != 2 { + t.Errorf("%s: %d pivot rows point at deleted tags, %d rows in all", round, orphans, count(t, gdb, "jz_bm_material_tag")) + } + } + + run() + check("first run") + run() + check("second run") +}