From d8aa989dfc9bf05487e39ef9741f56820ad3124b Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 15:38:15 +0100 Subject: [PATCH] fix: entry generation applies an unseen address change instead of overwriting it (WR-19) refreshEntry set entry.url to the descriptor's address unconditionally, so a slug change the job saw before any RecordSaved listener lost its 301 for good. An entry whose URL differs now goes through AddressChanged (via auto), which creates the 301 and moves the URL; an empty URL is filled. --- README.md | 2 +- classes/generate.go | 25 +++++++++++++++++++++---- seo_tab_test.go | 12 +++++++++++- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index d87cf1c..454c2dd 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ The frontend (a Nuxt layer or app) asks `GET /_seo/api/v1/resolve?url=...` for t - **Batches** (D-12): a list of changes (SEO by URL, redirects, URL rules) applied all or nothing under one batch id. A dry run answers the diff and the conflicts (loops, chains, noindex on a published record's address, a canonical pointing to a redirected or noindex address) and writes nothing. - **CSV import and export** of redirects, URL rules and the SEO of records (by URL), in the **Import and export** screen (element `golem15-seo-transfer`) and through MCP. Files are UTF-8 with a BOM, comma or semicolon; exported cells that start a formula (`=`, `+`, `-`, `@`, tab, CR) are prefixed with an apostrophe, which the import drops again. An import is previewed row by row (new, changed, unchanged, refused, with notes) and applied as one batch with via `import`, or not at all while a row is refused. The screen's apply token is an HMAC-SHA256 under the app key over the file and a fingerprint of the data the preview read; it is refused with `transfer.stale` when the data changed. The screen previews files up to 60 KB and downloads exports up to 250 KB; MCP imports take up to 900 KB per call and exports come in pages of 1000 rows. - **Record events.** A deleted record loses its entries after commit; a created record gets its address snapshot; an updated record whose address changed gets its 301. -- **Entry generation.** The `golem15.seo.generate_entries` job (conga) creates missing entries with the current address of every published record and refreshes the effective-value cache; editor values are never changed. +- **Entry generation.** The `golem15.seo.generate_entries` job (conga) creates missing entries with the current address of every published record and refreshes the effective-value cache; a record whose address changed without a record event gets its address change (with its 301) like an admin save. Editor values are never changed. - **Field types** `seotext`, `mlseotext`, `seorobots`, `mlseorobots`, `seoimage` and `robotstxt`, with the custom elements `golem15-seo-text`, `golem15-seo-robots`, `golem15-seo-serp` (the result preview widget), `golem15-seo-image`, `golem15-seo-robotstxt` (the robots.txt editor), `golem15-seo-robotstester` (the tester widget) and `golem15-seo-transfer` (the import and export widget). - **One permission**, `golem15.seo.manage`, for the SEO tab, rules, redirects, the 404 log, robots.txt, settings and history, in the admin and through MCP. - **Management API and MCP.** Every SEO admin operation has a JSON route under `/_seo/api/v1` and an MCP tool bound to it (pier, prefix `seo_`). Tools address pages by URL: a change goes to the record behind the URL, else to an exact URL rule, and the answer says where. A read grant sees only the read tools. `AdminOperations` maps each admin operation to its tool, and a test fails when one has none. diff --git a/classes/generate.go b/classes/generate.go index 378e095..4472a27 100644 --- a/classes/generate.go +++ b/classes/generate.go @@ -107,7 +107,9 @@ func (s *Service) GenerateEntries(ctx context.Context, typeKey string) (Generate } // refreshEntry creates or refreshes the entry of one record in one -// locale; created is true for a new row. +// locale; created is true for a new row. The job only sees published +// records, so an entry whose URL differs from the record's address gets +// the address change (with its 301), never a silent overwrite. func (s *Service) refreshEntry(ctx context.Context, tx *gorm.DB, typ pact.MenuItemType, id string, l pact.ContentLocale, settings models.Settings, ts models.TypeSetting) (bool, error) { entry, found, err := LoadEntry(tx, typ.Content.Model, id, l.Locale) if err != nil { @@ -123,12 +125,27 @@ func (s *Service) refreshEntry(ctx context.Context, tx *gorm.DB, typ pact.MenuIt } return true, tx.Create(&row).Error } - return false, tx.Model(&models.Entry{}).Where("id = ?", entry.ID).Updates(map[string]any{ - "url": l.URL, + updates := map[string]any{ "type_key": typ.Key, "effective_title": title, "effective_description": description, - }).Error + } + switch { + case entry.URL == "": + updates["url"] = l.URL + case entry.URL != l.URL: + // The published record's address changed and no RecordSaved + // listener has seen it yet (WR-19): apply it like the listener + // would, so its 301 is not lost; AddressChanged moves the entry's + // URL. + if _, err := s.AddressChanged(ctx, tx, AddressChange{ + TypeKey: typ.Key, ModelType: typ.Content.Model, RecordID: id, Locale: l.Locale, + Old: entry.URL, New: l.URL, + }); err != nil { + return false, err + } + } + return false, tx.Model(&models.Entry{}).Where("id = ?", entry.ID).Updates(updates).Error } // EffectiveValues are a record's effective title and description without diff --git a/seo_tab_test.go b/seo_tab_test.go index 23ccacb..1c92333 100644 --- a/seo_tab_test.go +++ b/seo_tab_test.go @@ -267,7 +267,8 @@ func TestEntryDeletedWithRecord(t *testing.T) { // TestGenerateEntries checks the D-94 generation job's work: every // published record gets an entry per locale with its URL, drafts none, and -// a second run refreshes addresses without touching editor values. +// a second run applies address changes (with their 301) without touching +// editor values. func TestGenerateEntries(t *testing.T) { env := bootEnv(t) if err := env.db.Exec(`INSERT INTO acme_pages_pages (title, slug, excerpt, published) VALUES ('One', 'one', 'First', TRUE), ('Two', 'two', '', TRUE), ('Draft', 'draft', '', FALSE)`).Error; err != nil { @@ -312,6 +313,15 @@ func TestGenerateEntries(t *testing.T) { if again.Title != "Editor title" || again.URL != "/pages/one-renamed" || again.EffectiveTitle != "Editor title" { t.Fatalf("after the second run = %+v, want the editor title kept and the URL refreshed", again) } + // The job applied the address change no RecordSaved announced, with + // its 301 (WR-19), in both locales. + redirects := env.redirects(t) + if redirectTo(redirects, "/pages/one") != "/pages/one-renamed" || redirectTo(redirects, "/pl/pages/one") != "/pl/pages/one-renamed" { + t.Fatalf("redirects after the job saw the slug change = %+v, want /pages/one and /pl/pages/one to 301", redirects) + } + if res, err = svc.GenerateEntries(t.Context(), ""); err != nil || res.Updated != 4 || len(env.redirects(t)) != len(redirects) { + t.Fatalf("third run = %+v err=%v, want no new redirect", res, err) + } if _, err := svc.GenerateEntries(t.Context(), "nope"); err == nil { t.Fatal("unknown type: want an error") }