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

Bug Fix for bug 4063. #97

Closed
wants to merge 1 commit into from
Closed

Bug Fix for bug 4063. #97

wants to merge 1 commit into from

Conversation

andrewgrz
Copy link

This is a bug fix for 4063. "Notice: Array to string conversion" when downloading CSV or TSV files from Visitor Log in 1.12.

Some custom variables are in the $csv array as arrays themselves. Line 283 was trying to handle this array as a string and was throwing the error. This fix parses the array into a string prior to the string formatting step. The csv will successfully download with this fix. Tested on PHP 5.4.13.

Pull request for master.

The bug is reproducible with a clean install and generated data using the plugin. The route is address to get the bug is:

index.php?module=API&method=Live.getLastVisitsDetails&format=CSV&idSite=1&period=day&date=yesterday&expanded=1&translateColumnNames=1&language=en&filter_limit=100

@diosmosis
Copy link
Member

I can reproduce w/ php 5.4 & php 5.5. Regarding your changes:

  • Your code assumes formatValue will only be called w/ custom variables in the Live.getLastVisitsDetails report. This may be the case now but it is not guaranteed to always be the case. A more durable solution that handles any array is required or Live.getLastVisitsDetails needs to be customized for this specific use case.
  • Can you add an integration test for this? Adding a test for Live.getLastVisitsDetails to CsvExportTest.php should do the trick, I think.

@diosmosis
Copy link
Member

Fixed in 1510afc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants