Skip to content
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

[Bug]: Migration fails with extension version 12.7.x #29970

Closed
racitores opened this issue Jan 29, 2025 · 1 comment · Fixed by #29968 or #30010
Closed

[Bug]: Migration fails with extension version 12.7.x #29970

racitores opened this issue Jan 29, 2025 · 1 comment · Fixed by #29968 or #30010
Assignees
Labels
regression-RC-12.11.0 Regression bug that was found in release candidate (RC) for release 12.11.0 release-12.12.0 Issue or pull request that will be included in release 12.12.0 team-transactions Transactions team type-bug Something isn't working

Comments

@racitores
Copy link
Contributor

Describe the bug

Migration fails when the origin extension is version 12.7.x (tested 1 and 2)
In this cases, the alert banner is displayed in the confirmation screen (e.g: send and swaps) when the parameter STX in settings was enabled in the older extension.

Expected behavior

The behaviour should be:
extension 12.11.0 < with STX activated -> update to 12.11.0 and no banner is displayed in the confirmation screen

Screenshots/Recordings

No response

Steps to reproduce

  1. Load 12.7.1
  2. Check STX is active
  3. Update with 12.11.0
  4. Check STX is active
  5. Go to swaps. No banner should be displayed

Error messages or log output

Detection stage

During release testing

Version

12.11.0

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

Same behaviour in Firefox

Severity

sev2

@racitores racitores added the type-bug Something isn't working label Jan 29, 2025
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Jan 29, 2025
@racitores racitores added the team-transactions Transactions team label Jan 29, 2025
@metamaskbot metamaskbot added the regression-RC-12.11.0 Regression bug that was found in release candidate (RC) for release 12.11.0 label Jan 29, 2025
github-merge-queue bot pushed a commit that referenced this issue Jan 30, 2025
## **Description**

Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x
Default
Migration](#28854)

Update migration to treat null preference value as an existing enabled
state, preventing unneeded banner display for v12.6.0-12.7.2 migrations.

The update removes the `null` check in the migration's `transformState`
function (near line 46 of `135.ts`)

```ts
  if (
    currentOptInStatus === undefined ||
    currentOptInStatus === null ||  // remove this line
    (currentOptInStatus === false && !hasExistingSmartTransactions(state))
  ) {
  ```

And let the condition fall through to the else block. This will ensure that `smartTransactionsMigrationApplied: false` and ensure that we don't see a banner on Transaction Confirmation.

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29968?quickstart=1)

## **Related issues**

Fixes: #29970  

**Description**
Migration fails when the origin extension is version 12.7.x (tested 1 and 2)
In this cases, the alert banner is displayed in the confirmation screen (e.g: send and swaps) when the parameter STX in settings was enabled in the older extension.

**Expected behavior**
The behaviour should be:
extension 12.11.0 < with STX activated -> update to 12.11.0 and no banner is displayed in the confirmation screen

## **Manual testing steps for fix**

1. Migrate from any version 12.6.* to 12.7.* to 12.10.1 (or 12.11.0 depending) 
2. Ensure that STX is ON in the old version (before migrating to the newer version)
3. After migration try to Swap or Send on Ethereum Mainnet  & Sepolia and ensure there is no STX Banner Alert shown

## **Screenshots/Recordings**

For users with STX ON before and after Migration:

### **Before**

<img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" />

### **After**

<img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" />

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable
- [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
- [x] 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.
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Jan 30, 2025
@metamaskbot metamaskbot added the release-12.12.0 Issue or pull request that will be included in release 12.12.0 label Jan 30, 2025
httpJunkie added a commit that referenced this issue Jan 30, 2025
## **Description**

Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x
Default
Migration](#28854)

Update migration to treat null preference value as an existing enabled
state, preventing unneeded banner display for v12.6.0-12.7.2 migrations.

The update removes the `null` check in the migration's `transformState`
function (near line 46 of `135.ts`)

```ts
  if (
    currentOptInStatus === undefined ||
    currentOptInStatus === null ||  // remove this line
    (currentOptInStatus === false && !hasExistingSmartTransactions(state))
  ) {
  ```

And let the condition fall through to the else block. This will ensure that `smartTransactionsMigrationApplied: false` and ensure that we don't see a banner on Transaction Confirmation.

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29968?quickstart=1)

## **Related issues**

Fixes: #29970  

**Description**
Migration fails when the origin extension is version 12.7.x (tested 1 and 2)
In this cases, the alert banner is displayed in the confirmation screen (e.g: send and swaps) when the parameter STX in settings was enabled in the older extension.

**Expected behavior**
The behaviour should be:
extension 12.11.0 < with STX activated -> update to 12.11.0 and no banner is displayed in the confirmation screen

## **Manual testing steps for fix**

1. Migrate from any version 12.6.* to 12.7.* to 12.10.1 (or 12.11.0 depending) 
2. Ensure that STX is ON in the old version (before migrating to the newer version)
3. After migration try to Swap or Send on Ethereum Mainnet  & Sepolia and ensure there is no STX Banner Alert shown

## **Screenshots/Recordings**

For users with STX ON before and after Migration:

### **Before**

<img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" />

### **After**

<img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" />

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable
- [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
- [x] 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.
dbrans pushed a commit that referenced this issue Jan 30, 2025
… state (#30010)

cherry-picks #29968

## **Description**

Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x
Default
Migration](#28854)

Update migration to treat null preference value as an existing enabled
state, preventing unneeded banner display for v12.6.0-12.7.2 migrations.

The update removes the `null` check in the migration's `transformState`
function (near line 46 of `135.ts`)

```ts
  if (
    currentOptInStatus === undefined ||
    currentOptInStatus === null ||  // remove this line
    (currentOptInStatus === false && !hasExistingSmartTransactions(state))
  ) {
  ```

And let the condition fall through to the else block. This will ensure that `smartTransactionsMigrationApplied: false` and ensure that we don't see a banner on Transaction Confirmation.

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29968?quickstart=1)

## **Related issues**

Fixes: #29970  

**Description**
Migration fails when the origin extension is version 12.7.x (tested 1 and 2)
In this cases, the alert banner is displayed in the confirmation screen (e.g: send and swaps) when the parameter STX in settings was enabled in the older extension.

**Expected behavior**
The behaviour should be:
extension 12.11.0 < with STX activated -> update to 12.11.0 and no banner is displayed in the confirmation screen

## **Manual testing steps for fix**

1. Migrate from any version 12.6.* to 12.7.* to 12.10.1 (or 12.11.0 depending) 
2. Ensure that STX is ON in the old version (before migrating to the newer version)
3. After migration try to Swap or Send on Ethereum Mainnet  & Sepolia and ensure there is no STX Banner Alert shown

## **Screenshots/Recordings**

For users with STX ON before and after Migration:

### **Before**

<img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" />

### **After**

<img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" />

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable
- [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
- [x] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-RC-12.11.0 Regression bug that was found in release candidate (RC) for release 12.11.0 release-12.12.0 Issue or pull request that will be included in release 12.12.0 team-transactions Transactions team type-bug Something isn't working
Projects
Archived in project
4 participants
@httpJunkie @racitores @metamaskbot and others