feat: orphan pages and merged CMS issues with fix routes
- orphan_page for listed indexable pages no crawled page links to, with pages mentioning the title or focus keyphrase as suggestions (D-82) - CMS-data image_alt and h1_in_content from the content descriptors (TopHeading 2 only), stored with the crawl as source cms (D-85, D-50) - seo-page names Tools edit seo_update_page; every stored issue carries a tool or fix code with details.what (P33-38) - TestIssueMerge (ignored fingerprints survive the next crawl, fixed pages lose their issues) and TestStagingCrawl (Pitfall 12)
This commit is contained in:
@@ -125,6 +125,8 @@ type Env struct {
|
||||
Staging bool
|
||||
// GenericAnchors are the generic link texts (any case).
|
||||
GenericAnchors []string
|
||||
// Keyphrases maps an address to its SEO entry's focus keyphrase.
|
||||
Keyphrases map[string]string
|
||||
}
|
||||
|
||||
func (e *Env) record(key string) *Record {
|
||||
@@ -198,6 +200,7 @@ func Check(res *Result, env *Env) []Finding {
|
||||
out = append(out, checkRenderedPage(res, p, rec)...)
|
||||
}
|
||||
out = append(out, checkLinks(res, env)...)
|
||||
out = append(out, checkOrphans(res, env)...)
|
||||
out = append(out, checkSitemaps(res, env)...)
|
||||
out = append(out, checkDuplicates(res, env)...)
|
||||
return out
|
||||
|
||||
@@ -221,7 +221,11 @@ func finish(ctx context.Context, svc *classes.Service, db *gorm.DB, row models.C
|
||||
if err := SavePages(tx, row.ID, res); err != nil {
|
||||
return err
|
||||
}
|
||||
findings := Check(res, &Env{Lookup: Lookup(ctx, svc, tx), Staging: svc.Staging(), GenericAnchors: DefaultGenericAnchors})
|
||||
keyphrases, err := Keyphrases(tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
findings := Check(res, &Env{Lookup: Lookup(ctx, svc, tx), Staging: svc.Staging(), GenericAnchors: DefaultGenericAnchors, Keyphrases: keyphrases})
|
||||
cms, err := svc.Issues(classes.WithTx(ctx, tx))
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -325,6 +329,22 @@ func Lookup(ctx context.Context, svc *classes.Service, db *gorm.DB) func(key str
|
||||
}
|
||||
}
|
||||
|
||||
// Keyphrases maps each address with an SEO entry to its focus keyphrase.
|
||||
func Keyphrases(db *gorm.DB) (map[string]string, error) {
|
||||
var rows []struct {
|
||||
URL string
|
||||
Keyphrase string
|
||||
}
|
||||
if err := db.Raw(`SELECT url, keyphrase FROM golem15_seo_entries WHERE url <> '' AND keyphrase <> ''`).Scan(&rows).Error; err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out := make(map[string]string, len(rows))
|
||||
for _, r := range rows {
|
||||
out[r.URL] = r.Keyphrase
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// errCancelledCrawl refuses cancelling a crawl that is not queued or
|
||||
// running.
|
||||
var errCancelledCrawl = errors.New("golem15.seo: the crawl is not running")
|
||||
|
||||
79
classes/crawl/orphans.go
Normal file
79
classes/crawl/orphans.go
Normal file
@@ -0,0 +1,79 @@
|
||||
package crawl
|
||||
|
||||
import (
|
||||
"strings"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
)
|
||||
|
||||
// maxSuggestions caps the pages suggested to link to an orphan.
|
||||
const maxSuggestions = 10
|
||||
|
||||
// minPhrase is the shortest phrase a suggestion matches (in bytes).
|
||||
const minPhrase = 3
|
||||
|
||||
// checkOrphans reports orphan_page (D-82): an indexable page a sitemap
|
||||
// lists that no crawled page links to (the home page is the crawl's
|
||||
// entry and never an orphan). Its suggestions are the pages whose main
|
||||
// text mentions the orphan's title or its focus keyphrase; the first
|
||||
// suggestion's record names the tool that adds the link, else the fix is
|
||||
// a link in the site's code.
|
||||
func checkOrphans(res *Result, env *Env) []Finding {
|
||||
links := inbound(res)
|
||||
var out []Finding
|
||||
for _, key := range res.Order {
|
||||
p := res.Pages[key]
|
||||
if key == HomePath || !p.InSitemap || !env.indexable(res, p) || len(links[key]) > 0 {
|
||||
continue
|
||||
}
|
||||
rec := env.record(key)
|
||||
var phrases []string
|
||||
add := func(s string) {
|
||||
if s = normal(s); len(s) >= minPhrase {
|
||||
for _, have := range phrases {
|
||||
if have == s {
|
||||
return
|
||||
}
|
||||
}
|
||||
phrases = append(phrases, s)
|
||||
}
|
||||
}
|
||||
if rec != nil {
|
||||
add(rec.Title)
|
||||
}
|
||||
if h1 := p.Facts.H1(); len(h1) > 0 {
|
||||
add(h1[0])
|
||||
}
|
||||
add(p.Facts.Title)
|
||||
if env.Keyphrases != nil {
|
||||
add(env.Keyphrases[key])
|
||||
}
|
||||
var suggestions []map[string]any
|
||||
var first *Record
|
||||
for _, other := range res.Order {
|
||||
if len(suggestions) >= maxSuggestions {
|
||||
break
|
||||
}
|
||||
op := res.Pages[other]
|
||||
if other == key || !op.OK() {
|
||||
continue
|
||||
}
|
||||
text := normal(op.Facts.Text)
|
||||
for _, phrase := range phrases {
|
||||
if strings.Contains(text, phrase) {
|
||||
suggestions = append(suggestions, map[string]any{"url": other, "phrase": phrase})
|
||||
if first == nil {
|
||||
first = env.record(other)
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
f := Finding{Kind: KindOrphanPage, URL: key, Locale: locale(rec), Record: first, Details: map[string]any{"count": len(suggestions), "suggestions": suggestions}}
|
||||
if first != nil {
|
||||
f.Content = []string{pact.ContentToolEdit}
|
||||
}
|
||||
out = append(out, f)
|
||||
}
|
||||
return out
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"git.golem15.com/golem15/sm-seo-plugin/models"
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
@@ -26,6 +27,13 @@ const (
|
||||
// keeps Googlebot from (D-24): an error, since the sitemap asks for a
|
||||
// page the crawler may not fetch.
|
||||
IssueSitemapBlocked = "sitemap_blocked"
|
||||
// IssueImageAlt is a record image without alternative text that is
|
||||
// not marked decorative (D-85), from the content descriptors.
|
||||
IssueImageAlt = "image_alt"
|
||||
// IssueH1InContent is an H1 in the body of a record whose page renders
|
||||
// the title as the only H1 (descriptor TopHeading 2, D-50; Journal's
|
||||
// H1 setting on).
|
||||
IssueH1InContent = "h1_in_content"
|
||||
)
|
||||
|
||||
// The severities an issue may name.
|
||||
@@ -92,7 +100,92 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) {
|
||||
data.listed = append(data.listed, address)
|
||||
}
|
||||
sort.Strings(data.listed)
|
||||
return s.issuesOf(ctx, data, s.canonicalProblem)
|
||||
out, err := s.issuesOf(ctx, data, s.canonicalProblem)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
content, err := s.ContentIssues(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return append(out, content...), nil
|
||||
}
|
||||
|
||||
// contentIssuesPage is the page size of the content walk.
|
||||
const contentIssuesPage = 200
|
||||
|
||||
// ContentIssues are the CMS-data checks on the content descriptors of
|
||||
// every published record (D-85, D-50): image_alt for images with an empty
|
||||
// alt that are not decorative, and h1_in_content for an H1 in the body of
|
||||
// a record whose descriptor sets TopHeading 2 (off when the type sets no
|
||||
// rule, as Journal does with its H1 setting off). Each names the record
|
||||
// type's fitting tool (ContentDescriptor.Tools: image_alt or headings,
|
||||
// else edit); "" when the type names none.
|
||||
func (s *Service) ContentIssues(ctx context.Context) ([]Issue, error) {
|
||||
reg := s.Registry()
|
||||
if reg == nil {
|
||||
return nil, nil
|
||||
}
|
||||
var out []Issue
|
||||
for _, typ := range reg.ContentTypes() {
|
||||
if typ.Content == nil || typ.Content.Records == nil {
|
||||
continue
|
||||
}
|
||||
tool := func(ops ...string) string {
|
||||
for _, op := range ops {
|
||||
if t := strings.TrimSpace(typ.Content.Tools[op]); t != "" {
|
||||
return t
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
after := ""
|
||||
for {
|
||||
page, err := reg.Records(ctx, typ.Key, after, contentIssuesPage, s.UnprefixedLocale())
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, rec := range page.Records {
|
||||
if !rec.Published {
|
||||
continue
|
||||
}
|
||||
for _, l := range rec.Locales {
|
||||
if strings.TrimSpace(l.URL) == "" {
|
||||
continue
|
||||
}
|
||||
var missing []string
|
||||
for _, img := range l.Images {
|
||||
if !img.Decorative && strings.TrimSpace(img.Alt) == "" && img.URL != "" {
|
||||
missing = append(missing, img.URL)
|
||||
}
|
||||
}
|
||||
if len(missing) > 0 {
|
||||
out = append(out, Issue{Kind: IssueImageAlt, URL: l.URL, Locale: l.Locale, Tool: tool(pact.ContentToolImageAlt, pact.ContentToolEdit), Details: map[string]any{
|
||||
"count": len(missing), "images": missing, "type": typ.Key, "id": rec.ID,
|
||||
}})
|
||||
}
|
||||
if rec.TopHeading == 2 {
|
||||
var h1 []string
|
||||
for _, h := range l.Headings {
|
||||
if h.Level == 1 {
|
||||
h1 = append(h1, h.Text)
|
||||
}
|
||||
}
|
||||
if len(h1) > 0 {
|
||||
out = append(out, Issue{Kind: IssueH1InContent, URL: l.URL, Locale: l.Locale, Tool: tool(pact.ContentToolHeadings, pact.ContentToolEdit), Details: map[string]any{
|
||||
"count": len(h1), "h1": h1, "type": typ.Key, "id": rec.ID,
|
||||
}})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if page.Next == "" || page.Next == after || len(page.Records) == 0 {
|
||||
break
|
||||
}
|
||||
after = page.Next
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// issueData is what the issue report reads from the database: the SEO
|
||||
|
||||
@@ -247,6 +247,9 @@ func PageMenuItemType(service func() *Service) pact.MenuItemType {
|
||||
Controller: PagesController,
|
||||
Model: PageModel,
|
||||
Kind: pact.ContentKindPage,
|
||||
// A registry page's fields are changed through seo_update_page,
|
||||
// the tool the site audit names for its content (P33-38).
|
||||
Tools: map[string]string{pact.ContentToolEdit: "seo_update_page"},
|
||||
Records: func(ctx context.Context, in pact.ContentList) (pact.ContentPage, error) {
|
||||
s, err := svc()
|
||||
if err != nil {
|
||||
|
||||
126
crawler_test.go
126
crawler_test.go
@@ -59,7 +59,7 @@ var checkedKinds = map[string]bool{
|
||||
"h1_title_mismatch": true, "jsonld_invalid": true, "jsonld_multiple": true, "jsonld_required": true,
|
||||
"jsonld_recommended": true, "hreflang_reciprocal": true, "hreflang_error": true, "generic_anchor": true,
|
||||
"broken_link": true, "redirected_link": true, "image_alt": true, "image_size": true, "featured_lazy": true,
|
||||
"content_js": true,
|
||||
"content_js": true, "orphan_page": true,
|
||||
}
|
||||
|
||||
// onlyChecked keeps the "kind url" entries of checkedKinds.
|
||||
@@ -175,6 +175,10 @@ func TestCrawlerChecks(t *testing.T) {
|
||||
t.Fatalf("content_js %s = %+v, want check %s naming %q", url, i, check, what)
|
||||
}
|
||||
}
|
||||
detail("orphan_page", "/p/orphan", "count", 1)
|
||||
if i := findIssue(t, list, "orphan_page", "/p/orphan"); !strings.Contains(fmt.Sprint(i.Details["suggestions"]), "/p/mentions") {
|
||||
t.Fatalf("orphan suggestions = %v", i.Details)
|
||||
}
|
||||
content("/c/js-h1", "cms", "Record heading")
|
||||
content("/c/js-words", "cms", "of 400 words")
|
||||
content("/c/js-links", "cms", "/c/parent")
|
||||
@@ -241,3 +245,123 @@ func TestRenderSkipped(t *testing.T) {
|
||||
}
|
||||
findIssue(t, env.issues(t, token, ""), "h1_multiple", "/p/two-h1")
|
||||
}
|
||||
|
||||
// TestIssueMerge checks the CMS-data issues stored with the crawl and the
|
||||
// fix routes (SC-3, P33-38): before any crawl the list holds the CMS
|
||||
// issues; after it they are stored with the crawl as source cms; image_alt
|
||||
// names the record type's image_alt tool; h1_in_content appears for a post
|
||||
// and a category whose descriptor sets TopHeading 2 and not for TopHeading
|
||||
// 0, naming the headings tool; a skipped level in a category's pillar text
|
||||
// (a type without an edit tool) is fix code; no issue lacks a route. An
|
||||
// ignored fingerprint stays ignored in the next crawl, and a fixed page's
|
||||
// issue is gone from it.
|
||||
func TestIssueMerge(t *testing.T) {
|
||||
env := bootCrawlEnv(t)
|
||||
token := env.admin(t, true)
|
||||
setCrawlRecords(t, map[string][]crawlRecord{
|
||||
crawlPostType: {
|
||||
{id: "1", path: "/c/post-a", title: "Post A", topHeading: 2, headings: []pact.ContentHeading{{Level: 1, Text: "Body H1"}, {Level: 2, Text: "Part"}},
|
||||
images: []pact.ContentImage{{URL: "/img/a.jpg"}, {URL: "/img/deco.png", Decorative: true}}},
|
||||
{id: "2", path: "/c/post-b", title: "Post B", topHeading: 0, headings: []pact.ContentHeading{{Level: 1, Text: "Body H1 of B"}}},
|
||||
},
|
||||
crawlCategoryType: {
|
||||
{id: "10", path: "/cat/news", title: "News", topHeading: 2, headings: []pact.ContentHeading{{Level: 1, Text: "Pillar H1"}, {Level: 2, Text: "Intro"}, {Level: 4, Text: "Deep"}}},
|
||||
},
|
||||
})
|
||||
site := newFakeSite()
|
||||
site.sitemap("/", "/c/post-a", "/c/post-b", "/cat/news", "/p/fixme")
|
||||
site.plant(
|
||||
doc{path: "/", title: "Home", links: []string{"/c/post-a", "/c/post-b", "/cat/news", "/p/fixme"}},
|
||||
doc{path: "/c/post-a", title: "Post A", body: `<h2>Part</h2><img src="/img/a.jpg" alt="Photo" width="1" height="1">`},
|
||||
doc{path: "/c/post-b", title: "Post B"},
|
||||
doc{path: "/cat/news", title: "News", body: "<h2>Intro</h2><h4>Deep</h4>"},
|
||||
doc{path: "/p/fixme", title: "Fix me", noDesc: true},
|
||||
)
|
||||
|
||||
before := env.issues(t, token, "")
|
||||
if i := findIssue(t, before, "image_alt", "/c/post-a"); i.Source != "cms" || i.CrawlID != nil || i.Tool != "acme_set_alt" {
|
||||
t.Fatalf("CMS image_alt before any crawl = %+v", i)
|
||||
}
|
||||
|
||||
ctx := siteCtx(t, site)
|
||||
first := env.crawlOnce(t, token, ctx)
|
||||
list := env.issues(t, token, "")
|
||||
alt := findIssue(t, list, "image_alt", "/c/post-a")
|
||||
if alt.Source != "cms" || alt.CrawlID == nil || *alt.CrawlID != first || alt.Tool != "acme_set_alt" || alt.Fix != "tool" {
|
||||
t.Fatalf("image_alt = %+v", alt)
|
||||
}
|
||||
if i := findIssue(t, list, "h1_in_content", "/c/post-a"); i.Tool != "acme_fix_headings" || i.Source != "cms" || i.Severity != "warning" {
|
||||
t.Fatalf("h1_in_content post = %+v", i)
|
||||
}
|
||||
if i := findIssue(t, list, "h1_in_content", "/cat/news"); i.Tool != "acme_category_headings" {
|
||||
t.Fatalf("h1_in_content category = %+v", i)
|
||||
}
|
||||
for _, i := range list {
|
||||
if i.Kind == "h1_in_content" && i.URL == "/c/post-b" {
|
||||
t.Fatalf("h1_in_content for TopHeading 0: %+v", i)
|
||||
}
|
||||
if i.Fix == "tool" && i.Tool == "" || i.Fix != "tool" && i.Fix != "code" {
|
||||
t.Fatalf("issue without a route: %+v", i)
|
||||
}
|
||||
if i.Fix == "code" && i.Details["what"] == nil {
|
||||
t.Fatalf("fix code without what: %+v", i)
|
||||
}
|
||||
}
|
||||
skipped := findIssue(t, list, "heading_skipped", "/cat/news")
|
||||
if skipped.Fix != "code" || skipped.Tool != "" {
|
||||
t.Fatalf("heading_skipped in pillar text = %+v", skipped)
|
||||
}
|
||||
findIssue(t, list, "missing_description", "/p/fixme")
|
||||
|
||||
// Ignore the skipped level, fix the description, crawl again.
|
||||
var fp string
|
||||
if err := env.db.Raw(`SELECT fingerprint FROM golem15_seo_issues WHERE id = ?`, skipped.ID).Scan(&fp).Error; err != nil || fp == "" {
|
||||
t.Fatalf("fingerprint: %q %v", fp, err)
|
||||
}
|
||||
if err := env.db.Exec(`INSERT INTO golem15_seo_issue_ignores (fingerprint, ignored_at) VALUES (?, NOW())`, fp).Error; err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
site.plant(doc{path: "/p/fixme", title: "Fix me"})
|
||||
second := env.crawlOnce(t, token, ctx)
|
||||
list = env.issues(t, token, "")
|
||||
again := findIssue(t, list, "heading_skipped", "/cat/news")
|
||||
if !again.Ignored || again.CrawlID == nil || *again.CrawlID != second {
|
||||
t.Fatalf("ignored issue in the next crawl = %+v", again)
|
||||
}
|
||||
for _, i := range list {
|
||||
if i.URL == "/p/fixme" {
|
||||
t.Fatalf("fixed page still has %+v", i)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestStagingCrawl checks RESEARCH Pitfall 12: on a non-production
|
||||
// environment every page carries the staging lock's noindex, and no
|
||||
// noindex-derived issue is reported; the pages still count as indexable.
|
||||
func TestStagingCrawl(t *testing.T) {
|
||||
env := bootCrawlEnv(t, "SUMMER_ENV=staging")
|
||||
if !env.seoPlugin(t).Service().Staging() {
|
||||
t.Fatal("the environment is not staging")
|
||||
}
|
||||
token := env.admin(t, true)
|
||||
site := newFakeSite()
|
||||
site.sitemap("/", "/a", "/b")
|
||||
for _, d := range []doc{
|
||||
{path: "/", title: "Home", links: []string{"/a", "/b", "/c"}},
|
||||
{path: "/a", title: "Same"},
|
||||
{path: "/b", title: "Same"},
|
||||
{path: "/c", title: "Unlisted"},
|
||||
} {
|
||||
d.robots = "noindex, nofollow"
|
||||
site.set(d.path, 200, d.render(), "X-Robots-Tag", "noindex")
|
||||
}
|
||||
env.crawlOnce(t, token, siteCtx(t, site))
|
||||
list := env.issues(t, token, "")
|
||||
for _, i := range list {
|
||||
if i.Kind == "sitemap_noindex" || i.Kind == "canonical_noindex" || i.Kind == "noindex_blocked" {
|
||||
t.Fatalf("staging noindex reported: %+v", i)
|
||||
}
|
||||
}
|
||||
findIssue(t, list, "duplicate_title", "/a")
|
||||
findIssue(t, list, "sitemap_missing", "/c")
|
||||
}
|
||||
|
||||
@@ -526,6 +526,10 @@ crawls:
|
||||
already_running: A crawl is already running (started :time). Wait for it, or cancel it in SEO › Crawls.
|
||||
no_site_url: The site address for crawls is not configured (golem15.seo.site_url). Ask the administrator to set it.
|
||||
issues:
|
||||
kind_orphan_page: "Orphan page (no internal links)"
|
||||
sum_orphan_page: ":count suggestions"
|
||||
kind_h1_in_content: "H1 in the content"
|
||||
sum_h1_in_content: ":count H1 headings"
|
||||
kind_jsonld_invalid: "Structured data cannot be read"
|
||||
sum_jsonld_invalid: ":error"
|
||||
kind_jsonld_multiple: "More than one JSON-LD graph"
|
||||
|
||||
@@ -540,6 +540,10 @@ crawls:
|
||||
already_running: Skanowanie już trwa (rozpoczęte :time). Poczekaj na nie lub anuluj je w SEO › Skanowania.
|
||||
no_site_url: Adres witryny do skanowania nie jest skonfigurowany (golem15.seo.site_url). Poproś administratora o jego ustawienie.
|
||||
issues:
|
||||
kind_orphan_page: "Strona osierocona (brak linków wewnętrznych)"
|
||||
sum_orphan_page: "Propozycje: :count"
|
||||
kind_h1_in_content: "H1 w treści"
|
||||
sum_h1_in_content: "Nagłówki H1: :count"
|
||||
kind_jsonld_invalid: "Nie można odczytać danych strukturalnych"
|
||||
sum_jsonld_invalid: ":error"
|
||||
kind_jsonld_multiple: "Więcej niż jeden graf JSON-LD"
|
||||
|
||||
@@ -8,7 +8,8 @@ import (
|
||||
var _ pact.HasMenuItemTypes = (*Plugin)(nil)
|
||||
|
||||
// MenuItemTypes registers the page registry's type seo-page (D-33): its
|
||||
// content descriptor (controller golem15.seo.pages, Kind page) lists,
|
||||
// content descriptor (controller golem15.seo.pages, Kind page, Tools edit
|
||||
// seo_update_page for the site audit's fix routes) lists,
|
||||
// describes and matches the registry pages, so every page gets the SEO
|
||||
// tab, resolves ok and is listed in the sitemaps grouped by its kind.
|
||||
func (p *Plugin) MenuItemTypes() []pact.MenuItemType {
|
||||
|
||||
Reference in New Issue
Block a user