Copy TypeScript version of PermissionLogController (from extension) along with tests into this repo #1826
Assignees
Labels
Comments
mcmire
changed the title
mcmire
changed the title
|
Hey team! Please add your planning poker estimate with Zenhub @cryptodev-2s @kanthesha @MajorLift @mcmire |
3 tasks
cryptodev-2s
added a commit
that referenced
this issue
Nov 7, 2023
## Description <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * What packages are you updating? * Are you introducing a breaking change to a package (renaming or removing a part of a public interface)? --> This PR adds the `permission-log-controller` from metamask-extension. ## Changes <!-- Pretend that you're updating a changelog. How would you categorize your changes? CATEGORY is one of: - BREAKING - ADDED - CHANGED - DEPRECATED - REMOVED - FIXED (Security-related changes should go through the Security Advisory process.) --> - **ADDED**: `permission-log-controller` controller ## References <!-- Are there any issues or other links that reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> * Fixes: #1826 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc) - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --------- Co-authored-by: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
This was referenced Feb 26, 2024
cryptodev-2s
added a commit
to MetaMask/metamask-extension
that referenced
this issue
Mar 1, 2024
…23182) ## **Description** Following the successful migration of PermissionLogController to the core monorepo (MetaMask/core#1826), this commit removes the redundant PermissionLogController logic from the extension. All future developments and maintenance will be concentrated on the core implementation to streamline efforts and enhance functionality coherence across the platform. [](https://codespaces.new/MetaMask/metamask-extension/pull/23182?quickstart=1) ## **Related issues** - Fixes: 23181 ## **Changes** The transition of this controller from the extension repo to the core monorepo unfolded in three phases: 1. The controller was integrated into Core, with more information available at MetaMask/core#1871 2. The logic of the controller was streamlined, with additional details at MetaMask/core#3662 3. The tests for the controller were overhauled, with further information at MetaMask/core#3937 ## **Manual testing steps** These instructions outline the process for conducting manual testing locally. 1. Launch the extension from the latest development branch. 2. Navigate to the [test-dapp](https://metamask.github.io/test-dapp/). 3. Initiate the REQUEST_PERMISSIONS action from the Permissions Actions menu. 4. Open the background.html inspect window. 5. Execute the script `chrome.storage.local.get(null, ({data}) => console.log(data.PermissionLogController))` in the console. 6. Record the output from the previous step. 7. Switch to the branch named `feature/23181-remove-Permissionlogcontroller`. 8. Repeat steps 2 through 6 for this branch. 9. Compare the outputs from step 6 for both the development and feature branches. Look for matching entries in `permissionHistory` and `permissionActivityLog` from the initial run in the second run's output. Note that the log history is limited to 100 entries. ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
](https://codespaces.new/MetaMask/metamask-extension/pull/23182?quickstart=1){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PermissionLogController, a controller in the extension, is written in JavaScript. We need to copy over the code, converting it to TypeScript along the way. We want to copy over the tests as well (taking care not to refactor them yet).
To decide: This controller can either be located in
@metamask/permission-controller(since it's related to permissions) or we can create a new package.The text was updated successfully, but these errors were encountered: