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

#2781 Add support for segmentation on Goals.getItems* API #121

Merged

Conversation

piwikpro-team
Copy link

Improvements in queryEcommerceItems function which make it works with segments query builder and add support for "log_conversion_item" table in segmenetation to join this table with log_visit.

Improvements in queryEcommerceItems function which make it works with segments query builder
@mattab
Copy link
Member

mattab commented Oct 13, 2013

Thanks for the PR!

The tests are failing: https://travis-ci.org/piwik/piwik/jobs/12363492

eg.

"Table 'log_link_visit_action', can't be joined for segmentation
#0 /home/travis/build/piwik/piwik/core/Segment.php(175): Piwik\Segment->generateJoins(Array)

#1 /home/travis/build/piwik/piwik/core/DataAccess/LogAggregator.php(87): Piwik\Segment->getSelectQuery('name as label, ...', Array, ' server_time >=...', Array, false, 'ecommerceType, ...')

#2 /home/travis/build/piwik/piwik/core/DataAccess/LogAggregator.php(354): Piwik\DataAccess\LogAggregator->generateQuery('name as label, ...', Array, ' server_time >=...', 'ecommerceType, ...', false)

#3 /home/travis/build/piwik/piwik/plugins/Goals/Archiver.php(236): Piwik\DataAccess\LogAggregator->queryEcommerceItems('idaction_sku')

#4 /home/travis/build/piwik/piwik/plugins/Goals/Archiver.php(90): Piwik\Plugins\Goals\Archiver->archiveEcommerceItems()

#5 /home/travis/build/piwik/piwik/plugins/Goals/Goals.php(505): Piwik\Plugins\Goals\Archiver->archiveDay()

#6 [internal function]: Piwik\Plugins\Goals\Goals->archiveDay(Object(Piwik\ArchiveProcessor\Day))

#7 /home/travis/build/piwik/piwik/core/EventDispatcher.php(98): call_user_func_array(Array, Array)

#8 /home/travis/build/piwik/piwik/core/Piwik.php(658): Piwik\EventDispatcher->postEvent('ArchiveProcesso...', Array, false, NULL)

#9 /home/travis/build/piwik/piwik/core/ArchiveProcessor/Day.php(124): Piwik\Piwik::postEvent('ArchiveProcesso...', Array)

#10 /home/travis/build/piwik/piwik/core/ArchiveProcessor.php(295): Piwik\ArchiveProcessor\Day->compute()

#11 /home/travis/build/piwik/piwik/core/ArchiveProcessor.php(198): Piwik\ArchiveProcessor->computeNewArchive('VisitsSummary', false)

#12 /home/travis/build/piwik/piwik/core/Archive.php(487): Piwik\ArchiveProcessor->preProcessArchive('VisitsSummary')

#13 /home/travis/build/piwik/piwik/core/Archive.php(403): Piwik\Archive->cacheArchiveIdsAfterLaunching(Array, Array)

Please fix build before I can merge PR.

@mattab
Copy link
Member

mattab commented Oct 17, 2013

Along with the PR here are suggestions for new tests:

Then these tests will generate new XML in processed/* directory. You can check these XML are correct and return only those products that were done by visitors matching the segment.

If the XML are valid copy them to the expected/* folder. Then run the tests again, they should pass (or repeat until they pass).

Btw the best way to run all tests is in the root of piwik run:

./console tests:run

if you want to run specific test:

 cd tests/PHPUnit/ && phpunit Integration/EcommerceOrderWithItemsTest.php

Note; running takes takes long time. Hope it helps to make a Pull Request including tests

If travis is green then the PR should be safe to merge (and we know functionnality will not break in the future, which is an amazing feeling!)

@czolnowski
Copy link
Contributor

Looks that tests passed. I've add to EcommerceOrderWithItemsTest acceptance tests with segment parameter and it works. :)

mattab pushed a commit that referenced this pull request Dec 8, 2013
….getItems

Fixes #2781 Add support for segmentation on Goals.getItems* API. Kuddos for the nice pull request and tests!
@mattab mattab merged commit 7778531 into matomo-org:master Dec 8, 2013
mattab added a commit that referenced this pull request Dec 8, 2013
…wo countries so that it's clear that the Segmenting on the country selects only that country.
mattab added a commit that referenced this pull request Dec 8, 2013
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.

None yet

3 participants