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.

Sign up for GitHub

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

Jump to bottom

fix: remove outdated ipfs gateways #6962

Merged
merged 11 commits into from
Aug 23, 2023
Merged

fix: remove outdated ipfs gateways #6962

merged 11 commits into from
Aug 23, 2023

Conversation

jpcloureiro
Copy link
Contributor

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add the appropiate QA label when dev review is completed
    • needs-qa: PR requires manual QA.
    • No QA/E2E only: PR does not require any manual QA effort. Prior to merging, ensure that you have successful end-to-end test runs in Bitrise.
    • Spot check on release build: PR does not require feature QA but needs non-automated verification. In the description section, provide test scenarios. Add screenshots, and or recordings of what was tested.
  5. Add QA Passed label when QA has signed off (Only required if the PR was labeled with needs-qa)
  6. Add your team's label, i.e. label starting with team- (or external-contributor label if your not a MetaMask employee)

Description

Remove outdated ipfs gateways from selectable list under advanced settings

migrate users from outdated ipfs gateways URLs to the default one

Issue

fixes https://github.com/MetaMask/mobile-planning/issues/1152

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@jpcloureiro jpcloureiro self-assigned this Aug 3, 2023
jpcloureiro
jpcloureiro commented Aug 14, 2023
View reviewed changes
app/store/migrations.js Outdated Show resolved Hide resolved
@jpcloureiro jpcloureiro marked this pull request as ready for review August 14, 2023 16:07
@jpcloureiro jpcloureiro requested a review from a team as a code owner August 14, 2023 16:07
@gauthierpetetin gauthierpetetin added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) and removed in-progress labels Aug 14, 2023
@Cal-L
Copy link
Contributor

Cal-L commented Aug 14, 2023

@jpcloureiro Could you provide videos demonstrating the last two scenarios in the acceptance criteria?

Cal-L
Cal-L reviewed Aug 14, 2023
View reviewed changes
app/store/migrations.js Outdated Show resolved Hide resolved
app/store/migrations.test.js Show resolved Hide resolved
app/store/migrations.js Show resolved Hide resolved
@jpcloureiro
migration: return same state if any obj prop is undefined
b1f2f23 
Make sure we return the same state if ...PreferencesController or
...PreferencesController.ipfsGateway happen to be undefined for some
reason
@jpcloureiro
Copy link
Contributor Author

@jpcloureiro Could you provide videos demonstrating the last two scenarios in the acceptance criteria?

Scenario: Upgrading from old version with offline gateway selected should migrate over to use https://cloudflare-ipfs.com/ipfs/ by default

https://recordit.co/nJruefTBl1

Scenario: Upgrading from old version without online gateway selected should persist selected IPFS gateway in upgraded app

https://recordit.co/OnMq5d33VF

They're also represented by the unit tests

@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2023 •
edited

Codecov Report

Merging #6962 (5ee1b2e) into main (1eed576) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #6962      +/-   ##
==========================================
+ Coverage   32.73%   32.75%   +0.01%     
==========================================
  Files         993      993              
  Lines       26614    26620       +6     
  Branches     2083     2083              
==========================================
+ Hits         8713     8719       +6     
  Misses      17487    17487              
  Partials      414      414
Files Changed Coverage Δ
app/constants/network.js 100.00% <100.00%> (ø)
app/store/migrations.js 11.90% <100.00%> (+2.70%) ⬆️

tommasini
tommasini previously approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

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

LGTM!

tommasini
tommasini approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

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

LGTM!

@Cal-L Cal-L added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Aug 23, 2023
@sonarcloud
Copy link

sonarcloud bot commented Aug 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Aug 23, 2023
cortisiko
cortisiko approved these changes Aug 23, 2023
View reviewed changes
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

✅ ✅

@Cal-L Cal-L added the release-7.7.0 Issue or pull request that will be included in release 7.7.0 label Aug 23, 2023
@Cal-L Cal-L merged commit b477985 into main Aug 23, 2023
13 of 14 checks passed
@Cal-L Cal-L deleted the cirt/170-ipfs-gateway branch August 23, 2023 23:33
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2023
@gauthierpetetin gauthierpetetin added team-mobile-platform team-mobile-ux DEPRECATED: please use "team-wallet-ux" label instead labels Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Cal-L Cal-L Cal-L left review comments

@cortisiko cortisiko cortisiko approved these changes

@tommasini tommasini tommasini approved these changes

Assignees

@jpcloureiro jpcloureiro

Labels
QA Passed A successful QA run through has been done release-7.7.0 Issue or pull request that will be included in release 7.7.0 team-mobile-platform team-mobile-ux DEPRECATED: please use "team-wallet-ux" label instead
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jpcloureiro @Cal-L @codecov-commenter @cortisiko @tommasini @gauthierpetetin