From 9436fb62d67e3fb580647bb21a0fe6bed68178db Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 9 Nov 2026 13:13:54 +0100 Subject: [PATCH] feat: personal pins on materials A per-user jz_bm_material_pins table and PUT/DELETE /materials/{id}/pin on the student API and its token twins: 204 for a material the caller may see, idempotent, 404 otherwise. List rows, course rows and the detail carry pinned. The plain list lists the caller's pins first (newest pin first); with q the relevance order stays. A pin writes only the pins table and never changes the material's revision. --- controllers/api/handlers_test.go | 2 + controllers/api/json.go | 2 + controllers/api/json_test.go | 29 +++++ controllers/api/material_pins.go | 121 ++++++++++++++++++ controllers/api/material_pins_test.go | 64 +++++++++ controllers/api/material_search_test.go | 3 + controllers/api/materials.go | 41 ++++-- mcp_test.go | 2 + models/material_pin.go | 16 +++ plugin_test.go | 12 +- registry.gen.go | 2 + routes.go | 10 +- ...261109130000_create_jz_bm_material_pins.go | 30 +++++ 13 files changed, 316 insertions(+), 18 deletions(-) create mode 100644 controllers/api/material_pins.go create mode 100644 controllers/api/material_pins_test.go create mode 100644 models/material_pin.go create mode 100644 updates/20261109130000_create_jz_bm_material_pins.go diff --git a/controllers/api/handlers_test.go b/controllers/api/handlers_test.go index cd00551..a1c2b3c 100644 --- a/controllers/api/handlers_test.go +++ b/controllers/api/handlers_test.go @@ -51,6 +51,8 @@ func handlers(h *api.Handlers) map[string]http.HandlerFunc { "unown": h.UnmarkOwned, "materials": h.Materials, "material": h.Material, + "pin": h.PinMaterial, + "unpin": h.UnpinMaterial, "mat-access": h.MaterialAccess, "mat-edit": h.EditMaterial, "tags": h.Tags, diff --git a/controllers/api/json.go b/controllers/api/json.go index 5dd4414..0e13a27 100644 --- a/controllers/api/json.go +++ b/controllers/api/json.go @@ -213,6 +213,8 @@ type materialListJSON struct { Tags []tagJSON `json:"tags"` FilesCount int64 `json:"files_count"` IsPrivate bool `json:"is_private"` + // Pinned is the caller's own pin; nobody else sees it. + Pinned bool `json:"pinned"` // Snippet is the matched passage of a search result (*snippetJSON, or // a plain string through MCP). It is nil, and the key absent, without // q or without a highlight. diff --git a/controllers/api/json_test.go b/controllers/api/json_test.go index 32d47a4..eae1e4d 100644 --- a/controllers/api/json_test.go +++ b/controllers/api/json_test.go @@ -77,6 +77,35 @@ func TestMaterialJSONCarriesPrivacy(t *testing.T) { } } +// TestMaterialJSONCarriesPinned checks that pinned is always present in a +// material row, false unless the caller set it, and that the detail carries +// the row's value. +func TestMaterialJSONCarriesPinned(t *testing.T) { + row := newMaterialListJSON(models.Material{ID: 1, Title: "Materiał", Type: "notatka"}, 0) + b, err := json.Marshal(row) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"pinned":false`) { + t.Errorf("material JSON %s does not contain pinned false", b) + } + row.Pinned = true + b, err = json.Marshal(row) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"pinned":true`) { + t.Errorf("pinned material JSON %s does not contain pinned true", b) + } + b, err = json.Marshal(materialDetailJSON{materialListJSON: row}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"pinned":true`) { + t.Errorf("pinned material detail JSON %s does not contain pinned true", b) + } +} + // TestVisibleMaterialSQL checks the visibility fragment: public, the author, // or a share of the caller, with the caller id bound twice. func TestVisibleMaterialSQL(t *testing.T) { diff --git a/controllers/api/material_pins.go b/controllers/api/material_pins.go new file mode 100644 index 0000000..2a52e8f --- /dev/null +++ b/controllers/api/material_pins.go @@ -0,0 +1,121 @@ +package api + +import ( + "net/http" + "strconv" + "time" + + "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" +) + +// pinnedFirstOrder orders the plain material list for userID: the caller's +// own pins first, newest pin first, then the plain list's order (newest +// publication first). id breaks every tie, so pages are stable. userID is +// the authenticated caller's id, never request input, and is inlined as a +// parsed integer like rankOrder. +func pinnedFirstOrder(userID uint) string { + return "(SELECT p.pinned_at FROM jz_bm_material_pins p WHERE p.material_id = jz_bm_materials.id AND p.user_id = " + + strconv.FormatUint(uint64(userID), 10) + + ") DESC NULLS LAST, jz_bm_materials.published_at DESC, jz_bm_materials.id DESC" +} + +// pinnedMaterialIDs tells which of ids userID pinned. No ids is an empty +// answer without a query. +func pinnedMaterialIDs(db *gorm.DB, userID uint, ids []uint) (map[uint]bool, error) { + out := map[uint]bool{} + if len(ids) == 0 { + return out, nil + } + var pinned []uint + err := db.Model(&models.MaterialPin{}). + Where("user_id = ? AND material_id IN ?", userID, ids). + Pluck("material_id", &pinned).Error + if err != nil { + return nil, err + } + for _, id := range pinned { + out[id] = true + } + return out, nil +} + +// pinTarget resolves {id} to a material userID may see and answers 404 +// itself otherwise: the same 404 as the detail for an unknown id, a draft, a +// future publication or another user's private material, so nothing hints +// that a hidden material exists. +func pinTarget(w http.ResponseWriter, r *http.Request, db *gorm.DB, now time.Time, userID uint) (uint, bool) { + id, ok := surf.IntParam(r, "id") + if !ok || id <= 0 { + writeNotFound(w) + return 0, false + } + var m models.Material + res := visibleMaterials(db.Model(&models.Material{}), now, userID). + Where("jz_bm_materials.id = ?", id). + Select("jz_bm_materials.id"). + Limit(1). + Find(&m) + if res.Error != nil { + writeOpaque500(w) + return 0, false + } + if res.RowsAffected == 0 { + writeNotFound(w) + return 0, false + } + return m.ID, true +} + +// PinMaterial answers PUT /materials/{id}/pin: it pins a material the +// caller may see, for the caller only. Pinning again keeps the first +// pinned_at. A pin is personal and is not content: it writes only +// jz_bm_material_pins and leaves the material row as it is, so the +// material's version, what search finds and what an open editor sees stay +// the same. It answers 204. +func (h *Handlers) PinMaterial(w http.ResponseWriter, r *http.Request) { + userID, ok := h.caller(w, r) + if !ok { + return + } + db, ok := h.db(w, r) + if !ok { + return + } + materialID, ok := pinTarget(w, r, db, h.now(), userID) + if !ok { + return + } + err := db.Exec(`INSERT INTO jz_bm_material_pins (material_id, user_id, pinned_at) VALUES (?, ?, ?) +ON CONFLICT (material_id, user_id) DO NOTHING`, materialID, userID, h.now().UTC()).Error + if err != nil { + writeOpaque500(w) + return + } + w.WriteHeader(http.StatusNoContent) +} + +// UnpinMaterial answers DELETE /materials/{id}/pin: it removes the caller's +// own pin from a material the caller may see. Clearing a material that is +// not pinned is not an error. Like PinMaterial it writes only +// jz_bm_material_pins and never the material itself. It answers 204. +func (h *Handlers) UnpinMaterial(w http.ResponseWriter, r *http.Request) { + userID, ok := h.caller(w, r) + if !ok { + return + } + db, ok := h.db(w, r) + if !ok { + return + } + materialID, ok := pinTarget(w, r, db, h.now(), userID) + if !ok { + return + } + if err := db.Where("material_id = ? AND user_id = ?", materialID, userID).Delete(&models.MaterialPin{}).Error; err != nil { + writeOpaque500(w) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/controllers/api/material_pins_test.go b/controllers/api/material_pins_test.go new file mode 100644 index 0000000..1720740 --- /dev/null +++ b/controllers/api/material_pins_test.go @@ -0,0 +1,64 @@ +package api_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "git.golem15.com/golem15/summercms/modules/bouncer" + "git.golem15.com/jakub/sm-bm-plugin/controllers/api" + "gorm.io/gorm" +) + +// pinnedFirstQueries runs GET /materials?course=7&type=notatka as user on a +// dry-run database and returns its count and page queries. +func pinnedFirstQueries(t *testing.T, user uint) (string, string) { + t.Helper() + db, rec := dryDB(t) + h := &api.Handlers{DB: func() *gorm.DB { return db }} + req := httptest.NewRequest(http.MethodGet, "/materials?course=7&type=notatka", nil) + req = req.WithContext(bouncer.WithUser(req.Context(), &bouncer.Principal{ID: user})) + w := httptest.NewRecorder() + h.Materials(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status %d: %s", w.Code, w.Body.String()) + } + var count, page string + for _, q := range rec.queries() { + switch { + case strings.HasPrefix(q, "SELECT count(*)"): + count = q + case strings.HasPrefix(q, "SELECT") && strings.Contains(q, `FROM "jz_bm_materials"`): + page = q + } + } + if count == "" || page == "" { + t.Fatalf("queries = %q, want a count and a page", rec.queries()) + } + return count, page +} + +// TestMaterialsPinnedFirstTrace checks the plain list's order: the caller's +// own pins first, then newest first with id as the tie-breaker, inside the +// same filters, while the count ignores pins. +func TestMaterialsPinnedFirstTrace(t *testing.T) { + count, page := pinnedFirstQueries(t, 5) + for name, q := range map[string]string{"count": count, "page": page} { + for _, want := range []string{"jz_bm_materials.course_id = 7", "jz_bm_materials.type = 'notatka'", "jz_bm_materials.is_private = FALSE"} { + if !strings.Contains(q, want) { + t.Errorf("%s query lacks %q:\n%s", name, want, q) + } + } + } + if want := "p.user_id = 5) DESC NULLS LAST, jz_bm_materials.published_at DESC, jz_bm_materials.id DESC"; !strings.Contains(page, want) { + t.Errorf("page query is not pinned first for user 5, want %q:\n%s", want, page) + } + if strings.Contains(count, "jz_bm_material_pins") { + t.Errorf("count query reads pins:\n%s", count) + } + _, other := pinnedFirstQueries(t, 6) + if !strings.Contains(other, "p.user_id = 6)") || strings.Contains(other, "p.user_id = 5") { + t.Errorf("page query of user 6 does not use only their pins:\n%s", other) + } +} diff --git a/controllers/api/material_search_test.go b/controllers/api/material_search_test.go index d83da9a..adc46b3 100644 --- a/controllers/api/material_search_test.go +++ b/controllers/api/material_search_test.go @@ -233,6 +233,9 @@ func TestMaterialsSearchTrace(t *testing.T) { if !strings.Contains(page, "array_position(ARRAY[9,3]::bigint[], jz_bm_materials.id)") { t.Errorf("page query is not in rank order:\n%s", page) } + if strings.Contains(page, "jz_bm_material_pins") { + t.Errorf("page query with q orders by pins, want the rank order only:\n%s", page) + } var out struct { Data []map[string]any `json:"data"` Meta struct { diff --git a/controllers/api/materials.go b/controllers/api/materials.go index e9d9b54..432d564 100644 --- a/controllers/api/materials.go +++ b/controllers/api/materials.go @@ -52,10 +52,12 @@ func preloadMaterial(q *gorm.DB) *gorm.DB { } // listMaterials returns one page of the published materials userID may -// see. With ids nil the page is newest first. With ids non-nil (search +// see. With ids nil the page lists the caller's pinned materials first +// (newest pin first), then newest first. With ids non-nil (search // candidates in the engine's rank order) both the count and the page are -// also limited to ids and the page follows their order; the ids never widen -// what visibleMaterials and the filter allow. An empty ids is an empty page. +// also limited to ids and the page follows their order, where pins are only +// marked; the ids never widen what visibleMaterials and the filter allow. +// An empty ids is an empty page. The count never depends on pins. func listMaterials(db *gorm.DB, now time.Time, userID uint, f materialFilter, ids []uint, page, perPage int) ([]materialListJSON, []models.Material, int64, error) { if ids != nil && len(ids) == 0 { return []materialListJSON{}, nil, 0, nil @@ -72,19 +74,20 @@ func listMaterials(db *gorm.DB, now time.Time, userID uint, f materialFilter, id if ids != nil { rowsQ = rowsQ.Where("jz_bm_materials.id IN ?", ids).Order(rankOrder(ids)) } else { - rowsQ = rowsQ.Order("jz_bm_materials.published_at DESC, jz_bm_materials.id DESC") + rowsQ = rowsQ.Order(pinnedFirstOrder(userID)) } var rows []models.Material err := rowsQ.Offset((page - 1) * perPage).Limit(perPage).Find(&rows).Error if err != nil { return nil, nil, 0, err } - out, err := materialsJSON(db, rows) + out, err := materialsJSON(db, userID, rows) return out, rows, total, err } -// materialsJSON builds list rows with their file counts. -func materialsJSON(db *gorm.DB, rows []models.Material) ([]materialListJSON, error) { +// materialsJSON builds list rows with their file counts and userID's own +// pins. +func materialsJSON(db *gorm.DB, userID uint, rows []models.Material) ([]materialListJSON, error) { ids := make([]string, 0, len(rows)) for _, m := range rows { ids = append(ids, strconv.FormatUint(uint64(m.ID), 10)) @@ -107,9 +110,15 @@ func materialsJSON(db *gorm.DB, rows []models.Material) ([]materialListJSON, err counts[row.AttachmentID] = row.Count } } + pinned, err := pinnedMaterialIDs(db, userID, materialIDs(rows)) + if err != nil { + return nil, err + } out := make([]materialListJSON, 0, len(rows)) for _, m := range rows { - out = append(out, newMaterialListJSON(m, counts[strconv.FormatUint(uint64(m.ID), 10)])) + row := newMaterialListJSON(m, counts[strconv.FormatUint(uint64(m.ID), 10)]) + row.Pinned = pinned[m.ID] + out = append(out, row) } return out, nil } @@ -119,7 +128,9 @@ func materialsJSON(db *gorm.DB, rows []models.Material) ([]materialListJSON, err // shape of lagoon.Paginate. With q (2 to 100 characters) the engine answers // candidates, which SQL re-gates, counts and pages in rank order, each row // with at most one snippet; when the engine cannot answer the request is a -// 503. Without q the list is newest first and never calls the engine. +// 503. Without q the caller's own pins come first (newest pin first), then +// newest first, and the engine is never called. With q the order is +// relevance and rows only carry pinned. func (h *Handlers) Materials(w http.ResponseWriter, r *http.Request) { userID, ok := h.caller(w, r) if !ok { @@ -227,7 +238,7 @@ func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) { // 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 +// files, can_manage, the revision and the caller's own pin. 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 @@ -259,6 +270,11 @@ func (h *Handlers) loadMaterialDetail(ctx context.Context, db *gorm.DB, userID, out.Files = append(out.Files, fileJSON{ID: f.ID, URL: f.URL(), Name: f.FileName, Size: f.FileSize, Mime: f.ContentType}) } out.FilesCount = int64(len(out.Files)) + pinned, err := pinnedMaterialIDs(db, userID, []uint{m.ID}) + if err != nil { + return materialDetailJSON{}, false, err + } + out.Pinned = pinned[m.ID] if m.AuthorUserID == nil || *m.AuthorUserID == userID { manager, err := h.managesMaterials(ctx, db, userID) if err != nil { @@ -270,7 +286,8 @@ func (h *Handlers) loadMaterialDetail(ctx context.Context, db *gorm.DB, userID, } // publishedMaterialsOfCourse lists a course's published materials that -// userID may see, newest first, for /courses/{id}. +// userID may see, newest first, for /courses/{id}. Rows carry the caller's +// pins but keep this order. func publishedMaterialsOfCourse(db *gorm.DB, now time.Time, userID uint, courseID uint) ([]materialListJSON, error) { var rows []models.Material err := preloadMaterial(visibleMaterials(db, now, userID)). @@ -280,5 +297,5 @@ func publishedMaterialsOfCourse(db *gorm.DB, now time.Time, userID uint, courseI if err != nil { return nil, err } - return materialsJSON(db, rows) + return materialsJSON(db, userID, rows) } diff --git a/mcp_test.go b/mcp_test.go index 84d66db..9d04c64 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -485,6 +485,8 @@ var studentRoutes = []string{ "DELETE /readings/{id}/owned", "GET /materials", "GET /materials/{id}", + "PUT /materials/{id}/pin", + "DELETE /materials/{id}/pin", "GET /checklists", "POST /checklists", "GET /checklists/{id}", diff --git a/models/material_pin.go b/models/material_pin.go new file mode 100644 index 0000000..a91fe03 --- /dev/null +++ b/models/material_pin.go @@ -0,0 +1,16 @@ +package models + +import "time" + +// MaterialPin is a personal pin: one row per material and user (a +// golem15.user users row). Only that user sees it, and it lists the +// material first on their own Materiały list. +type MaterialPin struct { + ID uint `gorm:"column:id;primaryKey"` + MaterialID uint `gorm:"column:material_id"` + UserID uint `gorm:"column:user_id"` + PinnedAt time.Time `gorm:"column:pinned_at"` +} + +// TableName is the plugin table. +func (MaterialPin) TableName() string { return "jz_bm_material_pins" } diff --git a/plugin_test.go b/plugin_test.go index 0a7b0cf..1a2f7b2 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -76,11 +76,11 @@ func TestPluginActivates(t *testing.T) { if p.ID() != "jz.bm" || !slices.Equal(p.Requires(), []string{"golem15.user"}) { t.Errorf("ID %q Requires %v", p.ID(), p.Requires()) } - if n := len(p.Models()); n != 14 { - t.Errorf("Models = %d, want 14", n) + if n := len(p.Models()); n != 15 { + t.Errorf("Models = %d, want 15", n) } - if n := len(p.Migrations()); n != 22 { - t.Errorf("Migrations = %d, want 22", n) + if n := len(p.Migrations()); n != 23 { + t.Errorf("Migrations = %d, want 23", n) } if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:seed" }) { t.Error("bm:seed is not registered") @@ -131,6 +131,8 @@ func TestRoutes(t *testing.T) { "DELETE /_bm/api/v1/readings/{id}/owned", "GET /_bm/api/v1/materials", "GET /_bm/api/v1/materials/{id}", + "PUT /_bm/api/v1/materials/{id}/pin", + "DELETE /_bm/api/v1/materials/{id}/pin", "GET /_bm/api/v1/checklists", "POST /_bm/api/v1/checklists", "GET /_bm/api/v1/checklists/{id}", @@ -164,6 +166,8 @@ func TestRoutes(t *testing.T) { "DELETE /_bm/api/v1/token/readings/{id}/owned", "GET /_bm/api/v1/token/materials", "GET /_bm/api/v1/token/materials/{id}", + "PUT /_bm/api/v1/token/materials/{id}/pin", + "DELETE /_bm/api/v1/token/materials/{id}/pin", "GET /_bm/api/v1/token/checklists", "POST /_bm/api/v1/token/checklists", "GET /_bm/api/v1/token/checklists/{id}", diff --git a/registry.gen.go b/registry.gen.go index e0595f7..0aa6a6f 100644 --- a/registry.gen.go +++ b/registry.gen.go @@ -22,6 +22,7 @@ func generatedModels() []any { &models.Material{}, &models.MaterialTag{}, &models.MaterialShare{}, + &models.MaterialPin{}, &models.Meeting{}, &models.Reading{}, &models.ReadingProgress{}, @@ -54,6 +55,7 @@ func generatedMigrations() []*gormigrate.Migration { updates.CreateMaterialFileTexts(), updates.MoveZjazdTagsToMeetings(), updates.AddRevisionToMaterials(), + updates.CreateMaterialPins(), } } diff --git a/routes.go b/routes.go index cc35f16..354c1ca 100644 --- a/routes.go +++ b/routes.go @@ -179,7 +179,8 @@ const TokenPrefix = APIPrefix + "/token" // Routes declares the student API and the management API. Every student // route needs a golem15.user JWT (jwt.auth); students only read, except for -// their own reading progress and owned marks and their own checklists. +// their own reading progress and owned marks, their own material pins and +// their own checklists. func (p *Plugin) Routes(r pact.Router) error { h := p.handlers() r.Group(APIPrefix, surf.Use("jwt.auth", "locale.from-principal"), func(g pact.Router) { @@ -406,7 +407,8 @@ func (p *Plugin) handlers() *api.Handlers { } // mountStudent mounts the student routes on g, with read appended to every -// GET route and write to the reading marks. The jwt.auth group and the token +// GET route and write to the reading marks, the material pins and the +// checklist writes. The jwt.auth group and the token // twins share it, so both lists cannot drift. func mountStudent(g pact.Router, h *api.Handlers, read, write []string) { g.Get("/dashboard", h.Dashboard, read...) @@ -428,6 +430,10 @@ func mountStudent(g pact.Router, h *api.Handlers, read, write []string) { g.Get("/materials", h.Materials, read...) g.Get("/materials/{id}", h.Material, read...) g.Where("id", "[0-9]+") + g.Put("/materials/{id}/pin", h.PinMaterial, write...) + g.Where("id", "[0-9]+") + g.Delete("/materials/{id}/pin", h.UnpinMaterial, write...) + g.Where("id", "[0-9]+") g.Get("/checklists", h.Checklists, read...) g.Post("/checklists", h.CreateChecklist, write...) g.Get("/checklists/{id}", h.Checklist, read...) diff --git a/updates/20261109130000_create_jz_bm_material_pins.go b/updates/20261109130000_create_jz_bm_material_pins.go new file mode 100644 index 0000000..2d3d8b6 --- /dev/null +++ b/updates/20261109130000_create_jz_bm_material_pins.go @@ -0,0 +1,30 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// CreateMaterialPins returns the +// 20261109130000_create_jz_bm_material_pins gormigrate entry: a student's +// own pins on materials. A pinned material lists first on that student's +// Materiały list, and nobody else sees the pin. The rows go with the +// material or the golem15.user user. +func CreateMaterialPins() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20261109130000_create_jz_bm_material_pins", + Migrate: func(tx *gorm.DB) error { + return execAll(tx, `CREATE TABLE jz_bm_material_pins ( + id BIGSERIAL PRIMARY KEY, + material_id BIGINT NOT NULL REFERENCES jz_bm_materials (id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + pinned_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + UNIQUE (material_id, user_id) +); +CREATE INDEX jz_bm_material_pins_user_id_index ON jz_bm_material_pins (user_id)`) + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec("DROP TABLE IF EXISTS jz_bm_material_pins").Error + }, + } +}