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

Processed metrics calculation refactor and removal of as much queued filter use as possible #6261

Closed
diosmosis opened this issue Sep 21, 2014 · 2 comments
Assignees
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@diosmosis
Copy link
Member

Processed metrics should be described as Reports/Dimensions/Segments currently are. That is there should be a class w/ metadata & related methods for each processed metric. For example,

class AvgTimeOnSite extends ProcessedMetrics
{
    public function calculate(Row $row) { ... }

    public function format($value) { ... }
}

Instead of using queued filters to calculate and format processed metrics, the list of processed metrics should be added to DataTable metadata. Then in ResponseBuilder a filter should be used to calculate all data. Then for non-'original' API renderers, a filter should be executed that formats columns.

This will avoid all future occurrences of the 'trying to add string + string' error in DataTable\Row::sumRow and will make it easier to manipulate tables since there will be less queued filters to worry about.

When finished, make sure to remove the clearQueuedFilters() call in PivotByDimension.php.

@diosmosis diosmosis added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. labels Sep 21, 2014
@mattab mattab added this to the Short term milestone Sep 25, 2014
@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Oct 21, 2014
@mattab mattab modified the milestones: Piwik 2.9.0, Short term Oct 22, 2014
@mattab
Copy link
Member

mattab commented Oct 22, 2014

Moved this issue into current milestone as it's become important.

@diosmosis your refactor work in this issue should solve #6485 which is a bug that has re-appeared and it's major priority as it breaks Scheduled Reports (eg. on our demo)

@mattab mattab added the duplicate For issues that already existed in our issue tracker and were reported previously. label Nov 10, 2014
@mattab mattab modified the milestones: Piwik 2.10.0 , Piwik 2.9.0 Nov 11, 2014
@mattab
Copy link
Member

mattab commented Nov 11, 2014

Moving to 2.10.0 as we can plan the refactor more broadly and revisit how processed metrics are handled in visualisations and plugins. see also Processed metrics metadata pull request #6589

@mattab mattab removed the duplicate For issues that already existed in our issue tracker and were reported previously. label Nov 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

2 participants