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.

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

Video contribution settings should not apply to non-video "media" sites #6034

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

zenparsing
Copy link
Collaborator

@zenparsing zenparsing commented Jul 8, 2020

Resolves brave/brave-browser#10546

This change replaces LedgerImpl::SaveMediaVisit with two new methods:

  • LedgerImpl::SaveVisit
  • LedgerImpl::SaveVideoVisit

Code in "media" that correspond to "video" sites use SaveVideoVisit while others use SaveVisit.

Additional changes:

  • Unused OnSaveMediaVisit callbacks have been replaced with empty lambdas.

Submitter Checklist:

Test Plan:

  • Open browser with a clean profile, pointed to staging.
  • Enable rewards.
  • Claim grant.
  • Navigate to brave://rewards.
  • Open auto-contribute settings and deselect "Allow contribution to videos"
  • Open a new tab and navigate to https://github.com/laurenwags.
  • Wait for AC minimum page time.
  • Open a new tab and navigate to https://vimeo.com/bravelaurenwags
  • Wait for AC minimum page time.
  • Activate the tab with the rewards page.
    • Verify that a Github site is listed in the AC sites table.
    • Verify that a Vimeo site is not listed in the AC sites table.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@zenparsing zenparsing marked this pull request as ready for review July 16, 2020 21:30
@zenparsing zenparsing requested review from a team and emerick and removed request for a team July 16, 2020 21:30
@zenparsing zenparsing self-assigned this Jul 16, 2020
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@zenparsing
Copy link
Collaborator Author

CI failed on unrelated windows signing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Desktop] GH not added to AC when "Allow contribution to videos" is unchecked
3 participants