Deprecate UserSettings plugin -> use DeviceDetection instead! #3962

Closed
mattab opened this Issue May 30, 2013 · 37 comments

3 participants

@mattab
Piwik Open Source Analytics member

In Piwik 1.12 we added the DevicesDetection plugin to provide advanced user detection of mobile devices, tvs, consoles, brands, models, Operating systems versions, etc. The code is still beta and we know covers only 50-80% of all devices.

Now that this plugin works well, we note redundancy between DevicesDetection algorithm and UserSettings.

eg the Following reports are processed and displayed in both plugins:

  • Browsers report (and browser versions)
  • Operating System report (and OS versions)

Tasks
Here is how to solve the redundancy and make code unified.

general

API:

  • the Following UserSettings.* API will be redirected from UserSettings to their equivalent in DevicesDetection (to keep backward compatibility)
    • getOS
    • getOSFamily
    • getMobileVsDesktop (now loads DevicesDetection.getType )
    • getBrowserVersion
    • getBrowserType
    • getBrowser
  • Remove code processing of these reports from UserSettings/Archiver

Core:

  • Enable DevicesDetection by default (add new Update file to enable plugin).
    • Officially out of beta!

Tests & Backward compatibility

  • If users re-process reports from the "old logs" (when UserSettings alone was doing detection), do the reports processed by DevicesDetection using old data display OK ?
  • If users migrate to Piwik 2.0 and use DevicesDetection for the first time
    • We must ensure that old UserSettings blobs are renamed to DevicesDetection names (example). So that the UserSettings blobs are read and used by DevicesDetection API.

Tracker:

  • remove the current UserAgentParser code to simplify Tracker (in getUserSettingsInformation())
    • If DevicesDetection plugin is disabled, there will not be any browsers or OS detected

At the end of this ticket we will see:

  • UserSettings plugin will be simpler
  • DevicesDetection plugin becomes critical part of Piwik architecture (by providing browsers/os parsing exclusively)
  • Simplification of Tracker in core
  • Consistent reports (browsers/OS only displayed once as expected)
  • Keep backward compatibility in terms of API and also DevicesDection can read old UserSettings blob data.
    • Better detection accuracy for most users (since they will now use DevicesDetection)

See also: Help improve DevicesDetection plugin: see piwik/device-detector#4215

@mattab
Piwik Open Source Analytics member

A bug was reported here: http://forum.piwik.org/read.php?2,105199,page=1#msg-105459

four new fields for the table log_visit have been introduced, one is called "config_os_version". In our intranet many user sessions are running under Citrix. The corresponding calculated value for this field is "Server 2003". With this value, the write operation goes wrong. The complete record is not written.
The call Piwik_Tracker getDatabase :: () -> query ($ sql, $ bind) in Visit.php is not coming back, the action is not counted!.
So 3,500 visitors per day with all actions are lost in our statistics. 

@peterbo

The root cause of this bug is the VARCHAR(10) config_* fields and the reporter's MySQL-Server running in "strict mode".

Since the config_os_version string "Server 2003" is 11 chars in length, MySQL in standard config would just cut the string to "Server 200"; MySQL running in strict mode throws an error and stops script execution.

I reported the issue back to Clearcode and Pjotr is working on a fix; Which could just be raise the width of the config_* fields.

@sgiehl
Piwik Open Source Analytics member

small note: As soon as that plugin is finished, we should also move the translations to the core file (to be consitent with all other plugins included in core)

@mattab
Piwik Open Source Analytics member

The bug fields config_os_version and config_device_type are not large enough was fixed in #4203

@mattab
Piwik Open Source Analytics member

Unit tests for the UserAgentEnhanced were added in: #115

@mattab
Piwik Open Source Analytics member

Changing scope to: Move DevicesDetection out of beta and enabled by default.

@sgiehl
Piwik Open Source Analytics member

In 3149eb8: refs #3962 use UserAgentParserEnhanced instead of UserAgentParser to detect android & ios devices

@mattab
Piwik Open Source Analytics member

Increasing priority as more people report issues in UserAgentParser. We need to clarify our vision to the community.

Btw random idea; Maybe it would be a good idea to make UserAgentParserEnhanced a separate open source project, hosted in the piwik family at github?

For this ticket though, the concept is to reomve completely UserAgentParser from codebase and remove completely the previous User Settings reports. This will be awesome and has been long overdue :)

If anyone wants to own this ticket, please go ahead, this would make a great different to cleaning up the last "pending migration" to our superb new & organised Piwik platform.

@sgiehl
Piwik Open Source Analytics member

I'm currently writing a plugin, but when I'm done I could have a look at that task.

Moving the Useragentparser to a seperate repo sounds good. Would you create a repository and give me access. So we can move the required files there.

Btw. We need to merge my pull request regarding the devicedetection first! Do you have some time to have a look at it?

@mattab
Piwik Open Source Analytics member

