diff --git a/CHANGELOG.md b/CHANGELOG.md index 5027978..903a9b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.18.0] - 2026-05-19 -Compose readability pass. No behavior change. +Compose readability pass + "Save Draft" regression fix. + +### Fixed + +- **Save Draft button published instead of drafting (#98).** `compose.js` + disabled the submitter button before constructing `FormData(form, + submitter)`. Per the HTML spec, disabled fields are skipped during + entry-list construction — including the submitter — so `draft=on` + never reached the wire. Reorder: build `body` before disabling + buttons. Pre-existing in v3.17.0; surfaced during the readability pass. +- **Draft post-save redirected to a 404.** Server redirected to + `canonicalPathForSlug(slug)` regardless of draft state; public routes + exclude drafts, so the operator landed on `/writing/` 404 after + a successful Save Draft. Now redirects to `/admin/drafts` when + `input.Draft` is true. Same fix applied to `HandleEdit` for the + edit-to-draft path. ### Changed diff --git a/internal/handlers/compose.go b/internal/handlers/compose.go index e25ae45..a6e13c6 100644 --- a/internal/handlers/compose.go +++ b/internal/handlers/compose.go @@ -251,6 +251,12 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) { reloadOK = false } + // Drafts have no public URL — canonicalPathForSlug would 404. Send to the + // drafts list where the operator can find and publish. + if input.Draft { + c.Redirect(http.StatusSeeOther, "/admin/drafts") + return + } // Redirect to the edited article, or feed if reload failed (stale cache would show old version) if reloadOK { c.Redirect(http.StatusSeeOther, h.canonicalPathForSlug(slug)) @@ -335,6 +341,12 @@ func (h *ComposeHandler) HandleSubmit(c *gin.Context) { reloadOK = false } + // Drafts have no public URL — canonicalPathForSlug would 404. Send to the + // drafts list where the operator can find and publish. + if input.Draft { + c.Redirect(http.StatusSeeOther, "/admin/drafts") + return + } // Redirect to the new post, or feed if reload failed (article won't be in memory) if !reloadOK || input.Title == "" { c.Redirect(http.StatusSeeOther, "/") diff --git a/internal/handlers/compose_test.go b/internal/handlers/compose_test.go index 192ed82..e345a34 100644 --- a/internal/handlers/compose_test.go +++ b/internal/handlers/compose_test.go @@ -1028,7 +1028,7 @@ func TestHandleSubmit(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) }) - t.Run("draft mode redirects to feed", func(t *testing.T) { + t.Run("draft mode redirects to /admin/drafts", func(t *testing.T) { handler, _ := createFormComposeHandler(t) router := gin.New() @@ -1050,13 +1050,10 @@ func TestHandleSubmit(t *testing.T) { router.ServeHTTP(w, req) - // Draft posts without title redirect to "/" but with title they also redirect to "/" - // because the condition is `!reloadOK || input.Title == ""` — with a title and - // successful reload, it goes to /writing/slug. But it's a draft so let's test: - // Actually, the redirect target depends on reloadOK and title presence. - // With MockArticleService.ReloadArticles returning nil and title set, it goes to /writing/slug. + // Drafts have no public URL — canonicalPathForSlug would 404. Operator + // must land on /admin/drafts to find and publish. assert.Equal(t, http.StatusSeeOther, w.Code) - assert.Equal(t, "/writing/draft-post", w.Header().Get("Location")) + assert.Equal(t, "/admin/drafts", w.Header().Get("Location")) }) t.Run("content without title redirects to feed", func(t *testing.T) { @@ -1207,6 +1204,33 @@ func TestHandleEdit(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) }) + t.Run("edit toggles to draft → redirects to /admin/drafts", func(t *testing.T) { + handler, tmpDir := createFormComposeHandler(t) + writeDraftArticle(t, tmpDir, "edit-to-draft", false) + + router := gin.New() + router.Use(func(c *gin.Context) { + c.Set("csrf_secure", false) + c.Next() + }) + router.POST("/compose/edit/:slug", handler.HandleEdit) + + form := url.Values{ + "content": {"Demoted to draft."}, + "title": {"Demoted"}, + "draft": {"on"}, + "_csrf": {"test-token"}, + } + req := httptest.NewRequest("POST", "/compose/edit/edit-to-draft", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusSeeOther, w.Code) + assert.Equal(t, "/admin/drafts", w.Header().Get("Location")) + }) + t.Run("edit reload failure redirects to feed", func(t *testing.T) { tmpDir := t.TempDir() cfg := &config.Config{ diff --git a/web/static/js/compose.js b/web/static/js/compose.js index 9028ebb..3102d0b 100644 --- a/web/static/js/compose.js +++ b/web/static/js/compose.js @@ -326,7 +326,13 @@ export function init() { const submitter = e.submitter; const isDraft = submitter?.value === 'on'; - // Disable both buttons during submit + // Build the body BEFORE disabling buttons. Per the HTML spec, + // entry-list construction skips disabled form fields \u2014 including + // the submitter when passed to FormData(form, submitter). Disabling + // first would drop the "draft=on" entry from a Save Draft click and + // silently publish. + const body = new FormData(form, submitter); + if (saveDraftBtn) saveDraftBtn.disabled = true; if (publishBtn) publishBtn.disabled = true; @@ -337,7 +343,7 @@ export function init() { try { const response = await authenticatedFetch(form.action, { method: 'POST', - body: new FormData(form, submitter), + body, headers: { Accept: 'text/html' }, });