From 705d828ce367bf013c02a034e3da736358d7bbba Mon Sep 17 00:00:00 2001 From: Bakhomious Basta Date: Thu, 29 Oct 2026 13:44:59 +0100 Subject: [PATCH] refactor(jira): share todos' Jira client for the time service The time service will read and write worklogs on the same Jira site, with the same settings and token as todos (decision 103), so the client and the code that reads its settings move to the root internal/jira. todos' poller of my tickets moves to its own package, tickets. Nothing changes in behaviour. Decision 180 plans the rest of time-tracking in eleven steps. --- docs/plan.md | 27 +++- .../internal => internal}/jira/client.go | 5 +- .../internal => internal}/jira/client_test.go | 2 +- internal/jira/env.go | 51 +++++++ internal/jira/env_test.go | 130 ++++++++++++++++++ .../internal => internal}/jira/fail_test.go | 2 +- .../internal => internal}/jira/fake_test.go | 2 +- .../todos/internal => internal}/jira/fetch.go | 0 .../internal => internal}/jira/fetch_test.go | 2 +- .../jira/fetchfail_test.go | 2 +- .../internal => internal}/jira/ticket.go | 0 services/todos/internal/api/api.go | 8 +- services/todos/internal/api/tickets.go | 4 +- services/todos/internal/api/tickets_test.go | 9 +- .../todos/internal/attention/attention.go | 21 +-- .../internal/attention/attention_test.go | 25 ++-- services/todos/internal/attention/inbox.go | 2 +- .../todos/internal/attention/inbox_test.go | 2 +- .../todos/internal/attention/polls_test.go | 2 +- .../internal/{jira => tickets}/done_test.go | 4 +- .../internal/{jira => tickets}/finder_test.go | 19 +-- .../todos/internal/{jira => tickets}/poll.go | 20 ++- .../internal/{jira => tickets}/poll_test.go | 2 +- services/todos/jira.go | 53 +------ services/todos/jira_test.go | 65 +-------- services/todos/main.go | 3 +- 26 files changed, 283 insertions(+), 179 deletions(-) rename {services/todos/internal => internal}/jira/client.go (94%) rename {services/todos/internal => internal}/jira/client_test.go (98%) create mode 100644 internal/jira/env.go create mode 100644 internal/jira/env_test.go rename {services/todos/internal => internal}/jira/fail_test.go (97%) rename {services/todos/internal => internal}/jira/fake_test.go (97%) rename {services/todos/internal => internal}/jira/fetch.go (100%) rename {services/todos/internal => internal}/jira/fetch_test.go (97%) rename {services/todos/internal => internal}/jira/fetchfail_test.go (98%) rename {services/todos/internal => internal}/jira/ticket.go (100%) rename services/todos/internal/{jira => tickets}/done_test.go (98%) rename services/todos/internal/{jira => tickets}/finder_test.go (79%) rename services/todos/internal/{jira => tickets}/poll.go (77%) rename services/todos/internal/{jira => tickets}/poll_test.go (98%) diff --git a/docs/plan.md b/docs/plan.md index d8f6113..8ce3451 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -160,13 +160,14 @@ The v1.2 design in §4 to §6 stays as reference. A piece of it is built only wh | `pulls-reminders` | At 10:00 and 15:00 on weekdays, in its time zone, the `pulls` service pushes one notification through notify, "Remind your reviewers", that lists my open pull requests that aren't drafts, were opened more than a day before and have fewer approvals than they need, a line each, such as "Fix the login (acme/web #1): 1 of 2 approvals"; a tap opens the PRs page. Without notify among its peers, pulls reminds no one. The last of `pr-reminders`' three steps (decision 177) | Done (#144) | | `icon-button` | The theme toggle's look, an icon with the header's hover underline under it, moves to a shared stylesheet as a class, `.icon-button`, which `notify-header`'s notifications bell will use too; nothing changes on screen. The first of the two steps that finish `notifications` (decision 178) | Done (#145) | | `notify-header` | The control that turns notifications on moves from the Nudge page to a bell in the header, before the theme toggle: its name and look say how this device's notifications stand, and its panel says it in words and turns them on. The last step of `notifications` (decision 179) | Done (#146) | +| `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. The first of `time-tracking`'s eleven steps (decision 180) | Done (#147) | | `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). Three components copy the card's style, a 1 px `--rule` border on the `--surface` with the offset shadow: Home's "Needs you", the Todos page's groups and its Jira tickets; 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`. +**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). Three components copy the card's style, a 1 px `--rule` border on the `--surface` with the offset shadow: Home's "Needs you", the Todos page's groups and its Jira tickets; 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. ## 1. Goal and scope @@ -1190,11 +1191,18 @@ 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). No timers: Clockwork already has them inside Jira. - - A day report: the hours I logged on each of the last 7 days, with every weekday that has none marked as missing, plus the week's total against the target. - - Fill 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. - - Attention items: today's hours until they're logged, and each missing weekday. +- `time-tracking` [time]. Needs `todos-jira`. Reads and writes standard Jira worklogs (§2, decision 46), with no timers: Clockwork already has them inside Jira. Eleven steps (decision 180): + - `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. + - `time-service`: a new `time` service polls the last 7 days' worklogs every 5 minutes and answers the day report at `GET /time/report`: the hours I logged on each day, with every weekday that has none marked as missing, plus the week's total against the target. + - `homelab-time`: in the homelab repo, `time` runs on the laptop with todos' Jira settings and token, and the gateway's and home's peers name it. + - `time-gateway`: the gateway forwards `/api/time` to `time`. + - `time-page`: the Time page shows the day report. + - `time-week`: `time` answers `TimeService/GetWeek` (§4.10). + - `home-week`: home asks `time` for the week, and the Home's "This week" shows it, or "Hours unavailable" when `time` doesn't answer (§7). + - `time-attention`: `time` answers the attention contract with today's hours until they're logged, and each missing weekday. + - `worklog-add`: the client adds a worklog, and `time` takes one at `POST /time/worklogs`. + - `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** @@ -1520,6 +1528,13 @@ Made on 2026-10-23: - **When the bell checks**: as the app starts, and each time the panel opens, since the browser's settings may have changed; until the answer, it says what it knew, and only the latest check counts. A check is `preparePush`, unchanged, so the app now registers the service worker on every page, and, while notifications are on, gives notify the subscription again each time the app starts. A failure belongs to the check it came from, so the next check forgets it. A tap on the button while it works does nothing. Once notifications are on, the button goes, and the bell takes the focus if it went with the button. - **The Nudge page** loses its control. - **Rejected**: an icon with a `title` and no panel; words in the header; a `
` menu, which a tap outside closes only by script; placing the panel by script; a button that turns notifications off; a centred panel for browsers without anchor positioning, which CI's browsers can't show. +180. **Time tracking in eleven steps**: the lead's choices (2026-10-29), for the user to review. `time-tracking` is far more than one step's 150 hand-written lines, so it splits in eleven (§8.2), each service step before the page step that shows it, as `bitbucket-prs`' went. + - **The Jira client is shared.** `time` is the second service that reads Jira, with the same site, account and scoped token as todos (decision 103), so `shared-jira` moves todos' client to the root `internal/jira`, a step ahead of its second user, as `shared-poller` (#109) moved the poller. `jira.FromEnv` reads the three Jira settings and the `jira-token` secret, as todos did, and gives no client when no setting is set. todos' poller of my tickets moves to `services/todos/internal/tickets`, as `tickets.Poller`, with its `Finder`, so `internal/jira` keeps only what talks to Jira. This amends decision 135, under which the `jira` package kept the `Finder` and `jira.Poller` stayed the poller's name. + - **Reading worklogs** (`jira-worklogs`): my account ID from `GET /rest/api/3/myself`; the tickets I logged on from `/rest/api/3/search/jql` with `worklogAuthor = currentUser()` and `worklogDate` bounds a day wider than the span on each side, since JQL dates are in the Jira profile's time zone (`fields=summary`, 100 a page, at most 10 pages); then each ticket's worklogs from `GET /rest/api/3/issue/{key}/worklog` with `startedAfter` and `startedBefore`, 5,000 a page, paging on `total`. Only my own worklogs count, by `author.accountId`, within the span. A worklog's `started` isn't RFC 3339 (`2006-01-02T15:04:05.000-0700`). §2's risk stays: should `worklogAuthor` miss worklogs, the fallback is `/worklog/updated` with `/worklog/list`. + - **The `time` service** polls the last 7 days every 5 minutes (§3) through `internal/poll`, keeps them in memory with no database, as `pulls` does, and listens on 127.0.0.1:8088 locally. It needs todos' Jira settings and token, so on the laptop it reuses todos' `todos-jira` settings and the `jira-token` in `todos-secrets` rather than copying them; the user said yes to that homelab PR being merged once its check is green (2026-10-29). + - **Days and the target**: days and weeks follow `time.Local`, the pod's `TZ`, as decision 175's reminders do. A week runs Monday to Sunday, and a worklog counts on the day it starts. The target is 8 hours a weekday, Monday to Friday, 40 a week, fixed in code. Today isn't missing until it's over; holidays show as missing until a setting for them (§0 idea). + - **Writing worklogs** (`worklog-add`, `fill-day`): `POST /rest/api/3/issue/{key}/worklog` with `started` and `timeSpentSeconds`, keeping Jira's defaults (`notifyUsers`, `adjustEstimate=auto`); it answers 201, so the client takes a 201 there. A new worklog starts at 09:00 local time, and `time` polls again after a write. + - **Rejected**: copying todos' client into `time`; a Jira client per service; timers (decision 46); a database of worklogs, which Jira already keeps. ## 10. Risks diff --git a/services/todos/internal/jira/client.go b/internal/jira/client.go similarity index 94% rename from services/todos/internal/jira/client.go rename to internal/jira/client.go index 07247ed..579fdcf 100644 --- a/services/todos/internal/jira/client.go +++ b/internal/jira/client.go @@ -1,5 +1,6 @@ -// Package jira finds the user's open tickets on a Jira Cloud site, through Jira's REST API v3 -// and a scoped API token (docs/plan.md §2, decisions 103 and 121). +// Package jira reads the user's tickets on a Jira Cloud site, through Jira's REST API v3 and a +// scoped API token: it finds the open tickets and fetches tickets by key, for each service that +// the Jira settings set up (docs/plan.md §2, decisions 103, 121, 133 and 180). package jira import ( diff --git a/services/todos/internal/jira/client_test.go b/internal/jira/client_test.go similarity index 98% rename from services/todos/internal/jira/client_test.go rename to internal/jira/client_test.go index 6edec0f..49f0ce6 100644 --- a/services/todos/internal/jira/client_test.go +++ b/internal/jira/client_test.go @@ -7,7 +7,7 @@ import ( "strings" "testing" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" ) func TestSearchAsksForTheOpenTickets(t *testing.T) { diff --git a/internal/jira/env.go b/internal/jira/env.go new file mode 100644 index 0000000..11e8aa1 --- /dev/null +++ b/internal/jira/env.go @@ -0,0 +1,51 @@ +package jira + +import ( + "context" + "fmt" + "slices" + + "github.com/codedogapp/bakhyard/internal/env" + "github.com/codedogapp/bakhyard/internal/secrets" +) + +// The settings of the Jira site (docs/setup.md). Jira is off when none is set. +const ( + siteVar = "BAKHYARD_JIRA_SITE" + apiVar = "BAKHYARD_JIRA_API" + emailVar = "BAKHYARD_JIRA_EMAIL" +) + +// tokenAccount is the Keychain account of the Jira API token, and the name of its file. +const tokenAccount = "jira-token" + +// FromEnv returns the client of the Jira site in the settings that getenv reads, with the API +// token that readSecret reads, or nil when none of the Jira settings is set. +func FromEnv(getenv func(string) string, readSecret secrets.Reader) (*Client, error) { + settings := []string{siteVar, apiVar, emailVar} + if !slices.ContainsFunc(settings, func(name string) bool { return getenv(name) != "" }) { + return nil, nil + } + + site, err := env.SecureURL(getenv, siteVar) + if err != nil { + return nil, err + } + + address, err := env.SecureURL(getenv, apiVar) + if err != nil { + return nil, err + } + + email, err := env.Required(getenv, emailVar) + if err != nil { + return nil, err + } + + token, err := readSecret(context.Background(), tokenAccount) + if err != nil { + return nil, fmt.Errorf("read the Jira token: %w", err) + } + + return New(Settings{Site: site, API: address, Email: email, Token: token}), nil +} diff --git a/internal/jira/env_test.go b/internal/jira/env_test.go new file mode 100644 index 0000000..b7d8cd8 --- /dev/null +++ b/internal/jira/env_test.go @@ -0,0 +1,130 @@ +package jira_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/codedogapp/bakhyard/internal/jira" + "github.com/codedogapp/bakhyard/internal/secrets" +) + +// The Jira site and REST API in the settings of most tests. +const ( + siteURL = "https://acme.atlassian.net" + apiURL = "https://api.atlassian.com/ex/jira/cloud-id" +) + +func TestFromEnvSearchesTheJiraInTheSettings(t *testing.T) { + client, err := jira.FromEnv(envOf(settingsAt(standIn(t))), + secretsWith(map[string]string{"jira-token": token})) + if err != nil { + t.Fatal(err) + } + + got, err := client.Search(t.Context()) + if err != nil || len(got) != 1 || got[0].URL != siteURL+"/browse/PAY-1" { + t.Errorf("got %v, %v, want PAY-1 on %s", got, err, siteURL) + } +} + +func TestFromEnvIsOffWithoutTheSettings(t *testing.T) { + client, err := jira.FromEnv(envOf(nil), secretsWith(nil)) + if client != nil || err != nil { + t.Errorf("got %v, %v, want no client and no error", client, err) + } +} + +func TestFromEnvRejects(t *testing.T) { + for _, test := range []rejection{ + {map[string]string{"BAKHYARD_JIRA_SITE": siteURL}, "BAKHYARD_JIRA_API is not set"}, + {map[string]string{"BAKHYARD_JIRA_API": apiURL}, "BAKHYARD_JIRA_SITE is not set"}, + {map[string]string{"BAKHYARD_JIRA_EMAIL": email}, "BAKHYARD_JIRA_SITE is not set"}, + {settingsWith("BAKHYARD_JIRA_SITE", ""), "BAKHYARD_JIRA_SITE is not set"}, + {settingsWith("BAKHYARD_JIRA_API", ""), "BAKHYARD_JIRA_API is not set"}, + {settingsWith("BAKHYARD_JIRA_EMAIL", ""), "BAKHYARD_JIRA_EMAIL is not set"}, + // internal/env tests which URLs pass; these show that both of Jira's URLs must. + notHTTPS("BAKHYARD_JIRA_SITE", "http://acme.atlassian.net"), + notHTTPS("BAKHYARD_JIRA_API", "http://example.com"), + } { + // Without the token, so the settings must be checked before it's read. + client, err := jira.FromEnv(envOf(test.settings), secretsWith(nil)) + if client != nil || err == nil || err.Error() != test.want { + t.Errorf("got %v, %v, want the error %s", client, err, test.want) + } + } +} + +func TestFromEnvNeedsTheToken(t *testing.T) { + client, err := jira.FromEnv(envOf(settingsAt(apiURL)), secretsWith(nil)) + + want := "read the Jira token: no secret jira-token" + if client != nil || err == nil || err.Error() != want { + t.Errorf("got %v, %v, want the error %s", client, err, want) + } +} + +// standIn returns the URL of a stand-in for Jira's REST API that has one open ticket, PAY-1, for +// email with token. +func standIn(t *testing.T) string { + t.Helper() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user, password, ok := r.BasicAuth() + if !ok || user != email || password != token { + w.WriteHeader(http.StatusUnauthorized) + + return + } + + answer(t, w, last(issue("PAY-1", "Pay", "To Do", "new"))) + })) + t.Cleanup(server.Close) + + return server.URL +} + +// settingsAt returns the Jira settings, with the REST API at address. +func settingsAt(address string) map[string]string { + return map[string]string{ + "BAKHYARD_JIRA_SITE": siteURL, "BAKHYARD_JIRA_API": address, "BAKHYARD_JIRA_EMAIL": email, + } +} + +// settingsWith returns the Jira settings, with name set to value. +func settingsWith(name, value string) map[string]string { + settings := settingsAt(apiURL) + settings[name] = value + + return settings +} + +// rejection is a row of TestFromEnvRejects: the settings, and the error they give. +type rejection struct { + settings map[string]string + want string +} + +// notHTTPS returns the rejection of value in the setting name as not an https URL. +func notHTTPS(name, value string) rejection { + return rejection{settingsWith(name, value), name + `: "` + value + `" is not an https URL`} +} + +// envOf returns an environment that holds only settings. +func envOf(settings map[string]string) func(string) string { + return func(name string) string { return settings[name] } +} + +// secretsWith returns a Reader of the secrets in items. +func secretsWith(items map[string]string) secrets.Reader { + return func(_ context.Context, name string) (string, error) { + secret, ok := items[name] + if !ok { + return "", fmt.Errorf("no secret %s", name) + } + + return secret, nil + } +} diff --git a/services/todos/internal/jira/fail_test.go b/internal/jira/fail_test.go similarity index 97% rename from services/todos/internal/jira/fail_test.go rename to internal/jira/fail_test.go index 0656d67..69ab7d8 100644 --- a/services/todos/internal/jira/fail_test.go +++ b/internal/jira/fail_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" ) func TestSearchFailsOnABadAnswer(t *testing.T) { diff --git a/services/todos/internal/jira/fake_test.go b/internal/jira/fake_test.go similarity index 97% rename from services/todos/internal/jira/fake_test.go rename to internal/jira/fake_test.go index 6aa4b72..3945d91 100644 --- a/services/todos/internal/jira/fake_test.go +++ b/internal/jira/fake_test.go @@ -8,7 +8,7 @@ import ( "sync" "testing" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" ) const ( diff --git a/services/todos/internal/jira/fetch.go b/internal/jira/fetch.go similarity index 100% rename from services/todos/internal/jira/fetch.go rename to internal/jira/fetch.go diff --git a/services/todos/internal/jira/fetch_test.go b/internal/jira/fetch_test.go similarity index 97% rename from services/todos/internal/jira/fetch_test.go rename to internal/jira/fetch_test.go index 4cdd65c..0ec5f6e 100644 --- a/services/todos/internal/jira/fetch_test.go +++ b/internal/jira/fetch_test.go @@ -8,7 +8,7 @@ import ( "slices" "testing" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" ) func TestFetchAsksForTheTickets(t *testing.T) { diff --git a/services/todos/internal/jira/fetchfail_test.go b/internal/jira/fetchfail_test.go similarity index 98% rename from services/todos/internal/jira/fetchfail_test.go rename to internal/jira/fetchfail_test.go index ab14536..35cf102 100644 --- a/services/todos/internal/jira/fetchfail_test.go +++ b/internal/jira/fetchfail_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" ) func TestFetchFailsOnABadAnswer(t *testing.T) { diff --git a/services/todos/internal/jira/ticket.go b/internal/jira/ticket.go similarity index 100% rename from services/todos/internal/jira/ticket.go rename to internal/jira/ticket.go diff --git a/services/todos/internal/api/api.go b/services/todos/internal/api/api.go index 906669e..d029014 100644 --- a/services/todos/internal/api/api.go +++ b/services/todos/internal/api/api.go @@ -8,7 +8,7 @@ import ( "github.com/go-chi/chi/v5" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" "github.com/codedogapp/bakhyard/services/todos/internal/todo" ) @@ -19,14 +19,14 @@ type list struct { // Register serves the todos in store on router: GET /todos lists them, POST /todos adds one, // PATCH /todos/{id} changes one and DELETE /todos/{id} deletes one. GET /todos/tickets answers -// what tickets has found in Jira, nil when Jira isn't set up. GET /todos/dismissals lists the +// what poller has found in Jira, nil when Jira isn't set up. GET /todos/dismissals lists the // tickets dismissed from the inbox, and PUT /todos/dismissals/{key} dismisses one. Other methods // get 405, and logger gets the errors that the database runs into. -func Register(router chi.Router, store *todo.Store, tickets *jira.Poller, logger *slog.Logger) { +func Register(router chi.Router, store *todo.Store, poller *tickets.Poller, logger *slog.Logger) { router.Get("/todos", listTodos(store, logger)) router.Post("/todos", addTodo(store, logger)) // Always there, so that chi doesn't take the path for /todos/{id}, which has no GET. - router.Get("/todos/tickets", listTickets(tickets)) + router.Get("/todos/tickets", listTickets(poller)) router.Get("/todos/dismissals", listDismissals(store, logger)) router.Put("/todos/dismissals/{key}", dismissTicket(store, logger)) router.Patch("/todos/{id}", changeTodo(store, logger)) diff --git a/services/todos/internal/api/tickets.go b/services/todos/internal/api/tickets.go index e24ac53..b5e38f2 100644 --- a/services/todos/internal/api/tickets.go +++ b/services/todos/internal/api/tickets.go @@ -3,12 +3,12 @@ package api import ( "net/http" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" ) // listTickets returns the handler of GET /todos/tickets, which answers what poller has found in // Jira, once its first poll has ended. Without Jira, poller is nil and the client gets a 404. -func listTickets(poller *jira.Poller) http.HandlerFunc { +func listTickets(poller *tickets.Poller) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if poller == nil { http.Error(w, "Jira is not set up", http.StatusNotFound) diff --git a/services/todos/internal/api/tickets_test.go b/services/todos/internal/api/tickets_test.go index bb53e9e..de70587 100644 --- a/services/todos/internal/api/tickets_test.go +++ b/services/todos/internal/api/tickets_test.go @@ -11,13 +11,14 @@ import ( "github.com/go-chi/chi/v5" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/services/todos/internal/api" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" ) // A bubble's clock starts at midnight UTC on 1 January 2000, when the poller searches first. func TestGetTicketsAnswersWhatThePollerFound(t *testing.T) { - tickets := []jira.Ticket{ + open := []jira.Ticket{ { Key: "PAY-2", Summary: `Answer "Re: "`, Status: "In Review", Category: jira.InProgress, URL: "https://acme.example/browse/PAY-2", @@ -40,7 +41,7 @@ func TestGetTicketsAnswersWhatThePollerFound(t *testing.T) { want string }{ { - tickets, nil, nil, + open, nil, nil, `{"tickets":[{"key":"PAY-2","summary":"Answer \"Re: \"",` + `"status":"In Review","category":"in-progress",` + `"url":"https://acme.example/browse/PAY-2"},` + @@ -98,7 +99,7 @@ func withJira( fake := fakeJira{search: search, done: done} - poller := jira.NewPoller(fake, fake.linked, slog.New(slog.DiscardHandler)) + poller := tickets.NewPoller(fake, fake.linked, slog.New(slog.DiscardHandler)) go poller.Run(t.Context()) router := chi.NewRouter() diff --git a/services/todos/internal/attention/attention.go b/services/todos/internal/attention/attention.go index 3f4cbda..dd8f192 100644 --- a/services/todos/internal/attention/attention.go +++ b/services/todos/internal/attention/attention.go @@ -8,7 +8,8 @@ import ( "strconv" attentionv1 "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" "github.com/codedogapp/bakhyard/services/todos/internal/todo" ) @@ -17,18 +18,18 @@ import ( type Lister struct { list func(context.Context) ([]todo.Todo, error) dismissed func(context.Context) ([]string, error) - tickets *jira.Poller + poller *tickets.Poller } // NewLister returns a lister of the todos that list reads, such as a Store's List, and of the Jira // inbox, which leaves out the tickets that dismissed reads, such as a Store's Dismissed. It learns -// from tickets which tickets are open and which are done; tickets is nil when Jira isn't set up. +// from poller which tickets are open and which are done; poller is nil when Jira isn't set up. func NewLister( list func(context.Context) ([]todo.Todo, error), dismissed func(context.Context) ([]string, error), - tickets *jira.Poller, + poller *tickets.Poller, ) *Lister { - return &Lister{list: list, dismissed: dismissed, tickets: tickets} + return &Lister{list: list, dismissed: dismissed, poller: poller} } // ListItems lists the Jira inbox's tickets, then, in the list's order, the todos left to do that @@ -67,14 +68,14 @@ func (l *Lister) ListItems( // found returns what the latest Jira poll to succeed found, once the first poll has ended, and // nothing when Jira isn't set up. -func (l *Lister) found(ctx context.Context) (jira.Found, error) { - if l.tickets == nil { - return jira.Found{}, nil +func (l *Lister) found(ctx context.Context) (tickets.Found, error) { + if l.poller == nil { + return tickets.Found{}, nil } - latest, err := l.tickets.Latest(ctx) + latest, err := l.poller.Latest(ctx) if err != nil { - return jira.Found{}, err + return tickets.Found{}, err } return latest.Found, nil diff --git a/services/todos/internal/attention/attention_test.go b/services/todos/internal/attention/attention_test.go index 6afa508..da4e32c 100644 --- a/services/todos/internal/attention/attention_test.go +++ b/services/todos/internal/attention/attention_test.go @@ -10,8 +10,9 @@ import ( "testing/synctest" attentionv1 "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/services/todos/internal/attention" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" "github.com/codedogapp/bakhyard/services/todos/internal/todo" ) @@ -48,9 +49,9 @@ const ( func TestListItemsListsTheTodosThatNeedMe(t *testing.T) { synctest.Test(t, func(t *testing.T) { - tickets := started(t, fakeJira{done: []jira.Ticket{webNine}}) + poller := started(t, fakeJira{done: []jira.Ticket{webNine}}) - got := list(t, todosOf(someTodos...), unread(t), tickets) + got := list(t, todosOf(someTodos...), unread(t), poller) want := []string{callTheBank, fixTheLogin, shipTheExport, deployTheFix} if !slices.Equal(got, want) { @@ -61,9 +62,9 @@ func TestListItemsListsTheTodosThatNeedMe(t *testing.T) { func TestListItemsListsNoneWithoutTodos(t *testing.T) { synctest.Test(t, func(t *testing.T) { - tickets := started(t, fakeJira{done: []jira.Ticket{webNine}}) + poller := started(t, fakeJira{done: []jira.Ticket{webNine}}) - if got := list(t, todosOf(), unread(t), tickets); len(got) != 0 { + if got := list(t, todosOf(), unread(t), poller); len(got) != 0 { t.Errorf("got %q, want no items", got) } }) @@ -94,7 +95,7 @@ type call struct{} func TestListItemsReadsWithinTheCall(t *testing.T) { synctest.Test(t, func(t *testing.T) { - tickets := started(t, fakeJira{search: searching(payOne)}) + poller := started(t, fakeJira{search: searching(payOne)}) within := func(ctx context.Context, what string) { if ctx.Value(call{}) == nil { t.Errorf("read the %s outside the call's context", what) @@ -111,7 +112,7 @@ func TestListItemsReadsWithinTheCall(t *testing.T) { return nil, nil } - res, err := attention.NewLister(todos, dismissed, tickets).ListItems( + res, err := attention.NewLister(todos, dismissed, poller).ListItems( context.WithValue(t.Context(), call{}, true), &attentionv1.ListItemsRequest{}) if got := lines(res); err != nil || !slices.Equal(got, []string{payTheInvoice}) { t.Errorf("got %q and %v, want PAY-1's item", got, err) @@ -139,12 +140,12 @@ func (f fakeJira) Fetch(context.Context, []string) ([]jira.Ticket, error) { } // started returns a poller of finder, and runs it until the test ends. -func started(t *testing.T, finder jira.Finder) *jira.Poller { +func started(t *testing.T, finder tickets.Finder) *tickets.Poller { t.Helper() none := func(context.Context) ([]string, error) { return nil, nil } - poller := jira.NewPoller(finder, none, slog.New(slog.DiscardHandler)) + poller := tickets.NewPoller(finder, none, slog.New(slog.DiscardHandler)) go poller.Run(t.Context()) return poller @@ -156,16 +157,16 @@ func todosOf(todos ...todo.Todo) func(context.Context) ([]todo.Todo, error) { } // list returns the lines of the items that a lister of todos and of the dismissed tickets lists -// with what tickets has found. +// with what poller has found. func list( t *testing.T, todos func(context.Context) ([]todo.Todo, error), dismissed func(context.Context) ([]string, error), - tickets *jira.Poller, + poller *tickets.Poller, ) []string { t.Helper() - res, err := attention.NewLister(todos, dismissed, tickets).ListItems(t.Context(), + res, err := attention.NewLister(todos, dismissed, poller).ListItems(t.Context(), &attentionv1.ListItemsRequest{}) if err != nil { t.Fatal(err) diff --git a/services/todos/internal/attention/inbox.go b/services/todos/internal/attention/inbox.go index 7459af9..4936810 100644 --- a/services/todos/internal/attention/inbox.go +++ b/services/todos/internal/attention/inbox.go @@ -5,7 +5,7 @@ import ( "slices" attentionv1 "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/services/todos/internal/todo" ) diff --git a/services/todos/internal/attention/inbox_test.go b/services/todos/internal/attention/inbox_test.go index 8a06624..3dea44c 100644 --- a/services/todos/internal/attention/inbox_test.go +++ b/services/todos/internal/attention/inbox_test.go @@ -8,8 +8,8 @@ import ( "testing/synctest" attentionv1 "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/services/todos/internal/attention" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" ) // Open tickets of the user's: PAY-1 and DOC-5, which no todo names, WEB-7, which a todo left to do diff --git a/services/todos/internal/attention/polls_test.go b/services/todos/internal/attention/polls_test.go index 77be85c..bc770df 100644 --- a/services/todos/internal/attention/polls_test.go +++ b/services/todos/internal/attention/polls_test.go @@ -10,8 +10,8 @@ import ( "time" attentionv1 "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/services/todos/internal/attention" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" "github.com/codedogapp/bakhyard/services/todos/internal/todo" ) diff --git a/services/todos/internal/jira/done_test.go b/services/todos/internal/tickets/done_test.go similarity index 98% rename from services/todos/internal/jira/done_test.go rename to services/todos/internal/tickets/done_test.go index 975c36a..12ce965 100644 --- a/services/todos/internal/jira/done_test.go +++ b/services/todos/internal/tickets/done_test.go @@ -1,4 +1,4 @@ -package jira_test +package tickets_test import ( "context" @@ -10,7 +10,7 @@ import ( "testing/synctest" "time" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" ) // webThreeJSON is webThree as JSON. diff --git a/services/todos/internal/jira/finder_test.go b/services/todos/internal/tickets/finder_test.go similarity index 79% rename from services/todos/internal/jira/finder_test.go rename to services/todos/internal/tickets/finder_test.go index 2f65c5c..cbdb539 100644 --- a/services/todos/internal/jira/finder_test.go +++ b/services/todos/internal/tickets/finder_test.go @@ -1,4 +1,4 @@ -package jira_test +package tickets_test import ( "bytes" @@ -10,7 +10,8 @@ import ( "sync/atomic" "testing" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" ) // reply is how a call of a fake search replies. @@ -67,10 +68,10 @@ func (f *fakeJira) fetched() []string { return f.asked } -// found replies with tickets, which are an empty list when there are none, as a Client's are. -func found(tickets ...jira.Ticket) reply { +// found replies with the open tickets, an empty list when there are none, as a Client's are. +func found(open ...jira.Ticket) reply { return func(context.Context) ([]jira.Ticket, error) { - return append([]jira.Ticket{}, tickets...), nil + return append([]jira.Ticket{}, open...), nil } } @@ -80,7 +81,7 @@ func failing(message string) reply { // polling starts a poller, which searches as the replies say until ctx ends, and returns it, its // fake Jira and its logs. The todos name no tickets. -func polling(ctx context.Context, replies ...reply) (*jira.Poller, *fakeJira, *bytes.Buffer) { +func polling(ctx context.Context, replies ...reply) (*tickets.Poller, *fakeJira, *bytes.Buffer) { fake := &fakeJira{replies: replies} poller, logs := pollingWith(ctx, fake) @@ -88,17 +89,17 @@ func polling(ctx context.Context, replies ...reply) (*jira.Poller, *fakeJira, *b } // pollingWith starts a poller on fake until ctx ends, and returns it and its logs. -func pollingWith(ctx context.Context, fake *fakeJira) (*jira.Poller, *bytes.Buffer) { +func pollingWith(ctx context.Context, fake *fakeJira) (*tickets.Poller, *bytes.Buffer) { var logs bytes.Buffer - poller := jira.NewPoller(fake, fake.linked, slog.New(slog.NewTextHandler(&logs, nil))) + poller := tickets.NewPoller(fake, fake.linked, slog.New(slog.NewTextHandler(&logs, nil))) go poller.Run(ctx) return poller, &logs } // latest returns what poller has found, as JSON. -func latest(t *testing.T, poller *jira.Poller) string { +func latest(t *testing.T, poller *tickets.Poller) string { t.Helper() snapshot, err := poller.Latest(t.Context()) diff --git a/services/todos/internal/jira/poll.go b/services/todos/internal/tickets/poll.go similarity index 77% rename from services/todos/internal/jira/poll.go rename to services/todos/internal/tickets/poll.go index 5040aaf..a13da12 100644 --- a/services/todos/internal/jira/poll.go +++ b/services/todos/internal/tickets/poll.go @@ -1,4 +1,6 @@ -package jira +// Package tickets polls Jira for the user's open tickets and for the done tickets that the todos +// left to do name (decisions 122 and 133). +package tickets import ( "context" @@ -6,6 +8,7 @@ import ( "slices" "time" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/internal/poll" ) @@ -15,19 +18,19 @@ const pollInterval = 5 * time.Minute // Finder finds tickets on Jira, as a Client does. type Finder interface { // Search returns the user's open tickets. - Search(ctx context.Context) ([]Ticket, error) + Search(ctx context.Context) ([]jira.Ticket, error) // Fetch returns the tickets that keys name, whatever their status. - Fetch(ctx context.Context, keys []string) ([]Ticket, error) + Fetch(ctx context.Context, keys []string) ([]jira.Ticket, error) } // Found is what a poll of Jira finds. type Found struct { // Tickets are the user's open tickets. They're nil, and left out of the JSON, until a poll has // succeeded; omitempty would leave out an empty list too. - Tickets []Ticket `json:"tickets,omitzero"` + Tickets []jira.Ticket `json:"tickets,omitzero"` // Done are the tickets that the todos left to do name and that are done (decision 133), left // out of the JSON when there are none. - Done []Ticket `json:"done,omitempty"` + Done []jira.Ticket `json:"done,omitempty"` } // Poller polls Jira in the background and keeps the latest of what it finds (decisions 122 and @@ -62,7 +65,7 @@ func lookUp( } keys = slices.DeleteFunc(keys, func(key string) bool { - return slices.ContainsFunc(open, func(ticket Ticket) bool { return ticket.Key == key }) + return slices.ContainsFunc(open, func(ticket jira.Ticket) bool { return ticket.Key == key }) }) fetched, err := finder.Fetch(ctx, keys) @@ -70,7 +73,10 @@ func lookUp( return Found{}, err } - done := slices.DeleteFunc(fetched, func(ticket Ticket) bool { return ticket.Category != Done }) + done := slices.DeleteFunc( + fetched, + func(ticket jira.Ticket) bool { return ticket.Category != jira.Done }, + ) return Found{Tickets: open, Done: done}, nil } diff --git a/services/todos/internal/jira/poll_test.go b/services/todos/internal/tickets/poll_test.go similarity index 98% rename from services/todos/internal/jira/poll_test.go rename to services/todos/internal/tickets/poll_test.go index 38070c5..ad6b7dc 100644 --- a/services/todos/internal/jira/poll_test.go +++ b/services/todos/internal/tickets/poll_test.go @@ -1,4 +1,4 @@ -package jira_test +package tickets_test import ( "strings" diff --git a/services/todos/jira.go b/services/todos/jira.go index 4babbf4..265f948 100644 --- a/services/todos/jira.go +++ b/services/todos/jira.go @@ -2,69 +2,24 @@ package main import ( "context" - "fmt" "log/slog" - "slices" - "github.com/codedogapp/bakhyard/internal/env" - "github.com/codedogapp/bakhyard/internal/secrets" - "github.com/codedogapp/bakhyard/services/todos/internal/jira" + "github.com/codedogapp/bakhyard/internal/jira" + "github.com/codedogapp/bakhyard/services/todos/internal/tickets" "github.com/codedogapp/bakhyard/services/todos/internal/todo" ) -// The settings of the Jira site whose tickets the service lists (docs/setup.md). Jira is off -// when none is set. -const ( - jiraSiteVar = "BAKHYARD_JIRA_SITE" - jiraAPIVar = "BAKHYARD_JIRA_API" - jiraEmailVar = "BAKHYARD_JIRA_EMAIL" -) - -// jiraAccount is the Keychain account of the Jira API token, and the name of its file. -const jiraAccount = "jira-token" - -// jiraClient returns the client of the Jira site in the settings, with the API token that -// readSecret reads, or nil when none of the Jira settings is set. -func jiraClient(getenv func(string) string, readSecret secrets.Reader) (*jira.Client, error) { - settings := []string{jiraSiteVar, jiraAPIVar, jiraEmailVar} - if !slices.ContainsFunc(settings, func(name string) bool { return getenv(name) != "" }) { - return nil, nil - } - - site, err := env.SecureURL(getenv, jiraSiteVar) - if err != nil { - return nil, err - } - - address, err := env.SecureURL(getenv, jiraAPIVar) - if err != nil { - return nil, err - } - - email, err := env.Required(getenv, jiraEmailVar) - if err != nil { - return nil, err - } - - token, err := readSecret(context.Background(), jiraAccount) - if err != nil { - return nil, fmt.Errorf("read the Jira token: %w", err) - } - - return jira.New(jira.Settings{Site: site, API: address, Email: email, Token: token}), nil -} - // poll starts polling Jira with client for the user's open tickets and the tickets that store's // todos name, until ctx ends, and returns the poller that keeps what the polls find, or nil // when Jira is off and client is nil. func poll( ctx context.Context, client *jira.Client, store *todo.Store, logger *slog.Logger, -) *jira.Poller { +) *tickets.Poller { if client == nil { return nil } - poller := jira.NewPoller(client, store.Linked, logger) + poller := tickets.NewPoller(client, store.Linked, logger) go poller.Run(ctx) return poller diff --git a/services/todos/jira_test.go b/services/todos/jira_test.go index 255baad..960f2f3 100644 --- a/services/todos/jira_test.go +++ b/services/todos/jira_test.go @@ -17,55 +17,6 @@ const ( jiraAPI = "https://api.atlassian.com/ex/jira/cloud-id" ) -func TestJiraClientSearchesTheJiraInTheSettings(t *testing.T) { - client, err := jiraClient(envOf(jiraSettings(jiraServer(t))), - secretsWith(map[string]string{"jira-token": "secret"})) - if err != nil { - t.Fatal(err) - } - - got, err := client.Search(t.Context()) - if err != nil || len(got) != 1 || got[0].URL != jiraSite+"/browse/PAY-1" { - t.Errorf("got %v, %v, want PAY-1 on %s", got, err, jiraSite) - } -} - -func TestJiraClientIsOffWithoutTheSettings(t *testing.T) { - client, err := jiraClient(envOf(nil), secretsWith(nil)) - if client != nil || err != nil { - t.Errorf("got %v, %v, want no client and no error", client, err) - } -} - -func TestJiraClientRejects(t *testing.T) { - for _, test := range []rejection{ - {map[string]string{jiraSiteVar: jiraSite}, "BAKHYARD_JIRA_API is not set"}, - {map[string]string{jiraAPIVar: jiraAPI}, "BAKHYARD_JIRA_SITE is not set"}, - {map[string]string{jiraEmailVar: "me@example.com"}, "BAKHYARD_JIRA_SITE is not set"}, - {jiraWith(jiraSiteVar, ""), "BAKHYARD_JIRA_SITE is not set"}, - {jiraWith(jiraAPIVar, ""), "BAKHYARD_JIRA_API is not set"}, - {jiraWith(jiraEmailVar, ""), "BAKHYARD_JIRA_EMAIL is not set"}, - // internal/env tests which URLs pass; these show that both of Jira's URLs must. - notHTTPS(jiraSiteVar, "http://acme.atlassian.net"), - notHTTPS(jiraAPIVar, "http://example.com"), - } { - client, err := jiraClient(envOf(test.settings), - secretsWith(map[string]string{"jira-token": "secret"})) - if client != nil || err == nil || err.Error() != test.want { - t.Errorf("got %v, %v, want the error %s", client, err, test.want) - } - } -} - -func TestJiraClientNeedsTheToken(t *testing.T) { - client, err := jiraClient(envOf(jiraSettings(jiraAPI)), secretsWith(nil)) - - want := "read the Jira token: no secret jira-token" - if client != nil || err == nil || err.Error() != want { - t.Errorf("got %v, %v, want the error %s", client, err, want) - } -} - func TestTodosListsTheJiraTickets(t *testing.T) { rec := getTickets(t.Context(), t, jiraServer(t)) if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"key":"PAY-1"`) || @@ -93,7 +44,7 @@ func TestTodosStopsSearchingJiraWhenItStops(t *testing.T) { } func TestTodosStopsOnABadJiraSetting(t *testing.T) { - _, err := routes(t.Context(), envOf(jiraWith(jiraEmailVar, "")), secretsWith(nil)) + _, err := routes(t.Context(), envOf(jiraWith("BAKHYARD_JIRA_EMAIL", "")), secretsWith(nil)) if err == nil || err.Error() != "BAKHYARD_JIRA_EMAIL is not set" { t.Errorf("got %v, want the error BAKHYARD_JIRA_EMAIL is not set", err) } @@ -192,7 +143,8 @@ func addTodo(t *testing.T, getenv func(string) string, readSecret secrets.Reader // jiraSettings returns the Jira settings, with the REST API at address. func jiraSettings(address string) map[string]string { return map[string]string{ - jiraSiteVar: jiraSite, jiraAPIVar: address, jiraEmailVar: "me@example.com", + "BAKHYARD_JIRA_SITE": jiraSite, "BAKHYARD_JIRA_API": address, + "BAKHYARD_JIRA_EMAIL": "me@example.com", } } @@ -204,17 +156,6 @@ func jiraWith(name, value string) map[string]string { return settings } -// rejection is a row of TestJiraClientRejects: the settings, and the error they give. -type rejection struct { - settings map[string]string - want string -} - -// notHTTPS returns the rejection of value in the setting name as not an https URL. -func notHTTPS(name, value string) rejection { - return rejection{jiraWith(name, value), name + `: "` + value + `" is not an https URL`} -} - // envOf returns an environment that holds only settings. func envOf(settings map[string]string) func(string) string { return func(name string) string { return settings[name] } diff --git a/services/todos/main.go b/services/todos/main.go index d2524ed..c58f254 100644 --- a/services/todos/main.go +++ b/services/todos/main.go @@ -16,6 +16,7 @@ import ( "github.com/codedogapp/bakhyard/internal/env" "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1/attentionv1connect" "github.com/codedogapp/bakhyard/internal/health" + "github.com/codedogapp/bakhyard/internal/jira" "github.com/codedogapp/bakhyard/internal/keychain" "github.com/codedogapp/bakhyard/internal/secrets" "github.com/codedogapp/bakhyard/internal/service" @@ -52,7 +53,7 @@ func routes( getenv func(string) string, readSecret secrets.Reader, ) (http.Handler, error) { - client, err := jiraClient(getenv, readSecret) + client, err := jira.FromEnv(getenv, readSecret) if err != nil { return nil, err }