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

Page data refactor #11575

Closed
NejcZdovc opened this issue Oct 17, 2017 · 9 comments
Closed

Page data refactor #11575

NejcZdovc opened this issue Oct 17, 2017 · 9 comments
Assignees
@NejcZdovc
Labels
feature/rewards QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/exclude
Milestone
0.19.x Hotfix 2 (...

Comments

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Oct 17, 2017 •
edited by luixxiul
Loading

Test plan

#11592 (comment)

Description

Refactor page data so that we handle about pages as well.
@NejcZdovc NejcZdovc self-assigned this Oct 17, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Oct 18, 2017
@NejcZdovc
Fixes publisher not added to the ledger
74e86f9 
Resolves brave#11575

Auditors:

Test Plan:
@NejcZdovc NejcZdovc mentioned this issue Oct 18, 2017
Merged
8 tasks
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Oct 19, 2017
@NejcZdovc
Fixes publisher not added to the ledger
0a22ce9 
Resolves brave#11575

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Oct 23, 2017
@NejcZdovc
Fixes publisher not added to the ledger
d7183f8 
Resolves brave#11575

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Oct 23, 2017
@NejcZdovc
Fixes publisher not added to the ledger
e17a494 
Resolves brave#11575

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Oct 23, 2017
@NejcZdovc
Fixes publisher not added to the ledger
07942a0 
Resolves brave#11575

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Oct 23, 2017
@NejcZdovc
Merge pull request #11592 from NejcZdovc/refactor/#11575-pageData
3cf6a50 
Fixes publisher not added to the ledger
NejcZdovc added a commit that referenced this issue Oct 23, 2017
@NejcZdovc
Merge pull request #11592 from NejcZdovc/refactor/#11575-pageData
2fa2f88 
Fixes publisher not added to the ledger
NejcZdovc added a commit that referenced this issue Oct 23, 2017
@NejcZdovc
Merge pull request #11592 from NejcZdovc/refactor/#11575-pageData
820ed5b 
Fixes publisher not added to the ledger
NejcZdovc added a commit that referenced this issue Oct 23, 2017
@NejcZdovc
Merge pull request #11592 from NejcZdovc/refactor/#11575-pageData
61c92a8 
Fixes publisher not added to the ledger
@kjozwiak
Copy link
Member

@NejcZdovc, is there anything QA can do here? Maybe load all the about: pages listed under about:about and ensure they're all being loaded/behaving correctly?

Can we remove the PR/pending-review label now that this has been closed? It looks like it's been reviewed by @bbondy in #11592.

@NejcZdovc
Copy link
Contributor Author

test plan is specified in #11592

This was referenced Oct 25, 2017
Closed
Closed
Closed
Closed
@LaurenWags
Copy link
Member

LaurenWags commented Oct 25, 2017 •
edited
Loading

Test Plan C from linked test plan does not produce the expected results:
final line of plan C says "switch to preference tab and new site should be added and old site should have 2 visits" but i'm seeing new site is added and old site has 1 visit (but the amount of time for that visit has increased)

cc @kjozwiak @srirambv

@LaurenWags LaurenWags reopened this Oct 25, 2017
@srirambv
Copy link
Collaborator

For plan C the view count is staying as 1 when the tab switch back to old site. Reloading the page and staying for another 15 sec just adds the time not the view. But opening the site in a new tab view count goes up to 2 and adds the time.

@NejcZdovc
Copy link
Contributor Author

@LaurenWags @srirambv I think this is the same problem as defined here #11633. Will close it for now. If you find anything else please create another issue for it.

@srirambv
Copy link
Collaborator

@NejcZdovc I think the steps are a little different. For Plan C this is what I followed

  1. Enable payments
  2. Visit site in tab 1 stay for 15 secs
  3. Visit different site in tab 2, stay for 15sec
  4. Switch back to tab 1, stay for 15 secs
  5. Payment shows view count 1 for both sites, and site from tab 1 has 30+secs and site from tab 2 has 15+sec

But as per steps in #11633, you mention stay on site for 2sec, which will not be added as the min time is not validated. Switching back and forth and staying on site for 15+ secs will add the view count as 1 as it would have satisfied the min time visit criteria

@NejcZdovc
Copy link
Contributor Author

@srirambv this Plan C is ok, because you switched back to existing tab you are basically revisiting site, so there shouldn't be counted as a new view, so view count should be 1

@NejcZdovc
Copy link
Contributor Author

I think that Plan C is not valid, will change it

This was referenced Oct 26, 2017
Closed
Closed
Closed
Closed
@LaurenWags
Copy link
Member

#11685 logged for issues with test plan D from #11592 (comment). Other tests passed on MacOS.

cezaraugusto pushed a commit that referenced this issue Oct 27, 2017
@NejcZdovc @cezaraugusto
Fixes publisher not added to the ledger
5670000 
Resolves #11575

Auditors:

Test Plan:
syuan100 pushed a commit to syuan100/browser-laptop that referenced this issue Nov 9, 2017
@NejcZdovc @syuan100
Fixes publisher not added to the ledger
794756d 
Resolves brave#11575

Auditors:

Test Plan:
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@NejcZdovc NejcZdovc

Labels
feature/rewards QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/exclude
Projects
None yet
Milestone
0.19.x Hotfix 2 (Release channel)
Development

No branches or pull requests
6 participants
@bbondy @kjozwiak @luixxiul @NejcZdovc @srirambv @LaurenWags