diff --git a/README.md b/README.md index 4268821..a09b26d 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A blog engine where you write first and categorize never. -Type two sentences without a title and it becomes a thought. Paste a URL with commentary and it becomes a link. Write something long with a title and it becomes an article. Ask a question and it becomes an AMA. Four content types, inferred automatically from what you write. No database. No build step. +Type two sentences without a title and it becomes a thought. Paste a URL with commentary and it becomes a link. Write something long with a title and it becomes an article. Ask a question and it becomes an AMA. Plus a fifth type — page — for evergreen content (explicit `type: page` in frontmatter, served at `/p/`, listed at `/p`). Four inferred, one explicit. No database. No build step. ## Quick Start diff --git a/docs/configuration.md b/docs/configuration.md index 19e0bbb..9d603de 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -144,7 +144,7 @@ The slug determines the URL: `/p/run-your-own` in this example. Pages support th - **URL:** `/p/` is canonical. Legacy `/writing/` requests for `type: page` articles 301-redirect to `/p/`. - **Feed exclusion:** pages do not appear in `/writing`, RSS (`/feed.xml`), JSON Feed (`/feed.json`), `/tags/`, or `/categories/`. -- **Sitemap:** pages are not currently emitted in the sitemap article section. Search engines discover pages via canonical links from articles or via direct URL. +- **Sitemap:** pages are emitted in `sitemap.xml` with their canonical `/p/` URLs, alongside a static `/p` index entry. `/about` is also included via the same predicate-aware logic (v3.14.0+). - **Search:** pages remain indexed; readers can find them via `/search?q=...` and follow the result to `/p/`. - **Date and "Updated" line:** hidden in the rendered page. Pages are evergreen, not dated. - **Tags:** rendered on the page itself but do not surface the page in tag indexes. @@ -160,10 +160,8 @@ Change `type: article` → `type: page` in the frontmatter and save. The article ### Future enhancements -- Auto-populated nav slot listing pages (v3.14.0+) -- `/p` index page (v3.14.0+; currently returns 404) -- `nav_priority` ordering frontmatter (v3.14.0+) -- Compose form "new page" affordance (v3.14.0+; for now, author pages by dropping markdown into `articles/` or editing an existing page via `/compose/edit/`) +- `nav_priority` ordering frontmatter (v3.14.0+ deferred; for now pages are listed alphabetically on `/p`) +- Compose form "new page" affordance (v3.14.0+ deferred; for now, author pages by dropping markdown into `articles/` or editing an existing page via `/compose/edit/`) ## Admin @@ -234,10 +232,10 @@ Operator-controllable copy on the AMA (Ask Me Anything) submission overlay. All | Variable | Default | Description | |----------|---------|-------------| -| `AMA_PAGE_HEADING` | `Ask me anything` | Heading shown at the top of the AMA sheet. | -| `AMA_PAGE_INTRO` | `Curious about something? Submit a question and I'll answer it publicly.` | Intro paragraph rendered between the heading and the form. | +| `AMA_PAGE_HEADING` | `Ask me anything` | Heading shown at the top of the AMA sheet AND in the `/about` reach section (v3.14.0+). Setting this empty in `.env` falls back to the default — hiding the AMA half requires overriding `about.html` via `TEMPLATES_PATH`. | +| `AMA_PAGE_INTRO` | `Curious about something? Submit a question and I'll answer it publicly.` | Intro paragraph rendered between the heading and the form on both the AMA sheet and the `/about` reach section (v3.14.0+). | | `AMA_FORM_PLACEHOLDER` | `What would you like to know?` | Placeholder for the question textarea. | -| `AMA_SUBMIT_LABEL` | `Submit Question` | Label on the submit button. | +| `AMA_SUBMIT_LABEL` | `Submit Question` | Label on the submit button on both the AMA sheet and the `/about` reach section (v3.14.0+). | | `AMA_THANKYOU_COPY` | `Question submitted! It will appear once answered.` | Toast shown after a successful submission. | ## Logging diff --git a/docs/design.md b/docs/design.md index a2b8582..a726ee2 100644 --- a/docs/design.md +++ b/docs/design.md @@ -101,9 +101,9 @@ The voice is **conversational and direct**. It sounds like a person, not a produ --- -## The Three Streams +## Content Types -MarkGo's content model has three types, inferred automatically from what you write. You never pick a type from a dropdown — the system figures it out from the shape of your content. This is MarkGo's most distinctive design decision. +MarkGo's content model has five types: four inferred automatically from what you write, plus pages for explicit evergreen content. You never pick a type from a dropdown — the system figures it out from the shape of your content. This is MarkGo's most distinctive design decision. **Thoughts** — No title, under 100 words. A fleeting idea, a reaction, a note-to-self that's worth sharing. Displayed as a simple text card with a left accent stripe in `--color-primary`. The full thought is visible in the feed — no teaser, no truncation. Shows relative time ("2 hours ago") and tags. @@ -125,7 +125,7 @@ The inference rules are intentionally simple (see `internal/services/article/inf For pages, the type field must be explicit (`type: page`) — pages have no inferable signal and shouldn't drift into existence by accident. The other four types (article/thought/link/ama) infer freely. -The first four types live in the same feed, filterable by type via server-rendered `` tag pills with query parameters (`/?type=thought`). Pages live outside the feed entirely; readers reach them by direct link or search. +The first four types live in the same feed, filterable by type via server-rendered `` tag pills with query parameters (`/?type=thought`). Pages live outside the feed entirely; readers reach them via the `/p` index (linked from the footer), direct link, sitemap, or search. --- diff --git a/internal/commands/serve/command.go b/internal/commands/serve/command.go index 8853d44..75f40af 100644 --- a/internal/commands/serve/command.go +++ b/internal/commands/serve/command.go @@ -403,6 +403,7 @@ func setupRoutes(router *gin.Engine, h *handlers.Router, sessionStore *middlewar registerGET(router, "/", h.Feed.Home) registerGET(router, "/writing", h.Post.Articles) registerGET(router, "/writing/:slug", h.Post.Article) + registerGET(router, "/p", h.Post.Pages) registerGET(router, "/p/:slug", h.Post.Page) registerGET(router, "/tags", h.Taxonomy.Tags) registerGET(router, "/tags/:tag", h.Taxonomy.ArticlesByTag) @@ -554,6 +555,7 @@ func setupTemplates(router *gin.Engine, templateService *services.TemplateServic "admin_ama.html", "category.html", "tag.html", + "pages.html", } for _, tmplName := range requiredTemplates { diff --git a/internal/handlers/about_handler.go b/internal/handlers/about_handler.go index b67132f..2f73933 100644 --- a/internal/handlers/about_handler.go +++ b/internal/handlers/about_handler.go @@ -30,7 +30,21 @@ func NewAboutHandler( } } -// ShowAbout handles the GET /about route. +// ShowAbout handles the GET /about route. Template data keys consumed by +// about.html: +// +// Identity: about_avatar, about_tagline, about_location +// Bio: bio_html (rendered HTML; absent if neither about.md nor +// ABOUT_BIO set) +// Social: social_links []socialLink, has_social +// Contact: has_contact (BLOG_AUTHOR_EMAIL set), +// has_contact_form (full SMTP-backed form available) +// Reach: about_ama_heading, about_ama_intro, about_ama_label +// (v3.14.0+, closes #75 — AMA copy from the existing v3.11.0 +// AMA_PAGE_* env vars so /ama and /about speak in the same +// operator voice; AMA half always renders, see below) +// +// Plus standard base-template keys via buildBaseTemplateData. func (h *AboutHandler) ShowAbout(c *gin.Context) { cfg := h.config data := h.buildBaseTemplateData("About - " + cfg.Blog.Title) @@ -78,6 +92,17 @@ func (h *AboutHandler) ShowAbout(c *gin.Context) { data["has_contact"] = hasEmail data["has_contact_form"] = hasContactForm + // Reach section (v3.14.0+): operator-voiced AMA promo reusing the + // v3.11.0 AMA_PAGE_* env vars. The AMA card in about.html renders + // unconditionally regardless of has_contact_form, matching pre-v3.14.0 + // behavior where /about always showed an AMA section. The mailto card + // inside about-reach is gated on has_contact && !has_contact_form (full + // SMTP form supplants the mailto fallback). Hiding the AMA card + // requires overriding about.html via TEMPLATES_PATH. + data["about_ama_heading"] = cfg.AMA.PageHeading + data["about_ama_intro"] = cfg.AMA.PageIntro + data["about_ama_label"] = cfg.AMA.SubmitLabel + h.enhanceTemplateDataWithSEO(data, "/about") h.renderHTML(c, http.StatusOK, "base.html", data) } diff --git a/internal/handlers/about_handler_test.go b/internal/handlers/about_handler_test.go index d493c87..4de6757 100644 --- a/internal/handlers/about_handler_test.go +++ b/internal/handlers/about_handler_test.go @@ -9,112 +9,161 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/1mb-dev/markgo/internal/config" ) -func createTestAboutHandler(cfg *config.Config) *AboutHandler { +func createTestAboutHandler(cfg *config.Config) (*AboutHandler, *MockTemplateService) { if cfg == nil { cfg = createTestConfig() } logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - base := NewBaseHandler(cfg, logger, &MockTemplateService{}, &BuildInfo{Version: "test"}, &MockSEOService{}) - return NewAboutHandler(base, &MockArticleService{}, &MockMarkdownRenderer{}) + mockTpl := &MockTemplateService{} + base := NewBaseHandler(cfg, logger, mockTpl, &BuildInfo{Version: "test"}, &MockSEOService{}) + return NewAboutHandler(base, &MockArticleService{}, &MockMarkdownRenderer{}), mockTpl } -func TestAboutHandler(t *testing.T) { - t.Run("minimal config shows author name", func(t *testing.T) { - cfg := &config.Config{ +// TestAboutHandler_TemplateData verifies the ShowAbout data contract +// (v3.14.0+ closes #75). The AMA copy keys must reach the template +// verbatim from the v3.11.0 AMA_PAGE_* env vars (operator voice), and +// has_contact / has_contact_form together gate the mailto card inside +// about-reach. The AMA card in the template always renders regardless +// of has_contact_form — matches pre-v3.14.0 behavior where /about +// always showed an AMA section. +func TestAboutHandler_TemplateData(t *testing.T) { + baseCfg := func() *config.Config { + return &config.Config{ Environment: "test", BaseURL: "http://localhost:3000", Blog: config.BlogConfig{ - Title: "Test Blog", - Author: "Test Author", + Title: "Test Blog", + Author: "Test Author", + AuthorEmail: "author@example.com", + }, + AMA: config.AMAConfig{ + PageHeading: "Ask me anything", + PageIntro: "Curious about something?", + SubmitLabel: "Submit Question", }, } + } - handler := createTestAboutHandler(cfg) - - router := gin.New() - router.GET("/about", handler.ShowAbout) - - req := httptest.NewRequest("GET", "/about", http.NoBody) - req.Header.Set("Accept", "application/json") - w := httptest.NewRecorder() - - router.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - }) - - t.Run("full config sets all template data", func(t *testing.T) { - cfg := &config.Config{ - Environment: "test", - BaseURL: "http://localhost:3000", - Blog: config.BlogConfig{ - Title: "Test Blog", - Author: "Jane Doe", - AuthorEmail: "jane@example.com", + tests := []struct { + name string + mutate func(*config.Config) + wantHasContact bool + wantHasContactForm bool + wantHeading string + wantIntro string + wantLabel string + }{ + { + name: "defaults: AMA + mailto", + mutate: func(*config.Config) {}, + wantHasContact: true, + wantHeading: "Ask me anything", + wantIntro: "Curious about something?", + wantLabel: "Submit Question", + }, + { + name: "no email — AMA still renders solo", + mutate: func(c *config.Config) { + c.Blog.AuthorEmail = "" }, - About: config.AboutConfig{ - Avatar: "img/avatar.jpg", - Tagline: "Building things", - Location: "San Francisco, CA", - GitHub: "janedoe", - Twitter: "@janedoe", - LinkedIn: "https://linkedin.com/in/janedoe", - Website: "janedoe.com", + wantHasContact: false, + wantHeading: "Ask me anything", + wantIntro: "Curious about something?", + wantLabel: "Submit Question", + }, + { + name: "custom AMA copy reaches template (operator voice)", + mutate: func(c *config.Config) { + c.AMA.PageHeading = "Hit me up" + c.AMA.PageIntro = "Got something on your mind?" + c.AMA.SubmitLabel = "Send it" }, - Email: config.EmailConfig{ - Host: "smtp.example.com", - Username: "user", + wantHasContact: true, + wantHeading: "Hit me up", + wantIntro: "Got something on your mind?", + wantLabel: "Send it", + }, + { + // Regression guard for the v3.14.0 PR #76 finding: AMA card + // must keep rendering even when SMTP is fully configured. + // has_contact_form=true gates the mailto card but the + // template (about.html) renders AMA unconditionally. + name: "SMTP configured — AMA still rendered, mailto suppressed", + mutate: func(c *config.Config) { + c.Email.Host = "smtp.example.com" + c.Email.Username = "user" }, - } - - handler := createTestAboutHandler(cfg) - - router := gin.New() - router.GET("/about", handler.ShowAbout) - - req := httptest.NewRequest("GET", "/about", http.NoBody) - req.Header.Set("Accept", "application/json") - w := httptest.NewRecorder() - - router.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - }) + wantHasContact: true, + wantHasContactForm: true, + wantHeading: "Ask me anything", + wantIntro: "Curious about something?", + wantLabel: "Submit Question", + }, + } - t.Run("contact section hidden without email", func(t *testing.T) { - cfg := &config.Config{ - Environment: "test", - BaseURL: "http://localhost:3000", - Blog: config.BlogConfig{ - Title: "Test Blog", - Author: "Test Author", - AuthorEmail: "", // no email - }, - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := baseCfg() + tt.mutate(cfg) + handler, mockTpl := createTestAboutHandler(cfg) - handler := createTestAboutHandler(cfg) + router := gin.New() + router.GET("/about", handler.ShowAbout) - router := gin.New() - router.GET("/about", handler.ShowAbout) + req := httptest.NewRequest(http.MethodGet, "/about", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) - req := httptest.NewRequest("GET", "/about", http.NoBody) - req.Header.Set("Accept", "application/json") - w := httptest.NewRecorder() + require.Equal(t, http.StatusOK, w.Code) + require.NotNil(t, mockTpl.LastData, "ShowAbout must invoke the template renderer") - router.ServeHTTP(w, req) + assert.Equal(t, tt.wantHasContact, mockTpl.LastData["has_contact"], "has_contact (BLOG_AUTHOR_EMAIL set) feeds the mailto card visibility") + assert.Equal(t, tt.wantHasContactForm, mockTpl.LastData["has_contact_form"], "has_contact_form (SMTP configured) gates the full contact form section") + assert.Equal(t, tt.wantHeading, mockTpl.LastData["about_ama_heading"], "operator voice — AMA heading reaches the template verbatim") + assert.Equal(t, tt.wantIntro, mockTpl.LastData["about_ama_intro"]) + assert.Equal(t, tt.wantLabel, mockTpl.LastData["about_ama_label"]) + }) + } +} - assert.Equal(t, http.StatusOK, w.Code) - }) +// TestAboutHandler_Identity verifies the identity/social/bio composition +// path still produces expected data after the v3.14.0 reach addition. +func TestAboutHandler_Identity(t *testing.T) { + cfg := &config.Config{ + Environment: "test", + BaseURL: "http://localhost:3000", + Blog: config.BlogConfig{Title: "Test Blog", Author: "Jane Doe"}, + About: config.AboutConfig{ + Avatar: "img/avatar.jpg", + Tagline: "Building things", + Location: "San Francisco, CA", + GitHub: "janedoe", + }, + } + handler, mockTpl := createTestAboutHandler(cfg) + + router := gin.New() + router.GET("/about", handler.ShowAbout) + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/about", http.NoBody)) + + require.Equal(t, http.StatusOK, w.Code) + require.NotNil(t, mockTpl.LastData) + assert.Equal(t, "img/avatar.jpg", mockTpl.LastData["about_avatar"]) + assert.Equal(t, "Building things", mockTpl.LastData["about_tagline"]) + assert.Equal(t, "San Francisco, CA", mockTpl.LastData["about_location"]) + assert.Equal(t, true, mockTpl.LastData["has_social"]) } func TestBuildSocialLinks(t *testing.T) { t.Run("no social links configured", func(t *testing.T) { - handler := createTestAboutHandler(nil) + handler, _ := createTestAboutHandler(nil) links := handler.buildSocialLinks() assert.Empty(t, links) }) @@ -122,7 +171,7 @@ func TestBuildSocialLinks(t *testing.T) { t.Run("normalizes github username to full URL", func(t *testing.T) { cfg := createTestConfig() cfg.About.GitHub = "testuser" - handler := createTestAboutHandler(cfg) + handler, _ := createTestAboutHandler(cfg) links := handler.buildSocialLinks() assert.Len(t, links, 1) @@ -133,7 +182,7 @@ func TestBuildSocialLinks(t *testing.T) { t.Run("preserves full URLs", func(t *testing.T) { cfg := createTestConfig() cfg.About.GitHub = "https://github.com/testuser" - handler := createTestAboutHandler(cfg) + handler, _ := createTestAboutHandler(cfg) links := handler.buildSocialLinks() assert.Len(t, links, 1) @@ -143,7 +192,7 @@ func TestBuildSocialLinks(t *testing.T) { t.Run("normalizes twitter handle", func(t *testing.T) { cfg := createTestConfig() cfg.About.Twitter = "@janedoe" - handler := createTestAboutHandler(cfg) + handler, _ := createTestAboutHandler(cfg) links := handler.buildSocialLinks() assert.Len(t, links, 1) @@ -157,7 +206,7 @@ func TestBuildSocialLinks(t *testing.T) { cfg.About.LinkedIn = "https://linkedin.com/in/user" cfg.About.Mastodon = "https://mastodon.social/@user" cfg.About.Website = "example.com" - handler := createTestAboutHandler(cfg) + handler, _ := createTestAboutHandler(cfg) links := handler.buildSocialLinks() assert.Len(t, links, 5) diff --git a/internal/handlers/article_test.go b/internal/handlers/article_test.go index 780c536..8215dba 100644 --- a/internal/handlers/article_test.go +++ b/internal/handlers/article_test.go @@ -35,6 +35,7 @@ func (m *TestArticleService) GetArticleBySlug(slug string) (*models.Article, err } return nil, apperrors.ErrArticleNotFound } +func (m *TestArticleService) GetPages() []*models.Article { return m.articles } func (m *TestArticleService) GetArticlesByTag(_ string) []*models.Article { return m.articles } func (m *TestArticleService) GetArticlesByCategory(_ string) []*models.Article { return m.articles } func (m *TestArticleService) GetArticlesForFeed(_ int) []*models.Article { return m.articles } @@ -224,6 +225,98 @@ func TestPage_HEAD_Supported(t *testing.T) { assert.Equal(t, http.StatusOK, w.Code) } +// TestPages_Handler verifies /p index renders pages sorted alphabetically +// by title (case-insensitive), shows an empty state when no pages exist, +// and exposes the expected template-data keys for the pages.html template. +func TestPages_Handler(t *testing.T) { + t.Run("empty pages list renders empty state", func(t *testing.T) { + base, svc := createTestBase() + svc.articles = nil + + mockTpl := requireMockTemplateService(t, base) + mockTpl.LastData = nil + + router := gin.New() + router.GET("/p", NewPostHandler(base, svc).Pages) + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/p", http.NoBody)) + + require.Equal(t, http.StatusOK, w.Code) + require.NotNil(t, mockTpl.LastData) + assert.Equal(t, "pages", mockTpl.LastData["template"]) + assert.Equal(t, 0, mockTpl.LastData["pageCount"]) + }) + + t.Run("alphabetical sort case-insensitive", func(t *testing.T) { + base, svc := createTestBase() + // Inject in non-alphabetical order with mixed case to verify + // case-insensitive lowercase comparison. + svc.articles = []*models.Article{ + {Slug: "zoo", Title: "Zoo", Type: "page", Date: time.Now()}, + {Slug: "alpha", Title: "alpha", Type: "page", Date: time.Now()}, + {Slug: "beta", Title: "Beta", Type: "page", Date: time.Now()}, + } + + mockTpl := requireMockTemplateService(t, base) + mockTpl.LastData = nil + + router := gin.New() + router.GET("/p", NewPostHandler(base, svc).Pages) + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/p", http.NoBody)) + + require.Equal(t, http.StatusOK, w.Code) + require.NotNil(t, mockTpl.LastData) + assert.Equal(t, 3, mockTpl.LastData["pageCount"]) + + pages, ok := mockTpl.LastData["pages"].([]*models.Article) + require.True(t, ok, "pages must be []*models.Article, got %T", mockTpl.LastData["pages"]) + require.Len(t, pages, 3) + assert.Equal(t, "alpha", pages[0].Title, "case-insensitive sort: 'alpha' first") + assert.Equal(t, "Beta", pages[1].Title) + assert.Equal(t, "Zoo", pages[2].Title) + }) + + t.Run("canonicalPath set to /p", func(t *testing.T) { + base, svc := createTestBase() + svc.articles = []*models.Article{{Slug: "x", Title: "X", Type: "page", Date: time.Now()}} + + mockTpl := requireMockTemplateService(t, base) + mockTpl.LastData = nil + + router := gin.New() + router.GET("/p", NewPostHandler(base, svc).Pages) + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/p", http.NoBody)) + + require.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "/p", mockTpl.LastData["canonicalPath"]) + }) +} + +// TestPages_HEAD_Supported — HEAD verb on /p returns the same status as GET. +func TestPages_HEAD_Supported(t *testing.T) { + base, svc := createTestBase() + svc.articles = []*models.Article{{Slug: "ok", Title: "OK", Type: "page", Date: time.Now()}} + router := gin.New() + router.HEAD("/p", NewPostHandler(base, svc).Pages) + + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodHead, "/p", http.NoBody)) + assert.Equal(t, http.StatusOK, w.Code) +} + +// requireMockTemplateService extracts the MockTemplateService from a +// BaseHandler for data-inspection tests. Fails the test if the base +// wasn't wired with a mock (which would render real templates and +// invalidate the data-capture). +func requireMockTemplateService(t *testing.T, base *BaseHandler) *MockTemplateService { + t.Helper() + mockTpl, ok := base.templateService.(*MockTemplateService) + require.True(t, ok, "createTestBase must wire a MockTemplateService for data inspection") + return mockTpl +} + // TestPage_BreadcrumbsExcludeWriting — pages live outside /writing, so // breadcrumbs must not include the "Writing" intermediate level (which // getArticleData would otherwise inject by default). diff --git a/internal/handlers/compose.go b/internal/handlers/compose.go index ceffd44..78bed58 100644 --- a/internal/handlers/compose.go +++ b/internal/handlers/compose.go @@ -12,7 +12,9 @@ import ( "github.com/gin-gonic/gin" apperrors "github.com/1mb-dev/markgo/internal/errors" + "github.com/1mb-dev/markgo/internal/models" "github.com/1mb-dev/markgo/internal/services" + articlepkg "github.com/1mb-dev/markgo/internal/services/article" "github.com/1mb-dev/markgo/internal/services/compose" ) @@ -53,6 +55,21 @@ func NewComposeHandler( } } +// canonicalPathForSlug resolves the canonical URL for a composed or +// edited post by looking up the article in the in-memory store. Existing +// type:page articles edited via /compose/edit/:slug must redirect to +// /p/, not /writing/ (which would hit the v3.13.0 301 and +// add a needless round-trip). For the rare case where lookup fails +// (article not yet reloaded into the repo after a failed reload), falls +// back to a synthetic empty-Type article — /writing/. The 301 +// redirect catches that fallback if it ever surfaces a type:page slug. +func (h *ComposeHandler) canonicalPathForSlug(slug string) string { + if art, err := h.articleService.GetArticleBySlug(slug); err == nil { + return articlepkg.CanonicalURLFor(art) + } + return articlepkg.CanonicalURLFor(&models.Article{Slug: slug}) +} + // ShowCompose renders the compose form. // Reads optional query params (title, text, url) to support PWA share_target. func (h *ComposeHandler) ShowCompose(c *gin.Context) { @@ -189,7 +206,7 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) { // Redirect to the edited article, or feed if reload failed (stale cache would show old version) if reloadOK { - c.Redirect(http.StatusSeeOther, "/writing/"+slug) + c.Redirect(http.StatusSeeOther, h.canonicalPathForSlug(slug)) } else { c.Redirect(http.StatusSeeOther, "/") } @@ -248,7 +265,7 @@ func (h *ComposeHandler) HandleSubmit(c *gin.Context) { if !reloadOK || input.Title == "" { c.Redirect(http.StatusSeeOther, "/") } else { - c.Redirect(http.StatusSeeOther, "/writing/"+slug) + c.Redirect(http.StatusSeeOther, h.canonicalPathForSlug(slug)) } } @@ -337,7 +354,7 @@ func (h *ComposeHandler) HandleQuickPublish(c *gin.Context) { c.JSON(http.StatusCreated, gin.H{ "slug": slug, - "url": "/writing/" + slug, + "url": h.canonicalPathForSlug(slug), "type": postType, "draft": input.Draft, "message": message, @@ -367,7 +384,7 @@ func (h *ComposeHandler) PublishDraft(c *gin.Context) { if !input.Draft { c.JSON(http.StatusOK, gin.H{ "slug": slug, - "url": "/writing/" + slug, + "url": h.canonicalPathForSlug(slug), "message": "Already published", }) return @@ -392,7 +409,7 @@ func (h *ComposeHandler) PublishDraft(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "slug": slug, - "url": "/writing/" + slug, + "url": h.canonicalPathForSlug(slug), "message": "Published", }) } diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index fadb160..e17fac3 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -41,6 +41,7 @@ func (m *MockArticleService) GetArticleBySlug(slug string) (*models.Article, err } return nil, errors.New("article not found") } +func (m *MockArticleService) GetPages() []*models.Article { return nil } func (m *MockArticleService) GetArticlesByTag(tag string) []*models.Article { return nil } func (m *MockArticleService) GetArticlesByCategory(category string) []*models.Article { return nil } func (m *MockArticleService) GetArticlesForFeed(limit int) []*models.Article { return nil } diff --git a/internal/handlers/post_handler.go b/internal/handlers/post_handler.go index 0bb7c04..c424fb0 100644 --- a/internal/handlers/post_handler.go +++ b/internal/handlers/post_handler.go @@ -2,6 +2,7 @@ package handlers import ( "net/http" + "sort" "strconv" "strings" @@ -27,6 +28,28 @@ func NewPostHandler(base *BaseHandler, articleService services.ArticleServiceInt } } +// Pages renders the /p index, listing all published type:page articles +// alphabetically by title. The sort is a presentation concern — the +// repository returns natural insertion order; ordering for reader +// browsing lives here. +func (h *PostHandler) Pages(c *gin.Context) { + pages := h.articleService.GetPages() + + sort.SliceStable(pages, func(i, j int) bool { + return strings.ToLower(pages[i].Title) < strings.ToLower(pages[j].Title) + }) + + data := h.buildBaseTemplateData("Pages - " + h.config.Blog.Title) + data["description"] = "Evergreen content from " + h.config.Blog.Title + data["pages"] = pages + data["pageCount"] = len(pages) + data["template"] = "pages" + data["canonicalPath"] = "/p" + + h.enhanceTemplateDataWithSEO(data, c.Request.URL.Path) + h.renderHTML(c, http.StatusOK, "base.html", data) +} + // Articles handles the articles listing page. func (h *PostHandler) Articles(c *gin.Context) { pageStr := c.DefaultQuery("page", "1") @@ -129,7 +152,7 @@ func (h *PostHandler) getArticleData(slug string) (map[string]any, error) { data["article"] = art data["description"] = art.Description data["template"] = templateArticle - data["canonicalPath"] = "/writing/" + art.Slug + data["canonicalPath"] = article.CanonicalURLFor(art) data["breadcrumbs"] = []services.Breadcrumb{ {Name: "Home", URL: "/"}, {Name: "Writing", URL: "/writing"}, @@ -179,7 +202,7 @@ func (h *PostHandler) getArticlesPage(page int) (map[string]any, error) { "position": i + 1, "headline": a.DisplayTitle(), "description": a.Excerpt, - "url": baseURL + "/writing/" + a.Slug, + "url": baseURL + article.CanonicalURLFor(a), "datePublished": a.Date.Format("2006-01-02T15:04:05Z07:00"), "author": map[string]any{ "@type": "Person", diff --git a/internal/handlers/seo_helper.go b/internal/handlers/seo_helper.go index e04d7a2..1bffbe5 100644 --- a/internal/handlers/seo_helper.go +++ b/internal/handlers/seo_helper.go @@ -92,7 +92,7 @@ func (h *SEODataHelper) GenerateArticleSEOData(article *models.Article) map[stri breadcrumbs = []services.Breadcrumb{ {Name: "Home", URL: baseURL}, {Name: "Writing", URL: baseURL + "/writing"}, - {Name: article.Title, URL: baseURL + "/writing/" + article.Slug}, + {Name: article.Title, URL: baseURL + articlepkg.CanonicalURLFor(article)}, } } if breadcrumbSchema, err := h.seoService.GenerateBreadcrumbSchema(breadcrumbs); err == nil { diff --git a/internal/handlers/taxonomy_handler.go b/internal/handlers/taxonomy_handler.go index d89fdea..2ebd374 100644 --- a/internal/handlers/taxonomy_handler.go +++ b/internal/handlers/taxonomy_handler.go @@ -10,6 +10,7 @@ import ( apperrors "github.com/1mb-dev/markgo/internal/errors" "github.com/1mb-dev/markgo/internal/models" "github.com/1mb-dev/markgo/internal/services" + articlepkg "github.com/1mb-dev/markgo/internal/services/article" ) // TaxonomyHandler handles tag and category pages. @@ -129,7 +130,7 @@ func (h *TaxonomyHandler) getTagArticles(tag string) (map[string]any, error) { items[i] = map[string]any{ "@type": "ListItem", "position": i + 1, - "url": baseURL + "/writing/" + a.Slug, + "url": baseURL + articlepkg.CanonicalURLFor(a), } } data["collectionSchema"] = map[string]any{ @@ -177,7 +178,7 @@ func (h *TaxonomyHandler) getCategoryArticles(category string) (map[string]any, items[i] = map[string]any{ "@type": "ListItem", "position": i + 1, - "url": baseURL + "/writing/" + a.Slug, + "url": baseURL + articlepkg.CanonicalURLFor(a), } } data["collectionSchema"] = map[string]any{ diff --git a/internal/services/article/cache.go b/internal/services/article/cache.go index 381b732..d9818f6 100644 --- a/internal/services/article/cache.go +++ b/internal/services/article/cache.go @@ -451,6 +451,13 @@ func (cr *CachedRepository) GetPublished() []*models.Article { return cr.repository.GetPublished() } +// GetPages is a passthrough — not cached, matching GetPublished / GetByTag / +// GetByCategory / GetFeatured / GetRecent / GetDrafts. Only GetBySlug and +// GetStats actually cache in this layer. +func (cr *CachedRepository) GetPages() []*models.Article { + return cr.repository.GetPages() +} + // GetDrafts retrieves all draft articles. func (cr *CachedRepository) GetDrafts() []*models.Article { return cr.repository.GetDrafts() diff --git a/internal/services/article/factory.go b/internal/services/article/factory.go index 07d31d7..3aebf9a 100644 --- a/internal/services/article/factory.go +++ b/internal/services/article/factory.go @@ -69,6 +69,11 @@ func (a *ServiceAdapter) GetAllArticles() []*models.Article { return a.service.GetAllArticles() } +// GetPages retrieves all published type:page articles. +func (a *ServiceAdapter) GetPages() []*models.Article { + return a.service.GetPages() +} + // GetArticleBySlug retrieves an article by slug. func (a *ServiceAdapter) GetArticleBySlug(slug string) (*models.Article, error) { return a.service.GetArticleBySlug(slug) diff --git a/internal/services/article/repository.go b/internal/services/article/repository.go index 124202a..0ca9d94 100644 --- a/internal/services/article/repository.go +++ b/internal/services/article/repository.go @@ -42,6 +42,7 @@ type Repository interface { GetByTag(tag string) []*models.Article GetByCategory(category string) []*models.Article GetPublished() []*models.Article + GetPages() []*models.Article GetDrafts() []*models.Article GetFeatured(limit int) []*models.Article GetRecent(limit int) []*models.Article @@ -209,6 +210,27 @@ func (r *FileSystemRepository) GetPublished() []*models.Article { return result } +// GetPages returns published articles with type == TypePage in natural +// insertion order (date-desc, matching sibling list methods). This is the +// only list-shaped accessor that returns dedicated-route content; +// GetPublished / GetByTag / GetByCategory / GetFeatured / GetRecent all +// exclude it via DedicatedRouteArticle. Sort by Title is a presentation +// concern handled in the /p index handler. Powers the /p index and the +// sitemap pages section. +func (r *FileSystemRepository) GetPages() []*models.Article { + r.mutex.RLock() + defer r.mutex.RUnlock() + + var result []*models.Article + for _, article := range r.articles { + if !article.Draft && article.Type == TypePage { + result = append(result, article) + } + } + + return result +} + // GetDrafts returns all draft articles func (r *FileSystemRepository) GetDrafts() []*models.Article { r.mutex.RLock() diff --git a/internal/services/article/repository_test.go b/internal/services/article/repository_test.go index fca9dde..c32b1c4 100644 --- a/internal/services/article/repository_test.go +++ b/internal/services/article/repository_test.go @@ -408,6 +408,109 @@ func TestGetDrafts_IncludesAboutDraft(t *testing.T) { assert.Equal(t, "about", drafts[0].Slug) } +// TestGetPages verifies the only list-shaped accessor that returns +// dedicated-route content. Filters by Type==TypePage, excludes drafts, +// returns natural insertion order (no implicit sort — that's the /p +// handler's job). +func TestGetPages(t *testing.T) { + t.Run("returns published pages only", func(t *testing.T) { + repo := setupTestRepo(t, map[string]string{ + "page.md": pageArticle, + "draft-page.md": pageDraftArticle, + "test-article.md": validArticle, + "about.md": aboutArticle, + }) + _, err := repo.LoadAll(context.Background()) + require.NoError(t, err) + + pages := repo.GetPages() + require.Len(t, pages, 1) + assert.Equal(t, "run-your-own", pages[0].Slug) + assert.Equal(t, TypePage, pages[0].Type) + }) + + t.Run("empty store returns empty slice, not nil-typed", func(t *testing.T) { + repo := setupTestRepo(t, map[string]string{}) + _, err := repo.LoadAll(context.Background()) + require.NoError(t, err) + + pages := repo.GetPages() + assert.Empty(t, pages) + assert.Len(t, pages, 0) + }) + + t.Run("no pages in store returns empty", func(t *testing.T) { + repo := setupTestRepo(t, map[string]string{ + "test-article.md": validArticle, + "about.md": aboutArticle, + }) + _, err := repo.LoadAll(context.Background()) + require.NoError(t, err) + + assert.Empty(t, repo.GetPages()) + }) + + t.Run("excludes drafts", func(t *testing.T) { + repo := setupTestRepo(t, map[string]string{ + "draft-page.md": pageDraftArticle, + }) + _, err := repo.LoadAll(context.Background()) + require.NoError(t, err) + + assert.Empty(t, repo.GetPages()) + }) + + t.Run("type filter is exact", func(t *testing.T) { + // aboutArticle slug is "about" and is excluded from list methods via + // the predicate, but it is NOT type:page — verify it doesn't sneak + // into GetPages. + repo := setupTestRepo(t, map[string]string{ + "about.md": aboutArticle, + "test-article.md": validArticle, + }) + _, err := repo.LoadAll(context.Background()) + require.NoError(t, err) + + assert.Empty(t, repo.GetPages(), "/about is dedicated-route but not type:page; GetPages must not include it") + }) + + t.Run("preserves natural insertion order (date-desc)", func(t *testing.T) { + // Build two pages with explicit different dates; LoadAll sorts by + // date desc, so GetPages returns the newer one first. + newerPage := `--- +title: "Newer Page" +slug: "newer-page" +type: "page" +date: 2026-01-01T10:00:00Z +draft: false +--- + +Body. +` + olderPage := `--- +title: "Older Page" +slug: "older-page" +type: "page" +date: 2025-01-01T10:00:00Z +draft: false +--- + +Body. +` + repo := setupTestRepo(t, map[string]string{ + "newer.md": newerPage, + "older.md": olderPage, + }) + _, err := repo.LoadAll(context.Background()) + require.NoError(t, err) + + pages := repo.GetPages() + require.Len(t, pages, 2) + assert.Equal(t, "newer-page", pages[0].Slug, "GetPages must preserve LoadAll's date-desc order; sort is the handler's job") + assert.Equal(t, "older-page", pages[1].Slug) + }) +} + // TestListMethods_ExcludePages verifies the dedicated-route predicate also // fires on type:page articles across all five list methods. func TestListMethods_ExcludePages(t *testing.T) { diff --git a/internal/services/article/service.go b/internal/services/article/service.go index 98bbb37..3730394 100644 --- a/internal/services/article/service.go +++ b/internal/services/article/service.go @@ -19,6 +19,7 @@ import ( type Service interface { // Repository operations GetAllArticles() []*models.Article + GetPages() []*models.Article GetArticleBySlug(slug string) (*models.Article, error) GetArticlesByTag(tag string) []*models.Article GetArticlesByCategory(category string) []*models.Article @@ -145,6 +146,12 @@ func (s *CompositeService) GetAllArticles() []*models.Article { return s.repository.GetPublished() } +// GetPages returns published type:page articles. See Repository.GetPages for +// the contract — natural insertion order, type-filtered, draft-excluded. +func (s *CompositeService) GetPages() []*models.Article { + return s.repository.GetPages() +} + // GetArticleBySlug retrieves an article by slug func (s *CompositeService) GetArticleBySlug(slug string) (*models.Article, error) { article, err := s.repository.GetBySlug(slug) diff --git a/internal/services/article/service_test.go b/internal/services/article/service_test.go index 65d63e9..1d3c723 100644 --- a/internal/services/article/service_test.go +++ b/internal/services/article/service_test.go @@ -70,6 +70,16 @@ func (m *mockRepository) GetPublished() []*models.Article { return result } +func (m *mockRepository) GetPages() []*models.Article { + var result []*models.Article + for _, a := range m.articles { + if !a.Draft && a.Type == TypePage { + result = append(result, a) + } + } + return result +} + func (m *mockRepository) GetDrafts() []*models.Article { var result []*models.Article for _, a := range m.articles { diff --git a/internal/services/feed/feed.go b/internal/services/feed/feed.go index 143ee3a..34bb38a 100644 --- a/internal/services/feed/feed.go +++ b/internal/services/feed/feed.go @@ -9,6 +9,7 @@ import ( "github.com/1mb-dev/markgo/internal/config" "github.com/1mb-dev/markgo/internal/models" "github.com/1mb-dev/markgo/internal/services" + articlepkg "github.com/1mb-dev/markgo/internal/services/article" ) // Service generates RSS, JSON Feed, and Sitemap content. @@ -55,12 +56,13 @@ func (s *Service) GenerateRSS() (string, error) { items := make([]rssItem, 0, len(published)) for _, a := range published { + canonical := s.config.BaseURL + articlepkg.CanonicalURLFor(a) items = append(items, rssItem{ Title: a.DisplayTitle(), - Link: s.config.BaseURL + "/writing/" + a.Slug, + Link: canonical, Description: a.Description, PubDate: a.Date.Format(time.RFC1123Z), - GUID: s.config.BaseURL + "/writing/" + a.Slug, + GUID: canonical, }) } @@ -88,9 +90,10 @@ func (s *Service) GenerateJSONFeed() (string, error) { items := make([]map[string]any, 0, len(published)) for _, a := range published { + canonical := s.config.BaseURL + articlepkg.CanonicalURLFor(a) item := map[string]any{ - "id": s.config.BaseURL + "/writing/" + a.Slug, - "url": s.config.BaseURL + "/writing/" + a.Slug, + "id": canonical, + "url": canonical, "title": a.DisplayTitle(), "summary": a.Description, "date_published": a.Date.Format(time.RFC3339), @@ -129,21 +132,33 @@ func (s *Service) GenerateJSONFeed() (string, error) { return string(out), nil } -// GenerateSitemap produces an XML sitemap. +// GenerateSitemap produces an XML sitemap. Includes: +// - Site-wide static entries (home, /writing, /tags, /categories, /about, /p) +// - Published articles (from GetAllArticles, which is GetPublished — already +// excludes dedicated-route content via the predicate) +// - Pages (from GetPages, the only list-shaped accessor that returns +// dedicated-route type:page content) +// +// All URLs route through articlepkg.CanonicalURLFor — no hardcoded paths. +// /about and /p surface as static entries since neither maps to a single +// article: /about is its own handler, /p is the pages index. func (s *Service) GenerateSitemap() (string, error) { allArticles := s.articleService.GetAllArticles() + pages := s.articleService.GetPages() urls := []models.SitemapURL{ {Loc: s.config.BaseURL, LastMod: time.Now(), ChangeFreq: "weekly", Priority: 1.0}, {Loc: s.config.BaseURL + "/writing", LastMod: time.Now(), ChangeFreq: "daily", Priority: 0.8}, {Loc: s.config.BaseURL + "/tags", LastMod: time.Now(), ChangeFreq: "weekly", Priority: 0.6}, {Loc: s.config.BaseURL + "/categories", LastMod: time.Now(), ChangeFreq: "weekly", Priority: 0.6}, + {Loc: s.config.BaseURL + "/about", LastMod: time.Now(), ChangeFreq: "yearly", Priority: 0.5}, + {Loc: s.config.BaseURL + "/p", LastMod: time.Now(), ChangeFreq: "monthly", Priority: 0.5}, } for _, a := range allArticles { if !a.Draft { urls = append(urls, models.SitemapURL{ - Loc: s.config.BaseURL + "/writing/" + a.Slug, + Loc: s.config.BaseURL + articlepkg.CanonicalURLFor(a), LastMod: a.Date, ChangeFreq: "monthly", Priority: 0.7, @@ -151,6 +166,15 @@ func (s *Service) GenerateSitemap() (string, error) { } } + for _, p := range pages { + urls = append(urls, models.SitemapURL{ + Loc: s.config.BaseURL + articlepkg.CanonicalURLFor(p), + LastMod: p.Date, + ChangeFreq: "yearly", + Priority: 0.5, + }) + } + sitemap := models.Sitemap{ Xmlns: "http://www.sitemaps.org/schemas/sitemap/0.9", URLs: urls, diff --git a/internal/services/feed/feed_test.go b/internal/services/feed/feed_test.go index 9384edb..7939121 100644 --- a/internal/services/feed/feed_test.go +++ b/internal/services/feed/feed_test.go @@ -16,9 +16,11 @@ import ( type mockArticleService struct { articles []*models.Article + pages []*models.Article } func (m *mockArticleService) GetAllArticles() []*models.Article { return m.articles } +func (m *mockArticleService) GetPages() []*models.Article { return m.pages } func (m *mockArticleService) GetArticleBySlug(_ string) (*models.Article, error) { return nil, nil } @@ -186,6 +188,8 @@ func TestGenerateSitemap(t *testing.T) { assert.Contains(t, sitemap, "http://localhost:3000") assert.Contains(t, sitemap, "http://localhost:3000/writing/hello-world") assert.Contains(t, sitemap, "http://localhost:3000/writing/second-post") + assert.Contains(t, sitemap, "http://localhost:3000/about", "static /about entry") + assert.Contains(t, sitemap, "http://localhost:3000/p", "static /p index entry") assert.NotContains(t, sitemap, "draft") // Verify valid XML @@ -193,6 +197,55 @@ func TestGenerateSitemap(t *testing.T) { xmlContent := strings.TrimPrefix(sitemap, ``+"\n") err = xml.Unmarshal([]byte(xmlContent), &sm) require.NoError(t, err) - // 4 static URLs + 2 published articles = 6 - assert.Equal(t, 6, len(sm.URLs)) + // 6 static URLs (home, /writing, /tags, /categories, /about, /p) + 2 published articles = 8 + assert.Equal(t, 8, len(sm.URLs)) +} + +// TestGenerateSitemap_IncludesPages verifies that type:page articles surface +// in sitemap.xml at their canonical /p/ URL via GetPages(). Pages are +// excluded from the article section (via the predicate in GetPublished) so +// they must be emitted via the separate pages loop. +func TestGenerateSitemap_IncludesPages(t *testing.T) { + articles := []*models.Article{ + {Slug: "regular-post", Title: "Post", Date: time.Now()}, + } + pages := []*models.Article{ + {Slug: "run-your-own", Title: "Run Your Own", Type: "page", Date: time.Now()}, + {Slug: "colophon", Title: "Colophon", Type: "page", Date: time.Now()}, + } + svc := NewService(&mockArticleService{articles: articles, pages: pages}, testConfig()) + + sitemap, err := svc.GenerateSitemap() + require.NoError(t, err) + + assert.Contains(t, sitemap, "http://localhost:3000/writing/regular-post") + assert.Contains(t, sitemap, "http://localhost:3000/p/run-your-own", "page must surface at /p/") + assert.Contains(t, sitemap, "http://localhost:3000/p/colophon") + // No /writing/ leak. + assert.NotContains(t, sitemap, "/writing/run-your-own") + assert.NotContains(t, sitemap, "/writing/colophon") +} + +// TestGenerateSitemap_AllURLsAreCanonical verifies no entry contains +// a hardcoded /writing/ for content that should canonicalize to /p +// or /about. Guard against future regressions of the v3.14.0 sweep. +func TestGenerateSitemap_AllURLsAreCanonical(t *testing.T) { + // Fixture: article + page + about-slug article. The mock's GetAllArticles + // returns ALL of them (no predicate filtering at mock layer per CLAUDE.md); + // but in production GetAllArticles → repo.GetPublished excludes pages and + // /about. To simulate real behavior, separate them into articles vs pages. + articles := []*models.Article{ + {Slug: "real-post", Title: "Real Post", Date: time.Now()}, + } + pages := []*models.Article{ + {Slug: "about-pages", Title: "About Pages", Type: "page", Date: time.Now()}, + } + svc := NewService(&mockArticleService{articles: articles, pages: pages}, testConfig()) + + sitemap, err := svc.GenerateSitemap() + require.NoError(t, err) + + // /writing/ must NEVER appear in sitemap + assert.NotContains(t, sitemap, "/writing/about-pages", "type:page must canonicalize to /p, not /writing") + assert.Contains(t, sitemap, "/p/about-pages") } diff --git a/internal/services/interfaces.go b/internal/services/interfaces.go index 78160c1..2818b63 100644 --- a/internal/services/interfaces.go +++ b/internal/services/interfaces.go @@ -11,6 +11,7 @@ import ( type ArticleServiceInterface interface { // Article retrieval methods GetAllArticles() []*models.Article + GetPages() []*models.Article GetArticleBySlug(slug string) (*models.Article, error) GetArticlesByTag(tag string) []*models.Article GetArticlesByCategory(category string) []*models.Article @@ -83,10 +84,11 @@ type FeedServiceInterface interface { GenerateSitemap() (string, error) } -// SEOServiceInterface defines the interface for SEO utilities (stateless) +// SEOServiceInterface defines the interface for SEO utilities (stateless). +// Sitemap generation lives in feed.Service (served at /sitemap.xml); this +// interface covers per-request SEO surfaces (robots, Schema.org, OG/Twitter +// meta, content analysis). type SEOServiceInterface interface { - // Sitemap generation (on-demand, no caching) - GenerateSitemap() ([]byte, error) GenerateRobotsTxt() ([]byte, error) // Schema.org structured data diff --git a/internal/services/seo/seo.go b/internal/services/seo/seo.go index d656c2f..467b655 100644 --- a/internal/services/seo/seo.go +++ b/internal/services/seo/seo.go @@ -3,12 +3,10 @@ package seo import ( - "encoding/xml" "fmt" "log/slog" "net/url" "regexp" - "sort" "strconv" "strings" "time" @@ -18,6 +16,11 @@ import ( articlepkg "github.com/1mb-dev/markgo/internal/services/article" ) +// ogTypeWebsite is the Open Graph type for non-article (page) content. +// Articles emit og:type=article; pages and the homepage emit og:type=website +// per the OG protocol. +const ogTypeWebsite = "website" + // Helper represents a simple SEO utility type Helper struct { articleService services.ArticleServiceInterface @@ -27,21 +30,6 @@ type Helper struct { enabled bool } -// URLSet represents the root sitemap XML structure -type URLSet struct { - XMLName xml.Name `xml:"urlset"` - Xmlns string `xml:"xmlns,attr"` - URLs []URL `xml:"url"` -} - -// URL represents a single URL entry in sitemap -type URL struct { - Location string `xml:"loc"` - LastModified string `xml:"lastmod,omitempty"` - ChangeFreq string `xml:"changefreq,omitempty"` - Priority string `xml:"priority,omitempty"` -} - // NewHelper creates a new SEO helper instance func NewHelper( articleService services.ArticleServiceInterface, @@ -64,89 +52,6 @@ func (h *Helper) IsEnabled() bool { return h.enabled } -// GenerateSitemap creates an XML sitemap from all published articles -func (h *Helper) GenerateSitemap() ([]byte, error) { - if !h.enabled { - return nil, fmt.Errorf("SEO not enabled") - } - - h.logger.Debug("Generating sitemap") - - // Get all published articles - articles := h.articleService.GetAllArticles() - if articles == nil { - return nil, fmt.Errorf("failed to retrieve articles") - } - - // Filter out drafts and sort by date - publishedArticles := make([]*models.Article, 0, len(articles)) - for _, article := range articles { - if !article.Draft { - publishedArticles = append(publishedArticles, article) - } - } - - // Sort by date (newest first) - sort.Slice(publishedArticles, func(i, j int) bool { - return publishedArticles[i].Date.After(publishedArticles[j].Date) - }) - - // Create URL set - urlSet := URLSet{ - Xmlns: "http://www.sitemaps.org/schemas/sitemap/0.9", - URLs: make([]URL, 0, len(publishedArticles)+3), - } - - // Add homepage - urlSet.URLs = append(urlSet.URLs, URL{ - Location: h.siteConfig.BaseURL, - ChangeFreq: "daily", - Priority: "1.0", - }) - - // Add articles - for _, article := range publishedArticles { - articleURL, err := h.buildCanonicalURL(article) - if err != nil { - h.logger.Warn("Failed to build URL for article", "slug", article.Slug, "error", err) - continue - } - - priority := "0.8" - if article.Featured { - priority = "0.9" - } - - changeFreq := "monthly" - if article.Date.After(time.Now().AddDate(0, -1, 0)) { - changeFreq = "weekly" - } - - urlSet.URLs = append(urlSet.URLs, URL{ - Location: articleURL, - LastModified: article.Date.Format("2006-01-02"), - ChangeFreq: changeFreq, - Priority: priority, - }) - } - - // Generate XML - xmlData, err := xml.MarshalIndent(urlSet, "", " ") - if err != nil { - return nil, fmt.Errorf("failed to marshal sitemap XML: %w", err) - } - - // Add XML header - result := append([]byte(xml.Header), xmlData...) - - h.logger.Info("Sitemap generated", - "articles", len(publishedArticles), - "total_urls", len(urlSet.URLs), - "size_bytes", len(result)) - - return result, nil -} - // GenerateRobotsTxt creates a robots.txt file func (h *Helper) GenerateRobotsTxt() ([]byte, error) { if !h.enabled { @@ -202,7 +107,7 @@ func (h *Helper) GenerateOpenGraphTags(article *models.Article, baseURL string) // only dated feed content uses og:type=article. ogType := "article" if article.Type == articlepkg.TypePage { - ogType = "website" + ogType = ogTypeWebsite } tags["og:type"] = ogType tags["og:title"] = article.Title @@ -420,7 +325,7 @@ func (h *Helper) GeneratePageMetaTags(title, description, path string) (map[stri tags["robots"] = "index, follow" // Open Graph for pages - tags["og:type"] = "website" + tags["og:type"] = ogTypeWebsite if title != "" { tags["og:title"] = title } diff --git a/internal/services/seo/service_test.go b/internal/services/seo/service_test.go index 180246f..fa25bf2 100644 --- a/internal/services/seo/service_test.go +++ b/internal/services/seo/service_test.go @@ -30,6 +30,7 @@ func (m *MockArticleService) GetArticleBySlug(slug string) (*models.Article, err } // Implement other required methods as no-ops +func (m *MockArticleService) GetPages() []*models.Article { return nil } func (m *MockArticleService) GetArticlesByTag(_ string) []*models.Article { return nil } func (m *MockArticleService) GetArticlesByCategory(_ string) []*models.Article { return nil } func (m *MockArticleService) GetArticlesForFeed(_ int) []*models.Article { return nil } @@ -93,41 +94,6 @@ func createTestHelper() (*Helper, *MockArticleService) { return helper, mockArticles } -func TestSitemapGeneration(t *testing.T) { - helper, _ := createTestHelper() - - sitemap, err := helper.GenerateSitemap() - if err != nil { - t.Fatalf("Failed to generate sitemap: %v", err) - } - - sitemapStr := string(sitemap) - - // Basic XML validation - if !strings.Contains(sitemapStr, "") { - t.Error("Sitemap missing homepage") - } - - // Check published article is included - if !strings.Contains(sitemapStr, "/writing/test-article") { - t.Error("Sitemap missing published article") - } - - // Check draft article is NOT included - if strings.Contains(sitemapStr, "draft-article") { - t.Error("Sitemap should not include draft articles") - } -} - func TestRobotsGeneration(t *testing.T) { helper, _ := createTestHelper() @@ -382,8 +348,8 @@ func TestDisabledHelper(t *testing.T) { t.Error("Helper should be disabled") } - _, err := helper.GenerateSitemap() + _, err := helper.GenerateRobotsTxt() if err == nil { - t.Error("Disabled helper should return error for sitemap generation") + t.Error("Disabled helper should return error for robots.txt generation") } } diff --git a/internal/services/template_test.go b/internal/services/template_test.go index bfa3cf9..3ba9539 100644 --- a/internal/services/template_test.go +++ b/internal/services/template_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/1mb-dev/markgo/internal/config" + "github.com/1mb-dev/markgo/internal/models" ) func TestNewTemplateService(t *testing.T) { @@ -701,6 +702,92 @@ func TestTemplateService_ErrorHandling(t *testing.T) { assert.Equal(t, []string{}, templates) } +// TestRender_PagesTemplate guards against the strict-typed-template +// silent-truncation gotcha (see project CLAUDE.md): markgo's or/not/eq +// are narrower than Go template builtins, and a misuse fails at render +// time by truncating mid-response instead of returning an error. This +// renders the real embedded pages.html block (via NewTemplateService +// fallback to web.Assets when filesystem path is empty) with empty, +// single, and many-page inputs to verify each shape completes cleanly. +func TestRender_PagesTemplate(t *testing.T) { + tempDir := t.TempDir() // empty dir → service falls back to embedded templates + cfg := &config.Config{ + Blog: config.BlogConfig{Title: "Test Blog"}, + } + service, err := NewTemplateService(tempDir, cfg) + require.NoError(t, err) + require.True(t, service.HasTemplate("pages.html"), "embedded pages.html must be loaded") + + mkPage := func(slug, title, desc string) *models.Article { + return &models.Article{Slug: slug, Title: title, Description: desc} + } + + tests := []struct { + name string + pageCount int + pages []*models.Article + wants []string + notWants []string + }{ + { + name: "empty state", + pageCount: 0, + pages: nil, + wants: []string{"No pages yet", "type: page"}, + notWants: []string{"pages-list-item"}, + }, + { + name: "single page", + pageCount: 1, + pages: []*models.Article{mkPage("about-us", "About Us", "Who we are")}, + wants: []string{"About Us", "Who we are", `href="/p/about-us"`, "pages-list-item"}, + notWants: []string{"No pages yet"}, + }, + { + name: "many pages without descriptions", + pageCount: 3, + pages: []*models.Article{ + mkPage("a", "Alpha", ""), + mkPage("b", "Beta", ""), + mkPage("c", "Gamma", ""), + }, + wants: []string{"Alpha", "Beta", "Gamma", `href="/p/a"`, `href="/p/c"`}, + notWants: []string{"pages-list-excerpt", "No pages yet"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf strings.Builder + data := map[string]any{ + "config": cfg, + "pageCount": tt.pageCount, + "pages": tt.pages, + } + err := service.GetTemplate().ExecuteTemplate(&buf, "pages-content", data) + require.NoError(t, err, "pages-content template must render without error — silent truncation would NOT raise here, so check output integrity too") + + out := buf.String() + assert.True(t, strings.HasSuffix(strings.TrimSpace(out), ""), + "render output must end with closing div, not truncate mid-template (got tail: %q)", tail(out, 80)) + + for _, want := range tt.wants { + assert.Contains(t, out, want) + } + for _, notWant := range tt.notWants { + assert.NotContains(t, out, notWant) + } + }) + } +} + +func tail(s string, n int) string { + if len(s) <= n { + return s + } + return s[len(s)-n:] +} + func createTestTemplateService(t *testing.T) *TemplateService { tempDir := t.TempDir() cfg := &config.Config{} diff --git a/web/static/css/about.css b/web/static/css/about.css index 5b52b89..9b98230 100644 --- a/web/static/css/about.css +++ b/web/static/css/about.css @@ -164,32 +164,62 @@ } /* ========================================================================== - AMA Section + Reach Section (v3.14.0+) — AMA + mailto consolidated in one card. + Replaces the prior split .about-ama / .about-email sections so /about + shows one labeled reach surface with multiple affordances. ========================================================================== */ -.about-ama { +.about-reach { max-width: var(--max-content-width); margin: 0 auto var(--spacing-5); padding: var(--spacing-5); border: 1px solid var(--color-border); border-radius: var(--radius-lg); - text-align: center; } -.about-ama-heading { +.about-reach-heading { font-size: var(--font-size-xl); font-weight: 600; + margin: 0 0 var(--spacing-4); + color: var(--color-text-primary); + text-align: center; +} + +.about-reach-affordances { + display: flex; + flex-direction: column; + gap: var(--spacing-4); +} + +@media (min-width: 769px) { + .about-reach-affordances { + flex-direction: row; + gap: var(--spacing-5); + } + + .about-reach-card { + flex: 1; + } +} + +.about-reach-card { + text-align: center; +} + +.about-reach-card-heading { + font-size: var(--font-size-lg); + font-weight: 600; margin: 0 0 var(--spacing-2); color: var(--color-text-primary); } -.about-ama-text { +.about-reach-card-text { font-size: var(--font-size-base); color: var(--color-text-secondary); - margin: 0 0 var(--spacing-4); + margin: 0 0 var(--spacing-3); } -.about-ama-btn { +.about-reach-btn { display: inline-block; padding: var(--spacing-3) var(--spacing-6); font-family: var(--font-family-sans); @@ -203,12 +233,30 @@ transition: background-color var(--transition-fast); } -.about-ama-btn:hover { +.about-reach-btn:hover { background-color: var(--color-primary-dark); } +.about-reach-link { + display: inline-block; + padding: var(--spacing-3) var(--spacing-6); + color: var(--color-primary); + font-weight: 500; + font-size: var(--font-size-base); + text-decoration: none; + border: 1px solid var(--color-primary); + border-radius: var(--radius-md); + transition: background-color var(--transition-fast), color var(--transition-fast); +} + +.about-reach-link:hover { + background-color: var(--color-primary); + color: var(--color-text-inverse); +} + /* ========================================================================== - Contact Section — card container + Contact Section — card container for full SMTP-backed contact form. + Mailto-only fallback consolidated into .about-reach (v3.14.0+). ========================================================================== */ .about-contact { @@ -232,29 +280,6 @@ flex-direction: column; } -/* Email link (when SMTP not configured) */ -.about-email { - text-align: center; - padding: var(--spacing-2) 0; -} - -.about-email-link { - display: inline-block; - padding: var(--spacing-3) var(--spacing-6); - color: var(--color-primary); - font-weight: 500; - font-size: var(--font-size-base); - text-decoration: none; - border: 1px solid var(--color-primary); - border-radius: var(--radius-md); - transition: background-color var(--transition-fast), color var(--transition-fast); -} - -.about-email-link:hover { - background-color: var(--color-primary); - color: var(--color-text-inverse); -} - /* ========================================================================== Contact Form Styles Form controls (.form-group, .form-input, etc.) live in components.css. diff --git a/web/static/css/pages.css b/web/static/css/pages.css new file mode 100644 index 0000000..f2282fc --- /dev/null +++ b/web/static/css/pages.css @@ -0,0 +1,47 @@ +.pages-index { + padding: var(--spacing-5) 0; +} + +.pages-list { + list-style: none; + padding: 0; + margin: 0; + max-width: 42rem; + margin-left: auto; + margin-right: auto; +} + +.pages-list-item { + border-bottom: 1px solid var(--color-border); +} + +.pages-list-item:last-child { + border-bottom: none; +} + +.pages-list-link { + display: block; + padding: var(--spacing-5) 0; + text-decoration: none; + color: inherit; + transition: color var(--transition-fast); +} + +.pages-list-title { + display: block; + font-size: var(--font-size-lg); + font-weight: 600; + margin-bottom: var(--spacing-1); +} + +.pages-list-excerpt { + display: block; + color: var(--color-text-muted); + font-size: var(--font-size-base); + line-height: 1.5; +} + +.pages-list-link:hover .pages-list-title, +.pages-list-link:focus .pages-list-title { + color: var(--color-primary); +} diff --git a/web/templates/about.html b/web/templates/about.html index f1f2a37..2bcf6e0 100644 --- a/web/templates/about.html +++ b/web/templates/about.html @@ -64,17 +64,30 @@

