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

Auto-contribute list should only contain included publishers #1175

Closed
ryanml opened this issue Sep 18, 2018 · 6 comments · Fixed by brave/brave-core#475
Closed

Auto-contribute list should only contain included publishers #1175

ryanml opened this issue Sep 18, 2018 · 6 comments · Fixed by brave/brave-core#475

Comments

@ryanml
Copy link
Contributor

ryanml commented Sep 18, 2018

Test Plan

See brave/brave-core#475

Count of excluded publishers should be a part of the state, and not filtered for on the client side

@ryanml ryanml self-assigned this Sep 18, 2018
@bbondy bbondy added this to the 1.x Backlog milestone Sep 22, 2018
@rebron rebron added the priority/P2 A bad problem. We might uplift this to the next planned release. label Sep 28, 2018
@rebron
Copy link
Collaborator

rebron commented Sep 28, 2018

Is there a privacy issue with this one @diracdeltas

@diracdeltas
Copy link
Member

what is sent in the fetch? (and what is the response)

@ryanml ryanml changed the title Auto-contribute should only fetch for included publishers Auto-contribute list should only contain included publishers Sep 28, 2018
@ryanml
Copy link
Contributor Author

ryanml commented Sep 28, 2018

@rebron @diracdeltas sorry for the confusion, there was not a "fetch" involved with this one (no http requests/server calls). This was just an issue with the auto-contribute list grabbed from the native ledger containing both included/excluded publishers. I've updated the issue title.

cc: @NejcZdovc

@diracdeltas
Copy link
Member

@ryanml thanks for clarifying!

@NejcZdovc NejcZdovc modified the milestones: 1.x Backlog, Releasable builds 0.55.x Oct 1, 2018
@ryanml ryanml removed the priority/P2 A bad problem. We might uplift this to the next planned release. label Oct 1, 2018
NejcZdovc pushed a commit to ryanml/brave-core that referenced this issue Oct 2, 2018
Auto-Contribute list only contains included pubs by default
numExcludedSites is now part of the AppState and passed as a modalContribute prop
@kjozwiak
Copy link
Member

kjozwiak commented Oct 9, 2018

@ryanml is there anything QA can do here in terms of verification? There's not much information here in terms of cases/test plans that QA can follow. If there's nothing required here, can you please mark this as QA/No? If there's something that QA should be verifying/checking, can you please add some test cases and add the QA/Yes label. Thanks!

@srirambv
Copy link
Contributor

srirambv commented Oct 9, 2018

Verification Passed on

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta (64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Linux
  • Verified test plan on Linux VM and works great

Verification passed on

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) (64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Windows 7

Used test plan from brave/brave-core#475

Verified passed with

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) beta(64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Mac OS X

Used test plan from brave/brave-core#475

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