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
Fix goal metric processing for device type / model / brand #10873
Conversation
@@ -231,7 +231,7 @@ private function getVisitFieldsPersist() | |||
$dimensions = VisitDimension::getAllDimensions(); | |||
|
|||
foreach ($dimensions as $dimension) { | |||
if ($dimension->hasImplementedEvent('onExistingVisit')) { | |||
if ($dimension->hasImplementedEvent('onExistingVisit') || $dimension->hasImplementedEvent('onAnyGoalConversion')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure it this is the correct solution.
Explanation why I've changed that:
I had following code in a visit dimension
public function onAnyGoalConversion(Request $request, Visitor $visitor, $action)
{
return $visitor->getVisitorColumn($this->columnName);
}
That didn't work properly, as the column didn't implement the onExistingVisit
event (which simply isn't needed) and thus the property wasn't set for an existing visit.
Instead of this change here, I could also add an empty onExistingVisit
event to the columns. But I think it is more useful this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good solution
4fb3be2
to
06d6e7b
Compare
Failing UI tests seem not to be related to this PR. |
LGTM. I'll confirm on demo3 in coming days when the reports work 👍 |
Processing goal metrics for device type / model / brand was not working properly.
The visitor column value was not available when processing the goal conversion, which should be fixed with this changes.
I've also added some small tests to prove the goal metrics are processed correctly for device type / brand / model.
fixes #9777