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

Adjustments for CustomReports plugin #16340

Merged
merged 3 commits into from Aug 25, 2020
Merged

Adjustments for CustomReports plugin #16340

merged 3 commits into from Aug 25, 2020

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Aug 24, 2020

While making CustomReport plugin compatible with Matomo 4, I've noticed a few dimensions/metrics having incorrect values.

  • Metrics using a custom sqlSegment like daysSinceFirstVisit are archived incorrectly. As archiving currently uses the plain field value all metrics using custom sqlSegments and those based on them are incorrect. Using the sqlSegment in ArchiveMetrics::getSql() fixes that problem.

  • Dimensions in milliseconds like the new performance metrics are showing 0 values (when used as dimension), as they are grouped incorrectly. The grouping already divided them by 1000, causing the formatting to 0 the values, as it divides them another time. Grouping them in 1000 segments instead seems more correct.

  • The pagviews with performance timings metrics had an incorrect type, causing them formatted as time instead of number

@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Aug 24, 2020
@sgiehl sgiehl added this to the 4.0.0 milestone Aug 24, 2020
@sgiehl sgiehl requested a review from tsteur August 24, 2020 12:25
@tsteur
Copy link
Member

tsteur commented Aug 24, 2020

@sgiehl

The pageviews with performance timings metrics had an incorrect type, causing them formatted as time instead of number

wouldn't that be expected? or you mean when requesting the API with format metrics off?

@@ -418,7 +418,7 @@ public function groupValue($value, $idSite)
case Dimension::TYPE_BOOL:
return !empty($value) ? '1' : '0';
case Dimension::TYPE_DURATION_MS:
return number_format($value / 1000, 2); // because we divide we need to group them and cannot do this in formatting step
return number_format($value / 1000, 2) * 1000; // because we divide we need to group them and cannot do this in formatting step
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we divide by 1000 in the first place then @sgiehl ?

@sgiehl
Copy link
Member Author

sgiehl commented Aug 24, 2020 via email

@tsteur tsteur merged commit 4df7ec5 into 4.x-dev Aug 25, 2020
@tsteur tsteur deleted the fixmetricsqlsegment branch August 25, 2020 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants