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
Use generic device model names if device type is known #13015
Conversation
24c975a
to
118587e
Compare
c171f6e
to
12e673a
Compare
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.
Some minor comments, otherwise looks good
return 'generic mobile'; | ||
} | ||
|
||
return ''; |
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.
This will also only affect new data, right? Would it be possible to do this during archiving so it can be applied to old reports?
Don't think this is super important if not, though.
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.
It's currently only for new data. Doing that while archiving would be possible but maybe a bit slow, as we would need to run a custom archiving query having an CASE
statement to set the generic name based on the device type field... Also it might be a bit weird to have reports showing a generic
model, but none can be found with segmentation as the log tables were not updated.
So maybe the correct way would be to write some kind of reattribution script like for geolocation.
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.
A reattribution script sounds good, though this can always be added later if users require it. Will merge as is.
} | ||
if (!$brand) { | ||
if (!$brand || $brand == Piwik::translate('General_Unknown')) { |
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.
Is $brand == Piwik::translate('General_Unknown'
necessary? There's the same check above.
12e673a
to
1048538
Compare
refs #12999