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

[4.1] Simplify statistic plugin #35983

Merged
merged 8 commits into from
Jan 14, 2022

Conversation

bembelimen
Copy link
Contributor

Based on the minutes from 19th October 2021 I recreated the statistic plugin:

  • Removed the "once" button, as this option is not really useful for the statistics.
  • Made more clear which data are sent (made the link to open up information more prominent)
  • Pointed out, that the data are anonymous

Before

grafik

Now

grafik

Remove "only once" data
Recreate text.
@bembelimen bembelimen requested a review from wilsonge as a code owner November 7, 2021 02:26
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.1-dev labels Nov 7, 2021
@bembelimen bembelimen added this to the Joomla 4.1 milestone Nov 7, 2021
Copy link
Member

@Kubik-Rubik Kubik-Rubik left a comment

Choose a reason for hiding this comment

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

Please also update the JavaScript code (media/plg_system_stats/js) of this plugin to reflect the "once" removal.

@chmst
Copy link
Contributor

chmst commented Nov 8, 2021

I have tested this item ✅ successfully on 669cf52


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

@joomla-cms-bot joomla-cms-bot added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Nov 8, 2021
@bembelimen
Copy link
Contributor Author

Please also update the JavaScript code (media/plg_system_stats/js) of this plugin to reflect the "once" removal.

Good find, thx, is fixed.

@brianteeman
Copy link
Contributor

The english changes here are not very good and they are sending the wrong message. I agree about removing the send once but the rest is not correct. Also it is not clear what happens to sites that have previously said "once" when the plugin is updated.

@bembelimen
Copy link
Contributor Author

The english changes here are not very good and they are sending the wrong message.

thanks for the feedback, could you elaborate on that? What would you change?

Also it is not clear what happens to sites that have previously said "once" when the plugin is updated.

Nothing, as they sent it already and now the plugin is disabled.

@brianteeman
Copy link
Contributor

Also it is not clear what happens to sites that have previously said "once" when the plugin is updated.

Nothing, as they sent it already and now the plugin is disabled.

then the quality of the stats will not improve

@bembelimen
Copy link
Contributor Author

then the quality of the stats will not improve

Not for the old one, but for the new one. It's all about sending regular data. "Once" is a "no" for regular data. I personaly would have prefered a opt-out solution and collecting more (anonymous) data, but this was just not possible. So the solution is to get more people clicking on "yes" by being more detailed.

@brianteeman
Copy link
Contributor

Honestly its a waste of time. This will have minimal impact on improving the statistics and their usefulness. More people will not be clicking on yes because the majority of people will never see it. The only way to do that would be to reset the plugin status when this new version is installed and ask EVERYONE the question.

@HLeithner
Copy link
Member

Honestly its a waste of time. This will have minimal impact on improving the statistics and their usefulness. More people will not be clicking on yes because the majority of people will never see it. The only way to do that would be to reset the plugin status when this new version is installed and ask EVERYONE the question.

I like the idea to reset the current status of the plugin if it is set to "once".

@chmst
Copy link
Contributor

chmst commented Nov 25, 2021

My2cent: 2 buttons, left no, right yes are a good solution.

Eyes (in ltr countries) go from left to right and stay on the most right button. This is why all consent messages have the "yes" button right. So I think that changing the order of buttons will have an impact.

Deciding between 2 alternatives is easy, 3 options make the user think. A normal user cannot know what's the difference between options always and once. What means "once"?

@Krshivam25
Copy link
Contributor

I have tested this item ✅ successfully on 40ee732


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

PLG_SYSTEM_STATS_BTN_NEVER_SEND="Never"
PLG_SYSTEM_STATS_BTN_SEND_ALWAYS="Always"
PLG_SYSTEM_STATS_BTN_NEVER_SEND="No"
PLG_SYSTEM_STATS_BTN_SEND_ALWAYS="Yes, I'll help Joomla!"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PLG_SYSTEM_STATS_BTN_SEND_ALWAYS="Yes, I'll help Joomla!"
PLG_SYSTEM_STATS_BTN_SEND_ALWAYS="Yes, I'll send statistics"

@HLeithner
Copy link
Member

I'm I the only one that would the yes button expect on the left side? Maybe I'm brain washed by windows but the primary button is always left (on windows) I think mac is diffrent?

@brianteeman
Copy link
Contributor

Its all irrelevant if we dont reset the plugin

@zero-24
Copy link
Contributor

zero-24 commented Nov 25, 2021

I'm I the only one that would the yes button expect on the left side? Maybe I'm brain washed by windows but the primary button is always left (on windows) I think mac is diffrent?

Thats another argumend raised by me on the discussion before so its not just you...

@zero-24
Copy link
Contributor

zero-24 commented Nov 25, 2021

Its all irrelevant if we dont reset the plugin

Do we have the right to reset such thing? The decison to only send them once (while it does not make much sense) is a decision we have to respect right? Now its gone and the most accurate replacement is the new "no" option, IMO we should have disabled the plugin on the "no" and "only once" button click in the first place to be correct so here just reset the thing.

If we want to I would say the only thing we should do would be to ask for a new decision on the "only once" users but keep the "no" decision intact.

PLG_SYSTEM_STATS_MSG_JOOMLA_WANTS_TO_SEND_DATA="To better understand our install base and end user environments it is helpful if you send some site information back to a Joomla! controlled central server. No identifying data is captured at any point. You can change these settings later from Plugins → System - Joomla! Statistics."
PLG_SYSTEM_STATS_MSG_WHAT_DATA_WILL_BE_SENT="Select here to see the information that will be sent."
PLG_SYSTEM_STATS_MSG_JOOMLA_WANTS_TO_SEND_DATA="We want to increase the compatibility of our Joomla! software with our user's server settings. Therefore we need anonymous data from your site to better understand the install base and end user environments. All data is anonymised and only sent to a Joomla! controlled central server. You can change these settings later from Plugins → System - Joomla! Statistics."
PLG_SYSTEM_STATS_MSG_WHAT_DATA_WILL_BE_SENT="No identifying data is captured at any point. Select here to review."
Copy link
Contributor

