diff --git a/.clinerules/project-standards.md b/.clinerules/project-standards.md deleted file mode 100644 index c003a5116..000000000 --- a/.clinerules/project-standards.md +++ /dev/null @@ -1,15 +0,0 @@ -# Project Standards - -Follow the coding standards defined in `.github/instructions/`: - -- General standards: See `.github/instructions/general.instructions.md` -- Astro components: See `.github/instructions/astro.instructions.md` -- JavaScript/TypeScript: See `.github/instructions/javascript.instructions.md` -- Markdown: See `.github/instructions/markdown.instructions.md` - -Key rules: - -- Use TypeScript exclusively -- Never use manual HTML fixtures -- No apologizing, be direct/concise -- Wait for permission before implementing suggestions diff --git a/.github/instructions/astro.instructions.md b/.github/instructions/astro.instructions.md deleted file mode 100644 index fc6ab723f..000000000 --- a/.github/instructions/astro.instructions.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -applyTo: "docs/**/*.astro" ---- - -# Astro template standards - -- Always use TypeScript for type safety. -- Always put script in a separate file and import it. - -## Testing Astro Components - -- When testing Astro components, use the Astro Container API instead of manual HTML fixtures. -- This ensures tests stay in sync with component changes automatically. -- Reference: Astro Container API Documentation at docs.astro.build/en/reference/container-reference/ -- There is a model component and test demonstrating this pattern in the src/components/Test directory named webComponent. diff --git a/.github/instructions/css.instructions.md b/.github/instructions/css.instructions.md deleted file mode 100644 index 6ea09ecfb..000000000 --- a/.github/instructions/css.instructions.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -applyTo: "**/*.css" ---- - -# CSS rules (specificity budget) - -- Prefer a single class selector for styling (`.componentPart`). -- Avoid ID selectors (`#something`) in CSS; use classes/attributes instead. -- Avoid chaining state across unrelated roots (e.g., `#header .child.is-open ...`). Put state on the owning component root. -- Avoid long selector chains; if you need more specificity, add a single component class rather than stacking selectors. -- Avoid `!important` except in vendor CSS and print rules. - -- Do not use Tailwind `dark:` variant classes for dark-mode theming. This project uses a custom theme system and CSS variables; use the project's theme tokens and helper classes instead of Tailwind `dark:` utilities. - -- Do not use Tailwind `dark:` variant classes for dark-mode theming. This project uses a custom theme system and CSS variables; use the project's theme tokens and helper classes instead of Tailwind `dark:` utilities. - -# Z-index tokens - -- Do not hard-code numeric `z-index` values. -- Use the z-index tokens defined in `src/styles/index.css` (e.g., `z-index: var(--z-modal)`). -- If no existing token fits, ask the user what z-layer to use before adding a new token. diff --git a/.github/instructions/docs.instructions.md b/.github/instructions/docs.instructions.md deleted file mode 100644 index 0343b561e..000000000 --- a/.github/instructions/docs.instructions.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -applyTo: "docs/**/*.md" ---- - -# Documentation standards - -- Use clear and concise language. -- Ensure headings follow a consistent hierarchy. -- Use sentence case for all headings and titles. -- Use backticks for inline code snippets. -- Use fenced code blocks with language identifiers for code samples. -- Use bullet points or numbered lists for steps and features. -- Include links to relevant resources or documentation. -- Ensure proper spacing and indentation for readability. -- End files with a single newline character. -- Follow the [Markdown linting instructions](./markdown.instructions.md) for formatting consistency. diff --git a/.github/instructions/general.instructions.md b/.github/instructions/general.instructions.md index 4f63ab973..1c1d93bba 100644 --- a/.github/instructions/general.instructions.md +++ b/.github/instructions/general.instructions.md @@ -40,85 +40,33 @@ applyTo: "**" # Code Organization and Directory Structure ## src/lib Directory Restrictions + - **The src/lib directory is for server-side build code ONLY** - NO client-side code can go in src/lib (it gets bundled into server-side builds) - Client-side utilities should go in src/components/scripts/ or appropriate component directories ## API Code Organization + - **API endpoints** go in `src/pages/api/` - **Code files related to API endpoints** go in `src/pages/api/` and are prefixed with `_` (e.g., `_utils/`, `_contracts/`) - **API utility files** go specifically in the `_utils/` folder - **API contract/type files** go in `_contracts/` folder for centralized type definitions ## API Endpoints (Permission Required) + - **Do not create, recreate, or restore `src/pages/api/*` endpoints without explicit user permission.** - Prefer Astro Actions (`/_actions/...`) for new backend behavior unless instructed otherwise. ## Mixed Concern Files + - Files that straddle server-side API and client-side concerns (like API client wrappers) require clarification - **Ask before placing such files** - they may need special handling or alternative organization - Example: gdpr.client.ts (API client wrapper) - unclear placement due to mixed server/client concerns -# Astro View Transitions Navigation - -Components may have behavior dependent on Astro View Transitions navigation events. Choose the appropriate navigation method: - -- **Fresh page load**: Use `page.goto(url)` for full browser navigation (no View Transitions, triggers full page lifecycle) -- **Client-side navigation**: Use `navigateToPage('/path')` for in-site navigation with View Transitions (triggers `astro:page-load` and other View Transition events) - -Always use the `navigateToPage()` method for client-side navigation - never ad-hoc `click('a[href]')` calls. This maintains centralized control. - -When a Playwright-native action (e.g., `page.click()`, `page.fill()`, `page.hover()`) is required, expose it through the shared `BasePage` helpers (e.g., `BasePage.click()`), then call that helper from tests instead of the raw Playwright API. This keeps all browser interactions centrally managed and makes future behavior changes (timeouts, logging, etc.) easier. - # Personality -# Testing Standards - -## Astro Component Testing - Container API (MANDATORY) - -- **NEVER use manual HTML strings in test files or fixtures.** They get out of sync with templates and are worse than no test at all. -- **ALWAYS use Astro's Container API** to create fixtures from actual .astro templates. See: https://docs.astro.build/en/reference/container-reference/ -- **Test fixtures MUST import actual components**, not duplicate HTML. Example: - ```astro - --- - import MyComponent from '@components/MyComponent/index.astro' - const { testProp } = Astro.props - --- - - ``` -- **Hard-coded HTML fixtures are FORBIDDEN.** If you find yourself writing HTML in a fixture, STOP and use the actual component instead. -- Reference the working example in src/components/Test/container.astro and its test file. -- Use experimental_AstroContainer.create() to instantiate the container. -- Use container.renderToString(Component) to get rendered HTML from actual Astro components. -- Configure Vitest with getViteConfig() from 'astro/config' to support Astro Container API. -- Test files should follow a client.spec.ts naming pattern or similar. -- Fixture files should follow a componentName.fixture.astro naming pattern (e.g., newsletter.fixture.astro). -- A working example test using the Container API is available at /home/kevin/Repos/Webstack Builders/Corporate Website/astro.webstackbuilders.com/src/components/Test/container.spec.ts - -## E2E Testing Standards - -- **NEVER hard-code content slugs in e2e tests** (e.g., `/articles/typescript-best-practices`, `/services/web-development`). Content can be deleted or renamed. Always dynamically fetch the first available item from listing pages (articles, services, case-studies, etc.) and navigate to it. This prevents test breakage when content changes. -- **Playwright E2E Tests**: ALWAYS run with `CI=1` and `FORCE_COLOR=1` environment variables (e.g., `CI=1 FORCE_COLOR=1 npx playwright test`). This prevents the Playwright test runner from launching its own dev server. The user maintains a running dev server for development. -- **NEVER run the full e2e test suite** unless explicitly requested by the user. The full suite is very resource intensive and takes over 10 minutes to run. Only run specific e2e test files when verification is needed (e.g., `CI=1 FORCE_COLOR=1 npx playwright test test/e2e/specific-file.spec.ts`). -- **NEVER start a dev server yourself**. The user runs their own dev server for development. When you need a dev server running, notify the user instead of starting one. - -### Astro View Transitions Testing - -- **Navigation method matters**: Choose between `page.goto()` and Astro's client-side navigation based on what you're testing: - - Use `page.goto(url)` for testing **fresh page loads** (full browser navigation, no View Transitions) - - Use `BasePage.click()` on navigation links or `BasePage.navigateToPage()` for testing **View Transitions** (client-side navigation within the site) so that all clicks flow through the centralized helpers -- **Wait for page load properly**: Use BasePage's `waitForPageLoad()` method to wait for `astro:page-load` event instead of arbitrary timeouts -- **NEVER use `page.waitForTimeout()`** for waiting on View Transitions - it's unreliable and slows tests. Use event-based waits instead -- **transition:persist directive**: Must be applied directly to HTML elements (including custom elements), not on Astro component wrappers. Example: - ```astro - - - - - - ``` - # Personality + - Do not apologize - Do not flatter me - Do not use superlatives lke "absolutely" @@ -126,6 +74,7 @@ When a Playwright-native action (e.g., `page.click()`, `page.fill()`, `page.hove - Be direct # Response Guidelines + - When the user asks "What would you suggest", "what do you recommend", or similar language, provide multiple options with clear explanations but do NOT begin implementation - Always wait for explicit permission before implementing suggested changes - Present suggestions as numbered options with pros/cons when applicable diff --git a/.github/instructions/javascript.instructions.md b/.github/instructions/javascript.instructions.md deleted file mode 100644 index d219164cd..000000000 --- a/.github/instructions/javascript.instructions.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -applyTo: "**/*.{js,jsx,ts,tsx}" ---- - -# JavaScript and TypeScript standards - -- Use `const` and `let` instead of `var`. -- Prefer arrow functions for anonymous functions. -- Use JSDoc for all public functions and components. -- Do not add semicolons at the end of statements (semicolon-free style). - -# Playwright E2E timeout policy (test/e2e/**) - -- For Playwright E2E code under `test/e2e/**` (spec files and page object models), **do not** introduce new numeric timeouts (e.g. `{ timeout: 5000 }`, `waitForTimeout(600)`, `test.setTimeout(…)`). -- Always choose an existing timeout knob from `test/e2e/helpers/waitTimeouts.ts` (use `import { wait } ...` and `wait.*`). -- If none of the existing knobs fit the use case, **stop and ask** what to do (e.g. whether to add a new `wait.bespoke*` knob, refactor to an event-based wait, or change the underlying behavior). \ No newline at end of file diff --git a/.github/instructions/markdown.instructions.md b/.github/instructions/markdown.instructions.md deleted file mode 100644 index 8ead227e7..000000000 --- a/.github/instructions/markdown.instructions.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -applyTo: "**/*.{md,mdx}" ---- - -# Markdown Linting Instructions - -## General Guidelines - -- For all Markdown files (`.md`), ensure that generated content adheres to the `markdownlint` rules configured for this project. -- List items should be correctly indented (`MD007`). -- No Multiple consecutive blank lines (`MD012`) -- Always use a blank line after a heading (`MD022`). -- Always add a blank line before and after lists (`MD032`). -- Avoid using backslashes for line breaks; use two spaces instead (`MD030`). -- Ensure all inline code blocks are surrounded by backticks (`MD046`). -- Bullet lists should be consistent (e.g., use `*` or `-`, but not both) (`MD044`). -- Files should end with a single newline character (`MD047`). -- Bare URLs should be wrapped in angle brackets (`MD034`). For example, use `` instead of `https://example.com`. -- Headings should not have trailing punctuation (`MD026`). Remove colons, periods, or other punctuation from the end of headings. -- All fenced code blocks must have a language specified (`MD040`). Use appropriate languages like `js`, `typescript`, `yaml`, `json`, `text`, `bash`, etc. -- Fenced code blocks should be surrounded by blank lines (`MD031`). -- All fenced code blocks must have a language specified (`MD040`). Use appropriate languages like `js`, `typescript`, `yaml`, `json`, `text`, `bash`, etc. This rule is already mentioned above but bears repeating. -- Lists should be surrounded by blank lines (`MD032`). Always add a blank line before and after lists (both ordered and unordered). -- Ordered lists should use consistent numbering (`MD029`). Either use sequential numbering (1, 2, 3) or all ones (1, 1, 1). Do not mix styles (e.g., 1, 2, 3, 4 is correct; 1, 2, 3, 4 where one item is numbered 2 when it should be 3 is incorrect). -- No trailing spaces on lines (`MD009`), except when intentionally using two trailing spaces for a line break. - -## MDX And Inline Code Safety - -- When you mean the less-than operator (e.g. `a < b`), do not write a raw `<` in prose. - - Prefer `<` (and `>` when needed), or wrap the expression in backticks. -- Always wrap code-y snippets in backticks so the Markdown/MDX parser doesn’t treat them as HTML/JSX. - - Examples: `{ param, ... }`, `generic`, `Array`, `T extends Foo`. - -## Mermaid Diagram Authoring - -- This repo renders Mermaid during builds; invalid or unsupported Mermaid syntax will fail `npm run build`. -- Subgraphs: - - Do not use quoted subgraph titles like `subgraph "Title"`. - - Prefer a simple subgraph id with a simple label in brackets: `subgraph myGroup[My Group]`. - - Avoid punctuation-heavy labels in subgraph brackets (parentheses, `+`, `%`, em-dashes). Keep labels plain words. -- Labels: - - Avoid double quotes inside node labels like `A[When is a trace "complete"?]`. - - Avoid `@` in node labels (it can be parsed as a reserved token in some Mermaid flowchart grammars). -- Diagram types: - - Do not use `quadrantChart` (not supported by the current renderer); use `graph TD` (or another supported type) instead. - -## How to Apply These Rules - -- If you are generating a new Markdown file, follow these rules from the beginning. -- If you are asked to refactor or fix a Markdown file, correct any issues that violate these guidelines. For example, if a list is inconsistently formatted, make it consistent. -- If a heading is missing a blank line after it, add one. -- Always add blank lines before and after lists to ensure proper spacing. -- When creating fenced code blocks, always specify the appropriate language identifier. -- Ensure all files end with exactly one newline character. -- In code reviews, flag any violations of these rules as a suggestion. - -## Code Block Language Guidelines - -- Use `js` for JavaScript code -- Use `typescript` or `ts` for TypeScript code -- Use `yaml` for YAML configuration files -- Use `json` for JSON data -- Use `bash` or `shell` for terminal commands -- Use `text` for plain text output or file structures -- Use `astro` for Astro component code -- Use appropriate language identifiers for other code types diff --git a/.github/instructions/testing.instructions.md b/.github/instructions/testing.instructions.md deleted file mode 100644 index 8fe11a5bb..000000000 --- a/.github/instructions/testing.instructions.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -applyTo: "**/*.spec.ts" ---- - -# Testing Standards - -## Astro Container API (Unit Tests) - -- **NEVER use manual HTML strings** - use Astro's Container API with actual .astro templates -- **Test fixtures MUST import actual components**, not duplicate HTML -- Naming: `filename.spec.ts` for tests, `componentName.fixture.astro` for fixtures -- Working example: `src/components/Test/__tests__/webComponent.spec.ts` - -## E2E Testing - -- **NEVER hard-code content slugs** - fetch dynamically from listing pages -- **Always run with `CI=1` and `FORCE_COLOR=1`** - e.g., `CI=1 FORCE_COLOR=1 npx playwright test test/e2e/file.spec.ts` -- **NEVER run full e2e suite** unless requested - it takes 10+ minutes -- **NEVER start dev server** - user maintains running server -- **Use `BasePage.waitForPageLoad()`** to wait for `astro:page-load` event -- **NEVER use `waitForTimeout()`** - use event-based waits -- **NEVER use ad-hoc numeric timeouts** in E2E specs or page objects - use `wait.*` from `test/e2e/helpers/waitTimeouts.ts`. -- If no existing `wait.*` knob fits, **ask what to do** before adding a new `wait.bespoke*` knob. -- **Avoid `waitForLoadState('networkidle')` for gating**: WebKit/mobile-safari can hang indefinitely due to long-lived requests. - - Prefer deterministic readiness signals: `waitForSelector`, `waitForFunction` on app-ready attributes, or `BasePage.waitForPageLoad()`. - - If you truly need a network-idle-ish gate, use `BasePage.waitForNetworkIdleBestEffort()`. -- **transition:persist**: Apply to HTML elements in component definition, not on component usage - -## Vite Optimized Deps (E2E Determinism) - -- If Playwright E2E becomes flaky due to Vite "optimized deps" issues (e.g., stale/outdated optimized modules, wrong MIME type, 504 "Outdated Optimize Dep"), fix it by: - - Adding the problematic package/entrypoint to `vite.optimizeDeps.include` in `astro.config.ts` - - Enabling `vite.optimizeDeps.force` for Playwright runs (`PLAYWRIGHT=true`) -- Priority is deterministic E2E runs over dev startup time. - -## View Transitions Testing - -- `page.goto(url)` = Full page reload (no View Transitions) -- `page.navigateToPage('/path')` = Astro View Transitions (client-side navigation) -- Always use `navigateToPage()` for consistency - never ad-hoc `click('a[href]')` diff --git a/.github/skills/css/SKILL.md b/.github/skills/css/SKILL.md new file mode 100644 index 000000000..8a7081641 --- /dev/null +++ b/.github/skills/css/SKILL.md @@ -0,0 +1,32 @@ +--- +name: css +description: Use this skill when writing or reviewing CSS in this repository. Triggers include .css files, selector specificity, z-index usage, dark-mode theming, and Tailwind dark variant questions. +--- + +# CSS skill + +Use this skill for stylesheet edits in this repository. + +## Rules + +- Prefer a single class selector for styling. +- Avoid ID selectors in CSS. Use classes or attributes instead. +- Avoid chaining state across unrelated roots. +- Avoid long selector chains. If more specificity is needed, add a single component class. +- Avoid `!important` except in vendor CSS and print rules. +- Do not use Tailwind `dark:` variant classes for dark-mode theming. +- Use the project's theme tokens and helper classes instead. + +## Z-index rules + +- Do not hard-code numeric `z-index` values. +- Use the z-index tokens defined in `src/styles/index.css`. +- If no existing token fits, ask the user before adding a new token. + +## Related guidance + +- Theme color tokens remain governed by the always-on `theme-colors.instructions.md` rules. + +## References + +- See `.github/skills/css/references/examples.md` for concrete repo examples. \ No newline at end of file diff --git a/.github/skills/css/references/examples.md b/.github/skills/css/references/examples.md new file mode 100644 index 000000000..e82d70079 --- /dev/null +++ b/.github/skills/css/references/examples.md @@ -0,0 +1,14 @@ +# CSS references + +Use these files as baseline examples for CSS patterns in this repo: + +- `src/components/Toasts/NetworkStatus/index.module.css` +- `src/styles/index.css` +- `src/styles/vendor/mermaid.css` +- `src/styles/theme-inline.css` + +Key repo-specific constraints: + +- Prefer low-specificity component classes. +- Use theme tokens instead of ad-hoc colors. +- Use z-index tokens from `src/styles/index.css` instead of numeric literals. diff --git a/.github/skills/docs-astro/SKILL.md b/.github/skills/docs-astro/SKILL.md new file mode 100644 index 000000000..5c5506df9 --- /dev/null +++ b/.github/skills/docs-astro/SKILL.md @@ -0,0 +1,27 @@ +--- +name: docs-astro +description: Use this skill when writing or editing Astro templates under docs/. Triggers include docs/**/*.astro, Astro frontmatter conventions, separate script files, and docs component testing guidance. +--- + +# Docs Astro skill + +Use this skill for Astro files under `docs/`. + +## Rules + +- Always use TypeScript for type safety. +- Put script logic in a separate file and import it rather than embedding complex script code inline. + +## Testing guidance + +- When testing Astro components, use the Astro Container API instead of manual HTML fixtures. +- This keeps tests in sync with the actual component implementation. +- Use the working examples in `src/components/Test` as the reference pattern. + +## Related guidance + +- For broader Astro component testing rules, also use the `testing` skill. + +## References + +- See `.github/skills/docs-astro/references/examples.md` for concrete repo examples. \ No newline at end of file diff --git a/.github/skills/docs-astro/references/examples.md b/.github/skills/docs-astro/references/examples.md new file mode 100644 index 000000000..380f99379 --- /dev/null +++ b/.github/skills/docs-astro/references/examples.md @@ -0,0 +1,14 @@ +# Docs Astro references + +Use these files as baseline examples for Astro patterns in this repo: + +- `src/components/Head/index.astro` +- `src/layouts/MarkdownLayout.astro` +- `src/components/Test/container.astro` +- `src/components/Test/__tests__/webComponent.spec.ts` + +Key repo-specific constraints: + +- Use TypeScript frontmatter. +- Keep script logic separated when practical. +- Use Astro Container API patterns for component testing. diff --git a/.github/skills/docs-markdown/SKILL.md b/.github/skills/docs-markdown/SKILL.md new file mode 100644 index 000000000..5768f6534 --- /dev/null +++ b/.github/skills/docs-markdown/SKILL.md @@ -0,0 +1,28 @@ +--- +name: docs-markdown +description: Use this skill when writing or editing Markdown files under docs/. Triggers include docs/**/*.md, documentation wording, heading hierarchy, and docs-specific structure questions. +--- + +# Docs Markdown skill + +Use this skill for Markdown files under `docs/`. + +## Rules + +- Use clear and concise language. +- Ensure headings follow a consistent hierarchy. +- Use sentence case for headings and titles. +- Use backticks for inline code snippets. +- Use fenced code blocks with language identifiers. +- Use bullet points or numbered lists for steps and features. +- Include links to relevant resources or documentation. +- Ensure spacing and indentation remain readable. +- End files with a single newline. + +## Related guidance + +- For Markdown linting, MDX safety, and Mermaid authoring, also use the `markdown-mermaid` skill. + +## References + +- See `.github/skills/docs-markdown/references/examples.md` for concrete repo examples. \ No newline at end of file diff --git a/.github/skills/docs-markdown/references/examples.md b/.github/skills/docs-markdown/references/examples.md new file mode 100644 index 000000000..9f2b18354 --- /dev/null +++ b/.github/skills/docs-markdown/references/examples.md @@ -0,0 +1,13 @@ +# Docs Markdown references + +Use these files as baseline examples for Markdown documentation in this repo: + +- `docs/DEPLOYMENT_SETUP.md` +- `docs/ENVIRONMENT_VARIABLES.md` +- `docs/newsletter/infrastructure.md` + +Key repo-specific constraints: + +- Keep headings in sentence case. +- Use concise, procedural writing for setup guides. +- Defer Markdown lint and Mermaid-specific rules to the `markdown-mermaid` skill. diff --git a/.github/skills/javascript-typescript/SKILL.md b/.github/skills/javascript-typescript/SKILL.md new file mode 100644 index 000000000..a083c0689 --- /dev/null +++ b/.github/skills/javascript-typescript/SKILL.md @@ -0,0 +1,30 @@ +--- +name: javascript-typescript +description: Use this skill when writing or reviewing JavaScript or TypeScript in this repository. Triggers include .js, .jsx, .ts, .tsx, semicolon style, JSDoc, and Playwright timeout knobs in test/e2e. +--- + +# JavaScript and TypeScript skill + +Use this skill for JavaScript and TypeScript edits in this repository. + +## Rules + +- Use `const` and `let` instead of `var`. +- Prefer arrow functions for anonymous functions. +- Use JSDoc for public functions and components. +- Do not add semicolons at the end of statements. + +## Playwright timeout policy + +- For code under `test/e2e/**`, do not introduce new numeric timeouts. +- Use existing timeout knobs from `test/e2e/helpers/waitTimeouts.ts`. +- If none fit, stop and ask whether to add a new timeout knob, refactor to an event-based wait, or change the underlying behavior. + +## Related guidance + +- For broader test rules, also use the `testing` skill. +- For Astro View Transitions navigation behavior, also use the `view-transitions` skill. + +## References + +- See `.github/skills/javascript-typescript/references/examples.md` for concrete repo examples. \ No newline at end of file diff --git a/.github/skills/javascript-typescript/references/examples.md b/.github/skills/javascript-typescript/references/examples.md new file mode 100644 index 000000000..93a785aaf --- /dev/null +++ b/.github/skills/javascript-typescript/references/examples.md @@ -0,0 +1,14 @@ +# JavaScript and TypeScript references + +Use these files as baseline examples for JavaScript and TypeScript patterns in this repo: + +- `src/components/Toasts/NetworkStatus/client/index.ts` +- `test/e2e/helpers/waitTimeouts.ts` +- `test/e2e/helpers/pageObjectModels/BasePage.ts` +- `src/middleware.ts` + +Key repo-specific constraints: + +- Follow the semicolon-free style already used across the codebase. +- Reuse `wait.*` timeout knobs in E2E code. +- Prefer the existing shared helpers over ad-hoc browser interaction code. diff --git a/.github/skills/markdown-mermaid/SKILL.md b/.github/skills/markdown-mermaid/SKILL.md new file mode 100644 index 000000000..027899a8a --- /dev/null +++ b/.github/skills/markdown-mermaid/SKILL.md @@ -0,0 +1,51 @@ +--- +name: markdown-mermaid +description: Use this skill when writing or editing Markdown or MDX, especially when Markdown linting, fenced code blocks, inline code safety, or Mermaid diagrams are involved. Triggers include .md, .mdx, markdownlint, mermaid, flowchart, graph TD, subgraph, and fenced code block formatting. +--- + +# Markdown and Mermaid skill + +Use this skill for `.md` and `.mdx` authoring in this repository. + +## Process + +1. Apply the Markdown structure and linting rules first. +2. If the file contains Mermaid, apply the Mermaid-specific authoring constraints before finishing edits. +3. Reuse the existing Mermaid docs and tests when choosing diagram syntax. + +## Markdown rules + +- Add a blank line after headings. +- Add a blank line before and after lists. +- Use consistent list markers. +- End files with a single newline. +- Wrap inline code-like snippets in backticks. +- Use fenced code blocks with explicit language identifiers. +- Wrap bare URLs in angle brackets. +- Avoid raw `<` in prose when it could be parsed as HTML or JSX. Prefer `<` or backticks. + +## MDX and inline code safety + +- Wrap code-y snippets in backticks so the parser does not treat them as HTML or JSX. +- This includes examples like `{ param, ... }`, `generic`, `Array`, and `T extends Foo`. + +## Mermaid rules + +- This repo renders Mermaid during builds, so invalid Mermaid syntax will fail `npm run build`. +- Prefer `flowchart TD` or `flowchart LR` for new flowchart-style diagrams. +- `graph TD` is acceptable when matching existing content, but treat it as legacy syntax. +- Do not use `quadrantChart`. +- Do not use quoted subgraph titles like `subgraph "Title"`. +- Prefer simple subgraph ids with plain labels like `subgraph myGroup[My Group]`. +- Avoid punctuation-heavy subgraph labels. +- Avoid double quotes inside node labels. +- Avoid `@` in node labels. + +## Review rules + +- If you are refactoring an existing Markdown file, fix formatting issues that violate these rules while you are there. +- In reviews, flag Markdown lint and Mermaid syntax issues explicitly. + +## References + +- See `.github/skills/markdown-mermaid/references/examples.md` for concrete repo examples. \ No newline at end of file diff --git a/.github/skills/markdown-mermaid/references/examples.md b/.github/skills/markdown-mermaid/references/examples.md new file mode 100644 index 000000000..721472ef6 --- /dev/null +++ b/.github/skills/markdown-mermaid/references/examples.md @@ -0,0 +1,16 @@ +# Markdown and Mermaid references + +Use these files as the baseline references for Markdown and Mermaid behavior in this repo: + +- `docs/MERMAID.md` +- `docs/DEPLOYMENT_SETUP.md` +- `src/lib/config/mermaid.ts` +- `src/lib/config/markdown.ts` +- `src/lib/markdown/__tests__/integration/rehype-mermaid-astro.spec.ts` +- `test/e2e/specs/04-components/markdown.spec.ts` + +Key repo-specific constraints already enforced by code and tests: + +- Mermaid is rendered to inline SVG at build time. +- Mermaid blocks are excluded from the standard Shiki code-block path. +- Build and test coverage already exist for Mermaid rendering, so new syntax should stay close to existing supported patterns. diff --git a/.github/skills/testing/SKILL.md b/.github/skills/testing/SKILL.md new file mode 100644 index 000000000..b91a25994 --- /dev/null +++ b/.github/skills/testing/SKILL.md @@ -0,0 +1,54 @@ +--- +name: testing +description: Use this skill when writing, reviewing, or running unit tests or E2E tests. Triggers include test, spec, fixture, Playwright, Vitest, Astro Container API, and wait timeout policy. +--- + +# Testing skill + +Use this skill for all test-related work in this repository, especially `*.spec.ts`, Astro component tests, and Playwright E2E coverage. + +## Process + +1. Identify the test type before editing anything: + - Astro component or unit test + - Playwright E2E test + - Test infrastructure or config +2. Apply the matching rules below. +3. Reuse the existing examples and helpers before introducing new patterns. +4. Run only targeted verification unless the user explicitly asks for broader coverage. + +## Astro component and unit test rules + +- Never use manual HTML strings in test files or fixtures. +- Use Astro's Container API with actual `.astro` templates. +- Test fixtures must import actual components, not duplicate rendered HTML. +- Prefer the naming pattern `filename.spec.ts` for tests and `componentName.fixture.astro` for fixtures. +- Use `experimental_AstroContainer.create()` to instantiate the container. +- Use `container.renderToString(Component)` to render actual Astro components. +- Configure Vitest with `getViteConfig()` from `astro/config` when Container API support is required. + +## E2E test rules + +- Never hard-code content slugs in E2E tests. Fetch dynamically from listing pages. +- Always run Playwright with `CI=1` and `FORCE_COLOR=1`. +- Never run the full E2E suite unless the user explicitly asks for it. +- Never start the dev server yourself. The user maintains the running dev server. +- Never use ad-hoc numeric timeouts in E2E specs or page objects. +- Use `wait.*` from `test/e2e/helpers/waitTimeouts.ts`. +- If no existing `wait.*` knob fits, ask before adding a bespoke timeout. +- Avoid `waitForLoadState('networkidle')` for gating. Prefer deterministic readiness signals. +- If a network-idle-like wait is truly needed, use `BasePage.waitForNetworkIdleBestEffort()`. + +## View Transitions note + +- For Astro View Transitions behavior, `transition:persist`, or navigation semantics, also use the `view-transitions` skill. + +## Vite optimized deps guidance + +- If Playwright becomes flaky due to optimized deps issues, prefer deterministic fixes over workarounds. +- Add the problematic package or entrypoint to `vite.optimizeDeps.include` in `astro.config.ts`. +- Enable `vite.optimizeDeps.force` for Playwright runs with `PLAYWRIGHT=true` when needed. + +## References + +- See `.github/skills/testing/references/examples.md` for concrete repo examples. diff --git a/.github/skills/testing/references/examples.md b/.github/skills/testing/references/examples.md new file mode 100644 index 000000000..aed817b65 --- /dev/null +++ b/.github/skills/testing/references/examples.md @@ -0,0 +1,10 @@ +# Testing references + +Use these existing repo files as the baseline examples before creating new testing patterns: + +- `src/components/Test/container.astro` +- `src/components/Test/container.spec.ts` +- `src/components/Test/__tests__/webComponent.spec.ts` +- `test/e2e/helpers/waitTimeouts.ts` + +When editing E2E tests, prefer shared BasePage helpers and existing timeout knobs instead of direct Playwright calls or numeric waits. diff --git a/.github/skills/view-transitions/SKILL.md b/.github/skills/view-transitions/SKILL.md new file mode 100644 index 000000000..d44bd4c57 --- /dev/null +++ b/.github/skills/view-transitions/SKILL.md @@ -0,0 +1,38 @@ +--- +name: view-transitions +description: Use this skill when working on Astro View Transitions behavior, transition:persist, client-side navigation, or related Playwright coverage. Triggers include navigateToPage, waitForPageLoad, astro:page-load, page.goto, BasePage helpers, and persisted UI across navigation. +--- + +# View Transitions skill + +Use this skill when changing Astro View Transitions behavior, implementing persisted UI, or writing tests that depend on client-side navigation. + +## Process + +1. Decide whether the behavior under test or implementation is a full page load or Astro client-side navigation. +2. Use the matching navigation primitive. +3. Reuse the shared BasePage helpers instead of ad-hoc Playwright interactions. +4. If persistence is involved, verify that `transition:persist` is applied in the component definition, not only at the call site. + +## Navigation rules + +- Use `page.goto(url)` for full browser navigations without View Transitions. +- Use `navigateToPage('/path')` for in-site client-side navigation with View Transitions. +- Never use ad-hoc `click('a[href]')` calls for client-side navigation flows. +- When a Playwright-native action is still required, expose it through shared `BasePage` helpers and reuse that helper from tests. + +## Synchronization rules + +- Use `BasePage.waitForPageLoad()` to wait for the `astro:page-load` event. +- Never use `page.waitForTimeout()` to gate View Transitions. +- Prefer deterministic readiness signals over generic waiting. + +## Persistence rules + +- Apply `transition:persist` directly to HTML elements, including custom elements, in the component definition. +- Do not place `transition:persist` only on an Astro component usage wrapper. +- When testing persistence, verify DOM identity and persisted state across navigation instead of only checking visual presence. + +## References + +- See `.github/skills/view-transitions/references/examples.md` for concrete repo examples. \ No newline at end of file diff --git a/.github/skills/view-transitions/references/examples.md b/.github/skills/view-transitions/references/examples.md new file mode 100644 index 000000000..8d42dbac3 --- /dev/null +++ b/.github/skills/view-transitions/references/examples.md @@ -0,0 +1,16 @@ +# View Transitions references + +Use these files as the baseline examples for Astro View Transitions behavior in this repo: + +- `test/e2e/helpers/pageObjectModels/BasePage.ts` +- `test/e2e/specs/04-components/theme-picker.spec.ts` +- `test/e2e/specs/09-persistence/themepicker.spec.ts` +- `test/e2e/specs/09-persistence/footer.spec.ts` +- `src/components/ThemePicker/index.astro` +- `src/components/Footer/index.astro` + +Key patterns already established here: + +- `navigateToPage()` for client-side navigation +- `waitForPageLoad()` for `astro:page-load` synchronization +- `transition:persist` applied to actual rendered elements diff --git a/src/actions/downloads/__tests__/action.spec.ts b/src/actions/downloads/__tests__/action.spec.ts new file mode 100644 index 000000000..8d0b1902f --- /dev/null +++ b/src/actions/downloads/__tests__/action.spec.ts @@ -0,0 +1,115 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +type ActionConfig = { + handler: (_input: Input, _context: unknown) => Promise +} + +const getMockedHandler = (action: unknown): ActionConfig['handler'] => { + return (action as ActionConfig).handler +} + +vi.mock('astro:actions', () => { + return { + defineAction: (config: unknown) => config, + } +}) + +vi.mock('@actions/gdpr/entities/consent', () => { + return { + createConsentRecord: vi.fn(async () => ({ id: 'consent-1' })), + } +}) + +vi.mock('@actions/utils/environment/environmentActions', () => { + return { + getPrivacyPolicyVersion: vi.fn(() => 'privacy-version-1'), + } +}) + +vi.mock('@actions/utils/errors', () => { + return { + handleActionsFunctionError: vi.fn(() => undefined), + } +}) + +vi.mock('@actions/utils/hubspot', () => { + return { + createOrUpdateContact: vi.fn(async () => ({ id: 'hubspot-1' })), + setMarketingOptIn: vi.fn(async () => undefined), + } +}) + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('downloads inputSchema', () => { + it('accepts omitted optional job fields', async () => { + const { inputSchema } = await import('../action') + + const result = inputSchema.parse({ + firstName: 'Jane', + lastName: 'Doe', + workEmail: 'jane@example.com', + }) + + expect(result).toEqual({ + firstName: 'Jane', + lastName: 'Doe', + workEmail: 'jane@example.com', + }) + }) + + it('normalizes blank optional job fields to undefined', async () => { + const { inputSchema } = await import('../action') + + const result = inputSchema.parse({ + firstName: 'Jane', + lastName: 'Doe', + workEmail: 'jane@example.com', + jobTitle: ' ', + companyName: '', + }) + + expect(result).toEqual({ + firstName: 'Jane', + lastName: 'Doe', + workEmail: 'jane@example.com', + jobTitle: undefined, + companyName: undefined, + }) + }) +}) + +describe('downloads.submit.handler', () => { + it('submits successfully without optional job fields', async () => { + const { downloads } = await import('../action') + const { createOrUpdateContact } = await import('@actions/utils/hubspot') + const { createConsentRecord } = await import('@actions/gdpr/entities/consent') + + const context = { + request: new Request('https://example.com/_actions/downloads/submit', { + method: 'POST', + headers: { 'user-agent': 'ua-1' }, + }), + clientAddress: '203.0.113.10', + } + + const response = await getMockedHandler(downloads.submit)({ + firstName: 'Jane', + lastName: 'Doe', + workEmail: 'jane@example.com', + }, context) + + expect(response).toEqual({ + success: true, + message: 'Form submitted successfully', + }) + expect(createOrUpdateContact).toHaveBeenCalledWith({ + email: 'jane@example.com', + firstname: 'Jane', + lastname: 'Doe', + }) + expect(createConsentRecord).not.toHaveBeenCalled() + }) +}) \ No newline at end of file diff --git a/src/actions/downloads/action.ts b/src/actions/downloads/action.ts index ba92e3a2c..4ff475c1c 100644 --- a/src/actions/downloads/action.ts +++ b/src/actions/downloads/action.ts @@ -7,6 +7,15 @@ import { getPrivacyPolicyVersion } from '@actions/utils/environment/environmentA import { handleActionsFunctionError } from '@actions/utils/errors' import { createOrUpdateContact, setMarketingOptIn } from '@actions/utils/hubspot' +const optionalTrimmedString = z.preprocess(value => { + if (typeof value !== 'string') { + return value + } + + const trimmedValue = value.trim() + return trimmedValue.length > 0 ? trimmedValue : undefined +}, z.string().optional()) + export const inputSchema = z.object({ firstName: z.string().trim().min(1), lastName: z.string().trim().min(1), @@ -15,8 +24,8 @@ export const inputSchema = z.object({ .trim() .min(1) .refine(value => emailValidator.validate(value), 'Invalid email address'), - jobTitle: z.string().trim().min(1), - companyName: z.string().trim().min(1), + jobTitle: optionalTrimmedString, + companyName: optionalTrimmedString, consent: z.boolean().optional(), DataSubjectId: z.uuid().optional(), }) @@ -59,8 +68,8 @@ export const downloads = { console.log('Download form submission:', { name: `${input.firstName} ${input.lastName}`, email: input.workEmail, - jobTitle: input.jobTitle, - company: input.companyName, + jobTitle: input.jobTitle ?? null, + company: input.companyName ?? null, timestamp: new Date().toISOString(), }) diff --git a/src/actions/newsletter/__tests__/action.spec.ts b/src/actions/newsletter/__tests__/action.spec.ts index 30caaaabe..5a783e8d3 100644 --- a/src/actions/newsletter/__tests__/action.spec.ts +++ b/src/actions/newsletter/__tests__/action.spec.ts @@ -300,7 +300,82 @@ describe('newsletter.subscribe.handler', () => { expect(throwActionError).toHaveBeenCalledWith( expect.any(Error), - { route: '/_actions/newsletter/subscribe', operation: 'subscribe' } + expect.objectContaining({ + route: '/_actions/newsletter/subscribe', + operation: 'subscribe', + extra: expect.objectContaining({ + stage: 'buildRequestFingerprint', + source: 'newsletter_form', + input: expect.objectContaining({ + consentGiven: true, + emailDomain: 'example.com', + hasDataSubjectId: false, + hasFirstName: false, + subjectIdSource: 'pending', + }), + }), + }) + ) + }) + + it('logs handled action errors with subscribe stage context', async () => { + vi.resetModules() + + const { createPendingSubscription } = await import('@actions/newsletter/domain') + vi.mocked(createPendingSubscription).mockRejectedValueOnce( + new (await import('@actions/utils/errors')).ActionsFunctionError('db unavailable', { + status: 500, + }) + ) + + const { newsletter } = await import('../action') + const { handleActionsFunctionError } = await import('@actions/utils/errors') + + const context = { + request: new Request('https://example.com/_actions/newsletter/subscribe', { + method: 'POST', + headers: { 'user-agent': 'ua-6' }, + }), + cookies: {} as unknown, + clientAddress: '203.0.113.16', + } + + await expect( + getMockedHandler(newsletter.subscribe)( + { email: 'test@example.com', consentGiven: true }, + context + ) + ).rejects.toMatchObject({ + name: 'ActionsFunctionError', + status: 500, + message: 'db unavailable', + }) + + expect(handleActionsFunctionError).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'ActionsFunctionError', + status: 500, + }), + expect.objectContaining({ + route: '/_actions/newsletter/subscribe', + operation: 'subscribe', + extra: expect.objectContaining({ + stage: 'createPendingSubscription', + fingerprint: 'fingerprint-1', + source: 'newsletter_form', + input: expect.objectContaining({ + emailDomain: 'example.com', + emailLength: 16, + consentGiven: true, + subjectIdSource: 'generated', + }), + request: expect.objectContaining({ + hasClientAddress: true, + hasUserAgent: true, + rateLimitIdentifier: 'newsletter:consent:fingerprint-1', + }), + }), + }) ) }) }) diff --git a/src/actions/newsletter/action.ts b/src/actions/newsletter/action.ts index 98048d2a8..a85abb2be 100644 --- a/src/actions/newsletter/action.ts +++ b/src/actions/newsletter/action.ts @@ -26,6 +26,61 @@ const confirmSchema = z.object({ token: z.string().min(1), }) +type NewsletterSubscribeStage = + | 'buildRequestFingerprint' + | 'checkRateLimit' + | 'validateEmail' + | 'validateConsent' + | 'resolveDataSubjectId' + | 'createConsentRecord' + | 'createPendingSubscription' + | 'sendConfirmationEmail' + +const getEmailDomain = (email: string): string | undefined => { + const normalizedEmail = email.trim().toLowerCase() + const atIndex = normalizedEmail.lastIndexOf('@') + + if (atIndex === -1 || atIndex === normalizedEmail.length - 1) { + return undefined + } + + return normalizedEmail.slice(atIndex + 1) +} + +const buildSubscribeErrorExtra = (options: { + body: z.infer + fingerprint: string | undefined + consentFunctional: boolean + stage: NewsletterSubscribeStage + userAgent: string + clientAddress: string | undefined + rateLimitIdentifier: string | undefined + subjectIdSource: 'generated' | 'provided' | 'pending' +}): Record => { + return { + stage: options.stage, + source: 'newsletter_form', + consentFunctional: options.consentFunctional, + fingerprint: options.fingerprint, + request: { + hasClientAddress: + typeof options.clientAddress === 'string' && options.clientAddress !== 'unknown', + hasUserAgent: options.userAgent !== 'unknown', + rateLimitIdentifier: options.rateLimitIdentifier, + }, + input: { + emailDomain: getEmailDomain(options.body.email), + emailLength: options.body.email.trim().length, + consentGiven: Boolean(options.body.consentGiven), + hasFirstName: + typeof options.body.firstName === 'string' && options.body.firstName.trim().length > 0, + hasDataSubjectId: + typeof options.body.DataSubjectId === 'string' && options.body.DataSubjectId.length > 0, + subjectIdSource: options.subjectIdSource, + }, + } +} + export const newsletter = { subscribe: defineAction({ accept: 'json', @@ -35,16 +90,26 @@ export const newsletter = { context ): Promise<{ success: true; message: string; requiresConfirmation: true }> => { const route = '/_actions/newsletter/subscribe' + let stage: NewsletterSubscribeStage = 'buildRequestFingerprint' + let fingerprint: string | undefined + let consentFunctional = false + let rateLimitIdentifier: string | undefined + let subjectIdSource: 'generated' | 'provided' | 'pending' = 'pending' + const userAgent = context.request.headers.get('user-agent') || 'unknown' try { - const { fingerprint } = buildRequestFingerprint({ + const requestFingerprint = buildRequestFingerprint({ route, request: context.request, cookies: context.cookies, clientAddress: context.clientAddress, }) - const rateLimitIdentifier = createRateLimitIdentifier('newsletter:consent', fingerprint) + fingerprint = requestFingerprint.fingerprint + consentFunctional = requestFingerprint.consentFunctional + + stage = 'checkRateLimit' + rateLimitIdentifier = createRateLimitIdentifier('newsletter:consent', fingerprint) const { success, reset } = await checkRateLimit(rateLimiters.consent, rateLimitIdentifier) if (!success) { @@ -53,8 +118,10 @@ export const newsletter = { throw new ActionsFunctionError(`Try again in ${retryAfterSeconds}s`, { status: 429 }) } + stage = 'validateEmail' const validatedEmail = validateEmail(body.email) + stage = 'validateConsent' if (!body.consentGiven) { throw new ActionsFunctionError( 'You must consent to receive marketing emails to subscribe.', @@ -62,15 +129,18 @@ export const newsletter = { ) } - const userAgent = context.request.headers.get('user-agent') || 'unknown' - + stage = 'resolveDataSubjectId' let subjectId = body.DataSubjectId if (!subjectId) { subjectId = uuidv4() + subjectIdSource = 'generated' } else if (!uuidValidate(subjectId)) { throw new ActionsFunctionError('Invalid DataSubjectId format', { status: 400 }) + } else { + subjectIdSource = 'provided' } + stage = 'createConsentRecord' await createConsentRecord({ dataSubjectId: subjectId, email: validatedEmail, @@ -86,6 +156,7 @@ export const newsletter = { verified: false, }) + stage = 'createPendingSubscription' const token = await createPendingSubscription({ email: validatedEmail, ...(body.firstName && { firstName: body.firstName }), @@ -96,6 +167,7 @@ export const newsletter = { source: 'newsletter_form', }) + stage = 'sendConfirmationEmail' await sendConfirmationEmail(validatedEmail, token, body.firstName) return { @@ -104,10 +176,27 @@ export const newsletter = { requiresConfirmation: true, } } catch (error) { + const errorContext = { + route, + operation: 'subscribe', + extra: buildSubscribeErrorExtra({ + body, + fingerprint, + consentFunctional, + stage, + userAgent, + clientAddress: context.clientAddress, + rateLimitIdentifier, + subjectIdSource, + }), + } as const + if (error instanceof ActionsFunctionError) { + handleActionsFunctionError(error, errorContext) throw error } - throwActionError(error, { route, operation: 'subscribe' }) + + throwActionError(error, errorContext) } }, }), diff --git a/src/actions/utils/errors/__tests__/actionsFunctionHandler.spec.ts b/src/actions/utils/errors/__tests__/actionsFunctionHandler.spec.ts index bc1468a2b..e73f04c9c 100644 --- a/src/actions/utils/errors/__tests__/actionsFunctionHandler.spec.ts +++ b/src/actions/utils/errors/__tests__/actionsFunctionHandler.spec.ts @@ -1,4 +1,16 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const captureExceptionMock = vi.fn() +const setTagsMock = vi.fn() +const setTagMock = vi.fn() +const setExtrasMock = vi.fn() +const withScopeMock = vi.fn((callback: (_scope: unknown) => void) => { + callback({ + setTags: setTagsMock, + setTag: setTagMock, + setExtras: setExtrasMock, + }) +}) vi.mock('astro:actions', () => { class ActionError extends Error { @@ -26,10 +38,18 @@ vi.mock('@actions/utils/sentry', () => ({ })) vi.mock('@sentry/astro', () => ({ - captureException: () => undefined, - withScope: (_fn: (_scope: unknown) => void) => undefined, + captureException: captureExceptionMock, + withScope: withScopeMock, })) +beforeEach(() => { + captureExceptionMock.mockReset() + setTagsMock.mockReset() + setTagMock.mockReset() + setExtrasMock.mockReset() + withScopeMock.mockClear() +}) + describe('actionsFunctionHandler', () => { it('converts server errors into ActionError with fallback message', async () => { const { ActionsFunctionError } = await import('../ActionsFunctionError') @@ -64,4 +84,71 @@ describe('actionsFunctionHandler', () => { expect(normalized.status).toBe(500) expect(normalized.message).toBe('boom') }) + + it('includes merged details in structured logs', async () => { + const { ActionsFunctionError } = await import('../ActionsFunctionError') + const { formatActionsErrorLogEntry } = await import('../actionsFunctionHandler') + + const error = new ActionsFunctionError('DB exploded', { + status: 500, + route: 'actions:test', + appCode: 'DB_WRITE_FAILED', + details: { stage: 'createPendingSubscription' }, + }) + + const entry = formatActionsErrorLogEntry(error, { + route: 'actions:test', + operation: 'subscribe', + extra: { fingerprint: 'fingerprint-1' }, + }) + + expect(entry.appCode).toBe('DB_WRITE_FAILED') + expect(entry.details).toEqual({ + stage: 'createPendingSubscription', + fingerprint: 'fingerprint-1', + }) + }) + + it('forwards merged details and appCode to sentry in production', async () => { + vi.resetModules() + + vi.doMock('@actions/utils/environment/environmentActions', () => ({ + isDev: () => false, + isProd: () => true, + isTest: () => false, + isUnitTest: () => false, + })) + + const { ActionsFunctionError } = await import('../ActionsFunctionError') + const { handleActionsFunctionError } = await import('../actionsFunctionHandler') + + const error = new ActionsFunctionError('DB exploded', { + status: 500, + route: 'actions:test', + appCode: 'DB_WRITE_FAILED', + details: { stage: 'persist' }, + }) + + const normalized = handleActionsFunctionError(error, { + route: 'actions:test', + operation: 'subscribe', + extra: { fingerprint: 'fingerprint-1' }, + }) + + expect(normalized).toBeInstanceOf(ActionsFunctionError) + expect(setTagsMock).toHaveBeenCalledWith( + expect.objectContaining({ + route: 'actions:test', + status: '500', + retryable: 'true', + appCode: 'DB_WRITE_FAILED', + operation: 'subscribe', + }) + ) + expect(setExtrasMock).toHaveBeenCalledWith({ + stage: 'persist', + fingerprint: 'fingerprint-1', + }) + expect(captureExceptionMock).toHaveBeenCalledWith(expect.any(ActionsFunctionError)) + }) }) diff --git a/src/actions/utils/errors/actionsFunctionHandler.ts b/src/actions/utils/errors/actionsFunctionHandler.ts index 7e405ab36..5b1bfc783 100644 --- a/src/actions/utils/errors/actionsFunctionHandler.ts +++ b/src/actions/utils/errors/actionsFunctionHandler.ts @@ -26,12 +26,14 @@ export interface ActionsErrorLogEntry { operation?: string | undefined status: number code?: string | undefined + appCode?: string | undefined retryable: boolean requestId?: string | undefined correlationId?: string | undefined runtime?: string | undefined region?: string | undefined message: string + details?: Record | undefined stack?: string | undefined cause?: string | undefined } @@ -50,6 +52,20 @@ const toCauseString = (cause: unknown): string | undefined => { return String(cause) } +const mergeErrorDetails = ( + error: ActionsFunctionError, + context: ActionsFunctionContext +): Record | undefined => { + if (error.details && context.extra) { + return { + ...error.details, + ...context.extra, + } + } + + return error.details ?? context.extra +} + /** * Normalizes an unknown thrown value into an `ActionsFunctionError`, logs it, and * reports it to Sentry in production. This is the central entrypoint for server @@ -77,12 +93,15 @@ export function handleActionsFunctionError( logActionsError(normalizedError, context) + const mergedDetails = mergeErrorDetails(normalizedError, context) + if (isProd()) { withScope(scope => { scope.setTags({ route: context.route, status: String(normalizedError.status), retryable: normalizedError.retryable ? 'true' : 'false', + ...(normalizedError.appCode && { appCode: normalizedError.appCode }), ...(context.operation && { operation: context.operation }), ...(context.runtime && { runtime: context.runtime }), ...(context.region && { region: context.region }), @@ -91,8 +110,8 @@ export function handleActionsFunctionError( if (context.requestId) scope.setTag('requestId', context.requestId) if (context.correlationId) scope.setTag('correlationId', context.correlationId) - if (context.extra && Object.keys(context.extra).length > 0) { - scope.setExtras(context.extra) + if (mergedDetails && Object.keys(mergedDetails).length > 0) { + scope.setExtras(mergedDetails) } captureException(normalizedError) @@ -116,6 +135,7 @@ export function formatActionsErrorLogEntry( route: context.route, status: error.status, code: error.code, + appCode: error.appCode, retryable: error.retryable, message: error.getSafeMessage(), } @@ -126,6 +146,11 @@ export function formatActionsErrorLogEntry( if (context.requestId) entry.requestId = context.requestId if (context.correlationId) entry.correlationId = context.correlationId + const mergedDetails = mergeErrorDetails(error, context) + if (mergedDetails && Object.keys(mergedDetails).length > 0) { + entry.details = mergedDetails + } + if (isDev() && !isUnitTest()) { if (error.stack) entry.stack = error.stack const causeText = toCauseString(error.cause) diff --git a/src/components/CallToAction/Newsletter/client/__tests__/index.spec.ts b/src/components/CallToAction/Newsletter/client/__tests__/index.spec.ts index d83774e8c..e416ec06a 100644 --- a/src/components/CallToAction/Newsletter/client/__tests__/index.spec.ts +++ b/src/components/CallToAction/Newsletter/client/__tests__/index.spec.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { experimental_AstroContainer as AstroContainer } from 'astro/container' import { TestError } from '@test/errors' +import * as errorHandlerModule from '@components/scripts/errors/handler' import Newsletter from '@components/CallToAction/Newsletter/index.astro' import type { NewsletterProps } from '@components/CallToAction/Newsletter/client/@types' import type { NewsletterFormElement } from '@components/CallToAction/Newsletter/client' @@ -212,6 +213,8 @@ describe.each(newsletterVariants)('NewsletterFormElement web component (%s)', va newsletterSubscribeMock.mockRejectedValueOnce(new TestError('Network error')) await renderNewsletter(async ({ elements }) => { + const handleScriptErrorSpy = vi.spyOn(errorHandlerModule, 'handleScriptError') + elements.emailInput.value = 'test@example.com' elements.consentCheckbox.checked = true @@ -219,6 +222,7 @@ describe.each(newsletterVariants)('NewsletterFormElement web component (%s)', va await flushPromises() expect(newsletterSubscribeMock).toHaveBeenCalled() + expect(handleScriptErrorSpy).not.toHaveBeenCalled() expect(elements.message.textContent).toBe( 'Network error. Please check your connection and try again.' ) diff --git a/src/components/CallToAction/Newsletter/client/index.ts b/src/components/CallToAction/Newsletter/client/index.ts index 69e640019..ee8e7158b 100644 --- a/src/components/CallToAction/Newsletter/client/index.ts +++ b/src/components/CallToAction/Newsletter/client/index.ts @@ -262,40 +262,41 @@ export class NewsletterFormElement extends LitElement { this.setLoading(true) this.showMessage('Sending confirmation email...', 'info') + let result: Awaited> + try { - const result = await actions.newsletter.subscribe({ + result = await actions.newsletter.subscribe({ email, consentGiven, ...(DataSubjectId ? { DataSubjectId } : {}), }) + } catch { + this.showMessage('Network error. Please check your connection and try again.', 'error') + return + } finally { + this.setLoading(false) + } - if (result.data?.success) { - markEmailCollected(email, 'newsletter_form') - this.showMessage( - result.data.message || - 'Check your email! Click the confirmation link to complete your subscription.', - 'success' - ) - this.submitButton.dispatchEvent( - new CustomEvent('confetti:fire', { bubbles: true, composed: true }) - ) - this.form?.reset() - this.setFieldInvalid(this.emailInput, false) - this.setFieldInvalid(this.consentCheckbox, false) - } else { + if (result.data?.success) { + markEmailCollected(email, 'newsletter_form') + this.showMessage( + result.data.message || + 'Check your email! Click the confirmation link to complete your subscription.', + 'success' + ) + this.submitButton.dispatchEvent( + new CustomEvent('confetti:fire', { bubbles: true, composed: true }) + ) + this.form?.reset() + this.setFieldInvalid(this.emailInput, false) + this.setFieldInvalid(this.consentCheckbox, false) + } else { + if (!result.data?.success) { this.showMessage( result.error?.message || 'Failed to subscribe. Please try again.', 'error' ) } - } catch (error) { - handleScriptError(error, { - scriptName: 'NewsletterFormElement', - operation: 'apiSubmission', - }) - this.showMessage('Network error. Please check your connection and try again.', 'error') - } finally { - this.setLoading(false) } } catch (error) { handleScriptError(error, context) diff --git a/src/components/Pages/Downloads/client/__tests__/__fixtures__/downloadForm.fixture.astro b/src/components/Pages/Downloads/client/__tests__/__fixtures__/downloadForm.fixture.astro index b57707854..796703f05 100644 --- a/src/components/Pages/Downloads/client/__tests__/__fixtures__/downloadForm.fixture.astro +++ b/src/components/Pages/Downloads/client/__tests__/__fixtures__/downloadForm.fixture.astro @@ -19,16 +19,6 @@ import Button from '@components/Button/index.astro' -
- - -
- -
- - -
-