diff --git a/docs/plan.md b/docs/plan.md index e43e093..658978b 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -171,13 +171,14 @@ The v1.2 design in §4 to §6 stays as reference. A piece of it is built only wh | `home-time-week` | The Home's "This week" shows my real week: home asks the time service for it over the time contract, at the same time as it asks its peers for their items and with the same 1.5 s, and answers it as the Home's `week`, beside a new `weekState` that says how asking went, `ok` or `unavailable`. A week that time couldn't give still leaves "This week" out, until `week-unavailable` shows "Hours unavailable". The tenth of `time-tracking`'s fourteen steps (decision 186) | Done (#155) | | `week-unavailable` | The Home's "This week" says "Hours unavailable" when home couldn't get the week from the time service, its `weekState` being `unavailable`: the head and its "Day report" link stay, and the card says so in place of the hours, in their type and in `--ink-muted`, without the "Logged on Jira" label and the bar. With `ok` and no week, as without Jira, or with neither member, the section stays out (decision 161). The eleventh of `time-tracking`'s fourteen steps (decision 187) | Done (#156) | | `time-attention` | The time service answers the attention contract, so the Home's "Needs you" lists the hours to log on Jira: a row for each weekday of the last 7 days with nothing logged, today's included until anything is logged, each due at 17:00 on its day, so the days before today show as late. The twelfth of `time-tracking`'s fourteen steps (decision 188) | Done (#157) | +| `jira-add-worklog` | The shared Jira client adds a worklog to one of my tickets, for the `time` service to take one: the work's start, with its own offset, and the time spent in whole seconds, keeping Jira's defaults; a key that isn't a Jira key fails before anything is sent, and decision 125's key rule moves from todos to the client. The thirteenth of `time-tracking`'s sixteen steps (decision 189) | Done (#158) | | `nudge-merge` | Nudge, the user's phone-questions tool for agents (`codedogapp/nudge`), moves into Bakhyard: one app, not two (decision 157). The nudge session does the work, one small PR per step, from its plan, [`bakhyard-merge.md`](https://github.com/codedogapp/nudge/blob/main/docs/bakhyard-merge.md); this session doesn't stop its own steps for it | In review | After these: the product phases (§8.2), each split into steps. **Deferred until a step needs it**: TypeScript code from the contracts and `buf breaking` (decision 158), NATS and events, OpenTelemetry, Prometheus metrics, streaming, unix sockets, a shared kit, per-service Go modules and `go.work`, the git hook, most mise tasks, and process-compose's detached mode and API server (§4.9). -**Ideas**: `sonar-issues` could page past 200 issues, or warn when it cuts them off. Longer task scripts could move into files that `check` shellchecks; today only the workflow's `run:` scripts are checked. A gate self-test could lint known-bad fixtures and expect every gate to fire, so a config typo can't switch one off silently (§10). pnpm's `savePrefix` setting could be empty, so `pnpm add` pins exact versions without `-E`. `//apps/web:check` could run `pnpm dedupe --check`, so a lockfile that a `pnpm add` by hand leaves with an older copy of a package fails too; Renovate's updates are deduped already (decision 101). The Home's components each style their own big numbers and mono labels (the app counts, the week's hours, the section head's note); shared type styles could replace the copies. The header's nav marks an app's link `aria-current="page"` on the app's deeper pages too (`/notes/x`); ARIA and GOV.UK's service navigation keep "page" for the link's own page and use "true" for a section the page is in, as the tab bar's More does (decision 52). Screen readers aren't told when a page changes in place; a polite live region that reads out the new title after each navigation could tell them, as SvelteKit's route announcer does. Each service's port is written twice in `process-compose.yaml`, in its `BAKHYARD_LISTEN` and its probe, and the gateway's `BAKHYARD_PEERS` repeats the other services'; a check could keep them in step. Nothing checks that every address is on the loopback, though nothing should listen on the LAN (§3). Pushover could join Web Push as a second notification channel, for alerts that repeat until acknowledged (§2, decision 64). After signing out, Back may bring back the signed-in page from the browser's back-forward cache, data included; the app could check its session again when a page is restored (`pageshow` with `persisted`). The web app's dev server (`//apps/web:dev`) proxies `/api` to the gateway on the loopback, which has answered 403 there since `auth-tailscale` (decision 77), so it shows the app without data; local development needs a way through the lock. The Notes filter matches the notes' paths only; a full-text search could look inside them too. A note with a dot in its name gets the web service's 404 when its page is reloaded (decision 94); the service could serve the app for every missing path under `/notes/`. Back and Forward can't put a note's page where it was left: the browser restores the scroll position once the page shows, before the note has loaded; the page could restore it itself once the note is in (the Navigation API's `scroll: 'manual'` and `event.scroll()`). Footnote links in a note go nowhere: rehype-sanitize prefixes the footnotes' ids but not the links to them, which GitHub fixes with a script (the vault has no footnotes yet). A task's checkbox has no accessible name, as on GitHub; it could take its item's text. These link forms wait until the vault uses them (decision 97): a note shown inline, where `![[Note]]` is a link today; heading and block links into other notes, which open the note at its top; links in a list property; links to files that aren't notes; relative paths (`../`); and links that match a note's name in another case. An image embed's size waits the same way (decision 98): `![[x.png|300]]` and `![[x.png|300x200]]` show the image at its own size today. So do embeds of other files, such as audio, video and PDFs, Markdown images of the vault's files (`![](x.png)`), and a way to see an image larger. Callouts, `> [!note]`, wait until the vault has one (decision 99). KaTeX's red for a formula that doesn't parse, `#cc0000`, has about 3.2:1 contrast on the dark paper, under WCAG AA's 4.5:1; a colour of our own could replace it. Every note's page loads KaTeX with the renderer, though one note in the vault has math; KaTeX could load only for a note that has some. The build also ships KaTeX's woff and ttf fonts, which a browser loads only when it can't read woff2, the first format KaTeX's stylesheet lists. Svelte's docs advise against the `class:` directive since 5.16, when the `class` attribute began to take an object or an array; `todos-groups` uses the attribute, while `AppCards` and the Notes `Tree` still use the directive. No test runs a migration's Down; a test could take each service's database down a migration and up again, so a Down that doesn't undo its Up fails. A todo dragged near the screen's top or bottom edge could scroll the page, so a todo on a long list can go further than the screen shows; today the user scrolls first, or uses the editor's moves (decision 120). Escape could cancel a drag. A dragged todo's row could follow the pointer while the other rows slide aside to make room, as iOS lists do, rather than a line marking the place; across groups, the cards would have to grow and shrink as it passes (decision 120). A todo could be dropped into a priority that has no todos, which has no section to drop it in; the editor's priority does that (decision 117). Thirteen components copy the card's style, a 1 px `--rule` border on the `--surface` with the offset shadow, from the Home's "Needs you" to the Time page's days; a shared class could replace the copies, as `list.css` did for the rows (decision 124). The end-to-end tests keep no trace of a failed test, so a flaky one can't be looked into afterwards; `trace: 'retain-on-failure'` would keep one. In `todos-ticket`, the Move test's five-second wait for the second move ran out twice in `mise run check`, on battery in Low Power Mode, though the move did land just after; it passed in 660 runs alone or in the whole suite. A linked todo's badge could link to its ticket in Jira; it sits in the box's label, where a tap ticks the todo, so the link needs a target of its own, at least 24 px and apart from the label (WCAG 2.2 SC 2.5.8, decision 126). The Jira inbox keeps a dismissal for good, so a dismissed ticket that leaves my open tickets and comes back later, reassigned to me or reopened, stays out of the inbox; the service could forget a dismissal once a search that succeeds no longer finds its ticket (decision 128). An inbox row could link to its ticket in Jira, as the Jira section's rows do; the row isn't a link, since it holds an Add button, so the link needs a target of its own, as the badge's does (decision 129). The Notes page's Sync could also make Gitea mirror the vault at once, through Gitea's `POST /repos/{owner}/{repo}/mirror-sync`, so a note that the Mac pushes shows on the laptop within a minute rather than up to ten (decision 151). The images workflow builds and tags all six images on every change, so Flux restarts every service each time (decision 152); it could build only the images that a change touches. GitHub's CI could build the images on each PR too, without pushing them, so a Dockerfile that no longer builds fails before the merge rather than in Gitea after it. The five Go services' Dockerfiles differ only in the service's name; one Dockerfile with the name as a build argument could replace them (decision 153). The Jira inbox's rule lives in two places, the Todos page's `inboxTickets` and todos' attention lister (decision 160); the page could take the inbox from todos through a JSON route, so the rule lives in one. The services talk in plaintext inside the cluster, gRPC over h2c and HTTP/1.1, behind the NetworkPolicies (decisions 137 and 158); mutual TLS between them, from a service mesh or from cert-manager's certificates, would encrypt that traffic. SonarQube Cloud's rule for clear-text protocols, S5332, looks for an `http://` literal, so it doesn't see these calls (decision 168). The services' connections to Postgres are encrypted but don't check the server's certificate (`sslmode=require`); with CloudNativePG's CA mounted and `sslmode=verify-full`, they would (decision 170). pulls could find my Bitbucket workspaces itself, through `GET /2.0/user/workspaces`, which needs the scope `read:workspace:bitbucket` that today's token lacks, and drop `BAKHYARD_BITBUCKET_WORKSPACES`. The time report's target is 8 hours each weekday, so a public holiday or a day off shows as a missing day (decision 180); a setting could list the days without a target. JSON can't encode a due time outside the years 0 to 9999, so an item that a peer sends with one makes home cut the whole Home's answer short, as hours that aren't finite would (decision 186); home could count that peer as unavailable instead. time's tests run in the process's zone, and CI's is UTC, so they can't tell a report or a due time in UTC from one in the local zone there (decision 188); `routes` could take the zone, so a test could pin one far from UTC. +**Ideas**: `sonar-issues` could page past 200 issues, or warn when it cuts them off. Longer task scripts could move into files that `check` shellchecks; today only the workflow's `run:` scripts are checked. A gate self-test could lint known-bad fixtures and expect every gate to fire, so a config typo can't switch one off silently (§10). pnpm's `savePrefix` setting could be empty, so `pnpm add` pins exact versions without `-E`. `//apps/web:check` could run `pnpm dedupe --check`, so a lockfile that a `pnpm add` by hand leaves with an older copy of a package fails too; Renovate's updates are deduped already (decision 101). The Home's components each style their own big numbers and mono labels (the app counts, the week's hours, the section head's note); shared type styles could replace the copies. The header's nav marks an app's link `aria-current="page"` on the app's deeper pages too (`/notes/x`); ARIA and GOV.UK's service navigation keep "page" for the link's own page and use "true" for a section the page is in, as the tab bar's More does (decision 52). Screen readers aren't told when a page changes in place; a polite live region that reads out the new title after each navigation could tell them, as SvelteKit's route announcer does. Each service's port is written twice in `process-compose.yaml`, in its `BAKHYARD_LISTEN` and its probe, and the gateway's `BAKHYARD_PEERS` repeats the other services'; a check could keep them in step. Nothing checks that every address is on the loopback, though nothing should listen on the LAN (§3). Pushover could join Web Push as a second notification channel, for alerts that repeat until acknowledged (§2, decision 64). After signing out, Back may bring back the signed-in page from the browser's back-forward cache, data included; the app could check its session again when a page is restored (`pageshow` with `persisted`). The web app's dev server (`//apps/web:dev`) proxies `/api` to the gateway on the loopback, which has answered 403 there since `auth-tailscale` (decision 77), so it shows the app without data; local development needs a way through the lock. The Notes filter matches the notes' paths only; a full-text search could look inside them too. A note with a dot in its name gets the web service's 404 when its page is reloaded (decision 94); the service could serve the app for every missing path under `/notes/`. Back and Forward can't put a note's page where it was left: the browser restores the scroll position once the page shows, before the note has loaded; the page could restore it itself once the note is in (the Navigation API's `scroll: 'manual'` and `event.scroll()`). Footnote links in a note go nowhere: rehype-sanitize prefixes the footnotes' ids but not the links to them, which GitHub fixes with a script (the vault has no footnotes yet). A task's checkbox has no accessible name, as on GitHub; it could take its item's text. These link forms wait until the vault uses them (decision 97): a note shown inline, where `![[Note]]` is a link today; heading and block links into other notes, which open the note at its top; links in a list property; links to files that aren't notes; relative paths (`../`); and links that match a note's name in another case. An image embed's size waits the same way (decision 98): `![[x.png|300]]` and `![[x.png|300x200]]` show the image at its own size today. So do embeds of other files, such as audio, video and PDFs, Markdown images of the vault's files (`![](x.png)`), and a way to see an image larger. Callouts, `> [!note]`, wait until the vault has one (decision 99). KaTeX's red for a formula that doesn't parse, `#cc0000`, has about 3.2:1 contrast on the dark paper, under WCAG AA's 4.5:1; a colour of our own could replace it. Every note's page loads KaTeX with the renderer, though one note in the vault has math; KaTeX could load only for a note that has some. The build also ships KaTeX's woff and ttf fonts, which a browser loads only when it can't read woff2, the first format KaTeX's stylesheet lists. Svelte's docs advise against the `class:` directive since 5.16, when the `class` attribute began to take an object or an array; `todos-groups` uses the attribute, while `AppCards` and the Notes `Tree` still use the directive. No test runs a migration's Down; a test could take each service's database down a migration and up again, so a Down that doesn't undo its Up fails. A todo dragged near the screen's top or bottom edge could scroll the page, so a todo on a long list can go further than the screen shows; today the user scrolls first, or uses the editor's moves (decision 120). Escape could cancel a drag. A dragged todo's row could follow the pointer while the other rows slide aside to make room, as iOS lists do, rather than a line marking the place; across groups, the cards would have to grow and shrink as it passes (decision 120). A todo could be dropped into a priority that has no todos, which has no section to drop it in; the editor's priority does that (decision 117). Thirteen components copy the card's style, a 1 px `--rule` border on the `--surface` with the offset shadow, from the Home's "Needs you" to the Time page's days; a shared class could replace the copies, as `list.css` did for the rows (decision 124). The end-to-end tests keep no trace of a failed test, so a flaky one can't be looked into afterwards; `trace: 'retain-on-failure'` would keep one. In `todos-ticket`, the Move test's five-second wait for the second move ran out twice in `mise run check`, on battery in Low Power Mode, though the move did land just after; it passed in 660 runs alone or in the whole suite. A linked todo's badge could link to its ticket in Jira; it sits in the box's label, where a tap ticks the todo, so the link needs a target of its own, at least 24 px and apart from the label (WCAG 2.2 SC 2.5.8, decision 126). The Jira inbox keeps a dismissal for good, so a dismissed ticket that leaves my open tickets and comes back later, reassigned to me or reopened, stays out of the inbox; the service could forget a dismissal once a search that succeeds no longer finds its ticket (decision 128). An inbox row could link to its ticket in Jira, as the Jira section's rows do; the row isn't a link, since it holds an Add button, so the link needs a target of its own, as the badge's does (decision 129). The Notes page's Sync could also make Gitea mirror the vault at once, through Gitea's `POST /repos/{owner}/{repo}/mirror-sync`, so a note that the Mac pushes shows on the laptop within a minute rather than up to ten (decision 151). The images workflow builds and tags all six images on every change, so Flux restarts every service each time (decision 152); it could build only the images that a change touches. GitHub's CI could build the images on each PR too, without pushing them, so a Dockerfile that no longer builds fails before the merge rather than in Gitea after it. The five Go services' Dockerfiles differ only in the service's name; one Dockerfile with the name as a build argument could replace them (decision 153). The Jira inbox's rule lives in two places, the Todos page's `inboxTickets` and todos' attention lister (decision 160); the page could take the inbox from todos through a JSON route, so the rule lives in one. The services talk in plaintext inside the cluster, gRPC over h2c and HTTP/1.1, behind the NetworkPolicies (decisions 137 and 158); mutual TLS between them, from a service mesh or from cert-manager's certificates, would encrypt that traffic. SonarQube Cloud's rule for clear-text protocols, S5332, looks for an `http://` literal, so it doesn't see these calls (decision 168). The services' connections to Postgres are encrypted but don't check the server's certificate (`sslmode=require`); with CloudNativePG's CA mounted and `sslmode=verify-full`, they would (decision 170). pulls could find my Bitbucket workspaces itself, through `GET /2.0/user/workspaces`, which needs the scope `read:workspace:bitbucket` that today's token lacks, and drop `BAKHYARD_BITBUCKET_WORKSPACES`. The time report's target is 8 hours each weekday, so a public holiday or a day off shows as a missing day (decision 180); a setting could list the days without a target. JSON can't encode a due time outside the years 0 to 9999, so an item that a peer sends with one makes home cut the whole Home's answer short, as hours that aren't finite would (decision 186); home could count that peer as unavailable instead. time's tests run in the process's zone, and CI's is UTC, so they can't tell a report or a due time in UTC from one in the local zone there (decision 188); `routes` could take the zone, so a test could pin one far from UTC. When Jira refuses a worklog, the error gives only Jira's status, such as `400 Bad Request`; Jira's answer says why, in `errorMessages` and `errors`, which the client could read into the error (decision 189). ## 1. Goal and scope @@ -1202,7 +1203,7 @@ Every item is pending unless it says done. The service an item lands in is in br - `pulls-reminders`: at 10:00 and 15:00 on weekdays, pulls pushes one reminder that lists my PRs under their required approvals (decision 177). Done (#144). - `homelab-pulls-notify`: done (codedogapp/homelab#40). In the homelab repo, pulls' `BAKHYARD_PEERS` names notify, `TZ` gives it the user's zone, and a NetworkPolicy lets pulls reach notify. - Later, with a database for pulls: snooze (2 h / tomorrow / until updated) and mark as nudged, from the PRs page. -- `time-tracking` [time]. Needs `todos-jira`. Reads and writes standard Jira worklogs (§2, decision 46), with no timers: Clockwork already has them inside Jira. Fourteen steps (decisions 180, 181, 183, 186, 187 and 188): +- `time-tracking` [time]. Needs `todos-jira`. Reads and writes standard Jira worklogs (§2, decision 46), with no timers: Clockwork already has them inside Jira. Sixteen steps (decisions 180, 181, 183, 186, 187, 188 and 189): - `shared-jira`: todos' Jira client moves to the root `internal/jira`, with the code that reads its settings and token, for the `time` service; todos' poller of my tickets moves to a package of its own, `tickets`. Nothing changes in behaviour. Done (#147). - `jira-worklogs`: the client reads my worklogs in a span of days: my account from `/myself`, the tickets I logged on from a `worklogAuthor` search, and each ticket's worklogs. Done (#148). - `time-report`: a package of the `time` service, `report`, works out the time report from my worklogs: the hours I logged on each of the last 7 days, with every weekday before today that has none marked as missing, plus this week's total against the target. Done (#149). @@ -1215,7 +1216,9 @@ Every item is pending unless it says done. The service an item lands in is in br - `home-time-week`: home asks `time` for the week, and answers it with how asking went, `weekState`, so the Home's "This week" shows my real week (decision 186). Done (#155). - `week-unavailable`: the Home's "This week" says "Hours unavailable" when home couldn't get the week from `time` (§7, decision 187). Done (#156). - `time-attention`: `time` answers the attention contract with an item for each weekday with nothing logged, today's included until anything is logged, due at 17:00 on its day (decision 188). Done (#157). - - `worklog-add`: the client adds a worklog, and `time` takes one at `POST /time/worklogs`. + - `jira-add-worklog`: the shared Jira client adds a worklog to one of my tickets (decision 189). Done (#158). + - `poll-now`: the shared poller polls on request, between its timed polls, and says when that poll is over (decision 189). + - `worklog-add`: `time` takes a worklog at `POST /time/worklogs`, adds it to Jira, and polls again before it answers (decision 189). - `fill-day`: the Time page fills a missing day: pick one of my open tickets (assigned to me, not done) and the hours; Bakhyard shows the worklog it will add, starting at 09:00 local time, and writes it only after I confirm. **Phase 4: GitHub** @@ -1604,6 +1607,13 @@ Made on 2026-10-23: - **A known limit**: the report's last day counts as today even when the report is old, as on the Time page. - **Code**: `Lister` in `services/time/internal/attention`, beside `week`, as pulls' lister is in its `attention`; `routes` registers it on the connect server that serves `GetWeek`, and names the local zone once, for both the poller and the lister, so `poll` now takes the zone. - **Rejected**: one item for all the missing days, which hides which they are; the "under 8 hours" rule, which would nag about every partly logged day; due at midnight, which makes today late only once it's over; no items until a read succeeds, which reads as nothing to log; the date in the detail, which Go and the app's en-GB format spell differently ("Sep" against "Sept"); a date type of its own, or a start time on the report's days; the weekday rule worked out again in `attention`. +189. **Adding a worklog** (`jira-add-worklog`, `poll-now`, `worklog-add`): the lead's choices (2026-10-29), for the user to review. `worklog-add` would change three packages: the shared Jira client, the shared poller and `time`. AGENTS.md wants a package a step, so it splits in three, and `time-tracking` has sixteen steps. + - **The steps**: `jira-add-worklog`, the client adds a worklog; `poll-now`, the poller polls on request, so `time` reads the new worklog before it answers, as decision 180 planned; `worklog-add`, `time` takes a worklog at `POST /time/worklogs`. + - **The client**: `AddWorklog(ctx, key, worklog)` posts `started`, in Jira's layout with the start's own offset (`2026-10-26T09:00:00.000+0100`), and `timeSpentSeconds`, the time spent in whole seconds, to `rest/api/3/issue/{key}/worklog`. It sends no query, so Jira's defaults hold, as decision 180 chose: Jira emails the ticket's watchers, and takes the time off the ticket's remaining estimate. It takes a 201 only and doesn't read the answer; any other status fails as `add a Jira worklog: `. It sends no comment. + - **The key**: the client refuses a key that isn't a Jira key before it sends anything, since the key is a segment of the path, which `.` or `..` would move. Decision 125's rule moves from todos to the client as `jira.IsKey`, which todos now calls; `ErrTicket` stays in todos. + - **The shared code**: the client's `do` takes the status it wants, and reads no answer when it's given none; `newPost` builds the JSON POSTs of `Fetch` and `AddWorklog`. + - **Rejected**: `timeSpent` as text ("1h 30m"), which Jira reads by the site's time-tracking settings; reading the new worklog from the answer, which nothing needs; Jira's error messages in a refusal (a §0 idea); one step for all three packages. + - **Open**: whether the scoped token may write worklogs (`write:jira-work`, decision 103); the first write tells. ## 10. Risks diff --git a/internal/jira/addworklog.go b/internal/jira/addworklog.go new file mode 100644 index 0000000..2e65d5c --- /dev/null +++ b/internal/jira/addworklog.go @@ -0,0 +1,38 @@ +package jira + +import ( + "context" + "fmt" + "net/http" + "time" +) + +// newWorklog is the body of a new worklog: when the work started, in Jira's layout, and how long +// it took. +type newWorklog struct { + Started string `json:"started"` + TimeSpentSeconds int64 `json:"timeSpentSeconds"` +} + +// AddWorklog logs worklog on the ticket key as the user, in whole seconds (decision 189). Jira +// keeps its defaults: it emails the ticket's watchers, and takes the time off the ticket's +// remaining estimate. A key that isn't a Jira key fails before anything is sent. +func (c *Client) AddWorklog(ctx context.Context, key string, worklog Worklog) error { + const action = "add a Jira worklog" + + // The key is a path segment, which "." or ".." would move. + if !IsKey(key) { + return fmt.Errorf("%s: %q isn't a Jira key", action, key) + } + + req, err := newPost(ctx, c.settings.API.JoinPath("rest/api/3/issue", key, "worklog"), + newWorklog{ + Started: worklog.Started.Format(startedLayout), + TimeSpentSeconds: int64(worklog.Spent / time.Second), + }) + if err != nil { + return fmt.Errorf("%s: %w", action, err) + } + + return c.do(req, http.StatusCreated, action, nil) +} diff --git a/internal/jira/addworklog_test.go b/internal/jira/addworklog_test.go new file mode 100644 index 0000000..68c8121 --- /dev/null +++ b/internal/jira/addworklog_test.go @@ -0,0 +1,105 @@ +package jira_test + +import ( + "io" + "net/http" + "testing" + "time" + + "github.com/codedogapp/bakhyard/internal/jira" +) + +// worklogPath is where the fake Jira takes WEB-9's worklogs. +const worklogPath = apiPath + "/rest/api/3/issue/WEB-9/worklog" + +// added is what the fake Jira got in a request. +type added struct { + method, path, query, contentType, accept, user, password, body string +} + +// adder returns a client whose fake Jira answers every request with status and body, and the +// requests it got. +func adder(t *testing.T, status int, body string) (*jira.Client, *[]added) { + t.Helper() + + var got []added + + client := clientOf(t, func(w http.ResponseWriter, r *http.Request) { + sent, err := io.ReadAll(r.Body) + if err != nil { + t.Error(err) + } + + user, password, _ := r.BasicAuth() + got = append(got, added{ + r.Method, r.URL.Path, r.URL.RawQuery, r.Header.Get("Content-Type"), + r.Header.Get("Accept"), user, password, string(sent), + }) + + w.WriteHeader(status) + _, _ = io.WriteString(w, body) + }) + + return client, &got +} + +func zone(t *testing.T, name string) *time.Location { + t.Helper() + + loc, err := time.LoadLocation(name) + if err != nil { + t.Fatal(err) + } + + return loc +} + +func TestAddWorklogPostsTheWorklog(t *testing.T) { + client, got := adder(t, http.StatusCreated, `{"id":"100028","timeSpentSeconds":5400}`) + start := time.Date(2026, 10, 26, 9, 0, 0, 0, zone(t, "Europe/Budapest")) + + err := client.AddWorklog(t.Context(), "WEB-9", jira.Worklog{ + Started: start, Spent: 90 * time.Minute, + }) + + want := added{ + http.MethodPost, worklogPath, "", "application/json", "application/json", email, token, + `{"started":"2026-10-26T09:00:00.000+0100","timeSpentSeconds":5400}`, + } + if err != nil || len(*got) != 1 || (*got)[0] != want { + t.Errorf("got %v and the requests %+v, want no error and one request %+v", err, *got, + want) + } +} + +func TestAddWorklogSendsTheStartAsItIs(t *testing.T) { + for _, test := range []struct { + started time.Time + spent time.Duration + want string + }{ + { + time.Date(2026, 10, 26, 8, 0, 0, 0, time.UTC), time.Minute, + `{"started":"2026-10-26T08:00:00.000+0000","timeSpentSeconds":60}`, + }, + { + time.Date(2026, 7, 1, 9, 0, 0, 0, zone(t, "America/St_Johns")), 24 * time.Hour, + `{"started":"2026-07-01T09:00:00.000-0230","timeSpentSeconds":86400}`, + }, + { + time.Date(2026, 10, 26, 9, 0, 0, 250_900_000, zone(t, "Europe/Budapest")), + 1500 * time.Millisecond, + `{"started":"2026-10-26T09:00:00.250+0100","timeSpentSeconds":1}`, + }, + } { + client, got := adder(t, http.StatusCreated, "") + + err := client.AddWorklog(t.Context(), "WEB-9", jira.Worklog{ + Started: test.started, Spent: test.spent, + }) + if err != nil || len(*got) != 1 || (*got)[0].body != test.want { + t.Errorf("%v for %v: got %v and the requests %+v, want the body %s", test.started, + test.spent, err, *got, test.want) + } + } +} diff --git a/internal/jira/addworklogfail_test.go b/internal/jira/addworklogfail_test.go new file mode 100644 index 0000000..486fda9 --- /dev/null +++ b/internal/jira/addworklogfail_test.go @@ -0,0 +1,99 @@ +package jira_test + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + "time" + + "github.com/codedogapp/bakhyard/internal/jira" +) + +func TestAddWorklogFailsUnlessJiraAddsIt(t *testing.T) { + for _, status := range []int{ + http.StatusOK, http.StatusNoContent, http.StatusBadRequest, http.StatusUnauthorized, + http.StatusForbidden, http.StatusNotFound, http.StatusRequestEntityTooLarge, + http.StatusInternalServerError, + } { + client, _ := adder(t, status, `{"errorMessages":["No"],"errors":{}}`) + + err := client.AddWorklog(t.Context(), "WEB-9", jira.Worklog{ + Started: time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC), Spent: time.Hour, + }) + + want := "add a Jira worklog: " + strconv.Itoa(status) + " " + http.StatusText(status) + if err == nil || err.Error() != want { + t.Errorf("%d: got %v, want %s", status, err, want) + } + } +} + +func TestAddWorklogRefusesWhatIsntAKey(t *testing.T) { + for _, key := range []string{ + "", ".", "..", "web-9", "WEB-0", "WEB-9/x", " WEB-9", "WEB-9\n", "../../myself", + } { + client, got := adder(t, http.StatusCreated, "") + + err := client.AddWorklog(t.Context(), key, jira.Worklog{ + Started: time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC), Spent: time.Hour, + }) + + want := "add a Jira worklog: " + strconv.Quote(key) + " isn't a Jira key" + if err == nil || err.Error() != want || len(*got) != 0 { + t.Errorf("%q: got %v after the requests %+v, want %s before any", key, err, *got, + want) + } + } +} + +func TestAddWorklogFailsWithoutJira(t *testing.T) { + server := httptest.NewServer(http.NotFoundHandler()) + server.Close() + + client := jira.New(jira.Settings{ + Site: parse(t, "https://acme.example"), API: parse(t, server.URL), + Email: email, Token: token, + }) + + err := client.AddWorklog(t.Context(), "WEB-9", jira.Worklog{ + Started: time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC), Spent: time.Hour, + }) + if err == nil || !strings.HasPrefix(err.Error(), "add a Jira worklog: ") || + strings.Contains(err.Error(), token) { + t.Errorf("got %v, want an error that names the worklog", err) + } +} + +func TestAddWorklogStopsWhenItsContextEnds(t *testing.T) { + client, got := adder(t, http.StatusCreated, "") + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + err := client.AddWorklog(ctx, "WEB-9", jira.Worklog{ + Started: time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC), Spent: time.Hour, + }) + if !errors.Is(err, context.Canceled) || len(*got) != 0 { + t.Errorf("got %v after the requests %+v, want context.Canceled before any", err, *got) + } +} + +func TestAddWorklogFailsOnAnAddressItCantAsk(t *testing.T) { + api := parse(t, "https://acme.example") + api.Host = "acme example" + + client := jira.New(jira.Settings{ + Site: parse(t, "https://acme.example"), API: api, Email: email, Token: token, + }) + + err := client.AddWorklog(t.Context(), "WEB-9", jira.Worklog{ + Started: time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC), Spent: time.Hour, + }) + if err == nil || !strings.HasPrefix(err.Error(), `add a Jira worklog: parse "https://acme`) { + t.Errorf("got %v, want an error that names the worklog", err) + } +} diff --git a/internal/jira/client.go b/internal/jira/client.go index f8e4df1..c65bc80 100644 --- a/internal/jira/client.go +++ b/internal/jira/client.go @@ -1,10 +1,11 @@ // Package jira reads the user's tickets and worklogs on a Jira Cloud site, through Jira's REST -// API v3 and a scoped API token: it finds the open tickets, fetches tickets by key and reads the -// user's worklogs, for each service that the Jira settings set up (docs/plan.md §2, decisions -// 103, 121, 133 and 180). +// API v3 and a scoped API token: it finds the open tickets, fetches tickets by key, and reads and +// adds the user's worklogs, for each service that the Jira settings set up (docs/plan.md §2, +// decisions 103, 121, 133, 180 and 189). package jira import ( + "bytes" "context" "encoding/json/v2" "fmt" @@ -42,7 +43,7 @@ type Settings struct { Token string } -// Client reads tickets and worklogs on a Jira site. +// Client reads tickets and worklogs on a Jira site, and adds worklogs. type Client struct { settings Settings client *http.Client @@ -149,19 +150,38 @@ func (c *Client) page(ctx context.Context, jql, fields, token string) (page, err return found, nil } -// get asks Jira for target and reads Jira's answer into answer, as do does. +// get asks Jira for target and reads Jira's answer, a 200, into answer, as do does. func (c *Client) get(ctx context.Context, target *url.URL, action string, answer any) error { req, err := http.NewRequestWithContext(ctx, http.MethodGet, target.String(), nil) if err != nil { return fmt.Errorf("%s: %w", action, err) } - return c.do(req, action, answer) + return c.do(req, http.StatusOK, action, answer) } -// do sends req to Jira as the user and reads Jira's answer into answer. Its errors start with -// action, such as "search Jira", or with "read Jira's answer" when the answer is no use. -func (c *Client) do(req *http.Request, action string, answer any) error { +// newPost returns a request that posts body to target as JSON. +func newPost(ctx context.Context, target *url.URL, body any) (*http.Request, error) { + data, err := json.Marshal(body) + if err != nil { + return nil, err + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, target.String(), + bytes.NewReader(data)) + if err != nil { + return nil, err + } + + req.Header.Set("Content-Type", "application/json") + + return req, nil +} + +// do sends req to Jira as the user and, unless answer is nil, reads Jira's answer into answer. +// Jira must answer with the status want. Its errors start with action, such as "search Jira", +// or with "read Jira's answer" when the answer is no use. +func (c *Client) do(req *http.Request, want int, action string, answer any) error { req.SetBasicAuth(c.settings.Email, c.settings.Token) req.Header.Set("Accept", "application/json") @@ -172,10 +192,14 @@ func (c *Client) do(req *http.Request, action string, answer any) error { defer func() { _ = res.Body.Close() }() - if res.StatusCode != http.StatusOK { + if res.StatusCode != want { return fmt.Errorf("%s: %s", action, res.Status) } + if answer == nil { + return nil + } + err = json.UnmarshalRead(io.LimitReader(res.Body, maxAnswer), answer) if err != nil { return fmt.Errorf("read Jira's answer: %w", err) diff --git a/internal/jira/fetch.go b/internal/jira/fetch.go index d649bde..6428a82 100644 --- a/internal/jira/fetch.go +++ b/internal/jira/fetch.go @@ -1,9 +1,7 @@ package jira import ( - "bytes" "context" - "encoding/json/v2" "fmt" "net/http" "slices" @@ -47,24 +45,15 @@ func (c *Client) Fetch(ctx context.Context, keys []string) ([]Ticket, error) { // fetch returns Jira's answer for a batch of at most fetchSize keys. func (c *Client) fetch(ctx context.Context, keys []string) (fetched, error) { - body, err := json.Marshal(bulkFetch{Keys: keys, Fields: []string{"summary", "status"}}) + req, err := newPost(ctx, c.settings.API.JoinPath("rest/api/3/issue/bulkfetch"), + bulkFetch{Keys: keys, Fields: []string{"summary", "status"}}) if err != nil { return fetched{}, fmt.Errorf("fetch Jira tickets: %w", err) } - target := c.settings.API.JoinPath("rest/api/3/issue/bulkfetch") - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, target.String(), - bytes.NewReader(body)) - if err != nil { - return fetched{}, fmt.Errorf("fetch Jira tickets: %w", err) - } - - req.Header.Set("Content-Type", "application/json") - var found fetched - err = c.do(req, "fetch Jira tickets", &found) + err = c.do(req, http.StatusOK, "fetch Jira tickets", &found) if err != nil { return fetched{}, err } diff --git a/internal/jira/key.go b/internal/jira/key.go new file mode 100644 index 0000000..3f2c333 --- /dev/null +++ b/internal/jira/key.go @@ -0,0 +1,13 @@ +package jira + +import "regexp" + +// keyRule matches a Jira ticket's key, as Jira makes them: its project's key, an upper-case +// letter and at least one more upper-case letter, digit or underscore, then a hyphen and the +// ticket's number (decision 125). +var keyRule = regexp.MustCompile(`^[A-Z][A-Z0-9_]+-[1-9][0-9]*$`) + +// IsKey reports whether s is a Jira ticket's key, such as ABC-123. +func IsKey(s string) bool { + return keyRule.MatchString(s) +} diff --git a/internal/jira/key_test.go b/internal/jira/key_test.go new file mode 100644 index 0000000..e500f27 --- /dev/null +++ b/internal/jira/key_test.go @@ -0,0 +1,29 @@ +package jira_test + +import ( + "testing" + + "github.com/codedogapp/bakhyard/internal/jira" +) + +func TestIsKeyTakesAnyJiraKey(t *testing.T) { + for _, key := range []string{ + "AB-1", "ABC-10", "A1-2", "A_B-3", "Z9_-2026", "ABCDEFGHIJ-99999", + } { + if !jira.IsKey(key) { + t.Errorf("%q: got false, want true", key) + } + } +} + +func TestIsKeyRefusesTheRest(t *testing.T) { + for _, text := range []string{ + "", ".", "..", "abc-1", "Abc-1", "aBC-1", "A-1", "1AB-2", "_AB-2", "AB-0", "AB-01", "AB--1", + "AB-1-2", "AB_1", "AB-", "AB", "-1", " AB-1", "AB-1 ", "AB-1\n", "AB 1", "ÄB-1", "AB-١", + "AB-1١", "AB-1x", "AB-1/x", "../AB-1", "AB-1%2F", + } { + if jira.IsKey(text) { + t.Errorf("%q: got true, want false", text) + } + } +} diff --git a/internal/jira/worklog.go b/internal/jira/worklog.go index 61683ce..59a2534 100644 --- a/internal/jira/worklog.go +++ b/internal/jira/worklog.go @@ -14,7 +14,8 @@ const ( // worklogPageSize is the number of worklogs that the client asks for in each page. A worklog // takes about 2.4 KB with its two users' avatars, so a page stays under maxAnswer. worklogPageSize = 1000 - // startedLayout is the layout of a worklog's start in Jira's answers, which isn't RFC 3339. + // startedLayout is the layout of a worklog's start in Jira's answers and in a new worklog, + // which isn't RFC 3339. startedLayout = "2006-01-02T15:04:05.000-0700" ) diff --git a/services/todos/internal/todo/dismiss.go b/services/todos/internal/todo/dismiss.go index c705192..cd4b7e8 100644 --- a/services/todos/internal/todo/dismiss.go +++ b/services/todos/internal/todo/dismiss.go @@ -5,6 +5,8 @@ import ( "fmt" "github.com/jackc/pgx/v5" + + "github.com/codedogapp/bakhyard/internal/jira" ) // dismissSQL stores the dismissal of ticket $1, unless it is stored already. @@ -17,7 +19,7 @@ const dismissedSQL = `SELECT jira_key FROM dismissals ORDER BY jira_key COLLATE // Dismiss dismisses the Jira ticket key from the inbox for good (decision 130), and reports whether // it wasn't dismissed already. A key that isn't a Jira key gets ErrTicket, and nothing is stored. func (s *Store) Dismiss(ctx context.Context, key string) (bool, error) { - if !jiraKey.MatchString(key) { + if !jira.IsKey(key) { return false, ErrTicket } diff --git a/services/todos/internal/todo/ticket.go b/services/todos/internal/todo/ticket.go index 910d1c3..23c650d 100644 --- a/services/todos/internal/todo/ticket.go +++ b/services/todos/internal/todo/ticket.go @@ -2,18 +2,14 @@ package todo import ( "errors" - "regexp" -) -// jiraKey matches a Jira ticket's key, as Jira makes them: its project's key, an upper-case letter -// and at least one more upper-case letter, digit or underscore, then a hyphen and the ticket's -// number (decision 125). -var jiraKey = regexp.MustCompile(`^[A-Z][A-Z0-9_]+-[1-9][0-9]*$`) + "github.com/codedogapp/bakhyard/internal/jira" +) // ErrTicket is the error of a ticket that is neither a Jira key nor empty. var ErrTicket = errors.New("invalid ticket: not a Jira key such as ABC-123") // validTicket reports whether ticket is a Jira key, or empty, which unlinks the todo's ticket. func validTicket(ticket string) bool { - return ticket == "" || jiraKey.MatchString(ticket) + return ticket == "" || jira.IsKey(ticket) }