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 }