Great to hear you'd be interested to work on this! :) it will be such a good feeling to deprecate the old UserSettings code and keep things tidy. I'll update the ticket description later with some clarifications in the spec.

Re: repo
Maybe we should pick a good name for this library... maybe UserAgent-ParserAwesome ? not sure... I have done a quick search around, and as far as I can see, we will be the best User Agent parser library around... this is cool!

we will directly compete with other libs such as https://github.com/tobie/ua-parser or many others...

@mattab
Piwik Open Source Analytics member

In d3a423d: Refs #3962 Removes undocumented feature of adding user agent column.

@mattab
Piwik Open Source Analytics member

See #4965: Use new device-detector library in Piwik to detect browsers, systems,
devices, brands and model

@mattab
Piwik Open Source Analytics member

In 6b28d41: Tweaking the Device detection titles. refs #3962
A nice creation by @sgiehl

@mattab
Piwik Open Source Analytics member

The universal Device Detector library is now available as Free software and hosted on github here: https://github.com/piwik/device-detector

We will now work on closing this ticket and finishing the loop nicely!

@mattab
Piwik Open Source Analytics member

see #5329 Enable DevicesDetection plugin by default

@mattab
Piwik Open Source Analytics member

We migrated to Device detector V2 in this pull request: #305 (comment)

Nice progress :)

@mattab mattab added this to the 2.5.0 - Piwik 2.5.0 milestone Jul 8, 2014
@fhemberger fhemberger pushed a commit that referenced this issue Jul 13, 2014
@mattab mattab Tweaking the Device detection titles. refs #3962
A nice creation by @sgiehl
6b28d41
@mattab mattab modified the milestone: Piwik 2.5.0, Piwik 2.6.0 Aug 4, 2014
@mattab mattab modified the milestone: Piwik 2.6.0, Piwik 2.7.0 Aug 25, 2014
@mattab mattab removed the Major label Sep 15, 2014
@mattab mattab modified the milestone: Piwik 2.9.0, Piwik 2.8.0 Sep 22, 2014
@sabl0r sabl0r pushed a commit to sabl0r/piwik that referenced this issue Sep 23, 2014
@sgiehl sgiehl refs #3962 use UserAgentParserEnhanced instead of UserAgentParser to …
…detect android & ios devices
3149eb8
@sgiehl
Piwik Open Source Analytics member

I've started to implement browser engine detection in piwik/device-detector#5263
Currently the detection in Piwik still runs with the old UserAgentParser based on the browser name, which is very inaccurate. We need to switch that, and maybe save the browser engine in the log table aswell.

@sgiehl sgiehl added a commit that referenced this issue Sep 28, 2014
@sgiehl sgiehl refs #3962 - implemented new detection of browser engines. Merged old…
… reports from UserSettings plugin to DevicesDetection plugin. Added segment option for browser engines. Added update script to move old archives and to update old visits with engine data
008ed65
@mattab mattab changed the title from DevicesDetection replaces UserSettings: device-detector is out of beta! to Deprecate UserSettings plugin -> use DeviceDetection instead! Oct 23, 2014
@mattab mattab modified the milestone: Piwik 2.9.0, Piwik 2.10.0 Oct 23, 2014
@sgiehl sgiehl self-assigned this Oct 24, 2014
@sgiehl sgiehl referenced this issue Oct 30, 2014
Merged

Remove duplicate reports from UserSettings plugin #6565

18 of 18 tasks complete
@mattab mattab added a commit that referenced this issue Nov 26, 2014
@mattab mattab Simplified widget selection, removed duplicate UserSettings<>DeviceDe…
…tection widgets and some other change now that we use Device Detector data #6565 #3962
aef0e89
@mattab
Piwik Open Source Analytics member

feedback after tests procedure done on demo2:

  • the "device type" report now shows all types even when they're 0, i think we can remove 0 visit row
  • UserSettings.getWideScreen widget was lost in the transition, it could redirect to the screen report
  • I checked UI test and they look legit changes, it should be green now
@sgiehl
Piwik Open Source Analytics member

Hm, UserSettings.getWideScreen widget should have been replaced with UserSettings.getScreenType here
I changed the device type report to show all by default. That way it's easy to see what piwik is able to report and you can easily open a transition for any device type, even if there were no visits with it in the current period.

@mattab
Piwik Open Source Analytics member

sorry wasnt clear, what I meant was that https://demo2.piwik.org/index.php?module=Widgetize&action=iframe&widget=1&moduleToWidgetize=UserSettings&actionToWidgetize=getWideScreen&idSite=1&period=day&date=yesterday&disableLink=1&widget=1&token_auth=xx returns Action 'getWideScreen' not found in the module 'UserSettings'. but BC should be redirected instead to new one

@mattab
Piwik Open Source Analytics member

I changed the device type report to show all by default. That way it's easy to see what piwik is able to report and you can easily open a transition for any device type, even if there were no visits with it in the current period.

that's a good idea. could we find an icon for Smart display ?

@sgiehl
Piwik Open Source Analytics member

I'll see if I can find an icon later

