@mattab opened this Issue on August 7th 2014 Owner

Reproduce:

Fixing this issue will make the API more powerful and useful. Today we had to work around it costing 1-2 hours of our life...

@tsteur commented on August 11th 2014 Owner

Using column visit_last_action_time should already work but would not make any sense of course since it is not visible in the output. And BTW: visit_last_action_time seems to be the default sort order anyway. So if no filter_column is set it could already work.

I am trying to apply the filter "clean visitors" before sorting but I do not know whether it has any side effects. I reckon one thing could be performance. So far the clean visitors filter was executed after GenericFilters and therefore only on the result set that is actually returned. Now we would execute it on all visitors, even on visitors that might get filtered out by GenericFilters. Also not sure whether still all other GenericFilters and all other use cases would work -> This would be a breaking API change.

Maybe we should rather run the sort filter twice and not execute the clean visitors filter immediately. So we would run GenericFilters which includes sort, clean the visitors and run the sort filter again.

@tsteur commented on August 11th 2014 Owner

As the UI tests do not work I did some manual testing and also checked Piwik Mobile codebase. There is a filter_sort_column set in the VisitorLog visualization: https://github.com/piwik/piwik/blob/5950_sortGetLastVisitsDetails/plugins/Live/VisitorLog.php#L44

Both solutions that I tried change the oder of the VisitorLog slightly since it is now actually able to sort by idVisit which was not the case before. I am not sure if this would be a fix or by which column the VisitorLog should be sorted @mattab?

@mattab commented on August 11th 2014 Owner

@tsteur Visitor Log should be sorted by visit_last_action_time / lastActionTimestamp

(lots of users use the Visitor Log as main reporting tool, and refresh it frequently "today" to see the latest visitors and actions on their website)

@mattab commented on August 11th 2014 Owner

Using column visit_last_action_time should already work but would not make any sense of course since it is not visible in the output

I didn't even think of that... at least now I get it why it didn't work :-)

I am trying to apply the filter "clean visitors" before sorting but I do not know whether it has any side effects. I reckon one thing could be performance.

performance may be an issue, because call to enrichVisitorArrayWithActions is heavy as it does JOINs on the raw log tables.

Maybe we should rather run the sort filter twice and not execute the clean visitors filter immediately. So we would run GenericFilters which includes sort, clean the visitors and run the sort filter again.

If this works then maybe it's a good solution?

@tsteur commented on August 12th 2014 Owner

Done. Gone with the second solution sorting a second time. I did some manual testing with the VisitorLog as the UI tests were not working (I think the build will work again since I fixed the travis.yml, will now in 30 min :) ). Will merge once the new integration tests pass and once it is in master I will check the UI tests just to keep it simple.

This Issue was closed on August 12th 2014
Powered by GitHub Issue Mirror