feat(groups): a group becomes somewhere you can go

It was a heading over some rows: a name, a colour, and whether it was folded.
Enough to tidy a list, and not enough to be what people mean by it — "Arbejde" is
a body of work with context of its own, and documents that belong to all of it
rather than to any one project inside it.

`/gruppe/{id}` carries three things a heading cannot. The projects in it, each
with what is still open — open tasks, not all of them, because "12" beside a
project somebody finished last year is a number that means nothing. A description,
written in place and saved on blur, the same shape a task's description has. And
the files that govern the whole group: the contract, the drawing, the agreement
everything else rests on.

The name in the sidebar is a link to it and the chevron beside it still folds.
Two intentions, two targets — the single one they used to share could only ever
serve the smaller.

Migration 0012 rebuilds `attachments`, because its CHECK spelled out exactly which
parent columns may be set and SQLite cannot alter a CHECK. The rule is still
"exactly one parent", counted rather than written out as pairwise comparisons:
with three columns that is three clauses, with four it is six, and the version
that is wrong looks exactly like the one that is right.

A group's file is reachable by its owner and nobody else. A group belongs to one
person and is never shared, so `reachAttachment` asks about ownership rather than
a project role — both old callers resolved straight to a task, so a group's
attachment answered 404 for the person who uploaded it.

Three tests also stopped racing their own navigation. Creating a project navigates
and `goto` is a promise nobody was holding, so a quick add fired straight after
could land in the Inbox instead — intermittent, and it read as the quick add
having failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/httpapi/comments_handlers.go

@@ -300,16 +300,48 @@ func (s *Server) storeUpload(file io.Reader) (string, int64, error) {
 	return rel, size, nil
 }
 
