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

Search result ad view/click confirmation. #16179

Merged
merged 4 commits into from
Jan 31, 2023
Merged

Search result ad view/click confirmation. #16179

merged 4 commits into from
Jan 31, 2023

Conversation

aseren
Copy link
Collaborator

@aseren aseren commented Nov 30, 2022 •
edited
Loading

Spec: https://docs.google.com/document/d/1ncuyw0Yv_qawR-FM0_e1r4pNU2pGDhS49KfwfHqqrqs/edit#heading=h.hnm67cu7xxpw

See logic description here: https://github.com/brave/security/issues/1128

Resolves brave/brave-browser#23002

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Should be tested here: brave/brave-browser#28252

@aseren aseren requested review from a team as code owners November 30, 2022 21:45
@aseren aseren changed the title Search result ad click confirmation. Search result ad view/click confirmation. Nov 30, 2022
This was referenced Dec 1, 2022
Closed
Closed
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch 4 times, most recently from 5343b04 to 370c020 Compare December 23, 2022 02:40
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from 370c020 to 2817dbe Compare January 6, 2023 16:58
@aseren aseren requested a review from bridiver January 6, 2023 16:58
@tmancey tmancey requested a review from diracdeltas January 16, 2023 18:21
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from 2817dbe to 837f56d Compare January 17, 2023 16:02
@aseren aseren requested a review from tmancey January 17, 2023 16:03
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from 837f56d to 691447a Compare January 18, 2023 20:32
@tmancey tmancey requested a review from diracdeltas January 24, 2023 19:33
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from 691447a to 85f1ac2 Compare January 24, 2023 20:09
@aseren aseren requested a review from goodov January 25, 2023 17:42
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from d81caf1 to cb7685e Compare January 26, 2023 03:32
@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from cb7685e to f29f134 Compare January 30, 2023 21:40
@aseren aseren requested a review from goodov January 30, 2023 21:42
tmancey
tmancey approved these changes Jan 31, 2023
View reviewed changes
Copy link
Collaborator

@tmancey tmancey left a comment

Choose a reason for hiding this comment

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

LGTM++

@aseren aseren force-pushed the issues/23002_search_result_ad_tab_helper branch from f29f134 to b32ae23 Compare January 31, 2023 17:39
@aseren aseren merged commit 83335a3 into master Jan 31, 2023
@aseren aseren deleted the issues/23002_search_result_ad_tab_helper branch January 31, 2023 20:42
@github-actions github-actions bot added this to the 1.49.x - Nightly milestone Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goodov goodov goodov approved these changes

@tmancey tmancey tmancey approved these changes

@bridiver bridiver Awaiting requested review from bridiver

@iefremov iefremov Awaiting requested review from iefremov

@darkdh darkdh Awaiting requested review from darkdh

@diracdeltas diracdeltas Awaiting requested review from diracdeltas

Assignees

@aseren aseren

Labels
None yet
Projects
None yet
Milestone
1.49.x - Release
Development

Successfully merging this pull request may close these issues.

Implement clicked confirmation for Brave Search Ads
5 participants
@aseren @bridiver @diracdeltas @tmancey @goodov