fix(compat): kill dead clicks on accept dialog and dashboard banners - #2894
Draft
posthog-eu[bot] wants to merge 2 commits into
Draft
fix(compat): kill dead clicks on accept dialog and dashboard banners#2894posthog-eu[bot] wants to merge 2 commits into
posthog-eu[bot] wants to merge 2 commits into
Conversation
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
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
Two dead-click traps and one silent write on the compatibility-acceptance journey.
Problem
CompatibilityBannerand the identically-shapedReleaseBanner("View adoption").acknowledge_compatibility_eventreturned silently on an RBAC denial, so users below release-manager got a success toast for a write that never happened.Changes
[app].observe.compatibility.vue)*+required+ autofocused, validation shown inline on the field instead of as a toast.CompatibilityBanner.vue,ReleaseBanner.vue)cursor-pointer, hover, keyboard-operable (role="button", Enter/Space); CTA stays as the visual affordance.acknowledge_compatibility_eventnowRAISEs 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_eventisSECURITY 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 identicalnot_authorized, preserving the no-existence-oracle property the previous no-op had while still surfacing the failure to the legitimate client.supabase/schemas/prod.sqlis treated as read-only reference and was not edited; the change lands as a CLI-generated migration.Test plan
bun run lintandbun run typecheck:frontend— pass.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 itsdata-testhook.compatibility-events.test.ts): non-member accept and unknown-id both now raisenot_authorizedand 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
bun run lint:backend && bun run lint.Created with PostHog Desktop from this inbox report.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.