diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index dfc94bf..e36ed3d 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -883,14 +883,19 @@ type taskRow struct { Created string Resolved string ResolvedBy string + // DueValue and Weight are the raw values the edit form posts back + // (Vikunja #509). Due above is for reading and says "—" for no date; a + // date input needs "2026-08-07" or the empty string. + DueValue string + Weight int // Why — the ranker's reason for this row's position (Vikunja #129), in // Russian, empty when nothing distinguished the task. Blank is the honest // rendering: he never said this one mattered more. Why string } -// handleTasks serves the task review surface (GET) and the four writes it -// offers (POST): add, confirm, done, drop. +// handleTasks serves the task review surface (GET) and the five writes it +// offers (POST): add, edit, confirm, done, drop. // // Not step-up gated, unlike /tools and /routines, and the difference is the // point: enabling a tool defines argv Maven will execute, and accepting a @@ -900,6 +905,13 @@ type taskRow struct { // still sits behind whatever transport auth fronts mavweb, like every other // page. // +// "edit" was re-argued on the same terms rather than inheriting the exemption +// (Vikunja #509), and it stays ungated. It rewrites a line on a list he reads +// himself, the same blast radius "drop" already has on this page, and the store +// refuses the two edits that would cost something: a resolved task keeps the +// text it was finished under, and a text collision with another live row is +// named instead of merged. +// // "confirm" is the only interesting move: it promotes a candidate Maven derived // from something she read into work he owns. That review step is why derived // tasks are captured as candidates in the first place. @@ -965,6 +977,7 @@ func handleTasks(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { ID: t.ID, Text: t.Text, Source: t.Source, Evidence: t.Evidence, Status: t.Status, Created: fmtTaskTime(&t.CreatedTs), Due: fmtTaskDate(t.Due), Resolved: fmtTaskTime(t.Resolved), + DueValue: fmtTaskDateValue(t.Due), Weight: t.Weight, Why: r.Reason, } if t.Status == "candidate" { @@ -1000,27 +1013,16 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri return "", errors.New("empty task text") } req := ipc.CaptureTaskReq{Text: text, Source: "tap:web", Status: "open", Ts: now()} - // Importance is his, stated on the form. Out-of-range values are - // clamped rather than rejected — a bad select is not worth a 400. - if v := r.FormValue("weight"); v != "" { - // strconv, not Sscanf: Sscanf("3junk", "%d") succeeds with 3, and a - // form value is not a place to accept trailing garbage. - wgt, err := strconv.Atoi(v) - if err != nil || wgt < 0 { - return "", fmt.Errorf("bad weight %q", v) - } - if wgt > tasks.MaxWeight { - wgt = tasks.MaxWeight - } - req.Weight = wgt + wgt, err := formWeight(r) + if err != nil { + return "", err } - if d := r.FormValue("due"); d != "" { - due, err := time.ParseInLocation("2006-01-02", d, now().Location()) - if err != nil { - return "", fmt.Errorf("bad due date %q", d) - } - req.Due = &due + req.Weight = wgt + due, err := formDue(r, now()) + if err != nil { + return "", err } + req.Due = due resp, err := core.CaptureTask(ctx, req) if err != nil { return "", err @@ -1038,6 +1040,36 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri if err != nil { return "", errors.New("invalid id") } + + if action == "edit" { + // The three fields capture set, and only those (Vikunja #509). Status + // is not editable here: that ladder is one-way and has its own buttons. + text := strings.TrimSpace(r.FormValue("text")) + if text == "" { + return "", errors.New("empty task text") + } + wgt, err := formWeight(r) + if err != nil { + return "", err + } + due, err := formDue(r, now()) + if err != nil { + return "", err + } + switch err := core.EditTask(ctx, id, text, due, wgt); { + case err == nil: + return "saved task", nil + case errors.Is(err, ipc.ErrTaskDuplicate): + // Naming the collision instead of merging: two live rows carry two + // provenances, and picking one is not the page's call. + return "", errors.New("another open task already says this — drop one of the two") + case errors.Is(err, ipc.ErrTaskResolved): + return "", errors.New("a resolved task keeps the text it was finished under") + default: + return "", err + } + } + var status, msg string switch action { case "confirm": @@ -1061,6 +1093,47 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri return msg, nil } +// fmtTaskDateValue renders a due date the way requires, or +// "" for no date. Separate from fmtTaskDate, which renders it for reading. +// formWeight reads the importance select. Out-of-range clamps rather than +// rejects — a bad select is not worth a 400 — but trailing garbage is refused, +// because strconv is not Sscanf and "3junk" is not a 3. +func formWeight(r *http.Request) (int, error) { + v := r.FormValue("weight") + if v == "" { + return 0, nil + } + wgt, err := strconv.Atoi(v) + if err != nil || wgt < 0 { + return 0, fmt.Errorf("bad weight %q", v) + } + if wgt > tasks.MaxWeight { + wgt = tasks.MaxWeight + } + return wgt, nil +} + +// formDue reads the date input. An empty field is nil, which on an edit means +// "clear the date" — the form has no other way to say it. +func formDue(r *http.Request, now time.Time) (*time.Time, error) { + d := r.FormValue("due") + if d == "" { + return nil, nil + } + due, err := time.ParseInLocation("2006-01-02", d, now.Location()) + if err != nil { + return nil, fmt.Errorf("bad due date %q", d) + } + return &due, nil +} + +func fmtTaskDateValue(t *time.Time) string { + if t == nil || t.IsZero() { + return "" + } + return t.Local().Format("2006-01-02") +} + func fmtTaskTime(t *time.Time) string { if t == nil || t.IsZero() { return "—" diff --git a/cmd/mavweb/tasks.html b/cmd/mavweb/tasks.html index e6daba3..583a486 100644 --- a/cmd/mavweb/tasks.html +++ b/cmd/mavweb/tasks.html @@ -60,12 +60,27 @@

open {{len .Open}}

most pressing first — by the deadlines and the urgency you gave. nothing about a task is guessed; the only signal that is not yours is age, which lifts anything sitting here for weeks.
{{if .Open}}
- + {{range .Open}} - + + -
taskwhyfromduecaptured
taskwhyfromcaptured
{{.Text}}
+ + + + + +
{{.Why}} {{.Source}}{{.Due}} {{.Created}}
diff --git a/internal/ipc/api.go b/internal/ipc/api.go index d2e597e..4cfa642 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -549,6 +549,16 @@ type setTaskStatusReq struct { By string `json:"by,omitempty"` } +// editTaskReq — the rewrite of the three fields capture set (Vikunja #509). +// Due nil clears the date, so "no date given" and "remove the date" cannot be +// the same request. +type editTaskReq struct { + ID int64 `json:"id"` + Text string `json:"text"` + Due *time.Time `json:"due,omitempty"` + Weight int `json:"weight,omitempty"` +} + // setTaskFieldsReq — the write for the two board columns. Both are sent every // time and both may be empty: clearing a blocker is as ordinary as setting one, // so an omitted field cannot mean "leave it alone" without a second way to say @@ -872,6 +882,13 @@ var ErrToolNotFound = errors.New("ipc: tool not found") // form can say which refusal it hit rather than "не найдено". var ErrTaskNoDoneWhen = errors.New("ipc: task has no definition of done") +// ErrTaskDuplicate — an edit would collide with another live task's normalised +// text (Vikunja #509). The surface says which row holds it rather than merging. +var ErrTaskDuplicate = errors.New("ipc: another live task already has this text") + +// ErrTaskResolved — a resolved task is not editable. +var ErrTaskResolved = errors.New("ipc: task is resolved") + // callerKey — context key for the authenticated caller. Server sets it from // SO_PEERCRED before dispatch; in-process callers omit it (the adapter treats // a missing Caller as "trusted same-process", the equivalent of the socket's diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 591a382..f917d01 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -515,6 +515,10 @@ func (c *Client) SetTaskStatus(ctx context.Context, id int64, status string, ts return c.call(ctx, MethodSetTaskStatus, setTaskStatusReq{ID: id, Status: status, Ts: ts, By: by}, nil) } +func (c *Client) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + return c.call(ctx, MethodEditTask, editTaskReq{ID: id, Text: text, Due: due, Weight: weight}, nil) +} + func (c *Client) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error { return c.call(ctx, MethodSetTaskFields, setTaskFieldsReq{ID: id, DoneWhen: doneWhen, BlockedOn: blockedOn}, nil) } diff --git a/internal/ipc/coreapi.go b/internal/ipc/coreapi.go index b4aa538..bec310a 100644 --- a/internal/ipc/coreapi.go +++ b/internal/ipc/coreapi.go @@ -142,6 +142,11 @@ type TaskAPI interface { // SetTaskStatus moves a task forward once: candidate→open|dropped, // open→done|dropped. Any other move is refused. SetTaskStatus(ctx context.Context, id int64, status string, ts time.Time, by string) error + // EditTask rewrites the three fields capture set: text, due date and + // weight. Status is not among them — that ladder is one-way and belongs to + // SetTaskStatus. A resolved task is refused, and a text edit that would + // duplicate another live task is refused rather than merged. + EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error // SetTaskFields writes the definition of done and the blocker. Not a // status move, so it is not one-way: he may sharpen a criterion, and a // blocker clears when the person answers. blockedOn is a canonical Nexus diff --git a/internal/ipc/maperr_test.go b/internal/ipc/maperr_test.go index 4857368..7bdc4b3 100644 --- a/internal/ipc/maperr_test.go +++ b/internal/ipc/maperr_test.go @@ -32,6 +32,8 @@ var mapErrPairs = []struct { {"ErrReminderState", store.ErrReminderState, ErrReminderState}, {"ErrToolNotFound", store.ErrToolNotFound, ErrToolNotFound}, {"ErrTaskNoDoneWhen", store.ErrTaskNoDoneWhen, ErrTaskNoDoneWhen}, + {"ErrTaskDuplicate", store.ErrTaskDuplicate, ErrTaskDuplicate}, + {"ErrTaskResolved", store.ErrTaskResolved, ErrTaskResolved}, } // unmappedStoreErrors — store sentinels that deliberately have no wire twin, diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 1e5b8fc..f68edb0 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -540,6 +540,9 @@ var methodTable = map[Method]handlerFunc{ MethodSetTaskStatus: withParamsVoid(func(ctx context.Context, api CoreAPI, p setTaskStatusReq) error { return api.SetTaskStatus(ctx, p.ID, p.Status, p.Ts, p.By) }), + MethodEditTask: withParamsVoid(func(ctx context.Context, api CoreAPI, p editTaskReq) error { + return api.EditTask(ctx, p.ID, p.Text, p.Due, p.Weight) + }), MethodSetTaskFields: withParamsVoid(func(ctx context.Context, api CoreAPI, p setTaskFieldsReq) error { return api.SetTaskFields(ctx, p.ID, p.DoneWhen, p.BlockedOn) }), diff --git a/internal/ipc/storeapi.go b/internal/ipc/storeapi.go index 656efb3..3fb2a30 100644 --- a/internal/ipc/storeapi.go +++ b/internal/ipc/storeapi.go @@ -356,6 +356,10 @@ func (a *storeAPI) SetTaskStatus(ctx context.Context, id int64, status string, t return mapErr(a.s.SetTaskStatus(ctx, id, status, ts, by)) } +func (a *storeAPI) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + return mapErr(a.s.EditTask(ctx, id, text, due, weight)) +} + func (a *storeAPI) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error { return mapErr(a.s.SetTaskFields(ctx, id, doneWhen, blockedOn)) } @@ -468,6 +472,10 @@ func mapErr(err error) error { return ErrToolNotFound case errors.Is(err, store.ErrTaskNoDoneWhen): return ErrTaskNoDoneWhen + case errors.Is(err, store.ErrTaskDuplicate): + return ErrTaskDuplicate + case errors.Is(err, store.ErrTaskResolved): + return ErrTaskResolved } return err } diff --git a/internal/ipc/unimplemented.go b/internal/ipc/unimplemented.go index 86b83a2..63ce042 100644 --- a/internal/ipc/unimplemented.go +++ b/internal/ipc/unimplemented.go @@ -114,6 +114,9 @@ func (UnimplementedCoreAPI) ListTasks(ctx context.Context, status string) ([]Tas func (UnimplementedCoreAPI) SetTaskStatus(ctx context.Context, id int64, status string, ts time.Time, by string) error { return ErrNotImplemented } +func (UnimplementedCoreAPI) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + return ErrNotImplemented +} func (UnimplementedCoreAPI) SetTaskFields(ctx context.Context, id int64, doneWhen, blockedOn string) error { return ErrNotImplemented } diff --git a/internal/ipc/wire.go b/internal/ipc/wire.go index 68ee161..d19888b 100644 --- a/internal/ipc/wire.go +++ b/internal/ipc/wire.go @@ -56,6 +56,7 @@ const ( MethodListTasks Method = "list_tasks" MethodSetTaskStatus Method = "set_task_status" MethodSetTaskFields Method = "set_task_fields" + MethodEditTask Method = "edit_task" MethodIngestMail Method = "ingest_mail" MethodSwapModel Method = "swap_model" MethodModelStatus Method = "model_status" diff --git a/internal/store/tasks.go b/internal/store/tasks.go index 16f4f9d..8e37258 100644 --- a/internal/store/tasks.go +++ b/internal/store/tasks.go @@ -98,6 +98,16 @@ var ( // finish line nobody wrote is how a board fills with rows that can never // leave it. Dropping such a candidate stays legal. ErrTaskNoDoneWhen = errors.New("store: task has no definition of done") + // ErrTaskDuplicate — an edit would give this task the normalised text of + // another live row (Vikunja #509). A refusal, not a merge: two live rows + // carry two provenances, two capture times and possibly two external + // identities, and merging picks a winner for all three silently. The + // surface tells the owner which row already holds the text and lets him + // drop one. + ErrTaskDuplicate = errors.New("store: another live task already has this text") + // ErrTaskResolved — a resolved task is not editable. Its text is the + // record of what was finished, and rewriting it rewrites history. + ErrTaskResolved = errors.New("store: task is resolved") ) // liveTaskStatuses — the two statuses that count as outstanding work. @@ -381,6 +391,50 @@ func (s *Store) setTaskStatus(ctx context.Context, id int64, status string, ts t return nil } +// EditTask rewrites the three fields capture set and nothing else: text, due +// date and weight (Vikunja #509). Status stays the one-way ladder SetTaskStatus +// owns, and a resolved task is refused outright — its text is the record of +// what was finished. +// +// Editing text re-normalises the dedupe key, which can collide with another +// live row. That is ErrTaskDuplicate and it is a refusal: merging would pick +// one row's provenance, capture time and external identity over the other's +// with nobody asked. +// +// due nil clears the date. Clearing has to be sayable, so an absent date and +// "remove the date" cannot be the same argument. +func (s *Store) EditTask(ctx context.Context, id int64, text string, due *time.Time, weight int) error { + text = strings.TrimSpace(text) + if text == "" { + return ErrTaskEmpty + } + cur, err := s.LookupTask(ctx, id) + if err != nil { + return err + } + if cur.Status != TaskCandidate && cur.Status != TaskOpen { + return fmt.Errorf("%w: id=%d is %s", ErrTaskResolved, id, cur.Status) + } + norm := NormalizeTaskText(text) + if norm != NormalizeTaskText(cur.Text) { + if other, err := s.lookupLiveTaskByNorm(ctx, norm); err == nil && other.ID != id { + return fmt.Errorf("%w: id=%d holds it", ErrTaskDuplicate, other.ID) + } else if err != nil && !errors.Is(err, ErrTaskNotFound) { + return err + } + } + var dueVal sql.NullInt64 + if due != nil { + dueVal = sql.NullInt64{Int64: due.UnixMilli(), Valid: true} + } + if _, err := s.db.ExecContext(ctx, + `UPDATE tasks SET text = ?, norm = ?, due_ts = ?, weight = ? WHERE id = ?`, + text, norm, dueVal, weight, id); err != nil { + return fmt.Errorf("edit task: %w", err) + } + return nil +} + // SetTaskFields writes the two board columns. Separate from SetTaskStatus // because a status move is one-way and these are not: he may sharpen a // definition of done, and a blocker clears when the person answers. diff --git a/internal/store/tasks_test.go b/internal/store/tasks_test.go index e525b01..1128924 100644 --- a/internal/store/tasks_test.go +++ b/internal/store/tasks_test.go @@ -465,3 +465,80 @@ func TestPromotingACandidateNeedsADefinitionOfDone(t *testing.T) { t.Errorf("status = %q, want open", got.Status) } } + +func TestEditTaskRewritesTheCaptureFields(t *testing.T) { + ctx := context.Background() + st := newTestStore(t) + now := time.Date(2026, 8, 5, 9, 0, 0, 0, time.UTC) + due := now.Add(48 * time.Hour) + + res, err := st.CaptureTask(ctx, Task{Text: "купить малако", Source: "tap:voice", CreatedTs: now, Weight: 1}) + if err != nil { + t.Fatal(err) + } + if err := st.EditTask(ctx, res.ID, " купить молоко ", &due, 3); err != nil { + t.Fatal(err) + } + got, err := st.LookupTask(ctx, res.ID) + if err != nil { + t.Fatal(err) + } + if got.Text != "купить молоко" || got.Weight != 3 || got.Due == nil || !got.Due.Equal(due) { + t.Fatalf("task = %+v, want the dictation typo fixed with the date and weight", got) + } + // Clearing the date has to be sayable, or an absent date and "remove the + // date" would be one argument. + if err := st.EditTask(ctx, res.ID, "купить молоко", nil, 3); err != nil { + t.Fatal(err) + } + if got, err = st.LookupTask(ctx, res.ID); err != nil { + t.Fatal(err) + } else if got.Due != nil { + t.Errorf("due = %v, want it cleared", got.Due) + } + // The dedupe key moved with the text: capturing the old wording is new work. + again, err := st.CaptureTask(ctx, Task{Text: "купить малако", Source: "tap:voice", CreatedTs: now}) + if err != nil { + t.Fatal(err) + } + if !again.Created { + t.Error("the old normalised text must be free after the edit") + } + if err := st.EditTask(ctx, res.ID, "", nil, 0); !errors.Is(err, ErrTaskEmpty) { + t.Errorf("empty text = %v, want ErrTaskEmpty", err) + } +} + +func TestEditTaskRefusesACollisionAndAResolvedRow(t *testing.T) { + ctx := context.Background() + st := newTestStore(t) + now := time.Date(2026, 8, 5, 9, 0, 0, 0, time.UTC) + + first, err := st.CaptureTask(ctx, Task{Text: "купить молоко", Source: "tap:voice", CreatedTs: now}) + if err != nil { + t.Fatal(err) + } + second, err := st.CaptureTask(ctx, Task{Text: "оплатить интернет", Source: "tap:voice", CreatedTs: now}) + if err != nil { + t.Fatal(err) + } + // Two live rows carry two provenances and two capture times, so a merge + // would pick a winner for both with nobody asked. + if err := st.EditTask(ctx, second.ID, "Купить молоко!", nil, 0); !errors.Is(err, ErrTaskDuplicate) { + t.Errorf("collision = %v, want ErrTaskDuplicate", err) + } + // Editing a row to the text it already has is not a collision with itself. + if err := st.EditTask(ctx, first.ID, "купить молоко", nil, 2); err != nil { + t.Errorf("re-saving the same text: %v", err) + } + if err := st.SetTaskStatus(ctx, first.ID, TaskDone, now.Add(time.Hour), "tap:web"); err != nil { + t.Fatal(err) + } + // A resolved task's text is the record of what was finished. + if err := st.EditTask(ctx, first.ID, "купить кефир", nil, 0); !errors.Is(err, ErrTaskResolved) { + t.Errorf("editing a resolved task = %v, want ErrTaskResolved", err) + } + if err := st.EditTask(ctx, 9999, "что-то", nil, 0); !errors.Is(err, ErrTaskNotFound) { + t.Errorf("editing a missing row = %v, want ErrTaskNotFound", err) + } +}