Skip to content
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

give the user the option if anonymous data will sent #8346

Merged
merged 12 commits into from
Nov 10, 2015

Conversation

rdeutz
Copy link
Contributor

@rdeutz rdeutz commented Nov 8, 2015

updated PR description: #8346 (comment)

Original PR description:This PR implements a 6 hour grace time before a site sends data to the stats server. So anyone is able to disable the pluging before data will be send.

This PR implements a 6 hour grace time before a site sends data to the stats server. So anyone is able to disable the pluging before data will be send.
@rdeutz rdeutz added this to the Joomla! 3.5.0 milestone Nov 8, 2015
@zero-24
Copy link
Contributor

zero-24 commented Nov 8, 2015

@rdeutz can you add this "6 hour grace time" to the postinstall message too?

@infograf768
Copy link
Member

@rdeutz can you add this "6 hour grace time" to the postinstall message too?

Agree.

@rdeutz
Copy link
Contributor Author

rdeutz commented Nov 9, 2015

@infograf768 @zero-24 I also agree to add this to the post-install if we find testers and merge it

@infograf768
Copy link
Member

Made a new install. Will let you know exactly when the message is sent (if sent).

@rdeutz
Copy link
Contributor Author

rdeutz commented Nov 9, 2015

I have testet the delay now, in fact it gives 6 + 12 hours because we are checking if the delay between last run and now is bigger then 12 hours. That makes it 18 hours after install what is more then enough to disable a plugin. Atm it fails, I think it is because the stats server isn't running but I will check it.

#How to test

  • if you have a tool like LittleSnitch you can control network connections and see if there is something going out
  • You can set a breakpoint at line 93, if the execution stops the connection will go out.

@wilsonge
Copy link
Contributor

wilsonge commented Nov 9, 2015

The stats server has been running since 4/11/15

@rdeutz
Copy link
Contributor Author

rdeutz commented Nov 9, 2015

@wilsonge could also be me being not fast enough to allow the connection in LittleSnitch

@rdeutz
Copy link
Contributor Author

rdeutz commented Nov 9, 2015

disabled the network filter and it works, but my test doesn't count :-)

@phproberto
Copy link
Contributor

I have sent a new PR to @rdeutz rdeutz#2

Now on first run the plugin will show a message asking the user to select the mode it will use:

stats-message8

This allows users to:

  • Always allow to send data.
  • Send data once and get asked next time the stats try to get sent.
  • Never send data.

That message will be shown the first time the plugin is loaded and never again except user selects Once option that will ask the user permission each time the plugin wants to send new data.

In the plugin parameters I have added also some options to configure it:

stats-params

So now the users can adjust:

  • Interval for new stats being sent.
  • Mode of the plugin
  • Enable debug mode to test the plugin.

@Bakual
Copy link
Contributor

Bakual commented Nov 10, 2015

You change basically makes it opt-in. We wanted it opt-out explicitely.

If you want to make it opt-in, you can as well just deliver it unpublished and show a postinstall message where people can enable it. No need for any special code then.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 25abae2

When loading my test site this morning, the first thing it did was trying to get https://developer.joomla.org

So, a good test for me.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@phproberto
Copy link
Contributor

I don't think it's opt-in or opt-out. It's opt. No decision is taken for the user.

A disabled plugin + postinstall message wouldn't be so easy to setup for users. It would require 2-3 clicks to explicitly enable it. Here you hace both options in 30 pixels and 1 click away. I don't think is the same...

If you think that nobody is going to enable it then is a bad idea to put it there without asking users.

@Bakual
Copy link
Contributor

Bakual commented Nov 10, 2015

I don't think it's opt-in or opt-out. It's opt. No decision is taken for the user.

I assume if the user doesn't click anything, it will not send anything. Thus it's opt-in. The user has to actively enable it.

@phproberto
Copy link
Contributor

I assume if the user doesn't click anything, it will not send anything. Thus it's opt-in. The user has to actively enable it.

Users have to actively enable OR disable it. The only mandatory thing (if user doesn't want to live with a message always shown in backend) is that they decide the initial mode.

Thus it's just opt :P

@phproberto
Copy link
Contributor

PR updated!

@Bakual
Copy link
Contributor

Bakual commented Nov 10, 2015

Users have to actively enable OR disable it.

The question is what happens if they don't do anything? Answer is: It doesn't send any data. Thus it's opt-in.
Granted, they keep having an annoying message on the screen until they disable it. Similar to postinstall messages, but more annoying 😄

Allow users to accept & configure stats sending
@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @infograf768


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Nov 10, 2015
@rdeutz
Copy link
Contributor Author

rdeutz commented Nov 10, 2015

@Bakual we bug them since they make a decision :-)

