From 327925c43e217e284210b6b30699bf0966793e53 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 11:37:20 +0100 Subject: [PATCH] feat: address-change redirects for writes outside the admin - the RecordSaved listener re-describes an updated record of a content type after commit and applies AddressChanged per changed locale, so Journal's API, MCP and imports get the 301 too; an admin save that already applied it changes nothing more --- listeners.go | 49 +++++++++++++++++++++++++++++---- redirects_test.go | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 5 deletions(-) diff --git a/listeners.go b/listeners.go index 0e4e362..9e81c82 100644 --- a/listeners.go +++ b/listeners.go @@ -3,14 +3,19 @@ package seo import ( "context" + "git.golem15.com/golem15/sm-seo-plugin/classes" "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/pact" + "gorm.io/gorm" ) // listen subscribes to the framework's record events (D-92): a deleted -// record of a content type loses its SEO entries (kept in history), and a -// created one gets its address snapshot. Listener errors are logged by the -// bus and never fail the write that fired them. +// record of a content type loses its SEO entries (kept in history), a +// created one gets its address snapshot, and an updated one whose address +// changed gets its 301 after commit (D-18), whoever wrote it: Journal's +// API, MCP, an import, or an admin form whose SEO tab the administrator +// does not see. Listener errors are logged by the bus and never fail the +// write that fired them. func (p *Plugin) listen(app *backpack.App) { if app == nil || app.Events == nil { return @@ -24,9 +29,43 @@ func (p *Plugin) listen(app *backpack.App) { }) app.Events.Listen("golem15.seo", func(ctx context.Context, ev *pact.RecordSaved) error { svc := p.Service() - if svc == nil || ev == nil || !ev.Created { + if svc == nil || ev == nil { return nil } - return svc.RecordCreated(ctx, ev) + if ev.Created { + return svc.RecordCreated(ctx, ev) + } + return p.addressesAfterSave(ctx, svc, ev.Model, ev.ID) + }) +} + +// addressesAfterSave re-describes a saved record of a content type in a +// transaction of its own and applies AddressChanged to every locale whose +// address differs from its entry's URL. After an admin save whose SEO tab +// already did this, nothing differs and nothing happens. +func (p *Plugin) addressesAfterSave(ctx context.Context, svc *classes.Service, model, id string) error { + reg := svc.Registry() + if reg == nil { + return nil + } + typ, ok := reg.ContentTypeForModel(model) + if !ok || typ.Content == nil { + return nil + } + db, err := svc.DB(ctx) + if err != nil { + return err + } + return db.Transaction(func(tx *gorm.DB) error { + changes, err := svc.AddressChanges(ctx, tx, typ, id) + if err != nil { + return err + } + for _, change := range changes { + if _, err := svc.AddressChanged(ctx, tx, change); err != nil { + return err + } + } + return nil }) } diff --git a/redirects_test.go b/redirects_test.go index c0e6287..6e7687d 100644 --- a/redirects_test.go +++ b/redirects_test.go @@ -2,6 +2,7 @@ package seo_test import ( "bytes" + "context" "encoding/json" "errors" "net/http" @@ -14,6 +15,9 @@ import ( "git.golem15.com/golem15/sm-seo-plugin/classes" "git.golem15.com/golem15/sm-seo-plugin/models" + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/pact" "gorm.io/gorm" ) @@ -575,3 +579,68 @@ func TestNotFoundLog(t *testing.T) { t.Fatalf("open filter = %v, want none", open) } } + +// TestAddressChangeFromEvent checks the after-commit path for writes +// outside the admin forms (D-18, D-92, D-102): a Journal-style write that +// changes a published record's slug and fires RecordSaved without a +// controller creates the 301; a save without an address change and a +// change of an unpublished record create nothing. +func TestAddressChangeFromEvent(t *testing.T) { + env := bootEnv(t) + token := env.admin(t, true) + id := env.createPage(t, token, map[string]any{"title": "Beta", "slug": "beta", "published": true}) + draft := env.createPage(t, token, map[string]any{"title": "Draft", "slug": "draft", "published": false}) + + write := func(id string, changes ...pact.RecordChange) { + t.Helper() + if err := lagoon.Transaction(t.Context(), env.db, func(ctx context.Context, tx *gorm.DB) error { + for _, c := range changes { + if err := tx.Exec(`UPDATE acme_pages_pages SET `+c.Attribute+` = ? WHERE id = ?`, c.New, id).Error; err != nil { + return err + } + } + cabana.EmitRecordSaved(ctx, env.app, tx, pact.RecordSaved{Model: pagesModel, ID: id, Changes: changes}) + return nil + }); err != nil { + t.Fatal(err) + } + } + + write(id, pact.RecordChange{Attribute: "title", Old: "Beta", New: "Beta 2"}) + if n := env.redirectCount(t); n != 0 { + t.Fatalf("redirects after a save without an address change = %d, want 0", n) + } + + write(id, pact.RecordChange{Attribute: "slug", Old: "beta", New: "gamma"}) + rows := env.redirects(t) + if redirectTo(rows, "/pages/beta") != "/pages/gamma" || redirectTo(rows, "/pl/pages/beta") != "/pl/pages/gamma" { + t.Fatalf("redirects after an API slug change = %+v, want /pages/beta -> /pages/gamma in both locales", rows) + } + for _, r := range rows { + if r.Origin != "slug" || r.Status != 301 { + t.Fatalf("redirect %+v, want a 301 with origin slug", r) + } + } + env.expectRedirect(t, "/pages/beta", 301, siteURL+"/pages/gamma") + if rows := env.revisions(t, "redirect"); len(rows) == 0 || rows[0].Via != "auto" { + t.Fatalf("redirect history = %+v, want via auto", rows) + } + + before := env.redirectCount(t) + write(draft, pact.RecordChange{Attribute: "slug", Old: "draft", New: "draft-2"}) + if n := env.redirectCount(t); n != before { + t.Fatalf("redirects after an unpublished record's slug change = %d, want %d", n, before) + } + var url string + if err := env.db.Raw(`SELECT url FROM golem15_seo_entries WHERE record_id = ? AND locale = 'en'`, draft).Scan(&url).Error; err != nil || url != "/pages/draft-2" { + t.Fatalf("unpublished entry URL = %q err=%v, want the new address without a redirect", url, err) + } + + // An admin save already applied the change: the event after it + // changes nothing more. + env.expect(t, http.StatusOK, http.MethodPut, adminAPI("/acme/pages/pages/"+id), token, map[string]any{"title": "Beta 2", "slug": "delta", "published": true}) + rows = env.redirects(t) + if redirectTo(rows, "/pages/gamma") != "/pages/delta" || redirectTo(rows, "/pages/beta") != "/pages/delta" { + t.Fatalf("redirects after the admin save = %+v, want both old addresses on /pages/delta", rows) + } +}