feat: editor write path
This commit is contained in:
Florian Sylvain committed
1 parent
d9158025d4
commit
a0b3c58c52
88 files changed
+5738
-263
No files matched your search
@@ -0,0 +1,427 @@
|
||||
package test
|
||||
|
||||
import (
|
||||
"RenewCMS/api/pkg/apperr"
|
||||
"RenewCMS/internal/domain/article"
|
||||
"RenewCMS/internal/domain/audit"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func revisionsOf(w *storyWorld, id uint32) (uint32, uint32) {
|
||||
stored := w.stored(id)
|
||||
return stored.EditRevision, stored.ContentRevision
|
||||
}
|
||||
|
||||
func TestArticleEditRevisionHTTP(t *testing.T) {
|
||||
t.Run("Title boundaries on PATCH, counted in runes", func(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
title string
|
||||
status int
|
||||
message string
|
||||
stored bool
|
||||
}{
|
||||
{"2 ASCII characters", `"ab"`, 400, apperr.ArticleTitleErrorMessage, false},
|
||||
{"3 ASCII characters", `"abc"`, 200, "", true},
|
||||
{"200 ASCII characters", `"` + strings.Repeat("a", 200) + `"`, 200, "", true},
|
||||
{"201 ASCII characters", `"` + strings.Repeat("a", 201) + `"`, 400, apperr.ArticleTitleErrorMessage, false},
|
||||
{"200 four-byte runes", `"` + strings.Repeat("😀", 200) + `"`, 200, "", true},
|
||||
{"empty", `""`, 400, apperr.ArticleTitleErrorMessage, false},
|
||||
{"null", `null`, 400, apperr.BodyErrorMessage, false},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":`+c.title+`}`)
|
||||
|
||||
if response.Code != c.status || (c.message != "" && response.text() != c.message+"\n") {
|
||||
t.Fatalf("expected %d %q, got %d %q", c.status, c.message, response.Code, response.text())
|
||||
}
|
||||
if changed := w.stored(10).Title != "Renovation costs"; changed != c.stored {
|
||||
t.Errorf("stored title changed = %v, expected %v", changed, c.stored)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("A domain title rejection on update is a 400, not a 500", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
w.articles.fail.failNextWith("Update", article.ErrInvalidTitle)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"Valid title"}`).expect(t, 400, apperr.ArticleTitleErrorMessage)
|
||||
})
|
||||
|
||||
t.Run("Editing the title never changes the slug", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"New title"}`)
|
||||
|
||||
response.expect(t, 200, "")
|
||||
if stored := w.stored(10); stored.Slug != "renovation-costs" || !stored.IsOnline {
|
||||
t.Errorf("slug and publication must be unchanged: %+v", stored)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Title and body in one request", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"New title","body":"<p>x</p>"}`)
|
||||
|
||||
response.expect(t, 200, "")
|
||||
if stored := w.stored(10); stored.Title != "New title" || stored.Body != "<p>x</p>" || stored.EditRevision != 4 {
|
||||
t.Errorf("both must be stored with revision 4: %+v", stored)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("The title is editable on a single-language site", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
w.sites.seed(3, "Archive", "archive", false)
|
||||
w.rows.rows[3] = storyRows("en", []string{"en"}, nil)
|
||||
w.put(article.Article{ID: 60, Title: "Old archive", SiteID: 3, CreatedBy: storyWriter1, Slug: "old-archive", Lang: "en", StoryID: 60})
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/60", `{"title":"New title"}`)
|
||||
|
||||
response.expect(t, 200, "")
|
||||
if w.stored(60).Slug != "old-archive" {
|
||||
t.Errorf("the slug must be unchanged")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Explicit slug edits", func(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
id string
|
||||
body string
|
||||
status int
|
||||
message string
|
||||
slug string
|
||||
header string
|
||||
}{
|
||||
{"online article", "10", `{"slug":"new-address"}`, 200, "", "new-address", ""},
|
||||
{"offline article", "12", `{"slug":"new-address"}`, 200, "", "new-address", ""},
|
||||
{"same as current", "10", `{"slug":"renovation-costs"}`, 200, "", "renovation-costs", ""},
|
||||
{"not a slug", "10", `{"slug":"Not A Slug"}`, 400, apperr.InvalidSlugErrorMessage, "renovation-costs", ""},
|
||||
{"used by another language of the same story", "10", `{"slug":"couts"}`, 200, "", "couts", ""},
|
||||
{"used by another en article", "10", `{"slug":"taken-en"}`, 409, apperr.ArticleSlugTakenErrorMessage, "renovation-costs", "taken-en-2"},
|
||||
{"empty regenerates from the title", "10", `{"slug":""}`, 200, "", "renovation-costs", ""},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
w.put(article.Article{ID: 70, Title: "Taken", SiteID: storyJournal, CreatedBy: 1, Slug: "taken-en", Lang: "en", StoryID: 70})
|
||||
id, _ := strconv.ParseUint(c.id, 10, 32)
|
||||
editBefore, _ := revisionsOf(w, uint32(id))
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/"+c.id, c.body)
|
||||
|
||||
response.expect(t, c.status, c.message)
|
||||
if got := w.stored(uint32(id)).Slug; got != c.slug {
|
||||
t.Errorf("slug %q, expected %q", got, c.slug)
|
||||
}
|
||||
if response.header(apperr.SlugSuggestionHeader) != c.header {
|
||||
t.Errorf("X-Slug-Suggestion %q, expected %q", response.header(apperr.SlugSuggestionHeader), c.header)
|
||||
}
|
||||
if editAfter, _ := revisionsOf(w, uint32(id)); (c.name == "same as current" || c.name == "empty regenerates from the title") && editAfter != editBefore {
|
||||
t.Errorf("a no-op slug must not change the revision")
|
||||
}
|
||||
if w.stored(uint32(id)).IsOnline != (c.id == "10") {
|
||||
t.Errorf("publication must be unchanged")
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("The same slug in two languages is not a conflict", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"slug":"couts"}`).expect(t, 200, "")
|
||||
|
||||
if w.stored(10).Slug != "couts" || w.stored(12).Slug != "couts" {
|
||||
t.Errorf("both articles must hold the slug couts")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Revision is exposed on every article read and write", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
requests := []struct {
|
||||
method string
|
||||
path string
|
||||
body string
|
||||
status int
|
||||
}{
|
||||
{http.MethodGet, "/v1/article/10", "", 200},
|
||||
{http.MethodGet, "/v1/article/", "", 200},
|
||||
{http.MethodPost, "/v1/article/", `{"title":"Plain draft"}`, 201},
|
||||
{http.MethodPatch, "/v1/article/10", `{"body":"x"}`, 200},
|
||||
{http.MethodPatch, "/v1/article/12/publish", "", 200},
|
||||
{http.MethodPatch, "/v1/article/12/unpublish", "", 200},
|
||||
}
|
||||
for _, request := range requests {
|
||||
response := w.send(storyOwner, request.method, request.path, request.body)
|
||||
if response.Code != request.status {
|
||||
t.Fatalf("%s %s: status %d %s", request.method, request.path, response.Code, response.text())
|
||||
}
|
||||
if strings.HasSuffix(request.path, "/") && request.method == http.MethodGet {
|
||||
for _, item := range decodeArticleItems(t, response.Body.Bytes()) {
|
||||
assertPositiveRevision(t, item)
|
||||
}
|
||||
continue
|
||||
}
|
||||
fields := response.fields(t)
|
||||
assertPositiveRevision(t, fields)
|
||||
for _, key := range legacyArticleKeys {
|
||||
if _, present := fields[key]; !present {
|
||||
t.Errorf("%s %s: legacy key %q missing", request.method, request.path, key)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
fieldCases := []struct {
|
||||
field string
|
||||
body string
|
||||
content bool
|
||||
setup func(*storyWorld)
|
||||
}{
|
||||
{"title", `{"title":"Changed title"}`, true, nil},
|
||||
{"body", `{"body":"<p>changed</p>"}`, true, nil},
|
||||
{"excerpt", `{"excerpt":"changed"}`, true, nil},
|
||||
{"slug", `{"slug":"new-address"}`, true, nil},
|
||||
{"cover_image_alt", `{"cover_image_alt":"changed"}`, true, nil},
|
||||
{"seo_title", `{"seo_title":"changed"}`, true, nil},
|
||||
{"seo_description", `{"seo_description":"changed"}`, true, nil},
|
||||
{"social_title", `{"social_title":"changed"}`, true, nil},
|
||||
{"social_description", `{"social_description":"changed"}`, true, nil},
|
||||
{"author_ids", `{"author_ids":[2]}`, false, nil},
|
||||
{"cover_image_id", `{"cover_image_id":7}`, false, nil},
|
||||
{"social_image_id", `{"social_image_id":8}`, false, nil},
|
||||
{"canonical_url", `{"canonical_url":"https://journal.example/other"}`, false, nil},
|
||||
{"noindex", `{"noindex":false}`, false, nil},
|
||||
{"seo_dismissed", `{"seo_dismissed":["cover_alt"]}`, false, nil},
|
||||
}
|
||||
t.Run("What raises revision and content_revision", func(t *testing.T) {
|
||||
for _, c := range fieldCases {
|
||||
t.Run(c.field, func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", c.body).expect(t, 200, "")
|
||||
|
||||
edit, content := revisionsOf(w, 10)
|
||||
wantContent := uint32(4)
|
||||
if c.content {
|
||||
wantContent = 5
|
||||
}
|
||||
if edit != 4 || content != wantContent {
|
||||
t.Errorf("revision %d and content_revision %d, expected 4 and %d", edit, content, wantContent)
|
||||
}
|
||||
})
|
||||
}
|
||||
t.Run("site_id of a single-version story", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
w.put(article.Article{ID: 60, Title: "Alone", CreatedBy: storyWriter1, Slug: "alone", Lang: "en", StoryID: 60, EditRevision: 3, ContentRevision: 4})
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/60", `{"site_id":2}`).expect(t, 200, "")
|
||||
|
||||
if edit, content := revisionsOf(w, 60); edit != 4 || content != 4 {
|
||||
t.Errorf("revision %d and content_revision %d, expected 4 and 4", edit, content)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("Several content fields in one PATCH raise each counter once", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"Changed title","body":"<p>b</p>","excerpt":"e"}`).expect(t, 200, "")
|
||||
|
||||
if edit, content := revisionsOf(w, 10); edit != 4 || content != 5 {
|
||||
t.Errorf("revision %d and content_revision %d, expected 4 and 5", edit, content)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("A PATCH that changes nothing leaves both counters alone", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"Renovation costs","body":"<p>Costs</p>","noindex":true}`).expect(t, 200, "")
|
||||
|
||||
if edit, content := revisionsOf(w, 10); edit != 3 || content != 4 {
|
||||
t.Errorf("revision %d and content_revision %d, expected 3 and 4", edit, content)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Publishing and unpublishing do not touch revisions", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
editBefore, contentBefore := revisionsOf(w, 12)
|
||||
|
||||
w.send(storyOwner, http.MethodPatch, "/v1/article/12/publish", "").expect(t, 200, "")
|
||||
w.send(storyOwner, http.MethodPatch, "/v1/article/12/unpublish", "").expect(t, 200, "")
|
||||
|
||||
if edit, content := revisionsOf(w, 12); edit != editBefore || content != contentBefore {
|
||||
t.Errorf("revisions changed to %d and %d", edit, content)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("if_revision matches", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"x","if_revision":3}`)
|
||||
|
||||
response.expect(t, 200, "")
|
||||
if string(response.fields(t)["revision"]) != "4" {
|
||||
t.Errorf("the response revision must be 4: %s", response.text())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("if_revision is stale", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
stored := w.stored(10)
|
||||
stored.EditRevision = 4
|
||||
w.articles.articles[10] = stored
|
||||
before := w.snapshot()
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"x","if_revision":3}`)
|
||||
|
||||
response.expect(t, 409, apperr.RevisionConflictErrorMessage)
|
||||
if response.header(apperr.CurrentRevisionHeader) != "4" {
|
||||
t.Errorf("X-Current-Revision %q", response.header(apperr.CurrentRevisionHeader))
|
||||
}
|
||||
w.expectUnchanged(before)
|
||||
for _, event := range w.eventsOf(audit.ActionArticleEdit) {
|
||||
if event.Outcome == audit.OutcomeSuccess {
|
||||
t.Errorf("no successful edit event expected: %+v", event)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("A stale if_revision wins over a no-op PATCH", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
stored := w.stored(10)
|
||||
stored.EditRevision = 4
|
||||
w.articles.articles[10] = stored
|
||||
|
||||
response := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"Renovation costs","if_revision":3}`)
|
||||
|
||||
response.expect(t, 409, apperr.RevisionConflictErrorMessage)
|
||||
})
|
||||
|
||||
t.Run("Overwrite uses the revision the conflict dialog showed", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
stored := w.stored(10)
|
||||
stored.EditRevision = 4
|
||||
w.articles.articles[10] = stored
|
||||
|
||||
overwrite := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"mine","if_revision":4}`)
|
||||
|
||||
overwrite.expect(t, 200, "")
|
||||
if string(overwrite.fields(t)["revision"]) != "5" {
|
||||
t.Errorf("revision must be 5: %s", overwrite.text())
|
||||
}
|
||||
w.articles.beforeUpdate = func() {
|
||||
saved := w.stored(10)
|
||||
saved.EditRevision = 6
|
||||
saved.Body = "theirs"
|
||||
w.articles.articles[10] = saved
|
||||
}
|
||||
|
||||
raced := w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"again","if_revision":5}`)
|
||||
|
||||
raced.expect(t, 409, apperr.RevisionConflictErrorMessage)
|
||||
if raced.header(apperr.CurrentRevisionHeader) != "6" || w.stored(10).Body != "theirs" {
|
||||
t.Errorf("the then-current revision 6 and their body expected: %q %q", raced.header(apperr.CurrentRevisionHeader), w.stored(10).Body)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Without if_revision the last write wins", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"x"}`).expect(t, 200, "")
|
||||
|
||||
if edit, _ := revisionsOf(w, 10); edit != 4 {
|
||||
t.Errorf("revision still rises by 1, got %d", edit)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Invalid if_revision", func(t *testing.T) {
|
||||
for _, value := range []string{`0`, `-1`, `"3"`, `1.5`, `null`} {
|
||||
t.Run(value, func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"x","if_revision":`+value+`}`).expect(t, 400, apperr.BodyErrorMessage)
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("if_revision alone is not an update", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"if_revision":3}`).expect(t, 400, apperr.BodyErrorMessage)
|
||||
})
|
||||
|
||||
t.Run("Other languages are untouched by an overwrite", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
before := w.stored(12)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"x","if_revision":3}`).expect(t, 200, "")
|
||||
|
||||
if !articlesEqual(before, w.stored(12)) {
|
||||
t.Errorf("article 12 changed")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Permissions are unchanged", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
w.send(storyReader, http.MethodPatch, "/v1/article/10", `{"body":"x","if_revision":1}`).expect(t, 403, apperr.MissingPermissionErrorMessage)
|
||||
w.send(storyAnon, http.MethodPatch, "/v1/article/10", `{"body":"x","if_revision":1}`).expect(t, 401, "")
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/9999", `{"body":"x"}`).expect(t, 404, apperr.ArticleNotFoundErrorMessage)
|
||||
before := w.snapshot()
|
||||
hidden := w.send(storyPartial, http.MethodPatch, "/v1/article/11", `{"body":"x","if_revision":99}`)
|
||||
hidden.expect(t, 404, apperr.ArticleNotFoundErrorMessage)
|
||||
if hidden.header("X-Current-Revision") != "" {
|
||||
t.Errorf("a caller who cannot read the article must not learn its revision")
|
||||
}
|
||||
w.send(storyPartial, http.MethodPatch, "/v1/article/11", `{"body":"x"}`).expect(t, 404, apperr.ArticleNotFoundErrorMessage)
|
||||
w.expectUnchanged(before)
|
||||
})
|
||||
|
||||
t.Run("Editing an online article without content.publish changes the live page right away", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"New title"}`).expect(t, 200, "")
|
||||
|
||||
public := w.send(storyAnon, http.MethodGet, "/v1/article/10", "")
|
||||
public.expect(t, 200, "")
|
||||
if string(public.fields(t)["title"]) != `"New title"` {
|
||||
t.Errorf("the public read must show the new title: %s", public.text())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Title and slug edits are recorded", func(t *testing.T) {
|
||||
w := newStoryWorld(t)
|
||||
|
||||
w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"title":"New title","slug":"new-address"}`).expect(t, 200, "")
|
||||
|
||||
events := w.eventsOf(audit.ActionArticleEdit)
|
||||
if len(events) != 1 {
|
||||
t.Fatalf("one edit event expected, got %d", len(events))
|
||||
}
|
||||
fields := []string{}
|
||||
for _, change := range events[0].Changes {
|
||||
fields = append(fields, change.Field)
|
||||
}
|
||||
if !strings.Contains(strings.Join(fields, ","), audit.FieldTitle) || !strings.Contains(strings.Join(fields, ","), audit.FieldSlug) || events[0].Params[audit.ParamLang] != "en" {
|
||||
t.Errorf("title, slug and lang expected: %v %v", fields, events[0].Params)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func assertPositiveRevision(t *testing.T, fields map[string]json.RawMessage) {
|
||||
t.Helper()
|
||||
value, err := strconv.Atoi(string(fields["revision"]))
|
||||
if err != nil || value < 1 {
|
||||
t.Errorf("revision must be a positive integer: %s", fields["revision"])
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user