PLG_SYSTEM_STATS_INTERVAL_DESC="Stats will be sent each X hours. Default is 12"
PLG_SYSTEM_STATS_INTERVAL_LABEL="Interval (hours)"
PLG_SYSTEM_STATS_LABEL_MESSAGE_TITLE="Joomla! would like your permission collect some basic statistics."
PLG_SYSTEM_STATS_MODE_DESC="Selec the way you want that statistics are sent"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+PLG_SYSTEM_STATS_MODE_DESC="Select the way that you want the statistics to be sent."

@rdeutz
Copy link
Contributor Author

rdeutz commented Nov 10, 2015

@infograf768 deleting the params for the plugin should do the job

@@ -4,9 +4,23 @@
; Note : All ini files need to be saved as UTF-8

PLG_SYSTEM_STATS="System - Joomla! Statistics"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference the style guide says

Whenever you can substitute "your Joomla CMS" in place of "Joomla" and still make sense, then it's descriptive and no bang needed.
So in this case I erred on J to mean the project and not the CMS so does need the !

@phproberto
Copy link
Contributor

rdeutz#3

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @infograf768


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@infograf768
Copy link
Member

@rdeutz
does not work here after deleting params = I do not get the Notice in Control Panel

@infograf768
Copy link
Member

Found out: the plugin has to be enabled first (as it does for updates or new installs)
Works nicely!

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 6d8f1b7


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@phproberto
Copy link
Contributor

I missed one typo. rdeutz#4

BTW Thanks for checking this @brianteeman @infograf768 !

PLG_SYSTEM_STATS_MODE_OPTION_NEVER_SEND="Never send"
PLG_SYSTEM_STATS_MODE_OPTION_ON_DEMAND="On demand"
PLG_SYSTEM_STATS_MSG_ALLOW_SENDING_DATA="Enable Joomla Statistics?"
PLG_SYSTEM_STATS_MSG_JOOMLA_WANTS_TO_SEND_DATA="In order to better understand our install base and end user environments, this plugin has been created to send those statistics back to a Joomla! controlled central server. No identifying data is captured at any point. You can change this settings later from Plugins > System Joomla! Statistics."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 21 "...You can change this settings later from..." should be "change this setting"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot - I am not wondering if it should be
change these settings

@Hils what do you think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be either - I can't look at the context just now (away) - maybe settings is more comfortable?

@Webdongle
Copy link
Contributor

I have tested this item ✅ successfully on 6d8f1b7

@rdeutz Great thanks

@jtester works


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @infograf768, @Webdongle


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@phproberto
Copy link
Contributor

I used "these changes" because there is more than one setting being applied.

rdeutz#6

Yet more typos :( 
We can squash when we merge it finally
@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @infograf768, @Webdongle


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@Chris-Jones-Gill
Copy link

Tested in 2 ways, with slightly differing results.

Methods

  1. Update from 3.4.5 to 3.5.0-beta, apply patch for PR using PatchTester
  2. New 3.5.0-beta install, but applying this PR before opening in browser

Results

open System - Control Panel
No decision box shown.
New post install message indicated.
New message reads

Stats Collection in Joomla

Since version 3.5.0

Since Joomla 3.5 Joomla contains an anonymous statistics tracking plugin that provides your Joomla version, PHP version, database engine and version, and server operating system. This data is collected in order to ensure that as we develop new versions of Joomla we provide the most optimal software taking advantage of the latest database and PHP features but without loosing significant numbers of users. It became especially clear this was required after the requirement of PHP 5.3.10 was implemented after the release of Joomla 3.3 for implementation of BCrypt passwords.

We are making this data publicly available through our API and we will be providing graphs representing the Joomla version, PHP versions and database engines of sites reporting their data in the interest of transparency to the community and to help 3rd party developers.

If you do not wish to provide the Joomla! project with this information you can simply disable the plugin called System - Joomla! Statistics

Open Plugins -> System - Joomla Statistics
All options showing, but mode is set to "Always send"

I guess that nobody would update a live system in this manner anyway, but I have included the results as a point of interest.

Works as expected
First login to Adminstrator looks like this...
3 5_beta_test_-administration-control_panel-_2015-11-10_14 57 31

@infograf768
Copy link
Member

2 tests. RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8346.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 10, 2015
@wilsonge
Copy link
Contributor

Thanks guys :) Merged

wilsonge added a commit that referenced this pull request Nov 10, 2015
give the user the option if anonymous data will sent
@wilsonge wilsonge merged commit d814580 into joomla:staging Nov 10, 2015
@rdeutz rdeutz deleted the implement_first_run_delay branch November 16, 2015 21:12
@wilsonge wilsonge removed the RTC This Pull Request is Ready To Commit label Jan 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.