Opened 4 years ago

Closed 4 years ago

#1374 closed New feature (fixed)

VisitorGenerator plugin

Reported by: halfdan Owned by:
Priority: normal Milestone: Piwik 0.6.3
Component: Plugins Wishlist Keywords:
Cc: Sensitive: no

Description

After #762, #1369 and #1371 I took the liberty and refactored the generateVisits.php into an own plugin (see Attachment). Basically the plugin replaces the generateVisits.php and introduces a smarter interface which can be called from a browser. It does not provide a CLI though.
When the plugin is activated it adds itself to the AdminMenu (if the user is SuperUser).

I set milestone to 0.6.2 as I think this can be easily included into the core. Fell free to change this to third party plugins.

Attachments (6)

visitorGenerator_form.png (36.4 KB) - added by halfdan 4 years ago.
visitorGenerator_result.png (31.4 KB) - added by halfdan 4 years ago.
VisitorGeneratorPlugin.patch (154.1 KB) - added by halfdan 4 years ago.
VisitorGenerator.zip (28.5 KB) - added by halfdan 4 years ago.
VisitorGenerator plugin + en.php.patch
API.patch (4.0 KB) - added by halfdan 4 years ago.
templates.patch (3.5 KB) - added by halfdan 4 years ago.

Download all attachments as: .zip

Change History (26)

Changed 4 years ago by halfdan

Changed 4 years ago by halfdan

comment:1 Changed 4 years ago by vipsoft (robocoder)

Just took a cursory look. For future reference, please indent using tab stops every 4 spaces. See wiki:CodingStandard

Are you sure you meant #762? That ticket was to move core/Tracker/Generator.php, core/Tracker/Generator/*, and misc/generateVisitsData/* into a plugin.

comment:2 Changed 4 years ago by halfdan

Mh, indeed #762 is a wrong reference.

I re-uploaded the archive. Tab indent should comply to the Coding Standard now.

comment:3 Changed 4 years ago by matt (mattab)

I think this is a good improvement to the current visit generator. code review:

  • have the language strings in the langs/en.php file. Language strings should not contain line breaks or HTML. It is better to create 3 different strings for the 'VisitorGenerator_Warning'
  • instead of action="index.php?module=VisitorGenerator&action=generate" you can use {url module=VisitorGenerator action=generate
  • move misc/generateVisitsData/ into the plugin
  • remove @author and change plugin info to be the standard core plugins info (no author, points to piwik.org, etc.)
  • change the message 'API_QuickDocumentation' (which was bad practise anyway as it contains links, html, etc.) on the API page to explain how to generate fake visits (enable the plugin, etc.).

We will use data from this website as golden logs: divezone.net - diving guide since it is a classic piwik use case website.

comment:4 Changed 4 years ago by vipsoft (robocoder)

IIRC core/Tracker/Generator/* has hardcoded references to misc/generateVisitsData, so it would be more self-contained to move these into the plugin as well.

comment:5 Changed 4 years ago by halfdan

matt: Alright, thanks for review. Adjusted everything and added patch to add the plugin.

I moved core/Tracker/Generator.php, core/Tracker/Generator/* and misc/generateVisitsData/* into the plugin.

comment:6 Changed 4 years ago by matt (mattab)

code review

  • Maybe the UI could use a table to have the labels aligned (like in the User Settings page).
  • split the 'API_QuickDocumentation' help string into multiple strings (so that there is no HTML in it, and the visit generator help string is in a different string)
  • the generator help string would be displayed only for the super user ( if($isSuperUser) in smarty. The message could read "If you don\'t have data for today you can first generate some data using the VisitorGenerator plugin. You can enable the VisitGenerator plugin, then click on the 'Visitor Generator' menu in the Piwik Admin area.
  • in the language, It will <b>not</b> should be It will %s not %s and %s replaced by <b> at run time
  • it seems you removed the 'token_auth' check, but it is important (against CSRF). You can use $this->checkTokenInUrl(); (see example in UsersManager.Controller.setIgnoreCookie )

comment:7 Changed 4 years ago by halfdan

New patch, all fixed.

comment:8 Changed 4 years ago by matt (mattab)

The patch seems to be missing the files core/Tracker/Generator/* in the plugin itself?

comment:9 Changed 4 years ago by halfdan

They have been moved into the plugin itself (see Tracker.php, etc.)

comment:10 Changed 4 years ago by matt (mattab)

yes but they are not in the patch, so when applying the patch, i get the error 'file missing xxx'

comment:11 Changed 4 years ago by halfdan

Very strange - reuploaded the patch, according to the trac output it should fix the problem.

comment:12 Changed 4 years ago by matt (mattab)

it seems your patch is not against trunk. For example, plugins/VisitorGenerator/Generator.php is a DIFF but this file doesn't exist in trunk, so it should be the full file in the patch. do you see what I mean?

comment:13 Changed 4 years ago by halfdan

matt: Yep, I see. Sorry for that, I exported another patch (this time using Netbeans - looked good to me). I seriously need to switch back to my Linux development machine, SVN under Windows is a pain :(

Changed 4 years ago by halfdan

VisitorGenerator plugin + en.php.patch

comment:14 Changed 4 years ago by matt (mattab)

(In [2245]) Refs #1374 Moving visitor generator tool to its own plugin. Patch by halfdan!

Also renamed setPluginsToLoad to loadPlugins()
Note: still missing updates to the API plugin help as this part was missing from the patch.

comment:15 Changed 4 years ago by matt (mattab)

Thanks for the patch!

Note:

  • the patch was not deleting the files in core/Tracker/Generator.php and Tracker/Generator/* but I deleted them manually.
  • the patch was missing the part for the API template to use the new help strings.

can you please submit patch for the API template + adding the english strings as well (I had to temporarily remove them).

Thanks!

Changed 4 years ago by halfdan

comment:16 Changed 4 years ago by halfdan

I knew I missed something ;) Added the patch for the API template. Thanks for integrating that into the core.

comment:17 Changed 4 years ago by matt (mattab)

  • Resolution set to fixed
  • Status changed from new to closed

(In [2247]) Fixes #1374 Patch by halfdan

I modified it a bit to simplify the translation messages

comment:18 Changed 4 years ago by halfdan

Just noticed that my patch is missing the templates for the VisitorGenerator plugin *sigh*. It's attached now..

Changed 4 years ago by halfdan

comment:19 Changed 4 years ago by vipsoft (robocoder)

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:20 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [2257]) fixes #1374 - check in the missing templates

Note: See TracTickets for help on using tickets.