Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fixed toggle excluding a site from ledger sometimes removes it from the list #7987

Closed
diracdeltas opened this issue Mar 30, 2017 · 2 comments

Comments

@diracdeltas
Copy link
Member

diracdeltas commented Mar 30, 2017

Test Plan

#8066 (comment)

  • enable payments
  • visit site for at least 8 seconds or until it appears in the synopsis table
  • set minimum visits to 5 instead of 1
  • site shouldn't be in the ledger table

Original issue description

mac os 0.14 and latest master

steps to repro (starting from a clean profile):

  1. enable payments
  2. visit nytimes.com for at least 8 seconds or until it appears in the synopsis table
  3. set minimum visits to 5 instead of 1
  4. exclude nytimes.com by toggling the switch

result:
nytimes.com suddenly disappears from the list when the switch is toggled

expected result:
when minimum visits is set to N, all sites with less than N visits should disappear immediately. the current behavior is very confusing because toggling the exclude switch should not remove the site from the list.

a similar issue exists for increasing the minimum time. if i increase it from 8 seconds to 1 minute, a site with 9 seconds remains in the ledger table. however if i then toggle the include switch on that site, it doesn’t disappear from the list, so the problem isn't as bad.

@diracdeltas diracdeltas changed the title toggle excluding a site from ledger removes it from the list toggle excluding a site from ledger sometimes removes it from the list Apr 1, 2017
@diracdeltas diracdeltas added this to the 0.14.1 milestone Apr 1, 2017
@alexwykoff alexwykoff modified the milestones: 0.14.2, 0.14.1 Apr 1, 2017
@diracdeltas
Copy link
Member Author

from slack:

just noticed a more general problem, which is that the ledger table is adding sites that have less than the required number of views. i set the minimum to 5 visits, went to https://www.kefairport.is/, and it showed up on the list after only 1 view

@bsclifton
Copy link
Member

bsclifton commented Apr 4, 2017

Assigning @NejcZdovc as he has looked into this issue the most (investigation still in progress)

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Apr 4, 2017
Resolves brave#7987

Auditors: @mrose17 @diracdeltas

Test Plan:
- enable payments
- visit site for at least 8 seconds or until it appears in the synopsis table
- set minimum visits to 5 instead of 1
- site shouldn't be in the ledger table
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Apr 4, 2017
Resolves brave#7987

Auditors: @mrose17 @diracdeltas

Test Plan:
- enable payments
- visit site for at least 8 seconds or until it appears in the synopsis table
- set minimum visits to 5 instead of 1
- site shouldn't be in the ledger table
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Apr 4, 2017
Resolves brave#7987

Auditors: @mrose17 @diracdeltas

Test Plan:
- enable payments
- visit site for at least 8 seconds or until it appears in the synopsis table
- set minimum visits to 5 instead of 1
- site shouldn't be in the ledger table
diracdeltas added a commit that referenced this issue Apr 5, 2017
Fixes usage of ledger advance settings in the ledger table
@alexwykoff alexwykoff changed the title toggle excluding a site from ledger sometimes removes it from the list Fixed toggle excluding a site from ledger sometimes removes it from the list Apr 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.