tasks: compare due dates in the caller's day, not in UTC

dayDelta truncated both instants to a UTC day. A task due at 02:00 Moscow time
tonight read as due tomorrow, and one due at 23:00 last night read as due
today, so the two classes that decide the whole order were assigned from the
wrong calendar. Both sides are now truncated in now's location. Dated work also
lost to age alone because the later-due score sat below the age cap, and the
tail said "и ещё 3" with no noun and no Russian plural agreement.

The page hardcoded time.Now, so none of this was testable from a fixed clock.
It now takes an injectable clock, parses the due date in that clock's location,
parses ids and weights with strconv instead of a hand-rolled scan, caps the
resolved table and says so, shows who resolved each row, and reports a
promotion as the confirmation it is.

Found in review of #61.
This commit is contained in:
kami
2026-08-01 14:16:56 +04:00
parent 708a69375f
commit 88d25d31ac
5 changed files with 287 additions and 38 deletions
+49 -21
View File
@@ -18,6 +18,7 @@ import (
"net/url"
"os"
"os/signal"
"strconv"
"strings"
"time"
@@ -865,17 +866,29 @@ func handleReminders(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) {
}
}
// now — the wall clock, indirected so the task page can be rendered at a fixed
// instant in a test. internal/tasks is pure and the daemon path already ranks
// through a clock it is handed; the page had no reason to be the one surface
// that could only be tested at whatever time it happened to run.
var now = time.Now
// resolvedShown — how many finished tasks the page renders. The list is
// history, it only grows, and the rows below the first screen are read by
// nobody.
const resolvedShown = 50
// taskRow is one line on /tasks, with every timestamp already formatted so the
// template holds no date logic.
type taskRow struct {
ID int64
Text string
Source string
Evidence string
Status string
Due string
Created string
Resolved string
ID int64
Text string
Source string
Evidence string
Status string
Due string
Created string
Resolved string
ResolvedBy string
// 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.
@@ -923,6 +936,7 @@ func handleTasks(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) {
// rows keep store order (newest first) — ranking finished work is pointless.
var live []tasks.Item
var resolved []taskRow
resolvedTotal := 0
for _, t := range all {
switch t.Status {
case "candidate", "open":
@@ -931,10 +945,18 @@ func handleTasks(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) {
Created: t.CreatedTs, Due: t.Due, Weight: t.Weight,
})
default:
resolvedTotal++
// Finished work is history, and the history only grows. The page
// showed every row that ever existed, which is a page that gets
// slower every month for a section nobody reads past the top of.
if len(resolved) >= resolvedShown {
continue
}
resolved = append(resolved, taskRow{
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),
ResolvedBy: t.ResolvedBy,
})
}
}
@@ -943,7 +965,7 @@ func handleTasks(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) {
byID[t.ID] = t
}
var cands, open []taskRow
for _, r := range tasks.Rank(live, time.Now()) {
for _, r := range tasks.Rank(live, now()) {
t := byID[r.ID]
row := taskRow{
ID: t.ID, Text: t.Text, Source: t.Source, Evidence: t.Evidence,
@@ -962,11 +984,12 @@ func handleTasks(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) {
}
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if err := tasksTmpl.Execute(w, struct {
Msg, Err string
Candidates []taskRow
Open []taskRow
Resolved []taskRow
}{msg, errMsg, cands, open, resolved}); err != nil {
Msg, Err string
Candidates []taskRow
Open []taskRow
Resolved []taskRow
ResolvedMore bool
}{msg, errMsg, cands, open, resolved, resolvedTotal > len(resolved)}); err != nil {
log.Printf("tasks render: %v", err)
}
}
@@ -981,12 +1004,14 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri
if text == "" {
return "", errors.New("empty task text")
}
req := ipc.CaptureTaskReq{Text: text, Source: "tap:web", Status: "open", Ts: time.Now()}
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 != "" {
var wgt int
if n, _ := fmt.Sscanf(v, "%d", &wgt); n != 1 || wgt < 0 {
// 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 {
@@ -995,7 +1020,7 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri
req.Weight = wgt
}
if d := r.FormValue("due"); d != "" {
due, err := time.ParseInLocation("2006-01-02", d, time.Local)
due, err := time.ParseInLocation("2006-01-02", d, now().Location())
if err != nil {
return "", fmt.Errorf("bad due date %q", d)
}
@@ -1005,14 +1030,17 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri
if err != nil {
return "", err
}
if resp.Promoted {
return "confirmed a candidate maven had found", nil
}
if !resp.Created {
return "already on the list", nil
}
return "added task", nil
}
var id int64
if n, _ := fmt.Sscanf(r.FormValue("id"), "%d", &id); n != 1 {
id, err := strconv.ParseInt(r.FormValue("id"), 10, 64)
if err != nil {
return "", errors.New("invalid id")
}
var status, msg string
@@ -1026,7 +1054,7 @@ func applyTaskPost(ctx context.Context, core ipc.CoreAPI, r *http.Request) (stri
default:
return "", fmt.Errorf("unknown action %q", action)
}
if err := core.SetTaskStatus(ctx, id, status, time.Now()); err != nil {
if err := core.SetTaskStatus(ctx, id, status, now(), "tap:web"); err != nil {
return "", err
}
return msg, nil
+7 -2
View File
@@ -9,6 +9,9 @@
<input type=hidden name=action value=add>
<input type=text name=text placeholder="что нужно сделать" size=44 required>
<input type=date name=due title="due date (optional)">
<!-- weight 1 is skipped on purpose: the two rungs here are the two words she
recognises out loud ("важно", "срочно"), so the form and the spoken markers
mean the same thing. -->
<select name=weight title="importance (optional)">
<option value=0>normal</option>
<option value=2>важно</option>
@@ -43,7 +46,7 @@
<section class=card>
<h2 class=card-title>open <span class=badge>{{len .Open}}</span></h2>
<div class=hint>most pressing first — by the deadlines and the urgency you gave, nothing guessed.</div>
<div class=hint>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.</div>
{{if .Open}}<div class=scroll><table>
<tr><th>task</th><th>why</th><th>from</th><th>due</th><th>captured</th><th></th><th></th></tr>
{{range .Open}}<tr>
@@ -72,12 +75,14 @@
<section class=card>
<h2 class=card-title>resolved <span class=badge>{{len .Resolved}}</span></h2>
<div class=scroll><table>
<tr><th>task</th><th>status</th><th>when</th></tr>
<tr><th>task</th><th>status</th><th>when</th><th>by</th></tr>
{{range .Resolved}}<tr>
<td class=text-max>{{.Text}}</td>
<td><span class="badge {{.Status}}">{{.Status}}</span></td>
<td class=muted>{{.Resolved}}</td>
<td class=hint>{{.ResolvedBy}}</td>
</tr>{{end}}</table></div>
{{if .ResolvedMore}}<div class=hint>only the {{len .Resolved}} most recent are shown.</div>{{end}}
</section>
{{end}}
{{template "shellBottom"}}
+119 -3
View File
@@ -2,6 +2,7 @@ package main
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
@@ -27,7 +28,10 @@ type fakeTaskCore struct {
statusID int64
statusVal string
statusBy string
statusErr error
promoted bool
}
func (f *fakeTaskCore) ListTasks(_ context.Context, status string) ([]ipc.Task, error) {
@@ -42,11 +46,11 @@ func (f *fakeTaskCore) CaptureTask(_ context.Context, req ipc.CaptureTaskReq) (i
if f.captureErr != nil {
return ipc.CaptureTaskResp{}, f.captureErr
}
return ipc.CaptureTaskResp{ID: 7, Created: f.created}, nil
return ipc.CaptureTaskResp{ID: 7, Created: f.created, Promoted: f.promoted}, nil
}
func (f *fakeTaskCore) SetTaskStatus(_ context.Context, id int64, status string, _ time.Time) error {
f.statusID, f.statusVal = id, status
func (f *fakeTaskCore) SetTaskStatus(_ context.Context, id int64, status string, _ time.Time, by string) error {
f.statusID, f.statusVal, f.statusBy = id, status, by
return f.statusErr
}
@@ -223,3 +227,115 @@ func TestApplyTaskPostClampsWeight(t *testing.T) {
t.Errorf("weight = %d, want the cap", core.captured[0].Weight)
}
}
// The page ranked with the wall clock while the daemon path ranked with a clock
// it was handed, so this was the one surface that could only be tested at
// whatever time it happened to run.
func TestHandleTasksRanksAtTheInjectedClock(t *testing.T) {
fixed := time.Date(2026, 8, 1, 10, 0, 0, 0, time.FixedZone("UTC+4", 4*3600))
old := now
now = func() time.Time { return fixed }
t.Cleanup(func() { now = old })
// Due tomorrow, local time, stored the way the store hands it back: UTC.
due := time.Date(2026, 8, 2, 0, 0, 0, 0, fixed.Location()).UTC()
core := &fakeTaskCore{tasks: []ipc.Task{
{ID: 1, Text: "оплатить интернет", Status: "open", CreatedTs: fixed, Due: &due},
}}
rec := httptest.NewRecorder()
handleTasks(rec, httptest.NewRequest(http.MethodGet, "/tasks", nil), core)
body := rec.Body.String()
if !strings.Contains(body, "завтра") {
t.Errorf("why column does not say завтра: %q", why(body))
}
if strings.Contains(body, "сегодня") || strings.Contains(body, "просрочено") {
t.Error("a task due tomorrow was ranked as today's or overdue")
}
}
// why is a crude excerpt of the rendered why column, for a readable failure.
func why(body string) string {
i := strings.Index(body, "<td class=hint>")
if i < 0 {
return body
}
j := i + 200
if j > len(body) {
j = len(body)
}
return body[i:j]
}
// The resolved section rendered every row that ever existed.
func TestHandleTasksBoundsResolved(t *testing.T) {
base := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
var rows []ipc.Task
for i := 0; i < resolvedShown+10; i++ {
ts := base.Add(time.Duration(i) * time.Minute)
rows = append(rows, ipc.Task{
ID: int64(i + 1), Text: fmt.Sprintf("задача %d", i), Status: "done",
CreatedTs: ts, Resolved: &ts,
})
}
core := &fakeTaskCore{tasks: rows}
rec := httptest.NewRecorder()
handleTasks(rec, httptest.NewRequest(http.MethodGet, "/tasks", nil), core)
body := rec.Body.String()
if n := strings.Count(body, "задача "); n != resolvedShown {
t.Errorf("rendered %d resolved rows, want the %d-row bound", n, resolvedShown)
}
if !strings.Contains(body, "most recent are shown") {
t.Error("the page must say it is showing only part of the history")
}
}
// A capture over a candidate is a confirmation, not a duplicate.
func TestHandleTasksAddSaysPromoted(t *testing.T) {
core := &fakeTaskCore{promoted: true}
form := url.Values{"action": {"add"}, "text": {"продлить страховку"}}
req := httptest.NewRequest(http.MethodPost, "/tasks", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
rec := httptest.NewRecorder()
handleTasks(rec, req, core)
if !strings.Contains(rec.Body.String(), "confirmed a candidate") {
t.Error("a promoted capture must not read as a duplicate")
}
}
// Sscanf accepted "3junk" as 3, and the same call parsed the row id.
func TestApplyTaskPostRejectsTrailingGarbage(t *testing.T) {
core := &fakeTaskCore{created: true}
form := url.Values{"action": {"add"}, "text": {"что-то"}, "weight": {"3junk"}}
req := httptest.NewRequest(http.MethodPost, "/tasks", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
rec := httptest.NewRecorder()
handleTasks(rec, req, core)
if len(core.captured) != 0 {
t.Errorf("captured %+v, want nothing on a malformed weight", core.captured)
}
if !strings.Contains(rec.Body.String(), "bad weight") {
t.Error("error not surfaced on the page")
}
core = &fakeTaskCore{}
form = url.Values{"action": {"done"}, "id": {"42junk"}}
req = httptest.NewRequest(http.MethodPost, "/tasks", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
handleTasks(httptest.NewRecorder(), req, core)
if core.statusID != 0 {
t.Errorf("SetTaskStatus called with id %d on a malformed id", core.statusID)
}
}
// A resolution says what resolved it: resolved_ts recorded when and never by
// what.
func TestHandleTasksRecordsTheCaller(t *testing.T) {
core := &fakeTaskCore{}
form := url.Values{"action": {"done"}, "id": {"42"}}
req := httptest.NewRequest(http.MethodPost, "/tasks", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
handleTasks(httptest.NewRecorder(), req, core)
if core.statusBy != "tap:web" {
t.Errorf("resolved by %q, want tap:web", core.statusBy)
}
}
+44 -12
View File
@@ -62,10 +62,13 @@ const (
scoreDueToday = 60
scoreDueTomorrow = 40
scoreDueWeek = 20
scoreDueLater = 5
scorePerWeight = 15 // "срочно" / "важно" / the web form's select
scorePerWeekOld = 1 // so nothing rots at the bottom forever
scoreAgeCap = 10
// Above scoreAgeCap on purpose: a dated task must outrank an undated one
// however long the undated one has sat, or the class ordering this block
// claims is inverted by age alone.
scoreDueLater = 12
scorePerWeight = 15 // "срочно" / "важно" / the web form's select
scorePerWeekOld = 1 // so nothing rots at the bottom forever
scoreAgeCap = 10
// MaxWeight — the highest importance hint capture accepts. Three rungs is
// as many as anyone can rank by hand honestly.
MaxWeight = 3
@@ -141,7 +144,13 @@ func score(it Item, now time.Time) (float64, string) {
if w > 0 {
total += float64(w * scorePerWeight)
if reason == "" {
// The rungs get their own words. The reason string is the one place
// the ranking explains itself, and reading "важно" back at a task
// he flagged "срочно" reports a word he did not say.
reason = "важно"
if w >= MaxWeight {
reason = "срочно"
}
}
}
@@ -161,15 +170,23 @@ func score(it Item, now time.Time) (float64, string) {
return total, reason
}
// dayDelta — calendar days from now to due, in due's own location. Whole days,
// not hours: a task due today is due today whether it is 09:00 or 23:00, and an
// dayDelta — calendar days from now to due, in NOW's location. Whole days, not
// hours: a task due today is due today whether it is 09:00 or 23:00, and an
// hours-based comparison would call this evening's task "overdue" all afternoon.
//
// The location has to come from now. A due date read back from the store is a
// UTC instant (store.scanTask ends in time.UnixMilli(...).UTC()), so taking the
// location from it compared calendar days in UTC while the page rendered the
// same date in local time. East of Greenwich that is off by one all morning: a
// task due tomorrow read "сегодня", and on its due date it read "просрочено на
// день" and scored 105 instead of 60, one table cell away from a due column
// that said otherwise.
func dayDelta(due, now time.Time) int {
loc := due.Location()
d := time.Date(due.Year(), due.Month(), due.Day(), 0, 0, 0, 0, loc)
n := now.In(loc)
n = time.Date(n.Year(), n.Month(), n.Day(), 0, 0, 0, 0, loc)
return int(d.Sub(n).Hours() / 24)
loc := now.Location()
d := due.In(loc)
dd := time.Date(d.Year(), d.Month(), d.Day(), 0, 0, 0, 0, loc)
nn := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, loc)
return int(dd.Sub(nn).Hours() / 24)
}
// SpokenLimit — how many tasks the spoken list names before it summarises the
@@ -232,7 +249,22 @@ func joinRU(rs []Ranked, limit int, withReasons bool) string {
}
s := strings.Join(parts, "; ")
if rest > 0 {
s += fmt.Sprintf("; и ещё %d", rest)
// With the noun. Spoken, a bare number trails off mid-sentence.
s += fmt.Sprintf("; и ещё %d %s", rest, pluralTasksRU(rest))
}
return s
}
// pluralTasksRU — the right form of "задача" for a count. Russian needs three.
func pluralTasksRU(n int) string {
if n%100 >= 11 && n%100 <= 14 {
return "задач"
}
switch n % 10 {
case 1:
return "задача"
case 2, 3, 4:
return "задачи"
}
return "задач"
}
+68
View File
@@ -175,3 +175,71 @@ func TestFormatRUEmpty(t *testing.T) {
t.Errorf("reply = %q", got)
}
}
// A due date read back from the store is a UTC instant, so comparing calendar
// days in ITS location put every date a day out east of Greenwich: the row said
// "сегодня" for a task due tomorrow, and "просрочено на день" on the due date
// itself while the due column one cell over said otherwise.
func TestRankComparesDaysInTheCallersLocation(t *testing.T) {
tz := time.FixedZone("UTC+4", 4*3600)
// Entered on the web form as 2026-08-02 local, stored and read back as UTC.
due := time.Date(2026, 8, 2, 0, 0, 0, 0, tz).UTC()
local := time.Date(2026, 8, 1, 10, 0, 0, 0, tz)
got := Rank([]Item{{ID: 1, Text: "оплатить интернет", Status: StatusOpen, Due: &due, Created: local}}, local)
if got[0].Reason != "завтра" {
t.Errorf("reason = %q, want завтра on the day before", got[0].Reason)
}
// The morning of the due date itself.
onTheDay := time.Date(2026, 8, 2, 10, 0, 0, 0, tz)
got = Rank([]Item{{ID: 1, Text: "оплатить интернет", Status: StatusOpen, Due: &due, Created: local}}, onTheDay)
if got[0].Reason != "сегодня" {
t.Errorf("reason = %q, want сегодня on the due date", got[0].Reason)
}
if got[0].Score != scoreDueToday {
t.Errorf("score = %v, want %v", got[0].Score, float64(scoreDueToday))
}
}
// "срочно" and "важно" are two rungs and the read-back said "важно" for both,
// which reports a word he did not say.
func TestRankNamesTheUrgencyHeStated(t *testing.T) {
got := Rank([]Item{
{ID: 1, Text: "оплатить интернет", Status: StatusOpen, Weight: 3, Created: now()},
{ID: 2, Text: "починить кран", Status: StatusOpen, Weight: 2, Created: now()},
}, now())
if got[0].Reason != "срочно" {
t.Errorf("reason = %q, want срочно", got[0].Reason)
}
if got[1].Reason != "важно" {
t.Errorf("reason = %q, want важно", got[1].Reason)
}
}
// The package doc guarantees a class ordering. Age used to invert it: an
// undated task at the age cap outscored a dated one three weeks out.
func TestRankDatedWorkBeatsAgeAlone(t *testing.T) {
got := Rank([]Item{
{ID: 1, Text: "старьё", Status: StatusOpen, Created: now().AddDate(0, 0, -70)},
{ID: 2, Text: "через три недели", Status: StatusOpen, Due: at(2026, 8, 22), Created: now()},
}, now())
if got[0].Text != "через три недели" {
t.Errorf("order = %v, want the dated task first", texts(got))
}
}
// A bare "и ещё 5" trails off when spoken.
func TestFormatRUTailCarriesTheNoun(t *testing.T) {
var items []Item
for i := 0; i < SpokenLimit+3; i++ {
items = append(items, Item{ID: int64(i), Text: "дело", Status: StatusOpen, Created: now()})
}
if got := FormatRU(Rank(items, now())); !strings.Contains(got, "и ещё 3 задачи") {
t.Errorf("reply = %q, want the count with its noun", got)
}
for n, want := range map[int]string{1: "задача", 2: "задачи", 5: "задач", 11: "задач", 21: "задача"} {
if got := pluralTasksRU(n); got != want {
t.Errorf("pluralTasksRU(%d) = %q, want %q", n, got, want)
}
}
}