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

Analytics Top content is sorted by title and not by views #1867

Closed
gmmedia opened this issue Aug 4, 2020 · 8 comments
Closed

Analytics Top content is sorted by title and not by views #1867

gmmedia opened this issue Aug 4, 2020 · 8 comments
Labels
P0 High priority Type: Bug Something isn't working
Milestone

Comments

@gmmedia
Copy link

gmmedia commented Aug 4, 2020

Bug Description

The Google Analytics Top content widget is sorted by the title column and not by the views column.

Steps to reproduce

Same wrong sorting on the 3 pages:

  • WordPress Dashboard widget
  • Site Kit Dashboard
  • Analytics

Screenshots

2020-08-04_074049

Additional Context

  • PHP Version: 7.3
  • OS: Win7
  • Browser: Chrome
  • Plugin Version: 1.13.0

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • For Analytics top content displays (and underlying queries), results should be sorted by descending views/pageviews. Top content appears in the following locations:

    • WordPress Dashboard - "Top content over the last 28 days"
    • Site Kit Dashboard - "Most popular content"
    • Analytics Module page - "Top content over the last [28] days"

Implementation Brief

  • In includes/Modules/Analytics.php, in the handler for case 'GET:report', in the orderby handling immediately following the inline comment // Order by.:
    • remove [0] from the if condition block which checks whether $orderby is a numeric array or not:
// When just object is passed we need to convert it to an array of objects.
if ( ! wp_is_numeric_array( $orderby[0] ) ) {
	$orderby = array( $orderby );
}
  • Create a new protected method parse_data_orderby in the includes/Modules/Analytics.php that accepts an instance of the Data_Request class and returns $orderby result using algorithm from the GET:report handler.
    • Create unit tests for this method.
  • Update GET:report handler to use the new method to get orderby settings.

QA Brief

  • Go to each of the following pages and check top content report, it should be sorted descendingly by the number of page views.
    • WordPress Dashboard widget
    • Site Kit Dashboard
    • Analytics

Changelog entry

  • Fix regression where Analytics top content wouldn't be sorted correctly by views.
@jamesozzie jamesozzie added the Type: Bug Something isn't working label Aug 4, 2020
@adamsilverstein adamsilverstein self-assigned this Aug 4, 2020
@adamsilverstein
Copy link
Collaborator

Thanks for the bug report @gmmedia.

I can confirm I am seeing this as well, top content is sorted by page title in version 1.9.0:

image

In 1.8.1 I see pages forted by descending Views:

image

Using git bisect I tracked down the issue to commit 2a0eb8f. After this commit, the sorting no longer works as expected. cc: @eugene-manuilov

@eugene-manuilov
Copy link
Collaborator

Thanks @adamsilverstein! This issue is caused by a typo here:

if ( ! wp_is_numeric_array( $orderby[0] ) ) {

We don't need [0] there, it should be just if ( ! wp_is_numeric_array( $orderby ) ). Once it's removed, everything will work as expected. We need to udpate IB to state that we just need to remove [0] from that line.

@eugene-manuilov
Copy link
Collaborator

IB is updated

@adamsilverstein
Copy link
Collaborator

✅ IB looks good to me!

@eugene-manuilov
Copy link
Collaborator

Created a new PR against master branch: #1875

@eugene-manuilov eugene-manuilov removed their assignment Aug 4, 2020
@eclarke1 eclarke1 added this to the Sprint 29 milestone Aug 5, 2020
@aaemnnosttv aaemnnosttv self-assigned this Aug 5, 2020
@aaemnnosttv aaemnnosttv added the P0 High priority label Aug 5, 2020
@aaemnnosttv
Copy link
Collaborator

@eugene-manuilov would you please add a QA Brief here so this can be merged?

Assigning to @felixarntz for a final pass on the PR.

@aaemnnosttv aaemnnosttv assigned felixarntz and unassigned aaemnnosttv Aug 5, 2020
@eugene-manuilov
Copy link
Collaborator

@aaemnnosttv QA Brief is added.

@felixarntz felixarntz removed their assignment Aug 5, 2020
@cole10up cole10up self-assigned this Aug 5, 2020
@cole10up
Copy link

cole10up commented Aug 5, 2020

Tested:

Installed 1.13.0 and activated SK and Analytics with a site having lots of data:

Notice error:
image

image

image

Reset SK, uninstalled, deleted and installed 1.13.1. Activated SK and Analytics with the same above site:

Notice fix:
image

image

image

Confirmed adjusting duration functions properly for sort:
image

Checked each duration - confirmed functional
Checked links driving to GA - confirmed functional within SK

One thing to note - Clicking links within WP Dashboard drive to the actual page and not GA:
image

Dashboard ‹ Hellogoodbye — WordPress

Checked see full stats link - confirmed functional
Checked removing and re-installing analytics - confirmed functional
Checked adding new properties, views and accounts functions properly

image

Passed QA ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants