@diosmosis opened this Pull Request on January 23rd 2018 Member

Changes:

  • Throw an exception in Period\Factory::build() if $date is null. This happened while I was developing and it took me a very long time to figure out what was going on. An exception would have helped.
  • Use Request::getRenamedModuleAndAction() in Visualization. I found that was the one spot that didn't call this function before using Piwik\Api\Proxy.
  • Add new SitesManager.shouldPerformEmptySiteCheck event so plugins can disable the check for certain sites.
  • Added API.Request.intercept event to in API\Proxy. If a plugin sets $returnedValue, the actual function call is skipped.
  • Added Request.shouldDisablePostProcessing so plugins can disable datatable post processing for certain requests (if, for example, the request's post processing is done somewhere else).
  • Make sure datatable metadata is serialized w/ the datatable so information like report totals shows up when serialize=1&original=1. Don't think this will have a effect on DB size since only the rows are stored in the DB. Not sure what else serializes DataTable.

Note: I can cherry pick this into multiple PRs if needed & am also open to changing it.

@tsteur commented on January 23rd 2018 Member

Re $context in Request.getRenamedModuleAndAction not sure. How come this is needed? Is there an API and Controller with the same name? In theory, Request.getRenamedModuleAndAction should actually only be applied to controllers as API's are not an action but rather a method: Request.getRenamedModuleAndMethod.

@diosmosis commented on January 29th 2018 Member

@tsteur Modified this PR based on our slack convo, had to add two more changes to get my use case to work. Let me know if there's still an issue.

@tsteur commented on January 29th 2018 Member

Looks good @diosmosis
I would have possibly triggered another event for the $returnedValue to simply not pass two parameters by reference and maybe some other plugin changes finalParameters but by then maybe some plugin has already set a $returnedValue based on the initial finalParameters but could also leave it like this. Having a separate event for that purpose of generating a response may be a bit more clear.

@diosmosis commented on February 11th 2018 Member

@tsteur Updated this PR to have a new event API.Request.intercept.

@tsteur commented on February 11th 2018 Member

👍

@diosmosis commented on March 5th 2018 Member

@matomo-org/core-team would it be possible to merge this PR?

@tsteur commented on March 5th 2018 Member

Just one more thing: Can you mention the new events in the CHANGELOG.md? Happy to merge afterwards

@diosmosis commented on March 6th 2018 Member

@tsteur rebased + edited the changelog, test failure seems unrelated.

@tsteur commented on March 6th 2018 Member

👍

This Pull Request was closed on March 6th 2018
Powered by GitHub Issue Mirror