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
Do not serialize every property of Periods which can appear in DataTable metadata. #13280
Conversation
Are we actually still supporting PHP serialisation? Does this have any side effects besides the API (probably not)? Be good to maybe have a test for this. No big opinion on the PR otherwise. |
Test for the changes here? Or for php serialized responses? |
php serialized response |
I just realize if this format is API it would be a breaking change in theory as you maybe wouldn't be able to use the output of 3.6.0 in an older version of Matomo, eg 3.5? Shouldn't be too big of an issue though but can be a problem. Really don't know how much we support that format. |
Tested locally from 3.5.1 => "instance w/ these changes" and it seems to work, the objects inflate. Only thing that doesn't work is getting a pretty string using a period in the datatable metadata. Not sure what kind of effect that would have. |
^ CC @mattab |
fyi we're planning to eventually deprecate this format in API responses: #8100 |
@diosmosis the builds are failing, can you please look into them? I'm merging anyway so we proceed with the RC release |
@mattab if all the UI code is in angular & the PHP UI layer doesn't make API requests, then we won't need the original format. Otherwise it's the only way to proxy (that I can think of). |
@diosmosis the issue #8100 is about removing format=php but we would still support format=original for internal API requests. Is there a problem with removing format=php do you think? if so please post a note in #8100 |
No issue w/ removing format=php, just format=original |
…ble metadata. (matomo-org#13280) * Do not serialize every property of Periods which can appear in DataTable metadata. * Add test for original PHP serialized response.
Since subperiods are created recursively and the Translator has all translations, serializing both of these properties massively inflates the API response when using php serialization.
@mattab / @tsteur would like to get this in 3.6.0