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

feat: Show link on blockaid banner to report false positives #7218

Merged
merged 2 commits into from
Oct 5, 2023
Merged

feat: Show link on blockaid banner to report false positives #7218

merged 2 commits into from
Oct 5, 2023

Conversation

segun
Copy link
Contributor

@segun segun commented Sep 13, 2023 •
edited

Description
We need to allow MetaMask users and developers to report false positives.

Following the guidelines here, we add a link to when clicked opens the MetaMask support page

Tasks

  • Add an option on the UI to report a false positive (see design). Clicking on the link should direct the users to customer support (link: https://support.metamask.io/hc/en-us - the link might change to an specific one for this feature)
  • Add new property to transactions and signatures events named external_link_clicked and pass the value security_alert_support_link to it whenever the user clicks on the above link within the warning.

Screenshots/Recordings

Before

Screenshot 2023-09-13 at 23 03 49 Screenshot 2023-09-13 at 23 03 57

After

Screenshot 2023-09-21 at 17 58 01 Screenshot 2023-09-21 at 17 58 43

Issue

See #1126

Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f4d0a5b4-5394-4d4d-b938-e39466cf3b2a

Checklist

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

@segun segun added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead team-confirmations-secure-ux-PR PR from the confirmations team labels Sep 13, 2023
@segun segun self-assigned this Sep 13, 2023
@segun segun requested a review from a team as a code owner September 13, 2023 22:30
@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2023 •
edited

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.

@segun segun force-pushed the olu/blockaid-false-positive-report branch from 342e3c1 to f4e7f19 Compare September 13, 2023 22:31
@codecov-commenter
Copy link

codecov-commenter commented Sep 13, 2023 •
edited

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (f92baae) 34.59% compared to head (36452cc) 34.59%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7218      +/-   ##
==========================================
- Coverage   34.59%   34.59%   -0.01%     
==========================================
  Files        1017     1017              
  Lines       27150    27159       +9     
  Branches     2211     2211              
==========================================
+ Hits         9393     9396       +3     
- Misses      17268    17276       +8     
+ Partials      489      487       -2
Files Coverage Δ
...ents/UI/BlockaidBanner/BlockaidBanner.constants.ts 100.00% <100.00%> (ø)
...ponents/UI/BlockaidBanner/BlockaidBanner.styles.ts 100.00% <ø> (ø)
...pp/components/UI/BlockaidBanner/BlockaidBanner.tsx 84.00% <100.00%> (ø)
...omponents/UI/BlockaidBanner/BlockaidBannerLink.tsx 100.00% <100.00%> (ø)
app/components/UI/MessageSign/MessageSign.tsx 68.29% <ø> (ø)
app/components/Views/Send/index.js 3.77% <ø> (+0.05%) ⬆️
app/constants/urls.ts 100.00% <100.00%> (ø)
app/util/confirmation/signatureUtils.js 96.42% <100.00%> (+7.53%) ⬆️
app/components/UI/PersonalSign/PersonalSign.tsx 74.50% <66.66%> (-0.50%) ⬇️
app/components/UI/TransactionReview/index.js 50.96% <83.33%> (+0.96%) ⬆️
... and 3 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@segun

This comment was marked as resolved.

Sign in to view
@segun segun force-pushed the olu/blockaid-false-positive-report branch 3 times, most recently from a41c2b9 to e985777 Compare September 20, 2023 14:11
@bschorchit
Copy link

Small UI comment @segun: the "Something doesn't look right? Contact us" should be within the details part and only show up when you expand it.

jpuri
jpuri reviewed Sep 21, 2023
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

Changes look good, but will be nice to have more test coverage in the PR.

@segun segun force-pushed the olu/blockaid-false-positive-report branch 2 times, most recently from 4cca52d to b0d46cd Compare September 21, 2023 17:57
@segun segun requested a review from jpuri September 22, 2023 12:33
jpuri
jpuri reviewed Sep 26, 2023
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

Changes looks good, I added some small feedbacks.

@segun segun force-pushed the olu/blockaid-false-positive-report branch 2 times, most recently from f30806d to 908ffab Compare September 28, 2023 18:28
@segun
Copy link
Contributor Author

segun commented Sep 28, 2023

I have read the CLA Document and I hereby sign the CLA

@segun segun force-pushed the olu/blockaid-false-positive-report branch from 7b713e0 to 1d4fac6 Compare September 28, 2023 21:43
@segun
Show link on blockaid banner to report false positives
36452cc 
move blockaid urls from en.json to constants/url

Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>

lint fixes

Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>

Part 2 of the PR...add an event when link clicked.

Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>

Move security alert something wrong link inside accordion details

Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>

Fixed snapshot

Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>

more unit tests to check something doesn't look right and link is showm.

Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>

more unit tests

Signed-off-by: Olusegun Akintayo <olusegun.akintayo@consensys.net>

fix snapshot

Remove blockaid from send page
@segun segun force-pushed the olu/blockaid-false-positive-report branch from 1d4fac6 to 36452cc Compare September 28, 2023 21:52
@sonarcloud
Copy link

sonarcloud bot commented Sep 29, 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 1 Code Smell

69.6% 69.6% Coverage
0.0% 0.0% Duplication

jpuri
jpuri approved these changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

LGTM

@segun segun merged commit e902c57 into main Oct 5, 2023
24 checks passed
@segun segun deleted the olu/blockaid-false-positive-report branch October 5, 2023 13:41
@github-actions github-actions bot locked and limited conversation to collaborators Oct 5, 2023
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Oct 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jpuri jpuri jpuri approved these changes

@blackdevelopa blackdevelopa blackdevelopa approved these changes

Assignees

@segun segun

Labels
release-7.10.0 team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead team-confirmations-secure-ux-PR PR from the confirmations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@segun @codecov-commenter @bschorchit @jpuri @blackdevelopa @metamaskbot