diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md deleted file mode 100644 index b6570be..0000000 --- a/.github/copilot-instructions.md +++ /dev/null @@ -1,102 +0,0 @@ -# Copilot Instructions - -## Project Overview - -**Thoughts** is a self-hosted, real-time collaborative retrospective tool. It's a full-stack app with a Go backend and a React/TypeScript frontend. The backend serves the bundled UI in production but runs separately during development. - -## Commands - -### Backend - -```bash -task dev # Hot-reload dev server (uses Air, watches *.go + migrations/) -task build # Production build — bundles UI first, then compiles with -tags bundled -task run # Build + run production binary -``` - -### Frontend - -```bash -cd ui -pnpm install -pnpm run dev # Vite dev server on :5173, proxies /api/* to :3000 -pnpm run build # Produces ui/dist/ (required before task build) -pnpm run lint -``` - -### Database migrations - -Migrations run automatically on startup via Goose. To manage manually: - -```bash -task tools # Installs goose + goreleaser -``` - -Migration files live in `migrations/*.sql` using `-- +goose Up` / `-- +goose Down` directives. They are embedded into the binary via `//go:embed` in `migrations/migrations.go`. - -### Docker - -```bash -task docker-build -task docker-run -``` - -## Architecture - -``` -cmd/thoughts/ -├── main.go # Server setup, DB init, migration runner -├── config.go # Viper config — all env vars defined here -├── routes.go # All route registration (applyRoutes) -├── controllers/ # HTTP handlers -├── model/ # Entity structs (Retro, Note, Vote, Task, User) -├── dal/ # Data Access Layer — raw sqlx queries, no ORM -├── resources/ # Response DTOs (JSON serialization) -├── requests/ # Request DTOs with go-playground/validator tags -├── event/ # Pub/sub broker for WebSocket events -├── socket/ # WebSocket upgrade + message dispatch -├── session/ # Gorilla session management -├── auth/ # Auth middleware (checks session cookie) -├── ai/ # OpenAI integration via langchaingo -└── gif/ # Tenor API integration -``` - -**Request flow:** HTTP → `routes.go` → auth middleware → controller → DAL → SQLite. Real-time updates flow via `event.Broker` → WebSocket → frontend. - -**Production build:** `task build-ui` generates `ui/dist/`, then `go build -tags bundled` embeds the dist into the binary and serves it as a static filesystem. - -## Key Conventions - -### Configuration - -All config is via environment variables with the `THOUGHTS_` prefix, managed by Viper. Defaults are set in `config.go`. Key variables: - -| Variable | Default | Notes | -|---|---|---| -| `THOUGHTS_ADDRESS` | `localhost:3000` | HTTP listen address | -| `THOUGHTS_DATA` | `./data` | SQLite + session key location | -| `THOUGHTS_OPENAI_API_KEY` | _(unset)_ | Enables AI template generation | -| `THOUGHTS_TENOR_API_KEY` | _(unset)_ | Enables GIF search | -| `THOUGHTS_TLS_CERT_PATH` / `THOUGHTS_TLS_KEY_PATH` | _(unset)_ | Optional TLS | - -### Layered request/response pattern - -- **Requests** (`requests/`) — bind and validate incoming JSON using `go-playground/validator` struct tags -- **Model** — internal entities used in DAL and business logic -- **Resources** (`resources/`) — outbound DTOs; never expose model structs directly in responses - -### DAL - -No ORM. Use `sqlx` directly. Queries are inline SQL strings. Follow the existing pattern of one file per entity (e.g., `dal/retros.go`, `dal/notes.go`). - -### Real-time events - -When a mutation should notify other clients in a retro session, publish to `event.Broker` after the DAL write. The event payload is `map[string]any`. Event name constants are defined in `event/`. The WebSocket handler in `socket/` routes incoming messages and the broker fan-outs to all connected clients for that retro. - -### Authentication - -Session-based (Gorilla Sessions, cookie-backed). The session key is auto-generated and persisted to `data/session.key` on first run. Auth is name-only — no passwords. The `auth.Middleware()` in `routes.go` wraps all `/api/*` routes except `/api/auth/`. - -### Frontend routing - -TanStack Router with file-based routes under `ui/src/routes/`. API calls use `axios`. WebSocket connection managed by `react-use-websocket`. Global state (auth, theme) via React Context in `ui/src/hooks/`. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..12bdcfc --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,68 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + go: + name: Go + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + - name: Build + run: go build ./... + + - name: Vet + run: go vet ./... + + - name: Test + run: go test -race ./... + + ui: + name: UI + runs-on: ubuntu-latest + defaults: + run: + working-directory: ui + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + with: + package_json_file: ui/package.json + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + cache-dependency-path: ui/pnpm-lock.yaml + + - name: Install + run: pnpm install --frozen-lockfile + + - name: Lint + run: pnpm lint + + - name: Typecheck + run: pnpm exec tsc -b + + - name: Test + run: pnpm test + + - name: Build + run: pnpm build diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..8c99578 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,130 @@ +# CLAUDE.md + +Thoughts is a self-hosted, real-time collaborative retrospective tool: a Go +backend serving a React/TypeScript SPA. In production the binary embeds the +built UI; in development the two run separately. + +## Commands + +```bash +task dev # Hot-reload backend (Air, watches *.go + migrations/) +task build # Bundles the UI, then compiles with -tags bundled +task run # Build and run the production binary +task tools # Installs goose + goreleaser +``` + +```bash +cd ui +pnpm run dev # Vite on :5173, proxying /api/* to :3000 +pnpm run build +pnpm run lint +pnpm test +``` + +Before saying a change is done: + +```bash +go build ./... && go vet ./... && go test ./... +cd ui && pnpm lint && pnpm exec tsc -b && pnpm test && pnpm build +``` + +## Architecture + +``` +cmd/thoughts/ +├── main.go Server setup, DB init, migration runner +├── config.go Viper config — every env var is defined here +├── routes.go All route registration (applyRoutes) +├── controllers/ HTTP handlers +├── model/ Entity structs (Retro, Note, Vote, Task, User) +├── dal/ Raw sqlx queries, no ORM, one file per entity +├── resources/ Outbound DTOs +├── requests/ Inbound DTOs with go-playground/validator tags +├── event/ Pub/sub broker for WebSocket events +├── socket/ WebSocket upgrade and message dispatch +├── session/ Gorilla session management +├── auth/ Session cookie middleware +├── ai/ OpenAI via langchaingo +└── gif/ GIF search behind a swappable provider +``` + +HTTP requests flow routes → auth middleware → controller → DAL → SQLite. +Real-time updates flow `event.Broker` → WebSocket → frontend. + +Migrations live in `migrations/*.sql` with `-- +goose Up` / `-- +goose Down`, +are embedded with `//go:embed`, and run automatically on startup. + +Frontend routing is TanStack Router, file-based under `ui/src/routes/`. HTTP is +axios, the socket is react-use-websocket, shared state is React context in +`ui/src/hooks/`. + +## Conventions + +Config is environment variables with a `THOUGHTS_` prefix, defaulted in +`config.go`. `THOUGHTS_GIF_API_KEY` and `THOUGHTS_GIF_PROVIDER` control GIF +search; pasting a link works without either. `THOUGHTS_OPENAI_API_KEY` enables +AI template generation. + +Never return a model struct from a handler; map it through `resources/`. + +When a mutation should reach other people in the retro, publish to +`event.Broker` after the DAL write. Payloads are `map[string]any` and are +decoded by `requests.FromMap`, which handles only flat scalars — no nested +structs or slices of structs. + +Auth is name-only, no passwords. **A name is a label, not an account.** Each +login creates a new user row, so two people called Alex are two people, and one +person entering their name twice gets two sessions. Do not add uniqueness to +`users.name` or try to reuse a row by name. + +Migrations are append-only once merged. A migration that changes or deletes +existing rows needs asking about first. + +## Writing code here + +Match the surrounding code. Prefer clear naming and small functions over +explanation. + +### Comments + +**The default is no comment.** Code that needs prose to be understood should be +rewritten instead. Most functions, types, props and exported symbols in this +repo have no comment, and that is correct — do not "improve" them by adding +one. + +A comment has to earn its place by carrying information that is *not in the +code and not inferable from it*. In practice that is almost always one of: + +- A library, browser or API quirk, named. *"popLayout clones each child with a + ref of its own, which used to overwrite this one."* +- Why a reasonable-looking alternative is wrong here. *"A transaction is the + wrong tool: sqlx issues a deferred BEGIN, so under WAL a concurrent writer + fails rather than serialising."* +- A value that must stay in step with something in another file. + +If you cannot state which of those a comment is, delete it. + +Never write a comment that: + +- Restates the code, the function name, or a type name in prose. +- Describes what a component renders or how a layout is arranged. +- Explains a design or styling choice nobody would question. +- Acts as a section header inside a function. +- Says what a test is testing when the test name already says it. + +One or two lines. A block comment longer than three lines needs a reason to +exist. Do not comment every branch of a switch, every field of a struct or +interface, or every step of a sequence. + +When editing existing code, leave surrounding comments alone unless they are +now wrong. + +## Verifying + +Run the app and check the change rather than assuming it works. Where a claim +cannot be verified — because the tooling cannot drive the interaction, or no key +is available — say so plainly rather than implying it was tested. + +A test written from documentation proves the code agrees with the documentation, +not with reality. Where an external API is involved, shape fixtures from a real +response and keep a live contract test behind an env var. diff --git a/Dockerfile b/Dockerfile index e730c85..81b4033 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,11 +18,18 @@ RUN wget -qO /app/thoughts.tar.gz "https://github.com/ellgreen/thoughts/releases FROM alpine:latest ENV THOUGHTS_ADDRESS=":3000" -ENV THOUGHTS_DATA="/data" +# The config key is data_path, so THOUGHTS_DATA was never read and the database +# landed in the container's working directory instead of the mounted volume. +ENV THOUGHTS_DATA_PATH="/data" ENV THOUGHTS_TLS_CERT_PATH="" ENV THOUGHTS_TLS_KEY_PATH="" +# Optional integrations, pass with -e to enable. +ENV THOUGHTS_GIF_PROVIDER="" +ENV THOUGHTS_GIF_API_KEY="" +ENV THOUGHTS_OPENAI_API_KEY="" + COPY --from=setup /thoughts /usr/local/bin/ RUN mkdir -p /data diff --git a/README.md b/README.md index 2511f86..71276f0 100644 --- a/README.md +++ b/README.md @@ -57,11 +57,28 @@ You can follow the instructions to get an API key here: -#### GIF Support +#### Images and GIFs -To enable support for gifs via Tenor, you will need to set the -`THOUGHTS_TENOR_API_KEY` environment variable. +Adding an image to a thought always works — paste any `https` image or GIF +link and you get a preview before it is attached. No configuration needed. -You can follow the instructions to get an API key here: +To also get in-app GIF **search**, set `THOUGHTS_GIF_API_KEY`. The default +provider is [Klipy](https://klipy.com/developers), which is free for life: +a test key works immediately, and a production key (a form in their partner +panel) lifts the rate limit. - +```shell +docker run -p 3000:3000 \ + -v $PWD/data:/data \ + -e THOUGHTS_GIF_API_KEY="your-key" \ + ghcr.io/ellgreen/thoughts:latest +``` + +| Variable | Values | Default | +| --- | --- | --- | +| `THOUGHTS_GIF_API_KEY` | Your provider's API key | unset — search disabled | +| `THOUGHTS_GIF_PROVIDER` | `klipy`, `giphy`, `none` | auto: `klipy` when a key is set | + +> [!NOTE] +> Google shut the Tenor API down on 30 June 2026, so `THOUGHTS_TENOR_API_KEY` +> no longer does anything. Thoughts logs a warning if it is still set. diff --git a/cmd/thoughts/ai/prompts/retro_template.go b/cmd/thoughts/ai/prompts/retro_template.go index 26ed0e7..bf4f5f9 100644 --- a/cmd/thoughts/ai/prompts/retro_template.go +++ b/cmd/thoughts/ai/prompts/retro_template.go @@ -8,6 +8,11 @@ import ( "github.com/tmc/langchaingo/llms" ) +const ( + maxColumns = 5 + maxFieldLength = 255 +) + const retroTemplateSystemPrompt = `You are a backend service that generates JSON-only retrospective board templates. You MUST respond with a single JSON object matching exactly this schema: @@ -79,7 +84,9 @@ func GenerateRetroTemplate(ctx context.Context, model ai.Model, userPrompt strin }, llms.WithJSONMode(), llms.WithTemperature(0.9), - llms.WithMaxTokens(250), + // Five columns with descriptions do not fit in 250, and a truncated + // response is invalid JSON. + llms.WithMaxTokens(700), ) if err != nil { return RetroTemplateResponse{}, fmt.Errorf("failed to generate content: %w", err) @@ -90,5 +97,29 @@ func GenerateRetroTemplate(ctx context.Context, model ai.Model, userPrompt strin return RetroTemplateResponse{}, fmt.Errorf("failed to parse response: %w", err) } - return resp, nil + return clamp(resp), nil +} + +func clamp(resp RetroTemplateResponse) RetroTemplateResponse { + if len(resp.Columns) > maxColumns { + resp.Columns = resp.Columns[:maxColumns] + } + + resp.Theme = truncate(resp.Theme, maxFieldLength) + + for i := range resp.Columns { + resp.Columns[i].Title = truncate(resp.Columns[i].Title, maxFieldLength) + resp.Columns[i].Description = truncate(resp.Columns[i].Description, maxFieldLength) + } + + return resp +} + +func truncate(value string, limit int) string { + runes := []rune(value) + if len(runes) <= limit { + return value + } + + return string(runes[:limit]) } diff --git a/cmd/thoughts/ai/prompts/retro_template_test.go b/cmd/thoughts/ai/prompts/retro_template_test.go new file mode 100644 index 0000000..435d13e --- /dev/null +++ b/cmd/thoughts/ai/prompts/retro_template_test.go @@ -0,0 +1,89 @@ +package prompts + +import ( + "strings" + "testing" +) + +func column(title, description string) struct { + Title string `json:"title"` + Description string `json:"description"` +} { + return struct { + Title string `json:"title"` + Description string `json:"description"` + }{Title: title, Description: description} +} + +func TestClampTrimsExtraColumns(t *testing.T) { + resp := RetroTemplateResponse{Theme: "Star Wars"} + + for range 8 { + resp.Columns = append(resp.Columns, column("A column ✨", "Something")) + } + + got := clamp(resp) + + if len(got.Columns) != maxColumns { + t.Errorf("got %d columns, want %d", len(got.Columns), maxColumns) + } +} + +func TestClampLeavesAGoodResponseAlone(t *testing.T) { + resp := RetroTemplateResponse{ + Theme: "Star Wars", + Columns: []struct { + Title string `json:"title"` + Description string `json:"description"` + }{ + column("Light Side Wins ✨", "What went well this sprint?"), + column("Dark Side Risks 🌑", "What is threatening us?"), + }, + } + + got := clamp(resp) + + if len(got.Columns) != 2 || got.Columns[0].Title != "Light Side Wins ✨" { + t.Errorf("a valid response was altered: %+v", got) + } + + if got.Theme != "Star Wars" { + t.Errorf("theme changed to %q", got.Theme) + } +} + +func TestClampTruncatesOverlongFields(t *testing.T) { + long := strings.Repeat("a", 400) + + got := clamp(RetroTemplateResponse{ + Theme: long, + Columns: []struct { + Title string `json:"title"` + Description string `json:"description"` + }{column(long, long)}, + }) + + if len([]rune(got.Theme)) != maxFieldLength { + t.Errorf("theme is %d runes, want %d", len([]rune(got.Theme)), maxFieldLength) + } + + if len([]rune(got.Columns[0].Title)) != maxFieldLength { + t.Errorf("title is %d runes, want %d", len([]rune(got.Columns[0].Title)), maxFieldLength) + } + + if len([]rune(got.Columns[0].Description)) != maxFieldLength { + t.Errorf("description is %d runes, want %d", len([]rune(got.Columns[0].Description)), maxFieldLength) + } +} + +func TestTruncateCountsRunesNotBytes(t *testing.T) { + got := truncate(strings.Repeat("🚀", 10), 4) + + if len([]rune(got)) != 4 { + t.Errorf("got %d runes, want 4", len([]rune(got))) + } + + if !strings.HasSuffix(got, "🚀") { + t.Errorf("truncation split a rune: %q", got) + } +} diff --git a/cmd/thoughts/auth/auth.go b/cmd/thoughts/auth/auth.go index 50f4d76..d49dfc3 100644 --- a/cmd/thoughts/auth/auth.go +++ b/cmd/thoughts/auth/auth.go @@ -27,19 +27,28 @@ func Middleware(db *sqlx.DB, sp *session.Provider) mux.MiddlewareFunc { return } + // Whether the browser sent a cookie at all is the useful + // thing here: none means it was never stored, one without a + // user means it failed to decode. + rejected(r, "session carries no user", "cookie_sent", hasSessionCookie(r)) w.WriteHeader(http.StatusUnauthorized) + return } user, err := dal.UserGet(r.Context(), db, userID) if err != nil { if errors.Is(err, sql.ErrNoRows) { + rejected(r, "session names a user that no longer exists", "user_id", userID) w.WriteHeader(http.StatusUnauthorized) + return } slog.Error("failed to get user", "error", err) w.WriteHeader(http.StatusInternalServerError) + + return } r = RequestWithUser(r, user) @@ -49,6 +58,24 @@ func Middleware(db *sqlx.DB, sp *session.Provider) mux.MiddlewareFunc { }) } +func rejected(r *http.Request, reason string, args ...any) { + slog.Debug( + "rejecting unauthenticated request", + append([]any{ + "reason", reason, + "path", r.URL.Path, + "origin", r.Header.Get("Origin"), + "host", r.Host, + }, args...)..., + ) +} + +func hasSessionCookie(r *http.Request) bool { + _, err := r.Cookie("session") + + return err == nil +} + func RequestWithUser(r *http.Request, user *model.User) *http.Request { ctx := r.Context() diff --git a/cmd/thoughts/auth/auth_test.go b/cmd/thoughts/auth/auth_test.go new file mode 100644 index 0000000..ef705eb --- /dev/null +++ b/cmd/thoughts/auth/auth_test.go @@ -0,0 +1,136 @@ +package auth_test + +import ( + "context" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/auth" + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/session" + "github.com/ellgreen/thoughts/cmd/thoughts/testutil" + "github.com/google/uuid" + "github.com/jmoiron/sqlx" +) + +func sessionFor(t *testing.T, sp *session.Provider, userID uuid.UUID) *http.Request { + t.Helper() + + recorder := httptest.NewRecorder() + seed := httptest.NewRequest(http.MethodGet, "/api/retros", nil) + + if err := sp.AddUserID(recorder, seed, userID); err != nil { + t.Fatalf("failed to seed session: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/retros", nil) + for _, cookie := range recorder.Result().Cookies() { + req.AddCookie(cookie) + } + + return req +} + +func serve(t *testing.T, db *sqlx.DB, sp *session.Provider, req *http.Request) *httptest.ResponseRecorder { + t.Helper() + + reached := false + + handler := auth.Middleware(db, sp)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + + if user := auth.UserFromRequest(r); user == nil { + t.Error("handler reached with no user") + } + + w.WriteHeader(http.StatusOK) + })) + + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, req) + + if recorder.Code != http.StatusOK && reached { + t.Errorf("handler ran despite a %d response", recorder.Code) + } + + return recorder +} + +func newProvider(t *testing.T) *session.Provider { + t.Helper() + + sp, err := session.LoadSessionProvider(filepath.Join(t.TempDir(), "session.key"), false) + if err != nil { + t.Fatalf("failed to load session provider: %v", err) + } + + return sp +} + +func TestMiddlewareAllowsAKnownUser(t *testing.T) { + db := testutil.NewDB(t) + sp := newProvider(t) + + user, err := dal.UserInsert(context.Background(), db, "Ada") + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + if code := serve(t, db, sp, sessionFor(t, sp, user.ID)).Code; code != http.StatusOK { + t.Errorf("got %d, want 200", code) + } +} + +func TestMiddlewareRejectsASessionForAUserThatNoLongerExists(t *testing.T) { + db := testutil.NewDB(t) + sp := newProvider(t) + + code := serve(t, db, sp, sessionFor(t, sp, uuid.New())).Code + + if code != http.StatusUnauthorized { + t.Errorf("got %d, want 401", code) + } +} + +func TestMiddlewareRejectsARequestWithNoSession(t *testing.T) { + db := testutil.NewDB(t) + sp := newProvider(t) + + req := httptest.NewRequest(http.MethodGet, "/api/retros", nil) + + if code := serve(t, db, sp, req).Code; code != http.StatusUnauthorized { + t.Errorf("got %d, want 401", code) + } +} + +func TestMiddlewareRejectsAnUndecodableCookie(t *testing.T) { + db := testutil.NewDB(t) + sp := newProvider(t) + + other := newProvider(t) + req := sessionFor(t, other, uuid.New()) + + if code := serve(t, db, sp, req).Code; code != http.StatusUnauthorized { + t.Errorf("got %d, want 401", code) + } +} + +func TestMiddlewareStopsOnALookupFailure(t *testing.T) { + db := testutil.NewDB(t) + sp := newProvider(t) + + user, err := dal.UserInsert(context.Background(), db, "Ada") + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + req := sessionFor(t, sp, user.ID) + + db.Close() + + if code := serve(t, db, sp, req).Code; code != http.StatusInternalServerError { + t.Errorf("got %d, want 500", code) + } +} diff --git a/cmd/thoughts/config.go b/cmd/thoughts/config.go index 1d20719..5e06696 100644 --- a/cmd/thoughts/config.go +++ b/cmd/thoughts/config.go @@ -13,6 +13,8 @@ type config struct { SessionKeyFile string `mapstructure:"session_key_file"` TLSKeyPath string `mapstructure:"tls_key_path"` TLSCertPath string `mapstructure:"tls_cert_path"` + GIFProvider string `mapstructure:"gif_provider"` + GIFAPIKey string `mapstructure:"gif_api_key"` TenorAPIKey string `mapstructure:"tenor_api_key"` OpenAIAPIKey string `mapstructure:"openai_api_key"` } @@ -28,6 +30,8 @@ func loadConfig() (*config, error) { v.SetDefault("session_key_file", "session.key") v.SetDefault("tls_key_path", "") v.SetDefault("tls_cert_path", "") + v.SetDefault("gif_provider", "") + v.SetDefault("gif_api_key", "") v.SetDefault("tenor_api_key", "") v.SetDefault("openai_api_key", "") diff --git a/cmd/thoughts/controllers/ai.go b/cmd/thoughts/controllers/ai.go index 82f49d7..2ef0b1f 100644 --- a/cmd/thoughts/controllers/ai.go +++ b/cmd/thoughts/controllers/ai.go @@ -1,14 +1,18 @@ package controllers import ( + "context" "log/slog" "net/http" + "time" "github.com/ellgreen/thoughts/cmd/thoughts/ai" "github.com/ellgreen/thoughts/cmd/thoughts/ai/prompts" "github.com/ellgreen/thoughts/cmd/thoughts/requests" ) +const generateTimeout = 30 * time.Second + type PromptRequest struct { Prompt string `json:"prompt" validate:"required,min=2,max=128"` } @@ -26,7 +30,10 @@ func AIRetroTemplate(aiModel ai.Model) http.Handler { return } - resp, err := prompts.GenerateRetroTemplate(r.Context(), aiModel, req.Prompt) + ctx, cancel := context.WithTimeout(r.Context(), generateTimeout) + defer cancel() + + resp, err := prompts.GenerateRetroTemplate(ctx, aiModel, req.Prompt) if err != nil { slog.Error("failed to generate retro template", "err", err) http.Error(w, "failed to generate retro template", http.StatusInternalServerError) diff --git a/cmd/thoughts/controllers/auth.go b/cmd/thoughts/controllers/auth.go index 11baede..6dc8085 100644 --- a/cmd/thoughts/controllers/auth.go +++ b/cmd/thoughts/controllers/auth.go @@ -3,6 +3,9 @@ package controllers import ( "log/slog" "net/http" + "strings" + "unicode" + "unicode/utf8" "github.com/ellgreen/thoughts/cmd/thoughts/auth" "github.com/ellgreen/thoughts/cmd/thoughts/dal" @@ -13,7 +16,7 @@ import ( ) type AuthLoginRequest struct { - Name string `json:"name" validate:"required,alpha,min=2,max=20"` + Name string `json:"name" validate:"required,max=32"` } func AuthLogin(db *sqlx.DB, sessionProvider *session.Provider) http.Handler { @@ -23,20 +26,27 @@ func AuthLogin(db *sqlx.DB, sessionProvider *session.Provider) http.Handler { return } - if len(req.Name) < 1 { - http.Error(w, "name should be more than one character", http.StatusBadRequest) + // Names are free text - people have spaces, hyphens and accents in + // them. Only control characters are worth rejecting. + name := strings.TrimSpace(req.Name) + + if utf8.RuneCountInString(name) < 2 { + http.Error(w, "Name should contain at least 2 characters", http.StatusBadRequest) return } - user, err := dal.UserInsert(r.Context(), db, req.Name) + if strings.ContainsFunc(name, func(r rune) bool { return !unicode.IsPrint(r) }) { + http.Error(w, "Name should not contain control characters", http.StatusBadRequest) + return + } + + user, err := dal.UserInsert(r.Context(), db, name) if err != nil { slog.Error("failed to insert user", "error", err) w.WriteHeader(http.StatusInternalServerError) return } - slog.Info("user created", "user_id", user.ID, "name", user.Name) - if err := sessionProvider.AddUserID(w, r, user.ID); err != nil { slog.Error("failed to add user id to session", "error", err) w.WriteHeader(http.StatusInternalServerError) diff --git a/cmd/thoughts/controllers/gifs.go b/cmd/thoughts/controllers/gifs.go index c4c1551..a86fa82 100644 --- a/cmd/thoughts/controllers/gifs.go +++ b/cmd/thoughts/controllers/gifs.go @@ -1,32 +1,48 @@ package controllers import ( + "log/slog" "net/http" + "strconv" + "strings" "github.com/ellgreen/thoughts/cmd/thoughts/gif" - "github.com/ellgreen/thoughts/cmd/thoughts/requests" ) -type GifSearchRequest struct { - Query string `json:"q" validate:"required,min=2,max=32"` -} +// maxGifQuery is generous enough for a phrase and short enough that nobody can +// use the proxy to smuggle a payload upstream. +const maxGifQuery = 64 -func GifSearch(gifProvider gif.Provider) http.Handler { - if gifProvider == nil { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Error(w, "gifs not available", http.StatusServiceUnavailable) +func GifSearch(provider gif.Provider) http.Handler { + if provider == nil { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "gif search is not configured", http.StatusServiceUnavailable) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - req, ok := requests.From[GifSearchRequest](w, r) - if !ok { + query := strings.TrimSpace(r.URL.Query().Get("q")) + if len(query) > maxGifQuery { + http.Error(w, "search is too long", http.StatusBadRequest) return } - results, err := gifProvider.Search(r.Context(), req.Query) + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + + var ( + results *gif.SearchPage + err error + ) + + if query == "" { + results, err = provider.Trending(r.Context(), page) + } else { + results, err = provider.Search(r.Context(), query, page) + } + if err != nil { - http.Error(w, "failed to search for gifs", http.StatusInternalServerError) + slog.Error("gif search failed", "provider", provider.Name(), "error", err) + http.Error(w, "could not reach the gif service", http.StatusBadGateway) return } diff --git a/cmd/thoughts/controllers/retros.go b/cmd/thoughts/controllers/retros.go index ab13d28..5a9f2f7 100644 --- a/cmd/thoughts/controllers/retros.go +++ b/cmd/thoughts/controllers/retros.go @@ -62,7 +62,7 @@ type RetroCreateRequest struct { Columns []struct { Title string `json:"title" validate:"required,min=2,max=255"` Description string `json:"description" validate:"max=255"` - } `json:"columns" validate:"required,min=2,dive,required"` + } `json:"columns" validate:"required,min=2,max=5,dive,required"` Unlisted bool `json:"unlisted"` Tags []string `json:"tags" validate:"omitempty,max=10,dive,min=1,max=50"` } diff --git a/cmd/thoughts/controllers/tags.go b/cmd/thoughts/controllers/tags.go index d129d12..cd8f9db 100644 --- a/cmd/thoughts/controllers/tags.go +++ b/cmd/thoughts/controllers/tags.go @@ -54,14 +54,10 @@ func TagRetros(db *sqlx.DB) http.Handler { return } - for _, retro := range retros { - tags, err := dal.RetroTagsList(r.Context(), db, retro.ID) - if err != nil { - slog.Error("problem fetching tags for retro", "error", err) - w.WriteHeader(http.StatusInternalServerError) - return - } - retro.Tags = tags + if err := dal.RetroTagsAttach(r.Context(), db, retros); err != nil { + slog.Error("problem fetching tags for retros", "error", err) + w.WriteHeader(http.StatusInternalServerError) + return } openTasks, err := dal.TagOpenTasks(r.Context(), db, tag) diff --git a/cmd/thoughts/dal/note.go b/cmd/thoughts/dal/note.go index 97519e7..6845187 100644 --- a/cmd/thoughts/dal/note.go +++ b/cmd/thoughts/dal/note.go @@ -121,6 +121,21 @@ func NoteDelete(ctx context.Context, db *sqlx.DB, id uuid.UUID) error { return nil } +// NoteCountForColumn reports how many notes a column holds, which is what +// decides whether it may be deleted. +func NoteCountForColumn(ctx context.Context, db *sqlx.DB, retroID, columnID uuid.UUID) (int, error) { + var count int + + err := db.GetContext(ctx, &count, + "select count(*) from notes where retro_id = ? and column_id = ?", retroID, columnID) + + if err != nil { + return 0, fmt.Errorf("%w: failed to count notes for column: %w", ErrExecution, err) + } + + return count, nil +} + func NoteList( ctx context.Context, db *sqlx.DB, diff --git a/cmd/thoughts/dal/retro.go b/cmd/thoughts/dal/retro.go index 8df0da4..4b9981a 100644 --- a/cmd/thoughts/dal/retro.go +++ b/cmd/thoughts/dal/retro.go @@ -32,12 +32,8 @@ func RetroList(ctx context.Context, db *sqlx.DB, includeUnlisted bool) ([]*model return nil, fmt.Errorf("%w: failed to select retros: %w", ErrExecution, err) } - for _, retro := range retros { - tags, err := RetroTagsList(ctx, db, retro.ID) - if err != nil { - return nil, err - } - retro.Tags = tags + if err := RetroTagsAttach(ctx, db, retros); err != nil { + return nil, err } return retros, nil @@ -117,6 +113,27 @@ func RetroUpdate( return retro, nil } +// RetroSetColumns persists a mutated columns blob. It takes the model rather +// than an id because the caller has already read the retro to validate the +// change, and needs the same object to broadcast afterwards. +func RetroSetColumns(ctx context.Context, db *sqlx.DB, retro *model.Retro, columns model.RetroColumns) error { + retro.Columns = columns.ToJSON() + retro.UpdatedAt = time.Now() + + _, err := db.NamedExecContext(ctx, ` + update retros set + columns = :columns, + updated_at = :updated_at + where id = :id + `, retro) + + if err != nil { + return fmt.Errorf("%w: failed to update retro columns: %w", ErrExecution, err) + } + + return nil +} + func RetroUpdateStatus(ctx context.Context, db *sqlx.DB, id uuid.UUID, status model.RetroStatus) error { _, err := db.ExecContext(ctx, ` update retros set status = ?, updated_at = ? where id = ? diff --git a/cmd/thoughts/dal/tags.go b/cmd/thoughts/dal/tags.go index 71bb028..e2d33b1 100644 --- a/cmd/thoughts/dal/tags.go +++ b/cmd/thoughts/dal/tags.go @@ -17,6 +17,47 @@ func RetroTagsList(ctx context.Context, db *sqlx.DB, retroID uuid.UUID) ([]strin return tags, nil } +// RetroTagsAttach loads the tags for a batch of retros in one query and hangs +// them off the models, replacing a RetroTagsList call per retro. +func RetroTagsAttach(ctx context.Context, db *sqlx.DB, retros []*model.Retro) error { + if len(retros) == 0 { + return nil + } + + ids := make([]uuid.UUID, len(retros)) + for i, retro := range retros { + ids[i] = retro.ID + retro.Tags = []string{} + } + + query, args, err := sqlx.In("select retro_id, tag from retro_tags where retro_id in (?) order by tag", ids) + if err != nil { + return fmt.Errorf("%w: failed to build tags query: %w", ErrExecution, err) + } + + var rows []struct { + RetroID uuid.UUID `db:"retro_id"` + Tag string `db:"tag"` + } + + if err := db.SelectContext(ctx, &rows, db.Rebind(query), args...); err != nil { + return fmt.Errorf("%w: failed to list tags for retros: %w", ErrExecution, err) + } + + byRetro := make(map[uuid.UUID][]string, len(retros)) + for _, row := range rows { + byRetro[row.RetroID] = append(byRetro[row.RetroID], row.Tag) + } + + for _, retro := range retros { + if tags, ok := byRetro[retro.ID]; ok { + retro.Tags = tags + } + } + + return nil +} + // RetroTagsSet replaces all tags for a retro atomically. func RetroTagsSet(ctx context.Context, db *sqlx.DB, retroID uuid.UUID, tags []string) error { tx, err := db.BeginTxx(ctx, nil) diff --git a/cmd/thoughts/dal/user.go b/cmd/thoughts/dal/user.go index cb452c4..b419d2e 100644 --- a/cmd/thoughts/dal/user.go +++ b/cmd/thoughts/dal/user.go @@ -19,6 +19,9 @@ func UserGet(ctx context.Context, db *sqlx.DB, id uuid.UUID) (*model.User, error return user, nil } +// UserInsert creates a new identity. Names are labels rather than accounts: +// two people called Alex are two people, and one person entering their name +// twice is two sessions. func UserInsert(ctx context.Context, db *sqlx.DB, name string) (*model.User, error) { user := &model.User{ ID: uuid.New(), diff --git a/cmd/thoughts/dal/user_test.go b/cmd/thoughts/dal/user_test.go new file mode 100644 index 0000000..3fa78a0 --- /dev/null +++ b/cmd/thoughts/dal/user_test.go @@ -0,0 +1,30 @@ +package dal_test + +import ( + "context" + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/testutil" +) + +// Two people can share a name, and one person entering theirs twice is two +// sessions. A previous attempt to treat the name as an account broke both. +func TestUserInsertGivesEachLoginItsOwnIdentity(t *testing.T) { + ctx := context.Background() + db := testutil.NewDB(t) + + first, err := dal.UserInsert(ctx, db, "Alex") + if err != nil { + t.Fatalf("failed to insert user: %v", err) + } + + second, err := dal.UserInsert(ctx, db, "Alex") + if err != nil { + t.Fatalf("failed to insert a second user with the same name: %v", err) + } + + if first.ID == second.ID { + t.Fatalf("both logins share the identity %s", first.ID) + } +} diff --git a/cmd/thoughts/event/broker.go b/cmd/thoughts/event/broker.go index 8ce472c..ef77da6 100644 --- a/cmd/thoughts/event/broker.go +++ b/cmd/thoughts/event/broker.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "sync" "github.com/ellgreen/thoughts/cmd/thoughts/model" "github.com/google/uuid" @@ -21,6 +22,8 @@ type Broker struct { handlers map[string]Handler events chan *Event userDependentEvents chan UserDependentEvent + + columnsMu sync.Mutex } func NewBroker(db *sqlx.DB, retroID uuid.UUID) *Broker { @@ -32,9 +35,12 @@ func NewBroker(db *sqlx.DB, retroID uuid.UUID) *Broker { b.register("retro_update", b.handleRetroUpdate(db, retroID)) b.register("status_update", b.handleStatusUpdate(db, retroID)) + b.register("column_create", b.handleColumnCreate(db, retroID)) + b.register("column_update", b.handleColumnUpdate(db, retroID)) + b.register("column_delete", b.handleColumnDelete(db, retroID)) b.register("note_create", b.handleNoteCreate(db, retroID)) b.register("note_update", b.handleNoteUpdate(db, retroID)) - b.register("note_delete", b.handleNoteDelete(db)) + b.register("note_delete", b.handleNoteDelete(db, retroID)) b.register("task_create", b.handleTaskCreate(db, retroID)) b.register("task_update", b.handleTaskUpdate(db)) b.register("task_complete", b.handleTaskComplete(db)) @@ -67,7 +73,16 @@ func (b *Broker) Handle(ctx context.Context, user *model.User, message io.Reader return err } - return handler(ctx, user, event.Payload) + err = handler(ctx, user, event.Payload) + + errorEvent := &ErrorEvent{} + if errors.As(err, &errorEvent) { + if ref, ok := event.Payload["ref"].(string); ok && ref != "" { + errorEvent.Payload["ref"] = ref + } + } + + return err } func (b *Broker) dispatch(event *Event) { diff --git a/cmd/thoughts/event/columns.go b/cmd/thoughts/event/columns.go new file mode 100644 index 0000000..4dc8434 --- /dev/null +++ b/cmd/thoughts/event/columns.go @@ -0,0 +1,175 @@ +package event + +import ( + "context" + "fmt" + "log/slog" + + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/ellgreen/thoughts/cmd/thoughts/requests" + "github.com/google/uuid" + "github.com/jmoiron/sqlx" +) + +const ( + minColumns = 2 + maxColumns = 5 +) + +type ( + columnCreateRequest struct { + Title string `json:"title" validate:"required,min=2,max=255"` + Description string `json:"description" validate:"max=255"` + } + + columnUpdateRequest struct { + ColumnID uuid.UUID `json:"id" validate:"required,uuid"` + Title string `json:"title" validate:"required,min=2,max=255"` + Description string `json:"description" validate:"max=255"` + } + + columnDeleteRequest struct { + ColumnID uuid.UUID `json:"id" validate:"required,uuid"` + } +) + +func (b *Broker) handleColumnCreate(db *sqlx.DB, retroID uuid.UUID) Handler { + return func(ctx context.Context, _ *model.User, payload Payload) error { + req, err := requests.FromMap[columnCreateRequest](payload) + if err != nil { + return newErrorEvent(err.Error()) + } + + retro, err := b.mutateColumns(ctx, db, retroID, + func(columns model.RetroColumns) (model.RetroColumns, error) { + if len(columns) >= maxColumns { + return nil, newErrorEvent(fmt.Sprintf("a retro can have at most %d columns", maxColumns)) + } + + column := &model.RetroColumn{ + ID: uuid.New(), + Title: req.Title, + Description: req.Description, + } + + return append(columns, column), nil + }) + + if err != nil { + return err + } + + b.dispatch(newRetroUpdatedEvent(retro)) + + return nil + } +} + +func (b *Broker) handleColumnUpdate(db *sqlx.DB, retroID uuid.UUID) Handler { + return func(ctx context.Context, _ *model.User, payload Payload) error { + req, err := requests.FromMap[columnUpdateRequest](payload) + if err != nil { + return newErrorEvent(err.Error()) + } + + retro, err := b.mutateColumns(ctx, db, retroID, + func(columns model.RetroColumns) (model.RetroColumns, error) { + column := columns.Find(req.ColumnID) + if column == nil { + return nil, newErrorEvent("column not found") + } + + column.Title = req.Title + column.Description = req.Description + + return columns, nil + }) + + if err != nil { + return err + } + + b.dispatch(newRetroUpdatedEvent(retro)) + + return nil + } +} + +func (b *Broker) handleColumnDelete(db *sqlx.DB, retroID uuid.UUID) Handler { + return func(ctx context.Context, _ *model.User, payload Payload) error { + req, err := requests.FromMap[columnDeleteRequest](payload) + if err != nil { + return newErrorEvent(err.Error()) + } + + retro, err := b.mutateColumns(ctx, db, retroID, + func(columns model.RetroColumns) (model.RetroColumns, error) { + if columns.Find(req.ColumnID) == nil { + return nil, newErrorEvent("column not found") + } + + if len(columns) <= minColumns { + return nil, newErrorEvent(fmt.Sprintf("a retro must have at least %d columns", minColumns)) + } + + count, err := dal.NoteCountForColumn(ctx, db, retroID, req.ColumnID) + if err != nil { + slog.Error("problem counting notes for column", "error", err) + return nil, newErrorEvent("problem checking the column") + } + + if count > 0 { + return nil, newErrorEvent("cannot delete a column that contains notes") + } + + return columns.Without(req.ColumnID), nil + }) + + if err != nil { + return err + } + + b.dispatch(newRetroUpdatedEvent(retro)) + + return nil + } +} + +// mutateColumns serialises the read-modify-write of the columns blob against +// other column changes and against note creation. +// +// A transaction is the wrong tool: sqlx issues a deferred BEGIN, so under WAL +// a concurrent writer fails with SQLITE_BUSY_SNAPSHOT rather than serialising. +// One broker per retro in a single process makes a mutex sufficient. +// +// The caller broadcasts after this returns: dispatch reaches a client's write +// pump, and holding the lock across it would let one wedged client block every +// column change in the retro. +func (b *Broker) mutateColumns( + ctx context.Context, + db *sqlx.DB, + retroID uuid.UUID, + mutate func(columns model.RetroColumns) (model.RetroColumns, error), +) (*model.Retro, error) { + b.columnsMu.Lock() + defer b.columnsMu.Unlock() + + retro, err := dal.RetroGet(ctx, db, retroID) + if err != nil { + slog.Error("problem getting retro", "error", err) + return nil, newErrorEvent("problem getting retro") + } + + columns, err := mutate(retro.GetColumns()) + if err != nil { + return nil, err + } + + if err := dal.RetroSetColumns(ctx, db, retro, columns); err != nil { + slog.Error("problem updating columns", "error", err) + return nil, newErrorEvent("problem updating columns") + } + + return retro, nil +} diff --git a/cmd/thoughts/event/columns_test.go b/cmd/thoughts/event/columns_test.go new file mode 100644 index 0000000..f61a79e --- /dev/null +++ b/cmd/thoughts/event/columns_test.go @@ -0,0 +1,363 @@ +package event_test + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "sync" + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/google/uuid" +) + +func (h *harness) currentColumns(t *testing.T) model.RetroColumns { + t.Helper() + + retro, err := dal.RetroGet(context.Background(), h.db, h.retro.ID) + if err != nil { + t.Fatalf("failed to re-read retro: %v", err) + } + + return retro.GetColumns() +} + +func TestColumnUpdateRenamesInPlace(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Facilitator") + + target := h.columns[0].ID + + if err := h.handle(t, user, "column_update", map[string]any{ + "id": target.String(), + "title": "What went well", + "description": "Wins worth repeating", + }); err != nil { + t.Fatalf("column_update failed: %v", err) + } + + if evt := h.next(t); evt.Name != "retro_updated" { + t.Fatalf("expected retro_updated, got %s", evt.Name) + } + + columns := h.currentColumns(t) + + if len(columns) != 2 { + t.Fatalf("expected 2 columns, got %d", len(columns)) + } + + updated := columns.Find(target) + if updated == nil { + t.Fatal("the column lost its id") + } + + if updated.Title != "What went well" || updated.Description != "Wins worth repeating" { + t.Errorf("column was not updated: %+v", updated) + } + + if columns[0].ID != target { + t.Error("columns were reordered") + } + + if columns[1].Title != "Went badly" { + t.Errorf("the neighbouring column changed to %q", columns[1].Title) + } +} + +func TestColumnUpdateClearsADescription(t *testing.T) { + h := newHarness(t) + + if err := h.handle(t, h.user(t, "Facilitator"), "column_update", map[string]any{ + "id": h.columns[0].ID.String(), + "title": "Went well", + "description": "", + }); err != nil { + t.Fatalf("column_update failed: %v", err) + } + + h.next(t) + + if got := h.currentColumns(t)[0].Description; got != "" { + t.Errorf("description = %q, want it cleared", got) + } +} + +func TestColumnUpdateRejectsBadInput(t *testing.T) { + cases := map[string]map[string]any{ + "missing id": {"title": "A title", "description": ""}, + "unknown id": {"id": uuid.New().String(), "title": "A title", "description": ""}, + "not a uuid": {"id": "tasks", "title": "A title", "description": ""}, + "empty title": {"title": "", "description": ""}, + "short title": {"title": "a", "description": ""}, + "missing title": {"description": ""}, + } + + for name, payload := range cases { + t.Run(name, func(t *testing.T) { + h := newHarness(t) + + if _, ok := payload["id"]; !ok { + if name != "missing id" { + payload["id"] = h.columns[0].ID.String() + } + } + + if err := h.handle(t, h.user(t, "Facilitator"), "column_update", payload); err == nil { + t.Errorf("expected %v to be rejected", payload) + } + }) + } +} + +func TestColumnDeleteRemovesAnEmptyColumn(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Facilitator") + + if err := h.handle(t, user, "column_create", map[string]any{ + "title": "Ideas", + "description": "Anything else", + }); err != nil { + t.Fatalf("column_create failed: %v", err) + } + + h.next(t) + + target := h.currentColumns(t)[2].ID + + if err := h.handle(t, user, "column_delete", map[string]any{"id": target.String()}); err != nil { + t.Fatalf("column_delete failed: %v", err) + } + + if evt := h.next(t); evt.Name != "retro_updated" { + t.Fatalf("expected retro_updated, got %s", evt.Name) + } + + columns := h.currentColumns(t) + + if len(columns) != 2 { + t.Fatalf("expected 2 columns after delete, got %d", len(columns)) + } + + if columns.Find(target) != nil { + t.Error("the deleted column is still there") + } +} + +func TestColumnDeleteRefusesWhenTheColumnHasNotes(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Author") + + if err := h.handle(t, user, "column_create", map[string]any{ + "title": "Ideas", + "description": "", + }); err != nil { + t.Fatalf("column_create failed: %v", err) + } + + h.next(t) + + h.createNote(t, user, "a thought worth keeping") + + assertErrorEvent( + t, + h.handle(t, user, "column_delete", map[string]any{"id": h.columns[0].ID.String()}), + "cannot delete a column that contains notes", + ) + + if len(h.currentColumns(t)) != 3 { + t.Error("the column was deleted despite holding a note") + } +} + +func TestColumnDeleteKeepsAMinimumOfTwo(t *testing.T) { + h := newHarness(t) + + assertErrorEvent( + t, + h.handle(t, h.user(t, "Facilitator"), "column_delete", map[string]any{ + "id": h.columns[0].ID.String(), + }), + "a retro must have at least 2 columns", + ) + + if len(h.currentColumns(t)) != 2 { + t.Error("a column was removed below the minimum") + } +} + +func TestColumnDeleteRejectsAnUnknownColumn(t *testing.T) { + h := newHarness(t) + + assertErrorEvent( + t, + h.handle(t, h.user(t, "Facilitator"), "column_delete", map[string]any{ + "id": uuid.New().String(), + }), + "column not found", + ) +} + +func TestColumnDeleteRejectsTheSyntheticTasksColumn(t *testing.T) { + h := newHarness(t) + + // The discuss stage renders a hardcoded "tasks" column that must not be + // able to reach a handler. + if err := h.handle(t, h.user(t, "Facilitator"), "column_delete", map[string]any{ + "id": "tasks", + }); err == nil { + t.Error("expected a non-uuid column id to be rejected") + } +} + +func TestColumnCreateAppendsUpToTheLimit(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Facilitator") + + for i := range 3 { + if err := h.handle(t, user, "column_create", map[string]any{ + "title": "Extra column", + "description": "", + }); err != nil { + t.Fatalf("column_create %d failed: %v", i, err) + } + + h.next(t) + } + + if len(h.currentColumns(t)) != 5 { + t.Fatalf("expected 5 columns, got %d", len(h.currentColumns(t))) + } + + assertErrorEvent( + t, + h.handle(t, user, "column_create", map[string]any{"title": "One too many", "description": ""}), + "a retro can have at most 5 columns", + ) +} + +func TestColumnCreateAssignsAnID(t *testing.T) { + h := newHarness(t) + + if err := h.handle(t, h.user(t, "Facilitator"), "column_create", map[string]any{ + "title": "Ideas", + "description": "Anything else", + }); err != nil { + t.Fatalf("column_create failed: %v", err) + } + + h.next(t) + + added := h.currentColumns(t)[2] + + if added.ID == uuid.Nil { + t.Error("the new column has no id") + } + + if added.Title != "Ideas" || added.Description != "Anything else" { + t.Errorf("the new column is wrong: %+v", added) + } +} + +func TestColumnsMayBeChangedInAnyStage(t *testing.T) { + for _, status := range []model.RetroStatus{ + model.RetroStatusBrainstorm, + model.RetroStatusGroup, + model.RetroStatusVote, + model.RetroStatusDiscuss, + } { + t.Run(string(status), func(t *testing.T) { + h := newHarness(t) + + if err := dal.RetroUpdateStatus(context.Background(), h.db, h.retro.ID, status); err != nil { + t.Fatalf("failed to set status: %v", err) + } + + if err := h.handle(t, h.user(t, "Facilitator"), "column_update", map[string]any{ + "id": h.columns[0].ID.String(), + "title": "Renamed mid retro", + "description": "", + }); err != nil { + t.Fatalf("column_update failed in %s: %v", status, err) + } + + h.next(t) + + if h.currentColumns(t)[0].Title != "Renamed mid retro" { + t.Errorf("the rename did not stick in %s", status) + } + }) + } +} + +func TestConcurrentColumnCreatesDoNotClobberEachOther(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Facilitator") + + // Columns are one JSON blob, so simultaneous edits race on a + // read-modify-write. + var wg sync.WaitGroup + + for i := range 3 { + wg.Add(1) + + go func() { + defer wg.Done() + + body, err := json.Marshal(map[string]any{ + "name": "column_create", + "payload": map[string]any{ + "title": fmt.Sprintf("Column %d", i), + "description": "", + }, + }) + if err != nil { + return + } + + _ = h.broker.Handle(context.Background(), user, bytes.NewReader(body)) + }() + } + + wg.Wait() + + for range 3 { + h.next(t) + } + + if got := len(h.currentColumns(t)); got != 5 { + t.Errorf("expected 5 columns after 3 concurrent creates, got %d", got) + } +} + +func TestNotesCannotBeCreatedInADeletedColumn(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Author") + + if err := h.handle(t, user, "column_create", map[string]any{ + "title": "Doomed", + "description": "", + }); err != nil { + t.Fatalf("column_create failed: %v", err) + } + + h.next(t) + + doomed := h.currentColumns(t)[2].ID + + if err := h.handle(t, user, "column_delete", map[string]any{"id": doomed.String()}); err != nil { + t.Fatalf("column_delete failed: %v", err) + } + + h.next(t) + + assertErrorEvent( + t, + h.handle(t, user, "note_create", map[string]any{ + "column_id": doomed.String(), + "content": "a homeless thought", + }), + "that column no longer exists", + ) +} diff --git a/cmd/thoughts/event/event.go b/cmd/thoughts/event/event.go index 32b9ea0..4c4c854 100644 --- a/cmd/thoughts/event/event.go +++ b/cmd/thoughts/event/event.go @@ -40,6 +40,24 @@ func newErrorEvent(message string) *ErrorEvent { type UserDependentEvent func(user *model.User) *Event +// refFrom pulls the client's correlation id out of an inbound payload. Clients +// use it to match server confirmations and failures back to the optimistic +// update they already applied locally. +func refFrom(payload Payload) string { + ref, _ := payload["ref"].(string) + + return ref +} + +// withRef echoes a correlation id back on an outbound payload. +func withRef(payload Payload, ref string) Payload { + if ref != "" { + payload["ref"] = ref + } + + return payload +} + func (e *Event) ToJSON() []byte { val, err := json.Marshal(e) if err != nil { diff --git a/cmd/thoughts/event/event_test.go b/cmd/thoughts/event/event_test.go new file mode 100644 index 0000000..285b74f --- /dev/null +++ b/cmd/thoughts/event/event_test.go @@ -0,0 +1,397 @@ +package event_test + +import ( + "bytes" + "context" + "encoding/json" + "testing" + "time" + + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/event" + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/ellgreen/thoughts/cmd/thoughts/testutil" + "github.com/google/uuid" + "github.com/jmoiron/sqlx" +) + +type harness struct { + db *sqlx.DB + broker *event.Broker + retro *model.Retro + columns model.RetroColumns + observed chan *event.Event +} + +func newHarness(t *testing.T) *harness { + t.Helper() + + db := testutil.NewDB(t) + + columns := model.RetroColumns{ + {Title: "Went well", Description: "The good bits"}, + {Title: "Went badly", Description: "The bad bits"}, + } + + retro, err := dal.RetroInsert(context.Background(), db, "A test retro", columns, false) + if err != nil { + t.Fatalf("failed to seed retro: %v", err) + } + + h := &harness{ + db: db, + broker: event.NewBroker(db, retro.ID), + retro: retro, + columns: retro.GetColumns(), + observed: make(chan *event.Event, 64), + } + + observer := &model.User{ID: uuid.New(), Name: "Observer"} + stop := make(chan struct{}) + t.Cleanup(func() { close(stop) }) + + go func() { + for { + select { + case <-stop: + return + case evt := <-h.broker.Listen(): + h.observed <- evt + case userDependent := <-h.broker.ListenUserDependent(): + h.observed <- userDependent(observer) + } + } + }() + + return h +} + +func (h *harness) user(t *testing.T, name string) *model.User { + t.Helper() + + user, err := dal.UserInsert(context.Background(), h.db, name) + if err != nil { + t.Fatalf("failed to create user %s: %v", name, err) + } + + return user +} + +func (h *harness) handle(t *testing.T, user *model.User, name string, payload map[string]any) error { + t.Helper() + + body, err := json.Marshal(map[string]any{"name": name, "payload": payload}) + if err != nil { + t.Fatalf("failed to encode event: %v", err) + } + + return h.broker.Handle(context.Background(), user, bytes.NewReader(body)) +} + +func (h *harness) next(t *testing.T) *event.Event { + t.Helper() + + select { + case evt := <-h.observed: + return evt + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for a broadcast event") + return nil + } +} + +func (h *harness) createNote(t *testing.T, user *model.User, content string) uuid.UUID { + t.Helper() + + if err := h.handle(t, user, "note_create", map[string]any{ + "column_id": h.columns[0].ID.String(), + "content": content, + }); err != nil { + t.Fatalf("note_create failed: %v", err) + } + + evt := h.next(t) + if evt.Name != "note_created" { + t.Fatalf("expected note_created, got %s", evt.Name) + } + + id, ok := evt.Payload["id"].(uuid.UUID) + if !ok { + t.Fatalf("note_created carried an id of type %T", evt.Payload["id"]) + } + + return id +} + +func assertErrorEvent(t *testing.T, err error, want string) { + t.Helper() + + if err == nil { + t.Fatalf("expected an error event saying %q, got nil", want) + } + + if err.Error() != want { + t.Errorf("expected error %q, got %q", want, err.Error()) + } +} + +func TestStatusTransitions(t *testing.T) { + cases := []struct { + from model.RetroStatus + to string + wantErr string + }{ + {model.RetroStatusBrainstorm, "group", ""}, + {model.RetroStatusBrainstorm, "vote", "invalid status transition"}, + {model.RetroStatusBrainstorm, "discuss", "invalid status transition"}, + {model.RetroStatusBrainstorm, "brainstorm", "status is already set to brainstorm"}, + {model.RetroStatusGroup, "brainstorm", ""}, + {model.RetroStatusGroup, "vote", ""}, + {model.RetroStatusGroup, "discuss", "invalid status transition"}, + {model.RetroStatusVote, "group", ""}, + {model.RetroStatusVote, "discuss", ""}, + {model.RetroStatusVote, "brainstorm", "invalid status transition"}, + {model.RetroStatusDiscuss, "vote", ""}, + {model.RetroStatusDiscuss, "group", "invalid status transition"}, + {model.RetroStatusDiscuss, "brainstorm", "invalid status transition"}, + } + + for _, tc := range cases { + t.Run(string(tc.from)+"_to_"+tc.to, func(t *testing.T) { + h := newHarness(t) + user := h.user(t, "Facilitator") + + if err := dal.RetroUpdateStatus(context.Background(), h.db, h.retro.ID, tc.from); err != nil { + t.Fatalf("failed to set starting status: %v", err) + } + + err := h.handle(t, user, "status_update", map[string]any{"status": tc.to}) + + if tc.wantErr != "" { + assertErrorEvent(t, err, tc.wantErr) + return + } + + if err != nil { + t.Fatalf("expected the transition to be allowed, got %v", err) + } + + if evt := h.next(t); evt.Name != "status_updated" { + t.Errorf("expected status_updated, got %s", evt.Name) + } + }) + } +} + +func TestStatusUpdateRejectsUnknownStatus(t *testing.T) { + h := newHarness(t) + + err := h.handle(t, h.user(t, "Facilitator"), "status_update", map[string]any{"status": "napping"}) + if err == nil { + t.Fatal("expected an unknown status to be rejected") + } +} + +func TestOnlyTheAuthorMayEditNoteContent(t *testing.T) { + h := newHarness(t) + + author := h.user(t, "Author") + other := h.user(t, "Someone Else") + + noteID := h.createNote(t, author, "my own thought") + + err := h.handle(t, other, "note_update", map[string]any{ + "id": noteID.String(), + "content": "not my thought", + }) + + assertErrorEvent(t, err, "you can only change your own notes") + + note, getErr := dal.NoteGet(context.Background(), h.db, noteID) + if getErr != nil { + t.Fatalf("failed to re-read note: %v", getErr) + } + + if note.Content != "my own thought" { + t.Errorf("content was changed to %q despite the rejection", note.Content) + } +} + +func TestOnlyTheAuthorMayAttachAGif(t *testing.T) { + h := newHarness(t) + + author := h.user(t, "Author") + other := h.user(t, "Someone Else") + + noteID := h.createNote(t, author, "my own thought") + + err := h.handle(t, other, "note_update", map[string]any{ + "id": noteID.String(), + "img_url": "https://example.com/cat.gif", + }) + + assertErrorEvent(t, err, "you can only change your own notes") +} + +func TestAnyoneMayMoveANote(t *testing.T) { + h := newHarness(t) + + author := h.user(t, "Author") + other := h.user(t, "Someone Else") + + noteID := h.createNote(t, author, "my own thought") + + err := h.handle(t, other, "note_update", map[string]any{ + "id": noteID.String(), + "column_id": h.columns[1].ID.String(), + "group_id": uuid.New().String(), + }) + + if err != nil { + t.Fatalf("expected a move by another user to be allowed, got %v", err) + } + + if evt := h.next(t); evt.Name != "note_updated" { + t.Errorf("expected note_updated, got %s", evt.Name) + } + + note, getErr := dal.NoteGet(context.Background(), h.db, noteID) + if getErr != nil { + t.Fatalf("failed to re-read note: %v", getErr) + } + + if note.ColumnID != h.columns[1].ID { + t.Errorf("note did not move: still in column %s", note.ColumnID) + } +} + +func TestOnlyTheAuthorMayDeleteANote(t *testing.T) { + h := newHarness(t) + + author := h.user(t, "Author") + other := h.user(t, "Someone Else") + + noteID := h.createNote(t, author, "my own thought") + + assertErrorEvent( + t, + h.handle(t, other, "note_delete", map[string]any{"id": noteID.String()}), + "you can only change your own notes", + ) + + if _, err := dal.NoteGet(context.Background(), h.db, noteID); err != nil { + t.Errorf("note was deleted despite the rejection: %v", err) + } + + if err := h.handle(t, author, "note_delete", map[string]any{"id": noteID.String()}); err != nil { + t.Fatalf("the author should be able to delete their own note, got %v", err) + } +} + +func TestNotesFromAnotherRetroAreInvisible(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + + author := h.user(t, "Author") + + otherRetro, err := dal.RetroInsert(ctx, h.db, "Another retro", model.RetroColumns{ + {Title: "One", Description: ""}, + {Title: "Two", Description: ""}, + }, false) + if err != nil { + t.Fatalf("failed to seed the second retro: %v", err) + } + + foreign, err := dal.NoteInsert(ctx, h.db, otherRetro.ID, author.ID, otherRetro.GetColumns()[0].ID, "elsewhere") + if err != nil { + t.Fatalf("failed to seed the foreign note: %v", err) + } + + assertErrorEvent( + t, + h.handle(t, author, "note_delete", map[string]any{"id": foreign.ID.String()}), + "note not found", + ) + + assertErrorEvent( + t, + h.handle(t, author, "note_update", map[string]any{"id": foreign.ID.String(), "content": "reached in"}), + "note not found", + ) + + if _, err := dal.NoteGet(ctx, h.db, foreign.ID); err != nil { + t.Errorf("the foreign note was deleted: %v", err) + } +} + +func TestErrorEventsEchoTheClientRef(t *testing.T) { + h := newHarness(t) + + author := h.user(t, "Author") + other := h.user(t, "Someone Else") + + noteID := h.createNote(t, author, "my own thought") + + err := h.handle(t, other, "note_update", map[string]any{ + "id": noteID.String(), + "content": "not my thought", + "ref": "abc-123", + }) + + errorEvent, ok := err.(*event.ErrorEvent) + if !ok { + t.Fatalf("expected an ErrorEvent, got %T", err) + } + + if errorEvent.Payload["ref"] != "abc-123" { + t.Errorf("expected the ref to be echoed back, got %v", errorEvent.Payload["ref"]) + } +} + +func TestConfirmationsEchoTheClientRef(t *testing.T) { + h := newHarness(t) + + if err := h.handle(t, h.user(t, "Author"), "note_create", map[string]any{ + "column_id": h.columns[0].ID.String(), + "content": "a thought", + "ref": "xyz-789", + }); err != nil { + t.Fatalf("note_create failed: %v", err) + } + + if got := h.next(t).Payload["ref"]; got != "xyz-789" { + t.Errorf("expected the ref to be echoed back, got %v", got) + } +} + +func TestNotesAreOnlyObfuscatedDuringBrainstorm(t *testing.T) { + h := newHarness(t) + ctx := context.Background() + + author := h.user(t, "Author") + + h.createNote(t, author, "secret thought") + + if err := dal.RetroUpdateStatus(ctx, h.db, h.retro.ID, model.RetroStatusGroup); err != nil { + t.Fatalf("failed to advance the retro: %v", err) + } + + if err := h.handle(t, author, "note_create", map[string]any{ + "column_id": h.columns[0].ID.String(), + "content": "open thought", + }); err != nil { + t.Fatalf("note_create failed: %v", err) + } + + if got := h.next(t).Payload["content"]; got != "open thought" { + t.Errorf("expected the content in the clear after brainstorm, got %q", got) + } +} + +func TestUnknownEventsAreRejected(t *testing.T) { + h := newHarness(t) + + if err := h.handle(t, h.user(t, "Author"), "drop_database", nil); err == nil { + t.Error("expected an unknown event name to be rejected") + } +} diff --git a/cmd/thoughts/event/notes.go b/cmd/thoughts/event/notes.go index 7b48e40..3c44cbd 100644 --- a/cmd/thoughts/event/notes.go +++ b/cmd/thoughts/event/notes.go @@ -2,6 +2,8 @@ package event import ( "context" + "database/sql" + "errors" "log/slog" "github.com/ellgreen/thoughts/cmd/thoughts/dal" @@ -12,6 +14,8 @@ import ( "github.com/jmoiron/sqlx" ) +var noteContentFields = []string{"content", "img_url", "remove_img_url"} + type noteCreateRequest struct { ColumnID uuid.UUID `json:"column_id" validate:"required,uuid"` Content string `json:"content" validate:"required,min=2,max=255"` @@ -24,29 +28,58 @@ func (b *Broker) handleNoteCreate(db *sqlx.DB, retroID uuid.UUID) Handler { return newErrorEvent(err.Error()) } - note, err := dal.NoteInsert(ctx, db, retroID, user.ID, req.ColumnID, req.Content) + retro, note, err := b.createNote(ctx, db, retroID, user, req) if err != nil { - slog.Error("problem inserting note", "error", err) - return newErrorEvent("problem inserting note") + return err } - b.dispatchUserDependent(newNoteCreatedEvent(note)) + b.dispatchUserDependent(newNoteCreatedEvent(note, retro, refFrom(payload))) return nil } } +func (b *Broker) createNote( + ctx context.Context, + db *sqlx.DB, + retroID uuid.UUID, + user *model.User, + req *noteCreateRequest, +) (*model.Retro, *model.Note, error) { + b.columnsMu.Lock() + defer b.columnsMu.Unlock() + + retro, err := dal.RetroGet(ctx, db, retroID) + if err != nil { + slog.Error("problem getting retro", "error", err) + return nil, nil, newErrorEvent("problem getting retro") + } + + if retro.GetColumns().Find(req.ColumnID) == nil { + return nil, nil, newErrorEvent("that column no longer exists") + } + + note, err := dal.NoteInsert(ctx, db, retroID, user.ID, req.ColumnID, req.Content) + if err != nil { + slog.Error("problem inserting note", "error", err) + return nil, nil, newErrorEvent("problem inserting note") + } + + return retro, note, nil +} + type noteUpdateRequest struct { - NoteID uuid.UUID `json:"id" validate:"required,uuid"` - ColumnID uuid.UUID `json:"column_id" validate:"omitempty,required_with=group_id,uuid"` - GroupID uuid.UUID `json:"group_id" validate:"omitempty,uuid"` - Content string `json:"content" validate:"omitempty,min=2,max=255"` - ImgURL string `json:"img_url" validate:"omitempty,url"` - RemoveImgURL bool `json:"remove_img_url"` + NoteID uuid.UUID `json:"id" validate:"required,uuid"` + ColumnID uuid.UUID `json:"column_id" validate:"omitempty,required_with=group_id,uuid"` + GroupID uuid.UUID `json:"group_id" validate:"omitempty,uuid"` + Content string `json:"content" validate:"omitempty,min=2,max=255"` + // These are user-supplied now, and a browser blocks mixed content anyway. + ImgURL string `json:"img_url" validate:"omitempty,url,startswith=https://,max=2048"` + RemoveImgURL bool `json:"remove_img_url"` } func (b *Broker) handleNoteUpdate(db *sqlx.DB, retroID uuid.UUID) Handler { - return func(ctx context.Context, _ *model.User, payload Payload) error { + return func(ctx context.Context, user *model.User, payload Payload) error { retro, err := dal.RetroGet(ctx, db, retroID) if err != nil { slog.Error("problem getting retro", "error", err) @@ -58,13 +91,17 @@ func (b *Broker) handleNoteUpdate(db *sqlx.DB, retroID uuid.UUID) Handler { return newErrorEvent(err.Error()) } + if err := authoriseNote(ctx, db, user, retroID, req.NoteID, payloadHasAny(payload, noteContentFields...)); err != nil { + return err + } + note, err := dal.NoteUpdate(ctx, db, req.NoteID, req.ColumnID, req.GroupID, req.Content, req.ImgURL, req.RemoveImgURL) if err != nil { slog.Error("problem updating note", "error", err) return newErrorEvent("problem updating note") } - b.dispatchUserDependent(newNoteUpdatedEvent(note, retro)) + b.dispatchUserDependent(newNoteUpdatedEvent(note, retro, refFrom(payload))) return nil } @@ -74,51 +111,95 @@ type noteDeleteRequest struct { NoteID uuid.UUID `json:"id" validate:"required,uuid"` } -func (b *Broker) handleNoteDelete(db *sqlx.DB) Handler { - return func(ctx context.Context, _ *model.User, payload Payload) error { +func (b *Broker) handleNoteDelete(db *sqlx.DB, retroID uuid.UUID) Handler { + return func(ctx context.Context, user *model.User, payload Payload) error { req, err := requests.FromMap[noteDeleteRequest](payload) if err != nil { return newErrorEvent(err.Error()) } + if err := authoriseNote(ctx, db, user, retroID, req.NoteID, true); err != nil { + return err + } + if err := dal.NoteDelete(ctx, db, req.NoteID); err != nil { slog.Error("problem deleting note", "error", err) return newErrorEvent("problem deleting note") } - b.dispatch(newNoteDeletedEvent(req.NoteID)) + b.dispatch(newNoteDeletedEvent(req.NoteID, refFrom(payload))) return nil } } -func newNoteCreatedEvent(note *model.Note) UserDependentEvent { +func authoriseNote( + ctx context.Context, + db *sqlx.DB, + user *model.User, + retroID uuid.UUID, + noteID uuid.UUID, + requireOwner bool, +) error { + note, err := dal.NoteGet(ctx, db, noteID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return newErrorEvent("note not found") + } + + slog.Error("problem getting note", "error", err) + + return newErrorEvent("problem getting note") + } + + if note.RetroID != retroID { + return newErrorEvent("note not found") + } + + if requireOwner && note.UserID != user.ID { + return newErrorEvent("you can only change your own notes") + } + + return nil +} + +func payloadHasAny(payload Payload, keys ...string) bool { + for _, key := range keys { + if _, ok := payload[key]; ok { + return true + } + } + + return false +} + +func newNoteCreatedEvent(note *model.Note, retro *model.Retro, ref string) UserDependentEvent { return func(user *model.User) *Event { - resource := resources.NoteFromModel(note, nil, user.ID, true) + resource := resources.NoteFromModel(note, nil, user.ID, retro.IsBrainstorming()) payload := resources.StructToMap(resource) return &Event{ Name: "note_created", - Payload: payload, + Payload: withRef(payload, ref), } } } -func newNoteUpdatedEvent(note *model.Note, retro *model.Retro) UserDependentEvent { +func newNoteUpdatedEvent(note *model.Note, retro *model.Retro, ref string) UserDependentEvent { return func(user *model.User) *Event { resource := resources.NoteFromModel(note, nil, user.ID, retro.IsBrainstorming()) payload := resources.StructToMap(resource) return &Event{ Name: "note_updated", - Payload: payload, + Payload: withRef(payload, ref), } } } -func newNoteDeletedEvent(noteID uuid.UUID) *Event { +func newNoteDeletedEvent(noteID uuid.UUID, ref string) *Event { return &Event{ Name: "note_deleted", - Payload: Payload{"id": noteID}, + Payload: withRef(Payload{"id": noteID}, ref), } } diff --git a/cmd/thoughts/event/retro.go b/cmd/thoughts/event/retro.go index 8724216..d1e58dc 100644 --- a/cmd/thoughts/event/retro.go +++ b/cmd/thoughts/event/retro.go @@ -15,7 +15,7 @@ import ( type retroUpdateRequest struct { Title string `json:"title" validate:"required,min=5,max=255"` Unlisted bool `json:"unlisted"` - MaxVotes int `json:"max_votes" validate:"required,number,min=1,max=15"` + MaxVotes int `json:"max_votes" validate:"min=1,max=15"` Tags []string `json:"tags" validate:"omitempty,max=10,dive,min=1,max=50"` } diff --git a/cmd/thoughts/exporters/markdown.go b/cmd/thoughts/exporters/markdown.go index 054a50c..9c2aa68 100644 --- a/cmd/thoughts/exporters/markdown.go +++ b/cmd/thoughts/exporters/markdown.go @@ -67,7 +67,15 @@ func (e *Exporter) ToMarkdown(ctx context.Context, retro *model.Retro) ([]byte, content = strings.ReplaceAll(content, "\n", "\n> ") buf.WriteString("\n> " + content) - buf.WriteString("\n>\n") + buf.WriteString("\n") + + if note.ImgURL.Valid && note.ImgURL.V != "" { + buf.WriteString(">\n") + buf.WriteString("> ![](" + note.ImgURL.V + ")") + buf.WriteString("\n") + } + + buf.WriteString(">\n") buf.WriteString("> — " + author + "") buf.WriteString("\n") } diff --git a/cmd/thoughts/exporters/markdown_test.go b/cmd/thoughts/exporters/markdown_test.go new file mode 100644 index 0000000..50d43dc --- /dev/null +++ b/cmd/thoughts/exporters/markdown_test.go @@ -0,0 +1,121 @@ +package exporters_test + +import ( + "context" + "strings" + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/exporters" + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/ellgreen/thoughts/cmd/thoughts/testutil" + "github.com/google/uuid" + "github.com/jmoiron/sqlx" +) + +func TestToMarkdown(t *testing.T) { + db := testutil.NewDB(t) + ctx := context.Background() + + retro, err := dal.RetroInsert(ctx, db, "Sprint 42 retro", model.RetroColumns{ + {Title: "Went well", Description: "The good bits"}, + {Title: "Went badly", Description: "The bad bits"}, + }, false) + if err != nil { + t.Fatalf("failed to seed retro: %v", err) + } + + columns := retro.GetColumns() + + author, err := dal.UserInsert(ctx, db, "Ada") + if err != nil { + t.Fatalf("failed to seed user: %v", err) + } + + if _, err := dal.NoteInsert(ctx, db, retro.ID, author.ID, columns[0].ID, "deploys were quick"); err != nil { + t.Fatalf("failed to seed note: %v", err) + } + + withGif, err := dal.NoteInsert(ctx, db, retro.ID, author.ID, columns[1].ID, "too many meetings") + if err != nil { + t.Fatalf("failed to seed note: %v", err) + } + + if _, err := dal.NoteUpdate( + ctx, db, withGif.ID, withGif.ColumnID, withGif.GroupID, "", "https://example.com/tired.gif", false, + ); err != nil { + t.Fatalf("failed to attach gif: %v", err) + } + + seedTask(t, db, retro.ID, "Ada", "book a smaller room", true) + seedTask(t, db, retro.ID, "Grace", "trim the standup", false) + + out, err := exporters.NewExporter(db).ToMarkdown(ctx, retro) + if err != nil { + t.Fatalf("export failed: %v", err) + } + + got := string(out) + + for _, want := range []string{ + "# Sprint 42 retro", + "## Went well", + "The good bits", + "> deploys were quick", + "> — Ada", + "## Went badly", + "> too many meetings", + "> ![](https://example.com/tired.gif)", + "- [x] Ada: book a smaller room", + "- [ ] Grace: trim the standup", + } { + if !strings.Contains(got, want) { + t.Errorf("export is missing %q\n---\n%s", want, got) + } + } +} + +func TestToMarkdownWithNoContent(t *testing.T) { + db := testutil.NewDB(t) + ctx := context.Background() + + retro, err := dal.RetroInsert(ctx, db, "Empty retro", model.RetroColumns{ + {Title: "Only column", Description: ""}, + {Title: "Second column", Description: ""}, + }, false) + if err != nil { + t.Fatalf("failed to seed retro: %v", err) + } + + out, err := exporters.NewExporter(db).ToMarkdown(ctx, retro) + if err != nil { + t.Fatalf("export failed: %v", err) + } + + got := string(out) + + for _, want := range []string{"# Empty retro", "## Tasks", "## Notes", "## Only column"} { + if !strings.Contains(got, want) { + t.Errorf("export is missing %q\n---\n%s", want, got) + } + } +} + +func seedTask(t *testing.T, db *sqlx.DB, retroID uuid.UUID, who, what string, completed bool) { + t.Helper() + + ctx := context.Background() + + task, err := dal.TaskInsert(ctx, db, retroID, who, what, "2026-09-01") + if err != nil { + t.Fatalf("failed to seed task: %v", err) + } + + if !completed { + return + } + + if _, err := dal.TaskUpdateComplete(ctx, db, task.ID, true); err != nil { + t.Fatalf("failed to complete task: %v", err) + } +} diff --git a/cmd/thoughts/gif/enabled.go b/cmd/thoughts/gif/enabled.go deleted file mode 100644 index b7845ed..0000000 --- a/cmd/thoughts/gif/enabled.go +++ /dev/null @@ -1,23 +0,0 @@ -package gif - -import "log/slog" - -var provider Provider - -func ResolveProvider(tenorAPIKey string) Provider { - if provider != nil { - return provider - } - - if tenorAPIKey != "" { - slog.Info("using tenor gif provider") - - provider = NewTenorProvider(tenorAPIKey) - } - - return provider -} - -func IsAvailable() bool { - return provider != nil -} diff --git a/cmd/thoughts/gif/gif_test.go b/cmd/thoughts/gif/gif_test.go new file mode 100644 index 0000000..5f6e002 --- /dev/null +++ b/cmd/thoughts/gif/gif_test.go @@ -0,0 +1,329 @@ +package gif + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" + "time" +) + +// Shaped from a real api.klipy.com response: size first, then format, under +// "file". An earlier guess had it the other way round, which parsed cleanly +// and produced nothing at all. +const klipyBody = `{ + "result": true, + "data": { + "data": [ + { + "slug": "dancing-cat", + "type": "gif", + "file": { + "hd": {"gif": {"url": "https://cdn.example/hd.gif", "width": 480, "height": 360}, + "webp": {"url": "https://cdn.example/hd.webp", "width": 480, "height": 360}}, + "md": {"gif": {"url": "https://cdn.example/md.gif", "width": 360, "height": 270}, + "webp": {"url": "https://cdn.example/md.webp", "width": 360, "height": 270}}, + "sm": {"gif": {"url": "https://cdn.example/sm.gif", "width": 240, "height": 180}, + "webp": {"url": "https://cdn.example/sm.webp", "width": 240, "height": 180}}, + "xs": {"gif": {"url": "https://cdn.example/xs.gif", "width": 87, "height": 90}} + } + }, + { + "slug": "an-advert", + "type": "ad", + "file": {} + }, + { + "slug": "only-one-size", + "type": "gif", + "file": { + "sm": {"gif": {"url": "https://cdn.example/only-sm.gif", "width": 100, "height": 100}} + } + } + ], + "current_page": 2, + "per_page": 24, + "has_next": true + } +}` + +func TestKlipySearchMapsResults(t *testing.T) { + var gotPath string + var gotQuery url.Values + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotQuery = r.URL.Query() + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(klipyBody)) + })) + defer server.Close() + + provider := &KlipyProvider{BaseURL: server.URL, APIKey: "test-key"} + + page, err := provider.Search(context.Background(), "dancing cat", 2) + if err != nil { + t.Fatalf("search failed: %v", err) + } + + if gotPath != "/test-key/gifs/search" { + t.Errorf("requested %q, want /test-key/gifs/search", gotPath) + } + + if gotQuery.Get("q") != "dancing cat" { + t.Errorf("q = %q, want %q", gotQuery.Get("q"), "dancing cat") + } + + if gotQuery.Get("page") != "2" { + t.Errorf("page = %q, want 2", gotQuery.Get("page")) + } + + if gotQuery.Get("per_page") != "24" { + t.Errorf("per_page = %q, want 24", gotQuery.Get("per_page")) + } + + if len(page.Results) != 2 { + t.Fatalf("got %d results, want 2", len(page.Results)) + } + + first := page.Results[0] + + if first.URL != "https://cdn.example/md.gif" { + t.Errorf("URL = %q, want the medium gif", first.URL) + } + + if first.PreviewURL != "https://cdn.example/sm.webp" { + t.Errorf("PreviewURL = %q, want the small webp", first.PreviewURL) + } + + if first.Width != 240 || first.Height != 180 { + t.Errorf("preview dimensions = %dx%d, want 240x180", first.Width, first.Height) + } + + second := page.Results[1] + + if second.URL != "https://cdn.example/only-sm.gif" || second.PreviewURL != "https://cdn.example/only-sm.gif" { + t.Errorf("fallback mapping produced %+v", second) + } + + if !page.HasNext || page.Page != 2 { + t.Errorf("pagination = page %d hasNext %v, want page 2 hasNext true", page.Page, page.HasNext) + } +} + +func TestKlipyTrendingUsesTheTrendingEndpoint(t *testing.T) { + var gotPath string + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + _, _ = w.Write([]byte(klipyBody)) + })) + defer server.Close() + + provider := &KlipyProvider{BaseURL: server.URL, APIKey: "k"} + + if _, err := provider.Trending(context.Background(), 0); err != nil { + t.Fatalf("trending failed: %v", err) + } + + if gotPath != "/k/gifs/trending" { + t.Errorf("requested %q, want /k/gifs/trending", gotPath) + } +} + +func TestKlipySurfacesUpstreamFailures(t *testing.T) { + cases := map[string]http.HandlerFunc{ + "http error": func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + }, + "unparseable body": func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("not json")) + }, + "result false": func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"result": false, "data": {}}`)) + }, + } + + for name, handler := range cases { + t.Run(name, func(t *testing.T) { + server := httptest.NewServer(handler) + defer server.Close() + + provider := &KlipyProvider{BaseURL: server.URL, APIKey: "k"} + + if _, err := provider.Search(context.Background(), "cat", 1); err == nil { + t.Error("expected an error to be surfaced") + } + }) + } +} + +// TestKlipyLiveContract talks to the real API, skipped unless a key is set so +// CI stays offline. It is the only thing that catches the response shape +// changing: a canned payload written from the docs cannot. +// +// THOUGHTS_GIF_API_KEY=... go test ./cmd/thoughts/gif/ -run Live +func TestKlipyLiveContract(t *testing.T) { + key := os.Getenv("THOUGHTS_GIF_API_KEY") + if key == "" { + t.Skip("set THOUGHTS_GIF_API_KEY to check the live Klipy contract") + } + + provider := NewKlipyProvider(key) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + for _, tc := range []struct { + name string + call func() (*SearchPage, error) + }{ + {"search", func() (*SearchPage, error) { return provider.Search(ctx, "cat", 1) }}, + {"trending", func() (*SearchPage, error) { return provider.Trending(ctx, 1) }}, + } { + t.Run(tc.name, func(t *testing.T) { + page, err := tc.call() + if err != nil { + t.Fatalf("%s failed: %v", tc.name, err) + } + + if len(page.Results) == 0 { + t.Fatalf("%s returned no usable results - the response shape has probably changed", tc.name) + } + + for i, result := range page.Results { + if result.URL == "" || result.PreviewURL == "" { + t.Errorf("result %d has an empty url: %+v", i, result) + } + + if !strings.HasPrefix(result.URL, "https://") { + t.Errorf("result %d url is not https: %s", i, result.URL) + } + } + }) + } +} + +func TestGiphySearchMapsResults(t *testing.T) { + var gotQuery url.Values + + body, _ := json.Marshal(map[string]any{ + "data": []map[string]any{ + { + "id": "abc", + "images": map[string]any{ + "fixed_width": map[string]any{"url": "https://cdn.giphy/fw.gif", "width": "200", "height": "150"}, + "downsized_medium": map[string]any{"url": "https://cdn.giphy/dm.gif"}, + "original": map[string]any{"url": "https://cdn.giphy/orig.gif"}, + }, + }, + {"id": "no-images", "images": map[string]any{}}, + }, + "pagination": map[string]any{"total_count": 100, "count": 2, "offset": 24}, + }) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.Query() + _, _ = w.Write(body) + })) + defer server.Close() + + provider := &GiphyProvider{BaseURL: server.URL, APIKey: "giphy-key"} + + page, err := provider.Search(context.Background(), "cat", 2) + if err != nil { + t.Fatalf("search failed: %v", err) + } + + if gotQuery.Get("api_key") != "giphy-key" { + t.Errorf("api_key = %q", gotQuery.Get("api_key")) + } + + // Giphy paginates by offset rather than page number. + if gotQuery.Get("offset") != "24" { + t.Errorf("offset = %q, want 24", gotQuery.Get("offset")) + } + + if len(page.Results) != 1 { + t.Fatalf("got %d results, want 1", len(page.Results)) + } + + got := page.Results[0] + + if got.URL != "https://cdn.giphy/dm.gif" || got.PreviewURL != "https://cdn.giphy/fw.gif" { + t.Errorf("mapping produced %+v", got) + } + + if got.Width != 200 || got.Height != 150 { + t.Errorf("dimensions = %dx%d, want 200x150", got.Width, got.Height) + } + + if !page.HasNext { + t.Error("expected more pages given 100 total results") + } +} + +func TestResolve(t *testing.T) { + cases := []struct { + name string + provider string + apiKey string + wantName string + wantNil bool + wantErrPart string + }{ + {name: "auto with a key picks klipy", apiKey: "k", wantName: ProviderKlipy}, + {name: "auto without a key disables search", wantNil: true}, + {name: "explicit none", provider: ProviderNone, apiKey: "k", wantNil: true}, + {name: "explicit klipy", provider: ProviderKlipy, apiKey: "k", wantName: ProviderKlipy}, + {name: "explicit giphy", provider: ProviderGiphy, apiKey: "k", wantName: ProviderGiphy}, + {name: "klipy without a key", provider: ProviderKlipy, wantErrPart: "THOUGHTS_GIF_API_KEY"}, + {name: "unknown provider", provider: "tenor", apiKey: "k", wantErrPart: "unknown gif provider"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + provider, err := Resolve(tc.provider, tc.apiKey) + + if tc.wantErrPart != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErrPart) { + t.Fatalf("expected an error containing %q, got %v", tc.wantErrPart, err) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tc.wantNil { + if provider != nil { + t.Errorf("expected search to be disabled, got %s", provider.Name()) + } + + if SearchAvailable() { + t.Error("SearchAvailable should be false with no provider") + } + + return + } + + if provider == nil { + t.Fatal("expected a provider") + } + + if provider.Name() != tc.wantName { + t.Errorf("provider = %s, want %s", provider.Name(), tc.wantName) + } + + if !SearchAvailable() { + t.Error("SearchAvailable should be true with a provider") + } + }) + } +} diff --git a/cmd/thoughts/gif/giphy.go b/cmd/thoughts/gif/giphy.go new file mode 100644 index 0000000..8efc47e --- /dev/null +++ b/cmd/thoughts/gif/giphy.go @@ -0,0 +1,145 @@ +package gif + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strconv" +) + +type GiphyProvider struct { + BaseURL string + APIKey string +} + +var _ Provider = (*GiphyProvider)(nil) + +const giphyBaseURL = "https://api.giphy.com/v1" + +func NewGiphyProvider(apiKey string) *GiphyProvider { + return &GiphyProvider{ + BaseURL: giphyBaseURL, + APIKey: apiKey, + } +} + +func (g *GiphyProvider) Name() string { + return ProviderGiphy +} + +func (g *GiphyProvider) Search(ctx context.Context, query string, page int) (*SearchPage, error) { + return g.fetch(ctx, "gifs/search", url.Values{"q": []string{query}}, page) +} + +func (g *GiphyProvider) Trending(ctx context.Context, page int) (*SearchPage, error) { + return g.fetch(ctx, "gifs/trending", url.Values{}, page) +} + +type ( + giphyImage struct { + URL string `json:"url"` + Width string `json:"width"` + Height string `json:"height"` + } + + giphyItem struct { + ID string `json:"id"` + Images struct { + FixedWidth giphyImage `json:"fixed_width"` + DownsizedMedium giphyImage `json:"downsized_medium"` + Original giphyImage `json:"original"` + } `json:"images"` + } + + giphyResponse struct { + Data []giphyItem `json:"data"` + Pagination struct { + TotalCount int `json:"total_count"` + Count int `json:"count"` + Offset int `json:"offset"` + } `json:"pagination"` + } +) + +func (g *GiphyProvider) fetch(ctx context.Context, path string, params url.Values, page int) (*SearchPage, error) { + page = normalisePage(page) + + endpoint, err := url.JoinPath(g.BaseURL, path) + if err != nil { + return nil, fmt.Errorf("failed to build giphy url: %w", err) + } + + offset := (page - 1) * perPage + + params.Set("api_key", g.APIKey) + params.Set("limit", strconv.Itoa(perPage)) + params.Set("offset", strconv.Itoa(offset)) + params.Set("rating", "pg-13") + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint+"?"+params.Encode(), nil) + if err != nil { + return nil, fmt.Errorf("failed to create giphy request: %w", err) + } + + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to reach giphy: %w", err) + } + + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("giphy responded with status %d", resp.StatusCode) + } + + var body giphyResponse + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + return nil, fmt.Errorf("failed to decode giphy response: %w", err) + } + + results := make([]SearchResult, 0, len(body.Data)) + + for _, item := range body.Data { + if result, ok := item.toResult(); ok { + results = append(results, result) + } + } + + return &SearchPage{ + Results: results, + Page: page, + HasNext: offset+len(body.Data) < body.Pagination.TotalCount, + }, nil +} + +func (i giphyItem) toResult() (SearchResult, bool) { + full := firstImage(i.Images.DownsizedMedium, i.Images.Original, i.Images.FixedWidth) + if full.URL == "" { + return SearchResult{}, false + } + + preview := firstImage(i.Images.FixedWidth, full) + + // Giphy reports dimensions as strings. + width, _ := strconv.Atoi(preview.Width) + height, _ := strconv.Atoi(preview.Height) + + return SearchResult{ + PreviewURL: preview.URL, + URL: full.URL, + Width: width, + Height: height, + }, true +} + +func firstImage(images ...giphyImage) giphyImage { + for _, image := range images { + if image.URL != "" { + return image + } + } + + return giphyImage{} +} diff --git a/cmd/thoughts/gif/klipy.go b/cmd/thoughts/gif/klipy.go new file mode 100644 index 0000000..fc68103 --- /dev/null +++ b/cmd/thoughts/gif/klipy.go @@ -0,0 +1,150 @@ +package gif + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strconv" +) + +type KlipyProvider struct { + BaseURL string + APIKey string +} + +var _ Provider = (*KlipyProvider)(nil) + +const klipyBaseURL = "https://api.klipy.com/api/v1" + +func NewKlipyProvider(apiKey string) *KlipyProvider { + return &KlipyProvider{ + BaseURL: klipyBaseURL, + APIKey: apiKey, + } +} + +func (k *KlipyProvider) Name() string { + return ProviderKlipy +} + +func (k *KlipyProvider) Search(ctx context.Context, query string, page int) (*SearchPage, error) { + return k.fetch(ctx, "gifs/search", url.Values{"q": []string{query}}, page) +} + +func (k *KlipyProvider) Trending(ctx context.Context, page int) (*SearchPage, error) { + return k.fetch(ctx, "gifs/trending", url.Values{}, page) +} + +type ( + klipyFile struct { + URL string `json:"url"` + Width int `json:"width"` + Height int `json:"height"` + } + + // Size first, then format, under "file" rather than "files". + klipyFormats struct { + Gif klipyFile `json:"gif"` + Webp klipyFile `json:"webp"` + } + + klipyItem struct { + Slug string `json:"slug"` + Type string `json:"type"` + File struct { + HD klipyFormats `json:"hd"` + MD klipyFormats `json:"md"` + SM klipyFormats `json:"sm"` + XS klipyFormats `json:"xs"` + } `json:"file"` + } + + klipyResponse struct { + Result bool `json:"result"` + Data struct { + Data []klipyItem `json:"data"` + CurrentPage int `json:"current_page"` + HasNext bool `json:"has_next"` + } `json:"data"` + } +) + +func (k *KlipyProvider) fetch(ctx context.Context, path string, params url.Values, page int) (*SearchPage, error) { + page = normalisePage(page) + + endpoint, err := url.JoinPath(k.BaseURL, k.APIKey, path) + if err != nil { + return nil, fmt.Errorf("failed to build klipy url: %w", err) + } + + params.Set("per_page", strconv.Itoa(perPage)) + params.Set("page", strconv.Itoa(page)) + params.Set("rating", "pg-13") + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint+"?"+params.Encode(), nil) + if err != nil { + return nil, fmt.Errorf("failed to create klipy request: %w", err) + } + + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to reach klipy: %w", err) + } + + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("klipy responded with status %d", resp.StatusCode) + } + + var body klipyResponse + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + return nil, fmt.Errorf("failed to decode klipy response: %w", err) + } + + if !body.Result { + return nil, fmt.Errorf("klipy reported a failed request") + } + + results := make([]SearchResult, 0, len(body.Data.Data)) + + for _, item := range body.Data.Data { + if result, ok := item.toResult(); ok { + results = append(results, result) + } + } + + return &SearchPage{ + Results: results, + Page: page, + HasNext: body.Data.HasNext, + }, nil +} + +func (i klipyItem) toResult() (SearchResult, bool) { + full := firstFile(i.File.MD.Gif, i.File.HD.Gif, i.File.SM.Gif) + if full.URL == "" { + return SearchResult{}, false + } + + preview := firstFile(i.File.SM.Webp, i.File.XS.Webp, i.File.SM.Gif, full) + + return SearchResult{ + PreviewURL: preview.URL, + URL: full.URL, + Width: preview.Width, + Height: preview.Height, + }, true +} + +func firstFile(files ...klipyFile) klipyFile { + for _, file := range files { + if file.URL != "" { + return file + } + } + + return klipyFile{} +} diff --git a/cmd/thoughts/gif/provider.go b/cmd/thoughts/gif/provider.go index a0f040c..7d9909a 100644 --- a/cmd/thoughts/gif/provider.go +++ b/cmd/thoughts/gif/provider.go @@ -1,3 +1,6 @@ +// Package gif provides GIF search behind a swappable provider. Google shut the +// Tenor API down on 2026-06-30, so the backend is now configurable rather than +// hard-wired to one vendor. package gif import "context" @@ -5,8 +8,28 @@ import "context" type SearchResult struct { PreviewURL string `json:"preview_url"` URL string `json:"url"` + Width int `json:"width,omitempty"` + Height int `json:"height,omitempty"` +} + +type SearchPage struct { + Results []SearchResult `json:"results"` + Page int `json:"page"` + HasNext bool `json:"has_next"` } type Provider interface { - Search(ctx context.Context, query string) ([]SearchResult, error) + Name() string + Search(ctx context.Context, query string, page int) (*SearchPage, error) + Trending(ctx context.Context, page int) (*SearchPage, error) +} + +const perPage = 24 + +func normalisePage(page int) int { + if page < 1 { + return 1 + } + + return page } diff --git a/cmd/thoughts/gif/registry.go b/cmd/thoughts/gif/registry.go new file mode 100644 index 0000000..0d9ab2d --- /dev/null +++ b/cmd/thoughts/gif/registry.go @@ -0,0 +1,73 @@ +package gif + +import ( + "fmt" + "log/slog" + "net/http" + "time" +) + +const ( + ProviderKlipy = "klipy" + ProviderGiphy = "giphy" + ProviderNone = "none" +) + +// http.DefaultClient has no timeout, so a hung provider would pin a goroutine. +var httpClient = &http.Client{Timeout: 8 * time.Second} + +var searchAvailable bool + +// SearchAvailable reports whether GIF search is configured. +func SearchAvailable() bool { + return searchAvailable +} + +func Resolve(name, apiKey string) (Provider, error) { + provider, err := build(name, apiKey) + if err != nil { + return nil, err + } + + searchAvailable = provider != nil + + if provider == nil { + slog.Info("gif search disabled, paste a link instead") + } else { + slog.Info("gif search enabled", "provider", provider.Name()) + } + + return provider, nil +} + +func build(name, apiKey string) (Provider, error) { + switch name { + case ProviderNone: + return nil, nil + + case "": + if apiKey == "" { + return nil, nil + } + + return NewKlipyProvider(apiKey), nil + + case ProviderKlipy: + if apiKey == "" { + return nil, fmt.Errorf("gif provider %q needs THOUGHTS_GIF_API_KEY", name) + } + + return NewKlipyProvider(apiKey), nil + + case ProviderGiphy: + if apiKey == "" { + return nil, fmt.Errorf("gif provider %q needs THOUGHTS_GIF_API_KEY", name) + } + + return NewGiphyProvider(apiKey), nil + + default: + return nil, fmt.Errorf("unknown gif provider %q, expected one of %q, %q or %q", + name, ProviderKlipy, ProviderGiphy, ProviderNone) + } +} diff --git a/cmd/thoughts/gif/tenor.go b/cmd/thoughts/gif/tenor.go deleted file mode 100644 index a3e13af..0000000 --- a/cmd/thoughts/gif/tenor.go +++ /dev/null @@ -1,86 +0,0 @@ -package gif - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "net/url" - - "github.com/samber/lo" -) - -type TenorProvider struct { - URL string - APIKey string -} - -var _ Provider = (*TenorProvider)(nil) - -func NewTenorProvider(apiKey string) *TenorProvider { - return &TenorProvider{ - URL: "https://tenor.googleapis.com", - APIKey: apiKey, - } -} - -type ( - tenorMediaFormat struct { - URL string `json:"url"` - } - - tenorResult struct { - ID string `json:"id"` - MediaFormats struct { - MediumGif tenorMediaFormat `json:"mediumgif"` - Gif tenorMediaFormat `json:"gif"` - } `json:"media_formats"` - } - - tenorSearchResponse struct { - Results []tenorResult `json:"results"` - } -) - -func (t *TenorProvider) Search(ctx context.Context, query string) ([]SearchResult, error) { - searchURL, err := url.JoinPath(t.URL, "/v2/search") - if err != nil { - return nil, fmt.Errorf("failed to join url path for tenor search: %w", err) - } - - searchURL += "?" + url.Values{ - "q": []string{query}, - "key": []string{t.APIKey}, - "limit": []string{"9"}, - "contentfilter": []string{"medium"}, - "media_filter": []string{"gif,mediumgif"}, - }.Encode() - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, searchURL, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request for tenor search: %w", err) - } - - resp, err := http.DefaultClient.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to perform request for tenor search: %w", err) - } - - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("tenor search failed with status code %d", resp.StatusCode) - } - - var searchResp tenorSearchResponse - if err := json.NewDecoder(resp.Body).Decode(&searchResp); err != nil { - return nil, fmt.Errorf("failed to decode tenor search response: %w", err) - } - - return lo.Map(searchResp.Results, func(res tenorResult, _ int) SearchResult { - return SearchResult{ - PreviewURL: res.MediaFormats.MediumGif.URL, - URL: res.MediaFormats.Gif.URL, - } - }), nil -} diff --git a/cmd/thoughts/main.go b/cmd/thoughts/main.go index ed905bd..9cf25ae 100644 --- a/cmd/thoughts/main.go +++ b/cmd/thoughts/main.go @@ -11,7 +11,6 @@ import ( "github.com/ellgreen/thoughts/cmd/thoughts/gif" "github.com/ellgreen/thoughts/cmd/thoughts/session" "github.com/ellgreen/thoughts/migrations" - "github.com/ellgreen/thoughts/ui" "github.com/gorilla/mux" "github.com/jmoiron/sqlx" "github.com/pressly/goose/v3" @@ -46,35 +45,48 @@ func main() { os.Exit(1) } + servesTLS := cfg.TLSCertPath != "" && cfg.TLSKeyPath != "" + sessionKeyPath := filepath.Join(cfg.DataPath, cfg.SessionKeyFile) - sessionProvider, err := session.LoadSessionProvider(sessionKeyPath) + sessionProvider, err := session.LoadSessionProvider(sessionKeyPath, servesTLS) if err != nil { slog.Error("failed to load session provider", "err", err) os.Exit(1) } + if cfg.TenorAPIKey != "" { + slog.Warn("THOUGHTS_TENOR_API_KEY is set but the Tenor API was shut down on 2026-06-30; " + + "use THOUGHTS_GIF_PROVIDER and THOUGHTS_GIF_API_KEY instead") + } + + gifProvider, err := gif.Resolve(cfg.GIFProvider, cfg.GIFAPIKey) + if err != nil { + slog.Error("failed to configure gif provider", "err", err) + os.Exit(1) + } + router := mux.NewRouter() applyRoutes( router, sessionProvider, ai.ResolveModel(cfg.OpenAIAPIKey), - gif.ResolveProvider(cfg.TenorAPIKey), + gifProvider, ) - corsCfg := cors.Default() - if !ui.IsBundled() { - corsCfg = cors.New(cors.Options{ - AllowedOrigins: []string{cfg.UIAddress}, - AllowedMethods: []string{"GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"}, - AllowedHeaders: []string{"Authorization", "Content-Type"}, - AllowCredentials: true, - }) - } - - handler := corsCfg.Handler(router) - - if cfg.TLSCertPath != "" && cfg.TLSKeyPath != "" { + // Always credential-aware, bundled or not. A wildcard origin cannot carry + // credentials at all, so a browser pointed at the Vite dev server while a + // bundled binary served the API would silently drop the session cookie: + // the login looked like a 200 in the network tab and every request after + // it came back 401. + handler := cors.New(cors.Options{ + AllowedOrigins: []string{cfg.UIAddress}, + AllowedMethods: []string{"GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"}, + AllowedHeaders: []string{"Authorization", "Content-Type"}, + AllowCredentials: true, + }).Handler(router) + + if servesTLS { slog.Info("starting server (with tls)", "addr", cfg.Address) if err := http.ListenAndServeTLS(cfg.Address, cfg.TLSCertPath, cfg.TLSKeyPath, handler); err != nil { diff --git a/cmd/thoughts/model/retro.go b/cmd/thoughts/model/retro.go index 24251a5..6ffd688 100644 --- a/cmd/thoughts/model/retro.go +++ b/cmd/thoughts/model/retro.go @@ -54,6 +54,29 @@ func (r *Retro) IsBrainstorming() bool { return r.Status == RetroStatusBrainstorm } +func (rc RetroColumns) Find(id uuid.UUID) *RetroColumn { + for _, column := range rc { + if column.ID == id { + return column + } + } + + return nil +} + +// Without returns a copy with the given column removed. +func (rc RetroColumns) Without(id uuid.UUID) RetroColumns { + remaining := make(RetroColumns, 0, len(rc)) + + for _, column := range rc { + if column.ID != id { + remaining = append(remaining, column) + } + } + + return remaining +} + func (rc *RetroColumns) AssignIDs() *RetroColumns { for _, c := range *rc { c.ID = uuid.New() diff --git a/cmd/thoughts/requests/map_test.go b/cmd/thoughts/requests/map_test.go new file mode 100644 index 0000000..1f3d49d --- /dev/null +++ b/cmd/thoughts/requests/map_test.go @@ -0,0 +1,135 @@ +package requests + +import ( + "testing" + + "github.com/google/uuid" +) + +type target struct { + Name string `json:"name" validate:"required,min=2"` + Count int `json:"count"` + ID uuid.UUID `json:"id"` + Tags []string `json:"tags"` + Flag bool `json:"flag"` + Untagged string +} + +func TestFromMapBindsSupportedTypes(t *testing.T) { + id := uuid.New() + + got, err := FromMap[target](map[string]any{ + "name": "a name", + "count": float64(7), // JSON numbers decode as float64 + "id": id.String(), + "tags": []any{"one", "two"}, + "flag": true, + }) + + if err != nil { + t.Fatalf("expected the payload to bind, got %v", err) + } + + if got.Name != "a name" { + t.Errorf("Name = %q, want %q", got.Name, "a name") + } + + if got.Count != 7 { + t.Errorf("Count = %d, want 7", got.Count) + } + + if got.ID != id { + t.Errorf("ID = %s, want %s", got.ID, id) + } + + if len(got.Tags) != 2 || got.Tags[0] != "one" || got.Tags[1] != "two" { + t.Errorf("Tags = %v, want [one two]", got.Tags) + } + + if !got.Flag { + t.Error("Flag = false, want true") + } +} + +func TestFromMapAcceptsIntsAsStrings(t *testing.T) { + got, err := FromMap[target](map[string]any{"name": "ok", "count": "42"}) + if err != nil { + t.Fatalf("expected a numeric string to bind, got %v", err) + } + + if got.Count != 42 { + t.Errorf("Count = %d, want 42", got.Count) + } +} + +func TestFromMapLeavesAbsentKeysAtTheirZeroValue(t *testing.T) { + got, err := FromMap[target](map[string]any{"name": "ok"}) + if err != nil { + t.Fatalf("expected a partial payload to bind, got %v", err) + } + + if got.Count != 0 || got.ID != uuid.Nil || got.Tags != nil || got.Flag { + t.Errorf("absent keys were not left at their zero value: %+v", got) + } +} + +func TestFromMapIgnoresUnknownAndUntaggedFields(t *testing.T) { + got, err := FromMap[target](map[string]any{ + "name": "ok", + "Untagged": "should be ignored", + "nonsense": 1, + }) + + if err != nil { + t.Fatalf("expected unknown keys to be ignored, got %v", err) + } + + if got.Untagged != "" { + t.Errorf("a field with no json tag was bound: %q", got.Untagged) + } +} + +func TestFromMapRejectsBadValues(t *testing.T) { + cases := map[string]map[string]any{ + "unparseable int": {"name": "ok", "count": "not a number"}, + "int from bool": {"name": "ok", "count": true}, + "unparseable uuid": {"name": "ok", "id": "tasks"}, + "uuid from number": {"name": "ok", "id": float64(3)}, + "slice of non-string": {"name": "ok", "tags": []any{1, 2}}, + "slice from string": {"name": "ok", "tags": "one"}, + } + + for name, payload := range cases { + t.Run(name, func(t *testing.T) { + if _, err := FromMap[target](payload); err == nil { + t.Errorf("expected %v to be rejected", payload) + } + }) + } +} + +func TestFromMapRunsValidation(t *testing.T) { + if _, err := FromMap[target](map[string]any{}); err == nil { + t.Error("expected a missing required field to be rejected") + } + + _, err := FromMap[target](map[string]any{"name": "a"}) + if err == nil { + t.Fatal("expected a too-short value to be rejected") + } + + if err.Error() != "Name should contain more than 2 characters" { + t.Errorf("unexpected validation message: %q", err.Error()) + } +} + +func TestFromMapTreatsAnEmptyUUIDStringAsNil(t *testing.T) { + got, err := FromMap[target](map[string]any{"name": "ok", "id": ""}) + if err != nil { + t.Fatalf("expected an empty uuid to bind as nil, got %v", err) + } + + if got.ID != uuid.Nil { + t.Errorf("ID = %s, want the nil UUID", got.ID) + } +} diff --git a/cmd/thoughts/requests/requests.go b/cmd/thoughts/requests/requests.go index 88a358c..a8e39a1 100644 --- a/cmd/thoughts/requests/requests.go +++ b/cmd/thoughts/requests/requests.go @@ -39,6 +39,12 @@ func validationMessage(validationErrors validator.ValidationErrors) string { fieldMessages = append(fieldMessages, fmt.Sprintf("%s should be less than %s characters", err.Field(), err.Param())) case "uuid": fieldMessages = append(fieldMessages, fmt.Sprintf("%s is not a valid UUID", err.Field())) + case "url": + fieldMessages = append(fieldMessages, fmt.Sprintf("%s is not a valid URL", err.Field())) + case "startswith": + fieldMessages = append(fieldMessages, fmt.Sprintf("%s should start with %s", err.Field(), err.Param())) + case "oneof": + fieldMessages = append(fieldMessages, fmt.Sprintf("%s should be one of: %s", err.Field(), err.Param())) default: fieldMessages = append(fieldMessages, fmt.Sprintf("%s is invalid", err.Field())) } diff --git a/cmd/thoughts/resources/retro.go b/cmd/thoughts/resources/retro.go index 040cac3..11cc998 100644 --- a/cmd/thoughts/resources/retro.go +++ b/cmd/thoughts/resources/retro.go @@ -22,7 +22,7 @@ type Retro struct { Columns []*RetroColumn `json:"columns"` Unlisted bool `json:"unlisted"` MaxVotes int `json:"max_votes"` - GIFsEnabled bool `json:"gifs_enabled"` + GIFSearchEnabled bool `json:"gif_search_enabled"` Tags []string `json:"tags"` CreatedAt time.Time `json:"created_at"` NoteCount int `json:"note_count"` @@ -49,7 +49,7 @@ func RetroFromModel(m *model.Retro) *Retro { }), Unlisted: m.Unlisted, MaxVotes: m.MaxVotes, - GIFsEnabled: gif.IsAvailable(), + GIFSearchEnabled: gif.SearchAvailable(), Tags: tags, CreatedAt: m.CreatedAt, NoteCount: m.NoteCount, diff --git a/cmd/thoughts/routes.go b/cmd/thoughts/routes.go index 4ec3ad9..3d6a220 100644 --- a/cmd/thoughts/routes.go +++ b/cmd/thoughts/routes.go @@ -40,7 +40,7 @@ func applyRoutes( authRouter.Handle("/tags", controllers.TagSuggestions(db)).Methods(http.MethodGet) authRouter.Handle("/tags/{tag}", controllers.TagRetros(db)).Methods(http.MethodGet) - authRouter.Handle("/gifs", controllers.GifSearch(gifProvider)).Methods(http.MethodPost) + authRouter.Handle("/gifs", controllers.GifSearch(gifProvider)).Methods(http.MethodGet) aiRouter := authRouter.PathPrefix("/ai").Subrouter() aiRouter.Handle("/retro-template", controllers.AIRetroTemplate(aiModel)).Methods(http.MethodPost) @@ -52,7 +52,14 @@ func applyRoutes( retroRouter := retrosRouter.PathPrefix(fmt.Sprintf("/{id:%s}", uuidRegex)).Subrouter() - retroRouter.Handle("/ws", socket.NewRetroSocketHandler(db)) + // The Vite dev server runs on a different origin, so it needs allowing + // explicitly. A bundled build is always same-origin. + devOrigin := "" + if !ui.IsBundled() { + devOrigin = cfg.UIAddress + } + + retroRouter.Handle("/ws", socket.NewRetroSocketHandler(db, devOrigin)) retroRouter.Handle("/notes", controllers.RetroNotesIndex(db)).Methods(http.MethodGet) retroRouter.Handle("/votes", controllers.VotesIndex(db)).Methods(http.MethodGet) retroRouter.Handle("/votes", controllers.Vote(db)).Methods(http.MethodPost) diff --git a/cmd/thoughts/session/session.go b/cmd/thoughts/session/session.go index 3b4d656..b4ed5e0 100644 --- a/cmd/thoughts/session/session.go +++ b/cmd/thoughts/session/session.go @@ -6,27 +6,64 @@ import ( "log/slog" "net/http" "os" + "strings" "github.com/google/uuid" "github.com/gorilla/securecookie" "github.com/gorilla/sessions" ) -const keyLength = 64 +const ( + keyLength = 64 + + sessionMaxAge = 86400 * 30 +) var ErrValueNotFound = errors.New("session: value not found") type Provider struct { store sessions.Store + + tls bool } -func LoadSessionProvider(keyPath string) (*Provider, error) { +func LoadSessionProvider(keyPath string, tls bool) (*Provider, error) { key, err := loadKey(keyPath) if err != nil { return nil, err } - return &Provider{store: sessions.NewCookieStore(key)}, nil + store := sessions.NewCookieStore(key) + + // gorilla/sessions defaults to Secure with SameSite=None, so the cookie is + // only ever stored over https. Chrome excepts http://localhost; Safari + // does not, which left people logged out after a login that looked fine. + store.Options = defaultOptions(tls) + + return &Provider{store: store, tls: tls}, nil +} + +func defaultOptions(secure bool) *sessions.Options { + return &sessions.Options{ + Path: "/", + MaxAge: sessionMaxAge, + HttpOnly: true, + // Not None, which would drag the Secure requirement back in with it. + SameSite: http.SameSiteLaxMode, + Secure: secure, + } +} + +func (sp *Provider) optionsFor(r *http.Request) *sessions.Options { + return defaultOptions(sp.tls || isHTTPS(r)) +} + +func isHTTPS(r *http.Request) bool { + if r.TLS != nil { + return true + } + + return strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") } func loadKey(path string) ([]byte, error) { @@ -53,12 +90,16 @@ func loadKey(path string) ([]byte, error) { func (sp *Provider) Get(w http.ResponseWriter, r *http.Request) (*sessions.Session, error) { sess, err := sp.store.Get(r, "session") + if sess != nil { + sess.Options = sp.optionsFor(r) + } + if err != nil { if sess == nil { return nil, fmt.Errorf("failed to get session: %w", err) } - slog.Warn("failed to get session, creating a new one", "err", err) + slog.Debug("could not decode the session cookie, starting a new one", "err", err) if err := sess.Save(r, w); err != nil { return nil, fmt.Errorf("failed to save session after retry: %w", err) diff --git a/cmd/thoughts/session/session_test.go b/cmd/thoughts/session/session_test.go new file mode 100644 index 0000000..a840c14 --- /dev/null +++ b/cmd/thoughts/session/session_test.go @@ -0,0 +1,136 @@ +package session_test + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/session" + "github.com/google/uuid" +) + +func newProvider(t *testing.T, tls bool) *session.Provider { + t.Helper() + + sp, err := session.LoadSessionProvider(filepath.Join(t.TempDir(), "session.key"), tls) + if err != nil { + t.Fatalf("failed to load session provider: %v", err) + } + + return sp +} + +func issue(t *testing.T, sp *session.Provider, req *http.Request) *http.Cookie { + t.Helper() + + recorder := httptest.NewRecorder() + + if err := sp.AddUserID(recorder, req, uuid.New()); err != nil { + t.Fatalf("failed to save session: %v", err) + } + + cookies := recorder.Result().Cookies() + if len(cookies) == 0 { + t.Fatal("no cookie was set") + } + + return cookies[len(cookies)-1] +} + +func TestCookieIsNotSecureOverPlainHTTP(t *testing.T) { + // gorilla/sessions defaults to Secure with SameSite=None, so the cookie was + // only ever stored over https. Chrome lets http://localhost away with it, + // Safari does not, and the login silently failed to stick. + cookie := issue(t, newProvider(t, false), httptest.NewRequest(http.MethodPost, "/api/auth/login", nil)) + + if cookie.Secure { + t.Error("cookie is marked Secure on a plain http server, so browsers will discard it") + } + + if cookie.SameSite == http.SameSiteNoneMode { + t.Error("SameSite=None requires Secure, which drags the same problem back in") + } +} + +func TestCookieIsSecureWhenServingTLS(t *testing.T) { + cookie := issue(t, newProvider(t, true), httptest.NewRequest(http.MethodPost, "/api/auth/login", nil)) + + if !cookie.Secure { + t.Error("cookie should be Secure when the server terminates TLS") + } +} + +func TestCookieIsSecureBehindAnHTTPSProxy(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/auth/login", nil) + req.Header.Set("X-Forwarded-Proto", "https") + + cookie := issue(t, newProvider(t, false), req) + + if !cookie.Secure { + t.Error("cookie should be Secure when the request arrived over https") + } +} + +func TestCookieIsHttpOnly(t *testing.T) { + cookie := issue(t, newProvider(t, false), httptest.NewRequest(http.MethodPost, "/api/auth/login", nil)) + + if !cookie.HttpOnly { + t.Error("the session cookie is never read from JavaScript, so it should be HttpOnly") + } +} + +func TestRoundTripsTheUserID(t *testing.T) { + sp := newProvider(t, false) + + userID := uuid.New() + + recorder := httptest.NewRecorder() + if err := sp.AddUserID(recorder, httptest.NewRequest(http.MethodPost, "/api/auth/login", nil), userID); err != nil { + t.Fatalf("failed to save session: %v", err) + } + + next := httptest.NewRequest(http.MethodGet, "/api/retros", nil) + for _, c := range recorder.Result().Cookies() { + next.AddCookie(c) + } + + got, err := sp.GetUserID(httptest.NewRecorder(), next) + if err != nil { + t.Fatalf("failed to read the session back: %v", err) + } + + if got != userID { + t.Errorf("got %s, want %s", got, userID) + } +} + +func TestForgetClearsTheUserID(t *testing.T) { + sp := newProvider(t, false) + + recorder := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/auth/login", nil) + + if err := sp.AddUserID(recorder, req, uuid.New()); err != nil { + t.Fatalf("failed to save session: %v", err) + } + + loggedIn := httptest.NewRequest(http.MethodPost, "/api/auth/logout", nil) + for _, c := range recorder.Result().Cookies() { + loggedIn.AddCookie(c) + } + + out := httptest.NewRecorder() + if err := sp.ForgetUserID(out, loggedIn); err != nil { + t.Fatalf("failed to forget: %v", err) + } + + after := httptest.NewRequest(http.MethodGet, "/api/retros", nil) + for _, c := range out.Result().Cookies() { + after.AddCookie(c) + } + + if _, err := sp.GetUserID(httptest.NewRecorder(), after); err == nil { + t.Error("expected the session to no longer name a user") + } +} diff --git a/cmd/thoughts/socket/client.go b/cmd/thoughts/socket/client.go index a0ec07e..455b153 100644 --- a/cmd/thoughts/socket/client.go +++ b/cmd/thoughts/socket/client.go @@ -12,17 +12,17 @@ import ( ) const ( - // Time allowed to write a message to the peer. writeWait = 10 * time.Second - // Time allowed to read the next pong message from the peer. pongWait = 60 * time.Second - // Send pings to peer with this period. Must be less than pongWait. pingPeriod = (pongWait * 9) / 10 - // Maximum message size allowed from peer. - maxMessageSize = 512 + // Maximum message size allowed from peer. A retro_update with a full + // title and ten tags is already well over 512 bytes. + maxMessageSize = 4096 + + sendBufferSize = 64 ) type Client struct { @@ -37,7 +37,7 @@ func NewClient(hub *Hub, conn *websocket.Conn, user *model.User) *Client { hub: hub, conn: conn, user: user, - send: make(chan []byte), + send: make(chan []byte, sendBufferSize), } } @@ -69,8 +69,13 @@ func (c *Client) ReadPump(ctx context.Context) { } } -func (c *Client) Send(message []byte) { - c.send <- message +func (c *Client) Send(message []byte) bool { + select { + case c.send <- message: + return true + default: + return false + } } func (c *Client) WritePump() { diff --git a/cmd/thoughts/socket/hub.go b/cmd/thoughts/socket/hub.go index ee9d617..f1ad67c 100644 --- a/cmd/thoughts/socket/hub.go +++ b/cmd/thoughts/socket/hub.go @@ -1,6 +1,8 @@ package socket import ( + "log/slog" + "github.com/ellgreen/thoughts/cmd/thoughts/event" ) @@ -42,20 +44,40 @@ func (h *Hub) Run() { h.connectionInfoSync() case event := <-h.broker.Listen(): - for client := range h.clients { - client.Send(event.ToJSON()) - } + h.broadcast(event.ToJSON()) case userDependentEvent := <-h.broker.ListenUserDependent(): for client := range h.clients { evt := userDependentEvent(client.user) - if evt != nil { - client.Send(evt.ToJSON()) + if evt == nil { + continue + } + + if !client.Send(evt.ToJSON()) { + h.drop(client) } } } } } +func (h *Hub) broadcast(data []byte) { + for client := range h.clients { + if !client.Send(data) { + h.drop(client) + } + } +} + +// drop removes a client whose send buffer has filled up. Its read pump will +// also unregister once the connection tears down; Run's unregister case +// tolerates a client that is already gone. +func (h *Hub) drop(client *Client) { + slog.Warn("dropping stalled websocket client", "user", client.user.Name) + + delete(h.clients, client) + close(client.send) +} + func (h *Hub) userNames() []string { users := make([]string, 0, len(h.clients)) for client := range h.clients { @@ -66,9 +88,5 @@ func (h *Hub) userNames() []string { } func (h *Hub) connectionInfoSync() { - data := event.NewConnectionInfoEvent(h.userNames()).ToJSON() - - for client := range h.clients { - client.Send(data) - } + h.broadcast(event.NewConnectionInfoEvent(h.userNames()).ToJSON()) } diff --git a/cmd/thoughts/socket/socket.go b/cmd/thoughts/socket/socket.go index 94392fb..cd5940e 100644 --- a/cmd/thoughts/socket/socket.go +++ b/cmd/thoughts/socket/socket.go @@ -4,6 +4,8 @@ import ( "context" "log/slog" "net/http" + "net/url" + "strings" "sync" "github.com/ellgreen/thoughts/cmd/thoughts/auth" @@ -14,13 +16,11 @@ import ( "github.com/jmoiron/sqlx" ) -func NewRetroSocketHandler(db *sqlx.DB) http.HandlerFunc { +func NewRetroSocketHandler(db *sqlx.DB, devOrigin string) http.HandlerFunc { upgrader := websocket.Upgrader{ ReadBufferSize: 1024, WriteBufferSize: 1024, - CheckOrigin: func(r *http.Request) bool { - return true //TODO: Only do this in development - }, + CheckOrigin: checkOrigin(devOrigin), } var hubs sync.Map @@ -71,3 +71,35 @@ func NewRetroSocketHandler(db *sqlx.DB) http.HandlerFunc { go client.WritePump() } } + +// checkOrigin rejects cross-site connections: sessions are cookie based, so +// any origin would let any page a logged-in user visits drive their retros. +// Requests with no Origin are not browsers and are allowed through. +func checkOrigin(devOrigin string) func(*http.Request) bool { + return func(r *http.Request) bool { + origin := r.Header.Get("Origin") + if origin == "" { + return true + } + + originURL, err := url.Parse(origin) + if err != nil { + slog.Warn("rejecting websocket with unparseable origin", "origin", origin) + return false + } + + if strings.EqualFold(originURL.Host, r.Host) { + return true + } + + if devOrigin != "" { + if devURL, err := url.Parse(devOrigin); err == nil && strings.EqualFold(originURL.Host, devURL.Host) { + return true + } + } + + slog.Warn("rejecting websocket from disallowed origin", "origin", origin, "host", r.Host) + + return false + } +} diff --git a/cmd/thoughts/testutil/db.go b/cmd/thoughts/testutil/db.go new file mode 100644 index 0000000..ff421cb --- /dev/null +++ b/cmd/thoughts/testutil/db.go @@ -0,0 +1,72 @@ +// Package testutil provides throwaway databases for tests. +package testutil + +import ( + "fmt" + "path/filepath" + "testing" + + "github.com/ellgreen/thoughts/migrations" + "github.com/jmoiron/sqlx" + "github.com/pressly/goose/v3" + _ "modernc.org/sqlite" +) + +// NewDB opens a throwaway SQLite database with every migration applied. It is +// removed with the test's temp directory. +func NewDB(t *testing.T) *sqlx.DB { + t.Helper() + + db := open(t) + + if err := goose.Up(db.DB, "."); err != nil { + t.Fatalf("failed to migrate test database: %v", err) + } + + return db +} + +// NewDBAt opens a throwaway database migrated only as far as the given goose +// version, so a migration can be exercised against data that predates it. +func NewDBAt(t *testing.T, version int64) *sqlx.DB { + t.Helper() + + db := open(t) + + if err := goose.UpTo(db.DB, ".", version); err != nil { + t.Fatalf("failed to migrate test database to %d: %v", version, err) + } + + return db +} + +// MigrateUp applies any migrations still outstanding on db. +func MigrateUp(t *testing.T, db *sqlx.DB) { + t.Helper() + + if err := goose.Up(db.DB, "."); err != nil { + t.Fatalf("failed to migrate test database: %v", err) + } +} + +func open(t *testing.T) *sqlx.DB { + t.Helper() + + goose.SetBaseFS(migrations.Embedded) + goose.SetLogger(goose.NopLogger()) + + if err := goose.SetDialect("sqlite3"); err != nil { + t.Fatalf("failed to set goose dialect: %v", err) + } + + dsn := fmt.Sprintf("file:%s?_foreign_keys=on", filepath.Join(t.TempDir(), "test.sqlite")) + + db, err := sqlx.Open("sqlite", dsn) + if err != nil { + t.Fatalf("failed to open test database: %v", err) + } + + t.Cleanup(func() { db.Close() }) + + return db +} diff --git a/cmd/thoughts/util/obfuscate.go b/cmd/thoughts/util/obfuscate.go index 9db85d8..6f5fe62 100644 --- a/cmd/thoughts/util/obfuscate.go +++ b/cmd/thoughts/util/obfuscate.go @@ -12,6 +12,9 @@ var ( alphaUpperCharSet = []rune("ABCDEFGHIJKLMNOPQRSTUVWXYZ") ) +// Obfuscate scrambles a string while preserving its length and the character +// class of each rune, so other people's notes read as noise of the right shape +// during the brainstorm stage. func Obfuscate(v string) string { var obfuscated strings.Builder @@ -24,15 +27,15 @@ func Obfuscate(v string) string { func obfuscateChar(c rune) rune { if slices.Contains(alphaLowerCharSet, c) { - return alphaLowerCharSet[rand.Intn(len(alphaLowerCharSet)-1)] + return alphaLowerCharSet[rand.Intn(len(alphaLowerCharSet))] } if slices.Contains(alphaUpperCharSet, c) { - return alphaUpperCharSet[rand.Intn(len(alphaUpperCharSet)-1)] + return alphaUpperCharSet[rand.Intn(len(alphaUpperCharSet))] } if slices.Contains(numCharSet, c) { - return numCharSet[rand.Intn(len(numCharSet)-1)] + return numCharSet[rand.Intn(len(numCharSet))] } return c diff --git a/cmd/thoughts/util/obfuscate_test.go b/cmd/thoughts/util/obfuscate_test.go new file mode 100644 index 0000000..665f481 --- /dev/null +++ b/cmd/thoughts/util/obfuscate_test.go @@ -0,0 +1,73 @@ +package util + +import ( + "strings" + "testing" + "unicode" +) + +func TestObfuscatePreservesShape(t *testing.T) { + input := "Sprint 9 went well - mostly! (a few blockers)" + got := Obfuscate(input) + + if len([]rune(got)) != len([]rune(input)) { + t.Fatalf("length changed: %d -> %d", len([]rune(input)), len([]rune(got))) + } + + for i, in := range []rune(input) { + out := []rune(got)[i] + + switch { + case unicode.IsLower(in) && in <= unicode.MaxASCII: + if !unicode.IsLower(out) { + t.Errorf("position %d: %q became %q, expected a lowercase letter", i, in, out) + } + case unicode.IsUpper(in) && in <= unicode.MaxASCII: + if !unicode.IsUpper(out) { + t.Errorf("position %d: %q became %q, expected an uppercase letter", i, in, out) + } + case unicode.IsDigit(in): + if !unicode.IsDigit(out) { + t.Errorf("position %d: %q became %q, expected a digit", i, in, out) + } + default: + // Punctuation and spacing are left alone so the text keeps its rhythm. + if out != in { + t.Errorf("position %d: %q became %q, expected it to be left alone", i, in, out) + } + } + } +} + +func TestObfuscateCanEmitEveryCharacterInAClass(t *testing.T) { + cases := map[string]struct { + input string + want rune + }{ + "lowercase": {strings.Repeat("a", 400), 'z'}, + "uppercase": {strings.Repeat("A", 400), 'Z'}, + "digits": {strings.Repeat("1", 400), '9'}, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + if !strings.ContainsRune(Obfuscate(tc.input), tc.want) { + t.Errorf("%q never appeared in 400 obfuscated runes", tc.want) + } + }) + } +} + +func TestObfuscateHidesTheOriginal(t *testing.T) { + input := "the quick brown fox jumps over the lazy dog and keeps on running" + + if Obfuscate(input) == input { + t.Error("obfuscated text came back unchanged") + } +} + +func TestObfuscateHandlesEmptyInput(t *testing.T) { + if got := Obfuscate(""); got != "" { + t.Errorf("expected an empty string, got %q", got) + } +} diff --git a/ui/README.md b/ui/README.md deleted file mode 100644 index 74872fd..0000000 --- a/ui/README.md +++ /dev/null @@ -1,50 +0,0 @@ -# React + TypeScript + Vite - -This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules. - -Currently, two official plugins are available: - -- [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/README.md) uses [Babel](https://babeljs.io/) for Fast Refresh -- [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh - -## Expanding the ESLint configuration - -If you are developing a production application, we recommend updating the configuration to enable type aware lint rules: - -- Configure the top-level `parserOptions` property like this: - -```js -export default tseslint.config({ - languageOptions: { - // other options... - parserOptions: { - project: ['./tsconfig.node.json', './tsconfig.app.json'], - tsconfigRootDir: import.meta.dirname, - }, - }, -}) -``` - -- Replace `tseslint.configs.recommended` to `tseslint.configs.recommendedTypeChecked` or `tseslint.configs.strictTypeChecked` -- Optionally add `...tseslint.configs.stylisticTypeChecked` -- Install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) and update the config: - -```js -// eslint.config.js -import react from 'eslint-plugin-react' - -export default tseslint.config({ - // Set the react version - settings: { react: { version: '18.3' } }, - plugins: { - // Add the react plugin - react, - }, - rules: { - // other rules... - // Enable its recommended rules - ...react.configs.recommended.rules, - ...react.configs['jsx-runtime'].rules, - }, -}) -``` diff --git a/ui/package.json b/ui/package.json index 72f8417..44277f0 100644 --- a/ui/package.json +++ b/ui/package.json @@ -8,6 +8,7 @@ "dev": "vite", "build": "tsc -b && vite build", "lint": "eslint .", + "test": "vitest run", "preview": "vite preview" }, "dependencies": { @@ -30,9 +31,9 @@ "axios": "^1.13.6", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", - "cmdk": "^1.1.1", "date-fns": "^3.6.0", "lucide-react": "^0.510.0", + "motion": "^13.1.0", "next-themes": "^0.4.6", "radix-ui": "^1.4.3", "react": "^19.2.4", @@ -62,6 +63,7 @@ "tw-animate-css": "^1.4.0", "typescript": "~5.6.3", "typescript-eslint": "^8.57.1", - "vite": "^6.4.1" + "vite": "^6.4.1", + "vitest": "^4.1.10" } } diff --git a/ui/pnpm-lock.yaml b/ui/pnpm-lock.yaml index 77e70ae..f16fb01 100644 --- a/ui/pnpm-lock.yaml +++ b/ui/pnpm-lock.yaml @@ -65,15 +65,15 @@ importers: clsx: specifier: ^2.1.1 version: 2.1.1 - cmdk: - specifier: ^1.1.1 - version: 1.1.1(@types/react-dom@19.2.3(@types/react@19.2.14))(@types/react@19.2.14)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) date-fns: specifier: ^3.6.0 version: 3.6.0 lucide-react: specifier: ^0.510.0 version: 0.510.0(react@19.2.4) + motion: + specifier: ^13.1.0 + version: 13.1.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) next-themes: specifier: ^0.4.6 version: 0.4.6(react-dom@19.2.4(react@19.2.4))(react@19.2.4) @@ -159,6 +159,9 @@ importers: vite: specifier: ^6.4.1 version: 6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0) + vitest: + specifier: ^4.1.10 + version: 4.1.10(@types/node@22.19.15)(msw@2.12.14(@types/node@22.19.15)(typescript@5.6.3))(vite@6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0)) packages: @@ -1693,6 +1696,9 @@ packages: resolution: {integrity: sha512-tlqY9xq5ukxTUZBmoOp+m61cqwQD5pHJtFY3Mn8CA8ps6yghLH/Hw8UPdqg4OLmFW3IFlcXnQNmo/dh8HzXYIQ==} engines: {node: '>=18'} + '@standard-schema/spec@1.1.0': + resolution: {integrity: sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w==} + '@swc/core-darwin-arm64@1.15.18': resolution: {integrity: sha512-+mIv7uBuSaywN3C9LNuWaX1jJJ3SKfiJuE6Lr3bd+/1Iv8oMU7oLBjYMluX1UrEPzwN2qCdY6Io0yVicABoCwQ==} engines: {node: '>=10'} @@ -1923,6 +1929,12 @@ packages: '@ts-morph/common@0.27.0': resolution: {integrity: sha512-Wf29UqxWDpc+i61k3oIOzcUfQt79PIT9y/MWfAGlrkjg6lBC1hwDECLXPVJAhWjiGbfBCxZd65F/LIZF3+jeJQ==} + '@types/chai@5.2.3': + resolution: {integrity: sha512-Mw558oeA9fFbv65/y4mHtXDs9bPnFMZAL/jxdPFUpOHHIXX91mcgEHbS5Lahr+pwZFR8A7GQleRWeI6cGFC2UA==} + + '@types/deep-eql@4.0.2': + resolution: {integrity: sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw==} + '@types/estree@1.0.8': resolution: {integrity: sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w==} @@ -2010,6 +2022,35 @@ packages: peerDependencies: vite: ^4 || ^5 || ^6 || ^7 + '@vitest/expect@4.1.10': + resolution: {integrity: sha512-YsCn+qAk1GWjQOWFEsEcL2gNQ0zmVmQu3T03qP6UyjhtmdtwtbuI+DASn/7iQB3HGTXkdBwGddzxPlmiql5vlA==} + + '@vitest/mocker@4.1.10': + resolution: {integrity: sha512-v0xaezt+DKEmKfaxg133ldzADrwLGd7Ze1MfQQTYfvs8OqZIwbxyxaYURivwV7sWy5fqn3rH5uOrSp07bp44Ow==} + peerDependencies: + msw: ^2.4.9 + vite: ^6.0.0 || ^7.0.0 || ^8.0.0 + peerDependenciesMeta: + msw: + optional: true + vite: + optional: true + + '@vitest/pretty-format@4.1.10': + resolution: {integrity: sha512-W1HsjSH4MXQ9YfmmhLAoIYf1HRfekQCGngeIgcei6MP5QQGWUe0gkopdZQaVCFO+JDJMrAJGwa5pRpNpvy4P8Q==} + + '@vitest/runner@4.1.10': + resolution: {integrity: sha512-IKI6kpIH+LmpROplyLwBBaCfMgOZOMsygVa6BARD6ahA04VRuJSa6OaVG7kRvSEMD870Vd91rSSw0eegtWyLGg==} + + '@vitest/snapshot@4.1.10': + resolution: {integrity: sha512-xRkfOT1qpTAi/Ti4Y1LtfRc3kEuqxGw59eN2jN9pRWMtS/XDevekhcFSqvQqjUNGksfjMJu3Y+oJ+4Ypn2OaJw==} + + '@vitest/spy@4.1.10': + resolution: {integrity: sha512-PLf/Ugvoq5wO/b4rwYCR1h2PSIdXz7wnkQFMiUpLdtM7l6pqVFcQIBEHyT1+l+cj7mNwAfZHzqXqDyjvOuwbDw==} + + '@vitest/utils@4.1.10': + resolution: {integrity: sha512-fy9am/HWxbaGt/Sawrp90vt6Y6jQwf1RX77cz3uwoJwJVMli/e1IEwRPnMNJ7vKfPTwo0diXifkpPvwH9v7nGA==} + accepts@2.0.0: resolution: {integrity: sha512-5cvg6CtKwfgdmVqY1WIiXKc3Q1bkRqGLi+2W/6ao+6Y7gu/RCwRuAhGEzh5B4KlszSuTLgZYuqFqo5bImjNKng==} engines: {node: '>= 0.6'} @@ -2069,6 +2110,10 @@ packages: resolution: {integrity: sha512-ik3ZgC9dY/lYVVM++OISsaYDeg1tb0VtP5uL3ouh1koGOaUMDPpbFIei4JkFimWUFPn90sbMNMXQAIVOlnYKJA==} engines: {node: '>=10'} + assertion-error@2.0.1: + resolution: {integrity: sha512-Izi8RQcffqCeNVgFigKli1ssklIbpHnCYc6AknXGYoB6grJqyeby7jv12JUQgmTAnIDnbck1uxksT4dzN3PWBA==} + engines: {node: '>=12'} + ast-types@0.16.1: resolution: {integrity: sha512-6t10qk83GOG8p0vKmaCr8eiilZwO171AvbROMtvvNiwrTly62t+7XkA8RdIIVbpMhCASAsxgAzdRSwh6nw/5Dg==} engines: {node: '>=4'} @@ -2141,6 +2186,10 @@ packages: caniuse-lite@1.0.30001780: resolution: {integrity: sha512-llngX0E7nQci5BPJDqoZSbuZ5Bcs9F5db7EtgfwBerX9XGtkkiO4NwfDDIRzHTTwcYC8vC7bmeUEPGrKlR/TkQ==} + chai@6.2.2: + resolution: {integrity: sha512-NUPRluOfOiTKBKvWPtSD4PhFvWCqOi0BGStNWs57X9js7XGTprSmFoz5F0tWhR4WPjNeR9jXqdC7/UpSJTnlRg==} + engines: {node: '>=18'} + chalk@4.1.2: resolution: {integrity: sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==} engines: {node: '>=10'} @@ -2176,12 +2225,6 @@ packages: resolution: {integrity: sha512-eYm0QWBtUrBWZWG0d386OGAw16Z995PiOVo2B7bjWSbHedGl5e0ZWaq65kOGgUSNesEIDkB9ISbTg/JK9dhCZA==} engines: {node: '>=6'} - cmdk@1.1.1: - resolution: {integrity: sha512-Vsv7kFaXm+ptHDMZ7izaRsP70GgrW9NBNGswt9OZaVBLlE0SNpDq8eu/VGXyF9r7M0azK3Wy7OlYXsuyYLFzHg==} - peerDependencies: - react: ^18 || ^19 || ^19.0.0-rc - react-dom: ^18 || ^19 || ^19.0.0-rc - code-block-writer@13.0.3: resolution: {integrity: sha512-Oofo0pq3IKnsFtuHqSF7TqBfr71aeyZDVJ0HpmqB7FBM2qEigL0iPONSCZSO9pE9dZTAxANe5XHG9Uy0YMv8cg==} @@ -2373,6 +2416,9 @@ packages: resolution: {integrity: sha512-Zf5H2Kxt2xjTvbJvP2ZWLEICxA6j+hAmMzIlypy4xcBg1vKVnx89Wy0GbS+kf5cwCVFFzdCFh2XSCFNULS6csw==} engines: {node: '>= 0.4'} + es-module-lexer@2.3.1: + resolution: {integrity: sha512-shc1dbU90Yl/xq1QrC7QRtfcwURZuVRfPhZbDoldJ1cn1gzDvBaBWlv0eFolj5+0znnPJz5TXLxsN77X/12KTA==} + es-object-atoms@1.1.1: resolution: {integrity: sha512-FGgH2h8zKNim9ljj7dankFPcICIK9Cp5bm+c2gQSYePhpaG5+esrLODihIorn+Pe6FGJzWhXQotPv73jTaldXA==} engines: {node: '>= 0.4'} @@ -2460,6 +2506,9 @@ packages: resolution: {integrity: sha512-MMdARuVEQziNTeJD8DgMqmhwR11BRQ/cBP+pLtYdSTnf3MIO8fFeiINEbX36ZdNlfU/7A9f3gUw49B3oQsvwBA==} engines: {node: '>=4.0'} + estree-walker@3.0.3: + resolution: {integrity: sha512-7RUKfXgSMMkzt6ZuXmqapOurLGPPfgj6l9uRZ7lRGolvk0y2yocc35LdcxKC5PQZdn2DMqioAQ2NoWcrTKmm6g==} + esutils@2.0.3: resolution: {integrity: sha512-kVscqXk4OCp68SZ0dkgEKVi6/8ij300KBWTJq32P/dYeWTSwK41WyTxalN1eRmA5Z9UU/LX9D7FWSmV9SAYx6g==} engines: {node: '>=0.10.0'} @@ -2484,6 +2533,10 @@ packages: resolution: {integrity: sha512-9Be3ZoN4LmYR90tUoVu2te2BsbzHfhJyfEiAVfz7N5/zv+jduIfLrV2xdQXOHbaD6KgpGdO9PRPM1Y4Q9QkPkA==} engines: {node: ^18.19.0 || >=20.5.0} + expect-type@1.4.0: + resolution: {integrity: sha512-KfYbmpRm0VbLjEvVa9yGwCi9GI34xvi7A/HXYWQO65CSD2u3MczUJSuwXKFIxlGsgBQizV9q5J9NHj4VG0n+pA==} + engines: {node: '>=12.0.0'} + express-rate-limit@8.3.1: resolution: {integrity: sha512-D1dKN+cmyPWuvB+G2SREQDzPY1agpBIcTa9sJxOPMCNeH3gwzhqJRDWCXW3gg0y//+LQ/8j52JbMROWyrKdMdw==} engines: {node: '>= 16'} @@ -2574,6 +2627,17 @@ packages: resolution: {integrity: sha512-buRG0fpBtRHSTCOASe6hD258tEubFoRLb4ZNA6NxMVHNw2gOcwHo9wyablzMzOA5z9xA9L1KNjk/Nt6MT9aYow==} engines: {node: '>= 0.6'} + framer-motion@13.1.0: + resolution: {integrity: sha512-QSZrF0Id3QGuHJ+OL+9PSY9pk86C8ERFalwAGSchzTm65+ZoGH/RM26lmEARLljcHj2lqhv0jZOOks+EI3COOw==} + peerDependencies: + react: ^18.0.0 || ^19.0.0 + react-dom: ^18.0.0 || ^19.0.0 + peerDependenciesMeta: + react: + optional: true + react-dom: + optional: true + fresh@2.0.0: resolution: {integrity: sha512-Rx/WycZ60HOaqLKAi6cHRKKI7zxWbJ31MhntmtwMoaTeF7XFH9hhBp8vITaMidfljRQ6eYWCKkaTK+ykVJHP2A==} engines: {node: '>= 0.8'} @@ -3031,6 +3095,23 @@ packages: minimist@1.2.8: resolution: {integrity: sha512-2yyAR8qBkN3YuheJanUpWC5U3bb5osDywNB8RzDVlDwDHbocAJveqqj1u8+SVD7jkWT4yvsHCpWqqWqAxb0zCA==} + motion-dom@13.0.0: + resolution: {integrity: sha512-Xk+SJas70uMAUIApg+m3lZDShxI3LBFHq7mFGbBKoRXc2PVPDyAKmzN64Bbzt4CZdP/CItTiJxWtn4TA0v53Ng==} + + motion-utils@13.0.0: + resolution: {integrity: sha512-7DnN7TmbLcYXcG4RVadXIihWlyuM9afoUww8Y5Agg431kGKiuL2/OMyP4mJ5wLz+pvN3t5ySClLOaVXJ+wekRQ==} + + motion@13.1.0: + resolution: {integrity: sha512-qtvscq59uCPdWnNW4SdSkrxR+BS/QYsa923bx7ocA+4p+ZGNbbVQwkSnG4aukB81QWjtl3AxX36plxNyZLmHCA==} + peerDependencies: + react: ^18.0.0 || ^19.0.0 + react-dom: ^18.0.0 || ^19.0.0 + peerDependenciesMeta: + react: + optional: true + react-dom: + optional: true + ms@2.1.3: resolution: {integrity: sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==} @@ -3102,6 +3183,10 @@ packages: resolution: {integrity: sha512-EFVjAYfzWqWsBMRHPMAXLCDIJnpMhdWAqR7xG6M6a2cs6PMFpl/+Z20w9zDW4vkxOFfddegBKq9Rehd0bxWE7A==} engines: {node: '>= 10'} + obug@2.1.4: + resolution: {integrity: sha512-4a+OsYv9UktOJKE+l1A4OufDgdRF9PifWj+tJnHURo/P+WOxpG4GzUFL9qCalmWauao6ogiG+QvnCovwPoyAWA==} + engines: {node: '>=12.20.0'} + on-finished@2.4.1: resolution: {integrity: sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg==} engines: {node: '>= 0.8'} @@ -3431,6 +3516,9 @@ packages: resolution: {integrity: sha512-ZX99e6tRweoUXqR+VBrslhda51Nh5MTQwou5tnUDgbtyM0dBgmhEDtWGP/xbKn6hqfPRHujUNwz5fy/wbbhnpw==} engines: {node: '>= 0.4'} + siginfo@2.0.0: + resolution: {integrity: sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g==} + signal-exit@3.0.7: resolution: {integrity: sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==} @@ -3459,10 +3547,16 @@ packages: resolution: {integrity: sha512-i5uvt8C3ikiWeNZSVZNWcfZPItFQOsYTUAOkcUPGd8DqDy1uOUikjt5dG+uRlwyvR108Fb9DOd4GvXfT0N2/uQ==} engines: {node: '>= 12'} + stackback@0.0.2: + resolution: {integrity: sha512-1XMJE5fQo1jGH6Y/7ebnwPOBEkIEnT4QF32d5R1+VXdXveM0IBMJt8zfaxX1P3QhVwrYe+576+jkANtSS2mBbw==} + statuses@2.0.2: resolution: {integrity: sha512-DvEy55V3DB7uknRo+4iOGT5fP1slR8wQohVdknigZPMpMstaKJQWhwiYBACJE3Ul2pTnATihhBYnRhZQHGBiRw==} engines: {node: '>= 0.8'} + std-env@4.2.0: + resolution: {integrity: sha512-oCUKSupKTHX53EyjDtuZQ64pjLJ6yYCtpmEw0goYxtjG9KpbRe8KAsl2tBUGU9DyMcJ0RwJ8GqJAFzMXcXW1Rw==} + stdin-discarder@0.2.2: resolution: {integrity: sha512-UhDfHmA92YAlNnCfhmq0VeNL5bDbiZGg7sZ2IvPsXubGkiNa9EC+tUTsjBRsYUAz87btI6/1wf4XoVvQ3uRnmQ==} engines: {node: '>=18'} @@ -3530,10 +3624,21 @@ packages: tiny-warning@1.0.3: resolution: {integrity: sha512-lBN9zLN/oAf68o3zNXYrdCt1kP8WsiGW8Oo2ka41b2IM5JL/S1CTyX1rW0mb/zSuJun0ZUrDxx4sqvYS2FWzPA==} + tinybench@2.9.0: + resolution: {integrity: sha512-0+DUvqWMValLmha6lr4kD8iAMK1HzV0/aKnCtWb9v9641TnP/MFb7Pc2bxoxQjTXAErryXVgUOfv2YqNllqGeg==} + + tinyexec@1.3.0: + resolution: {integrity: sha512-QKAl9m8gWWGHV8jZcPeym6j+XULi6tOf1mT83WYJ4Lk2ytW/uwAWkrP0uFsdoYMdueVJ0qs26wZ+23xeB4ibNQ==} + engines: {node: '>=18'} + tinyglobby@0.2.15: resolution: {integrity: sha512-j2Zq4NyQYG5XMST4cbs02Ak8iJUdxRM0XI5QyxXuZOzKOINmWurp3smXu3y5wDcJrptwpSjgXHzIQxR0omXljQ==} engines: {node: '>=12.0.0'} + tinyrainbow@3.1.1: + resolution: {integrity: sha512-yau8yJdTt989Mm0Bd/236QnzEiPf2xLLTqUZRUJOo/3CB078LSwzei343DgtJVmfJKJE3TMINY1u42SQsP6mXw==} + engines: {node: '>=14.0.0'} + tldts-core@7.0.27: resolution: {integrity: sha512-YQ7uPjgWUibIK6DW5lrKujGwUKhLevU4hcGbP5O6TcIUb+oTjJYJVWPS4nZsIHrEEEG6myk/oqAJUEQmpZrHsg==} @@ -3708,6 +3813,47 @@ packages: yaml: optional: true + vitest@4.1.10: + resolution: {integrity: sha512-R9jUTe5S4Qb0HCd4TNqpC7oGcrMssMRGXLW80ubjWsW9VH5GF8y1Y0SFLY9AbqSk6nt0PnOx4H4WNJYZ13GUPw==} + engines: {node: ^20.0.0 || ^22.0.0 || >=24.0.0} + hasBin: true + peerDependencies: + '@edge-runtime/vm': '*' + '@opentelemetry/api': ^1.9.0 + '@types/node': ^20.0.0 || ^22.0.0 || >=24.0.0 + '@vitest/browser-playwright': 4.1.10 + '@vitest/browser-preview': 4.1.10 + '@vitest/browser-webdriverio': 4.1.10 + '@vitest/coverage-istanbul': 4.1.10 + '@vitest/coverage-v8': 4.1.10 + '@vitest/ui': 4.1.10 + happy-dom: '*' + jsdom: '*' + vite: ^6.0.0 || ^7.0.0 || ^8.0.0 + peerDependenciesMeta: + '@edge-runtime/vm': + optional: true + '@opentelemetry/api': + optional: true + '@types/node': + optional: true + '@vitest/browser-playwright': + optional: true + '@vitest/browser-preview': + optional: true + '@vitest/browser-webdriverio': + optional: true + '@vitest/coverage-istanbul': + optional: true + '@vitest/coverage-v8': + optional: true + '@vitest/ui': + optional: true + happy-dom: + optional: true + jsdom: + optional: true + web-streams-polyfill@3.3.3: resolution: {integrity: sha512-d2JWLCivmZYTSIoge9MsgFCZrt571BikcWGYkjC1khllbTeDlGqZ2D8vD8E/lJa8WGWbb7Plm8/XJYV7IJHZZw==} engines: {node: '>= 8'} @@ -3725,6 +3871,11 @@ packages: engines: {node: ^16.13.0 || >=18.0.0} hasBin: true + why-is-node-running@2.3.0: + resolution: {integrity: sha512-hUrmaWBdVDcxvYqnyh09zunKzROWjbZTiNy8dBEjkS7ehEDQibXJ7XvlmtbwuTclUiIyN+CyXQD4Vmko8fNm8w==} + engines: {node: '>=8'} + hasBin: true + word-wrap@1.2.5: resolution: {integrity: sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==} engines: {node: '>=0.10.0'} @@ -5216,6 +5367,8 @@ snapshots: '@sindresorhus/merge-streams@4.0.0': {} + '@standard-schema/spec@1.1.0': {} + '@swc/core-darwin-arm64@1.15.18': optional: true @@ -5426,6 +5579,13 @@ snapshots: minimatch: 10.2.4 path-browserify: 1.0.1 + '@types/chai@5.2.3': + dependencies: + '@types/deep-eql': 4.0.2 + assertion-error: 2.0.1 + + '@types/deep-eql@4.0.2': {} + '@types/estree@1.0.8': {} '@types/json-schema@7.0.15': {} @@ -5545,6 +5705,48 @@ snapshots: transitivePeerDependencies: - '@swc/helpers' + '@vitest/expect@4.1.10': + dependencies: + '@standard-schema/spec': 1.1.0 + '@types/chai': 5.2.3 + '@vitest/spy': 4.1.10 + '@vitest/utils': 4.1.10 + chai: 6.2.2 + tinyrainbow: 3.1.1 + + '@vitest/mocker@4.1.10(msw@2.12.14(@types/node@22.19.15)(typescript@5.6.3))(vite@6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0))': + dependencies: + '@vitest/spy': 4.1.10 + estree-walker: 3.0.3 + magic-string: 0.30.21 + optionalDependencies: + msw: 2.12.14(@types/node@22.19.15)(typescript@5.6.3) + vite: 6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0) + + '@vitest/pretty-format@4.1.10': + dependencies: + tinyrainbow: 3.1.1 + + '@vitest/runner@4.1.10': + dependencies: + '@vitest/utils': 4.1.10 + pathe: 2.0.3 + + '@vitest/snapshot@4.1.10': + dependencies: + '@vitest/pretty-format': 4.1.10 + '@vitest/utils': 4.1.10 + magic-string: 0.30.21 + pathe: 2.0.3 + + '@vitest/spy@4.1.10': {} + + '@vitest/utils@4.1.10': + dependencies: + '@vitest/pretty-format': 4.1.10 + convert-source-map: 2.0.0 + tinyrainbow: 3.1.1 + accepts@2.0.0: dependencies: mime-types: 3.0.2 @@ -5597,6 +5799,8 @@ snapshots: dependencies: tslib: 2.8.1 + assertion-error@2.0.1: {} + ast-types@0.16.1: dependencies: tslib: 2.8.1 @@ -5683,6 +5887,8 @@ snapshots: caniuse-lite@1.0.30001780: {} + chai@6.2.2: {} + chalk@4.1.2: dependencies: ansi-styles: 4.3.0 @@ -5722,18 +5928,6 @@ snapshots: clsx@2.1.1: {} - cmdk@1.1.1(@types/react-dom@19.2.3(@types/react@19.2.14))(@types/react@19.2.14)(react-dom@19.2.4(react@19.2.4))(react@19.2.4): - dependencies: - '@radix-ui/react-compose-refs': 1.1.2(@types/react@19.2.14)(react@19.2.4) - '@radix-ui/react-dialog': 1.1.15(@types/react-dom@19.2.3(@types/react@19.2.14))(@types/react@19.2.14)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) - '@radix-ui/react-id': 1.1.1(@types/react@19.2.14)(react@19.2.4) - '@radix-ui/react-primitive': 2.1.4(@types/react-dom@19.2.3(@types/react@19.2.14))(@types/react@19.2.14)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) - react: 19.2.4 - react-dom: 19.2.4(react@19.2.4) - transitivePeerDependencies: - - '@types/react' - - '@types/react-dom' - code-block-writer@13.0.3: {} color-convert@2.0.1: @@ -5867,6 +6061,8 @@ snapshots: es-errors@1.3.0: {} + es-module-lexer@2.3.1: {} + es-object-atoms@1.1.1: dependencies: es-errors: 1.3.0 @@ -6020,6 +6216,10 @@ snapshots: estraverse@5.3.0: {} + estree-walker@3.0.3: + dependencies: + '@types/estree': 1.0.8 + esutils@2.0.3: {} etag@1.8.1: {} @@ -6057,6 +6257,8 @@ snapshots: strip-final-newline: 4.0.0 yoctocolors: 2.1.2 + expect-type@1.4.0: {} + express-rate-limit@8.3.1(express@5.2.1): dependencies: express: 5.2.1 @@ -6175,6 +6377,15 @@ snapshots: forwarded@0.2.0: {} + framer-motion@13.1.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4): + dependencies: + motion-dom: 13.0.0 + motion-utils: 13.0.0 + tslib: 2.8.1 + optionalDependencies: + react: 19.2.4 + react-dom: 19.2.4(react@19.2.4) + fresh@2.0.0: {} fs-extra@11.3.4: @@ -6517,6 +6728,20 @@ snapshots: minimist@1.2.8: {} + motion-dom@13.0.0: + dependencies: + motion-utils: 13.0.0 + + motion-utils@13.0.0: {} + + motion@13.1.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4): + dependencies: + framer-motion: 13.1.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) + tslib: 2.8.1 + optionalDependencies: + react: 19.2.4 + react-dom: 19.2.4(react@19.2.4) + ms@2.1.3: {} msw@2.12.14(@types/node@22.19.15)(typescript@5.6.3): @@ -6584,6 +6809,8 @@ snapshots: object-treeify@1.1.33: {} + obug@2.1.4: {} + on-finished@2.4.1: dependencies: ee-first: 1.1.1 @@ -7032,6 +7259,8 @@ snapshots: side-channel-map: 1.0.1 side-channel-weakmap: 1.0.2 + siginfo@2.0.0: {} + signal-exit@3.0.7: {} signal-exit@4.1.0: {} @@ -7049,8 +7278,12 @@ snapshots: source-map@0.7.6: {} + stackback@0.0.2: {} + statuses@2.0.2: {} + std-env@4.2.0: {} + stdin-discarder@0.2.2: {} strict-event-emitter@0.5.1: {} @@ -7105,11 +7338,17 @@ snapshots: tiny-warning@1.0.3: {} + tinybench@2.9.0: {} + + tinyexec@1.3.0: {} + tinyglobby@0.2.15: dependencies: fdir: 6.5.0(picomatch@4.0.3) picomatch: 4.0.3 + tinyrainbow@3.1.1: {} + tldts-core@7.0.27: {} tldts@7.0.27: @@ -7246,6 +7485,33 @@ snapshots: lightningcss: 1.32.0 tsx: 4.21.0 + vitest@4.1.10(@types/node@22.19.15)(msw@2.12.14(@types/node@22.19.15)(typescript@5.6.3))(vite@6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0)): + dependencies: + '@vitest/expect': 4.1.10 + '@vitest/mocker': 4.1.10(msw@2.12.14(@types/node@22.19.15)(typescript@5.6.3))(vite@6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0)) + '@vitest/pretty-format': 4.1.10 + '@vitest/runner': 4.1.10 + '@vitest/snapshot': 4.1.10 + '@vitest/spy': 4.1.10 + '@vitest/utils': 4.1.10 + es-module-lexer: 2.3.1 + expect-type: 1.4.0 + magic-string: 0.30.21 + obug: 2.1.4 + pathe: 2.0.3 + picomatch: 4.0.3 + std-env: 4.2.0 + tinybench: 2.9.0 + tinyexec: 1.3.0 + tinyglobby: 0.2.15 + tinyrainbow: 3.1.1 + vite: 6.4.1(@types/node@22.19.15)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0) + why-is-node-running: 2.3.0 + optionalDependencies: + '@types/node': 22.19.15 + transitivePeerDependencies: + - msw + web-streams-polyfill@3.3.3: {} webpack-virtual-modules@0.6.2: {} @@ -7258,6 +7524,11 @@ snapshots: dependencies: isexe: 3.1.5 + why-is-node-running@2.3.0: + dependencies: + siginfo: 2.0.0 + stackback: 0.0.2 + word-wrap@1.2.5: {} wrap-ansi@6.2.0: diff --git a/ui/src/app.tsx b/ui/src/app.tsx new file mode 100644 index 0000000..c402a6a --- /dev/null +++ b/ui/src/app.tsx @@ -0,0 +1,51 @@ +import { RouterProvider } from "@tanstack/react-router"; +import { domMax, LazyMotion, MotionConfig } from "motion/react"; +import { useEffect, useRef } from "react"; +import AuthProvider from "./components/auth.tsx"; +import ThemeProvider from "./components/theme.tsx"; +import { Spinner } from "./components/ui/spinner.tsx"; +import { useAuth } from "./hooks/use-auth.ts"; +import { router } from "./router.tsx"; + +function InnerApp() { + const auth = useAuth(); + const lastStatus = useRef(auth.status); + + // A stale session has to re-run the route guards, or whatever is on screen + // keeps firing requests that will only ever 401. + useEffect(() => { + // Not on the first run: the router has no context yet, so beforeLoad + // would read isAuthenticated off an undefined auth. + if (lastStatus.current === auth.status) return; + + lastStatus.current = auth.status; + + router.invalidate(); + }, [auth.status]); + + if (auth.status === "pending") { + return ( +
+ +
+ ); + } + + return ; +} + +export default function App() { + return ( + + {/* domMax rather than domAnimation: the board leans on layout and + shared-element transitions. strict keeps us on `m.*`. */} + + + + + + + + + ); +} diff --git a/ui/src/components/auth.tsx b/ui/src/components/auth.tsx index f66a9a2..d64e213 100644 --- a/ui/src/components/auth.tsx +++ b/ui/src/components/auth.tsx @@ -1,5 +1,10 @@ -import { AuthContext, getStoredUser, setStoredUser } from "@/hooks/use-auth"; -import { api } from "@/lib/api"; +import { + AuthContext, + AuthStatus, + getStoredUser, + setStoredUser, +} from "@/hooks/use-auth"; +import { api, setSessionExpiredHandler } from "@/lib/api"; import { User } from "@/types"; import { useCallback, useEffect, useState } from "react"; @@ -9,50 +14,67 @@ export default function AuthProvider({ children: React.ReactNode; }) { const [user, setUser] = useState(getStoredUser()); + const [status, setStatus] = useState("pending"); - const isAuthenticated = !!user; - - const logout = useCallback(async () => { - await api.post("/api/auth/logout"); - + const clearSession = useCallback(() => { setStoredUser(null); setUser(null); + setStatus("anonymous"); }, []); const login = useCallback(async (name: string) => { const res = await api.post("/api/auth/login", { name }); - if (res && res.status === 200) { - setStoredUser(res.data); - setUser(res.data); - } + setStoredUser(res.data); + setUser(res.data); + setStatus("authenticated"); }, []); - useEffect(() => { - const stored = getStoredUser(); - setUser(stored); + const logout = useCallback(async () => { + await api.post("/api/auth/logout").catch(() => {}); + + clearSession(); + }, [clearSession]); + useEffect(() => { let cancelled = false; api .get("/api/auth/self") .then((res) => { - if (!cancelled && res.status === 200) { - setStoredUser(res.data); - setUser(res.data); - } + if (cancelled) return; + + setStoredUser(res.data); + setUser(res.data); + setStatus("authenticated"); }) .catch(() => { - // ignore; the axios interceptor already handles 401s by redirecting + if (cancelled) return; + + clearSession(); }); return () => { cancelled = true; }; - }, []); + }, [clearSession]); + + useEffect(() => { + setSessionExpiredHandler(clearSession); + + return () => setSessionExpiredHandler(() => {}); + }, [clearSession]); return ( - + {children} ); diff --git a/ui/src/components/container.tsx b/ui/src/components/container.tsx new file mode 100644 index 0000000..b6aab37 --- /dev/null +++ b/ui/src/components/container.tsx @@ -0,0 +1,16 @@ +import { twMerge } from "tailwind-merge"; + +export default function Container({ + className, + ...props +}: React.ComponentProps<"div">) { + return ( +
+ ); +} diff --git a/ui/src/components/nav.tsx b/ui/src/components/nav.tsx index ee1c4ff..5b352dd 100644 --- a/ui/src/components/nav.tsx +++ b/ui/src/components/nav.tsx @@ -1,8 +1,8 @@ +import Container from "@/components/container"; import { useAuth } from "@/hooks/use-auth"; import useTheme, { Theme } from "@/hooks/use-theme"; -import { Link, useNavigate } from "@tanstack/react-router"; +import { Link } from "@tanstack/react-router"; import { LogOut, Monitor, Moon, Sun } from "lucide-react"; -import { useEffect } from "react"; import { Button } from "./ui/button"; import { DropdownMenu, @@ -18,18 +18,12 @@ import { export default function Nav() { const { theme, setTheme } = useTheme(); const { user, logout } = useAuth(); - const navigate = useNavigate(); - - useEffect(() => { - if (user) return; - navigate({ to: "/login" }); - }, [user]); const themeIcon = theme === "dark" ? : theme === "light" ? : ; return (
-
+ -
+
); } diff --git a/ui/src/components/retro/ai-retro-template.tsx b/ui/src/components/retro/ai-retro-template.tsx index 6e017be..de97aca 100644 --- a/ui/src/components/retro/ai-retro-template.tsx +++ b/ui/src/components/retro/ai-retro-template.tsx @@ -1,130 +1,193 @@ -import { Sparkles } from "lucide-react"; -import { Popover, PopoverContent, PopoverTrigger } from "../ui/popover"; -import { Button } from "../ui/button"; -import { - Form, - FormControl, - FormField, - FormItem, - FormMessage, -} from "../ui/form"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { api } from "@/lib/api"; +import { spring, springy } from "@/lib/motion"; import { AIRetroTemplateResponse } from "@/types"; -import z from "zod"; -import { useForm, useFormContext } from "react-hook-form"; import { zodResolver } from "@hookform/resolvers/zod"; -import { api } from "@/lib/api"; -import { toast } from "sonner"; -import { - InputGroup, - InputGroupAddon, - InputGroupButton, - InputGroupInput, -} from "../ui/input-group"; +import { Sparkles, WandSparkles } from "lucide-react"; +import { m } from "motion/react"; import { useState } from "react"; -import { Spinner } from "../ui/spinner"; +import { useForm } from "react-hook-form"; +import { toast } from "sonner"; +import z from "zod"; -const aiTemplateSchema = z.object({ +const schema = z.object({ prompt: z .string() - .min(4, "Please enter a prompt") - .max(128, "Prompt is too long (max 128 characters)") - .transform((value) => value.trim()), + .trim() + .min(2, "Give it something to work with") + .max(128, "That's a bit long. Keep it under 128 characters"), }); -export default function AIRetroTemplate() { - const { setValue } = useFormContext(); - const [popoverOpen, setPopoverOpen] = useState(false); - const [isGenerating, setIsGenerating] = useState(false); +const suggestions = [ + "Star Wars", + "The Great British Bake Off", + "Pirates", + "Deep sea", + "Heist movie", + "Formula 1", +]; + +export interface GeneratedColumn { + title: string; + description: string; +} + +export default function AIRetroTemplate({ + onApply, + onGeneratingChange, +}: { + onApply: (columns: GeneratedColumn[]) => void; + onGeneratingChange?: (generating: boolean) => void; +}) { + const [generating, setGenerating] = useState(false); const form = useForm({ - resolver: zodResolver(aiTemplateSchema), - defaultValues: { - prompt: "", - }, + resolver: zodResolver(schema), + defaultValues: { prompt: "" }, }); - function handleSubmit(data: z.infer) { - if (isGenerating) return; - setIsGenerating(true); + function generate(prompt: string) { + if (generating) return; + + setGenerating(true); + onGeneratingChange?.(true); api - .post("/api/ai/retro-template", { - prompt: data.prompt, - }) + .post("/api/ai/retro-template", { prompt }) .then((res) => { - setValue("columns", res.data.columns); - form.reset(); - toast.success(`🎉 ${res.data.theme} retro generated!`, { - description: `${res.data.columns.length} fresh columns are ready to go 🚀`, - duration: 5000, + onApply(res.data.columns); + + toast.success(`${res.data.theme} it is ✨`, { + description: `${res.data.columns.length} columns ready. Tweak anything you like.`, }); }) .catch(() => { - toast.error("Failed to generate columns from AI"); + toast.error("Couldn't dream that one up", { + description: "Try a different theme, or pick a template instead.", + }); }) .finally(() => { - setIsGenerating(false); - setPopoverOpen(false); + setGenerating(false); + onGeneratingChange?.(false); }); } + // Validates through react-hook-form without an enclosing
element. + const submit = form.handleSubmit((data) => generate(data.prompt)); + + function applySuggestion(suggestion: string) { + form.setValue("prompt", suggestion); + generate(suggestion); + } + + const error = form.formState.errors.prompt?.message; + return ( - - - - - -

Generate a retro template using AI.

-

- Enter a prompt to generate a retro template. The AI will generate - columns based on the prompt. +

+ + +
+
+ + + + +

Conjure a themed board

+ + + Powered by AI + +
+ +

+ Name a theme and we'll write the columns around it.

- - ( - - - - { - if (e.key === "Enter") { - e.preventDefault(); - form.handleSubmit(handleSubmit)(); - } - }} - /> - - - {isGenerating ? ( - - ) : ( - - Generate - - )} - - - - - - - )} + {/* Deliberately not a : this sits inside the create-retro form, + and HTML has no nested forms - the browser drops the inner one, so + a submit button here would submit the outer form and create the + retro instead of generating anything. */} +
+ { + if (event.key !== "Enter") return; + + event.preventDefault(); + submit(); + }} + {...form.register("prompt")} /> - - - + + +
+ + {error &&

{error}

} + +
+ {suggestions.map((suggestion, i) => ( + applySuggestion(suggestion)} + initial={{ opacity: 0, y: 3 }} + animate={{ opacity: 1, y: 0 }} + transition={{ ...spring, delay: 0.03 * i }} + whileHover={{ y: -1 }} + className="rounded-full border border-border/70 px-2 py-0.5 text-xs text-muted-foreground transition-colors hover:border-[var(--chart-1)] hover:text-foreground disabled:opacity-50" + > + {suggestion} + + ))} +
+
+
); } diff --git a/ui/src/components/retro/board.tsx b/ui/src/components/retro/board.tsx index 44f3857..9b07614 100644 --- a/ui/src/components/retro/board.tsx +++ b/ui/src/components/retro/board.tsx @@ -1,3 +1,9 @@ +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { + Collapsible, + CollapsibleContent, +} from "@/components/ui/collapsible"; import { createSocketEvent, PayloadConnectionInfo, @@ -6,30 +12,22 @@ import { SocketEvent, } from "@/events"; import useRetro from "@/hooks/use-retro"; +import { panelVariants } from "@/lib/motion"; +import { stageLabel } from "@/lib/stages"; import { RetroStatus } from "@/types"; -import { useEffect, useState } from "react"; import { Link } from "@tanstack/react-router"; -import { toast } from "sonner"; -import { Badge } from "@/components/ui/badge"; -import { Button } from "@/components/ui/button"; -import { Collapsible, CollapsibleContent } from "@/components/ui/collapsible"; import { ChevronDownIcon, ChevronUpIcon } from "lucide-react"; +import { AnimatePresence, m } from "motion/react"; +import { useEffect, useState } from "react"; +import { toast } from "sonner"; import Brainstorm from "./brainstorm"; -import ChangeStatusButton from "./change-status-button"; import ConnectionIndicator from "./connection-indicator"; import Discuss from "./discuss"; import Group from "./group"; import Settings from "./settings"; -import StatusIndicator from "./status-indicator"; -import Vote from "./vote"; import ShowMarkdown from "./show-markdown"; - -const stageLabel: Record = { - brainstorm: "Brainstorm", - group: "Group", - vote: "Vote", - discuss: "Discuss", -}; +import StageRail from "./stage-rail"; +import Vote from "./vote"; export default function Board() { const { @@ -37,7 +35,9 @@ export default function Board() { socket: { sendJsonMessage, lastJsonMessage, readyState }, } = useRetro(); const [status, setStatus] = useState(retro.status); - const [connectionInfo, setConnectionInfo] = useState({ users: [] }); + const [connectionInfo, setConnectionInfo] = useState({ + users: [], + }); const [votesRemaining, setVotesRemaining] = useState(0); const [expanded, setExpanded] = useState(true); @@ -46,7 +46,9 @@ export default function Board() { const event = lastJsonMessage as SocketEvent; switch (event.name) { case "error": - toast("Something went wrong", { description: (event.payload as PayloadError).message }); + toast("Something went wrong", { + description: (event.payload as PayloadError).message, + }); return; case "status_updated": setStatus((event.payload as PayloadStatusUpdated).status); @@ -63,58 +65,89 @@ export default function Board() { return (
- {/* Sticky board header */}
-
- {/* Compact bar — always visible */} -
- +
+
+ {retro.title} - {!expanded && ( - - {stageLabel[status]} - - )} - - - + +
+ {!expanded && ( + + {stageLabel(status)} + + )} + + + + +
{/* Expanded detail row */} -
-
+
+
{retro.tags && retro.tags.length > 0 ? ( retro.tags.map((tag) => ( - + #{tag} )) ) : ( - no tags + + no tags + )}
-
- -
-
- {status === "vote" && ( - {votesRemaining} votes left - )} + +
+ + {status === "vote" && ( + + {votesRemaining} votes left + + )} + + {status === "discuss" && } - +
@@ -122,7 +155,20 @@ export default function Board() {
- + + + + +
); } diff --git a/ui/src/components/retro/brainstorm.tsx b/ui/src/components/retro/brainstorm.tsx index cc0a5d5..193264e 100644 --- a/ui/src/components/retro/brainstorm.tsx +++ b/ui/src/components/retro/brainstorm.tsx @@ -1,18 +1,23 @@ import { createSocketEvent } from "@/events"; +import { useColumnActions } from "@/hooks/use-columns"; import { useNotes } from "@/hooks/use-notes"; import useRetro from "@/hooks/use-retro"; -import { DndContext, DragEndEvent } from "@dnd-kit/core"; +import { DragEndEvent } from "@dnd-kit/core"; import { Plus } from "lucide-react"; +import { AnimatePresence } from "motion/react"; import { Button } from "../ui/button"; +import { EmptyColumn, NoteSkeletons } from "./column-states"; import { Columns, DroppableColumn } from "./columns"; import { DraggableNote, Note } from "./note"; import NoteDialog from "./note-dialog"; +import NoteDndContext from "./note-dnd"; export default function Brainstorm() { const { - retro: { columns, gifs_enabled }, + retro: { columns }, } = useRetro(); - const { notes, dispatch } = useNotes(); + const { notes, loaded, dispatch } = useNotes(); + const columnActions = useColumnActions(notes); function handleNewNote(columnId: string, content: string) { dispatch( @@ -27,94 +32,93 @@ export default function Brainstorm() { const overColumnId = event.over?.id; const note = notes.find((n) => n.id === event.active?.id); - if (note?.column_id === overColumnId) return; + if (!note || !overColumnId) return; + + if (note.column_id === overColumnId) return; dispatch( createSocketEvent("note_update", { - id: note?.id, + id: note.id, column_id: overColumnId, }), ); } function handleNoteEdit(noteId: string, content: string) { - dispatch( - createSocketEvent("note_update", { - id: noteId, - content, - }), - ); + dispatch(createSocketEvent("note_update", { id: noteId, content })); } function handleNoteDelete(noteId: string) { - dispatch( - createSocketEvent("note_delete", { - id: noteId, - }), - ); + dispatch(createSocketEvent("note_delete", { id: noteId })); } function handleNoteGifSelected(noteId: string, url: string) { - dispatch( - createSocketEvent("note_update", { - id: noteId, - img_url: url, - }), - ); + dispatch(createSocketEvent("note_update", { id: noteId, img_url: url })); } function handleNoteGifRemoved(noteId: string) { dispatch( - createSocketEvent("note_update", { - id: noteId, - remove_img_url: true, - }), + createSocketEvent("note_update", { id: noteId, remove_img_url: true }), ); } return ( - - - {columns.map((column) => ( - - handleNewNote(column.id, content)} + + + {columns.map((column, index) => { + const columnNotes = notes.filter((n) => n.column_id === column.id); + + return ( + - - + handleNewNote(column.id, content)} + > + + + + {!loaded && } - {notes - .filter((n) => n.column_id == column.id) - .map((note) => ( -
- {note.created_by_me ? ( + {loaded && columnNotes.length === 0 && ( + Nothing here yet. + )} + + {/* Not popLayout: it tears a note out of the flow to animate + it away while its layoutId is gliding it to a new column. */} + + {columnNotes.map((note) => + note.created_by_me ? ( handleNoteEdit(note.id, content)} onDelete={() => handleNoteDelete(note.id)} - onGifSelected={ - gifs_enabled - ? (url) => handleNoteGifSelected(note.id, url) - : undefined - } - onGifRemoved={ - gifs_enabled - ? () => handleNoteGifRemoved(note.id) - : undefined + onGifSelected={(url) => + handleNoteGifSelected(note.id, url) } + onGifRemoved={() => handleNoteGifRemoved(note.id)} /> ) : ( - - )} -
- ))} - - ))} + + ), + )} + + + ); + })} - + ); } diff --git a/ui/src/components/retro/change-status-button.tsx b/ui/src/components/retro/change-status-button.tsx deleted file mode 100644 index 780c2a5..0000000 --- a/ui/src/components/retro/change-status-button.tsx +++ /dev/null @@ -1,93 +0,0 @@ -import { RetroStatus } from "@/types"; -import { ChevronsLeft, ChevronsRight } from "lucide-react"; -import { useState } from "react"; -import { Button } from "@/components/ui/button"; -import { - Dialog, - DialogClose, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, - DialogTrigger, -} from "@/components/ui/dialog"; - -const variants = { - next: { - brainstorm: ["group", "Group", "Proceed to grouping stage?"], - group: ["vote", "Vote", "Done grouping? Proceed to vote."], - vote: ["discuss", "Discuss", "Done voting? Proceed to discussion."], - discuss: [], - }, - prev: { - brainstorm: [], - discuss: ["vote", "Vote", "Back to voting stage?"], - vote: ["group", "Group", "Back to grouping stage?"], - group: ["brainstorm", "Brainstorm", "Back to brainstorming stage?"], - }, -}; - -export default function ChangeStatusButton({ - variant, - status, - onStatusUpdate, -}: { - variant: keyof typeof variants; - status: RetroStatus; - onStatusUpdate: (status: RetroStatus) => void; -}) { - const [open, setOpen] = useState(false); - - if (variant === "next" && status === "discuss") { - return null; - } - - if (variant === "prev" && status === "brainstorm") { - return null; - } - - const [newStatus, buttonText, title] = variants[variant][status]; - - return ( - - - - - - - - {title} - - Make sure everyone is ready before moving to the{" "} - {variant === "next" ? "next " : "previous "} - stage! - - - - - - - - - - - - ); -} diff --git a/ui/src/components/retro/column-delete-dialog.tsx b/ui/src/components/retro/column-delete-dialog.tsx new file mode 100644 index 0000000..3023a76 --- /dev/null +++ b/ui/src/components/retro/column-delete-dialog.tsx @@ -0,0 +1,40 @@ +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + AlertDialogTrigger, +} from "@/components/ui/alert-dialog"; + +export default function ColumnDeleteDialog({ + children, + columnTitle, + onDelete, +}: { + children: React.ReactNode; + columnTitle: string; + onDelete: () => void; +}) { + return ( + + {children} + + + Delete “{columnTitle}”? + + The column is empty, so nothing is lost, but everyone in this retro + will see it disappear. + + + + Cancel + Delete + + + + ); +} diff --git a/ui/src/components/retro/column-dialog.tsx b/ui/src/components/retro/column-dialog.tsx new file mode 100644 index 0000000..83247c7 --- /dev/null +++ b/ui/src/components/retro/column-dialog.tsx @@ -0,0 +1,128 @@ +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@/components/ui/dialog"; +import { + Form, + FormControl, + FormDescription, + FormField, + FormItem, + FormLabel, + FormMessage, +} from "@/components/ui/form"; +import { Input } from "@/components/ui/input"; +import { RetroColumn } from "@/types"; +import { zodResolver } from "@hookform/resolvers/zod"; +import { useState } from "react"; +import { useForm } from "react-hook-form"; +import { z } from "zod"; + +// Mirrors the server's column validation. +const schema = z.object({ + title: z.string().trim().min(2).max(255), + description: z.string().trim().max(255), +}); + +export type ColumnData = z.infer; + +export default function ColumnDialog({ + children, + title, + description, + column, + onSave, +}: { + children: React.ReactNode; + title: string; + description: string; + column?: RetroColumn; + onSave: (data: ColumnData) => void; +}) { + const [open, setOpen] = useState(false); + + const form = useForm({ + resolver: zodResolver(schema), + defaultValues: { + title: column?.title ?? "", + description: column?.description ?? "", + }, + }); + + function handleOpenChange(next: boolean) { + setOpen(next); + + // Reopening should show what the column says now, not a half-finished edit + // or someone else's live rename. + if (next) { + form.reset({ + title: column?.title ?? "", + description: column?.description ?? "", + }); + } + } + + return ( + + {children} + + + + {title} + {description} + + +
+ { + setOpen(false); + onSave(data); + })} + > + ( + + Title + + + + + + )} + /> + + ( + + Description + + + + + A prompt to help people fill this column in. Optional. + + + + )} + /> + + + + + + +
+ + ); +} diff --git a/ui/src/components/retro/column-states.tsx b/ui/src/components/retro/column-states.tsx new file mode 100644 index 0000000..50b78e2 --- /dev/null +++ b/ui/src/components/retro/column-states.tsx @@ -0,0 +1,37 @@ +import { cn } from "@/lib/utils"; + +export function NoteSkeletons({ count = 2 }: { count?: number }) { + const heights = [64, 88, 72, 96]; + + return ( +
+ {Array.from({ length: count }).map((_, i) => ( +
+ ))} +
+ ); +} + +export function EmptyColumn({ + children, + className, +}: { + children: React.ReactNode; + className?: string; +}) { + return ( +
+ {children} +
+ ); +} diff --git a/ui/src/components/retro/columns.tsx b/ui/src/components/retro/columns.tsx index 91a815a..8b0032a 100644 --- a/ui/src/components/retro/columns.tsx +++ b/ui/src/components/retro/columns.tsx @@ -1,53 +1,224 @@ +import { Button } from "@/components/ui/button"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "@/components/ui/tooltip"; +import { ColumnActions } from "@/hooks/use-columns"; +import { accentStyle } from "@/lib/column-accent"; import * as types from "@/types"; import { useDroppable } from "@dnd-kit/core"; +import { Pencil, Plus, Trash2 } from "lucide-react"; import { Children } from "react"; import { twMerge } from "tailwind-merge"; import { Heading } from "../typography"; +import ColumnDeleteDialog from "./column-delete-dialog"; +import ColumnDialog, { ColumnData } from "./column-dialog"; -function Columns({ children }: { children: React.ReactNode }) { - const gridCols = [ - "grid-cols-2", - "grid-cols-3", - "grid-cols-4", - "grid-cols-5", - "grid-cols-6", - ][Children.count(children) - 2]; +// Literal strings so Tailwind's scanner finds them. Below lg the board is a +// horizontal snap-scroller instead. +const gridColumns = [ + "lg:grid-cols-2", + "lg:grid-cols-3", + "lg:grid-cols-4", + "lg:grid-cols-5", + "lg:grid-cols-6", +]; + +function Columns({ + children, + count, + onAddColumn, + canAddColumn, +}: { + children: React.ReactNode; + count?: number; + onAddColumn?: (data: ColumnData) => void; + canAddColumn?: boolean; +}) { + const columnCount = Math.min( + Math.max(count ?? Children.count(children), 2), + 6, + ); return ( -
- {children} +
+ {onAddColumn && ( +
+ + + +
+ )} + +
+ {children} +
); } const Column = function Column({ column, + index = 0, children, className, + style, + onEdit, + onDelete, + canDelete, ...props }: { column: types.RetroColumn; + index?: number; children: React.ReactNode; className?: string; -} & React.ComponentProps<"div">) { +} & Partial & + React.ComponentProps<"div">) { + const hasActions = !!(onEdit || onDelete); + return ( -
-
- {column.title} -

{column.description}

+
+
+
+
+ + + {column.title} + +
+ + {column.description && ( +

+ {column.description} +

+ )} +
+ + {hasActions && ( +
+ {onEdit && ( + + + + )} + + {onDelete && ( + + )} +
+ )}
-
{children}
+ +
+ +
{children}
); }; +function ColumnDeleteButton({ + column, + onDelete, + canDelete, +}: { + column: types.RetroColumn; +} & Pick) { + const button = ( + + ); + + if (canDelete) { + return ( + + {button} + + ); + } + + return ( + + + {button} + + + Only empty columns can be deleted, and a retro needs at least two. + + + ); +} + function DroppableColumn({ column, + index, children, + ...actions }: { column: types.RetroColumn; + index?: number; children: React.ReactNode; -}) { +} & Partial) { const { setNodeRef, isOver } = useDroppable({ id: column.id, }); @@ -56,7 +227,20 @@ function DroppableColumn({ {children} diff --git a/ui/src/components/retro/connection-indicator.tsx b/ui/src/components/retro/connection-indicator.tsx index 504f251..755767b 100644 --- a/ui/src/components/retro/connection-indicator.tsx +++ b/ui/src/components/retro/connection-indicator.tsx @@ -1,11 +1,22 @@ -import { PayloadConnectionInfo } from "@/events"; import { Popover, PopoverContent, PopoverTrigger, } from "@/components/ui/popover"; +import { PayloadConnectionInfo } from "@/events"; +import { accentForName, initialsFor } from "@/lib/column-accent"; +import { spring, springy } from "@/lib/motion"; +import { AnimatePresence, m } from "motion/react"; import { Button } from "../ui/button"; -import { User, Zap } from "lucide-react"; + +const states: Record = { + 0: { label: "Connecting", dot: "bg-yellow-500", live: false }, + 1: { label: "Connected", dot: "bg-green-500", live: true }, + 2: { label: "Disconnecting", dot: "bg-yellow-500", live: false }, + 3: { label: "Disconnected", dot: "bg-red-500", live: false }, +}; + +const maxAvatars = 3; export default function ConnectionIndicator({ connectionInfo, @@ -14,32 +25,84 @@ export default function ConnectionIndicator({ connectionInfo: PayloadConnectionInfo; readyState: number; }) { - const [state, stateClassName] = { - 0: ["Connecting", "text-yellow-500"], - 1: ["Connected", "text-green-500"], - 2: ["Disconnecting", "text-yellow-500"], - 3: ["Disconnected", "text-red-500"], - }[readyState] || ["Unknown", "text-red-500"]; + const state = states[readyState] ?? { + label: "Unknown", + dot: "bg-red-500", + live: false, + }; + + const users = connectionInfo.users; + const visible = users.slice(0, maxAvatars); + const overflow = users.length - visible.length; return ( - - - {connectionInfo.users.length === 0 ? ( -
- No users connected -
+ + +

{state.label}

+ + {users.length === 0 ? ( +

Nobody else is here.

) : ( -
    - {connectionInfo.users.map((user) => ( -
  • - - {user} +
      + {users.map((user) => ( +
    • + + {initialsFor(user)} + + {user}
      • ))}
    diff --git a/ui/src/components/retro/creator.tsx b/ui/src/components/retro/creator.tsx index 5b1aa84..532fdf1 100644 --- a/ui/src/components/retro/creator.tsx +++ b/ui/src/components/retro/creator.tsx @@ -1,12 +1,15 @@ +import { AutoTextarea } from "@/components/ui/auto-textarea"; import { Button } from "@/components/ui/button"; -import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Checkbox } from "@/components/ui/checkbox"; import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from "@/components/ui/dropdown-menu"; + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@/components/ui/dialog"; import { Form, FormControl, @@ -17,38 +20,54 @@ import { FormMessage, } from "@/components/ui/form"; import { Input } from "@/components/ui/input"; -import useTemplates from "@/hooks/use-templates"; +import { useAuth } from "@/hooks/use-auth"; import { api } from "@/lib/api"; +import { cardVariants, spring, stagger } from "@/lib/motion"; +import { cn } from "@/lib/utils"; import { Route as RetrosRoute } from "@/routes/_auth.retros.$retroId"; import { Retro } from "@/types"; import { zodResolver } from "@hookform/resolvers/zod"; import { useNavigate } from "@tanstack/react-router"; -import { BookDashed, Trash2 } from "lucide-react"; -import { useFieldArray, useForm, useFormContext } from "react-hook-form"; +import { Columns3, Plus, Trash2 } from "lucide-react"; +import { AnimatePresence, m } from "motion/react"; +import { useState } from "react"; +import { Control, useFieldArray, useForm } from "react-hook-form"; +import { toast } from "sonner"; import { z } from "zod"; import AIRetroTemplate from "./ai-retro-template"; -import { useAuth } from "@/hooks/use-auth"; import TagInput from "./tag-input"; +import TemplatePicker from "./template-picker"; + +const minColumns = 2; +const maxColumns = 5; +const maxDescription = 255; const schema = z.object({ - title: z.string().min(5).max(255), + title: z.string().trim().min(5).max(255), columns: z .array( z.object({ - title: z.string().min(2).max(255), - description: z.string().max(255), + title: z.string().trim().min(2).max(255), + description: z.string().trim().max(maxDescription), }), ) - .min(2, "At least 2 columns are required") - .max(5, "At most 5 columns are allowed"), + .min(minColumns, `Add at least ${minColumns} columns`) + .max(maxColumns, `${maxColumns} columns is the most a board can hold`), unlisted: z.boolean().optional(), tags: z.array(z.string().min(1).max(50)).max(10).optional(), }); -export default function Creator({ className }: { className?: string }) { +type FormValues = z.infer; +type ColumnDraft = FormValues["columns"][number]; + +const emptyColumn: ColumnDraft = { title: "", description: "" }; + +export default function Creator() { const navigate = useNavigate(); + const [open, setOpen] = useState(false); + const [generating, setGenerating] = useState(false); - const form = useForm({ + const form = useForm({ resolver: zodResolver(schema), defaultValues: { title: "", @@ -58,23 +77,53 @@ export default function Creator({ className }: { className?: string }) { }, }); - function handleSubmit(data: z.infer) { - api.post("/api/retros", data).then((response) => { - navigate({ to: RetrosRoute.path, params: { retroId: response.data.id } }); - }); + const columns = useFieldArray({ control: form.control, name: "columns" }); + + function handleSubmit(data: FormValues) { + api + .post("/api/retros", data) + .then((response) => { + setOpen(false); + form.reset(); + navigate({ + to: RetrosRoute.path, + params: { retroId: response.data.id }, + }); + }) + .catch(() => { + toast.error("Couldn't create the retro", { + description: "Give it another go in a moment.", + }); + }); + } + + function applyColumns(next: ColumnDraft[]) { + columns.replace(next.slice(0, maxColumns)); + form.clearErrors("columns"); } return ( - - - Create a retrospective - + + + + + + + + Create a retrospective + + Start from a template, dream one up, or write your own. Everything + here can be changed later. + + -
    Title - - Give your retrospective a title - )} /> - - - ( - - - - -
    - Unlisted - - If checked, the retrospective will not be listed on the - home page. - -
    -     - )} + fields={columns.fields} + generating={generating} + onApply={applyColumns} + onAdd={() => columns.append(emptyColumn)} + onRemove={columns.remove} + onClear={() => applyColumns([])} + onGeneratingChange={setGenerating} /> - ( - - Tags - - - - - Tag retros to group them by team or project. Press Enter or comma to add. - - - - )} - /> +
    - + + + + - - + +
    ); } -function Columns() { - const { fields, append, remove } = useFieldArray({ name: "columns" }); - const { control } = useFormContext(); - +function ColumnsSection({ + control, + fields, + generating, + onApply, + onAdd, + onRemove, + onClear, + onGeneratingChange, +}: { + control: Control; + fields: { id: string }[]; + generating: boolean; + onApply: (columns: ColumnDraft[]) => void; + onAdd: () => void; + onRemove: (index: number) => void; + onClear: () => void; + onGeneratingChange: (generating: boolean) => void; +}) { const { user } = useAuth(); + const full = fields.length >= maxColumns; + return ( -
    -

    Columns

    - - {fields.map((field, index) => ( - remove(index)} - /> - ))} +
    +
    +
    +

    Columns

    + + {fields.length} of {maxColumns} + +
    - } /> +
    + {fields.length > 0 && ( + + )} -
    - + -
    - +
    +
    - {user?.ai_enabled && ( -
    - -
    + {user?.ai_enabled && ( + + )} + +
    0 && "opacity-50", )} + > + + {fields.length === 0 && generating && ( + + )} + + {fields.length === 0 && !generating && } + + {fields.map((field, index) => ( + onRemove(index)} + /> + ))} +
    -
    + + } + /> +
    ); } -function ColumnInput({ +function ColumnCard({ + control, index, onRemove, }: { + control: Control; index: number; onRemove: () => void; }) { - const { control } = useFormContext(); - return ( -
    -
    - + +
    + Column {index + 1} +
    -
    +
    ( - Title + Title - + @@ -239,41 +337,133 @@ function ColumnInput({ name={`columns.${index}.description`} render={({ field }) => ( - Description + + Description{" "} + + (optional) + + - + - + +
    + + +
    )} />
    -
    +     + ); +} + +function CharacterCount({ value }: { value: string }) { + const remaining = maxDescription - value.length; + + if (remaining > 40) return null; + + return ( + + {remaining} left + ); } -function FromTemplateDropDown() { - const templates = useTemplates(); - const { setValue } = useFormContext(); +function EmptyColumns() { + return ( + + +

    No columns yet

    +

    + Pick a template, name a theme above, or add your own. +

    +     + ); +} +function GeneratingPlaceholder() { return ( - - - - - - {templates.map((template) => ( - setValue("columns", template.columns)} - > - {template.title} - - ))} - - + + {[0, 1, 2].map((i) => ( +
    +
    +
    +
    +
    + ))} + + ); +} + +function Details({ control }: { control: Control }) { + return ( +
    + ( + + Tags + + + + + Groups retros together by team or project. Tab or Enter to add. + + + + )} + /> + + ( + + + + +
    + Unlisted + + Keep it off the home page. Anyone with the link can still join. + +
    +     + )} + /> +
    ); } diff --git a/ui/src/components/retro/discuss.tsx b/ui/src/components/retro/discuss.tsx index 7d02a7b..778a762 100644 --- a/ui/src/components/retro/discuss.tsx +++ b/ui/src/components/retro/discuss.tsx @@ -1,11 +1,14 @@ import { createSocketEvent, SocketEvent } from "@/events"; +import { useColumnActions } from "@/hooks/use-columns"; import { useNotes } from "@/hooks/use-notes"; import useRetro from "@/hooks/use-retro"; import { api } from "@/lib/api"; import { Task as TaskType } from "@/types"; import { Plus } from "lucide-react"; +import { AnimatePresence } from "motion/react"; import { useCallback, useEffect, useState } from "react"; import { Button } from "../ui/button"; +import { EmptyColumn, NoteSkeletons } from "./column-states"; import { Column, Columns } from "./columns"; import { Note } from "./note"; import { NoteGroup } from "./note-group"; @@ -23,7 +26,8 @@ export default function Discuss() { retro, socket: { lastJsonMessage }, } = useRetro(); - const { groupedNotes, dispatch } = useNotes(); + const { notes, groupedNotes, loaded, dispatch } = useNotes(); + const columnActions = useColumnActions(notes); const [votes, setVotes] = useState([]); const [tasks, setTasks] = useState([]); @@ -101,61 +105,92 @@ export default function Discuss() { } return ( - - {retro.columns.map((column) => ( - - {groupedNotesForColumn(column.id).map(([groupId, groupNotes]) => ( - v.group_id === groupId)?.count ?? 0, - total: votes.reduce((acc, v) => acc + v.count, 0), - }} - authors={Array.from( - new Set( - groupNotes - .map((note) => note.created_by_name) - .filter((name): name is string => Boolean(name)), - ), - )} - > - {groupNotes.map((note) => ( - + + {retro.columns.map((column, index) => { + const groups = groupedNotesForColumn(column.id); + + return ( + + {!loaded && } + + {loaded && groups.length === 0 && ( + Nothing came up here. + )} + + + {groups.map(([groupId, groupNotes]) => ( + v.group_id === groupId)?.count ?? 0, + total: votes.reduce((acc, v) => acc + v.count, 0), + }} + authors={Array.from( + new Set( + groupNotes + .map((note) => note.created_by_name) + .filter((name): name is string => Boolean(name)), + ), + )} + > + {groupNotes.map((note) => ( + + ))} + ))} - - ))} - - ))} + + + ); + })} - {tasks - .sort((a, b) => Number(a.completed) - Number(b.completed)) - .map((task) => ( - handleEditTask(task.id, d)} - onComplete={(c) => handleTaskComplete(task.id, c)} - /> - ))} + {tasks.length === 0 && ( + No action items yet. + )} + + + {[...tasks] + .sort((a, b) => Number(a.completed) - Number(b.completed)) + .map((task) => ( + handleEditTask(task.id, d)} + onComplete={(c) => handleTaskComplete(task.id, c)} + /> + ))} + ); diff --git a/ui/src/components/retro/gif-dialog.tsx b/ui/src/components/retro/gif-dialog.tsx index ae54972..76d98a1 100644 --- a/ui/src/components/retro/gif-dialog.tsx +++ b/ui/src/components/retro/gif-dialog.tsx @@ -6,29 +6,15 @@ import { DialogTitle, DialogTrigger, } from "@/components/ui/dialog"; +import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; -import { z } from "zod"; -import { useForm } from "react-hook-form"; -import { zodResolver } from "@hookform/resolvers/zod"; -import { - Form, - FormControl, - FormDescription, - FormField, - FormItem, - FormMessage, -} from "../ui/form"; -import { api } from "@/lib/api"; -import { useState } from "react"; - -const schema = z.object({ - query: z.string().min(2).max(32), -}); - -interface SearchResult { - preview_url: string; - url: string; -} +import { Spinner } from "@/components/ui/spinner"; +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; +import { GifResult, useGifSearch } from "@/hooks/use-gif-search"; +import useRetro from "@/hooks/use-retro"; +import { cn } from "@/lib/utils"; +import { ImageOff, Link2, SearchIcon, SearchX } from "lucide-react"; +import { useEffect, useRef, useState } from "react"; export default function GIFDialog({ children, @@ -37,79 +23,256 @@ export default function GIFDialog({ children: React.ReactNode; onSelect: (url: string) => void; }) { + const { + retro: { gif_search_enabled }, + } = useRetro(); + const [open, setOpen] = useState(false); - const [results, setResults] = useState([]); - - const form = useForm({ - resolver: zodResolver(schema), - defaultValues: { - query: "", - }, - }); - - function handleSubmit(data: z.infer) { - api - .post("/api/gifs", { - q: data.query, - }) - .then((res) => { - setResults(res.data); - }); + function handleSelect(url: string) { + setOpen(false); + onSelect(url); } return ( {children} - + + - Choose a GIF + Add an image - Search for a GIF to add to your note. + {gif_search_enabled + ? "Search for a GIF, or paste a link to any image." + : "Paste a link to any image or GIF."} -
    - - ( - - - - - - Press enter to search - - - - )} - /> - - - -
      - {results.map((result, i) => ( -
    • - -
      • - ))} -
    + + + + + Search + + + + Paste link + + + + + {gif_search_enabled && } + + + + + +
    ); } + +function SearchTab({ onSelect }: { onSelect: (url: string) => void }) { + const [query, setQuery] = useState(""); + const { results, status, hasNext, loadMore } = useGifSearch(query, true); + + const sentinel = useRef(null); + + useEffect(() => { + const target = sentinel.current; + if (!target || !hasNext) return; + + const observer = new IntersectionObserver( + ([entry]) => entry.isIntersecting && loadMore(), + { rootMargin: "200px" }, + ); + + observer.observe(target); + + return () => observer.disconnect(); + }, [hasNext, loadMore]); + + const isFirstLoad = status === "loading"; + + return ( +
    + setQuery(e.target.value)} + placeholder="dancing cat" + aria-label="Search for a GIF" + /> + +
    + {status === "error" && ( + } + title="Couldn't reach the GIF service" + body="Try again in a moment, or paste a link instead." + /> + )} + + {isFirstLoad && } + + {status !== "error" && !isFirstLoad && results.length === 0 && ( + } + title={query.trim() ? `No GIFs for "${query.trim()}"` : "Nothing to show"} + body="Try a different search." + /> + )} + + {!isFirstLoad && results.length > 0 && ( + <> +
    + {results.map((result, i) => ( + onSelect(result.url)} + /> + ))} +
    + +
    + {status === "loading-more" && } +
    + + )} +
    +
    + ); +} + +function Tile({ + result, + onSelect, +}: { + result: GifResult; + onSelect: () => void; +}) { + const [loaded, setLoaded] = useState(false); + + // Reserve the tile's space up front so the grid does not reflow as images + // stream in. Providers do not always report dimensions, so fall back square. + const aspectRatio = + result.width && result.height ? result.width / result.height : 1; + + return ( + + ); +} + +function TileSkeletons() { + const heights = [140, 96, 120, 108, 152, 88, 116, 132, 100]; + + return ( +
    + {heights.map((height, i) => ( +
    + ))} +
    + ); +} + +function LinkTab({ onSelect }: { onSelect: (url: string) => void }) { + const [value, setValue] = useState(""); + const [broken, setBroken] = useState(false); + + const url = value.trim(); + const looksValid = /^https:\/\/\S+$/i.test(url); + const canUse = looksValid && !broken; + + return ( +
    { + e.preventDefault(); + if (canUse) onSelect(url); + }} + > + { + setValue(e.target.value); + setBroken(false); + }} + placeholder="https://media.giphy.com/…" + aria-label="Image URL" + /> + +

    + Any https image or GIF link works. Right-click an image anywhere and + copy its address. +

    + +
    + {!looksValid && ( +

    Preview appears here

    + )} + + {looksValid && broken && ( +

    + That link didn't load as an image +

    + )} + + {looksValid && ( + setBroken(true)} + className={cn("max-h-full max-w-full object-contain", broken && "hidden")} + /> + )} +
    + + +
    + ); +} + +function Empty({ + icon, + title, + body, +}: { + icon: React.ReactNode; + title: string; + body: string; +}) { + return ( +
    + {icon} +

    {title}

    +

    {body}

    +
    + ); +} diff --git a/ui/src/components/retro/group.tsx b/ui/src/components/retro/group.tsx index 7a21b35..4fae927 100644 --- a/ui/src/components/retro/group.tsx +++ b/ui/src/components/retro/group.tsx @@ -1,16 +1,21 @@ import { createSocketEvent } from "@/events"; +import { useColumnActions } from "@/hooks/use-columns"; import { useNotes } from "@/hooks/use-notes"; import useRetro from "@/hooks/use-retro"; -import { DndContext, DragEndEvent } from "@dnd-kit/core"; +import { DragEndEvent } from "@dnd-kit/core"; +import { AnimatePresence } from "motion/react"; +import { EmptyColumn, NoteSkeletons } from "./column-states"; import { Columns, DroppableColumn } from "./columns"; import { DraggableNote } from "./note"; +import NoteDndContext from "./note-dnd"; import { DroppableNoteGroup } from "./note-group"; export default function Group() { const { retro: { columns }, } = useRetro(); - const { notes, groupedNotes, dispatch } = useNotes(); + const { notes, groupedNotes, loaded, dispatch } = useNotes(); + const columnActions = useColumnActions(notes); function handleDragEnd(event: DragEndEvent) { const overId = event.over?.id as string | undefined; @@ -22,7 +27,6 @@ export default function Group() { let groupId = ""; if (overId.includes(".")) { - // Dragged to a group [columnId, groupId] = overId.split("."); } @@ -37,27 +41,58 @@ export default function Group() { ); } + function handleUngroup(noteId: string) { + dispatch(createSocketEvent("note_update", { id: noteId, group_id: "" })); + } + return ( - - - {columns.map((column) => ( - - {Object.entries(groupedNotes[column.id] ?? []).map( - ([groupId, groupNotes]) => ( - - {groupNotes.map((note) => ( - - ))} - - ), - )} - - ))} + + + {columns.map((column, index) => { + const groups = Object.entries(groupedNotes[column.id] ?? {}); + + return ( + + {!loaded && } + + {loaded && groups.length === 0 && ( + No thoughts in this column. + )} + + + {groups.map(([groupId, groupNotes]) => ( + + {groupNotes.map((note) => ( + 1 + ? () => handleUngroup(note.id) + : undefined + } + /> + ))} + + ))} + + + ); + })} - + ); } diff --git a/ui/src/components/retro/hero.tsx b/ui/src/components/retro/hero.tsx index b597a0e..cb20b2a 100644 --- a/ui/src/components/retro/hero.tsx +++ b/ui/src/components/retro/hero.tsx @@ -1,9 +1,18 @@ +import { spring } from "@/lib/motion"; import { - ZapIcon, - UsersIcon, + animate, + m, + useInView, + useMotionValue, + useTransform, +} from "motion/react"; +import { useEffect, useRef } from "react"; +import { + CheckSquareIcon, FileDownIcon, SparklesIcon, - CheckSquareIcon, + UsersIcon, + ZapIcon, } from "lucide-react"; interface HeroStats { @@ -20,48 +29,69 @@ const features = [ { icon: FileDownIcon, label: "Markdown export" }, ]; -export default function Hero({ stats }: { stats: HeroStats }) { +const stats = [ + { key: "retro_count", label: "retrospectives", accent: "var(--chart-1)" }, + { key: "note_count", label: "thoughts", accent: "var(--chart-2)" }, + { key: "task_count", label: "actions", accent: "var(--chart-4)" }, +] as const; + +export default function Hero({ stats: values }: { stats: HeroStats }) { return ( -
    - {/* Wispy background orbs */} -
    -
    -
    -
    +
    + + + -
    - {/* Left: tagline + features */} +
    -

    - A simple tool for running better retrospectives with your team. -

    +

    + Better retrospectives, + together. +

    +
    - {features.map(({ icon: Icon, label }) => ( - + {features.map(({ icon: Icon, label }, i) => ( + {label} - + ))}
    - {/* Right: stats */} -
    - - - +
    + {stats.map((stat) => ( + + ))}
    @@ -71,19 +101,34 @@ export default function Hero({ stats }: { stats: HeroStats }) { function Stat({ value, label, - gradient, + accent, }: { value: number; label: string; - gradient: string; + accent: string; }) { + const ref = useRef(null); + const inView = useInView(ref, { once: true }); + + const count = useMotionValue(0); + const rounded = useTransform(count, (v) => Math.round(v).toLocaleString()); + + useEffect(() => { + if (!inView) return; + + const controls = animate(count, value, { duration: 0.9, ease: "easeOut" }); + + return () => controls.stop(); + }, [inView, value, count]); + return ( -
    -
    + - {value} -
    + {rounded} +
    {label}
    ); diff --git a/ui/src/components/retro/list.tsx b/ui/src/components/retro/list.tsx index 4967a42..1f901d3 100644 --- a/ui/src/components/retro/list.tsx +++ b/ui/src/components/retro/list.tsx @@ -1,71 +1,95 @@ -import { - Card, - CardContent, - CardDescription, - CardHeader, - CardTitle, -} from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; -import { Link, useNavigate } from "@tanstack/react-router"; +import { accentForIndex } from "@/lib/column-accent"; +import { cardVariants, spring, stagger } from "@/lib/motion"; +import { stageLabel } from "@/lib/stages"; import { Retro, RetroStatus } from "@/types"; -import { CircleCheck, StickyNote } from "lucide-react"; +import { Link, useNavigate } from "@tanstack/react-router"; +import { CircleCheck, StickyNote, Telescope } from "lucide-react"; +import { AnimatePresence, m } from "motion/react"; -const statusLabel: Record = { - brainstorm: "Brainstorm", - group: "Grouping", - vote: "Voting", - discuss: "Discuss", +const statusAccent: Record = { + brainstorm: accentForIndex(0), + group: accentForIndex(1), + vote: accentForIndex(3), + discuss: accentForIndex(2), }; export default function List({ retros }: { retros?: Retro[] }) { - return ( - - - Recent Retros - Your last {retros?.length ?? 0} retrospectives - + if (!retros || retros.length === 0) { + return ( +
    + +

    No retros yet

    +

    + Start one above and share the link. Everyone joins by typing their + name. +

    +
    + ); + } - - {!retros || retros.length === 0 ? ( -

    No retros yet 😢

    - ) : ( -
    - {retros.map((retro) => ( - - ))} -
    - )} -     -     + return ( +
    + + {retros.map((retro, i) => ( + + ))} + +
    ); } -export function RetroItem({ retro }: { retro: Retro }) { +export function RetroItem({ + retro, + index = 0, +}: { + retro: Retro; + index?: number; +}) { const navigate = useNavigate(); const allTasksDone = retro.task_count > 0 && retro.task_count === retro.task_completed_count; + function open() { + navigate({ to: "/retros/$retroId", params: { retroId: retro.id } }); + } + return ( -
    navigate({ to: "/retros/$retroId", params: { retroId: retro.id } })} + onClick={open} onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { - navigate({ to: "/retros/$retroId", params: { retroId: retro.id } }); + e.preventDefault(); + open(); } }} - className="flex flex-col gap-3 rounded-lg border bg-card p-4 hover:bg-accent hover:text-accent-foreground transition-colors cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring" + className="group relative flex cursor-pointer flex-col gap-3 overflow-hidden rounded-xl bg-surface-raised p-4 pl-5 ring-1 ring-border/70 transition-shadow hover:shadow-[0_8px_28px_-10px_rgb(0_0_0/0.25)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:outline-none" > - {/* Title + status */} + +
    - {retro.title} - - {statusLabel[retro.status]} + {retro.title} + + {stageLabel(retro.status)}
    - {/* Tags */} {retro.tags && retro.tags.length > 0 && (
    {retro.tags.map((tag) => ( @@ -74,7 +98,7 @@ export function RetroItem({ retro }: { retro: Retro }) { to="/tags/$tag" params={{ tag }} onClick={(e) => e.stopPropagation()} - className="text-xs text-muted-foreground hover:text-foreground transition-colors" + className="text-xs text-muted-foreground transition-colors hover:text-foreground" > #{tag} @@ -82,22 +106,25 @@ export function RetroItem({ retro }: { retro: Retro }) {
    )} - {/* Footer */} -
    +
    - {retro.note_count} notes + {retro.note_count} {retro.task_count > 0 && ( - + - {retro.task_completed_count}/{retro.task_count} tasks - + {retro.task_completed_count}/{retro.task_count} + )}
    {new Date(retro.created_at).toLocaleDateString()}
    -
    + ); } diff --git a/ui/src/components/retro/note-dialog.tsx b/ui/src/components/retro/note-dialog.tsx index f369cbd..b222e20 100644 --- a/ui/src/components/retro/note-dialog.tsx +++ b/ui/src/components/retro/note-dialog.tsx @@ -1,7 +1,9 @@ +import { Button } from "@/components/ui/button"; import { Dialog, DialogContent, DialogDescription, + DialogFooter, DialogHeader, DialogTitle, DialogTrigger, @@ -9,7 +11,6 @@ import { import { Form, FormControl, - FormDescription, FormField, FormItem, FormLabel, @@ -77,15 +78,22 @@ export default function NoteDialog({ Note - + - Press enter to submit - )} /> + + + + diff --git a/ui/src/components/retro/note-dnd.tsx b/ui/src/components/retro/note-dnd.tsx new file mode 100644 index 0000000..09ebb1a --- /dev/null +++ b/ui/src/components/retro/note-dnd.tsx @@ -0,0 +1,74 @@ +import { Note } from "@/types"; +import { + CollisionDetection, + DndContext, + DragEndEvent, + DragOverlay, + DragStartEvent, + KeyboardSensor, + PointerSensor, + pointerWithin, + rectIntersection, + useSensor, + useSensors, +} from "@dnd-kit/core"; +import { useState } from "react"; +import { NoteOverlay } from "./note"; + +// By area a group always beats the column it sits in, leaving no way to drop +// a note into open space. pointerWithin ranks the tightest rect around the +// cursor first; keyboard drags have no pointer and fall back to overlap. +const collisionDetection: CollisionDetection = (args) => { + const underPointer = pointerWithin(args); + + return underPointer.length > 0 ? underPointer : rectIntersection(args); +}; + +export default function NoteDndContext({ + notes, + showAuthor, + onDragEnd, + children, +}: { + notes: Note[]; + showAuthor?: boolean; + onDragEnd: (event: DragEndEvent) => void; + children: React.ReactNode; +}) { + const [activeId, setActiveId] = useState(null); + + const sensors = useSensors( + // Without a threshold every click on the grip starts a drag. + useSensor(PointerSensor, { activationConstraint: { distance: 5 } }), + useSensor(KeyboardSensor), + ); + + const active = notes.find((note) => note.id === activeId); + + function handleDragStart(event: DragStartEvent) { + setActiveId(String(event.active.id)); + } + + function handleDragEnd(event: DragEndEvent) { + setActiveId(null); + onDragEnd(event); + } + + return ( + setActiveId(null)} + > + {children} + + {/* The default drop animation flies the card back to the slot it + started in, which is the one place it is no longer going. */} + + {active && } + + + ); +} diff --git a/ui/src/components/retro/note-group.tsx b/ui/src/components/retro/note-group.tsx index 2b394ca..199253d 100644 --- a/ui/src/components/retro/note-group.tsx +++ b/ui/src/components/retro/note-group.tsx @@ -1,48 +1,60 @@ +import { cardVariants, spring, springy } from "@/lib/motion"; import { useDroppable } from "@dnd-kit/core"; import { Check, Flame, TrendingUp, X } from "lucide-react"; +import { AnimatePresence, m } from "motion/react"; import React, { Children } from "react"; import { twMerge } from "tailwind-merge"; import { Badge } from "../ui/badge"; import { Button } from "../ui/button"; -interface NoteGroupProps extends React.HTMLAttributes { +// Omit children: motion widens it to accept MotionValues, which Children.count +// cannot deal with. +interface NoteGroupProps + extends Omit, "children"> { voteCount?: { forGroup: number; total: number; }; authors?: string[]; + children?: React.ReactNode; } - export const NoteGroup = ({ voteCount, authors, className, children, ...props -}: NoteGroupProps & React.ComponentProps<"div">) => { +}: NoteGroupProps) => { const hasVoteCount = voteCount !== undefined; const childCount = Children.count(children); const isGrouped = childCount > 1; const hasNonZeroVotes = hasVoteCount && voteCount.forGroup > 0; const showFooter = authors || hasNonZeroVotes; - const classes = twMerge( - "rounded-lg transition-all duration-100", - isGrouped || hasVoteCount - ? "bg-muted/60 border border-border/60 p-2 space-y-2" - : "space-y-1", - className, - ); - return ( -
    + {children} {showFooter && (
    {authors && ( - + {authors.join(", ")} )} @@ -52,7 +64,7 @@ export const NoteGroup = ({ )}
    )} -
    + ); }; @@ -61,21 +73,20 @@ const hotThreshold = 0.098; function VoteCount({ forGroup, total }: { forGroup: number; total: number }) { if (forGroup === 0 || total === 0) return null; - let Icon: React.ElementType; - let variant: "destructive" | "secondary"; - - if (forGroup / total >= hotThreshold) { - Icon = Flame; - variant = "destructive"; - } else { - Icon = TrendingUp; - variant = "secondary"; - } + const isHot = forGroup / total >= hotThreshold; + const Icon = isHot ? Flame : TrendingUp; return ( - - {forGroup} - + + + {forGroup} + + ); } @@ -94,8 +105,12 @@ export function DroppableNoteGroup({ return ( {children} @@ -114,18 +129,28 @@ export function VotableNoteGroup({ canVote: boolean; }) { return ( -
    -
    {children}
    +
    {children}
    -
    + ); } diff --git a/ui/src/components/retro/note.tsx b/ui/src/components/retro/note.tsx index 1df2777..d90eb40 100644 --- a/ui/src/components/retro/note.tsx +++ b/ui/src/components/retro/note.tsx @@ -1,83 +1,191 @@ +import { accentForName, initialsFor } from "@/lib/column-accent"; +import { cardVariants, spring } from "@/lib/motion"; import { Note as NoteType } from "@/types"; import { DraggableAttributes, DraggableSyntheticListeners, useDraggable, } from "@dnd-kit/core"; -import { GripVertical, Image, ImageOff, Pencil, Trash2 } from "lucide-react"; -import React from "react"; +import { + GripVertical, + Image, + ImageOff, + Pencil, + Trash2, + Ungroup, +} from "lucide-react"; +import { m } from "motion/react"; +import React, { useState } from "react"; import { twMerge } from "tailwind-merge"; import { Button } from "../ui/button"; -import NoteDialog from "./note-dialog"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "../ui/tooltip"; import GIFDialog from "./gif-dialog"; import NoteDeleteDialog from "./note-delete-dialog"; +import NoteDialog from "./note-dialog"; -interface NoteProps extends React.HTMLAttributes { +interface NoteProps { note: NoteType; showGrip?: boolean; blur?: boolean; + showAuthor?: boolean; listeners?: DraggableSyntheticListeners; attributes?: DraggableAttributes; onEdit?: (content: string) => void; onDelete?: () => void; onGifSelected?: (url: string) => void; onGifRemoved?: () => void; + onUngroup?: () => void; } +const shellClassName = + "group relative rounded-lg bg-surface-raised p-2 ring-1 ring-border/70"; + export const Note = ({ note, showGrip, blur, + showAuthor, listeners, attributes, onEdit, onDelete, onGifSelected, onGifRemoved, + onUngroup, className, + ref, ...props -}: NoteProps & React.ComponentProps<"div">) => { - const hasActions = !!(onGifSelected || onGifRemoved || onDelete || onEdit); - +}: NoteProps & React.ComponentProps) => { return ( -
    - {note.img_url && ( -
    - -
    + + + ); +}; + +export function NoteOverlay({ + note, + showAuthor, +}: { + note: NoteType; + showAuthor?: boolean; +}) { + return ( +
    + +
    + ); +} + +function NoteBody({ + note, + showGrip, + blur, + showAuthor, + listeners, + attributes, + onEdit, + onDelete, + onGifSelected, + onGifRemoved, + onUngroup, +}: NoteProps) { + const hasActions = !!( + onGifSelected || + onGifRemoved || + onDelete || + onEdit || + onUngroup + ); + + return ( + <> + {note.img_url && }
    {showGrip && ( )} -

    +

    {note.content}

    + {showAuthor && note.created_by_name && ( + + )} + {hasActions && ( -
    +
    + {onUngroup && ( + + + + + Take out of this group + + )} + {onGifSelected && note.img_url === "" && ( - @@ -88,6 +196,7 @@ export const Note = ({ variant="ghost" size="icon" className="size-6" + aria-label="Remove the image" onClick={onGifRemoved} > @@ -96,7 +205,12 @@ export const Note = ({ {onDelete && ( - @@ -104,57 +218,106 @@ export const Note = ({ {onEdit && ( - )}
    )} + + ); +} + +function NoteImage({ src, blur }: { src: string; blur?: boolean }) { + const [loaded, setLoaded] = useState(false); + + return ( +
    + {!loaded &&
    } + + setLoaded(true)} + className={twMerge( + "h-full w-full object-contain transition-opacity duration-300", + loaded ? "opacity-100" : "opacity-0", + )} + />
    ); -}; +} + +function Author({ name }: { name: string }) { + return ( +
    + + {initialsFor(name)} + + {name} +
    + ); +} export function DraggableNote({ note, - onEdit, - onDelete, - onGifSelected, - onGifRemoved, + ref, + ...actions }: { note: NoteType; - onEdit?: (content: string) => void; - onDelete?: () => void; - onGifSelected?: (url: string) => void; - onGifRemoved?: () => void; -}) { - const { setNodeRef, transform, listeners, attributes } = useDraggable({ + ref?: React.Ref; +} & Pick< + NoteProps, + | "onEdit" + | "onDelete" + | "onGifSelected" + | "onGifRemoved" + | "onUngroup" + | "showAuthor" +>) { + const { setNodeRef, listeners, attributes, isDragging } = useDraggable({ id: note.id, }); - const style = transform - ? { - transform: `translate3d(${transform.x}px, ${transform.y}px, 0)`, - } - : undefined; - return ( { + setNodeRef(node); + + if (typeof ref === "function") ref(node); + else if (ref) ref.current = node; + }} note={note} - style={style} showGrip listeners={listeners} attributes={attributes} - onEdit={onEdit} - onDelete={onDelete} - onGifSelected={onGifSelected} - onGifRemoved={onGifRemoved} + // Animated rather than classed: motion writes opacity inline. + animate={isDragging ? { opacity: 0.3, scale: 0.98 } : "animate"} + {...actions} /> ); } diff --git a/ui/src/components/retro/settings.tsx b/ui/src/components/retro/settings.tsx index 648c363..ea66b82 100644 --- a/ui/src/components/retro/settings.tsx +++ b/ui/src/components/retro/settings.tsx @@ -21,7 +21,7 @@ import useRetro from "@/hooks/use-retro"; import { Retro } from "@/types"; import { zodResolver } from "@hookform/resolvers/zod"; import { Cog } from "lucide-react"; -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useForm } from "react-hook-form"; import { toast } from "sonner"; import { z } from "zod"; @@ -39,11 +39,16 @@ const schema = z.object({ export default function Settings() { const { retro, - setRetro, socket: { sendJsonMessage, lastJsonMessage }, } = useRetro(); + // retro_updated also fires for column edits and for other people's changes, + // so only confirm a save this dialog actually started. + const pendingSave = useRef(false); + function handleSubmit(data: PayloadRetroUpdate) { + pendingSave.current = true; + sendJsonMessage(createSocketEvent("retro_update", data)); } @@ -52,11 +57,11 @@ export default function Settings() { const event = lastJsonMessage as SocketEvent; - if (event.name === "retro_updated") { - setRetro(event.payload as Retro); + if (event.name === "retro_updated" && pendingSave.current) { + pendingSave.current = false; toast("Settings updated", { - description: "The settings for this retrospective been updated.", + description: "The settings for this retrospective have been updated.", }); } }, [lastJsonMessage]); diff --git a/ui/src/components/retro/stage-rail.tsx b/ui/src/components/retro/stage-rail.tsx new file mode 100644 index 0000000..c26fc9c --- /dev/null +++ b/ui/src/components/retro/stage-rail.tsx @@ -0,0 +1,158 @@ +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogClose, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "@/components/ui/tooltip"; +import { spring } from "@/lib/motion"; +import { nextStage, previousStage, stageIndex, stages } from "@/lib/stages"; +import { cn } from "@/lib/utils"; +import { RetroStatus } from "@/types"; +import { ArrowRight, Check, Undo2 } from "lucide-react"; +import { m } from "motion/react"; +import { useState } from "react"; + +export default function StageRail({ + status, + onStatusUpdate, +}: { + status: RetroStatus; + onStatusUpdate: (status: RetroStatus) => void; +}) { + const [pending, setPending] = useState(null); + + const current = stageIndex(status); + const next = nextStage(status); + const previous = previousStage(status); + + const pendingStage = stages.find((stage) => stage.status === pending); + const goingBack = pendingStage ? stageIndex(pendingStage.status) < current : false; + + return ( + <> +
    +
      + {stages.map((stage, index) => { + const done = index < current; + const active = index === current; + const Icon = done ? Check : stage.icon; + + return ( +
    1. + {index > 0 && ( + + )} + + + {active && ( + + )} + + + + + {stage.label} + + + +
      2. + ); + })} +
    + + {previous && ( + + + + + Back to {previous.label.toLowerCase()} + + )} + + {next && ( + + )} +
    + + !open && setPending(null)} + > + + + + {goingBack ? "Go back to" : "Move on to"}{" "} + {pendingStage?.label.toLowerCase()}? + + + Everyone sees this change straight away, so make sure the room is + ready. + + + + + + + + + + + + + ); +} diff --git a/ui/src/components/retro/status-indicator.tsx b/ui/src/components/retro/status-indicator.tsx deleted file mode 100644 index b10d153..0000000 --- a/ui/src/components/retro/status-indicator.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import { RetroStatus } from "@/types"; -import { Brain, CheckCircle2, ChevronRight, Group, Speech, Vote } from "lucide-react"; - -const stages = [ - { status: "brainstorm" as RetroStatus, icon: Brain, label: "Brainstorm" }, - { status: "group" as RetroStatus, icon: Group, label: "Group" }, - { status: "vote" as RetroStatus, icon: Vote, label: "Vote" }, - { status: "discuss" as RetroStatus, icon: Speech, label: "Discuss" }, -]; - -export default function StatusIndicator({ status }: { status: RetroStatus }) { - const currentIndex = stages.findIndex((s) => s.status === status); - - return ( -
      - {stages.map((stage, index) => { - const isDone = index < currentIndex; - const isActive = index === currentIndex; - const Icon = isDone ? CheckCircle2 : stage.icon; - - return ( -
    1. - {index > 0 && ( - - )} - - - {stage.label} - -
      2. - ); - })} -
    - ); -} diff --git a/ui/src/components/retro/tag-input.tsx b/ui/src/components/retro/tag-input.tsx index 91b068a..1fcdd0e 100644 --- a/ui/src/components/retro/tag-input.tsx +++ b/ui/src/components/retro/tag-input.tsx @@ -1,10 +1,13 @@ import { Badge } from "@/components/ui/badge"; -import { Command, CommandEmpty, CommandGroup, CommandItem, CommandList } from "@/components/ui/command"; -import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; +import { + Popover, + PopoverContent, + PopoverTrigger, +} from "@/components/ui/popover"; import { api } from "@/lib/api"; import { cn } from "@/lib/utils"; import { XIcon } from "lucide-react"; -import { useEffect, useRef, useState } from "react"; +import { useEffect, useId, useRef, useState } from "react"; interface TagInputProps { value: string[]; @@ -13,11 +16,31 @@ interface TagInputProps { className?: string; } -export default function TagInput({ value, onChange, placeholder = "Add tag…", className }: TagInputProps) { +const maxTags = 10; +const maxSuggestions = 8; + +interface Option { + value: string; + label: string; + isNew?: boolean; +} + +function normalise(tag: string): string { + return tag.trim().toLowerCase().replace(/\s+/g, "-"); +} + +export default function TagInput({ + value, + onChange, + placeholder = "Add tag...", + className, +}: TagInputProps) { const [input, setInput] = useState(""); const [suggestions, setSuggestions] = useState([]); const [open, setOpen] = useState(false); + const [activeIndex, setActiveIndex] = useState(0); const inputRef = useRef(null); + const listId = useId(); useEffect(() => { api.get("/api/tags").then((res) => { @@ -25,99 +48,172 @@ export default function TagInput({ value, onChange, placeholder = "Add tag…", }); }, []); - const filtered = suggestions.filter( - (s) => !value.includes(s) && (input === "" || s.toLowerCase().includes(input.toLowerCase())) - ); + const typed = normalise(input); + + const matches = suggestions + .filter((s) => !value.includes(s) && (typed === "" || s.includes(typed))) + .slice(0, maxSuggestions); - // Also show freetyped value as an option if it's new - const canAddInput = input.trim() && !value.includes(input.trim().toLowerCase()); + const options: Option[] = [ + ...matches.map((tag) => ({ value: tag, label: tag })), + ...(typed && !matches.includes(typed) && !value.includes(typed) + ? [{ value: typed, label: `Add "${typed}"`, isNew: true }] + : []), + ]; + + const showList = open && options.length > 0; + const active = showList ? options[Math.min(activeIndex, options.length - 1)] : undefined; function addTag(tag: string) { - const clean = tag.trim().toLowerCase().replace(/\s+/g, "-"); - if (!clean || value.includes(clean) || value.length >= 10) return; + const clean = normalise(tag); + + if (!clean || value.includes(clean) || value.length >= maxTags) return; + onChange([...value, clean]); setInput(""); + setActiveIndex(0); } function removeTag(tag: string) { onChange(value.filter((t) => t !== tag)); } + function move(delta: number) { + if (options.length === 0) return; + + setOpen(true); + setActiveIndex((i) => { + const next = Math.min(i, options.length - 1) + delta; + + return (next + options.length) % options.length; + }); + } + function handleKeyDown(e: React.KeyboardEvent) { - if ((e.key === "Enter" || e.key === ",") && input.trim()) { - e.preventDefault(); - addTag(input); - setOpen(false); - } else if (e.key === "Backspace" && !input && value.length > 0) { - removeTag(value[value.length - 1]); - } else if (e.key === "Escape") { - setOpen(false); + switch (e.key) { + case "ArrowDown": + e.preventDefault(); + move(1); + return; + + case "ArrowUp": + e.preventDefault(); + move(-1); + return; + + case "Tab": + if (e.shiftKey || !input.trim() || !active) return; + + e.preventDefault(); + addTag(active.value); + return; + + case "Enter": + if (!input.trim() && !active) return; + + e.preventDefault(); + addTag(active ? active.value : input); + return; + + case ",": + if (!input.trim()) return; + + e.preventDefault(); + addTag(input); + return; + + case "Backspace": + if (!input && value.length > 0) removeTag(value[value.length - 1]); + return; + + case "Escape": + setOpen(false); + return; } } return ( - 0 || !!canAddInput)} onOpenChange={setOpen}> +
    { inputRef.current?.focus(); setOpen(true); }} + onClick={() => { + inputRef.current?.focus(); + setOpen(true); + }} > {value.map((tag) => ( - + {tag} ))} + { setInput(e.target.value); setOpen(true); }} + onChange={(e) => { + setInput(e.target.value); + setActiveIndex(0); + setOpen(true); + }} onKeyDown={handleKeyDown} onFocus={() => setOpen(true)} + role="combobox" + aria-expanded={showList} + aria-controls={listId} + aria-autocomplete="list" + aria-activedescendant={ + active ? `${listId}-${active.value}` : undefined + } + aria-label="Add a tag" placeholder={value.length === 0 ? placeholder : ""} - className="flex-1 min-w-[120px] bg-transparent outline-none placeholder:text-muted-foreground" + className="min-w-[120px] flex-1 bg-transparent outline-none placeholder:text-muted-foreground" />
    + e.preventDefault()} onInteractOutside={() => setOpen(false)} > - - - No matching tags. - - {canAddInput && ( - { addTag(input); setOpen(false); }} - > - Add "{input.trim()}" - +
      + {options.map((option, index) => ( +
    • setActiveIndex(index)} + onMouseDown={(e) => { + e.preventDefault(); + addTag(option.value); + }} + className={cn( + "cursor-pointer rounded-sm px-2 py-1.5 text-sm", + index === activeIndex && "bg-accent text-accent-foreground", + option.isNew && "text-muted-foreground", )} - {filtered.slice(0, 8).map((tag) => ( - { addTag(tag); setOpen(false); }} - > - {tag} - - ))} - - - + > + {option.label} +
      • + ))} +
    ); diff --git a/ui/src/components/retro/task.tsx b/ui/src/components/retro/task.tsx index f56c1d0..0841daf 100644 --- a/ui/src/components/retro/task.tsx +++ b/ui/src/components/retro/task.tsx @@ -16,6 +16,8 @@ import { DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; import { DialogTrigger } from "@/components/ui/dialog"; +import { cardVariants, spring } from "@/lib/motion"; +import { m } from "motion/react"; import { twMerge } from "tailwind-merge"; export function Task({ @@ -32,10 +34,16 @@ export function Task({ const isOverdue = !task.completed && new Date(task.when) < today; return ( -
    @@ -65,7 +73,7 @@ export function Task({ > {task.what}

    -
    + ); } diff --git a/ui/src/components/retro/template-picker.tsx b/ui/src/components/retro/template-picker.tsx new file mode 100644 index 0000000..8f5ca5b --- /dev/null +++ b/ui/src/components/retro/template-picker.tsx @@ -0,0 +1,67 @@ +import { Button } from "@/components/ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; +import useTemplates from "@/hooks/use-templates"; +import { accentForIndex } from "@/lib/column-accent"; +import { BookDashed, ChevronDown } from "lucide-react"; + +interface TemplateColumn { + title: string; + description: string; +} + +export default function TemplatePicker({ + onApply, +}: { + onApply: (columns: TemplateColumn[]) => void; +}) { + const templates = useTemplates(); + + return ( + + + + + + + + Tried and tested formats + + + {templates.map((template) => ( + onApply(template.columns)} + className="flex-col items-start gap-1.5 py-2" + > + {template.title} + + + {template.columns.map((column, i) => ( + + {column.title} + + ))} + + + ))} + + + ); +} diff --git a/ui/src/components/retro/vote.tsx b/ui/src/components/retro/vote.tsx index c9a7f9d..91eaec3 100644 --- a/ui/src/components/retro/vote.tsx +++ b/ui/src/components/retro/vote.tsx @@ -1,7 +1,10 @@ +import { useColumnActions } from "@/hooks/use-columns"; import { useNotes } from "@/hooks/use-notes"; import useRetro from "@/hooks/use-retro"; import { api } from "@/lib/api"; +import { AnimatePresence } from "motion/react"; import { useEffect, useState } from "react"; +import { EmptyColumn, NoteSkeletons } from "./column-states"; import { Column, Columns } from "./columns"; import { Note } from "./note"; import { VotableNoteGroup } from "./note-group"; @@ -16,7 +19,8 @@ export default function Vote({ setVotesRemaining: (votesRemaining: number) => void; }) { const { retro } = useRetro(); - const { groupedNotes } = useNotes(); + const { notes, groupedNotes, loaded } = useNotes(); + const columnActions = useColumnActions(notes); const [votes, setVotes] = useState([]); @@ -25,7 +29,7 @@ export default function Vote({ setVotes(res.data); setVotesRemaining(retro.max_votes - res.data.length); }); - }, [retro.id, retro.max_votes]); + }, [retro.id, retro.max_votes, setVotesRemaining]); function handleVote(groupId: string, value: boolean) { api @@ -39,25 +43,43 @@ export default function Vote({ } return ( - - {retro.columns.map((column) => ( - - {Object.entries(groupedNotes[column.id] ?? []).map( - ([groupId, groupNotes]) => ( - handleVote(groupId, value)} - voted={!!votes.find((vote) => vote.group_id === groupId)} - canVote={retro.max_votes > votes.length} - key={groupId} - > - {groupNotes.map((note) => ( - - ))} - - ), - )} - - ))} + + {retro.columns.map((column, index) => { + const groups = Object.entries(groupedNotes[column.id] ?? {}); + + return ( + + {!loaded && } + + {loaded && groups.length === 0 && ( + Nothing to vote on here. + )} + + + {groups.map(([groupId, groupNotes]) => ( + handleVote(groupId, value)} + voted={!!votes.find((vote) => vote.group_id === groupId)} + canVote={retro.max_votes > votes.length} + key={groupId} + > + {groupNotes.map((note) => ( + + ))} + + ))} + + + ); + })} ); } diff --git a/ui/src/components/ui/auto-textarea.tsx b/ui/src/components/ui/auto-textarea.tsx new file mode 100644 index 0000000..b474a87 --- /dev/null +++ b/ui/src/components/ui/auto-textarea.tsx @@ -0,0 +1,60 @@ +import { cn } from "@/lib/utils"; +import { useCallback, useLayoutEffect, useRef } from "react"; +import { textareaClassName } from "./textarea-class"; + +export function AutoTextarea({ + className, + value, + onChange, + maxHeight = 180, + // Pulled out of props: callers spread a react-hook-form field here, whose + // own ref would otherwise land after ours and win. + ref: forwardedRef, + ...props +}: React.ComponentProps<"textarea"> & { maxHeight?: number }) { + const ref = useRef(null); + + const attachRef = useCallback( + (node: HTMLTextAreaElement | null) => { + ref.current = node; + + if (typeof forwardedRef === "function") { + forwardedRef(node); + } else if (forwardedRef) { + forwardedRef.current = node; + } + }, + [forwardedRef], + ); + + const resize = useCallback(() => { + const el = ref.current; + if (!el) return; + + // Collapse first, or scrollHeight only ever reports the current height. + el.style.height = "auto"; + + const next = Math.min(el.scrollHeight, maxHeight); + + el.style.height = `${next}px`; + el.style.overflowY = el.scrollHeight > maxHeight ? "auto" : "hidden"; + }, [maxHeight]); + + useLayoutEffect(resize, [value, resize]); + + return ( +