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/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 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 }], + }, }); }; 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 } 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/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": { diff --git a/wdio/features/Confirmations/SendEthMultisig.feature b/wdio/features/Confirmations/SendEthMultisig.feature index f9cc6b8a17c..6cac21c176f 100644 --- a/wdio/features/Confirmations/SendEthMultisig.feature +++ b/wdio/features/Confirmations/SendEthMultisig.feature @@ -3,23 +3,23 @@ @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 + 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