Goals per page - new reports and metrics for tracking page conversions #18221
Conversation
…justed percent formatting
… values, refactored goal visualisation classes to minimise duplicate methods
…to match expected calculations
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
|
@bx80 anything needed here to progress this? |
|
@justinvelluppillai Just needs reviewing. |
There was a problem hiding this comment.
Only had a very rough look through the code, as it actually doesn't seem to work.
Running the archiving on this branch results in an error for me:
ERROR [2021-11-22 15:38:22] 23504 Got invalid response from API request: ?module=API&method=CoreAdminHome.archiveReports&idSite=1&period=week&date=2021-11-15&format=json&trigger=archivephp. Response was '{"result":"error","message":"SQLSTATE[42S22]: Column not found: 1054 Unknown column 'log_conversion.server_time' in 'on clause' - in plugin Goals. #0 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(284): Piwik\\ArchiveProcessor\\PluginsArchiver->callAggregateAllPlugins('0', NULL, true) #1 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(176): Piwik\\ArchiveProcessor\\Loader->prepareAllPluginsArchive('0', NULL) #2 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(159): Piwik\\ArchiveProcessor\\Loader->insertArchiveData(0, 0) #3 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(99): Piwik\\ArchiveProcessor\\Loader->prepareArchiveImpl('VisitsSummary') #4 \/srv\/matomo\/core\/Context.php(75): Piwik\\ArchiveProcessor\\Loader->Piwik\\ArchiveProcessor\\{closure}() #5 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(103): Piwik\\Context::changeIdSite(1, Object(Closure)) #6 \/srv\/matomo\/plugins\/CoreAdminHome\/API.php(278): Piwik\\ArchiveProcessor\\Loader->prepareArchive('VisitsSummary') #7 \/srv\/matomo\/core\/Archive.php(847): Piwik\\Plugins\\CoreAdminHome\\API->archiveReports(1, Object(Piwik\\Period\\Day), '2021-11-17', '', 'VisitsSummary', '') #8 \/srv\/matomo\/core\/Archive.php(645): Piwik\\Archive->prepareArchive(Array, Object(Piwik\\Site), Object(Piwik\\Period\\Day)) #9 \/srv\/matomo\/core\/Archive.php(592): Piwik\\Archive->cacheArchiveIdsAfterLaunching(Array, Array) #10 \/srv\/matomo\/core\/Archive.php(518): Piwik\\Archive->getArchiveIds(Array) #11 \/srv\/matomo\/core\/Archive.php(328): Piwik\\Archive->get(Array, 'numeric') #12 \/srv\/matomo\/core\/ArchiveProcessor.php(630): Piwik\\Archive->getDataTableFromNumeric(Array) #13 \/srv\/matomo\/core\/ArchiveProcessor.php(265): Piwik\\ArchiveProcessor->getAggregatedNumericMetrics(Array, false) #14 \/srv\/matomo\/core\/ArchiveProcessor\/PluginsArchiver.php(306): Piwik\\ArchiveProcessor->aggregateNumericMetrics(Array) #15 \/srv\/matomo\/core\/ArchiveProcessor\/PluginsArchiver.php(104): Piwik\\ArchiveProcessor\\PluginsArchiver->aggregateMultipleVisitsMetrics() #16 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(278): Piwik\\ArchiveProcessor\\PluginsArchiver->callAggregateCoreMetrics() #17 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(176): Piwik\\ArchiveProcessor\\Loader->prepareAllPluginsArchive('22', 0) #18 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(159): Piwik\\ArchiveProcessor\\Loader->insertArchiveData('22', 0) #19 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(99): Piwik\\ArchiveProcessor\\Loader->prepareArchiveImpl(false) #20 \/srv\/matomo\/core\/Context.php(75): Piwik\\ArchiveProcessor\\Loader->Piwik\\ArchiveProcessor\\{closure}() #21 \/srv\/matomo\/core\/ArchiveProcessor\/Loader.php(103): Piwik\\Context::changeIdSite(1, Object(Closure)) #22 \/srv\/matomo\/plugins\/CoreAdminHome\/API.php(278): Piwik\\ArchiveProcessor\\Loader->prepareArchive(false) #23 [internal function]: Piwik\\Plugins\\CoreAdminHome\\API->archiveReports('1', Object(Piwik\\Period\\Week), '2021-11-15', false, false, false) #24 \/srv\/matomo\/core\/API\/Proxy.php(244): call_user_func_array(Array, Array) #25 \/srv\/matomo\/core\/Context.php(28): Piwik\\API\\Proxy->Piwik\\API\\{closure}() #26 \/srv\/matomo\/core\/API\/Proxy.php(335): Piwik\\Context::executeWithQueryParameters(Array, Object(Closure)) #27 \/srv\/matomo\/core\/API\/Request.php(266): Piwik\\API\\Proxy->call('\\\\Piwik\\\\Plugins\\\\...', 'archiveReports', Array) #28 \/srv\/matomo\/plugins\/API\/Controller.php(46): Piwik\\API\\Request->process() #29 [internal function]: Piwik\\Plugins\\API\\Controller->index() #30 \/srv\/matomo\/core\/FrontController.php(619): call_user_func_array(Array, Array) #31 \/srv\/matomo\/core\/FrontController.php(168): Piwik\\FrontController->doDispatch('API', false, Array) #32 \/srv\/matomo\/core\/dispatch.php(32): Piwik\\FrontController->dispatch() #33 \/srv\/matomo\/index.php(25): require_once('\/srv\/matomo\/cor...') #34 \/srv\/matomo\/core\/CliMulti\/RequestCommand.php(79): require_once('\/srv\/matomo\/ind...') #35 \/srv\/matomo\/vendor\/symfony\/console\/Symfony\/Component\/Console\/Command\/Command.php(257): Piwik\\CliMulti\\RequestCommand->execute(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput)) #36 \/srv\/matomo\/vendor\/symfony\/console\/Symfony\/Component\/Console\/Application.php(874): Symfony\\Component\\Console\\Command\\Command->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput)) #37 \/srv\/matomo\/vendor\/symfony\/console\/Symfony\/Component\/Console\/Application.php(195): Symfony\\Component\\Console\\Application->doRunCommand(Object(Piwik\\CliMulti\\RequestCommand), Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput)) #38 [internal function]: Symfony\\Component\\Console\\Application->doRun(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput)) #39 \/srv\/matomo\/core\/Console.php(130): call_user_func(Array, Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput)) #40 \/srv\/matomo\/core\/Access.php(670): Piwik\\Console->Piwik\\{closure}() #41 \/srv\/matomo\/core\/Console.php(131): Piwik\\Access::doAsSuperUser(Object(Closure)) #42 \/srv\/matomo\/core\/Console.php(82): Piwik\\Console->doRunImpl(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput)) #43 \/srv\/matomo\/vendor\/symfony\/console\/Symfony\/Component\/Console\/ ... ut))\n#57 \/srv\/matomo\/console(32): Symfony\\Component\\Console\\Application->run()\n#58 {main}"}'
The column exists in the table, so guess it might be something with a missing table alias or similar.
8000
plugins/Goals/DataTable/Filter/RemoveUnusedGoalRevenueColumns.php
Outdated
Show resolved
Hide resolved
|
@sgiehl Looks like I managed to come up with a query in LogAggregator::queryConversionsByPageView() that worked in MariaDB but not in MySQL with strict mode. 🤦 |
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
| 'period' => $periodName, | ||
| 'date' => ($periodName === 'range' ? $date.','.$period->getDateEnd()->toString() : $date), | ||
| 'idSite' => $idSite, | ||
| 'idGoal' => $idGoal |
There was a problem hiding this comment.
I'd assume that this request might lag the segment.
There was a problem hiding this comment.
@sgiehl It could yes, the Goals.get method is going to load the whole blob data table again which could be slow on large datasets. Since we're storing the total conversions for each goal and period in the numeric archives I think it would be more efficient to directly lookup the total we need. Goals.getConversions does this but has unfortunately been disabled on the API with the @ignore annotation for some reason.
I've committed a change where the conversion total is retrieved directly using the Archive::getNumeric method instead of an API call. This should be a lot faster with no API overhead and only a single value being queried instead of deserializing a datatable, applying filters, etc.
| foreach ($goalIds as $idGoal => $g) { | ||
|
|
||
| $date = ($periodName === 'range' ? $date.','.$period->getDateEnd()->toString() : $date); | ||
| $archive = Archive::build($idSite, $periodName, $date); |
There was a problem hiding this comment.
This one actually lags the segment as well. I'm wondering if it might be easier to move that to the Actions API. So we fetch the goal totals there and then pass the fetched totals to the filter call. That way it's a lot easier to access the required archive parameters and we don't need to rely on the datatable metadata.
There was a problem hiding this comment.
That makes sense, cleaner to have the goals totals injected into the filter than have the filter accessing archive data directly. I've committed an update, but I'm not sure that it will speed things up much.
There was a problem hiding this comment.
Actually I don't think this is going to work, filterActionsDataTable can accept a DataTable\Map as well as a DataTable so the goal retrieval will need to happen at the filter level since we may be dealing with multiple different datatables. I'll revert the change.
…te filter to the Actions API and then pass to the filter
…30-goals-per-page
An attempt to lookup goal conversion totals is only made for goals that are contained in the row metadata. So if there isn't any goal metadata then no queries will be made and the filter will not run at all. This does mean looping the table checking the metadata on each row and putting the goal ids into an array, but this should be fast with in-memory data, the alternative would be to make another API call to get all the available goals for the site and then check totals for each of these goals even if they are not present in the table - I think this would be much slower, especially if the site has dozens of goals but the data table is only being viewed for a single goal. |
|
There are some failed tests I'll need to check. |
plugins/Actions/API.php
Outdated
|
|
||
| // For each goal in the table retrieve the period total conversions and add to the array | ||
| foreach ($goalIds as $idGoal => $g) { | ||
| $archive = Archive::build($idSite, $period, $date); |
There was a problem hiding this comment.
Should that Archive::build include the segment? Otherwise we will compare segmented action data, with unsegmented conversion numbers. Or is that intended. If so it would be good to add a comment
There was a problem hiding this comment.
It should probably include the segment as we'll want to show the viewed before rate relative to the conversions that happened for the segment. I've included the segment in the archive::build call.
… api. Included segment in archive build for goal conversions totals.
|
The missing conversion rate formatting on the |
fe7f98f to
f65b2fd
Compare
Description:
Fixes #2030
Adds new reports for goal conversions by page url, page title and entry page to the goals overview, see the detailed specification in #2030 for full details.
System tests have now been added.
Review