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

Insights widget shows only "Others" page titles #17615

Closed
MichaIng opened this issue May 24, 2021 · 7 comments · Fixed by #17656
Closed

Insights widget shows only "Others" page titles #17615

MichaIng opened this issue May 24, 2021 · 7 comments · Fixed by #17656
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone

Comments

@MichaIng
Copy link
Contributor

Since Matomo v4.3.0 I think, the dashboard widgets "Insights Overview" and "Moves and Shakers" of the Insights plugin show only "Others +100%" and "Others -100%" where most changed page title views should be listed. It looks like page titles with less views/changes are accumulated as "Others", but all of them together change most, which doesn't seem like intended, at least makes it useless.

Expected Behavior

Two distinct most changed page titles should be shown in the widgets.

Current Behavior

PAGE TITLES UNIQUE PAGEVIEWS EVOLUTION
Others +5259 +100%
Others -5691 -100%

Possible Solution

No idea. Probably an underlying API has changed, when obtaining a list of page titles, where less important page titles are merged?

Steps to Reproduce (for Bugs)

  1. Enable Insights plugin
  2. Add both Insights widgets "Insights Overview" and "Moves and Shakers" to dashboard

Context

Your Environment

  • Matomo Version: 4.3.1-rc1 (issue was present with 4.3.0 already)
  • PHP Version: 8.0.5
  • Server Operating System: Debian Bullseye
  • Additionally installed plugins:
API, Actions, Annotations, BotTracker 2.02, BulkTracking, CoreAdminHome, CoreConsole, CoreHome, CorePluginsAdmin, CoreUpdater, CoreVisualizations, DBStats, DarkTheme 1.1.6, Dashboard, DevicePlugins, DevicesDetection, Diagnostics, Goals, ImageGraph, Insights, Installation, Intl, LanguagesManager, Live, LogViewer 4.0.1, Login, Marketplace, Monolog, Morpheus, PagePerformance, PrivacyManager, Proxy, Referrers, Resolution, SEO, SegmentEditor, SitesManager, Transitions, UserLanguage, UsersManager, VisitFrequency, VisitTime, VisitorInterest, VisitsSummary, WebsiteMeasurable
  • Browser: Opera 78
  • Operating System: Windows 10 21H1
@MichaIng MichaIng added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label May 24, 2021
@diosmosis
Copy link
Member

Hi @MichaIng, thanks for the report, we see this on our instances as well.

@diosmosis diosmosis added Bug For errors / faults / flaws / inconsistencies etc. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels May 25, 2021
@tsteur tsteur added the Regression Indicates a feature used to work in a certain way but it no longer does even though it should. label May 27, 2021
@tsteur tsteur added this to the 4.4.0 milestone May 27, 2021
@flamisz flamisz self-assigned this Jun 1, 2021
@flamisz
Copy link
Contributor

flamisz commented Jun 2, 2021

I investigated the issue. I haven't sold it yet, but it looks like it's happening because we modified the logic around the summary row for datatables.

matomo/core/DataTable.php

Lines 841 to 849 in c3bf4c8

public function addSummaryRow(Row $row)
{
$this->summaryRow = $row;
$row->setIsSummaryRow();
// NOTE: the summary row does not go in the index, since it will overwrite rows w/ label == -1
return $row;
}

It adds an extra row and we don't filter that out.

@flamisz
Copy link
Contributor

flamisz commented Jun 3, 2021

The last working version is 4.3.0-rc1.
The diff between 4.3.0-rc1 and 4.3.0-rc2 is here: 4.3.0-rc1...4.3.0-rc2

It has a commit about summary rows. I check if it works without that commit.

@flamisz
Copy link
Contributor

flamisz commented Jun 3, 2021

@diosmosis I checked and this commit causes the issue here: 8366ad8

Do you have an idea why it could happen? As I check the data I see we have an extra row now and don't filter that out during the process.

@diosmosis
Copy link
Member

diosmosis commented Jun 3, 2021

@flamisz no, I'm not sure why that would happen. To figure out why, I might take the following approaches:

  • look in the Insights code for getRowFromLabel()/getRows() calls since those were the main methods that changed
  • look in the Insights code for uses of subtables & summary rows and see if the use could cause different results now
  • create an automated test that fails on 4.x-dev but passes when checking out 8366ad8^, and figure out what the difference in execution is

@flamisz
Copy link
Contributor

flamisz commented Jun 3, 2021

It looks like the problem is in the rebuildIndex() method.

matomo/core/DataTable.php

Lines 740 to 753 in 8b4b8ce

public function rebuildIndex()
{
$this->rowsIndexByLabel = array();
$this->rebuildIndexContinuously = true;
foreach ($this->rows as $id => $row) {
$label = $row->getColumn('label');
if ($label !== false) {
$this->rowsIndexByLabel[$label] = $id;
}
}
$this->indexNotUpToDate = false;
}

The diff is:

public function rebuildIndex()
{
    $this->rowsIndexByLabel = array();
    $this->rebuildIndexContinuously = true;
    foreach ($this->rows as $id => $row) {
        $label = $row->getColumn('label');
        if ($label !== false) {
            $this->rowsIndexByLabel[$label] = $id;
        }
    }

-    if ($this->summaryRow) {
-        $label = $this->summaryRow->getColumn('label');
-        if ($label !== false) {
-            $this->rowsIndexByLabel[$label] = DataTable::ID_SUMMARY_ROW;
-        }
-    }

    $this->indexNotUpToDate = false;
}

Is this make sense @diosmosis? Is this something that should be fixed in this method or something should be fixed in Insight plugin?

@diosmosis
Copy link
Member

diosmosis commented Jun 4, 2021

@flamisz we can't add it to the rowsIndexByLabel variable, since the summary row label is -1, but sometimes users track data as -1, so one of the two gets lost or overwritten. So it would probably have to be fixed in the Insight plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants