feat(projects): drag the sidebar into the order you want
Projects have carried sort_order since the first migration and nothing ever set it, so the sidebar was ordered by creation date for good. POST /projects/reorder takes the whole list rather than one move. A person has a handful of projects, so sending the order you want is simpler to reason about than midpoints between neighbours — and it cannot run out of precision, which is the failure the task ordering has to respace a section to recover from. It also cannot land half-applied the way a sequence of individual moves can. Projects you do not own are skipped rather than refused. sort_order is a column on the project, not a preference per viewer, so a shared project sits where its owner put it and a member rearranging their own sidebar does not rearrange somebody else's. The sidebar only makes your own projects draggable, so that rule never shows up as a row that springs back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Kristian Wind
co-authored by
Claude Opus 5
parent
3f4af6c0ab
commit
c4e9e1519b
@@ -756,6 +756,35 @@ paths:
|
|||||||
"201": { $ref: "#/components/responses/Project" }
|
"201": { $ref: "#/components/responses/Project" }
|
||||||
"422": { $ref: "#/components/responses/Validation" }
|
"422": { $ref: "#/components/responses/Validation" }
|
||||||
|
|
||||||
|
/api/v1/projects/reorder:
|
||||||
|
post:
|
||||||
|
tags: [projects]
|
||||||
|
summary: Set the order projects appear in.
|
||||||
|
description: |
|
||||||
|
The whole list at once rather than one move at a time. A person has a
|
||||||
|
handful of projects, so sending the order you want is simpler than
|
||||||
|
midpoints and cannot land half-applied.
|
||||||
|
|
||||||
|
Projects you do not own are skipped: `sort_order` is a column on the
|
||||||
|
project rather than a preference per viewer, so a shared one stays where
|
||||||
|
its owner put it.
|
||||||
|
requestBody:
|
||||||
|
required: true
|
||||||
|
content:
|
||||||
|
application/json:
|
||||||
|
schema:
|
||||||
|
type: object
|
||||||
|
required: [ids]
|
||||||
|
properties:
|
||||||
|
ids:
|
||||||
|
type: array
|
||||||
|
maxItems: 500
|
||||||
|
items: { type: string }
|
||||||
|
description: Project ids, top to bottom.
|
||||||
|
responses:
|
||||||
|
"204": { description: Reordered. }
|
||||||
|
"422": { $ref: "#/components/responses/Validation" }
|
||||||
|
|
||||||
/api/v1/projects/{projectID}:
|
/api/v1/projects/{projectID}:
|
||||||
parameters:
|
parameters:
|
||||||
- { name: projectID, in: path, required: true, schema: { type: string } }
|
- { name: projectID, in: path, required: true, schema: { type: string } }
|
||||||
|
|||||||
@@ -69,6 +69,39 @@ type projectRequest struct {
|
|||||||
Archived *bool `json:"archived"`
|
Archived *bool `json:"archived"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type reorderProjectsRequest struct {
|
||||||
|
// IDs in the order they should appear, top to bottom.
|
||||||
|
IDs []string `json:"ids"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// handleReorderProjects sets the sidebar order.
|
||||||
|
//
|
||||||
|
// The whole list rather than one move: a person has a handful of projects, and
|
||||||
|
// sending the order you want is both simpler to reason about and impossible to
|
||||||
|
// get into the half-ordered state a sequence of individual moves can produce if
|
||||||
|
// one of them fails.
|
||||||
|
//
|
||||||
|
// Projects the caller does not own are skipped — sort_order is a column on the
|
||||||
|
// project, so a shared one sits where its owner put it.
|
||||||
|
func (s *Server) handleReorderProjects(w http.ResponseWriter, r *http.Request) {
|
||||||
|
var req reorderProjectsRequest
|
||||||
|
if err := decodeJSON(w, r, &req); err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
// A ceiling rather than a guess at what is reasonable: this is a bound on
|
||||||
|
// the transaction, not an opinion about how many projects somebody may have.
|
||||||
|
if len(req.IDs) > 500 {
|
||||||
|
writeFieldErrors(w, map[string]string{"ids": "at most 500 at a time"})
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := s.db.ReorderProjects(r.Context(), userFrom(r.Context()).ID, req.IDs); err != nil {
|
||||||
|
s.internal(w, "reorder projects", err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
w.WriteHeader(http.StatusNoContent)
|
||||||
|
}
|
||||||
|
|
||||||
func (s *Server) handleCreateProject(w http.ResponseWriter, r *http.Request) {
|
func (s *Server) handleCreateProject(w http.ResponseWriter, r *http.Request) {
|
||||||
var req projectRequest
|
var req projectRequest
|
||||||
if err := decodeJSON(w, r, &req); err != nil {
|
if err := decodeJSON(w, r, &req); err != nil {
|
||||||
|
|||||||
@@ -81,3 +81,75 @@ func TestTheTrashIsPerOwner(t *testing.T) {
|
|||||||
t.Errorf("status %d, want 404", resp.StatusCode)
|
t.Errorf("status %d, want 404", resp.StatusCode)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestReorderProjectsWritesTheGivenOrder(t *testing.T) {
|
||||||
|
ts := newTestServer(t)
|
||||||
|
ts.bootstrap(t)
|
||||||
|
|
||||||
|
var ids []string
|
||||||
|
for _, name := range []string{"Alfa", "Bravo", "Charlie"} {
|
||||||
|
_, body := ts.do(t, "POST", "/api/v1/projects", map[string]string{"name": name})
|
||||||
|
ids = append(ids, body["id"].(string))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Back to front.
|
||||||
|
reversed := []string{ids[2], ids[1], ids[0]}
|
||||||
|
resp, _ := ts.do(t, "POST", "/api/v1/projects/reorder", map[string]any{"ids": reversed})
|
||||||
|
if resp.StatusCode != http.StatusNoContent {
|
||||||
|
t.Fatalf("reorder: status %d", resp.StatusCode)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, list := ts.do(t, "GET", "/api/v1/projects", nil)
|
||||||
|
projects, _ := list["projects"].([]any)
|
||||||
|
|
||||||
|
var got []string
|
||||||
|
for _, raw := range projects {
|
||||||
|
p := raw.(map[string]any)
|
||||||
|
// The Inbox sorts ahead of everything by its own rule; it is not part of
|
||||||
|
// what was reordered.
|
||||||
|
if p["is_inbox"] == true {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
got = append(got, p["id"].(string))
|
||||||
|
}
|
||||||
|
if len(got) != 3 || got[0] != reversed[0] || got[1] != reversed[1] || got[2] != reversed[2] {
|
||||||
|
t.Errorf("order = %v, want %v", got, reversed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// sort_order is a column on the project, not a per-viewer preference, so
|
||||||
|
// reordering must not rearrange a project somebody else owns.
|
||||||
|
func TestReorderProjectsLeavesSomebodyElsesAlone(t *testing.T) {
|
||||||
|
ts := newTestServer(t)
|
||||||
|
ts.bootstrap(t)
|
||||||
|
|
||||||
|
other := ts.newUser(t, "anden@example.dk", "Anden")
|
||||||
|
_, theirs := other.do(t, "POST", "/api/v1/projects", map[string]string{"name": "Deres"})
|
||||||
|
theirID := theirs["id"].(string)
|
||||||
|
|
||||||
|
_, before := other.do(t, "GET", "/api/v1/projects", nil)
|
||||||
|
var wanted float64
|
||||||
|
for _, raw := range before["projects"].([]any) {
|
||||||
|
if p := raw.(map[string]any); p["id"] == theirID {
|
||||||
|
wanted = p["sort_order"].(float64)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Ask, as somebody else, to put their project first.
|
||||||
|
resp, _ := ts.do(t, "POST", "/api/v1/projects/reorder", map[string]any{
|
||||||
|
"ids": []string{theirID},
|
||||||
|
})
|
||||||
|
if resp.StatusCode != http.StatusNoContent {
|
||||||
|
t.Fatalf("reorder: status %d", resp.StatusCode)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, after := other.do(t, "GET", "/api/v1/projects", nil)
|
||||||
|
for _, raw := range after["projects"].([]any) {
|
||||||
|
if p := raw.(map[string]any); p["id"] == theirID {
|
||||||
|
if p["sort_order"] != wanted {
|
||||||
|
t.Errorf("sort_order changed from %v to %v — somebody else moved it",
|
||||||
|
wanted, p["sort_order"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -259,6 +259,8 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server
|
|||||||
r.Route("/projects", func(r chi.Router) {
|
r.Route("/projects", func(r chi.Router) {
|
||||||
r.Get("/", s.handleListProjects)
|
r.Get("/", s.handleListProjects)
|
||||||
r.Post("/", s.handleCreateProject)
|
r.Post("/", s.handleCreateProject)
|
||||||
|
// Above /{projectID}, or chi reads "reorder" as an id.
|
||||||
|
r.Post("/reorder", s.handleReorderProjects)
|
||||||
|
|
||||||
r.Route("/{projectID}", func(r chi.Router) {
|
r.Route("/{projectID}", func(r chi.Router) {
|
||||||
// Read access is the floor for the whole subtree; the writes
|
// Read access is the floor for the whole subtree; the writes
|
||||||
|
|||||||
@@ -556,3 +556,30 @@ func joinComma(parts []string) string {
|
|||||||
}
|
}
|
||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ReorderProjects writes the given order, and only for projects the user owns.
|
||||||
|
//
|
||||||
|
// The whole list at once, rather than the midpoint-between-neighbours dance the
|
||||||
|
// tasks do. A person has a handful of projects, not a thousand, so writing
|
||||||
|
// 0, 1, 2 … is one small transaction that cannot run out of precision — the
|
||||||
|
// failure the task ordering has to respace a section to recover from.
|
||||||
|
//
|
||||||
|
// Scoped to ownership because sort_order lives on the project row rather than
|
||||||
|
// per viewer: a shared project sits where its owner put it, and a member
|
||||||
|
// reordering their sidebar must not rearrange somebody else's. Ids that are not
|
||||||
|
// the caller's are skipped rather than refused, so one shared project in the
|
||||||
|
// list does not fail the whole drag.
|
||||||
|
func (db *DB) ReorderProjects(ctx context.Context, userID 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 projects SET sort_order = ?, updated_at = ?
|
||||||
|
WHERE id = ? AND owner_id = ? AND deleted_at IS NULL`,
|
||||||
|
float64(i), now, id, userID); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
@@ -176,6 +176,7 @@ export const api = {
|
|||||||
createProject: (data) => post('/projects', data),
|
createProject: (data) => post('/projects', data),
|
||||||
updateProject: (id, data) => patch(`/projects/${id}`, data),
|
updateProject: (id, data) => patch(`/projects/${id}`, data),
|
||||||
deleteProject: (id) => del(`/projects/${id}`),
|
deleteProject: (id) => del(`/projects/${id}`),
|
||||||
|
reorderProjects: (ids) => post('/projects/reorder', { ids }),
|
||||||
listTrashedProjects: () => get('/trash/projects'),
|
listTrashedProjects: () => get('/trash/projects'),
|
||||||
restoreProject: (id) => post(`/trash/projects/${id}/restore`),
|
restoreProject: (id) => post(`/trash/projects/${id}/restore`),
|
||||||
|
|
||||||
|
|||||||
@@ -33,8 +33,54 @@
|
|||||||
});
|
});
|
||||||
|
|
||||||
let shared = $derived(app.projects.filter((p) => !p.is_inbox && p.shared));
|
let shared = $derived(app.projects.filter((p) => !p.is_inbox && p.shared));
|
||||||
let own = $derived(app.projects.filter((p) => !p.is_inbox && !p.shared));
|
// Sorted here rather than relying on the order the server sent, so a drag
|
||||||
|
// settles the moment it is dropped instead of after the round trip.
|
||||||
|
let own = $derived(
|
||||||
|
app.projects
|
||||||
|
.filter((p) => !p.is_inbox && !p.shared)
|
||||||
|
.sort((a, b) => a.sort_order - b.sort_order)
|
||||||
|
);
|
||||||
let current = $derived($page.url.pathname);
|
let current = $derived($page.url.pathname);
|
||||||
|
|
||||||
|
// --- reordering ---------------------------------------------------------------
|
||||||
|
|
||||||
|
let draggingId = $state(null);
|
||||||
|
let overId = $state(null);
|
||||||
|
let overBelow = $state(false);
|
||||||
|
|
||||||
|
function onDragStart(event, project) {
|
||||||
|
draggingId = project.id;
|
||||||
|
event.dataTransfer.effectAllowed = 'move';
|
||||||
|
// Firefox refuses to start a drag unless something is set on the transfer.
|
||||||
|
event.dataTransfer.setData('text/plain', project.id);
|
||||||
|
}
|
||||||
|
|
||||||
|
function onDragOver(event, project) {
|
||||||
|
if (!draggingId || draggingId === project.id) return;
|
||||||
|
event.preventDefault();
|
||||||
|
event.dataTransfer.dropEffect = 'move';
|
||||||
|
|
||||||
|
const box = event.currentTarget.getBoundingClientRect();
|
||||||
|
overId = project.id;
|
||||||
|
overBelow = event.clientY > box.top + box.height / 2;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function onDrop(event, target) {
|
||||||
|
event.preventDefault();
|
||||||
|
const id = draggingId;
|
||||||
|
const below = overBelow;
|
||||||
|
draggingId = null;
|
||||||
|
overId = null;
|
||||||
|
if (!id || id === target.id) return;
|
||||||
|
|
||||||
|
const without = own.filter((p) => p.id !== id);
|
||||||
|
const at = without.findIndex((p) => p.id === target.id);
|
||||||
|
if (at < 0) return;
|
||||||
|
|
||||||
|
const ordered = [...without];
|
||||||
|
ordered.splice(below ? at + 1 : at, 0, own.find((p) => p.id === id));
|
||||||
|
await app.reorderProjects(ordered.map((p) => p.id));
|
||||||
|
}
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
<nav class="sidebar" aria-label="Hovedmenu">
|
<nav class="sidebar" aria-label="Hovedmenu">
|
||||||
@@ -90,8 +136,21 @@
|
|||||||
{#each own as project (project.id)}
|
{#each own as project (project.id)}
|
||||||
<a
|
<a
|
||||||
href="/projekt/{project.id}"
|
href="/projekt/{project.id}"
|
||||||
|
class="sortable"
|
||||||
class:active={current === `/projekt/${project.id}`}
|
class:active={current === `/projekt/${project.id}`}
|
||||||
|
class:dragging={draggingId === project.id}
|
||||||
|
class:drop-above={overId === project.id && !overBelow}
|
||||||
|
class:drop-below={overId === project.id && overBelow}
|
||||||
onclick={onnavigate}
|
onclick={onnavigate}
|
||||||
|
draggable="true"
|
||||||
|
ondragstart={(e) => onDragStart(e, project)}
|
||||||
|
ondragend={() => {
|
||||||
|
draggingId = null;
|
||||||
|
overId = null;
|
||||||
|
}}
|
||||||
|
ondragover={(e) => onDragOver(e, project)}
|
||||||
|
ondragleave={() => (overId = null)}
|
||||||
|
ondrop={(e) => onDrop(e, project)}
|
||||||
>
|
>
|
||||||
<span class="dot" aria-hidden="true"></span>
|
<span class="dot" aria-hidden="true"></span>
|
||||||
{project.name}
|
{project.name}
|
||||||
@@ -280,6 +339,39 @@
|
|||||||
font-weight: 500;
|
font-weight: 500;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Only your own projects reorder. A shared one sits where its owner put it —
|
||||||
|
sort_order is a column on the project, not a preference per viewer. */
|
||||||
|
a.sortable {
|
||||||
|
position: relative;
|
||||||
|
}
|
||||||
|
|
||||||
|
a.sortable.dragging {
|
||||||
|
opacity: 0.4;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* A line in the gap rather than a highlighted row: the gap is the target. */
|
||||||
|
a.sortable::before {
|
||||||
|
content: '';
|
||||||
|
position: absolute;
|
||||||
|
left: var(--s2);
|
||||||
|
right: var(--s2);
|
||||||
|
height: 2px;
|
||||||
|
background: var(--accent);
|
||||||
|
opacity: 0;
|
||||||
|
pointer-events: none;
|
||||||
|
transition: opacity var(--fast) var(--ease);
|
||||||
|
}
|
||||||
|
|
||||||
|
a.sortable.drop-above::before {
|
||||||
|
top: -1px;
|
||||||
|
opacity: 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
a.sortable.drop-below::before {
|
||||||
|
bottom: -1px;
|
||||||
|
opacity: 1;
|
||||||
|
}
|
||||||
|
|
||||||
.dot {
|
.dot {
|
||||||
width: 6px;
|
width: 6px;
|
||||||
height: 6px;
|
height: 6px;
|
||||||
|
|||||||
@@ -246,6 +246,29 @@ class AppState {
|
|||||||
this.projects = projects;
|
this.projects = projects;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Puts the projects in the given order.
|
||||||
|
*
|
||||||
|
* The whole list rather than one moved item: it is a handful of rows, and
|
||||||
|
* sending the order you want cannot land in the half-applied state a sequence
|
||||||
|
* of individual moves can. `sort_order` is set here as well as on the server,
|
||||||
|
* so the sidebar settles immediately rather than after a round trip.
|
||||||
|
*/
|
||||||
|
async reorderProjects(ids) {
|
||||||
|
const previous = this.projects;
|
||||||
|
const rank = new Map(ids.map((id, i) => [id, i]));
|
||||||
|
|
||||||
|
this.projects = this.projects.map((p) =>
|
||||||
|
rank.has(p.id) ? { ...p, sort_order: rank.get(p.id) } : p
|
||||||
|
);
|
||||||
|
try {
|
||||||
|
await api.reorderProjects(ids);
|
||||||
|
} catch (e) {
|
||||||
|
this.projects = previous;
|
||||||
|
this.toast(humanMessage(e));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- toasts -------------------------------------------------------------------
|
// --- toasts -------------------------------------------------------------------
|
||||||
|
|
||||||
toast(message) {
|
toast(message) {
|
||||||
|
|||||||
Reference in New Issue
Block a user