From cab17e02305278bef5e40c73bc610ca9f4ba9256 Mon Sep 17 00:00:00 2001 From: Scrubs McBarkley Date: Thu, 16 Apr 2026 10:39:40 +0000 Subject: [PATCH 01/12] docs: add CONTRIBUTING.md with branch strategy Document the three-branch GitOps model (dev/uat/main), developer workflow, promotion flow, and branch protection rules. Refs GRO-702 Co-Authored-By: Paperclip --- CONTRIBUTING.md | 84 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..8dd1af6 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,84 @@ +# Contributing to GroomBook + +## Branch Strategy + +GroomBook uses a three-branch GitOps model: + +| Branch | Environment | Purpose | +|--------|-------------|---------| +| `dev` | Development | Active development target — all feature/fix PRs target this branch | +| `uat` | UAT / Staging | Promoted from `dev` by the CTO for acceptance testing | +| `main` | Production | Promoted from `uat` by the CEO; triggers production deployment | + +**Never open a PR directly to `uat` or `main`.** All work flows through `dev` first. + +## Developer Workflow + +1. **Branch from `dev`** — create a feature or fix branch: + ```bash + git checkout dev + git pull origin dev + git checkout -b feat/my-feature + ``` + +2. **Open a PR targeting `dev`** — include the issue identifier in the title and cc @cpfarhood: + ```bash + gh pr create --base dev --title "feat: description (GRO-NNN)" \ + --body "Closes GRO-NNN\n\ncc @cpfarhood" + ``` + +3. **Pipeline gates before merge to `dev`:** + - QA (Lint Roller) reviews first — code quality, test coverage, CI pass + - CTO (The Dogfather) reviews second — architecture and final approval + - Both must approve; 2 approving reviews required by branch protection + +## Promotion Flow + +### Dev → UAT + +After merging to `dev`, the CTO opens a PR from `dev` → `uat`: + +```bash +gh pr create --base uat --head dev \ + --title "chore: promote dev to uat (YYYY.MM.DD)" \ + --body "Promoting dev to UAT for regression and security review.\n\ncc @cpfarhood" +``` + +Gates: +- Shedward Scissorhands runs regression/acceptance tests +- Barkley Trimsworth performs security review +- CTO approves and merges (1 approving review required) + +### UAT → Main (Production) + +After UAT passes, the CTO assigns the promotion PR to the CEO: + +Gates: +- CEO (Scrubs McBarkley) reviews for business alignment and merges +- 1 approving review required; triggers auto-deploy to Production + +## Branch Protection Summary + +| Branch | Required Approvals | Who approves | +|--------|--------------------|-------------| +| `dev` | 2 | QA (Lint Roller) + CTO (The Dogfather) | +| `uat` | 1 | CTO (The Dogfather) | +| `main` | 1 | CEO (Scrubs McBarkley) | + +Force-pushes and branch deletions are disabled on all three branches. + +## Commit Style + +Use [Conventional Commits](https://www.conventionalcommits.org/): +- `feat:` — new feature +- `fix:` — bug fix +- `chore:` — maintenance (dependency updates, build config, promotions) +- `docs:` — documentation only +- `ci:` — CI/CD changes +- `refactor:` — code restructure without behaviour change + +Reference the Paperclip issue in the commit body: `Refs GRO-NNN`. + +## Questions? + +Open a Paperclip issue in the GRO project or ask in the team channel. From 4a65c30d40e57fdeedec6f1cd6e73d763379709e Mon Sep 17 00:00:00 2001 From: Scrubs McBarkley Date: Thu, 16 Apr 2026 10:43:12 +0000 Subject: [PATCH 02/12] =?UTF-8?q?docs:=20fix=20bash=20snippet=20quoting=20?= =?UTF-8?q?and=20add=20uat=E2=86=92main=20pr=20command?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix \n quoting in two gh pr create commands: use ANSI-C $'...' quoting so newlines render correctly in PR bodies (not literal \n) - Add missing gh pr create example for the UAT → main promotion step Addresses Greptile review feedback on PR #304. Co-Authored-By: Paperclip --- CONTRIBUTING.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8dd1af6..38582cf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,7 +24,7 @@ GroomBook uses a three-branch GitOps model: 2. **Open a PR targeting `dev`** — include the issue identifier in the title and cc @cpfarhood: ```bash gh pr create --base dev --title "feat: description (GRO-NNN)" \ - --body "Closes GRO-NNN\n\ncc @cpfarhood" + --body $'Closes GRO-NNN\n\ncc @cpfarhood' ``` 3. **Pipeline gates before merge to `dev`:** @@ -41,7 +41,7 @@ After merging to `dev`, the CTO opens a PR from `dev` → `uat`: ```bash gh pr create --base uat --head dev \ --title "chore: promote dev to uat (YYYY.MM.DD)" \ - --body "Promoting dev to UAT for regression and security review.\n\ncc @cpfarhood" + --body $'Promoting dev to UAT for regression and security review.\n\ncc @cpfarhood' ``` Gates: @@ -51,7 +51,13 @@ Gates: ### UAT → Main (Production) -After UAT passes, the CTO assigns the promotion PR to the CEO: +After UAT passes, the CTO opens a PR from `uat` → `main` and assigns it to the CEO: + +```bash +gh pr create --base main --head uat \ + --title "chore: promote uat to main (YYYY.MM.DD)" \ + --body $'Promoting UAT to production.\n\ncc @cpfarhood' +``` Gates: - CEO (Scrubs McBarkley) reviews for business alignment and merges From 85c76b5209c7e9d801fe9818f72d8e40f2880136 Mon Sep 17 00:00:00 2001 From: "groombook-engineer[bot]" <269742240+groombook-engineer[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2026 18:58:03 +0000 Subject: [PATCH 03/12] fix(GRO-724): rename dev hostname from groombook.dev.farh.net to dev.groombook.dev (#308) Updates playwright baseURL to the canonical dev.groombook.dev FQDN per canonical infra targets. Co-authored-by: Flea Flicker Co-authored-by: Paperclip --- apps/web/e2e/playwright.config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/e2e/playwright.config.ts b/apps/web/e2e/playwright.config.ts index fad7857..ddc725b 100644 --- a/apps/web/e2e/playwright.config.ts +++ b/apps/web/e2e/playwright.config.ts @@ -3,7 +3,7 @@ import { defineConfig, devices } from "@playwright/test"; /** * Playwright configuration for GroomBook Web E2E tests. * - * Targets the deployed dev environment at groombook.dev.farh.net. + * Targets the deployed dev environment at dev.groombook.dev. * Uses the dev login selector (/login) for authentication — no hardcoded credentials. * * Run locally: @@ -19,7 +19,7 @@ export default defineConfig({ reporter: process.env.CI ? "github" : "list", use: { - baseURL: "https://groombook.dev.farh.net", + baseURL: "https://dev.groombook.dev", trace: "on-first-retry", screenshot: "only-on-failure", serviceWorkers: "block", From c7865443697c858c66d0f8459d7559f8c4a5c6de Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 14 Apr 2026 15:17:01 +0000 Subject: [PATCH 04/12] Fix frontend error handling and code quality (GRO-642) HIGH Priority: 1. SetupWizard.jsx -> SetupWizard.tsx: renamed to .tsx with proper TypeScript types 2. deleteAppt missing error handling: added try/catch, response.ok check, alert on failure 3. GlobalSearch missing error state: added error state with user-visible error message MEDIUM Priority: 4. CustomerPortal unsafe type cast: fixed 'as any' to proper PortalAppointment type 5. Logo upload XSS risk: sanitized MIME types to png/jpeg/gif/webp only, removed SVG 6. Reports error handling: added ok checks before json() parsing to guard against invalid JSON on error responses LOW Priority: 8. Modal accessibility: added role='dialog', aria-modal='true', focus trap, Escape key handler, restore focus on close 9. PetPhotoUpload file size: added 50MB max file size check before resize 10. Types package: added photoKey and photoUploadedAt to Pet interface Co-Authored-By: Paperclip --- apps/web/src/App.tsx | 2 +- apps/web/src/components/GlobalSearch.tsx | 16 +++- apps/web/src/components/PetPhotoUpload.tsx | 6 ++ apps/web/src/pages/Appointments.tsx | 54 ++++++++++- apps/web/src/pages/Reports.tsx | 10 +-- apps/web/src/pages/Settings.tsx | 6 +- apps/web/src/pages/SetupWizard.d.ts | 2 +- .../{SetupWizard.jsx => SetupWizard.tsx} | 89 +++++++++---------- apps/web/src/portal/CustomerPortal.tsx | 7 +- apps/web/src/portal/sections/Appointments.tsx | 2 +- packages/types/src/index.ts | 2 + 11 files changed, 131 insertions(+), 65 deletions(-) rename apps/web/src/pages/{SetupWizard.jsx => SetupWizard.tsx} (89%) diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 28bde7c..83e95d6 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -12,7 +12,7 @@ import { SettingsPage } from "./pages/Settings.js"; import { BookingConfirmedPage } from "./pages/BookingConfirmed.js"; import { BookingCancelledPage } from "./pages/BookingCancelled.js"; import { BookingErrorPage } from "./pages/BookingError.js"; -import { SetupWizard } from "./pages/SetupWizard.jsx"; +import { SetupWizard } from "./pages/SetupWizard.tsx"; import { CustomerPortal } from "./portal/CustomerPortal.js"; import { DevLoginSelector, getDevUser } from "./pages/DevLoginSelector.js"; import { DevSessionIndicator } from "./components/DevSessionIndicator.js"; diff --git a/apps/web/src/components/GlobalSearch.tsx b/apps/web/src/components/GlobalSearch.tsx index 8971fde..deb4770 100644 --- a/apps/web/src/components/GlobalSearch.tsx +++ b/apps/web/src/components/GlobalSearch.tsx @@ -26,6 +26,7 @@ export function GlobalSearch() { const [query, setQuery] = useState(""); const [results, setResults] = useState(null); const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); const [open, setOpen] = useState(false); const inputRef = useRef(null); const dropdownRef = useRef(null); @@ -45,15 +46,18 @@ export function GlobalSearch() { debounceRef.current = setTimeout(async () => { setLoading(true); + setError(null); try { const res = await fetch(`/api/search?q=${encodeURIComponent(trimmed)}`); if (res.ok) { const data: SearchResults = await res.json(); setResults(data); setOpen(true); + } else { + setError("Search failed. Please try again."); } - } catch (err) { - console.warn("GlobalSearch: fetch error", err); + } catch { + setError("Search failed. Please try again."); } finally { setLoading(false); } @@ -160,7 +164,13 @@ export function GlobalSearch() { )} - {!loading && !hasResults && ( + {!loading && error && ( +
+ {error} +
+ )} + + {!loading && !error && !hasResults && (
No results found
diff --git a/apps/web/src/components/PetPhotoUpload.tsx b/apps/web/src/components/PetPhotoUpload.tsx index f33ce48..0b479f3 100644 --- a/apps/web/src/components/PetPhotoUpload.tsx +++ b/apps/web/src/components/PetPhotoUpload.tsx @@ -71,6 +71,12 @@ export function PetPhotoUpload({ petId, onUploaded }: Props) { } async function handleFile(file: File) { + const MAX_FILE_SIZE = 50 * 1024 * 1024; + if (file.size > MAX_FILE_SIZE) { + setState({ status: "error", message: "File exceeds 50MB limit. Please choose a smaller image." }); + return; + } + if (!ACCEPTED_TYPES.includes(file.type)) { setState({ status: "error", message: "Please select a JPEG, PNG, WebP, or GIF image." }); return; diff --git a/apps/web/src/pages/Appointments.tsx b/apps/web/src/pages/Appointments.tsx index 386354d..1dd7046 100644 --- a/apps/web/src/pages/Appointments.tsx +++ b/apps/web/src/pages/Appointments.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState, useCallback } from "react"; +import { useEffect, useState, useCallback, useRef } from "react"; import type { Appointment, Client, Pet, Service, Staff } from "@groombook/types"; // ─── Helpers ──────────────────────────────────────────────────────────────── @@ -273,7 +273,15 @@ export function AppointmentsPage() { cascade !== "this_only" ? `/api/appointments/${id}?cascade=${cascade}` : `/api/appointments/${id}`; - await fetch(url, { method: "DELETE" }); + try { + const res = await fetch(url, { method: "DELETE" }); + if (!res.ok) { + const err = (await res.json()) as { error?: string }; + throw new Error(err.error ?? `HTTP ${res.status}`); + } + } catch (e: unknown) { + alert(e instanceof Error ? e.message : "Failed to delete appointment"); + } setSelectedAppt(null); await loadAppointments(); } @@ -819,8 +827,49 @@ function AppointmentDetail({ } function Modal({ children, onClose }: { children: React.ReactNode; onClose: () => void }) { + const modalRef = useRef(null); + + useEffect(() => { + const previouslyFocused = document.activeElement as HTMLElement; + const focusableSelectors = 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'; + const focusableElements = modalRef.current?.querySelectorAll(focusableSelectors); + const firstFocusable = focusableElements?.[0]; + firstFocusable?.focus(); + + function handleKeyDown(e: KeyboardEvent) { + if (e.key === "Escape") { + onClose(); + return; + } + if (e.key !== "Tab") return; + if (!modalRef.current) return; + const focusables = modalRef.current.querySelectorAll(focusableSelectors); + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (e.shiftKey) { + if (document.activeElement === first) { + e.preventDefault(); + last?.focus(); + } + } else { + if (document.activeElement === last) { + e.preventDefault(); + first?.focus(); + } + } + } + + document.addEventListener("keydown", handleKeyDown); + return () => { + document.removeEventListener("keydown", handleKeyDown); + previouslyFocused?.focus(); + }; + }, [onClose]); + return (
{ if (e.target === e.currentTarget) onClose(); }} >
, - revRes.json() as Promise<{ byPeriod: RevenuePeriod[]; byGroomer: RevenueByGroomer[] }>, - apptRes.json() as Promise<{ byPeriod: ApptPeriod[] }>, - svcRes.json() as Promise<{ rows: ServiceRow[] }>, - clientRes.json() as Promise, + summRes.ok ? summRes.json() as Promise : summRes.text().then(() => { throw new Error("summary response not ok"); }), + revRes.ok ? revRes.json() as Promise<{ byPeriod: RevenuePeriod[]; byGroomer: RevenueByGroomer[] }> : revRes.text().then(() => { throw new Error("revenue response not ok"); }), + apptRes.ok ? apptRes.json() as Promise<{ byPeriod: ApptPeriod[] }> : apptRes.text().then(() => { throw new Error("appointments response not ok"); }), + svcRes.ok ? svcRes.json() as Promise<{ rows: ServiceRow[] }> : svcRes.text().then(() => { throw new Error("services response not ok"); }), + clientRes.ok ? clientRes.json() as Promise : clientRes.text().then(() => { throw new Error("clients response not ok"); }), ]); setSummary(summData); diff --git a/apps/web/src/pages/Settings.tsx b/apps/web/src/pages/Settings.tsx index 088a685..5ccb943 100644 --- a/apps/web/src/pages/Settings.tsx +++ b/apps/web/src/pages/Settings.tsx @@ -149,9 +149,9 @@ export function SettingsPage() { return; } - const validTypes = ["image/png", "image/svg+xml", "image/jpeg", "image/webp"]; + const validTypes = ["image/png", "image/jpeg", "image/gif", "image/webp"]; if (!validTypes.includes(file.type)) { - setMessage({ type: "error", text: "Logo must be PNG, SVG, JPEG, or WebP." }); + setMessage({ type: "error", text: "Logo must be PNG, JPEG, GIF, or WebP." }); return; } @@ -393,7 +393,7 @@ issuerUrl: authForm.issuerUrl, diff --git a/apps/web/src/pages/SetupWizard.d.ts b/apps/web/src/pages/SetupWizard.d.ts index 5758e2b..786c80d 100644 --- a/apps/web/src/pages/SetupWizard.d.ts +++ b/apps/web/src/pages/SetupWizard.d.ts @@ -1 +1 @@ -export { SetupWizard } from "./SetupWizard.jsx"; +export { SetupWizard } from "./SetupWizard.tsx"; diff --git a/apps/web/src/pages/SetupWizard.jsx b/apps/web/src/pages/SetupWizard.tsx similarity index 89% rename from apps/web/src/pages/SetupWizard.jsx rename to apps/web/src/pages/SetupWizard.tsx index 666b67c..8587519 100644 --- a/apps/web/src/pages/SetupWizard.jsx +++ b/apps/web/src/pages/SetupWizard.tsx @@ -2,16 +2,39 @@ import { useState, useEffect } from "react"; import { useNavigate } from "react-router-dom"; import { useBranding } from "../BrandingContext.js"; -export function SetupWizard({ onSetupComplete }) { +interface SetupStatus { + showAuthProviderStep?: boolean; +} + +interface TestResult { + ok: boolean; + error?: string; +} + +interface AuthFormState { + providerId: string; + displayName: string; + issuerUrl: string; + internalBaseUrl: string; + clientId: string; + clientSecret: string; + scopes: string; +} + +interface Step { + id: string; + title: string; + description: string; +} + +export function SetupWizard({ onSetupComplete }: { onSetupComplete?: () => void }) { const navigate = useNavigate(); const { refresh: refreshBranding } = useBranding(); - // Fetch setup status to determine if auth provider step is needed - const [setupStatus, setSetupStatus] = useState(null); // null = loading + const [setupStatus, setSetupStatus] = useState(null); const [loadingStatus, setLoadingStatus] = useState(true); - // Auth provider form state - const [authForm, setAuthForm] = useState({ + const [authForm, setAuthForm] = useState({ providerId: "authentik", displayName: "", issuerUrl: "", @@ -21,16 +44,16 @@ export function SetupWizard({ onSetupComplete }) { scopes: "openid profile email", }); const [testingConnection, setTestingConnection] = useState(false); - const [testResult, setTestResult] = useState(null); // {ok: boolean, error?: string} + const [testResult, setTestResult] = useState(null); const [step, setStep] = useState(0); const [businessName, setBusinessName] = useState(""); const [loading, setLoading] = useState(false); - const [error, setError] = useState(null); + const [error, setError] = useState(null); useEffect(() => { fetch("/api/setup/status") - .then((r) => r.json()) + .then((r) => r.json() as Promise) .then((data) => { setSetupStatus(data); setLoadingStatus(false); @@ -40,8 +63,7 @@ export function SetupWizard({ onSetupComplete }) { }); }, []); - // Build steps dynamically based on setup status - const STEPS = setupStatus?.showAuthProviderStep + const STEPS: Step[] = setupStatus?.showAuthProviderStep ? [ { id: "welcome", title: "Welcome", description: "Welcome to GroomBook! Let's get your business set up." }, { id: "auth", title: "Auth Provider", description: "Configure your authentication provider to secure your GroomBook instance." }, @@ -63,9 +85,8 @@ export function SetupWizard({ onSetupComplete }) { const isFirst = step === 0; const canGoBack = step > 0 && step < STEPS.length - 1; - // Determine if we can proceed - depends on which step we're on const canGoNext = (() => { - if (step === STEPS.length - 1) return true; // done step + if (step === STEPS.length - 1) return true; if (current?.id === "business") return businessName.trim().length > 0; if (current?.id === "auth") { return ( @@ -94,9 +115,9 @@ export function SetupWizard({ onSetupComplete }) { scopes: authForm.scopes, }), }); - const data = await res.json(); + const data = (await res.json()) as TestResult; setTestResult(data); - } catch (e) { + } catch { setTestResult({ ok: false, error: "Network error. Please try again." }); } finally { setTestingConnection(false); @@ -105,12 +126,10 @@ export function SetupWizard({ onSetupComplete }) { const handleNext = async () => { if (step === STEPS.length - 1) { - // Done - redirect to admin navigate("/admin"); return; } - // Submit auth provider config if (current?.id === "auth") { setLoading(true); setError(null); @@ -129,12 +148,12 @@ export function SetupWizard({ onSetupComplete }) { }), }); if (!res.ok) { - const data = await res.json(); + const data = (await res.json()) as { error?: string }; setError(data.error || "Failed to save auth provider configuration. Please try again."); setLoading(false); return; } - } catch (e) { + } catch { setError("Network error. Please try again."); setLoading(false); return; @@ -142,7 +161,6 @@ export function SetupWizard({ onSetupComplete }) { setLoading(false); } - // Submit business name and complete setup if (current?.id === "business" && businessName.trim()) { setLoading(true); setError(null); @@ -153,16 +171,14 @@ export function SetupWizard({ onSetupComplete }) { body: JSON.stringify({ businessName: businessName.trim() }), }); if (!res.ok) { - const data = await res.json(); + const data = (await res.json()) as { error?: string }; setError(data.error || "Setup failed. Please try again."); setLoading(false); return; } - // Refresh branding so the nav bar shows the new business name refreshBranding(); - // Clear needsSetup state in App so the redirect to /admin sticks if (onSetupComplete) onSetupComplete(); - } catch (e) { + } catch { setError("Network error. Please try again."); setLoading(false); return; @@ -192,7 +208,7 @@ export function SetupWizard({ onSetupComplete }) { ); } - const inputStyle = { + const inputStyle: React.CSSProperties = { width: "100%", padding: "0.6rem 0.85rem", borderRadius: 8, @@ -220,7 +236,6 @@ export function SetupWizard({ onSetupComplete }) { maxWidth: 480, width: "100%", }}> - {/* Progress dots */}
{STEPS.map((_, i) => (
- {/* Step indicator */}

Step {step + 1} of {STEPS.length}

- {/* Title */}

{current?.title}

- {/* Description */}

{current?.description}

- {/* Step: Business name input */} {current?.id === "business" && ( setBusinessName(e.target.value)} - onKeyDown={(e) => e.key === "Enter" && canGoNext && handleNext()} + onKeyDown={(e) => e.key === "Enter" && canGoNext && void handleNext()} autoFocus style={inputStyle} /> )} - {/* Step: Auth provider config form */} {current?.id === "auth" && (
- {/* Provider ID */}
- {/* Display Name */}
- {/* Issuer URL */}
- {/* Internal Base URL (optional) */}
- {/* Client ID */}
- {/* Client Secret */}
- {/* Scopes */}
- {/* Test Connection button */} - {/* Test result */} {testResult && (
)} - {/* Step: Super user info */} {current?.id === "superuser" && (
)} - {/* Step: Second admin info */} {current?.id === "admin" && (
)} - {/* Error message */} {error && (

)} - {/* Navigation buttons */}

)}