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
Screen resolution should be tracked correctly #9914
Conversation
I'm not sure if we should do too much validation here in Tracker API. In this case the user was using an old version of Piwik and it would have been fixed by updating. I'd rather have the raw data in DB whatever was sent and format if needed later in the report etc. Ideally the code was written well enough that we need to format output only only in one place wherever resolution is used. This gives developers to track whatever they want as the Tracking API is kind of generic and might not only relate to screen resolutions but possibly also resolutions. Now, if we think of pixels, they will be likely always an integer. But if we think of other resolutions such as tracking it in Also re check for width > 9999. There are already screens with 7680×4320 and likely there are at some point soon, if not already, screens > 9999. Something like this should definitely not be done in Tracking API. The tracking API has to be "stupid" and not do such things like checking whether the width is greater than something. |
Similar problem where API tried to be smart is here #9842 (comment) . |
noticed #9913 re 9999px. I would still remove it here meanwhile. And ideally also not use |
I agree generally Tracking API shouldn't validate data. in this case I thought it makes some sense, because the In general if it's not good to validate data in tracking API, an alternative solution would be that the Archiver validates the resolution labels and removes those that are invalid... is this maybe this you would prefer? |
Yes that's something I would prefer. This let's us later reuse the same field for tracking some "resolution" in "mm". In some cases it might even make sense to track screen resolution in mm instead of px. We'd need to create a custom report / archiver for this but that's fine. |
In general we need to rethink some of the meanings while we're going generic. Don't have to change the meaning everywhere but could make sense for some columns. As for Piwik 2.16.0 the bug should be fixed already. For Piwik 3+ we could issue an |
OK I understand your point of view. We don't really need this change for now.
Covered in #9913 |
fixes #9911