diff --git a/controllers/api/handlers_test.go b/controllers/api/handlers_test.go index 25eec08..bb8603c 100644 --- a/controllers/api/handlers_test.go +++ b/controllers/api/handlers_test.go @@ -52,6 +52,7 @@ func handlers(h *api.Handlers) map[string]http.HandlerFunc { "materials": h.Materials, "material": h.Material, "mat-access": h.MaterialAccess, + "mat-edit": h.EditMaterial, "feed": h.CalendarFeed, "feed-put": h.UpdateCalendarFeed, "rotate": h.RotateCalendarFeed, diff --git a/controllers/api/material_access.go b/controllers/api/material_access.go index 90d0c0d..2f31333 100644 --- a/controllers/api/material_access.go +++ b/controllers/api/material_access.go @@ -15,9 +15,11 @@ import ( "git.golem15.com/golem15/summercms/modules/cabana" "git.golem15.com/golem15/summercms/modules/lagoon" "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/golem15/summercms/modules/wire" "git.golem15.com/jakub/sm-bm-plugin/controllers" "git.golem15.com/jakub/sm-bm-plugin/models" "gorm.io/gorm" + "gorm.io/gorm/clause" ) // Private materials: a material with is_private is visible only to its @@ -229,6 +231,80 @@ func (h *Handlers) managedMaterialForCaller(w http.ResponseWriter, r *http.Reque return m, true } +// writeForbidden answers 403 to a caller who may see a resource but not +// change it. +func writeForbidden(w http.ResponseWriter) { + wire.WriteJSON(w, http.StatusForbidden, map[string]any{"error": true, "message": "Forbidden."}) +} + +// editableMaterialForCaller loads the material of the {id} parameter for the +// author's routes (edit, unpublish, image upload). Unlike +// managedMaterialForCaller it tells "not yours" from "not there" (D-07): a +// material the caller cannot see (private and not theirs, a draft, not +// published yet, unknown) is a 404, and one they see but may not manage (no +// jz.bm.access_materials, or another author) a 403. Both helpers stay: the +// access panel keeps its 404. It answers itself and returns ok false then; +// manager tells whether the caller holds jz.bm.access_materials, for the +// re-check on the locked row. +func (h *Handlers) editableMaterialForCaller(w http.ResponseWriter, r *http.Request, db *gorm.DB, userID uint) (models.Material, bool, bool) { + var m models.Material + id, ok := surf.IntParam(r, "id") + if !ok { + writeNotFound(w) + return m, false, false + } + res := visibleMaterials(db, h.now(), userID).Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) + if res.Error != nil { + writeOpaque500(w) + return m, false, false + } + if res.RowsAffected == 0 { + writeNotFound(w) + return m, false, false + } + manager, err := h.managesMaterials(r.Context(), db, userID) + if err != nil { + writeOpaque500(w) + return m, false, false + } + if !canManageMaterial(m, userID, manager) { + writeForbidden(w) + return m, manager, false + } + return m, manager, true +} + +// editOutcome is what lockEditableMaterial found on the locked row. +type editOutcome int + +const ( + editAllowed editOutcome = iota + // editGone: the material is no longer visible to the caller. + editGone + // editForbidden: the caller may no longer manage it. + editForbidden +) + +// lockEditableMaterial re-loads material id inside tx, locked FOR UPDATE, +// and re-checks that the caller still sees it and may manage it, so a +// concurrent access or publication change cannot slip between the check +// of editableMaterialForCaller and the write. +func (h *Handlers) lockEditableMaterial(tx *gorm.DB, userID, id uint, manager bool) (models.Material, editOutcome, error) { + var m models.Material + res := visibleMaterials(tx.Clauses(clause.Locking{Strength: "UPDATE"}), h.now(), userID). + Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) + if res.Error != nil { + return m, editGone, res.Error + } + if res.RowsAffected == 0 { + return m, editGone, nil + } + if !canManageMaterial(m, userID, manager) { + return m, editForbidden, nil + } + return m, editAllowed, nil +} + // writeMaterialAccess answers the access of m as the caller sees it. func writeMaterialAccess(w http.ResponseWriter, db *gorm.DB, m models.Material, userID uint) { shares, err := materialShareRefs(db, []uint{m.ID}) diff --git a/controllers/api/material_edit.go b/controllers/api/material_edit.go new file mode 100644 index 0000000..1580a6b --- /dev/null +++ b/controllers/api/material_edit.go @@ -0,0 +1,276 @@ +package api + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "slices" + "strconv" + "strings" + + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/wire" + "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" +) + +// The author's routes (D-07): the student app edits, unpublishes and adds +// editor images to a material through them. They run under jwt.auth only, +// with no token twin (tokens and MCP write through the management API), and +// each one goes through editableMaterialForCaller: a material the caller +// cannot see is a 404, one they see but may not manage a 403. None of them +// changes the author, the publication time (except unpublish), is_private or +// the shares (D-08, D-10). + +// maxEditTags caps the tags the student editor may give a material. +const maxEditTags = 4 + +// materialEditKeys are the keys of PUT /materials/{id}, all required. +var materialEditKeys = []string{"revision", "title", "type", "body", "source", "url", "meeting_id", "tags"} + +// materialEditInput is the body of the student PUT /materials/{id} (D-09). +// It has no course, author, publication or access keys, so none can be sent +// (unknown keys are refused); the course is the stored one. +type materialEditInput struct { + Revision *uint `json:"revision"` + Title *string `json:"title"` + Type *string `json:"type"` + Body *string `json:"body"` + Source *string `json:"source"` + URL *string `json:"url"` + MeetingID *uint `json:"meeting_id"` + Tags *[]string `json:"tags"` +} + +// decodeMaterialEdit decodes the edit body, refusing unknown keys (the +// error, answered as "invalid JSON"). The returned messages name every key +// that is missing, and revision, title, type or tags sent as null; the +// caller adds its own rules to them before checkMaterial. +func decodeMaterialEdit(raw []byte) (materialEditInput, map[string][]string, error) { + var in materialEditInput + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&in); err != nil { + return in, nil, err + } + var keys map[string]json.RawMessage + if err := json.Unmarshal(raw, &keys); err != nil { + return in, nil, err + } + errs := map[string][]string{} + for _, k := range materialEditKeys { + if _, ok := keys[k]; !ok { + errs[k] = []string{"The " + k + " field is required."} + } + } + for k, missing := range map[string]bool{ + "revision": in.Revision == nil, + "title": in.Title == nil, + "type": in.Type == nil, + "tags": in.Tags == nil, + } { + if _, sent := keys[k]; sent && missing { + errs[k] = []string{"The " + k + " field is required."} + } + } + return in, errs, nil +} + +// normalizeBody normalises a note body the way bm-notatka push does (A6): +// CRLF and CR become LF, surrounding whitespace goes, an empty body becomes +// null and any other ends with exactly one newline. So a web save, a pull +// and a push of the same text are byte-identical and never bump the +// revision. +func normalizeBody(b *string) *string { + if b == nil { + return nil + } + s := strings.ReplaceAll(*b, "\r\n", "\n") + s = strings.ReplaceAll(s, "\r", "\n") + s = strings.TrimSpace(s) + if s == "" { + return nil + } + s += "\n" + return &s +} + +// hasWebScheme reports whether u starts with http:// or https://, in any +// case. +func hasWebScheme(u string) bool { + l := strings.ToLower(u) + return strings.HasPrefix(l, "http://") || strings.HasPrefix(l, "https://") +} + +// checkMaterialEdit adds the student-only rules of the edit body to errs: +// at most maxEditTags distinct tags, every tag from the jz_bm_tags +// dictionary (the app never creates a tag, A2), and a url that is a web +// address. +func checkMaterialEdit(db *gorm.DB, in materialEditInput, errs map[string][]string) error { + if in.URL != nil && strings.TrimSpace(*in.URL) != "" && !hasWebScheme(*in.URL) { + errs["url"] = []string{"The url must start with http:// or https://."} + } + if in.Tags == nil { + return nil + } + var slugs []string + for _, name := range *in.Tags { + slug := slugify(strings.TrimSpace(name)) + if slug != "" && !slices.Contains(slugs, slug) { + slugs = append(slugs, slug) + } + } + if len(slugs) > maxEditTags { + errs["tags"] = []string{"A material may have at most " + strconv.Itoa(maxEditTags) + " tags."} + return nil + } + if len(slugs) == 0 { + return nil + } + var n int64 + if err := db.Model(&models.Tag{}).Where("slug IN ?", slugs).Distinct("slug").Count(&n).Error; err != nil { + return err + } + if int(n) != len(slugs) { + errs["tags"] = []string{"Pick the tags from the dictionary."} + } + return nil +} + +// EditMaterial answers the student PUT /materials/{id} (D-09): the author +// (or any material manager while it has no author) saves the note's title, +// type, body, source, url, meeting and tags. Every key of +// materialEditInput is required and nothing else is accepted; the course, +// the author, the publication and the access stay as stored. revision is +// required: a stale one is a 409 with the current version +// (writeStudentConflict) and nothing is written (D-01, D-04). The row is +// locked FOR UPDATE and re-checked inside the transaction. The answer is +// the student detail, 200. +func (h *Handlers) EditMaterial(w http.ResponseWriter, r *http.Request) { + userID, ok := h.caller(w, r) + if !ok { + return + } + db, ok := h.db(w, r) + if !ok { + return + } + m, manager, ok := h.editableMaterialForCaller(w, r, db, userID) + if !ok { + return + } + raw, ok := body(w, r) + if !ok { + return + } + in, errs, err := decodeMaterialEdit(raw) + if err != nil { + writeInvalid(w, "invalid JSON: "+err.Error()) + return + } + if err := checkMaterialEdit(db, in, errs); err != nil { + writeOpaque500(w) + return + } + d := materialDraft{ + in: materialInput{ + CourseID: m.CourseID, + MeetingID: in.MeetingID, + Title: deref(in.Title), + Type: deref(in.Type), + Body: normalizeBody(in.Body), + Source: in.Source, + URL: in.URL, + Tags: in.Tags, + Revision: in.Revision, + }, + hasMeetingID: true, + } + if !h.checkMaterial(w, db, &d, errs) { + return + } + outcome := editAllowed + conflict, taken := false, false + err = lagoon.Transaction(r.Context(), db, func(ctx context.Context, tx *gorm.DB) error { + cur, why, err := h.lockEditableMaterial(tx, userID, m.ID, manager) + if err != nil || why != editAllowed { + outcome = why + return err + } + if staleRevision(d.in.Revision, cur.Revision) { + conflict = true + return nil + } + // The course is the locked row's (Pitfall 3): apply writes it as + // sent, and the edit body never carries one. + d.in.CourseID = cur.CourseID + before, err := models.MaterialContent(tx, cur.ID) + if err != nil { + return err + } + var n int64 + if err := d.identity(tx.Model(&models.Material{})).Where("id <> ?", cur.ID).Count(&n).Error; err != nil { + return err + } + if n > 0 { + taken = true + return nil + } + if err := d.apply(tx, &cur, false, h.now(), userID); err != nil { + return err + } + return h.revise(ctx, tx, cur.ID, before) + }) + switch { + case err != nil: + writeOpaque500(w) + case outcome == editGone: + writeNotFound(w) + case outcome == editForbidden: + writeForbidden(w) + case conflict: + h.writeStudentConflict(w, r, db, userID, m.ID) + case taken: + writeValidation(w, map[string][]string{"title": {"Another material already has this title and course."}}) + default: + h.MaterialIndex.Sync(r.Context(), db, m.ID) + out, found, err := h.loadMaterialDetail(r.Context(), db, userID, m.ID) + switch { + case err != nil: + writeOpaque500(w) + case !found: + writeNotFound(w) + default: + writeData(w, out) + } + } +} + +// writeStudentConflict answers 409 to a student edit that sent a stale +// revision: the current revision, its updated_at and the material in the +// student detail shape (with its body), so the editor can show the server +// version. A material that vanished meanwhile is a 404. +func (h *Handlers) writeStudentConflict(w http.ResponseWriter, r *http.Request, db *gorm.DB, userID, id uint) { + out, found, err := h.loadMaterialDetail(r.Context(), db, userID, id) + if err != nil { + writeOpaque500(w) + return + } + if !found { + writeNotFound(w) + return + } + var row models.Material + if err := db.Select("id", "updated_at").Where("id = ?", id).Take(&row).Error; err != nil { + writeOpaque500(w) + return + } + wire.WriteJSON(w, http.StatusConflict, map[string]any{ + "error": true, + "message": conflictMessage, + "revision": out.Revision, + "updated_at": models.FormatDateTime(row.UpdatedAt), + "data": out, + }) +} diff --git a/controllers/api/material_edit_test.go b/controllers/api/material_edit_test.go new file mode 100644 index 0000000..142cb99 --- /dev/null +++ b/controllers/api/material_edit_test.go @@ -0,0 +1,146 @@ +package api + +import ( + "strings" + "testing" +) + +const fullEditBody = `{"revision":3,"title":"Notatka","type":"notatka","body":"Tekst","source":null,"url":null,"meeting_id":null,"tags":["lektura"]}` + +// TestDecodeMaterialEdit checks the student edit body (D-09): every key is +// required, revision, title, type and tags may not be null, and unknown keys +// such as course_id or published_at are refused. +func TestDecodeMaterialEdit(t *testing.T) { + in, errs, err := decodeMaterialEdit([]byte(fullEditBody)) + if err != nil { + t.Fatal(err) + } + if len(errs) != 0 { + t.Errorf("full body errors = %v, want none", errs) + } + if in.Revision == nil || *in.Revision != 3 || deref(in.Title) != "Notatka" || in.Tags == nil || len(*in.Tags) != 1 { + t.Errorf("full body decoded to %+v", in) + } + for _, key := range materialEditKeys { + raw := dropKey(t, fullEditBody, key) + _, errs, err := decodeMaterialEdit([]byte(raw)) + if err != nil { + t.Errorf("without %s: decode error %v", key, err) + continue + } + if _, ok := errs[key]; !ok || len(errs) != 1 { + t.Errorf("without %s: errors = %v, want one on %s", key, errs, key) + } + } + for _, key := range []string{"revision", "title", "type", "tags"} { + raw := nullKey(t, fullEditBody, key) + _, errs, err := decodeMaterialEdit([]byte(raw)) + if err != nil { + t.Errorf("null %s: decode error %v", key, err) + continue + } + if _, ok := errs[key]; !ok || len(errs) != 1 { + t.Errorf("null %s: errors = %v, want one on %s", key, errs, key) + } + } + for _, key := range []string{"body", "source", "url", "meeting_id"} { + _, errs, err := decodeMaterialEdit([]byte(nullKey(t, fullEditBody, key))) + if err != nil || len(errs) != 0 { + t.Errorf("null %s = %v %v, want no error", key, errs, err) + } + } + for _, extra := range []string{`"course_id":1`, `"published_at":null`, `"author_user_id":2`, `"is_private":true`, `"share_user_ids":[]`} { + raw := strings.TrimSuffix(fullEditBody, "}") + "," + extra + "}" + if _, _, err := decodeMaterialEdit([]byte(raw)); err == nil { + t.Errorf("body with %s decoded, want an unknown-field error", extra) + } + } + if _, _, err := decodeMaterialEdit([]byte(`[1]`)); err == nil { + t.Error("an array body decoded, want an error") + } +} + +// dropKey removes key and its value from the flat JSON object raw. +func dropKey(t *testing.T, raw, key string) string { + t.Helper() + parts := splitTop(raw) + out := make([]string, 0, len(parts)) + for _, p := range parts { + if !strings.HasPrefix(p, `"`+key+`":`) { + out = append(out, p) + } + } + if len(out) == len(parts) { + t.Fatalf("key %s not in %s", key, raw) + } + return "{" + strings.Join(out, ",") + "}" +} + +// nullKey sets key to null in the flat JSON object raw. +func nullKey(t *testing.T, raw, key string) string { + t.Helper() + parts := splitTop(raw) + found := false + for i, p := range parts { + if strings.HasPrefix(p, `"`+key+`":`) { + parts[i] = `"` + key + `":null` + found = true + } + } + if !found { + t.Fatalf("key %s not in %s", key, raw) + } + return "{" + strings.Join(parts, ",") + "}" +} + +// splitTop splits a flat JSON object into its "key":value members, keeping +// commas inside arrays. +func splitTop(raw string) []string { + inner := strings.TrimSuffix(strings.TrimPrefix(raw, "{"), "}") + var parts []string + depth, start := 0, 0 + for i, c := range inner { + switch c { + case '[': + depth++ + case ']': + depth-- + case ',': + if depth == 0 { + parts = append(parts, inner[start:i]) + start = i + 1 + } + } + } + return append(parts, inner[start:]) +} + +// TestNormalizeBody checks the body normalisation of the edit route (A6): +// LF line ends, no surrounding blank lines, one trailing newline, and an +// empty or whitespace-only body as null. +func TestNormalizeBody(t *testing.T) { + s := func(v string) *string { return &v } + cases := []struct { + name string + in *string + want *string + }{ + {"nil", nil, nil}, + {"empty", s(""), nil}, + {"whitespace", s(" \n\t\r\n "), nil}, + {"crlf", s("a\r\nb\r\n"), s("a\nb\n")}, + {"cr", s("a\rb"), s("a\nb\n")}, + {"blank lines around", s("\n\n# Tytuł\n\ntekst\n\n\n"), s("# Tytuł\n\ntekst\n")}, + {"no newline", s("tekst"), s("tekst\n")}, + {"already normal", s("tekst\n"), s("tekst\n")}, + } + for _, tc := range cases { + got := normalizeBody(tc.in) + switch { + case tc.want == nil && got != nil: + t.Errorf("%s: normalizeBody = %q, want nil", tc.name, *got) + case tc.want != nil && (got == nil || *got != *tc.want): + t.Errorf("%s: normalizeBody = %v, want %q", tc.name, got, *tc.want) + } + } +} diff --git a/controllers/api/materials.go b/controllers/api/materials.go index 41a98e3..e9d9b54 100644 --- a/controllers/api/materials.go +++ b/controllers/api/materials.go @@ -1,6 +1,7 @@ package api import ( + "context" "net/http" "strconv" "strings" @@ -208,27 +209,41 @@ func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) { return } id, ok := surf.IntParam(r, "id") - if !ok { + if !ok || id <= 0 { writeNotFound(w) return } - var m models.Material - res := preloadMaterial(visibleMaterials(db, h.now(), userID)).Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) - if res.Error != nil { + out, found, err := h.loadMaterialDetail(r.Context(), db, userID, uint(id)) + if err != nil { writeOpaque500(w) return } - if res.RowsAffected == 0 { + if !found { writeNotFound(w) return } + writeData(w, materialView(out, viaMCP(r), includeBody(r))) +} + +// loadMaterialDetail builds the student detail of material id as userID +// sees it: the published, visible row with its course and tags, its public +// files, can_manage and the revision. found is false when the caller cannot +// see it. GET /materials/{id} and the author's routes answer with it. +func (h *Handlers) loadMaterialDetail(ctx context.Context, db *gorm.DB, userID, id uint) (materialDetailJSON, bool, error) { + var m models.Material + res := preloadMaterial(visibleMaterials(db, h.now(), userID)).Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) + if res.Error != nil { + return materialDetailJSON{}, false, res.Error + } + if res.RowsAffected == 0 { + return materialDetailJSON{}, false, nil + } var files []attach.File err := db.Where("attachment_type = ? AND attachment_id = ? AND field = ?", m.MorphName(), strconv.FormatUint(uint64(m.ID), 10), models.MaterialFilesField). Order("sort_order, id"). Find(&files).Error if err != nil { - writeOpaque500(w) - return + return materialDetailJSON{}, false, err } out := materialDetailJSON{ materialListJSON: newMaterialListJSON(m, int64(len(files))), @@ -245,14 +260,13 @@ func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) { } out.FilesCount = int64(len(out.Files)) if m.AuthorUserID == nil || *m.AuthorUserID == userID { - manager, err := h.managesMaterials(r.Context(), db, userID) + manager, err := h.managesMaterials(ctx, db, userID) if err != nil { - writeOpaque500(w) - return + return materialDetailJSON{}, false, err } out.CanManage = canManageMaterial(m, userID, manager) } - writeData(w, materialView(out, viaMCP(r), includeBody(r))) + return out, true, nil } // publishedMaterialsOfCourse lists a course's published materials that diff --git a/material_edit_postgres_test.go b/material_edit_postgres_test.go new file mode 100644 index 0000000..0d81937 --- /dev/null +++ b/material_edit_postgres_test.go @@ -0,0 +1,174 @@ +package bm_test + +import ( + "encoding/json" + "net/http" + "testing" + + "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/jakub/sm-bm-plugin/models" +) + +// TestMaterialEditAgainstDatabase checks the author's student routes +// (D-07..D-11, D-16) against Postgres: who may edit (the author; 403 for +// another manager or a non-manager; 404 for what the caller cannot see), +// the revision check with the server version in the 409, the edit body's +// rules, and that an edit keeps the course and the author. +func TestMaterialEditAgainstDatabase(t *testing.T) { + app, plugins, gdb := migrated(t) + seedFixture(t, plugins, "testdata/seed.example.json") + h, err := surf.Assemble(app, plugins) + if err != nil { + t.Fatalf("Assemble: %v", err) + } + authorID := createUser(t, gdb, "edit-author@example.test") + otherID := createUser(t, gdb, "edit-other@example.test") + plainID := createUser(t, gdb, "edit-plain@example.test") + var groupID uint + if err := gdb.Raw(` +INSERT INTO user_groups (name, code, permissions, created_at, updated_at) +VALUES ('BM materials', 'bm-materials-edit', '{"jz.bm.access_materials":1}', NOW(), NOW()) +RETURNING id`).Scan(&groupID).Error; err != nil { + t.Fatal(err) + } + for _, uid := range []uint{authorID, otherID} { + if err := gdb.Exec(`INSERT INTO users_groups (user_id, user_group_id) VALUES (?, ?)`, uid, groupID).Error; err != nil { + t.Fatal(err) + } + } + author := apiClient{t: t, h: h, token: tokenFor(t, authorID)} + other := apiClient{t: t, h: h, token: tokenFor(t, otherID)} + plain := apiClient{t: t, h: h, token: tokenFor(t, plainID)} + const base = "/_bm/api/v1" + + for _, tag := range [][2]string{{"Lektura", "lektura"}, {"Cyber", "cyber"}, {"Wojna", "wojna"}, {"Pokój", "pokoj"}, {"Prawo", "prawo"}} { + if err := gdb.Exec(`INSERT INTO jz_bm_tags (name, slug, created_at, updated_at) VALUES (?, ?, NOW(), NOW())`, tag[0], tag[1]).Error; err != nil { + t.Fatal(err) + } + } + var course models.Course + if err := gdb.Order("id").Take(&course).Error; err != nil { + t.Fatal(err) + } + var meeting models.Meeting + if err := gdb.Order("number").Take(&meeting).Error; err != nil { + t.Fatal(err) + } + insert := func(title string, private, draft bool, authorUserID *uint) uint { + t.Helper() + published := "NOW() - INTERVAL '1 day'" + if draft { + published = "NULL" + } + var id uint + err := gdb.Raw(`INSERT INTO jz_bm_materials (course_id, title, type, body, published_at, is_private, author_user_id) +VALUES (?, ?, 'notatka', 'Stara treść', `+published+`, ?, ?) RETURNING id`, course.ID, title, private, authorUserID).Scan(&id).Error + if err != nil { + t.Fatal(err) + } + return id + } + own := insert("Notatka autora", false, false, &authorID) + othersPrivate := insert("Prywatna innego", true, false, &otherID) + draft := insert("Szkic autora", false, true, &authorID) + insert("Zajęty tytuł", false, false, &authorID) + + edit := func(revision uint, mutate func(map[string]any)) string { + t.Helper() + b := map[string]any{ + "revision": revision, + "title": "Nowy tytuł", + "type": "notatka", + "body": "\r\n# Nagłówek\r\n\r\ntekst\r\n\r\n", + "source": nil, + "url": "https://example.test/a", + "meeting_id": meeting.ID, + "tags": []string{"Lektura", "cyber"}, + } + if mutate != nil { + mutate(b) + } + raw, err := json.Marshal(b) + if err != nil { + t.Fatal(err) + } + return string(raw) + } + path := func(id uint) string { return base + "/materials/" + jsonID(id) } + row := func(id uint) models.Material { + t.Helper() + var m models.Material + if err := gdb.Where("id = ?", id).Take(&m).Error; err != nil { + t.Fatal(err) + } + return m + } + + // The author saves: revision +1, the body normalised, course and author + // kept, the meeting and tags as sent. + saved := obj(author.want(http.StatusOK, "PUT", path(own), edit(1, nil))["data"]) + if saved["revision"] != float64(2) || saved["body"] != "# Nagłówek\n\ntekst\n" || saved["title"] != "Nowy tytuł" { + t.Errorf("saved = %v, want revision 2 and the normalised body", saved) + } + if id(saved["course_id"]) != jsonID(course.ID) || id(saved["meeting_id"]) != jsonID(meeting.ID) { + t.Errorf("saved course/meeting = %v/%v", saved["course_id"], saved["meeting_id"]) + } + if tags, _ := saved["tags"].([]any); len(tags) != 2 { + t.Errorf("saved tags = %v, want 2", saved["tags"]) + } + if m := row(own); m.AuthorUserID == nil || *m.AuthorUserID != authorID || m.CourseID == nil || *m.CourseID != course.ID || m.PublishedAt == nil || m.IsPrivate { + t.Errorf("row after the edit = %+v, want author, course and publication kept", m) + } + var tagCount int64 + gdb.Model(&models.Tag{}).Count(&tagCount) + if tagCount != 5 { + t.Errorf("tags = %d, want the 5 of the dictionary (none created)", tagCount) + } + + // A stale revision: 409 with the server version, nothing written. + conflict := author.want(http.StatusConflict, "PUT", path(own), edit(1, func(b map[string]any) { b["body"] = "Inna treść" })) + if conflict["revision"] != float64(2) || conflict["error"] != true || conflict["updated_at"] == nil { + t.Errorf("409 = %v, want revision 2", conflict) + } + if d := obj(conflict["data"]); d["body"] != "# Nagłówek\n\ntekst\n" || d["revision"] != float64(2) { + t.Errorf("409 data = %v, want the stored body", d) + } + if m := row(own); m.Body == nil || *m.Body != "# Nagłówek\n\ntekst\n" { + t.Errorf("body after the 409 = %v", m.Body) + } + + // Who may write: 403 for what the caller sees but may not manage, 404 + // for what they cannot see. + other.want(http.StatusForbidden, "PUT", path(own), edit(2, nil)) + plain.want(http.StatusForbidden, "PUT", path(own), edit(2, nil)) + author.want(http.StatusNotFound, "PUT", path(othersPrivate), edit(1, nil)) + author.want(http.StatusNotFound, "PUT", path(draft), edit(1, nil)) + author.want(http.StatusNotFound, "PUT", path(999999), edit(1, nil)) + + // The edit body's rules. + for name, mutate := range map[string]func(map[string]any){ + "5 tags": func(b map[string]any) { b["tags"] = []string{"lektura", "cyber", "wojna", "pokoj", "prawo"} }, + "unknown tag": func(b map[string]any) { b["tags"] = []string{"nieznany"} }, + "ftp url": func(b map[string]any) { b["url"] = "ftp://example.test/a" }, + "missing key": func(b map[string]any) { delete(b, "title") }, + "published_at": func(b map[string]any) { b["published_at"] = nil }, + "course_id": func(b map[string]any) { b["course_id"] = course.ID }, + "null tags": func(b map[string]any) { b["tags"] = nil }, + "no revision": func(b map[string]any) { delete(b, "revision") }, + "bad meeting": func(b map[string]any) { b["meeting_id"] = 999999 }, + "taken title": func(b map[string]any) { b["title"] = "Zajęty tytuł" }, + } { + code, out := author.send("PUT", path(own), edit(2, mutate)) + if code != http.StatusUnprocessableEntity { + t.Errorf("%s = %d %v, want 422", name, code, out) + } + } + if m := row(own); m.Revision != 2 { + t.Errorf("revision after the refused edits = %d, want 2", m.Revision) + } + + // An unchanged save keeps the revision. + if again := obj(author.want(http.StatusOK, "PUT", path(own), edit(2, nil))["data"]); again["revision"] != float64(2) { + t.Errorf("unchanged save revision = %v, want 2", again["revision"]) + } +} diff --git a/mcp_test.go b/mcp_test.go index 0f5ea7b..6d9a3a0 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -497,6 +497,14 @@ var studentRoutes = []string{ "DELETE /checklist-items/{id}/done", } +// authorRoutes are the author's material routes under APIPrefix: JWT only, +// with no token twin. +var authorRoutes = []string{ + "GET /materials/{id}/access", + "PUT /materials/{id}/access", + "PUT /materials/{id}", +} + // TestTokenMirrorRoutes is the invariant of the two student surfaces: every // student route exists under APIPrefix (jwt.auth, no user.* middleware) and // under TokenPrefix (user.api_token, the token bucket and the matching @@ -560,6 +568,19 @@ func TestTokenMirrorRoutes(t *testing.T) { t.Errorf("%s has a token twin", route) } } + // The author's routes (D-07) are JWT-only, like the access panel. + for _, route := range authorRoutes { + method, pattern, _ := strings.Cut(route, " ") + jwt, ok := byRoute[method+" "+bm.APIPrefix+pattern] + if !ok { + t.Errorf("%s %s is not registered", method, bm.APIPrefix+pattern) + } else if !slices.Contains(jwt, "jwt.auth") { + t.Errorf("%s %s lacks jwt.auth: %v", method, bm.APIPrefix+pattern, jwt) + } + if _, ok := byRoute[method+" "+bm.TokenPrefix+pattern]; ok { + t.Errorf("%s %s has a token twin", method, bm.TokenPrefix+pattern) + } + } } // dispatched returns r as pier dispatches it in-process: the api_token diff --git a/plugin_test.go b/plugin_test.go index 6b45b7b..9675365 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -147,6 +147,7 @@ func TestRoutes(t *testing.T) { "DELETE /_bm/api/v1/calendar-feed", "GET /_bm/api/v1/materials/{id}/access", "PUT /_bm/api/v1/materials/{id}/access", + "PUT /_bm/api/v1/materials/{id}", "GET /_bm/api/v1/token/dashboard", "GET /_bm/api/v1/token/courses", "GET /_bm/api/v1/token/courses/{id}", diff --git a/routes.go b/routes.go index acf5200..78671a3 100644 --- a/routes.go +++ b/routes.go @@ -167,6 +167,11 @@ func (p *Plugin) Routes(r pact.Router) error { g.Where("id", "[0-9]+") g.Put("/materials/{id}/access", h.UpdateMaterialAccess, "body.limit:16384") g.Where("id", "[0-9]+") + // The author's routes (D-07): edit, unpublish and editor images. Like + // the access panel they have no token twin; tokens and MCP write + // materials through the management API. + g.Put("/materials/{id}", h.EditMaterial, "throttle:30,1") + g.Where("id", "[0-9]+") if p.app != nil && p.app.HasPlugin("golem15.chat") { g.Get("/chat", h.Chat, bucketChat) g.Get("/chat/members", h.ChatMembers, bucketChat)