From 59f0457c69fa07088fd369cc930f6c351b9fae6e Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 4 Nov 2026 13:39:31 +0100 Subject: [PATCH] feat: sitemap-aware batch conflict and the sitemap_blocked issue - noindex_published fires only when a sitemap file lists the address now (read once per batch through contract.Listing) - CMS issue sitemap_blocked (severity error, tool seo_update_robots, details line and rule): a listed address the production robots.txt keeps Googlebot from - the SEO tab's P1 shows status.excluded_title with the reason (noindex, canonical elsewhere, redirect, or the type switched off in the sitemap settings via ListingSettings.TypesOff) as a plain callout - robots.txt is composed once per check of many addresses --- batch_test.go | 2 + classes/batch.go | 43 +++++++++++-- classes/issues.go | 34 ++++++++++ classes/partials.go | 29 ++++++++- classes/robots.go | 13 +++- classes/sitemap_filter.go | 131 ++++++++++++++++++++++++++++++++++---- controllers/api/issues.go | 2 +- extension/_seo_status.htm | 2 +- lang/en/lang.yaml | 5 ++ lang/pl/lang.yaml | 5 ++ sitemap_filter_test.go | 126 ++++++++++++++++++++++++++++++++++++ 11 files changed, 370 insertions(+), 22 deletions(-) diff --git a/batch_test.go b/batch_test.go index d5c8400..c6ffc09 100644 --- a/batch_test.go +++ b/batch_test.go @@ -70,6 +70,8 @@ func TestBatch(t *testing.T) { token := env.admin(t, true) env.createPage(t, token, map[string]any{"title": "Page A", "slug": "a", "excerpt": "About A", "published": true}) env.createPage(t, token, map[string]any{"title": "Page B", "slug": "b", "excerpt": "About B", "published": true}) + // noindex_published needs the address in a sitemap file. + env.rebuildSitemap(t) agent := env.principal(t, env.admin(t, false, "golem15.seo.manage")) changes := []any{ diff --git a/classes/batch.go b/classes/batch.go index 478847a..bfe007e 100644 --- a/classes/batch.go +++ b/classes/batch.go @@ -290,8 +290,9 @@ func (s *Service) runBatch(ctx context.Context, opts BatchOptions, build func(ct } res.Changes = append(res.Changes, item) } + listed := &listedOnce{svc: s} for i := range res.Changes { - late, err := s.batchConflicts(ctx, tx, i, changes[i], res.Changes[i]) + late, err := s.batchConflicts(ctx, tx, listed, i, changes[i], res.Changes[i]) if err != nil { return err } @@ -452,12 +453,34 @@ func (s *Service) applyChange(ctx context.Context, tx *gorm.DB, index int, ch Ba return errUnknownOp } +// listedOnce reads the addresses the sitemaps list once per batch. +type listedOnce struct { + svc *Service + read bool + ok bool + listed map[string]bool +} + +// has reports whether the sitemaps list address; ok is false without the +// sitemap plugin's Listing. +func (l *listedOnce) has(ctx context.Context, address string) (listed, ok bool, err error) { + if !l.read { + l.listed, l.ok, err = l.svc.SitemapListed(context.WithoutCancel(ctx)) + if err != nil { + return false, false, err + } + l.read = true + } + return l.listed[address], l.ok, nil +} + // batchConflicts are the warnings that depend on the whole batch, checked // once every change is in place: noindex on the address of a published -// record (the records the sitemaps list; Phase 33's sitemap filter -// refines it) and a canonical pointing to a redirected or noindex -// address. -func (s *Service) batchConflicts(ctx context.Context, tx *gorm.DB, index int, ch BatchChange, item BatchItem) ([]Conflict, error) { +// record that a sitemap file lists now (read through contract.Listing, so +// an address the sitemaps already leave out, such as one of a content +// type switched off, is no conflict; D-12, D-24), and a canonical pointing +// to a redirected or noindex address. +func (s *Service) batchConflicts(ctx context.Context, tx *gorm.DB, listed *listedOnce, index int, ch BatchChange, item BatchItem) ([]Conflict, error) { if item.Error != "" { return nil, nil } @@ -490,7 +513,15 @@ func (s *Service) batchConflicts(ctx context.Context, tx *gorm.DB, index int, ch if err != nil && !errors.Is(err, ErrBadURL) { return nil, err } - if err == nil && owner.WrittenTo == WrittenToRecord && owner.Published { + inSitemap := err == nil && owner.WrittenTo == WrittenToRecord && owner.Published + if inSitemap { + has, ok, lerr := listed.has(ctx, owner.URL) + if lerr != nil { + return nil, lerr + } + inSitemap = !ok || has + } + if inSitemap { out = append(out, Conflict{Change: index, Kind: ConflictNoindexPublished, URL: owner.URL, Message: s.PhraseCtx(ctx, "golem15.seo::lang.api.conflict_noindex_published", map[string]string{"url": owner.URL})}) } diff --git a/classes/issues.go b/classes/issues.go index 4b09cd0..4db8a1b 100644 --- a/classes/issues.go +++ b/classes/issues.go @@ -21,6 +21,15 @@ const ( IssueCanonicalNoindex = "canonical_noindex" IssueCanonicalRedirected = "canonical_redirected" IssueNoindexBlocked = "noindex_blocked" + // IssueSitemapBlocked is a sitemap address the production robots.txt + // keeps Googlebot from (D-24): an error, since the sitemap asks for a + // page the crawler may not fetch. + IssueSitemapBlocked = "sitemap_blocked" +) + +// The severities an issue may name. +const ( + SeverityError = "error" ) // Issue is one finding of the issue report: the address, the kind, the @@ -31,6 +40,9 @@ type Issue struct { Locale string `json:"locale,omitempty"` Details map[string]any `json:"details,omitempty"` Tool string `json:"tool"` + // Severity is set for kinds that are errors (sitemap_blocked); "" for + // the others. + Severity string `json:"severity,omitempty"` } // The tools the issues point to (pier prefixes them with seo_). @@ -71,6 +83,14 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) { if data.noindex, err = noindexAddresses(db); err != nil { return nil, err } + listed, _, err := s.SitemapListed(ctx) + if err != nil { + return nil, err + } + for address := range listed { + data.listed = append(data.listed, address) + } + sort.Strings(data.listed) return s.issuesOf(ctx, data, s.canonicalProblem) } @@ -85,6 +105,8 @@ type issueData struct { redirects []models.Redirect robots models.Robots noindex []string + // listed are the addresses the sitemaps list (path with query). + listed []string } // issuesOf builds the report from data; canonical classifies a canonical @@ -228,10 +250,22 @@ func (s *Service) issuesOf(ctx context.Context, data issueData, canonical func(c } // Noindex addresses robots.txt blocks. + file := ComposeRobots(data.robots.Content, data.robots.Bots, s.SitemapLines(ctx)) for _, u := range s.blockedNoindex(data.noindex, data.robots.Content, data.robots.Bots) { out = append(out, Issue{Kind: IssueNoindexBlocked, URL: u, Tool: toolUpdateRobots}) } + // Sitemap addresses the production robots.txt blocks for Googlebot. + for _, u := range data.listed { + test := s.testRobotsFile(file, data.robots.Content, "Googlebot", u) + if test.Allowed { + continue + } + out = append(out, Issue{Kind: IssueSitemapBlocked, URL: u, Tool: toolUpdateRobots, Severity: SeverityError, Details: map[string]any{ + "line": test.Line, "rule": test.Rule, + }}) + } + sort.SliceStable(out, func(i, j int) bool { if out[i].Kind != out[j].Kind { return out[i].Kind < out[j].Kind diff --git a/classes/partials.go b/classes/partials.go index a7d53a0..8f84158 100644 --- a/classes/partials.go +++ b/classes/partials.go @@ -8,6 +8,7 @@ import ( "strings" "git.golem15.com/golem15/sm-seo-plugin/models" + "git.golem15.com/golem15/sm-sitemap-plugin/contract" "gorm.io/gorm" ) @@ -15,10 +16,12 @@ import ( // the request locale), never a GORM model; html/template escapes every // value. -// Callout is one P1 warning. +// Callout is one P1 callout: a warning, or a plain note when Plain is +// set (the sitemap status). type Callout struct { Title string Text string + Plain bool } // StatusView is the P1 seo_status view model. @@ -202,9 +205,33 @@ func (s *Service) RecordStatus(ctx context.Context, key, id string) (StatusView, } } } + exclusions, err := s.RecordSitemapExclusions(ctx, key, id) + if err != nil { + return view, err + } + for _, x := range exclusions { + key, ok := excludedTexts[x.Reason] + if !ok { + continue + } + view.Callouts = append(view.Callouts, Callout{ + Title: s.PhraseCtx(ctx, "golem15.seo::lang.status.excluded_title", nil), + Text: s.PhraseCtx(ctx, key, map[string]string{"url": x.Target}), + Plain: true, + }) + } return view, nil } +// excludedTexts are the P1 texts of the sitemap exclusion reasons (UI-SPEC +// C16). +var excludedTexts = map[string]string{ + contract.ReasonNoindex: "golem15.seo::lang.status.excluded_noindex", + contract.ReasonCanonical: "golem15.seo::lang.status.excluded_canonical", + contract.ReasonRedirect: "golem15.seo::lang.status.excluded_redirect", + contract.ReasonType: "golem15.seo::lang.status.excluded_type", +} + // ruleCallout is the override callout of the first rule that sets a value. func (s *Service) ruleCallout(ctx context.Context, rules []models.Rule) (Callout, bool) { for _, r := range rules { diff --git a/classes/robots.go b/classes/robots.go index 9a39f66..03f82dc 100644 --- a/classes/robots.go +++ b/classes/robots.go @@ -205,7 +205,12 @@ type RobotsTest struct { // grobotstxt, Google's matcher (D-22, D-99): longest match, Allow wins a // tie, * and $. func (s *Service) TestRobots(content string, bots models.BotSwitches, agent, path string) RobotsTest { - file := ComposeRobots(content, bots, s.SitemapLines(context.Background())) + return s.testRobotsFile(ComposeRobots(content, bots, s.SitemapLines(context.Background())), content, agent, path) +} + +// testRobotsFile is TestRobots on a composed file; content is the editor's +// part of it (the generated bot rules follow it). +func (s *Service) testRobotsFile(file, content, agent, path string) RobotsTest { uri := AbsoluteURL(s.SiteURL(), path) if s.SiteURL() == "" { uri = "http://localhost" + path @@ -269,8 +274,12 @@ func (s *Service) BlockedNoindex(db *gorm.DB, content string, bots models.BotSwi // the production file composed from content and bots. func (s *Service) blockedNoindex(urls []string, content string, bots models.BotSwitches) []string { var out []string + if len(urls) == 0 { + return out + } + file := ComposeRobots(content, bots, s.SitemapLines(context.Background())) for _, u := range urls { - if !s.TestRobots(content, bots, "Googlebot", u).Allowed { + if !s.testRobotsFile(file, content, "Googlebot", u).Allowed { out = append(out, u) } } diff --git a/classes/sitemap_filter.go b/classes/sitemap_filter.go index b4b671a..89eb6d9 100644 --- a/classes/sitemap_filter.go +++ b/classes/sitemap_filter.go @@ -132,23 +132,31 @@ func (s *Service) newFilterBatch(ctx context.Context) (*filterBatch, error) { }, nil } -// decide is the decision of one candidate: a redirected address first (it -// is no page), then noindex, then a canonical URL elsewhere. +// decide is the decision of one candidate. func (b *filterBatch) decide(ctx context.Context, c contract.Candidate) (contract.Decision, error) { + d, _, err := b.decideTarget(ctx, c) + return d, err +} + +// decideTarget is the decision of one candidate and, for a canonical or a +// redirect, the address the sitemaps would list instead: a redirected +// address first (it is no page), then noindex, then a canonical URL +// elsewhere. +func (b *filterBatch) decideTarget(ctx context.Context, c contract.Candidate) (contract.Decision, string, error) { path, query, rawQuery, ok := b.candidatePath(c.URL) if !ok { - return contract.Decision{}, nil + return contract.Decision{}, "", nil } norm, changed := Normalize(&url.URL{Path: path, RawQuery: rawQuery}, b.opts) if changed { - return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, nil + return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, joinQuery(norm.Path, norm.RawQuery), nil } chain, err := b.set.Follow(b.site, norm.Path, norm.RawQuery, MaxRedirectHops) if err != nil { - return contract.Decision{}, err + return contract.Decision{}, "", err } if len(chain.Hops) > 0 { - return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, nil + return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, displayURL(b.site, chain.Final), nil } key := "" var match *pact.ContentMatchResult @@ -160,20 +168,121 @@ func (b *filterBatch) decide(ctx context.Context, c contract.Candidate) (contrac } res, err := b.resolver.resolveAt(ctx, b.db, b.settings, path, query, rawQuery, key, match, nil, true) if err != nil { - return contract.Decision{}, err + return contract.Decision{}, "", err } if res.SEO == nil { - return contract.Decision{}, nil + return contract.Decision{}, "", nil } if r, err := ParseRobots(res.SEO.Robots); err == nil && r.Index == "noindex" { - return contract.Decision{Exclude: true, Reason: contract.ReasonNoindex}, nil + return contract.Decision{Exclude: true, Reason: contract.ReasonNoindex}, "", nil } if !sameAddress(res.SEO.Canonical, res.URL) { - return contract.Decision{Exclude: true, Reason: contract.ReasonCanonical}, nil + return contract.Decision{Exclude: true, Reason: contract.ReasonCanonical}, displayURL(b.site, res.SEO.Canonical), nil } - return contract.Decision{}, nil + return contract.Decision{}, "", nil } +// SitemapExclusion is why a record's address is left out of the sitemaps: +// a contract reason and, for canonical and redirect, the address listed +// instead. +type SitemapExclusion struct { + Locale string + Reason string + Target string +} + +// RecordSitemapExclusions lists why the sitemaps leave out the addresses +// of record id of content type key, one per reason (the first locale that +// has it): the type switched off in the sitemap settings (read through +// contract.Listing), else the SEO plugin's own decision on each address +// (noindex, canonical elsewhere, redirect). Nothing for an unpublished +// record: the sitemaps never list it. +func (s *Service) RecordSitemapExclusions(ctx context.Context, key, id string) ([]SitemapExclusion, error) { + reg := s.Registry() + if reg == nil { + return nil, nil + } + rec, err := reg.Describe(ctx, key, id, s.UnprefixedLocale()) + if err != nil || rec == nil || !rec.Published { + return nil, err + } + if l := s.listing(); l != nil { + st, err := l.Settings(ctx) + if err != nil { + return nil, err + } + for _, off := range st.TypesOff { + if off == key { + return []SitemapExclusion{{Reason: contract.ReasonType}}, nil + } + } + } + b, err := s.newFilterBatch(ctx) + if err != nil { + return nil, err + } + var out []SitemapExclusion + seen := map[string]bool{} + for _, l := range rec.Locales { + if strings.TrimSpace(l.URL) == "" { + continue + } + d, target, err := b.decideTarget(ctx, contract.Candidate{TypeKey: key, RecordID: id, Locale: l.Locale, URL: l.URL}) + if err != nil { + return nil, err + } + if d.Exclude && !seen[d.Reason] { + seen[d.Reason] = true + out = append(out, SitemapExclusion{Locale: l.Locale, Reason: d.Reason, Target: target}) + } + } + return out, nil +} + +// SitemapListed is the set of addresses the sitemaps list, read through +// contract.Listing: their paths with the query, as the owner of a URL +// names them. ok is false without a published Listing. At most +// MaxListedAddresses are read. +func (s *Service) SitemapListed(ctx context.Context) (map[string]bool, bool, error) { + l := s.listing() + if l == nil { + return nil, false, nil + } + st, err := l.Settings(ctx) + if err != nil { + return nil, false, err + } + out := map[string]bool{} + read := 0 + for _, g := range st.Groups { + after := "" + for read < MaxListedAddresses { + page, err := l.Entries(ctx, contract.EntriesQuery{Group: g.Key, After: after, Limit: 1000}) + if err != nil { + return nil, false, err + } + for _, e := range page.Entries { + read++ + if u, err := url.Parse(e.URL); err == nil { + path := u.Path + if path == "" { + path = "/" + } + out[joinQuery(path, u.RawQuery)] = true + } + } + if page.Next == "" || page.Next == after { + break + } + after = page.Next + } + } + return out, true, nil +} + +// MaxListedAddresses caps the sitemap addresses SitemapListed reads. +const MaxListedAddresses = 100000 + // candidatePath turns a candidate's address (a path, or an absolute URL on // the site or its www twin) into the decoded path and its query; ok is // false for an address the plugin cannot judge (another host, malformed). diff --git a/controllers/api/issues.go b/controllers/api/issues.go index ad6a32a..9e254df 100644 --- a/controllers/api/issues.go +++ b/controllers/api/issues.go @@ -9,7 +9,7 @@ import ( // ListIssuesInput is the query of GET /_seo/api/v1/issues. type ListIssuesInput struct { - Kind string `json:"kind,omitempty" query:"kind" jsonschema:"one kind only: missing_title, missing_description, duplicate_title, duplicate_description, rule_overrides_record, redirect_chain, canonical_noindex, canonical_redirected or noindex_blocked"` + Kind string `json:"kind,omitempty" query:"kind" jsonschema:"one kind only: missing_title, missing_description, duplicate_title, duplicate_description, rule_overrides_record, redirect_chain, canonical_noindex, canonical_redirected, noindex_blocked or sitemap_blocked"` } // ListIssues serves GET /_seo/api/v1/issues (D-13, D-14 CMS part): the diff --git a/extension/_seo_status.htm b/extension/_seo_status.htm index de192cc..f9f7fc2 100644 --- a/extension/_seo_status.htm +++ b/extension/_seo_status.htm @@ -1,5 +1,5 @@ {{- range .Data.Callouts -}} -
+