-func (s *Server) handleDownloadAttachment(w http.ResponseWriter, r *http.Request) {
-	attachmentID := chi.URLParam(r, "attachmentID")
+// reachAttachment answers whether this person may see an attachment at all, and
+// whether they may change it.
+//
+// An attachment hangs on exactly one of three things, and each has its own idea of
+// who may reach it. A task's is governed by its project's roles, directly or
+// through the comment it hangs on. A group's is governed by nothing but ownership:
+// a group belongs to one person, is never shared, and a group that is not yours is
+// not found — the same rule the rest of the group routes use.
+//
+// Both callers below used to resolve straight to a task, so a group's attachment
+// answered 404 for its own owner.
+func (s *Server) reachAttachment(r *http.Request, attachmentID string) (canEdit bool, err error) {
 	user := userFrom(r.Context())
 
+	a, err := s.db.GetAttachment(r.Context(), attachmentID)
+	if err != nil {
+		return false, err
+	}
+
+	if a.GroupID != "" {
+		if _, err := s.db.GetProjectGroup(r.Context(), a.GroupID, user.ID); err != nil {
+			return false, err
+		}
+		// Nobody else can reach it, so whoever can, can change it.
+		return true, nil
+	}
+
 	taskID, err := s.db.AttachmentTask(r.Context(), attachmentID)
 	if err != nil {
-		s.storeError(w, r, "attachment task", err)
-		return
+		return false, err
 	}
-	if _, err := store.TaskRole(r.Context(), s.db, taskID, user.ID); err != nil {
+	role, err := store.TaskRole(r.Context(), s.db, taskID, user.ID)
+	if err != nil {
+		return false, err
+	}
+	return role.CanEdit(), nil
+}
+
+func (s *Server) handleDownloadAttachment(w http.ResponseWriter, r *http.Request) {
+	attachmentID := chi.URLParam(r, "attachmentID")
+
+	if _, err := s.reachAttachment(r, attachmentID); err != nil {
 		writeError(w, http.StatusNotFound, CodeNotFound, "not found")
 		return
 	}
@@ -354,15 +386,9 @@ func (s *Server) handleDownloadAttachment(w http.ResponseWriter, r *http.Request
 
 func (s *Server) handleDeleteAttachment(w http.ResponseWriter, r *http.Request) {
 	attachmentID := chi.URLParam(r, "attachmentID")
-	user := userFrom(r.Context())
 
-	taskID, err := s.db.AttachmentTask(r.Context(), attachmentID)
-	if err != nil {
-		s.storeError(w, r, "attachment task", err)
-		return
-	}
-	role, err := store.TaskRole(r.Context(), s.db, taskID, user.ID)
-	if err != nil || !role.CanEdit() {
+	canEdit, err := s.reachAttachment(r, attachmentID)
+	if err != nil || !canEdit {
 		writeError(w, http.StatusNotFound, CodeNotFound, "not found")
 		return
 	}

internal/httpapi/projectgroups_handlers.go

@@ -2,6 +2,8 @@ package httpapi
 
 import (
 	"net/http"
+	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/go-chi/chi/v5"
@@ -17,16 +19,17 @@ import (
 // that is not yours is not found.
 
 type projectGroupJSON struct {
-	ID        string  `json:"id"`
-	Name      string  `json:"name"`
-	Color     string  `json:"color"`
-	Collapsed bool    `json:"collapsed"`
-	SortOrder float64 `json:"sort_order"`
+	ID          string  `json:"id"`
+	Name        string  `json:"name"`
+	Color       string  `json:"color"`
+	Description string  `json:"description"`
+	Collapsed   bool    `json:"collapsed"`
+	SortOrder   float64 `json:"sort_order"`
 }
 
 func toProjectGroupJSON(g store.ProjectGroup) projectGroupJSON {
 	return projectGroupJSON{
-		ID: g.ID, Name: g.Name, Color: g.Color,
+		ID: g.ID, Name: g.Name, Color: g.Color, Description: g.Description,
 		Collapsed: g.Collapsed, SortOrder: g.SortOrder,
 	}
 }
@@ -45,9 +48,10 @@ func (s *Server) handleListProjectGroups(w http.ResponseWriter, r *http.Request)
 }
 
 type projectGroupRequest struct {
-	Name      *string `json:"name"`
-	Color     *string `json:"color"`
-	Collapsed *bool   `json:"collapsed"`
+	Name        *string `json:"name"`
+	Color       *string `json:"color"`
+	Description *string `json:"description"`
+	Collapsed   *bool   `json:"collapsed"`
 }
 
 func (s *Server) handleCreateProjectGroup(w http.ResponseWriter, r *http.Request) {
@@ -73,6 +77,9 @@ func (s *Server) handleCreateProjectGroup(w http.ResponseWriter, r *http.Request
 	if req.Color != nil {
 		g.Color = *req.Color
 	}
+	if req.Description != nil {
+		g.Description = strings.TrimSpace(*req.Description)
+	}
 	if req.Collapsed != nil {
 		g.Collapsed = *req.Collapsed
 	}
@@ -105,8 +112,18 @@ func (s *Server) handleUpdateProjectGroup(w http.ResponseWriter, r *http.Request
 		return
 	}
 
+	if req.Description != nil {
+		description := strings.TrimSpace(*req.Description)
+		if len(description) > 4000 {
+			writeFieldErrors(w, map[string]string{"description": "must be at most 4000 characters"})
+			return
+		}
+		req.Description = &description
+	}
+
 	if err := s.db.UpdateProjectGroup(r.Context(), groupID, userID, store.ProjectGroupUpdate{
-		Name: req.Name, Color: req.Color, Collapsed: req.Collapsed,
+		Name: req.Name, Color: req.Color, Description: req.Description,
+		Collapsed: req.Collapsed,
 	}); err != nil {
 		s.storeError(w, r, "update project group", err)
 		return
@@ -120,6 +137,126 @@ func (s *Server) handleUpdateProjectGroup(w http.ResponseWriter, r *http.Request
 	writeJSON(w, http.StatusOK, toProjectGroupJSON(*g))
 }
 
+// groupProjectJSON is a project as the group's page lists it: the project, plus
+// how much is left in it. Open tasks, not all of them — "12" beside a project
+// somebody finished last year is a number that means nothing.
+type groupProjectJSON struct {
+	projectJSON
+	OpenTasks int `json:"open_tasks"`
+}
+
+// handleGetProjectGroup is the group as a place rather than a heading.
+//
+// A heading can carry a name. A page carries what the group *is* — the sentence
+// that says why these projects are one body of work, the documents that belong to
+// all of them rather than to any one, and the projects themselves with what is
+// left in each.
+//
+// The projects come with the group rather than being fetched per row, because the
+// page is nothing without them and two round trips to draw one list is one too
+// many.
+func (s *Server) handleGetProjectGroup(w http.ResponseWriter, r *http.Request) {
+	groupID := chi.URLParam(r, "groupID")
+	userID := userFrom(r.Context()).ID
+
+	g, err := s.db.GetProjectGroup(r.Context(), groupID, userID)
+	if err != nil {
+		s.storeError(w, r, "get project group", err)
+		return
+	}
+
+	projects, err := s.db.ListProjects(r.Context(), userID, false)
+	if err != nil {
+		s.internal(w, r, "list projects", err)
+		return
+	}
+	counts, err := s.db.OpenTaskCounts(r.Context(), groupID, userID)
+	if err != nil {
+		s.internal(w, r, "open task counts", err)
+		return
+	}
+	inside := make([]groupProjectJSON, 0, 4)
+	for _, p := range projects {
+		if p.GroupID == groupID {
+			inside = append(inside, groupProjectJSON{
+				projectJSON: toProjectJSON(p), OpenTasks: counts[p.ID],
+			})
+		}
+	}
+
+	files, err := s.db.ListGroupAttachments(r.Context(), groupID)
+	if err != nil {
+		s.internal(w, r, "list group attachments", err)
+		return
+	}
+	attachments := make([]attachmentJSON, 0, len(files))
+	for _, a := range files {
+		attachments = append(attachments, toAttachmentJSON(a))
+	}
+
+	writeJSON(w, http.StatusOK, map[string]any{
+		"group": toProjectGroupJSON(*g), "projects": inside, "attachments": attachments,
+	})
+}
+
+// handleUploadGroupAttachment puts a document on the group itself.
+//
+// The same storage as everywhere else — content-addressed by hash, so the path
+// never contains anything the uploader chose. What differs is only the parent.
+func (s *Server) handleUploadGroupAttachment(w http.ResponseWriter, r *http.Request) {
+	groupID := chi.URLParam(r, "groupID")
+	user := userFrom(r.Context())
+
+	// Scoped by the lookup rather than by a separate check: a group that is not
+	// yours is not found, which is the rule for everything in this file.
+	if _, err := s.db.GetProjectGroup(r.Context(), groupID, user.ID); err != nil {
+		s.storeError(w, r, "get project group", err)
+		return
+	}
+
+	r.Body = http.MaxBytesReader(w, r.Body, maxUploadBytes+(1<<20))
+	if err := r.ParseMultipartForm(8 << 20); err != nil {
+		writeError(w, http.StatusRequestEntityTooLarge, CodePayloadTooLarge,
+			"that file is too large (25 MB maximum)")
+		return
+	}
+	defer r.MultipartForm.RemoveAll()
+
+	file, header, err := r.FormFile("file")
+	if err != nil {
+		writeFieldErrors(w, map[string]string{"file": "required"})
+		return
+	}
+	defer file.Close()
+
+	if header.Size > maxUploadBytes {
+		writeError(w, http.StatusRequestEntityTooLarge, CodePayloadTooLarge,
+			"that file is too large (25 MB maximum)")
+		return
+	}
+
+	stored, size, err := s.storeUpload(file)
+	if err != nil {
+		s.internal(w, r, "store upload", err)
+		return
+	}
+
+	a := &store.Attachment{
+		GroupID:    groupID,
+		Filename:   safeFilename(header.Filename),
+		MimeType:   detectMime(header.Filename, header.Header.Get("Content-Type")),
+		Size:       size,
+		Path:       stored,
+		UploadedBy: user.ID,
+	}
+	if err := s.db.CreateAttachment(r.Context(), a); err != nil {
+		os.Remove(filepath.Join(s.cfg.FilesDir(), stored))
+		s.internal(w, r, "record attachment", err)
+		return
+	}
+	writeJSON(w, http.StatusCreated, toAttachmentJSON(*a))
+}
+
 // handleDeleteProjectGroup removes the heading. The projects under it come back
 // out as ungrouped, which is why this is not behind a confirmation about losing
 // work: there is none to lose.

internal/httpapi/projectgroups_test.go

@@ -269,3 +269,83 @@ func TestAColourMustBeOneFromThePalette(t *testing.T) {
 		t.Errorf("name = %v", renamed["name"])
 	}
 }
+
+// A group is somewhere you can go, not only a heading.
+func TestAGroupIsAPageWithItsProjectsDescriptionAndFiles(t *testing.T) {
+	ts := newTestServer(t)
+	ts.bootstrap(t)
+
+	_, group := ts.do(t, "POST", "/api/v1/project-groups", map[string]any{"name": "Arbejde"})
+	groupID := group["id"].(string)
+
+	_, first := ts.do(t, "POST", "/api/v1/projects", map[string]any{
+		"name": "Sæsonstart", "group_id": groupID,
+	})
+	ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Regnskab", "group_id": groupID})
+	// And one outside it, which must not appear.
+	ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Haven"})
+
+	// Two open, one closed — the count is what is left, not what has ever been.
+	ts.do(t, "POST", "/api/v1/tasks", map[string]any{
+		"content": "skriv oplæg", "project_id": first["id"].(string),
+	})
+	_, done := ts.do(t, "POST", "/api/v1/tasks", map[string]any{
+		"content": "book lokale", "project_id": first["id"].(string),
+	})
+	ts.do(t, "POST", "/api/v1/tasks/"+done["id"].(string)+"/complete", nil)
+
+	resp, page := ts.do(t, "GET", "/api/v1/project-groups/"+groupID, nil)
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("get group: status %d, body %v", resp.StatusCode, page)
+	}
+	projects, _ := page["projects"].([]any)
+	if len(projects) != 2 {
+		t.Fatalf("the page lists %d projects, want 2", len(projects))
+	}
+	for _, raw := range projects {
+		p := raw.(map[string]any)
+		if p["name"] == "Haven" {
+			t.Error("a project outside the group is on its page")
+		}
+		if p["name"] == "Sæsonstart" && p["open_tasks"] != float64(1) {
+			t.Errorf("open_tasks = %v, want 1 — the closed one must not count", p["open_tasks"])
+		}
+	}
+
+	// The description, which is the part a heading cannot carry.
+	if resp, _ := ts.do(t, "PATCH", "/api/v1/project-groups/"+groupID, map[string]any{
+		"description": "Alt det, der har en faktura for enden.",
+	}); resp.StatusCode != http.StatusOK {
+		t.Fatalf("set description: status %d", resp.StatusCode)
+	}
+	_, again := ts.do(t, "GET", "/api/v1/project-groups/"+groupID, nil)
+	if g, _ := again["group"].(map[string]any); g["description"] != "Alt det, der har en faktura for enden." {
+		t.Errorf("description = %v", g["description"])
+	}
+	// And setting only the description must not lose the name or colour.
+	if g, _ := again["group"].(map[string]any); g["name"] != "Arbejde" {
+		t.Errorf("the name went with the description: %v", g["name"])
+	}
+}
+
+// A group belongs to one person and is never shared, so somebody else's group is
+// not found — including its files.
+func TestAGroupsPageAndFilesAreItsOwnersAlone(t *testing.T) {
+	ts := newTestServer(t)
+	ts.bootstrap(t)
+	other := ts.newUser(t, "anden@example.dk", "Anden")
+
+	_, group := ts.do(t, "POST", "/api/v1/project-groups", map[string]any{"name": "Arbejde"})
+	groupID := group["id"].(string)
+
+	for _, c := range []struct{ method, path string }{
+		{"GET", "/api/v1/project-groups/" + groupID},
+		{"PATCH", "/api/v1/project-groups/" + groupID},
+		{"POST", "/api/v1/project-groups/" + groupID + "/attachments"},
+	} {
+		resp, _ := other.do(t, c.method, c.path, map[string]any{"description": "min nu"})
+		if resp.StatusCode != http.StatusNotFound {
+			t.Errorf("%s %s as somebody else: status %d, want 404", c.method, c.path, resp.StatusCode)
+		}
+	}
+}

internal/httpapi/server.go

@@ -344,8 +344,10 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server
 				r.Post("/", s.handleCreateProjectGroup)
 				// Above /{groupID}, or chi reads "reorder" as an id.
 				r.Post("/reorder", s.handleReorderProjectGroups)
+				r.Get("/{groupID}", s.handleGetProjectGroup)
 				r.Patch("/{groupID}", s.handleUpdateProjectGroup)
 				r.Delete("/{groupID}", s.handleDeleteProjectGroup)
+				r.Post("/{groupID}/attachments", s.handleUploadGroupAttachment)
 			})
 
 			// The trash is outside the block above: a deleted project is invisible