Joomla! Issue Tracker | Joomla! CMS #8386 - Statistics plugin improvements

? ?

Closed
29 Jan 2016
Medium
Build: staging
Easy Test
# 8386
Diff
phproberto:stats

Success

Success Human Test Results: 3 Successful 0 Failed. Details

User tests: Successful: Unsuccessful:

phproberto
12 Nov 2015

Description

This changes the JFormField to:

Add a new render() function that can be used to render layouts inside other layouts.
Allow to easily override the layout include paths in any child field creating an override of the getLayoutPaths() method.

Render layouts inside other layouts is used like:

<a href="http://joomla.org">Joomla</a>
<?php 
    // Render `sample.layout` layout inside this layout
    echo $field->render('sample.layout', $displayData);
?>
<p>Another paragraph</p>

You can see below how I used it to render a common stats layout that is renderer in both message and field.data layouts.

This PR also improves the statistics system plugin:

Changed plugin fields prefix to plgsystemstats to avoid conflicts with any extension using stats prefix. Before: type="stats.uniqueid" after: type="plgsystemstats.uniqueid"
Remove uniqueid field getInput() method and use the fields layout system so 100% of the plugin output is overridable.
Create new field data that will be used to render the stats that are going to be sent in the plugin settings view.

How to test

This changes the field layout system so you should test that fields that are currently using layouts work as expected. The only fields using it are JFormFieldRadio & JFormFieldCheckboxes so the best way to check is to edit the codemirror plugin that includes both fields in its settings.
Enable debug in statistics system plugin and test that the message selecting the user to configure statistics mode is shown. Also confirm that when you click in Click here to see which information will be sent. data is shown below.
Go to statistics system plugin settings and confirm that when you click in Click here to see which information will be sent. data appears below.

phproberto - open - 12 Nov 2015

phproberto - change - 12 Nov 2015

Status

New

⇒

Pending

joomla-cms-bot - change - 12 Nov 2015

Labels

Added: ?

phproberto - change - 12 Nov 2015

The description was changed

joomla-cms-bot - change - 12 Nov 2015

Labels

Added: ?

phproberto - comment - 12 Nov 2015

I have also cleaned versions & added a language string for each data sent

zero-24 - change - 12 Nov 2015

Milestone

Added:

brianteeman - test_item - 12 Nov 2015 - Tested successfully

brianteeman - comment - 12 Nov 2015

I have tested this item successfully on 4dff4a2

Great stuff - works as described

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

brianteeman - change - 12 Nov 2015

Category

⇒

Plugins

brianteeman - change - 12 Nov 2015

Labels

phproberto - change - 19 Nov 2015

Labels

joomla-cms-bot - comment - 19 Nov 2015

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

phproberto - comment - 19 Nov 2015

I have updated the PR and added an extra check that will show the error message from the stats server if something when wrong (like data validation).

zero-24 - change - 25 Nov 2015

Labels
Easy	No	⇒	Yes

roland-d - test_item - 25 Nov 2015 - Tested successfully

roland-d - comment - 25 Nov 2015

I have tested this item successfully on e18b20c

Works as described, the statistics plugin data is shown. The settings in CodeMirror keep working as they did before.

@phproberto This is more than just a statistics plugin improvement if I understand it correctly. You also changed the way JLayouts work by adding the getLayoutPaths() method by which form fields can override the list of paths to check the layout files. In the uniqueid.php and data.php we get duplicate code but I guess that can't be prevented the way it is setup now.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

joomla-cms-bot - comment - 26 Nov 2015

This PR has received new commits.

CC: @brianteeman, @roland-d

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

phproberto - comment - 26 Nov 2015

I have updated the PR and added some changes:

Avoid to make public getRenderer() methods in JFormField as suggested by @wilsonge and create a new public method called render(). That method will be used when you want to display a layout inside another field layout.
I have created a new base field for the plugin fields to avoid the duplicated code reported by @roland-d
I have ensured that the data field only shows/hides the data attached to it.

phproberto - comment - 26 Nov 2015

And yes @roland-d I have modified the JFormField because I found that extensions (which is the case of this plugin) need to setup their own layout paths for fields.

infograf768 - comment - 3 Dec 2015

Tested OK for codemirror and stats message.
Concerning that one I suggest to add some <p> in the layout as the link is not very visible on the blue back-ground.
Instead of

we would get

Gerlof - comment - 8 Dec 2015

Am I right that this PR cannot be tested using Joomla! Patch Tester?
Clicking [Apply Patch] shows message "The file marked for modification does not exist: plugins/system/stats/layouts/message.php"