Choose a reason for hiding this comment

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

You know that atleast in germany the IP is interpreted as identifying data so just tell them that technically we collect the IP but do not use that.

@zero-24
Copy link
Contributor

zero-24 commented Nov 25, 2021

And as noted above the "no" button should go and disable the plugin all together there is no point in running that plugin all the time when its not sending anything at all.

@HLeithner
Copy link
Member

I think the plugin should use the task scheduler... ok sorry a bit offtopic^^

Resetting "only once" should be ok, because this options doesn't exists anymore. The other options should be as they are now. On the other side you can argument that "only once" means for us "no" after this... don't care if it's only for new installation it's fine for me too.

@brianteeman
Copy link
Contributor

The IP of the server is not PII just the users internet connection IP

@richard67
Copy link
Member

@Shubhamverma2796 I can see a positive test from you counted for this pull request, but it seems you have not used the "Test this" button in the issue tracker to submit your test result, you have used the "Alter test" button, so we can see that only in the issue tracker but not on GitHub.

2021-11-28_pr-35983-test-item-change

If you have really tested this PR, then please use the "Test this" button in the issue tracker https://issues.joomla.org/tracker/joomla-cms/35983 to submit your test result.

In the issue tracker and on GitHub you will see than a comment like this:

2021-11-28_pr-35983-test-item-submitted

The "alter test" button is meant to correct a previously submitted test result, e.g. in case if you have forgotten to select the appropriate result before submitting.

Thanks in advance.

@ditsuke
Copy link
Contributor

ditsuke commented Jan 8, 2022

I have tested this item ✅ successfully on 236b5eb


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

1 similar comment
@softforge
Copy link
Contributor

I have tested this item ✅ successfully on 236b5eb


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.1 milestone Jan 8, 2022
@alikon
Copy link
Contributor

alikon commented Jan 8, 2022

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 8, 2022
@richard67 richard67 added this to the Joomla 4.1 milestone Jan 8, 2022
@brianteeman
Copy link
Contributor

On the other side you can argument that "only once" means for us "no" after this...

That's why it is the way it is now. If we reset "only once" (which means once then never), we could reactivate all plugins ... Which means we have to republish it and set the parameter.
I have no feelings about this additional step, if someone delivers the code.

If we don't reset then we will not get any improved stats and this PR is pointless.

I see no issue in doing this. It will not break the site.

@bembelimen
Copy link
Contributor Author

So I thought a lot over if we should reset the decisions the user made (at least for "once") or not.
At the end I think, that the "once" was a decision of the user to not send regular data, so we should respect it. Also was the idea of the PR only to improve the text.

So I will go with the current version.

@bembelimen bembelimen merged commit 1e27199 into joomla:4.1-dev Jan 14, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 14, 2022
@brianteeman
Copy link
Contributor

Your choice of course but the plugin is still producing (or more accurately not producing) any usable statistics

@bembelimen
Copy link
Contributor Author

Your choice of course but the plugin is still producing (or more accurately not producing) any usable statistics

I don't disagree, but that was not the intention of this PR. Probably for 4.2 a complete rewrite could happen. @fancyFranci @roland-d

@roland-d
Copy link
Contributor

A rewrite is fine but first we need a description of what it should be doing, otherwise there is no use doing a rewrite. We need to have a list of requirements that the plugin should fulfill. If we do that, are we actually using the data collected...

@zero-24
Copy link
Contributor

zero-24 commented Jan 15, 2022

I still disagree with the texts using suggestive language, its a bad sign to me that we think we have to use such suggestive language to gather questionable statistics.. :(

@brianteeman
Copy link
Contributor

Just to point out that while the joomla stats only show 3% on j4.
Stats from akeeba show ten times that number.

@zero-24
Copy link
Contributor

zero-24 commented Jan 20, 2022

Jup but that is not fixed by just changing the language and removing the once option. And as mentiond by Nic the serverside (of our statistics) have to be changed too like invalidate old reports etc.

And we have to keep in mind that even when its widly used akeeba is not installed on all sites but maybe more specificly used on well maintained sites.

@brianteeman
Copy link
Contributor

I've not said remove the once option. Just that we should ask again (even if they said once in the past) for a major new release.

I thought the serverside part of the stats had been updated sometime ago to invalidate any reports from more than x ago

Of course akeeba stats will be skewed towards the higher end. But its a good indicator that the stats we have and supposedly use to determine various things are way off

@zero-24
Copy link
Contributor

zero-24 commented Jan 20, 2022

I'm not aware of any such change on the serverside but lucky me not everything goes via my desk. :-D

By the commit logs we only had php and joomla version changes latly.

@brianteeman
Copy link
Contributor

I must have been thinking of this joomla/statistics-server#60

@zero-24
Copy link
Contributor

zero-24 commented Jan 20, 2022

Possibly :-)

@cyrezdev
Copy link
Contributor

cyrezdev commented Feb 5, 2022

I agree with @brianteeman about reset for all, as the plugin will be changed. If the message is clear about why Joomla needs stats, it will have more effect, and some may say yes when they click no by the past.
In case of update, it would need maybe an additionnal information message (alert) to inform why we ask again.

@bembelimen bembelimen deleted the 4.1/statistic-plugin branch March 15, 2024 00:08
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 NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.