Skip to content

feat: enhance existing permissions handling #283

Open
mj-kiwi wants to merge 6 commits intomainfrom
feat/existing-permissions-enhancements
Open

feat: enhance existing permissions handling #283
mj-kiwi wants to merge 6 commits intomainfrom
feat/existing-permissions-enhancements

Conversation

@mj-kiwi
Copy link
Contributor

@mj-kiwi mj-kiwi commented Mar 11, 2026

Description

This PR enhances the existing permissions page to provide a better user experience and increase transparency. It implements three key improvements:

  1. Multi-chain Permission Display: Shows all chains granted to an origin, not just the requested chain. Users now see the full picture of their existing permissions across all networks.

  2. Account Grouping: Existing permissions are grouped by account address (CAIP-10 format) for better organization and clarity.

  3. Loading State with Skeleton UI: Adds a skeleton/loading state while permissions are being formatted. The dialog shows placeholder content immediately and updates with real data once formatting completes, providing visual feedback to users during loading.

  4. Permission Validation: Validates permissions before display to ensure data integrity. Invalid or malformed permissions are filtered out before rendering.

Additionally, permission request validation now occurs earlier in the request lifecycle (before showing existing permissions dialog) to catch invalid requests immediately.

Related issues

Fixes: N/A

Manual testing steps

Screenshots/Recordings

Before

After

Screen.Recording.2026-03-12.at.1.17.03.PM.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches permission-request flow and existing-permissions rendering, including earlier request validation and changing which stored permissions are surfaced (now all chains for an origin). Also adds caching to TokenMetadataService, which can affect display correctness if cache keys/invalidations are wrong.

Overview
Improves the existing permissions dialog by showing an immediate skeleton state (buildExistingPermissionsSkeletonContent) and then swapping in the fully formatted content once token metadata formatting completes.

Changes existing-permissions lookup to return all non-revoked permissions for a given origin across all chains (instead of filtering by requested chainId), filters out malformed stored permissions before rendering, and adjusts grouping/display to key by from address (Hex) plus UTC rendering for stream startTime.

Adds TokenMetadataService.getTokenMetadata() with an in-memory per-chainId/asset cache and updates permission formatting to use cached metadata and format both maxAmount and periodAmount. Includes new/expanded Jest coverage for ExistingPermissionsService and token metadata caching.

Written by Cursor Bugbot for commit 72a6533. This will update automatically on new commits. Configure here.

@mj-kiwi mj-kiwi marked this pull request as ready for review March 12, 2026 00:18
@mj-kiwi mj-kiwi requested a review from a team as a code owner March 12, 2026 00:18
jeffsmale90
jeffsmale90 previously approved these changes Mar 12, 2026
Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I think we don't need to memoize rn - wdyt?

permissionData: permissionRequest.permission.data,
});

// Validate the permission request early, before showing any UI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

} catch {
// If formatting fails, dialog still shows with skeleton
// This is acceptable - user can still interact with the dialog
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting failure permanently shows disabled skeleton button

Medium Severity

If the try block (formatting or dialogInterface.show(actualContent)) throws, the catch silently swallows the error and leaves the skeleton UI visible. The skeleton's confirm button has disabled={true}, so even though the handler is registered afterward at line 167, the user can never click it. The user is stuck on a permanent loading state with no error feedback — only able to dismiss via the X button. The comment "user can still interact with the dialog" is misleading since the only possible interaction is cancellation.

Additional Locations (1)
Fix in Cursor Fix in Web

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.

2 participants