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

Pubs which are already in a-c table are getting deleted on upgraded profile after revisiting #3162

Closed
GeetaSarvadnya opened this issue Jan 30, 2019 · 12 comments · Fixed by brave/brave-core#1514

Comments

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jan 30, 2019

Description

Follow up of #3134
Pubs which are already in a-c table are getting deleted on upgraded profile after revisiting

Steps to Reproduce

  1. Clean profile 0.58.21
  2. Enable rewards and restore wallet
  3. Change the Rewards Default settings ( Set the min page time to 5secs)
  4. Add few sites to a-c table ( I have added 5 verified+nonverified sites)
  5. Tip a verified publisher ( I have tipped only one site)
  6. Make sure all the sites are added to a-c table
  7. Upgrade profile to 0.59.32
  8. Revisit the pubs which are already in a-c table ( step 4)

Actual result:

when revisiting publishers that are already in the table, publishers are getting removed from a-c table.

Expected result:

pubs which are in a-c table should not get deleted after revisit

Reproduces how often:

Easy

Brave version (brave://version info)

Brave 0.59.32 Chromium: 72.0.3626.81 (Official Build) (64-bit)
Revision ac8b982e05014492d1bd7d317628a4f22a97ffa0-refs/branch-heads/3626@{#796}
OS Windows 10

Reproducible on current release:

  • Does it reproduce on brave-browser dev/beta builds? no

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields? na
  • Is the issue reproducible on the latest version of Chrome? na

Additional Information

@NejcZdovc @brave/legacy_qa

@GeetaSarvadnya GeetaSarvadnya added this to the 1.x Backlog milestone Jan 30, 2019
@NejcZdovc
Copy link
Contributor

@GeetaSarvadnya can you please paste before and after ac table

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Jan 30, 2019

@NejcZdovc Please find the below screen shots:

Before:

UI:
image

publisher_info_db:

image

After:

UI:

image

publisher_info_db:
image

@LaurenWags
Copy link
Member

Reproduced on 0.58.21 --> 0.59.32 update on macOS. Here are my publisher_info_db tables
0.58.21:
screen shot 2019-01-30 at 10 10 38 am

0.59.32 (after visiting a site to trigger recalculation):
screen shot 2019-01-30 at 10 34 27 am

@LaurenWags
Copy link
Member

LaurenWags commented Jan 30, 2019

Steps:

  1. Install 0.58.21 and enable rewards. Change Auto-Contribute Settings default settings for page time (change from 8s to 5s)
  2. Visit 3 sites (clifton, ddg, google)
  3. Close and relaunch Brave
  4. Visit clifton --> a-c % does not change.
  5. Visit nytimes and cnn. They are added to a-c table with 0%.
  6. Visit publisher_info_db to see negative scores.
  7. Update to 0.59.32
  8. publisher_info_db still has negative scores.
  9. Visit a site in the a-c table (I used ddg) for at least min time.
  10. All % values recalculate but google is dropped from the table.

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Jan 30, 2019

@LaurenWags Able to reproduce the above steps on Windows 10 x64

Negative scores are stores in a-c table when we relaunch and add few more sites to a-c table in 0.58.21

image

@LaurenWags
Copy link
Member

Additionally, if you follow these modified steps (no change to settings) you get % values which are off after updating to 0.59.32:

  1. Install 0.58.21 and enable rewards. Do not change Auto-Contribute Settings default settings for page time (no change of 8s to 5s)
  2. Visit 3 sites (clifton, ddg, google)
  3. Close and relaunch Brave
  4. Visit clifton --> a-c % does change.
  5. Visit nytimes and cnn. They are added to a-c table with appropriate % values
  6. Visit publisher_info_db (no negative scores)
  7. Update to 0.59.32
  8. publisher_info_db still fine.
  9. Visit a site in the a-c table (I used ddg) for at least min time.
  10. All % values recalculate. DDG appears to have a way off % value.

0.58.21:
screen shot 2019-01-30 at 11 25 45 am

Update to 0.59.32 and visit DDG:
screen shot 2019-01-30 at 11 29 23 am

@LaurenWags
Copy link
Member

LaurenWags commented Jan 30, 2019

Verified passed with

Brave 0.59.33 Chromium: 72.0.3626.81 (Official Build) (64-bit)
Revision ac8b982e05014492d1bd7d317628a4f22a97ffa0-refs/branch-heads/3626@{#796}
OS Mac OS X

Verification passed on

Brave 0.59.33 Chromium: 72.0.3626.81 (Official Build) (64-bit)
Revision ac8b982e05014492d1bd7d317628a4f22a97ffa0-refs/branch-heads/3626@{#796}
OS Windows 10
  • Verified the STR mentioned in description

Verified passed with

Brave 0.59.33 Chromium: 72.0.3626.81 (Official Build) (64-bit)
Revision ac8b982e05014492d1bd7d317628a4f22a97ffa0-refs/branch-heads/3626@{#796}
OS Linux

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Jan 31, 2019

Verified comments from (#3162 (comment)).
verified a profile from 0.58.21 with issues described in #3134 is updated to 0.59.33. Below are my observations

  1. Rightafter update to 0.59.33, the sites (cnn and nytimes) added in step 5 got dropped? when i revisit any of the sites which are already added in a-c table (step2), the sites (cnn and nytimes) are getting re-added to a-c table. - FAIL
  2. If i re-visit the sites which are already added in a-c table, % value calculated correctly and weights displayed in DB are accurate - PASS
  3. If i add new site, sites are getting added with correct % attention value and and weights displayed in DB are accurate - PASS

@NejcZdovc can you please look into the 1st point, which seems wrong to me, correct me if i am wrong.

@LaurenWags
Copy link
Member

@NejcZdovc please correct if wrong, but I think what @GeetaSarvadnya is seeing is expected because she has not yet visited any sites and the code to correct the a-c table (scores, % values, and weights) does not run until you visit a site. (also, we're not seeing any 0% items in the a-c table in 0.59.x due to #2365)

@NejcZdovc
Copy link
Contributor

@LaurenWags you are correct and this is expected

@srirambv
Copy link
Contributor

@NejcZdovc shouldn't the recalculations happen automatically upon upgrade which basically fixes the issue? We shouldn't wait until the user visits a site for the recalculation to happen with this fix. Could this be fixed for 0.60.x ?

@NejcZdovc
Copy link
Contributor

@srirambv I think it doesn't matter as it will automatically be fixed when user do any action. So data will not be wrong for any operation that follows.

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

Successfully merging a pull request may close this issue.

6 participants