Skip to content

fix(compat): kill dead clicks on accept dialog and dashboard banners - #2894

Draft
posthog-eu[bot] wants to merge 2 commits into
mainfrom
posthog-self-driving/fixcompat-make-accept-dialog-and-81753b
Draft

fix(compat): kill dead clicks on accept dialog and dashboard banners#2894
posthog-eu[bot] wants to merge 2 commits into
mainfrom
posthog-self-driving/fixcompat-make-accept-dialog-and-81753b

Conversation

@posthog-eu

@posthog-eu posthog-eu Bot commented Aug 6, 2026

Copy link
Copy Markdown

Summary

Two dead-click traps and one silent write on the compatibility-acceptance journey.

Problem

  • Accept dialog: users click Accept, nothing happens, they give up — the button was always enabled but the handler bailed with a toast (rendered elsewhere) whenever the reason was empty. Nothing on the field said a reason was required.
  • Dashboard banners: only the small CTA button was clickable; the card's icon/heading/count were inert markup, so clicks on the obvious big target did nothing. Affected CompatibilityBanner and the identically-shaped ReleaseBanner ("View adoption").
  • acknowledge_compatibility_event returned silently on an RBAC denial, so users below release-manager got a success toast for a write that never happened.

Changes

Area Fix
Accept dialog ([app].observe.compatibility.vue) Reason is required: confirm button disabled until it has content, field marked * + required + autofocused, validation shown inline on the field instead of as a toast.
Banners (CompatibilityBanner.vue, ReleaseBanner.vue) Whole card is now the click target — cursor-pointer, hover, keyboard-operable (role="button", Enter/Space); CTA stays as the visual affordance.
RPC (new migration) acknowledge_compatibility_event now RAISEs instead of no-op, so the client stops reporting success on a failed write.

Why "not authorized" and "unknown id" raise the same error

acknowledge_compatibility_event is SECURITY DEFINER. Raising a distinct error only when the row exists-but-you-lack-permission would let a caller outside the org probe which sequential event ids exist. Both branches raise the identical not_authorized, preserving the no-existence-oracle property the previous no-op had while still surfacing the failure to the legitimate client. supabase/schemas/prod.sql is treated as read-only reference and was not edited; the change lands as a CLI-generated migration.

Test plan

  • bun run lint and bun run typecheck:frontend — pass.
  • Updated Playwright e2e (compatibility-events.spec.ts): confirm button is disabled while the reason is empty (no RPC fires) and enables once a reason is typed; banner card still navigates via its data-test hook.
  • Updated DB tests (compatibility-events.test.ts): non-member accept and unknown-id both now raise not_authorized and leave the event unresolved. These require a running Supabase (Docker) and were not executed in the authoring sandbox.

Screenshots

Not captured in this environment (no browser/app runtime available here). Behavior is covered by the updated e2e assertions above.

Checklist

  • My code follows the code style of this project and passes bun run lint:backend && bun run lint.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • My change has adequate E2E test coverage.
  • I have tested my code manually, and I have provided steps how to reproduce my tests.

Created with PostHog Desktop from this inbox report.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Review in cubic

The compatibility acceptance flow had two dead-click traps and one silent write:

- Accept dialog: the primary button was always enabled but the handler bailed
  with a toast when the reason was empty. Make the reason required — disable the
  button until it has content, mark and autofocus the field, and show the
  validation error inline instead of as a toast that appears elsewhere.
- Dashboard banners (CompatibilityBanner, ReleaseBanner): only the small CTA
  button was clickable; the card itself was inert. Make the whole card the click
  target (cursor-pointer, hover, keyboard-operable) with the CTA as the visual
  affordance.
- acknowledge_compatibility_event RPC returned silently on an RBAC denial, so
  the client showed a success toast for a write that never happened. Raise
  instead. Unknown-id and permission-denied raise the SAME error on purpose, so
  the SECURITY DEFINER RPC does not become an existence oracle for callers
  outside the org.

Generated-By: PostHog Code
Task-Id: dd09eec1-0332-4255-a63a-54b9b37c8d7d
SonarCloud flagged role="button" on the banner divs (Web:S6819). Make the card
a native <button> and demote the inner CTA to a visual-only <span>, which both
resolves the a11y smell and avoids nesting an interactive element inside another.
Keyboard operation and focus ring now come from the native button.

Generated-By: PostHog Code
Task-Id: dd09eec1-0332-4255-a63a-54b9b37c8d7d
@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants