@flamisz opened this Pull Request on June 8th 2021 Contributor

Description:

fixes #17615

Review

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@diosmosis commented on June 9th 2021 Member

code looks good :+1:

@MichaIng commented on June 9th 2021

Many thanks. I applied the new code, but I think I have to wait for the next report generation to see the results.

Where is the new function getPastRowFromCurrent actually used?
EDIT: Ah sorry, I see now it is an override.

@MichaIng commented on June 9th 2021

Okay, it does not fix the issue in our case. I added the code, verified it, cleared all caches, but it still shows:

Others +100%
Others -100%

on all page title lists.

Are there other companion changes required, committed outside of this PR?

@diosmosis commented on June 9th 2021 Member

@MichaIng This by itself seemed to fix it for me locally (and the test works on this branch but fails on 4.x-dev). I'll reopen the original issue.

@diosmosis commented on June 9th 2021 Member

@MichaIng could you post the following screenshots to help us debug?

  • a screenshot of the insights overview page w/ the issue visible (and the period / date visible)
  • a screenshot of the page titles report for the same period/date, sorted in descending order so the Others row is shown first
  • a screenshot of the page titles report (as before) but for the previous period/date (again sorted so we see the Others row)
@MichaIng commented on June 11th 2021

insights

The page titles report does not show "Others" at all, neither for the current period (this is one week), nor for any previous period, long or short.

@diosmosis commented on June 11th 2021 Member

@MichaIng I'll try to reproduce this again. Would you be willing to provide view access to your Matomo in the event I can't?

@MichaIng commented on June 11th 2021

Yes, I can do that.

@diosmosis commented on June 11th 2021 Member

@MichaIng can you email dizzy@innocraft.com with access details? I'll take a closer look today.

@MichaIng commented on June 11th 2021

Okay that is strange, the issue seems to fade currently, while all I just did is updating another app. Is the "Others" entry preserved in reports and because of this still shown even that the code has been fixed? The very same time range the screenshot above is from now does not shot "Others" anymore, the "PAGE TITLES" section in the "Insights Overview" widget is gone completely instead. I still see "Others" when switching to "day" or "week" view (above is "date range", covering last seven days).

But I'll send you access details regardless.

@diosmosis commented on June 11th 2021 Member

@MichaIng

Is the "Others" entry preserved in reports and because of this still shown even that the code has been fixed?

It is preserved in reports, but the Insights report is not stored in the database. It's built dynamically using stored reports for the current period and previous period, for example, the page titles reports. So if the data in the periods for other reports (like the page titles reports) change, then the Insights reports should display different data immediately. I'm not sure why it would show nothing now... I'll take a look later and see what I can find.

@diosmosis commented on June 13th 2021 Member

Resolved via email, the PR does work.

@MichaIng commented on June 13th 2021

Yes, many thanks. The entries from the old records basically needed to fade out.

@sgiehl commented on June 13th 2021 Member

@diosmosis so we can close #17615 again?

@diosmosis commented on June 13th 2021 Member

@sgiehl yes :+1:. Closed it.

This Pull Request was closed on June 9th 2021
Powered by GitHub Issue Mirror