Bakual - comment - 8 Dec 2015

@gerlof You need to test this on staging. 3.5.0-beta may not work.

Gerlof - comment - 8 Dec 2015

@Bakual Joomla! Update shows

"You are on the "Testing" update channel. This channel is designed for testing new releases and fixes in Joomla.
It is only intended for JBS (Joomla! Bug Squad™) members and others within the Joomla community who are testing. Do not use this setting on a production site.
You already have the latest Joomla version, 3.5.6-dev."

How to change "3.5.6.-dev" into the right version? I think I have to change a setting in a db table, but which one to pick? ;)

Bakual - comment - 8 Dec 2015

I have no clue how you got onto a 3.5.6-dev. That version doesn't exist.
I would just set up a new Joomla instance instead of trying to fix a broken one.

For staging, you can download https://github.com/joomla/joomla-cms/archive/staging.zip and install from that.

Gerlof - comment - 8 Dec 2015

@Bakual Yes, strange, isn't it? This happened after installing 3.5 Beta.

Gerlof - comment - 8 Dec 2015

@Bakual Note: wrong version number was caused by the file libraries/cms/version/version.php, from line 35:

/**
* Maintenance version.
*
* @var string
* @since 3.5
*/
const DEV_LEVEL = '6-dev';

Bakual - comment - 8 Dec 2015

Ah. @mbabker fixed that: 5a20901

joomla-cms-bot - comment - 9 Jan 2016

This PR has received new commits.

CC: @brianteeman, @roland-d

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

phproberto - comment - 9 Jan 2016

Updated copyrights to 2016

infograf768 - comment - 10 Jan 2016

Any reason for using
+ * @var boolean $spellchec Spellcheck state for the form field.
instead of
+ * @var boolean $spellcheck Spellcheck state for the form field.
?

dgt41 - comment - 10 Jan 2016

@infograf768 the way the vars get populated for the layout is by calling the parent class field and then merge any other, specific vars for the field. So this comment block is, for the most of it, generic...
Check https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/form/fields/checkboxes.php#L150-L167
and
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/form/field.php#L952-L990

Fedik - comment - 10 Jan 2016

@dgt41 I think @infograf768 mean $spellchec vs $spellcheck

dgt41 - comment - 10 Jan 2016

@Fedik oops, that's a typo

0b16e61 28 Jan 2016

[imp] make JFormField::getRenderer() public and allow to override layout paths

43425a4 28 Jan 2016

[imp] plg_system_stats improvements

c5113de 28 Jan 2016

[imp] clean versions & translation strings for send data in plg_system_stats

c718f58 28 Jan 2016

[imp] show error message when sending stats fails

6acb656 28 Jan 2016

[imp] keep getRenderer private and create render() public functions

5aa3d29 28 Jan 2016

[imp] change copyright

7fb97f4 28 Jan 2016

[fix[ typo

joomla-cms-bot - comment - 28 Jan 2016

This PR has received new commits.

CC: @brianteeman, @roland-d

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

phproberto - comment - 28 Jan 2016

Updated again. Please if this is not going to be merged save us time and close it.

andrepereiradasilva - comment - 28 Jan 2016

Nice work.
One question: Should the stats plugin autoload the language?
https://github.com/phproberto/joomla-cms/blob/43425a448031db5af1f95a09bd9bfcab1a7e4894/plugins/system/stats/stats.php#L52

infograf768 - test_item - 29 Jan 2016 - Tested successfully

infograf768 - comment - 29 Jan 2016

I have tested this item successfully on 7fb97f4

One more tester needed.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

Fedik - comment - 29 Jan 2016

all works if follow the test instruction

However I noticed issue, but not sure if it related to changes made in current pull request.
When in the request permission I chose "allow" all work fine. But when I chose "once" or "never" then the request permission message annoying me after each page refresh.

upd: seems it not because changes in current pull request, it something old

Gerlof - test_item - 29 Jan 2016 - Tested successfully

Gerlof - comment - 29 Jan 2016

I have tested this item successfully on 7fb97f4

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

MATsxm - test_item - 29 Jan 2016 - Tested successfully

MATsxm - comment - 29 Jan 2016

I have tested this item successfully on 7fb97f4

tested again - all fine here
Thanks!

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8386.}

wilsonge - change - 29 Jan 2016

Status	Pending	⇒	Closed
Closed_Date	0000-00-00 00:00:00	⇒	2016-01-29 12:42:27
Closed_By		⇒	wilsonge

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#8386] - Statistics plugin improvements

Description

How to test

Add a Comment