From c2de64e659d8df80d2d4a1447253a34cb246c5c3 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam <45455812+vinistevam@users.noreply.github.com> Date: Tue, 1 Aug 2023 20:19:28 +0100 Subject: [PATCH 1/6] remove onBackdropPress for signatures (#6936) --- app/components/UI/SignatureRequest/Root/Root.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/components/UI/SignatureRequest/Root/Root.tsx b/app/components/UI/SignatureRequest/Root/Root.tsx index 8e617c08d26..865001b80a3 100644 --- a/app/components/UI/SignatureRequest/Root/Root.tsx +++ b/app/components/UI/SignatureRequest/Root/Root.tsx @@ -52,7 +52,6 @@ const Root = ({ backdropOpacity={1} animationInTiming={600} animationOutTiming={600} - onBackdropPress={onSignReject} onBackButtonPress={ showExpandedMessage ? toggleExpandedMessage : onSignReject } From d396b2050d4ab79057b57682ccde668206fa0139 Mon Sep 17 00:00:00 2001 From: sleepytanya <104780023+sleepytanya@users.noreply.github.com> Date: Tue, 1 Aug 2023 22:38:29 -0400 Subject: [PATCH 2/6] feat: Migrate wdio 'SendEthMultisig' test to Detox (#6934) --- e2e/pages/TabBarComponent.js | 5 ++ .../confirmations/send-eth-multisig.spec.js | 60 +++++++++++++++++++ .../Confirmations/SendEthMultisig.feature | 25 -------- 3 files changed, 65 insertions(+), 25 deletions(-) create mode 100644 e2e/specs/confirmations/send-eth-multisig.spec.js delete mode 100644 wdio/features/Confirmations/SendEthMultisig.feature diff --git a/e2e/pages/TabBarComponent.js b/e2e/pages/TabBarComponent.js index 13e5ad6ffff..96f83e425f1 100644 --- a/e2e/pages/TabBarComponent.js +++ b/e2e/pages/TabBarComponent.js @@ -4,6 +4,7 @@ import { TAB_BAR_BROWSER_BUTTON, TAB_BAR_SETTING_BUTTON, TAB_BAR_WALLET_BUTTON, + TAB_BAR_ACTIVITY_BUTTON, } from '../../wdio/screen-objects/testIDs/Components/TabBar.testIds'; export default class TabBarComponent { @@ -25,4 +26,8 @@ export default class TabBarComponent { static async tapSettings() { await TestHelpers.waitAndTap(TAB_BAR_SETTING_BUTTON); } + + static async tapActivity() { + await TestHelpers.waitAndTap(TAB_BAR_ACTIVITY_BUTTON); + } } diff --git a/e2e/specs/confirmations/send-eth-multisig.spec.js b/e2e/specs/confirmations/send-eth-multisig.spec.js new file mode 100644 index 00000000000..1b4610288cd --- /dev/null +++ b/e2e/specs/confirmations/send-eth-multisig.spec.js @@ -0,0 +1,60 @@ +'use strict'; + +import { Regression } from '../../tags'; +import TestHelpers from '../../helpers'; + +import AmountView from '../../pages/AmountView'; +import SendView from '../../pages/SendView'; +import TransactionConfirmationView from '../../pages/TransactionConfirmView'; +import { + importWalletWithRecoveryPhrase, + addLocalhostNetwork, +} from '../../viewHelper'; +import TabBarComponent from '../../pages/TabBarComponent'; +import WalletActionsModal from '../../pages/modals/WalletActionsModal'; +import Accounts from '../../../wdio/helpers/Accounts'; +import Ganache from '../../../app/util/test/ganache'; +import root from '../../../locales/languages/en.json'; + +const validAccount = Accounts.getValidAccount(); +const MULTISIG_ADDRESS = '0x0C1DD822d1Ddf78b0b702df7BF9fD0991D6255A1'; +const AMOUNT_TO_SEND = '0.12345'; +const TOKEN_NAME = root.unit.eth; + +describe(Regression('Send tests'), () => { + let ganacheServer; + beforeAll(async () => { + jest.setTimeout(2500000); + if (device.getPlatform() === 'android') { + await device.reverseTcpPort('8081'); // because on android we need to expose the localhost ports to run ganache + await device.reverseTcpPort('8545'); + } + ganacheServer = new Ganache(); + await ganacheServer.start({ mnemonic: validAccount.seedPhrase }); + }); + + afterAll(async () => { + await ganacheServer.quit(); + }); + + it('Send ETH to a Multisig address from inside MetaMask wallet', async () => { + await importWalletWithRecoveryPhrase(); + await addLocalhostNetwork(); + + await TabBarComponent.tapActions(); + await WalletActionsModal.tapSendButton(); + + await SendView.inputAddress(MULTISIG_ADDRESS); + await SendView.tapNextButton(); + + await AmountView.typeInTransactionAmount(AMOUNT_TO_SEND); + await AmountView.tapNextButton(); + + await TransactionConfirmationView.tapConfirmButton(); + await TabBarComponent.tapActivity(); + + await TestHelpers.checkIfElementByTextIsVisible( + AMOUNT_TO_SEND + ' ' + TOKEN_NAME, + ); + }); +}); diff --git a/wdio/features/Confirmations/SendEthMultisig.feature b/wdio/features/Confirmations/SendEthMultisig.feature deleted file mode 100644 index f9cc6b8a17c..00000000000 --- a/wdio/features/Confirmations/SendEthMultisig.feature +++ /dev/null @@ -1,25 +0,0 @@ -@androidApp -@confirmations -@regression - -Feature: Send ETH to a Multisig - A user should be able to send ETH to a Multisig address. - @ganache - @multisig - Scenario: should successfully send ETH to a Multisig address from inside MetaMask wallet - Given I have imported my wallet - And I tap No Thanks on the Enable security check screen - And I tap No thanks on the onboarding welcome tutorial - And Ganache network is selected - When On the Main Wallet view I tap on the Send Action - And I enter address "MultisigAddress" in the sender's input box - When I tap button "Next" on Send To view - Then I proceed to the amount view - When I type amount "1" into amount input field - And I tap button "Next" on the Amount view - Then I should be taken to the transaction confirmation view - And the token amount 1 to be sent is visible - When I tap button "Send" on Confirm Amount view - Then I am on the main wallet view - When I tap on the Activity tab option - Then "Smart contract interaction" transaction is displayed \ No newline at end of file From 8c0959b9c384f2eb57e634ca85872bf029e615aa Mon Sep 17 00:00:00 2001 From: Gauthier Petetin Date: Wed, 2 Aug 2023 10:31:45 -0300 Subject: [PATCH 3/6] feat: github action to check if PR has requested labels before being merged (#6794) * feat(action): check if pr includes requested labels * fix(template): adjust PR temaple * fix(action): add missing QA label * fix(action): check if no label prevents merging * fix(action): remove QA label check * fix(action): add missing reopened condition * fix(action): increase list of labels which prevent merges * fix(action): add labeling guidelines * fix(action): typo * fix(action): typo --- .github/CONTRIBUTING.md | 1 + .../coding_guidelines/LABELING_GUIDELINES.md | 24 +++++ .github/pull_request_template.md | 1 + .../scripts/check-pr-has-required-labels.ts | 97 +++++++++++++++++++ .github/workflows/check-pr-labels.yml | 38 ++++++++ .github/workflows/ci-all-branches.yml | 17 ---- package.json | 3 +- 7 files changed, 163 insertions(+), 18 deletions(-) create mode 100644 .github/coding_guidelines/LABELING_GUIDELINES.md create mode 100644 .github/scripts/check-pr-has-required-labels.ts create mode 100644 .github/workflows/check-pr-labels.yml delete mode 100644 .github/workflows/ci-all-branches.yml diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 5c7b6e81a1f..8e0d2e6d515 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -21,5 +21,6 @@ When you're done with your project / bugfix / feature and ready to submit a PR, - [ ] **Keep it simple**: Try not to include multiple features in a single PR, and don't make extraneous changes outside the scope of your contribution. All those touched files make things harder to review ;) - [ ] **PR against `main`**: Submit your PR against the `main` branch. This is where we merge new features to be included in forthcoming releases. When we initiate a new release, we create a branch named `release/x.y.z`, serving as a snapshot of the `main` branch. This particular branch is utilized to construct the builds, which are then tested during the release regression testing phase before they are submitted to the stores for production. In the event your PR is a hot-fix for a bug identified on the `release/x.y.z` branch, you should still submit your PR against the `main` branch. This PR will subsequently be cherry-picked into the `release/x.y.z` branch by our release engineers. - [ ] **Get the PR reviewed by code owners**: At least two code owner approvals are mandatory before merging any PR. +- [ ] **Ensure the PR is correctly labeled.**: More detail about labels definitions can be found [here](https://github.com/MetaMask/metamask-mobile/blob/main/.github/coding_guidelines/LABELING_GUIDELINES.md). And that's it! Thanks for helping out. diff --git a/.github/coding_guidelines/LABELING_GUIDELINES.md b/.github/coding_guidelines/LABELING_GUIDELINES.md new file mode 100644 index 00000000000..fd9ade79e97 --- /dev/null +++ b/.github/coding_guidelines/LABELING_GUIDELINES.md @@ -0,0 +1,24 @@ +# PR Labeling Guidelines +To maintain a consistent and efficient development workflow, we have set specific label guidelines for all pull requests (PRs). Please ensure you adhere to the following instructions: + +### Mandatory Labels: +- **Internal Developers**: Every PR raised by an internal developer must include a label prefixed with `team-` (e.g., `team-mobile-ux`, `team-mobile-platform`, etc.). This indicates the respective internal team responsible for the PR. + +- **External Contributors**: PRs from contributors outside the organization must have the `external-contributor` label. + +It's essential to ensure that PRs have the appropriate labels before they are considered for merging. + +### Prohibited Labels: +Any PR that includes one of the following labels can not be merged: + +- **needs-qa**: The PR requires a full manual QA prior to being added to a release. +- **QA'd but questions**: The PR has been checked by QA, but there are pending questions or clarifications needed on minor issues that were found. +- **issues-found**: The PR has been checked by QA or other reviewers, and appeared to include issues that need to be addressed. +- **need-ux-ds-review**: The PR requires a review from the User Experience or Design System teams. +- **blocked**: There are unresolved dependencies or other issues blocking the progress of this PR. +- **stale**: The PR has not had recent activity in the last 90 days. It will be closed in 7 days. +- **DO-NOT-MERGE**: The PR should not be merged under any circumstances. + +To maintain code quality and project integrity, it's crucial to respect these label guidelines. Please ensure you review and update labels appropriately throughout the PR lifecycle. + +Thank you for your cooperation! diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index cef08a51ff4..b2524a54569 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -13,6 +13,7 @@ Please ensure that any applicable requirements below are satisfied before submit - `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** diff --git a/.github/scripts/check-pr-has-required-labels.ts b/.github/scripts/check-pr-has-required-labels.ts new file mode 100644 index 00000000000..88476e94f02 --- /dev/null +++ b/.github/scripts/check-pr-has-required-labels.ts @@ -0,0 +1,97 @@ +import * as core from '@actions/core'; +import { context, getOctokit } from '@actions/github'; +import { GitHub } from '@actions/github/lib/utils'; + +main().catch((error: Error): void => { + console.error(error); + process.exit(1); +}); + +async function main(): Promise { + // "GITHUB_TOKEN" is an automatically generated, repository-specific access token provided by GitHub Actions. + const githubToken = process.env.GITHUB_TOKEN; + if (!githubToken) { + core.setFailed('GITHUB_TOKEN not found'); + process.exit(1); + } + + // Initialise octokit, required to call Github GraphQL API + const octokit: InstanceType = getOctokit(githubToken); + + // Retrieve pull request info from context + const prRepoOwner = context.repo.owner; + const prRepoName = context.repo.repo; + const prNumber = context.payload.pull_request?.number; + if (!prNumber) { + core.setFailed('Pull request number not found'); + process.exit(1); + } + + // Retrieve pull request labels + const prLabels = await retrievePullRequestLabels(octokit, prRepoOwner, prRepoName, prNumber); + + const preventMergeLabels = ["needs-qa", "QA'd but questions", "issues-found", "need-ux-ds-review", "blocked", "stale", "DO-NOT-MERGE"]; + + let hasTeamLabel = false; + + // Check pull request has at least required QA label and team label + for (const label of prLabels) { + if (label.startsWith("team-") || label === "external-contributor") { + console.log(`PR contains a team label as expected: ${label}`); + hasTeamLabel = true; + } + if (preventMergeLabels.includes(label)) { + throw new Error(`PR cannot be merged because it still contains this label: ${label}`); + } + if (hasTeamLabel) { + return; + } + } + + // Otherwise, throw an arror to prevent from merging + let errorMessage = ''; + if (!hasTeamLabel) { + errorMessage += 'No team labels found on the PR. '; + } + errorMessage += `Please make sure the PR is appropriately labeled before merging it.\n\nSee labeling guidelines for more detail: https://github.com/MetaMask/metamask-mobile/blob/main/.github/coding_guidelines/LABELING_GUIDELINES.md`; + throw new Error(errorMessage); + +} + +// This function retrieves the pull request on a specific repo +async function retrievePullRequestLabels(octokit: InstanceType, repoOwner: string, repoName: string, prNumber: number): Promise { + + const retrievePullRequestLabelsQuery = ` + query RetrievePullRequestLabels($repoOwner: String!, $repoName: String!, $prNumber: Int!) { + repository(owner: $repoOwner, name: $repoName) { + pullRequest(number: $prNumber) { + labels(first: 100) { + nodes { + name + } + } + } + } + } + `; + + const retrievePullRequestLabelsResult: { + repository: { + pullRequest: { + labels: { + nodes: { + name: string; + }[]; + } + }; + }; + } = await octokit.graphql(retrievePullRequestLabelsQuery, { + repoOwner, + repoName, + prNumber, + }); + + const pullRequestLabels = retrievePullRequestLabelsResult?.repository?.pullRequest?.labels?.nodes?.map(labelObject => labelObject?.name); + + return pullRequestLabels || []; +} diff --git a/.github/workflows/check-pr-labels.yml b/.github/workflows/check-pr-labels.yml new file mode 100644 index 00000000000..47de292ca1b --- /dev/null +++ b/.github/workflows/check-pr-labels.yml @@ -0,0 +1,38 @@ +name: "Check PR has required labels" +on: + pull_request: + branches: + - main + types: + - opened + - reopened + - synchronize + - labeled + - unlabeled + +jobs: + check-pr-labels: + runs-on: ubuntu-latest + permissions: + pull-requests: read + + steps: + - name: Checkout repository + uses: actions/checkout@v3 + with: + fetch-depth: 0 # This is needed to checkout all branches + + - name: Set up Node.js + uses: actions/setup-node@v3 + with: + node-version-file: '.nvmrc' + cache: yarn + + - name: Install dependencies + run: yarn --immutable + + - name: Check PR has required labels + id: check-pr-has-required-labels + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: npm run check-pr-has-required-labels diff --git a/.github/workflows/ci-all-branches.yml b/.github/workflows/ci-all-branches.yml deleted file mode 100644 index 5592bfa7e84..00000000000 --- a/.github/workflows/ci-all-branches.yml +++ /dev/null @@ -1,17 +0,0 @@ -# Fails the pull request if it has the "DO-NOT-MERGE" label - -name: ci / check all branches - -on: - pull_request: - types: [opened, reopened, labeled, unlabeled, synchronize] - -jobs: - do-not-merge: - runs-on: ubuntu-latest - if: ${{ contains(github.event.pull_request.labels.*.name, 'DONOTMERGE') }} - steps: - - name: 'Check for label "DONOTMERGE"' - run: | - echo 'This check fails PRs with the "DONOTMERGE" label to block merging' - exit 1 diff --git a/package.json b/package.json index 3ad49bc163a..f383c8f3275 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,8 @@ "update-changelog": "./scripts/auto-changelog.sh", "prestorybook": "rnstl", "create-release": "./scripts/set-versions.sh && yarn update-changelog", - "add-release-label-to-pr-and-linked-issues": "ts-node ./.github/scripts/add-release-label-to-pr-and-linked-issues.ts" + "add-release-label-to-pr-and-linked-issues": "ts-node ./.github/scripts/add-release-label-to-pr-and-linked-issues.ts", + "check-pr-has-required-labels": "ts-node ./.github/scripts/check-pr-has-required-labels.ts" }, "husky": { "hooks": { From e426592e99fc90c2c1d595169546b8bfe60279a0 Mon Sep 17 00:00:00 2001 From: sleepytanya <104780023+sleepytanya@users.noreply.github.com> Date: Wed, 2 Aug 2023 13:14:09 -0400 Subject: [PATCH 4/6] fix: update README.md (#6853) --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1d00cbf4289..963563a9306 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ sudo gem install cocoapods -v 1.12.1 - Locate `NDK (Side-by-side)` option in the tools list - Check NDK version `21.4.7075529` - Locate `CMake` option in the tools list - - Check CMake version `3.10.2` + - Check CMake version `3.22.1` - Click "Apply" or "OK" to download - Linux only: - Ensure that you have the `secret-tool` binary on your machine. @@ -60,13 +60,14 @@ sudo gem install cocoapods -v 1.12.1 - Install the correct emulator - Follow the instructions at: - [React Native Getting Started - Android](https://reactnative.dev/docs/environment-setup#installing-dependencies) _(React Native CLI Quickstart -> [your OS] -> Android)_ + - FYI: as of today (7/18/23) there is currently an issue when running detox on android 12 and 13 (API 32/33) which prevents the tests from running. The issue is, the tap() action is treated like a tapAndHold() action. See the open issue in wix/detox [here](https://github.com/wix/Detox/issues/3762) - More details can be found [on the Android Developer site](https://developer.android.com/studio/run/emulator) - You should use the following: - **Android OS Version:** Latest, unless told otherwise - - **Device:** Google Pixel 3 + - **Device:** Google Pixel 5 - Finally, start the emulator from Android Studio: - Open "Virtual Device Manager" - - Launch emulator for "Pixel 3 " + - Launch emulator for "Pixel 5 " #### iOS From 9c35255507862cac44ad79832ff5972101f5a2fa Mon Sep 17 00:00:00 2001 From: sleepytanya <104780023+sleepytanya@users.noreply.github.com> Date: Wed, 2 Aug 2023 13:28:04 -0400 Subject: [PATCH 5/6] Restore deleted SendEthMultisig.feature file (#6956) --- .../Confirmations/SendEthMultisig.feature | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 wdio/features/Confirmations/SendEthMultisig.feature diff --git a/wdio/features/Confirmations/SendEthMultisig.feature b/wdio/features/Confirmations/SendEthMultisig.feature new file mode 100644 index 00000000000..6cac21c176f --- /dev/null +++ b/wdio/features/Confirmations/SendEthMultisig.feature @@ -0,0 +1,25 @@ +@androidApp +@confirmations +@regression + +Feature: Send ETH to a Multisig + A user should be able to send ETH to a Multisig address. + @ganache + @multisig + Scenario: should successfully send ETH to a Multisig address from inside MetaMask wallet + Given I have imported my wallet + And I tap No Thanks on the Enable security check screen + And I tap No thanks on the onboarding welcome tutorial + And Ganache network is selected + When On the Main Wallet view I tap on the Send Action + And I enter address "MultisigAddress" in the sender's input box + When I tap button "Next" on Send To view + Then I proceed to the amount view + When I type amount "1" into amount input field + And I tap button "Next" on the Amount view + Then I should be taken to the transaction confirmation view + And the token amount 1 to be sent is visible + When I tap button "Send" on Confirm Amount view + Then I am on the main wallet view + When I tap on the Activity tab option + Then "Smart contract interaction" transaction is displayed From 68c642c55a26c20a16eee37fce70d6e9fa213622 Mon Sep 17 00:00:00 2001 From: Brian August Nguyen Date: Wed, 2 Aug 2023 12:11:23 -0700 Subject: [PATCH 6/6] fix: custom position logic for badgeWrapper (#6864) * Fixed custom position logic for badgeWrapper * Fixed lint error --- .../BadgeWrapper/BadgeWrapper.styles.ts | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/app/component-library/components/Badges/BadgeWrapper/BadgeWrapper.styles.ts b/app/component-library/components/Badges/BadgeWrapper/BadgeWrapper.styles.ts index 0448a89ece8..59404ef31c9 100644 --- a/app/component-library/components/Badges/BadgeWrapper/BadgeWrapper.styles.ts +++ b/app/component-library/components/Badges/BadgeWrapper/BadgeWrapper.styles.ts @@ -27,6 +27,7 @@ const styleSheet = (params: { const { style, anchorElementShape, badgePosition, containerSize } = vars; let anchoringOffset, positionObj, xOffset, yOffset; const elementHeight = containerSize?.height || 0; + let isCustomPosition = false; switch (anchorElementShape) { case BadgeAnchorElementShape.Circular: @@ -75,6 +76,7 @@ const styleSheet = (params: { break; default: positionObj = badgePosition; + isCustomPosition = true; xOffset = 0; yOffset = 0; } @@ -84,16 +86,21 @@ const styleSheet = (params: { { position: 'relative', alignSelf: 'flex-start' } as ViewStyle, style, ) as ViewStyle, - badge: { - // This is needed to pass the anchor element's bounding box to the Badge. - position: 'absolute', - height: elementHeight, - aspectRatio: 1, - alignItems: 'center', - justifyContent: 'center', - ...positionObj, - transform: [{ translateX: xOffset }, { translateY: yOffset }], - }, + badge: isCustomPosition + ? { + position: 'absolute', + ...positionObj, + } + : { + // This is needed to pass the anchor element's bounding box to the Badge. + position: 'absolute', + height: elementHeight, + aspectRatio: 1, + alignItems: 'center', + justifyContent: 'center', + ...positionObj, + transform: [{ translateX: xOffset }, { translateY: yOffset }], + }, }); };