{{ .config.Blog.Author }}

{{ end }} - -
-

Ask Me Anything

-

Curious about something? Submit a question and I'll answer it publicly.

- + +
+

Reach out

+
+
+

{{ .about_ama_heading }}

+

{{ .about_ama_intro }}

+ +
+ {{ if and .has_contact (not .has_contact_form) }} +
+

Email

+

Or drop a line directly.

+ {{ .config.Blog.AuthorEmail }} +
+ {{ end }} +
- + {{ if .has_contact }} -
- {{ if .has_contact_form }} + {{ if .has_contact_form }} +

Get in touch

@@ -134,15 +147,9 @@

Get in touch

Your information will never be shared.

- {{ else }} -
- - Say hello → {{ .config.Blog.AuthorEmail }} - -
- {{ end }}
{{ end }} + {{ end }} diff --git a/web/templates/base.html b/web/templates/base.html index 1b6f75c..10f5119 100644 --- a/web/templates/base.html +++ b/web/templates/base.html @@ -159,6 +159,7 @@ + @@ -181,6 +182,7 @@ {{ if eq $tpl "feed" }}{{ template "feed-head" . }}{{ end }} {{ if eq $tpl "compose" }}{{ template "compose-head" . }}{{ end }} {{ if eq $tpl "articles" }}{{ template "articles-head" . }}{{ end }} + {{ if eq $tpl "pages" }}{{ template "pages-head" . }}{{ end }} {{ if eq $tpl "article" }}{{ template "article-head" . }}{{ end }} {{ if eq $tpl "search" }}{{ template "search-head" . }}{{ end }} {{ if eq $tpl "about" }}{{ template "about-head" . }}{{ end }} @@ -198,6 +200,7 @@ Sign in to continue {{ if eq $tpl "feed" }}{{ template "feed-content" . }}{{ end }} {{ if eq $tpl "compose" }}{{ template "compose-content" . }}{{ end }} {{ if eq $tpl "articles" }}{{ template "articles-content" . }}{{ end }} + {{ if eq $tpl "pages" }}{{ template "pages-content" . }}{{ end }} {{ if eq $tpl "article" }}{{ template "article-content" . }}{{ end }} {{ if eq $tpl "search" }}{{ template "search-content" . }}{{ end }} {{ if eq $tpl "about" }}{{ template "about-content" . }}{{ end }} @@ -448,6 +452,7 @@

Sign in to continue

Subscribe Tags Categories + Pages About Writing diff --git a/web/templates/pages.html b/web/templates/pages.html new file mode 100644 index 0000000..97faacb --- /dev/null +++ b/web/templates/pages.html @@ -0,0 +1,36 @@ +{{ define "pages-content" }} +
+
+
+

Pages

+

Evergreen content from {{ .config.Blog.Title }}.

+
+ + {{ if eq .pageCount 0 }} +
+

No pages yet

+

Evergreen pages (set type: page in frontmatter) will appear here.

+
+ {{ else }} + + {{ end }} +
+
+{{ end }} + +{{ define "pages-head" }} +{{ if not .config.SEO.Enabled }} + +{{ end }} +{{ end }}