This patch add the possibility of sharing Koha usage statistics with the community on http://hea.koha-community.org/ The data sent anonymously can be find here : http://wiki.koha-community.org/wiki/KohaUsageStat_RFC#Which_data_are_sent_.3F It add a choice in the web installer to send, not send or remind later to share these stats. If you choose 'Yes', it will send anonymously stats every months. If you choose 'No', it will not send your stats and never remind you to do it. If you choose 'Later', it will remind you to make a choice in 7 days on the Administration page. This message has the same 3 possibilities : 'Yes' : it will send anonymously stats every months. 'No' : it will not send your stats and never remind you to do it. 'Later' : it will show you again this message in 7 days. If you have an already installed koha, it will show you directly the message on the Administration page with the same choices and effect than above. This patch depends on Bug 13585 - HEA - Cronjob Setup which will activate the monthly cron to actually share statistics.
Created attachment 41212 [details] [review] Bug 14608 : Add a message on the Administration page to share statistics This patch add a message on the Administration page to share your usage statistics to hea.koha-community.org If you click on : 'Yes' : It set the 'UsageStats' syspref to 1 (activate sharing) and the 'UsageStatsLastDisplay' syspref to 0. 'Later' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to the today's date and it will show again the message after 7 days. 'No' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to 0.
Created attachment 41213 [details] [review] Bug 14608 : Add Hea activation in Koha Web Installer This patch add the possibility to share your usage statistics to hea.koha-community.org in the web installer. If you click on : 'Yes' : It set the 'UsageStats' syspref to 1 (activate sharing) and the 'UsageStatsLastDisplay' syspref to 0. 'Later' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to the today's date. A message on the Administration page will offer you again the possibility of sharing your stats in 7 days. 'No' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to 0.
This patch is playing with 2 sysprefs in Home › Administration › System preferences > Administration : 'UsageStats' which allow or not the stats sharing => it can take 2 values : 'share' or 'don't share' 'UsageStatsLastDisplay' which is the last time you see the message asking you if you want to share your stats. => its value is free but : - empty means you never see the message - 0 means you don't want to see the message - XXXXXXXXXXX is the last time you see the message (in ms from epoch) TEST PLAN --------- Make a new Koha install At the end of step 3, you should see the possibility of sharing your usage statistics with the community, with 3 possibilities : - Yes - --------- --> set the 'UsageStats' syspref to "share" This enable the stats sharing. --> set the 'UsageStatsLastDisplay' syspref to 0 This prevent showing the message in the administration page. You should never see it. - Later - --------- --> set the 'UsageStats' syspref to "don't share" This disable the stats sharing. --> set the 'UsageStatsLastDisplay' syspref to today's date (in ms from epoch) You shouldn't see the message before 7 days. Try to change your system clock to NOW+7 days, then reload the Administration page, you should see the message again. - No - --------- --> set the 'UsageStats' syspref to "don't share" This disable the stats sharing. --> set the 'UsageStatsLastDisplay' syspref to 0 This prevent showing the message in the administration page. You should never see it.
The new pref should be added to the updatedb (atomicupdate file).
The UsageStatsLastDisplay pref should be hidden (like UsageStatsID and usagestatslastupdatetime).
Otherwise everything looks good. Marked as Failed QA.
Created attachment 41712 [details] [review] Bug 14608 : Adding the new syspref UsageStatsLastDisplay to the updatedb (atomicupdate dir) In response of Joubu comment #4
Created attachment 41713 [details] [review] Big 14608 : Hide UsageStatsLastDisplay syspref In response of Joubu comment #5
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref
Created attachment 55773 [details] [review] Bug 14608 : Add a message on the Administration page to share statistics This patch add a message on the Administration page to share your usage statistics to hea.koha-community.org If you click on : 'Yes' : It set the 'UsageStats' syspref to 1 (activate sharing) and the 'UsageStatsLastDisplay' syspref to 0. 'Later' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to the today's date and it will show again the message after 7 days. 'No' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to 0. This patch also add the possibility to enable the sharing feature from the web installer. If you click on: 'Yes' : It set the 'UsageStats' syspref to 1 (activate sharing) and the 'UsageStatsLastDisplay' syspref to 0. 'Later' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to the today's date. A message on the Administration page will offer you again the possibility of sharing your stats in 7 days. 'No' : It set the 'UsageStats' syspref to 0 and the 'UsageStatsLastDisplay' syspref to 0.
All patches squashed and rebased on master
I am going to rework this code.
Created attachment 58340 [details] [review] Bug 14608: Add a page to configure shared statistics This patch set adds: - a reference to Hea at the end of the installation process - a link to the new page from the admin home page - a new page to easily configure shared statistics
Created attachment 58341 [details] [review] Bug 14608: Add a link from the admin home page
Created attachment 58342 [details] [review] Bug 14608: Add a reference to Hea at the end of the installer process
I have redone this code to make it more attractive. The goal is to create a new page to configure the information shared by the libraries with the Koha community. I'd like help to add more information about 1. how libraries should share them, 2. how and why it is useful for the Koha community to get them A link to the privacy policy and the license information should also be added (https://github.com/biblibre/hea-app/pull/13). Not really ready for a signoff but changing the status to highlight the fact it is testable anyway.
I would like to see a new last step at the end of the installation process where the usage statistics preferences could be saved directly. That is, show the whole form from admin/usage_statistics.pl.
I was looking at bug 17809 and wondering why country authorised values are not set or available in every language. It would be nicer than having country names hard coded.
(In reply to M. Tompsett from comment #18) > I was looking at bug 17809 and wondering why country authorised values are > not set or available in every language. It would be nicer than having > country names hard coded. On the contrary, the problem we are seeking to solve is not having multiple names for the same country. Using authorised values does not solve this, as they are user editable.
Also authorised values are only translatable in an odd way, you have to create your own web installer. Them being in the templates makes it much easier.
(In reply to Owen Leonard from comment #17) > I would like to see a new last step at the end of the installation process > where the usage statistics preferences could be saved directly. That is, > show the whole form from admin/usage_statistics.pl. That was the first idea, but finally I don't know if it is a good one. For instance if we want to add geoloc info (bug 17802) or other info at branch level, the form will not be complete at the end of the installer (branches do not exist yet).
(In reply to Owen Leonard from comment #17) > I would like to see a new last step at the end of the installation process > where the usage statistics preferences could be saved directly. That is, > show the whole form from admin/usage_statistics.pl. In bug 17855, the whole onboarding (must be regional or expertise lingo) process could deal with that. (In reply to Jonathan Druart from comment #21) > That was the first idea, but finally I don't know if it is a good one. > For instance if we want to add geoloc info (bug 17802) or other info at > branch level, the form will not be complete at the end of the installer > (branches do not exist yet). But they could, if they were onboard'd. See bug 17855 for some enhancement dreaming. :)
(In reply to M. Tompsett from comment #22) > But they could, if they were onboard'd. See bug 17855 for some enhancement > dreaming. :) Bug 17855 is huge and does not seem to be ready to be pushed. I'd prefer not to wait for it to get this one in.
(In reply to Jonathan Druart from comment #23) > (In reply to M. Tompsett from comment #22) > > But they could, if they were onboard'd. See bug 17855 for some enhancement > > dreaming. :) > > Bug 17855 is huge and does not seem to be ready to be pushed. > I'd prefer not to wait for it to get this one in. It is ready for QA now. :)
I have tested and it works fine! I have not activated the cronjobs because we're on development environment. I have not create the signed patch, because I stll doesn'r know how to do it, soon I will do.
Can I add one small request before this goes to QA? In the form, instead of: <span class="label">Your country: </span> It should be a real <label> tag: <label for="UsageStatsCountry">Your country: </label> <span class="label"> is used when we want to have a line in a form which isn't a label/form field pair, but we want the style to look consistent. See for example, the "City ID" line when editing a city in Administration. Along with that, each corresponding <input> or <select> should have an "id" attribute which matches the "for" attribute of the label. This allows the user to click the label to move the focus to the corresponding form field.
Comment on attachment 58340 [details] [review] Bug 14608: Add a page to configure shared statistics Review of attachment 58340 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/usage_statistics.tt @@ +8,5 @@ > +</head> > + > +<body id="admin_usage_statistics" class="admin"> > +[% INCLUDE 'header.inc' %] > +[% INCLUDE 'patrons-admin-search.inc' %] I'm not sure this is the correct search area to have at the top of the page. The value of script_name isn't defined, and while empty works on patron categories administrative page, it doesn't work here. I'm thinking cat-search.inc might be better.
Created attachment 58968 [details] [review] Bug 14608: Add a page to configure shared statistics This patch set adds: - a reference to Hea at the end of the installation process - a link to the new page from the admin home page - a new page to easily configure shared statistics Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 58969 [details] [review] Bug 14608: Add a link from the admin home page Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 58970 [details] [review] Bug 14608: Add a reference to Hea at the end of the installer process Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 58971 [details] [review] Bug 14608: Follow up on labels and search box This addresses concerns raised in comment #26 and comment #27.
I've signed off on this, but given that my patch is an attempt to address #26 and #27, I'll let someone else like Owen or Jonathan sign it off to get it ready for QA.
Created attachment 59949 [details] [review] Bug 14608: Add a page to configure shared statistics This patch set adds: - a reference to Hea at the end of the installation process - a link to the new page from the admin home page - a new page to easily configure shared statistics Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 59950 [details] [review] Bug 14608: Add a link from the admin home page Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 59951 [details] [review] Bug 14608: Add a reference to Hea at the end of the installer process Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 59952 [details] [review] Bug 14608: Follow up on labels and search box This addresses concerns raised in comment #26 and comment #27. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Please move the countries somewhere else :)
(In reply to Marcel de Rooy from comment #37) > Please move the countries somewhere else :) I do not know what you mean, it's how it is done in admin/preferences/admin.pref I know it is not ideal to have that list in 2 places, but it will not be easy to centralise it. I would prefer to deal with that on another bug report, unless you have something easy to suggest?
(In reply to Jonathan Druart from comment #38) > (In reply to Marcel de Rooy from comment #37) > > Please move the countries somewhere else :) > > I do not know what you mean, it's how it is done in > admin/preferences/admin.pref > I know it is not ideal to have that list in 2 places, but it will not be > easy to centralise it. > I would prefer to deal with that on another bug report, unless you have > something easy to suggest? We could start with putting the country list of this report in a general include file. Removing the list from admin.pref yaml file is for another report. (We should probably load them from the general include and insert them into admin/preferences.pl somewhere. Yaml cannot include files..)
Created attachment 61031 [details] [review] Bug 14608: Move country list to an include file
Marcel, is it what you were expecting?
(In reply to Jonathan Druart from comment #41) > Marcel, is it what you were expecting? Yes, looks like it. Will continue here later this week..
Just a note: Translating the countries is quite a bit of work, please make sure the include file is translatable - looking at it I am not sure as there is no HTML.
(In reply to Katrin Fischer from comment #43) > Just a note: Translating the countries is quite a bit of work, please make > sure the include file is translatable - looking at it I am not sure as there > is no HTML. I thought they were already not translatable on the prefs page, because they are translated in the DB and how we handle all of that is completely buggy. But actually they are translated... I really would prefer to deal with that on another bug report, and find a way to use the same file from the pref and admin pages.
They are translated via the yaml file - the memory of translating them is still fresh. Having only one list would be preferable of course.
Just wondering: - What value will be sent to the HEA database? - The translated one? - Does it depend on what language is choosen at the moment you set the value for UsageStatsCountry? - Deutschland or Germany?
(In reply to Marc Véron from comment #46) > Just wondering: > - What value will be sent to the HEA database? The non-translated ones.
Created attachment 61201 [details] [review] Bug 14608: Add a page to configure shared statistics This patch set adds: - a reference to Hea at the end of the installation process - a link to the new page from the admin home page - a new page to easily configure shared statistics Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61202 [details] [review] Bug 14608: Add a link from the admin home page Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61203 [details] [review] Bug 14608: Add a reference to Hea at the end of the installer process Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61204 [details] [review] Bug 14608: Follow up on labels and search box This addresses concerns raised in comment #26 and comment #27. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61205 [details] [review] Bug 14608: Move country list to an include file Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks good to me. Note that I expected more from this patch set than it actually does. It just seems to be another form for setting the HEA prefs. As mentioned on the report, you will deal with translating the country list on a new report. Looking forward to that.
(In reply to Marcel de Rooy from comment #53) > Looks good to me. Note that I expected more from this patch set than it > actually does. It just seems to be another form for setting the HEA prefs. What would you expect more? Seen bug 18066?
(In reply to Jonathan Druart from comment #54) > (In reply to Marcel de Rooy from comment #53) > > Looks good to me. Note that I expected more from this patch set than it > > actually does. It just seems to be another form for setting the HEA prefs. > > What would you expect more? Seen bug 18066? Only the title :)
Pushed to Master - Should be in the 17.05 release. Thanks!
This won't get ported back to 16.11.x as it is an enhancement.
(In reply to Katrin Fischer from comment #57) > This won't get ported back to 16.11.x as it is an enhancement. That will need to be backported into stable releases, otherwise we will have to wait too long to get enough people using it.
(In reply to Jonathan Druart from comment #58) > (In reply to Katrin Fischer from comment #57) > > This won't get ported back to 16.11.x as it is an enhancement. > > That will need to be backported into stable releases, otherwise we will have > to wait too long to get enough people using it. This is especially true for bug 18066.
Pushed to 3.22.x for 3.22.21 (required for bug 18066)
Pushed to 16.05.x, for 16.05.13 release