feat: dismiss bogus permits from UI#456
feat: dismiss bogus permits from UI#456energypantry wants to merge 3 commits intoubiquity:developmentfrom
Conversation
📝 WalkthroughWalkthroughAdds a beneficiary-triggered permit dismissal flow: a new "Dismiss" button is rendered on permit rows for connected beneficiaries who haven't claimed and are not currently claiming. Clicking confirms and invokes a new onDismissPermit callback (threaded DashboardPage → PermitsTable → PermitRow) which marks the permit status as "Invalid" in the local cache. CSS for 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Friendly bump for review when you have a moment. This adds a If you'd prefer persisting invalidations (e.g., Supabase-backed) instead of local-only, I'm happy to adjust the approach. |
|
@gentlementlegen Quick follow-up: PR #456 for #455 is ready (1 commit; local-only Dismiss). If this approach looks OK, could you please review/merge when you have a moment so the payout can proceed? Commit: 57f2057 |
|
This does not follow the spec, which states that permits should be invalidated, not just hidden from the UI. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/permit-row.tsx (2)
64-65:canDismissdoesn't gate ononDismissPermitbeing provided.The Dismiss button renders when
canDismissis true, but ifonDismissPermitisundefined, clicking it silently does nothing after the confirm dialog. Consider including the callback in the guard:- const canDismiss = Boolean(isConnected && isBeneficiary && !isClaimed && !isClaimingThis); + const canDismiss = Boolean(onDismissPermit && isConnected && isBeneficiary && !isClaimed && !isClaimingThis);
149-156:window.confirmis functional but not ideal for UX consistency.This works for an MVP. If the app has a modal/dialog system, consider migrating to it later for a consistent look-and-feel.
| const onDismissPermit = useCallback( | ||
| (permit: PermitData) => { | ||
| // Local-only dismissal: mark as invalid so it naturally gets filtered out from the UI. | ||
| updatePermitStatusCache(permit.signature, { status: "Invalid" }); | ||
| }, | ||
| [updatePermitStatusCache] | ||
| ); |
There was a problem hiding this comment.
Local-only dismissal — be aware of the persistence gap.
This stores the override only in browser storage. Clearing site data or switching browsers restores dismissed permits. The PR reviewer (@gentlementlegen) flagged that the spec expects server-side invalidation.
If local-only is intentional for now, consider adding a brief comment noting the limitation and linking to the follow-up plan.
|
@gentlementlegen Thanks for calling that out. I updated the implementation to match the #455 wording (“invalidate so it’s naturally filtered out”), instead of just hiding:
Commit: Local checks: |
|
Quick update: I pushed a follow-up fix so dismissed/bogus permits are treated as Head: Could you please take another look when you get a chance? |
|
Quick follow-up on the re-review request: I pushed the fix that marks dismissed permits as When you have a moment, could you please take another look and let me know if you’d prefer a different behavior (e.g. still display but visually de-emphasize)? Happy to adjust. |
|
Pushed the CodeRabbit-requested tweak.
|
|
Follow-up update on #456:
Local verification (run in repo):
@gentlementlegen Could you please take another look when you have a moment? If you want the confirm UX migrated from |
Resolves #455
Adds a Dismiss action for the connected beneficiary to hide unwanted/bogus permits from the pending list.
Implementation details:
{ status: "Claimed", isNonceUsed: true }so it naturally gets filtered outVerification: