Skip to content

Feat: Existing permissions banner with list#288

Merged
mj-kiwi merged 12 commits intomainfrom
feat/existing-permissions-banner-with-list
Mar 23, 2026
Merged

Feat: Existing permissions banner with list#288
mj-kiwi merged 12 commits intomainfrom
feat/existing-permissions-banner-with-list

Conversation

@mj-kiwi
Copy link
Contributor

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

Description

This PR improves the permission confirmation experience when a dApp origin already has active (non-revoked) granted permissions stored in profile sync.

User-facing behavior

  • Shows an informational or warning banner when existing permissions are detected, with copy that distinguishes similar (same broad category: stream vs periodic) vs dissimilar permissions.
  • Adds a “Review them” action that swaps the confirmation UI to a read-only list of existing permissions (grouped by account, with formatted token amounts where applicable) and a Continue control to return to the normal grant flow.

Implementation notes

  • Introduces ExistingPermissionsService and supporting UI (existingPermissionsContent, PermissionCard, permissionFormatter) integrated via PermissionRequestLifecycleOrchestrator and PermissionHandler (event handlers for show/hide existing-permissions views).
  • Prefetches existing-permissions status in parallel with the introduction flow to avoid blocking the first paint of confirmation content.
  • Follow-up hardening: optional showExistingPermissions on BaseContext (avoids requiring every buildContext to set it); suffix-based stream/periodic categorization (-stream / -periodic); structured logger usage on storage failures and debug logging when token metadata formatting fails; aligned extractDescriptorName import with the rest of the snap; removed unused constructor dependency; fixed invalid barrel type export; expanded unit tests (existingPermissionsService, permissionFormatter).

Related issues

Fixes:

Manual testing steps

  1. yarn prepare:snap (if needed), yarn build, then yarn start and open the dev site (e.g. http://localhost:8000) with MetaMask Flask and the local snaps (kernel / gator) connected.
  2. Grant a permission to the test origin for a stream- or periodic-type permission, then trigger another permission request from the same origin:
    • Confirm a banner appears (warning vs info depending on similar vs dissimilar case).
  3. Click “Review them” and verify the existing permissions list renders (accounts, chains, amounts/labels as expected); click Continue and confirm you return to the standard confirmation UI.
  4. Cancel / complete the flow and confirm no crashes or unhandled rejections when dismissing during intro (existing-permissions status promise should not reject the lifecycle).

Screenshots/Recordings

Before

After

Screen.Recording.2026-03-23.at.3.58.00.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
Changes the permission confirmation UI flow and orchestration (including async updates and button handling), which could impact users’ ability to grant/cancel requests if edge cases regress. Data/security impact is limited since it only reads stored grants and adjusts UI/state.

Overview
Adds an existing-permissions banner + review subview to the permission confirmation experience: when stored non-revoked grants exist for the requesting origin, the confirmation screen shows an info/warning banner (similar vs dissimilar stream/periodic) with a Review them action that swaps to a read-only, account-grouped list of existing permissions and a Back to request control.

Refactors confirmation rendering so the footer grant/cancel buttons live inside PermissionHandlerContent (not ConfirmationDialog), and updates the orchestrator to prefetch a single profile-sync snapshot for both banner state and list rendering, with skeleton + fallback UI and improved logging/error handling. Updates permission formatting/display (PermissionCard, permissionFormatter) to support structured labeled fields and token-amount formatting, and expands tests to cover status classification, formatting behavior, and failure paths.

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

mj-kiwi added 3 commits March 20, 2026 14:09
- Updated mock implementations for ExistingPermissionsService to include methods for getting existing permissions status and creating existing permissions content.
- Set default mock return values for existing permissions service methods to improve test reliability.
- Adjusted tests to utilize the new existing permissions status in the PermissionRequestLifecycleOrchestrator tests.
@mj-kiwi mj-kiwi marked this pull request as ready for review March 22, 2026 22:24
@mj-kiwi mj-kiwi requested a review from a team as a code owner March 22, 2026 22:24
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.

Fix All in Cursor

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

@mj-kiwi mj-kiwi changed the title Feat/existing permissions banner with list Feat: Existing permissions banner with list Mar 22, 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 great!

I think it would be good if we could reduce the refetches of existing granted permissions, as they shouldn't ever change during the lifecycle of a permissions request.

mj-kiwi and others added 5 commits March 23, 2026 14:48
…xistingPermissionsContent.tsx

Co-authored-by: jeffsmale90 <6363749+jeffsmale90@users.noreply.github.com>
…xistingPermissionsService.ts

Co-authored-by: jeffsmale90 <6363749+jeffsmale90@users.noreply.github.com>
@mj-kiwi mj-kiwi requested a review from jeffsmale90 March 23, 2026 03:40
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.

lgtm!

@mj-kiwi mj-kiwi merged commit af92586 into main Mar 23, 2026
16 checks passed
@mj-kiwi mj-kiwi deleted the feat/existing-permissions-banner-with-list branch March 23, 2026 03:50
@cursor cursor bot mentioned this pull request Mar 23, 2026
6 tasks
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