@sgiehl
Piwik Open Source Analytics member

@mattab I'm currently thinking about removing the ScreenType report completely. The current implementation is not really accurate. See https://github.com/piwik/piwik/blob/master/plugins/UserSettings/functions.php#L55
I don't think that the screentype can be detected by the resolution in a good way. Even my smartphone has a bigger resolution than that what is detected as mobile right now.
What do you think?

@mattab
Piwik Open Source Analytics member

@sgiehl Sounds good to kill this report. It was very low value and not correct anyway. It reminds me of this issue: #4663

@sgiehl
Piwik Open Source Analytics member

@mattab Besides fixing the update script, is there anything still open to solve this issue?
I remember we discussed to remove UserSettings plugin completely. Should the remaining reports be simply moved to DevicesDetection plugin? Otherwise I guess we are done here.

@mattab
Piwik Open Source Analytics member

Hi @sgiehl,

Regarding this issue, I think it is mostly done!

  • should we move UserSettings/images/browsers|os/* into DeviceDetection plugin?
  • make update script faster/ failsafe

Regarding next step: we can leave the UserSettings for now but maybe we can create an issue for the follow up task. Do you think it would make sense to:

  • move "Resolution" report to own plugin,
  • move "plugins tracking" to own plugin as well (maybe DevicePlugins?)
  • and move "Language report" to own plugin too.

(Kinda related to #6782)

@sgiehl
Piwik Open Source Analytics member

I have thought about moving the images to DevicesDetection plugin. The reason why I haven't done it yet is that I wasn't sure if it might break any older reports containing those as external images or something like that. Do you think it should be save moving them?

Moving those stuff to own/other plugins sounds good. So creating new tickets should be fine.

@mattab
Piwik Open Source Analytics member
@mattab
Piwik Open Source Analytics member
@sgiehl sgiehl added a commit that referenced this issue Dec 8, 2014
@sgiehl sgiehl refs #3962 - added note to changelog 250a186
@mattab mattab modified the milestone: Piwik 2.10.0 , Piwik 2.11.0 Jan 4, 2015
@sgiehl
Piwik Open Source Analytics member

All reports and API methods are moved now. Last remaining part is the page template showing the reports

@mattab
Piwik Open Source Analytics member

Nice work @sgiehl - it would be nice to get it done in 2.11.0 and this would successfully conclude this project! :)

@sgiehl
Piwik Open Source Analytics member

Hm. I'm not sure where to move the template displaying the various reports.
Should I move it to CoreHome and dynamically add the reports using an event or something like that?

@mattab
Piwik Open Source Analytics member

Not sure how this should be solved but it seems good use case for the platform, to let plugins enrich existing reports with their data. ideally we would provide a nice API for plugins to compose report pages.

  • Maybe we could add events similar to http://developer.piwik.org/api-reference/events#controllermoduleaction but that would be for Twig templates, where other plugins could "append" string content to it?
  • maybe better idea could be to replace the current way of creating reports eg. in Usersettings $view->dataTableResolution = $this->renderReport(new GetResolution());. Maybe we could introduce a new object for "Composite reports" (eg. a page in Piwik made of several reports). This object would let plugins add or modify reports to it, setting order, maybe right/left column, etc.?

Thoughts?

@sgiehl
Piwik Open Source Analytics member

I'm not sure how to solve that best. Maybe we should reorganize the technical reports.

We currently have:

  • Devices
    • Device type
    • Device brand
    • Device model
    • Browser (Versions)
    • OS (Versions)
    • Browser engine
  • Visitor settings
    • Resolution
    • Configurations (OS / Browser / Resolution)
    • Browser plugins
    • Browser language

I don't think that this splitting makes much sense. Maybe something like that would fit more:

  • Devices
    • Device type
    • Device brand
    • Device model
  • Browser & Operating System
    • Browser (Versions)
    • OS (Versions)
    • Browser engine
    • Resolution
    • Browser plugins

I would maybe remove the Configurations completely or make it available as widget only. Not sure if that report is useful at all.
The browser language reports could be moved to location reports maybe, as it is more related there.

@mattab
Piwik Open Source Analytics member

Maybe something like that would fit more:

maybe we could also move "Resolution" into "Devices" ?

Browser & Operating System

we need to find shorter name so it fits in the sub-menu... do you maybe have an idea?

I would maybe remove the Configurations completely or make it available as widget only. Not sure if that report is useful at all.

it's a useful report, some users have mentioned using it before (in the forums and in the tracker). Although maybe we could move this feature into a plugin that would be on the Marketplace rather than in core?

The browser language reports could be moved to location reports maybe, as it is more related there.

👍

@mattab
Piwik Open Source Analytics member

Hi @sgiehl the UserSettings plugin looks now quite "deprecated" and almost empty. maybe we close this issue and create smaller follow up issue?

@sgiehl
Piwik Open Source Analytics member

Yes. Let's create follow ups.

@sgiehl sgiehl closed this Feb 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment