@anonymous-piwik-user opened this Issue on August 17th 2011

When you pass in a custom variable that is scoped to a page instead of visit, no results are returned. Looking in the database in log_link_visit_action I can see the custom variables are recorded.

The solution I came up with was to join log_visit with log_link_visit_action to check if the custom variable is for a visit or page. I have created a patch. It might not be the best answer, but the code should explain what is going on.

@anonymous-piwik-user commented on August 17th 2011

Attachment: Patch to join log_link_visit_action when calculating number of visits
diff.txt

@BeezyT commented on September 7th 2011 Member

Attachment: Archives for failing unit test
archives.png

@mattab commented on August 18th 2011 Member

Thanks for the patch. We will try and investigate for the next release

@BeezyT commented on September 2nd 2011 Member

(In [5115]) Refs #2633 API can handle segmentation by different scopes (page, visit...)

@BeezyT commented on September 3rd 2011 Member

(In [5116]) Refs #2633 Removed recognition of available segments on visits

@BeezyT commented on September 3rd 2011 Member

(In [5117]) Refs #2633 Another attempt at segment scope recognition

@mattab commented on September 6th 2011 Member

(In [5129]) Refs #2633 - revert previous commits since they are temporary solution (final patch in the works by Timo)

@BeezyT commented on September 7th 2011 Member

Here's a very short explination of the patch...

Result

  • All reports can use all available segments
  • CustomVars per page are referenced using CustomVariablePage(Name|Value)[1-5]
  • CustomVars per visit are referenced as before
  • New segments: goalConversion==idgoal, pageUrl, pageTitle

Implementation

  • If segements are based on other scopes than the report, the segmentation API handles the joins
  • Plugins need to supply the table of the segments as well (not only column name) via the Metdata API
  • Segment.test.php, testgetSelectQuery* are a good starting point to understand the way the new segmentation API has to be used. $segment->getSqlQuery() gets the custom parts of the query and does all the magic based on the requested segment.

I did manual tests on all API methods with different segments (often joining all three tables). If anybody would like to do some further testing, that would be good, since this is a core part of Piwik.

PS: Somehow revision [5138] is missing in the ticket. That's the related changeset.

@BeezyT commented on September 7th 2011 Member

(In [5139]) Refs #2633 updated unit test

@BeezyT commented on September 7th 2011 Member

In [5139], I fixed a failing unit test.

The integration test "test_oneVisitor_oneWebsite_severalDays_DateRange" was expecting 22 blobs for archive_numeric_2010_12 but there were 28. I'm not sure whether this is expected behavior or not. The changeset is just a quick and dirty fix, please review and explain what the expected number of blobs is in this case. I added a screenshot of the actual archives in the table. Maybe someone can spot the ones that shouldn't be there.

@BeezyT commented on September 8th 2011 Member

(In [5140]) Refs #2633 fixed table prefix in Segment.php unit tests. This should also fix the webtests.

@BeezyT commented on September 9th 2011 Member

(In [5141]) Refs #2633 fixing impact of lang changes

@mattab commented on September 11th 2011 Member

(In [5148]) Refs #2633 Adding one test + mysql function replace also replaces _ (for functions such as UNIX_TIMESTAMP)

@mattab commented on September 11th 2011 Member

(In [5150]) Refs #2633
Number of records in archive_numeric increased, because Timo added segmentation support to Frequency API

@mattab commented on September 11th 2011 Member

(In [5151]) Refs #2633

  • Code review I noticed one line was removed from ArchiveProcessing/Day.php - restoring
  • Fixing documentation output displayed at index.php?module=API&action=listSegments which is in turn used for the page: http://piwik.org/docs/analytics-api/segmentation/
    The fix is that now, custom variable of scope "page" work!!!! :) Great stuff Timo!
@BeezyT commented on September 12th 2011 Member

Thanks for the updates, Matt.

Only one comment: I think my condition in ArchiveProcessing_Day was correct.

This is what it was before (at revision 5114):

$segmentSql = $this->getSegment()->getSql();
$sqlSegment = $segmentSql['sql'];
if(empty($sqlSegment) 
    || self::getPluginBeingProcessed($this->getRequestedReport()) == 'VisitsSummary')

This is my version (at revision 5141):

$segment = $this->getSegment();
$segmentation = !$segment->isEmpty();
$reportType = self::getPluginBeingProcessed($this->getRequestedReport());
if (!$segmentation || ($reportType == 'VisitsSummary'))

I did change empty($segment->getSql()['sql']) into $segment->isEmpty(), but that's on purpose, since getSql() has been removed. Apart from that, it should be the same condition.

This is your fix (at revision 5151):

$reportType = self::getPluginBeingProcessed($this->getRequestedReport());
if ($this->shouldProcessReportsAllPlugins($this->getSegment(), $this->period)
    || ($reportType == 'VisitsSummary'))

IMHO this is different to the original condition.

Please review.

@mattab commented on September 12th 2011 Member

(In [5160]) Refs #2633 - confirmed code is correct

This Issue was closed on October 14th 2011
Powered by GitHub Issue Mirror