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
Get goals through Request::processRequest instead of directly from AP… #13293
Conversation
…I so events can fire.
plugins/Ecommerce/Controller.php
Outdated
@@ -44,7 +44,7 @@ public function getSparklines() | |||
$goalDefinition['name'] = $this->translator->translate('Goals_Ecommerce'); | |||
$goalDefinition['allow_multiple'] = true; | |||
} else { | |||
$goals = GoalsApi::getInstance()->getGoals($this->idSite); | |||
$goals = Request::processRequest('Goals.getGoals', ['idSite' => $this->idSite]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the filter_limit = -1
here as well to be safe and set default to []
? eg the controller might send a filter_limit etc?
Added a comment but otherwise looks good to merge if tests pass. Needs to fix some conflicting files by the looks as well. |
@@ -77,7 +77,7 @@ public function getConversionsOverview() | |||
$view = new View('@Ecommerce/conversionOverview'); | |||
$idGoal = Common::getRequestVar('idGoal', null, 'string'); | |||
|
|||
$goalMetrics = Request::processRequest('Goals.get', array('idGoal' => $idGoal)); | |||
$goalMetrics = Request::processRequest('Goals.get', array('idGoal' => $idGoal, 'filter_limit' => '-1'), $default = []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I wonder if we also need to set always filter_offset
in case it is sent along the request somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think by setting default=[]
likely not needed, forgot about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$default = []
changes the default values of the request from being $_GET + $_POST
to []
, so if filter_limit/filter_offset is sent in the request, it will be ignored. I'm pretty sure filter_limit = -1 isn't necessary w/ default, but like was discussed before, can't hurt.
#13293) * Get goals through Request::processRequest instead of directly from API so events can fire. * Do not use Request in tracker requests since renderers are not loaded. * Add default = [] & filter_limit = -1 to changed Goals.getGoals calls. * Add empty default request to one Goals.get call. * fix another test * Use 3.x-dev file.
matomo-org#13293) * Get goals through Request::processRequest instead of directly from API so events can fire. * Do not use Request in tracker requests since renderers are not loaded. * Add default = [] & filter_limit = -1 to changed Goals.getGoals calls. * Add empty default request to one Goals.get call. * fix another test * Use 3.x-dev file.
…I so events can fire.