@tiraeth opened this Pull Request on April 4th 2013

I've found an issue while using showColumns parameter in API calls that use VisitsSummary module. It just doesn't work because hideShowMetrics is checking keys in array $availableReport['metrics'] that is just invalid in this one case. I don't know why it's not built as other module's arrays and made a workaround (decorated with FIXME comment) for this one case.

To not duplicate myself (is_int($name)) I decided to refactor the whole function and iterate over $columns array passed to it only once.

I recommend considering this as a hotfix if VisitsSummary is working just fine with it's metrics array and include similiar patch in 1.11.1 release.

@halfdan commented on April 4th 2013 Member

Not sure about this issue, but tests fail right now.

@halfdan commented on April 4th 2013 Member

There is no reason to get upset, so please watch your tongue.

I did not close this issue, nor do I intend to do so right now. All I'm asking, is that you make the tests pass. That's how PRs usually work on Github - if you send a PR, make sure the code still passes the tests.

We will look into the array indexing issue - thank you for pointing that out.

@mattab commented on April 4th 2013 Member

@tiraeth we only look at Pull requests when they pass the tests, to avoid losing time reviewing broken code. Thanks for your understanding! :+1:

This Pull Request was closed on April 4th 2013
Powered by GitHub Issue Mirror