From e46aaa462a054598724ec2f7a13b354251128667 Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Thu, 20 Aug 2026 14:56:26 +0200 Subject: [PATCH] =?UTF-8?q?feat(projects):=20sektioner=20kan=20tr=C3=A6kke?= =?UTF-8?q?s=20op=20og=20ned,=20og=20b=C3=A5ndet=20st=C3=A5r=20ikke=20l?= =?UTF-8?q?=C3=A6ngere=20sk=C3=A6vt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rækkefølgen af sektioner *er* planen — "Planlægning" står før "I gang", fordi det er den vej arbejdet går — og indtil nu kunne den kun laves om ved at slette en sektion og lave den igen, hvilket smider dens opgaver ud i det usektionerede område på vejen. Hele overskriften er håndtaget. En sektionsoverskrift indeholder ikke noget, der selv bliver trukket, så der er ingen geste at stjæle — hvilket er grunden til, at sidebjælkens rækker skulle have et greb, og grunden til at disse ikke skal. Ikke draggable mens den omdøbes: et draggable element lader ikke markøren vælge tekst i det felt, det indeholder. Serversiden er hele listen på én gang som projekter og grupper, og af samme grund: et projekt har en håndfuld sektioner, så 0, 1, 2 … er én lille transaktion uden præcision at løbe tør for, og en liste kan ikke ende halvt anvendt. `ReorderSections` er scopet til projektet og ikke til en ejer — en sektion har ingen ejer, den hører til et projekt, og om du må rykke rundt er et spørgsmål om det projekt, som handleren allerede har stillet. Id'er fra et andet projekt springes over af WHERE-klausulen; der er en prøve på præcis det, fordi ingenting ellers kontrollerer id'erne enkeltvis. Båndet stod fire pixels skævt. Det var trukket ud med --s2 og polstret med --s3 — samme idé gjort med to forskellige tal — så det blev otte pixels bredere end listen i hver side, og teksten landede fire pixels uden for den kolonne, rækkerne holder. Til venstre var det usynligt. Til højre var det Omdøb og Slet, der stod fire pixels forbi alt over og under sig, hvilket er præcis langt nok til at ligne en fejl og ikke langt nok til at se hvorfor. En række er `.row` med `padding: var(--s2)` inde i en boks uden, så båndet har nu samme polstring og ingen margen. Board-visningen er ikke rørt: der er kolonnerne vandrette, og det er en anden geste. Co-Authored-By: Claude Opus 5 --- docs/openapi.yaml | 30 +++++ internal/httpapi/projects_handlers.go | 28 +++++ internal/httpapi/projects_test.go | 65 ++++++++++ internal/httpapi/server.go | 1 + internal/store/projects.go | 29 +++++ web/e2e/smoke.spec.js | 70 +++++++++++ web/src/lib/api.js | 4 + web/src/lib/dnd.js | 1 + web/src/routes/projekt/[id]/+page.svelte | 152 +++++++++++++++++++++-- 9 files changed, 373 insertions(+), 7 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 46db003..75f03a1 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2049,6 +2049,36 @@ paths: "404": { $ref: "#/components/responses/NotFound" } "422": { $ref: "#/components/responses/Validation" } + /api/v1/projects/{projectID}/sections/reorder: + parameters: + - { name: projectID, in: path, required: true, schema: { type: string } } + post: + tags: [projects] + summary: Set the order the project's sections appear in. Editors and owners. + description: | + The whole list at once, for the same reasons as `/projects/reorder`: a + project has a handful of sections, so sending the order you want is + simpler than midpoints and cannot land half-applied. + + Ids belonging to another project are skipped rather than refused. + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [ids] + properties: + ids: + type: array + maxItems: 500 + items: { type: string } + description: Section ids, top to bottom. + responses: + "204": { description: Reordered. } + "404": { $ref: "#/components/responses/NotFound" } + "422": { $ref: "#/components/responses/Validation" } + /api/v1/sections/{sectionID}: parameters: - { name: sectionID, in: path, required: true, schema: { type: string } } diff --git a/internal/httpapi/projects_handlers.go b/internal/httpapi/projects_handlers.go index 0ba3dd0..2ee6908 100644 --- a/internal/httpapi/projects_handlers.go +++ b/internal/httpapi/projects_handlers.go @@ -356,6 +356,34 @@ func (s *Server) handleCreateSection(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusCreated, sectionJSON{sec.ID, sec.ProjectID, sec.Name, sec.SortOrder}) } +// handleReorderSections sets the order of the sections inside one project. +// +// Under /projects/{projectID}/ rather than beside the other section routes, which +// hang off a section id: the thing being changed here is the project's list, not +// any one section in it. That also puts it behind the same project permission +// check as everything else in this subtree, instead of resolving a section to its +// project one id at a time and hoping they all named the same one. +func (s *Server) handleReorderSections(w http.ResponseWriter, r *http.Request) { + var req reorderProjectsRequest + if err := decodeJSON(w, r, &req); err != nil { + return + } + // The same ceiling as the projects list, and a bound on the transaction rather + // than an opinion about how many sections a project may have. + if len(req.IDs) > 500 { + writeFieldErrors(w, map[string]string{"ids": "at most 500 at a time"}) + return + } + + projectID := chi.URLParam(r, "projectID") + if err := s.db.ReorderSections(r.Context(), projectID, req.IDs); err != nil { + s.internal(w, r, "reorder sections", err) + return + } + s.activity(r, projectID, "", "section.updated", nil) + w.WriteHeader(http.StatusNoContent) +} + // sectionAccess resolves a section to its project and checks the caller may edit // it. Sections have no permissions of their own — they belong to a project, and // that is where access is decided. diff --git a/internal/httpapi/projects_test.go b/internal/httpapi/projects_test.go index 8742ccc..5a938c9 100644 --- a/internal/httpapi/projects_test.go +++ b/internal/httpapi/projects_test.go @@ -327,6 +327,71 @@ func TestAProjectCanHaveSeveralSections(t *testing.T) { } } +// Sections can be dragged into another order, and the order is the project's. +// +// The list is what a project's plan *is* — "Planlægning" before "I gang" is the +// order the work happens in — and until this endpoint existed the only way to +// change it was to delete a section and make it again, which moves its tasks to +// the unsectioned area on the way past. +func TestSectionsCanBeReordered(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + _, project := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Ombygning"}) + id := project["id"].(string) + + ids := map[string]string{} + names := []string{"Planlægning", "I gang", "Til gennemsyn"} + for _, name := range names { + _, body := ts.do(t, "POST", "/api/v1/projects/"+id+"/sections", map[string]any{"name": name}) + ids[name] = body["id"].(string) + } + + want := []string{"Til gennemsyn", "Planlægning", "I gang"} + order := make([]string, len(want)) + for i, name := range want { + order[i] = ids[name] + } + + resp, body := ts.do(t, "POST", "/api/v1/projects/"+id+"/sections/reorder", + map[string]any{"ids": order}) + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("reorder: status %d, body %v", resp.StatusCode, body) + } + + _, listed := ts.do(t, "GET", "/api/v1/projects/"+id+"/sections", nil) + got, _ := listed["sections"].([]any) + if len(got) != len(want) { + t.Fatalf("the project has %d sections, want %d", len(got), len(want)) + } + for i, raw := range got { + if name := raw.(map[string]any)["name"]; name != want[i] { + t.Errorf("section %d is %v, want %q", i, name, want[i]) + } + } + + // A section belonging to somebody else's project is not moved by naming it + // here. The ids in the body are not checked one by one — the WHERE clause is + // what refuses them — so this is the assertion that it actually does. + _, other := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Sommerhus"}) + otherID := other["id"].(string) + _, foreign := ts.do(t, "POST", "/api/v1/projects/"+otherID+"/sections", + map[string]any{"name": "Havearbejde"}) + foreignID := foreign["id"].(string) + + resp, _ = ts.do(t, "POST", "/api/v1/projects/"+id+"/sections/reorder", + map[string]any{"ids": []string{foreignID}}) + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("reorder with a foreign id: status %d", resp.StatusCode) + } + + _, stillThere := ts.do(t, "GET", "/api/v1/projects/"+otherID+"/sections", nil) + elsewhere, _ := stillThere["sections"].([]any) + if len(elsewhere) != 1 || elsewhere[0].(map[string]any)["name"] != "Havearbejde" { + t.Errorf("the other project's section did not survive: %v", elsewhere) + } +} + // A project can be created straight into a group. // // `group_id` was declared on the request type, documented on it, and applied by diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index bab03d2..afb964b 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -426,6 +426,7 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server r.Group(func(r chi.Router) { r.Use(s.requireProject(store.RoleEditor)) r.Post("/sections", s.handleCreateSection) + r.Post("/sections/reorder", s.handleReorderSections) }) r.Group(func(r chi.Router) { diff --git a/internal/store/projects.go b/internal/store/projects.go index bb3ab83..840d771 100644 --- a/internal/store/projects.go +++ b/internal/store/projects.go @@ -646,6 +646,35 @@ func (db *DB) ReorderProjects(ctx context.Context, userID string, ids []string) }) } +// ReorderSections writes the order of one project's sections, top to bottom. +// +// The whole list at once, the same as ReorderProjects and for the same reasons: a +// project has a handful of sections, so writing 0, 1, 2 … is one small +// transaction that cannot run out of precision, and a list is impossible to get +// into the half-ordered state a sequence of individual moves leaves behind when +// one of them fails. +// +// Scoped to the project rather than to a user. A section has no owner of its own — +// it belongs to a project, and whether the caller may rearrange it is a question +// about that project, which the handler has already asked. Ids belonging to some +// other project are skipped by the WHERE clause rather than refused, so a stale +// list from a client that has just moved a section elsewhere cannot rearrange a +// project the caller was never looking at. +func (db *DB) ReorderSections(ctx context.Context, projectID string, ids []string) error { + return db.Tx(ctx, func(tx *sql.Tx) error { + now := time.Now().Unix() + for i, id := range ids { + if _, err := tx.ExecContext(ctx, + `UPDATE sections SET sort_order = ?, updated_at = ? + WHERE id = ? AND project_id = ? AND deleted_at IS NULL`, + float64(i), now, id, projectID); err != nil { + return err + } + } + return nil + }) +} + // SectionByName resolves a "/name" from quick add within one project. // // Case-insensitive, and scoped to the project the task is landing in — a section diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index 58c7d0b..850ad4b 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -1973,6 +1973,76 @@ test('et projekt kan have flere sektioner @forms', async ({ page }) => { expect(trouble).toEqual([]); }); +/** + * Sektioner kan trækkes op og ned. + * + * Rækkefølgen af sektioner *er* planen — "Planlægning" står før "I gang", fordi + * det er den vej arbejdet går — og indtil nu kunne den kun laves om ved at slette + * en sektion og lave den igen, hvilket smider dens opgaver ud i det usektionerede + * område på vejen. + * + * Målt på rækkefølgen af overskrifterne på siden og bagefter på, hvad serveren + * svarer efter en genindlæsning: den første halvdel kan bestå af, at fladen + * flyttede et element, den anden kan kun bestå, hvis skrivningen gik igennem. + */ +test('sektioner kan trækkes i en anden rækkefølge, og den overlever en genindlæsning', async ({ + page +}) => { + const trouble = watchForTrouble(page); + await page.goto('/'); + + const sidebar = page.getByRole('navigation', { name: 'Hovedmenu' }); + await sidebar.getByRole('button', { name: 'Nyt projekt' }).click(); + await sidebar.getByLabel('Projektnavn').fill('Havehus'); + await sidebar.getByLabel('Projektnavn').press('Enter'); + await expect(page.getByRole('heading', { name: 'Havehus' })).toBeVisible(); + + for (const name of ['Fundament', 'Rejsning', 'Tag']) { + await page.getByRole('button', { name: '+ Tilføj sektion' }).click(); + const field = page.getByLabel('Ny sektion'); + await field.fill(name); + await field.press('Enter'); + await expect(page.getByRole('heading', { name, exact: true })).toBeVisible(); + } + + const order = () => + page.locator('.section-head h2').evaluateAll((els) => els.map((e) => e.textContent.trim())); + + expect(await order()).toEqual(['Fundament', 'Rejsning', 'Tag']); + + // Tag trækkes helt op. Sluppet i den øverste halvdel af "Fundament", som er + // gaben over den — hvilken halvdel man rammer, er hele forskellen på over og + // under, så positionen er skrevet ud og ikke overladt til midten. + await page + .locator('.section-head') + .filter({ hasText: 'Tag' }) + .dragTo(page.locator('.section-head').filter({ hasText: 'Fundament' }), { + targetPosition: { x: 40, y: 3 } + }); + + await expect.poll(order).toEqual(['Tag', 'Fundament', 'Rejsning']); + + await page.reload(); + await expect(page.getByRole('heading', { name: 'Tag', exact: true })).toBeVisible(); + expect(await order(), 'serveren tog ikke rækkefølgen').toEqual([ + 'Tag', + 'Fundament', + 'Rejsning' + ]); + + // Og den anden vej: sluppet i den nederste halvdel lander den under. + await page + .locator('.section-head') + .filter({ hasText: 'Tag' }) + .dragTo(page.locator('.section-head').filter({ hasText: 'Rejsning' }), { + targetPosition: { x: 40, y: 28 } + }); + + await expect.poll(order).toEqual(['Fundament', 'Rejsning', 'Tag']); + + expect(trouble).toEqual([]); +}); + /** * Form fields on a touch device are at least 16px. * diff --git a/web/src/lib/api.js b/web/src/lib/api.js index a96cdc2..354611f 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -283,6 +283,10 @@ export const api = { createSection: (projectId, name) => post(`/projects/${projectId}/sections`, { name }), updateSection: (id, data) => patch(`/sections/${id}`, data), deleteSection: (id) => del(`/sections/${id}`), + // The whole list, the same as projects and groups: a project has a handful of + // sections, and an order that cannot land half-applied is worth more than the + // bytes a single move would save. + reorderSections: (projectId, ids) => post(`/projects/${projectId}/sections/reorder`, { ids }), listMembers: (projectId) => get(`/projects/${projectId}/members`), invite: (projectId, email, role) => post(`/projects/${projectId}/invites`, { email, role }), diff --git a/web/src/lib/dnd.js b/web/src/lib/dnd.js index faf5c8b..b8e6fd4 100644 --- a/web/src/lib/dnd.js +++ b/web/src/lib/dnd.js @@ -18,6 +18,7 @@ export const TASK = 'application/x-verdande-task'; export const PROJECT = 'application/x-verdande-project'; export const GROUP = 'application/x-verdande-group'; export const NOTE = 'application/x-verdande-note'; +export const SECTION = 'application/x-verdande-section'; /** Starts a drag carrying one id of the given kind. */ export function startDrag(event, type, id) { diff --git a/web/src/routes/projekt/[id]/+page.svelte b/web/src/routes/projekt/[id]/+page.svelte index f4dfaf0..37d0b7e 100644 --- a/web/src/routes/projekt/[id]/+page.svelte +++ b/web/src/routes/projekt/[id]/+page.svelte @@ -6,7 +6,7 @@ import { app, completedView, projectName } from '$lib/stores.svelte.js'; import { COLORS, colorVar } from '$lib/colors.js'; import { eventName, eventDetail } from '$lib/events.js'; - import { TASK, carries, dragged, accept } from '$lib/dnd.js'; + import { TASK, SECTION, startDrag, carries, dragged, accept } from '$lib/dnd.js'; import TaskList from '$lib/components/TaskList.svelte'; import TaskRow from '$lib/components/TaskRow.svelte'; import QuickAdd from '$lib/components/QuickAdd.svelte'; @@ -386,6 +386,80 @@ } } + // --- moving a section --------------------------------------------------------- + // + // The order of the sections is the order of the plan — "Planlægning" stands + // before "I gang" because that is when the work happens — and until this it + // could only be changed by deleting a section and making it again, which drops + // its tasks into the unsectioned area on the way past. + // + // The whole heading is the handle, not a grip beside it. A section heading + // holds nothing that is itself dragged, so there is no gesture to steal — which + // is the reason the sidebar's rows needed one, and the reason these do not. + + let draggingSection = $state(null); + /** The heading whose gap is lit up, and which side of it. */ + let overSectionEdge = $state(null); + let overSectionBelow = $state(false); + + function clearSectionDrag() { + draggingSection = null; + overSectionEdge = null; + } + + function onSectionDragStart(event, section) { + draggingSection = section.id; + startDrag(event, SECTION, section.id); + } + + function onHeadDragOver(event, section) { + if (!canEdit || !carries(event, SECTION) || draggingSection === section.id) return; + accept(event); + const box = event.currentTarget.getBoundingClientRect(); + overSectionEdge = section.id; + overSectionBelow = event.clientY > box.top + box.height / 2; + } + + /** + * Optimistic, and the whole list at once. + * + * The array is what the page renders from, so moving the entry moves the + * section; `sort_order` is set alongside it only so a later read of this list + * agrees with what is on screen. If the write fails the previous array goes + * back — a half-moved section is worse than one that did not move. + */ + async function onHeadDrop(event, target) { + if (!carries(event, SECTION)) return; + event.preventDefault(); + event.stopPropagation(); + const id = dragged(event, SECTION) || draggingSection; + const below = overSectionBelow; + clearSectionDrag(); + if (!id || id === target.id || !canEdit) return; + + const moved = sections.find((s) => s.id === id); + if (!moved) return; + + const without = sections.filter((s) => s.id !== id); + const at = without.findIndex((s) => s.id === target.id); + if (at < 0) return; + + const ordered = [...without]; + ordered.splice(below ? at + 1 : at, 0, moved); + + const previous = sections; + sections = ordered.map((s, i) => ({ ...s, sort_order: i })); + try { + await api.reorderSections( + project.id, + ordered.map((s) => s.id) + ); + } catch (e) { + sections = previous; + app.toast(humanMessage(e)); + } + } + async function removeSection(section) { const count = open.filter((t) => t.section_id === section.id).length; const warning = count @@ -739,7 +813,22 @@ ondragleave={() => (overSection = null)} ondrop={(e) => onSectionDrop(e, section.id)} > -
+ + +
onSectionDragStart(e, section)} + ondragend={clearSectionDrag} + ondragover={(e) => onHeadDragOver(e, section)} + ondragleave={() => (overSectionEdge = null)} + ondrop={(e) => onHeadDrop(e, section)} + > {#if renamingSection === section.id}
renameSection(e, section)}>