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

Publishers can now be included and excluded #591

Merged
merged 3 commits into from
Oct 23, 2018
Merged

Publishers can now be included and excluded #591

merged 3 commits into from
Oct 23, 2018

Conversation

jasonrsadler
Copy link
Contributor

@jasonrsadler jasonrsadler commented Oct 9, 2018

Resolves brave/brave-browser#1283

Ledger native brave-intl/bat-native-ledger#144

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. Start Brave
  2. Add publishers to rewards auto-contribution
  3. Visit an added publisher and open Rewards panel
  4. Click 'Include in auto-contribute'
  5. Check publisher_info_db and ensure that excluded was changed to 1
  6. Click 'Include in auto-contribute' in panel for same site
  7. Check publisher_info_db and ensure that excluded was changed to 2

Toggle switch currently does not slide and will be handled in future PR.
(Based on native and script type mismatches)

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

@jasonrsadler jasonrsadler changed the title wip - Include/Exclude publisher via panel Publishers can now be included and excluded in database Oct 16, 2018
@bbondy
Copy link
Member

bbondy commented Oct 16, 2018

needs a rebase

@jasonrsadler
Copy link
Contributor Author

Rebased.

@jasonrsadler jasonrsadler changed the title Publishers can now be included and excluded in database Publishers can now be included and excluded Oct 16, 2018
Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

../../brave/components/brave_rewards/browser/rewards_service_impl.h:113:8: error: 'brave_rewards::RewardsServiceImpl::SetContributionAutoInclude' hides overloaded virtual function [-Werror,-Woverloaded-virtual]
  void SetContributionAutoInclude(
       ^
../../brave/vendor/bat-native-ledger/include/bat/ledger/ledger_client.h:120:16: note: hidden overloaded virtual function 'ledger::LedgerClient::SetContributionAutoInclude' declared here: different number of parameters (3 vs 2)
  virtual void SetContributionAutoInclude(
               ^
../../brave/components/brave_rewards/browser/rewards_service_factory.cc:47:11: error: allocating an object of abstract class type 'brave_rewards::RewardsServiceImpl'
      new RewardsServiceImpl(Profile::FromBrowserContext(context)));
          ^
../../brave/vendor/bat-native-ledger/include/bat/ledger/ledger_client.h:120:16: note: unimplemented pure virtual method 'SetContributionAutoInclude' in 'RewardsServiceImpl'
  virtual void SetContributionAutoInclude(
               ^
2 errors generated.
[14/350] CXX obj/content/browser/browser/browser_context.o^C

It's not building for me

@jasonrsadler
Copy link
Contributor Author

brave-intl/bat-native-ledger#144 goes with it.

Jason Sadler and others added 3 commits October 23, 2018 09:03
wip - debugging

wip -

wip -

wip - include_switch

linting

clean up debugging

Brackets for reducer switch case

clean up debug lines

linting for CI
Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

will create follow up that will handle all cases in the panel brave/brave-browser#1797

@NejcZdovc NejcZdovc merged commit 831c60d into brave:master Oct 23, 2018
NejcZdovc added a commit that referenced this pull request Oct 23, 2018
Publishers can now be included and excluded
NejcZdovc added a commit that referenced this pull request Oct 23, 2018
Publishers can now be included and excluded
NejcZdovc added a commit that referenced this pull request Oct 23, 2018
Publishers can now be included and excluded
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Oct 23, 2018

master 831c60d
0.57.x 871cc0a
0.56.x 7a5ac9a
0.55.x aa8e7a6

NejcZdovc added a commit that referenced this pull request Oct 24, 2018
Publishers can now be included and excluded
@jasonrsadler jasonrsadler deleted the feature/panel_inc_exc_core branch November 7, 2018 23:39
@bbondy bbondy added this to the 0.55.x - Release milestone Jan 14, 2019
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.

Add exclude/include option to the panel
3 participants