-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: Use AccountGroup metadata for sorting in core permission helpers #29295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jiexi
wants to merge
46
commits into
main
Choose a base branch
from
jl/use-account-group-metadata-for-sorting-in-core-permission
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
d8ad969
bump chain-agnostic-permission and multichain-api-middleware
jiexi c9a6288
pass in sortAccountIdsByLastSelected where necessary
jiexi 70593ba
Fix spec
jiexi 5d5cb42
add specs
jiexi 79e82bb
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi 604df80
yarn dedupe
jiexi d5e87f4
lint
jiexi 0368835
update comment
jiexi 7df2bce
add explicit error check to try catch in AccountGroupChange handler
jiexi e2185d9
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi ec887ba
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi 33bc472
fix spec
jiexi 45a0781
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi 6795aff
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi afdd175
refetch caveat value again
jiexi 397d716
sort using AccountGroup metadata
jiexi 8fe874a
lint
jiexi 079a44f
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi b8c5e46
Merge remote-tracking branch 'origin/jl/WAPI-1387/multichain-api-acco…
jiexi 3c05cfd
Revert "lint"
jiexi 9edb66d
Revert "sort using AccountGroup metadata"
jiexi 724e71e
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi 3203940
Reapply "sort using AccountGroup metadata"
jiexi 09e60f9
Reapply "lint"
jiexi 45f624c
remove setTimeout hack
jiexi 6c8aed9
lint
jiexi fd17a8d
Merge branch 'jl/WAPI-1387/multichain-api-accounts-ordering' into jl/…
jiexi 65154d1
Merge branch 'main' into jl/WAPI-1387/multichain-api-accounts-ordering
jiexi e7728c2
Merge branch 'jl/WAPI-1387/multichain-api-accounts-ordering' into jl/…
jiexi f0b2327
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi 5166b38
lint
jiexi 65c7973
Update app/core/Permissions/index.test.ts
jiexi b7c67b3
Update app/core/Permissions/index.test.ts
jiexi bbffdf9
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi 4055330
Merge remote-tracking branch 'origin/jl/use-account-group-metadata-fo…
jiexi 9f702c1
remove unused imports
jiexi 8ba0fcf
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi 97c4b45
lint
jiexi a9161a0
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi 2341564
fix spec
jiexi 93431ee
fix spec
jiexi a4df7df
lint
jiexi fd55c79
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi ab23364
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi 91f00b1
Merge branch 'main' into jl/use-account-group-metadata-for-sorting-in…
jiexi 94f075e
fix spec
jiexi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test name contradicts its actual assertion
Medium Severity
The test is named "does not call notifyCaipAuthorizationChange if the caveat is revoked before the setTimeout fires" but line 1830 asserts
expect(notifySpy).toHaveBeenCalledWith(mockCaveatValue), confirming it WAS called. Since thesetTimeoutwas removed and the function now fires synchronously, the assertion body is correct for the new behavior, but the test name claims the opposite — providing false documentation of the contract.Reviewed by Cursor Bugbot for commit 2341564. Configure here.