{{ .Title }}

{{- if .Text }}

{{ .Text }}

{{ end }}
diff --git a/lang/en/lang.yaml b/lang/en/lang.yaml index 826e4a3..80c39a8 100644 --- a/lang/en/lang.yaml +++ b/lang/en/lang.yaml @@ -63,6 +63,11 @@ robots: one: ":count noindex address is blocked by these rules, so Google cannot see its noindex: :list" other: ":count noindex addresses are blocked by these rules, so Google cannot see their noindex: :list" status: + excluded_title: Not in the sitemaps + excluded_noindex: This page is set to noindex, so the sitemaps leave it out. + excluded_canonical: The canonical URL points to “:url”, so the sitemaps list that address instead. + excluded_redirect: This address redirects to “:url”, so the sitemaps leave it out. + excluded_type: Its content type is turned off in the sitemap settings. rule_title: A URL rule overrides part of this tab rule_text: The rule for “:url” sets :fields. Change them in SEO › URL rules. canonical_noindex: The canonical URL “:url” is set to noindex, so search engines may ignore both pages. diff --git a/lang/pl/lang.yaml b/lang/pl/lang.yaml index 64456af..e667c58 100644 --- a/lang/pl/lang.yaml +++ b/lang/pl/lang.yaml @@ -65,6 +65,11 @@ robots: many: ":count adresów z noindex jest blokowanych przez te reguły, więc Google nie zobaczy noindex: :list" other: ":count adresu z noindex jest blokowane przez te reguły, więc Google nie zobaczy noindex: :list" status: + excluded_title: Poza mapami witryny + excluded_noindex: Ta strona ma ustawione noindex, więc mapy witryny ją pomijają. + excluded_canonical: Adres kanoniczny wskazuje „:url”, więc mapy witryny podają tamten adres. + excluded_redirect: Ten adres przekierowuje na „:url”, więc mapy witryny go pomijają. + excluded_type: Jej typ treści jest wyłączony w ustawieniach map witryny. rule_title: Reguła adresu nadpisuje część tej zakładki rule_text: 'Reguła dla „:url” ustawia: :fields. Zmień je w SEO › Reguły adresów.' canonical_noindex: Adres kanoniczny „:url” ma ustawione noindex, więc wyszukiwarki mogą zignorować obie strony. diff --git a/sitemap_filter_test.go b/sitemap_filter_test.go index 03fa573..bfaaf98 100644 --- a/sitemap_filter_test.go +++ b/sitemap_filter_test.go @@ -1,6 +1,7 @@ package seo_test import ( + "encoding/json" "net/http" "strings" "testing" @@ -88,3 +89,128 @@ func TestSitemapFilterTracer(t *testing.T) { }) } } + +// seoStatus fetches the SEO tab's P1 of a fake page. +func (e *testEnv) seoStatus(t *testing.T, token, id string) string { + t.Helper() + return e.expect(t, http.StatusOK, http.MethodGet, adminAPI("/acme/pages/pages/partials/seo_status?id="+id), token, nil).Body.String() +} + +// switchTypeOff turns a content type off in the sitemap settings. +func (e *testEnv) switchTypeOff(t *testing.T, typeKey string) { + t.Helper() + if err := e.db.Exec(`INSERT INTO golem15_sitemap_types (type_key, include, slug, titles, updated_at) VALUES (?, FALSE, '', '{}', NOW()) +ON CONFLICT (type_key) DO UPDATE SET include = FALSE`, typeKey).Error; err != nil { + t.Fatal(err) + } +} + +// TestFilterReasons covers the other reasons (D-95, D-24): a canonical +// pointing elsewhere, a redirected address, an address the URL rules set +// to noindex; an additional address of the sitemap settings is judged like +// any address; and P1 of the SEO tab shows status.excluded_title with the +// reason's text, also for a type switched off in the sitemap settings. +func TestFilterReasons(t *testing.T) { + env := bootEnv(t) + token := env.admin(t, true) + canonical := env.createPage(t, token, map[string]any{"title": "Copy", "slug": "copy", "published": true}) + moved := env.createPage(t, token, map[string]any{"title": "Moved", "slug": "moved", "published": true}) + ruled := env.createPage(t, token, map[string]any{"title": "Ruled", "slug": "ruled", "published": true}) + plain := env.createPage(t, token, map[string]any{"title": "Plain", "slug": "plain", "published": true}) + env.expect(t, http.StatusOK, http.MethodPut, adminAPI("/acme/pages/pages/"+canonical), token, map[string]any{ + "title": "Copy", "slug": "copy", "published": true, "seo_canonical": map[string]any{"en": "/pages/plain"}, + }) + env.expect(t, http.StatusCreated, http.MethodPost, "/_seo/api/v1/redirects", token, map[string]any{"source": "/pages/moved", "target": "/pages/plain"}) + env.expect(t, http.StatusCreated, http.MethodPost, "/_seo/api/v1/rules", token, map[string]any{"url": "/pages/ruled", "match": "exact", "robots": "noindex,follow"}) + env.rebuildSitemap(t) + + for id, want := range map[string]string{canonical: "canonical", moved: "redirect", ruled: "noindex", plain: ""} { + if got := env.sitemapReason(t, "acme-page", id, "en"); got != want { + t.Errorf("page %s reason = %q, want %q", id, got, want) + } + } + file := env.sitemapFile(t, "sitemap-pages-1.xml") + if !strings.Contains(file, ".test/pages/plain") || strings.Contains(file, ".test/pages/copy") || strings.Contains(file, ".test/pages/moved") { + t.Fatalf("pages file:\n%s", file) + } + + for id, want := range map[string]string{ + canonical: "The canonical URL points to “/pages/plain”, so the sitemaps list that address instead.", + moved: "This address redirects to “/pages/plain”, so the sitemaps leave it out.", + ruled: "This page is set to noindex, so the sitemaps leave it out.", + } { + status := env.seoStatus(t, token, id) + if !strings.Contains(status, "Not in the sitemaps") || !strings.Contains(status, want) { + t.Errorf("P1 of %s = %s, want %q", id, status, want) + } + if !strings.Contains(status, `"attrs":{"class":"summer-callout","role":"status"}`) { + t.Errorf("P1 of %s: the sitemap callout is not plain: %s", id, status) + } + } + if status := env.seoStatus(t, token, plain); strings.Contains(status, "Not in the sitemaps") { + t.Errorf("P1 of a listed page = %s", status) + } + env.switchTypeOff(t, "acme-page") + if status := env.seoStatus(t, token, plain); !strings.Contains(status, "Its content type is turned off in the sitemap settings.") { + t.Errorf("P1 with the type off = %s", status) + } +} + +// TestNoindexPublishedConflict covers D-12 with the sitemap (D-24): a batch +// setting noindex on an address a sitemap file lists reports +// noindex_published; on a published address the sitemaps leave out (its +// type switched off) it does not. +func TestNoindexPublishedConflict(t *testing.T) { + env := bootEnv(t) + token := env.admin(t, true) + env.createPage(t, token, map[string]any{"title": "Listed", "slug": "listed", "published": true}) + env.rebuildSitemap(t) + agent := env.principal(t, env.admin(t, false, "golem15.seo.manage")) + batch := func() map[string]bool { + dry := env.mcpExpect(t, http.StatusOK, http.MethodPost, "/_seo/api/v1/batch", agent, map[string]any{"dry_run": true, "changes": []any{ + map[string]any{"op": "set_seo", "url": "/pages/listed", "values": map[string]any{"robots": "noindex"}}, + }}) + return conflictKinds(dry) + } + if kinds := batch(); !kinds["noindex_published"] { + t.Fatalf("conflicts = %v, want noindex_published for a listed address", kinds) + } + env.switchTypeOff(t, "acme-page") + env.rebuildSitemap(t) + if kinds := batch(); kinds["noindex_published"] { + t.Fatalf("conflicts = %v, want none for an address no sitemap lists", kinds) + } +} + +// TestSitemapBlockedIssue covers the sitemap_blocked CMS issue (D-24): a +// listed address the production robots.txt keeps Googlebot from is an +// error pointing to seo_update_robots, with the deciding line. +func TestSitemapBlockedIssue(t *testing.T) { + env := bootEnv(t) + token := env.admin(t, true) + env.createPage(t, token, map[string]any{"title": "Blocked", "slug": "x", "published": true}) + env.createPage(t, token, map[string]any{"title": "Open", "slug": "y", "published": true}) + env.rebuildSitemap(t) + env.expect(t, http.StatusOK, http.MethodPut, "/_seo/api/v1/robots", token, map[string]any{"content": "User-agent: *\nDisallow: /pages/x\n"}) + + var body struct { + Data []struct { + Kind string `json:"kind"` + URL string `json:"url"` + Tool string `json:"tool"` + Severity string `json:"severity"` + Details map[string]any `json:"details"` + } `json:"data"` + } + raw := env.expect(t, http.StatusOK, http.MethodGet, "/_seo/api/v1/issues?kind=sitemap_blocked", token, nil).Body.Bytes() + if err := json.Unmarshal(raw, &body); err != nil { + t.Fatal(err) + } + if len(body.Data) != 1 { + t.Fatalf("sitemap_blocked issues = %s, want one", raw) + } + got := body.Data[0] + if got.URL != "/pages/x" || got.Tool != "seo_update_robots" || got.Severity != "error" || got.Details["line"] != float64(2) || got.Details["rule"] != "Disallow: /pages/x" { + t.Fatalf("issue = %+v", got) + } +}