refactor: build the issue report from loaded data

Issues loads the entries, rules, redirects, robots row and noindex
addresses, then issuesOf builds the report and blockedNoindex checks the
addresses against robots.txt, so the report's rules can be unit-tested
without a database. No behaviour change.

classes/issues.go

@@ -54,15 +54,44 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) {
 	if err != nil {
 		return nil, err
 	}
+	var data issueData
+	if err := db.Where("url <> ''").Order("url ASC, locale ASC").Find(&data.entries).Error; err != nil {
+		return nil, err
+	}
+	if err := db.Order("url ASC").Find(&data.rules).Error; err != nil {
+		return nil, err
+	}
+	data.locale = s.DefaultLocale(ctx, db)
+	if data.redirects, err = LoadRedirects(db); err != nil {
+		return nil, err
+	}
+	if data.robots, err = LoadRobots(db); err != nil {
+		return nil, err
+	}
+	if data.noindex, err = noindexAddresses(db); err != nil {
+		return nil, err
+	}
+	return s.issuesOf(ctx, data, s.canonicalProblem)
+}
+
+// issueData is what the issue report reads from the database: the SEO
+// entries with an address (by url and locale), the URL rules (by url), the
+// default content locale, the redirects, the robots row and the noindex
+// addresses.
+type issueData struct {
+	entries   []models.Entry
+	rules     []models.Rule
+	locale    string
+	redirects []models.Redirect
+	robots    models.Robots
+	noindex   []string
+}
+
+// issuesOf builds the report from data; canonical classifies a canonical
+// value (canonicalProblem).
+func (s *Service) issuesOf(ctx context.Context, data issueData, canonical func(context.Context, string) (string, string, error)) ([]Issue, error) {
 	var out []Issue
-	var entries []models.Entry
-	if err := db.Where("url <> ''").Order("url ASC, locale ASC").Find(&entries).Error; err != nil {
-		return nil, err
-	}
-	var rules []models.Rule
-	if err := db.Order("url ASC").Find(&rules).Error; err != nil {
-		return nil, err
-	}
+	entries, rules := data.entries, data.rules
 
 	// Missing and duplicate effective values.
 	type place struct{ url, locale string }
@@ -82,7 +111,7 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) {
 			descs[k] = append(descs[k], place{e.URL, e.Locale})
 		}
 	}
-	locale := s.DefaultLocale(ctx, db)
+	locale := data.locale
 	for _, r := range rules {
 		if r.Match != models.MatchExact {
 			continue
@@ -148,10 +177,7 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) {
 	}
 
 	// Redirect chains.
-	redirects, err := LoadRedirects(db)
-	if err != nil {
-		return nil, err
-	}
+	redirects := data.redirects
 	set := NewRedirectSet(redirects)
 	site := s.SiteURL()
 	for _, r := range redirects {
@@ -175,8 +201,8 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) {
 	}
 
 	// Canonicals pointing to a noindex or redirected address.
-	canonical := func(u, loc, value, tool string, details map[string]any) error {
-		kind, target, err := s.canonicalProblem(ctx, value)
+	addCanonical := func(u, loc, value, tool string, details map[string]any) error {
+		kind, target, err := canonical(ctx, value)
 		if err != nil || kind == "" {
 			return err
 		}
@@ -191,26 +217,18 @@ func (s *Service) Issues(ctx context.Context) ([]Issue, error) {
 		return nil
 	}
 	for _, e := range entries {
-		if err := canonical(e.URL, e.Locale, e.Canonical, toolSetURLSEO, nil); err != nil {
+		if err := addCanonical(e.URL, e.Locale, e.Canonical, toolSetURLSEO, nil); err != nil {
 			return nil, err
 		}
 	}
 	for _, r := range rules {
-		if err := canonical(r.URL, "", r.Canonical, toolUpdateRule, map[string]any{"rule_id": r.ID}); err != nil {
+		if err := addCanonical(r.URL, "", r.Canonical, toolUpdateRule, map[string]any{"rule_id": r.ID}); err != nil {
 			return nil, err
 		}
 	}
 
 	// Noindex addresses robots.txt blocks.
-	robots, err := LoadRobots(db)
-	if err != nil {
-		return nil, err
-	}
-	blocked, err := s.BlockedNoindex(db, robots.Content, robots.Bots)
-	if err != nil {
-		return nil, err
-	}
-	for _, u := range blocked {
+	for _, u := range s.blockedNoindex(data.noindex, data.robots.Content, data.robots.Bots) {
 		out = append(out, Issue{Kind: IssueNoindexBlocked, URL: u, Tool: toolUpdateRobots})
 	}
 

classes/robots.go

@@ -221,11 +221,17 @@ func (s *Service) BlockedNoindex(db *gorm.DB, content string, bots models.BotSwi
 	if err != nil {
 		return nil, err
 	}
+	return s.blockedNoindex(urls, content, bots), nil
+}
+
+// blockedNoindex lists the addresses of urls Googlebot may not fetch under
+// the production file composed from content and bots.
+func (s *Service) blockedNoindex(urls []string, content string, bots models.BotSwitches) []string {
 	var out []string
 	for _, u := range urls {
 		if !s.TestRobots(content, bots, "Googlebot", u).Allowed {
 			out = append(out, u)
 		}
 	}
-	return out, nil
+	return out
 }