Bug 7470

Summary: Babeltheque improvement and social networks links (like, +1,...)
Product: Koha Reporter: claire.hernandez <claire.hernandez>
Component: OPACAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: claire.hernandez, f.demians, jonathan.druart, katrin.fischer, m.de.rooy, mathilde.formery, nengard, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 7897, 10129    
Attachments: Output for update_social_data.pl
Bug 7470 Babeltheque integration
Output for get_report_social_data.pl
Bug 7470 Babeltheque integration
Bug 7470 Babeltheque integration
Bug 7470 Babeltheque integration
Bug 7470: Followup: include g+ api if syspref SocielNetwork is on
Bug 7470: Move the SocialNetworks syspref to the "Enhanced Content" tab
Bug 7470: Followup: Changes twitter and mailto links
Bug 7470: Followup: Changes twitter and mailto links
Bug 7470: Followup Move Babeltheque css into a specific fil
[SIGNED-OFF] Bug 7470: Followup Move Babeltheque css into a specific file
Bug 7470: Followup: Changes mailto links
[SIGNED-OFF] Bug 7470: Followup: Changes mailto links

Description claire.hernandez@biblibre.com 2012-01-25 16:28:02 UTC
We did a new implementation of babeltheque in koha, more complete and better intergrated (for the moment in community version, babeltheque is not configurable and hardcoded in opac-bottom.inc).

3 features in this one:
- adds social network information in search results
- adds babeltheque data in opac-detail
- adds social network links in opac-detail too (google+, twitter, mail and co.)


This dev is in production in 2 of our customes (co-funder).

French article with screenshots: http://docmiop.wordpress.com/2012/01/18/quand-babeltheque-enrichit-koha/
Comment 1 claire.hernandez@biblibre.com 2012-01-25 16:28:55 UTC
(MT8241)
Comment 2 Jonathan Druart 2012-03-02 13:40:24 UTC
Created attachment 7984 [details]
Output for update_social_data.pl

This file is the result for the script update_social_data.pl
Comment 3 Jonathan Druart 2012-03-02 13:40:41 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2012-03-02 13:43:08 UTC
Created attachment 7986 [details]
Output for get_report_social_data.pl

This file is the result for the script get_report_social_data.pl (facultative)
Comment 5 Jonathan Druart 2012-03-19 11:06:39 UTC Comment hidden (obsolete)
Comment 6 MathildeF 2012-03-19 16:41:00 UTC
** Affichage : 
Pb affichage dans Firefox < 3.6 :
- les boutons "Internet user critics" et "Extracts" ne s'affichent pas correctement
- le bloc "exemplaire - note - commentaire" ne se présente pas en onglet

sous Firefox (test sur 3.5, 3.6 et 11.0) : le popup des vidéos est excentré à droite et ne se recadre pas à la taille de la fenetre / de l'écran  


** Reseaux sociaux 

- le lien dans facebook vers la notice n'est pas correctement créé 
(ex : http://www.facebook.com/cgi-bin/koha/opac-detail.pl?biblionumber=1553)

- la fenetre de partage G+ se ferme (parfois) immédiatemment
Comment 7 MathildeF 2012-03-20 08:38:36 UTC
Firefox <3.6:
- Buttons "Internet user Critics" and "Extracts" are not displayed correctly
- The block "copy - note - comment" does not show in tab

Firefox (tested on 3.5, 3.6 and 11.0):the videos are right justified and does not reframes the size of the window / screen

- The link in facebook is not created correctly
(http://www.facebook.com/cgi-bin/koha/opac-detail.pl?biblionumber=1553 and not "http://mykoha.tld/cgi-bin/koha/opac-detail.pl?biblionumber=1553)

- the window Goole+ closes  (sometimes) immediately
Comment 8 Jonathan Druart 2012-03-20 09:45:54 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2012-03-20 12:58:02 UTC
(In reply to comment #7)
> Firefox <3.6:
> - Buttons "Internet user Critics" and "Extracts" are not displayed correctly
> - The block "copy - note - comment" does not show in tab
> 

It's a bug in koha. A js error occured: "$(".close") is null"
It's not come from this feature.
Maybe, if you used FF3.5, you can upgrade your browser :-)

> Firefox (tested on 3.5, 3.6 and 11.0):the videos are right justified and
> does not reframes the size of the window / screen

It's caused by a css specific in sandboxes. I remove the contain of the system preference "opacstylesheet" and the display is good.

> - The link in facebook is not created correctly
> (http://www.facebook.com/cgi-bin/koha/opac-detail.pl?biblionumber=1553 and
> not "http://mykoha.tld/cgi-bin/koha/opac-detail.pl?biblionumber=1553)
> 

The system preference "OPACBaseURL" must be filled with the good value

> - the window Goole+ closes  (sometimes) immediately

Whow, it seems the guilty is the Google+ API, I have not the control on this behaviour.
Comment 10 Paul Poulain 2012-03-22 04:37:14 UTC
QA comments:

(In reply to comment #9)
> (In reply to comment #7)
> > Firefox <3.6:
> > - Buttons "Internet user Critics" and "Extracts" are not displayed correctly
> > - The block "copy - note - comment" does not show in tab
> It's a bug in koha. A js error occured: "$(".close") is null"
> It's not come from this feature.
> Maybe, if you used FF3.5, you can upgrade your browser :-)

Does the problem exist in OPAC ? Does it mean the block doesn't appear at all or the display is crapy ? (that may make a difference for deciding to "pass QA" or not: having a crapy display on old browsers is acceptable. Having no display isn't)
> > Firefox (tested on 3.5, 3.6 and 11.0):the videos are right justified and
> > does not reframes the size of the window / screen
> 
> It's caused by a css specific in sandboxes. I remove the contain of the
> system preference "opacstylesheet" and the display is good.
OK, then it does not prevent from passing QA
(note: it's not specific to sandbox, but to sandbox database you're choosen: the sandbox koha code is purely koha-community)

> > - The link in facebook is not created correctly
> > (http://www.facebook.com/cgi-bin/koha/opac-detail.pl?biblionumber=1553 and
> > not "http://mykoha.tld/cgi-bin/koha/opac-detail.pl?biblionumber=1553)
> The system preference "OPACBaseURL" must be filled with the good value
OK,

> > - the window Goole+ closes  (sometimes) immediately
> Whow, it seems the guilty is the Google+ API, I have not the control on this
> behaviour.
could you give more details ?

Other QA comments:
* C4/SocialData.pm is not perlcritic compliant:
Bareword file handle opened at line 72, column 5.  See pages 202,204 of PBP.  (Severity: 5)
Two-argument "open" used at line 72, column 5.  See page 207 of PBP.  (Severity: 5)

* patch does not apply anymore, please rebase when fixing the previous comment (will be a trivial rebase)

* opac.css classes= I don't understand why you've BABELTHEQUE classes in opac.css & commented. Please add some explanations

otherwise, I think I haven't more QA comments to do.
Comment 11 Paul Poulain 2012-03-22 04:38:00 UTC
Frederic, you're the author of the 1st version of babeltheque, feel free to have a look to this new version !
Comment 12 Jonathan Druart 2012-03-22 09:38:58 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2012-03-23 16:26:48 UTC
QA Comments: Larger patch. Looks good in general. Controlled by several prefs. So does not hurt anyone who does not use it. Only miss comments from current Babeltheque users.


Resolved Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Trailing whitespace at apply.

Double dbrev in updatedatabase: please combine into one. Paul could do that at pushing time..
As it appears, the backquotes are not liked by PostgreSQL. Not a blocker until now? Lots of these in updatedatabase..

opac-detail: May need some further design work (followup). Not my specialty. OK

update_data: say "$i data insered or updated"; Where does this output go to? Note spelling error too ;)

get_report: This seems quite a job. Just curiosity: but how often would you like to do that in a cron job?

update_social_data.pl: /tmp is probably ok for rights, but could be an issue. bunzip2 will not be in /bin for every linux distro.

opac-detail.tt: https://apis.google.com/js/plusone.js: Do you need to include this js if you do not use this feature? Probably not.
<a href="mailto:ADRESSE?subject=TO READ : [% title %]>: Does not look great.

In conclusion:
May need some followups. But have no objection to push it further.
Comment 14 Jonathan Druart 2012-03-26 12:43:09 UTC Comment hidden (obsolete)
Comment 15 Paul Poulain 2012-03-26 12:49:14 UTC
(In reply to comment #13)
> QA Comments: Larger patch. Looks good in general. Controlled by several
> prefs. So does not hurt anyone who does not use it. Only miss comments from
> current Babeltheque users.
> 
> 
> Resolved Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
> Trailing whitespace at apply.
trivial conflict, resolved.

> Double dbrev in updatedatabase: please combine into one. Paul could do that
> at pushing time..
I prefer to have 2 dbrevs: the babeltheque stuff is specific to babelio customers, probably only french libraries
The other (social networks) is related to having links to "like", "+1",... that are a different feature. So having 2 DBrevs will enforce the visibility of the 2nd feature.
> As it appears, the backquotes are not liked by PostgreSQL. Not a blocker
> until now? Lots of these in updatedatabase..
good point. I've removed them in the commit

> update_data: say "$i data insered or updated"; Where does this output go to?
> Note spelling error too ;)
The update_data script is called by a cronjob, that's the result of the cron (with the generated file), so no need to worry here
I've fixed the typo in the commit.

> get_report: This seems quite a job. Just curiosity: but how often would you
> like to do that in a cron job?
for our customers, it will be once a week. We have 2 medium size catalogue (300 000 items) running it for now. Maybe for AixMarseille (1 million !) that will even be less frequent.

> update_social_data.pl: /tmp is probably ok for rights, but could be an
> issue. bunzip2 will not be in /bin for every linux distro.
I think you're right. Jonathan, I think having the patch in the cron PATH is something we can request. Thus, just calling bunzip2 should be OK.
You could also try to guess the patch by testing some common ones. There's something like that in rebuild_zebra.pl, to find zebraidx

> opac-detail.tt: https://apis.google.com/js/plusone.js: Do you need to
> include this js if you do not use this feature? Probably not.
> <a href="mailto:ADRESSE?subject=TO READ : [% title %]>: Does not look great.
> 
you're right Marcel. Jonathan, could you provide a follow-up

> In conclusion:
> May need some followups. But have no objection to push it further.

So I push it now ;-)
Comment 16 Paul Poulain 2012-03-26 13:03:07 UTC
Patch pushed, with a little change:
the SocialNetworks syspref has been set to 0 => the rule, when including a switchable improvement is to have it as it was before the improvement. So, in this case, switched OFF

Another suggestion : The syspref would be better in the tab Enhanced content > All, instead of Searching (just realised that after pushing)
Comment 17 Paul Poulain 2012-03-26 13:22:39 UTC
switching back to "assigned" to address the small issues that have been raised
Comment 18 Paul Poulain 2012-03-26 13:22:56 UTC
Comment on attachment 8476 [details] [review]
Bug 7470 Babeltheque integration

this patch has been pushed
Comment 19 Paul Poulain 2012-03-26 13:23:13 UTC
Comment on attachment 8630 [details] [review]
Bug 7470: Followup: include g+ api if syspref SocielNetwork is on

this patch has been pushed
Comment 20 Katrin Fischer 2012-03-27 15:43:30 UTC
I noticed the new syspref on update and wonder if we could perhaps rename it? The feature and release are not fully out of the door yet...

Currently it is:
Babeltheque_url_js 	Defined the url for the Babeltheque javascript file (eg. http://www.babeltheque.com/bw_XX.js) 

I know we don't have much of a standard for naming system prefernces, but at least we had no underscores so far :)
Comment 21 Katrin Fischer 2012-03-27 15:47:54 UTC
Ok, it is possible that I am being silly here...
Comment 22 Nicole C. Engard 2012-03-28 16:23:47 UTC
- adds social network information in search results

I don't see this. I see the social share buttons on the detail page, but not on the search results with XSLT and Marc21.

Nicole
Comment 23 Owen Leonard 2012-03-28 19:54:14 UTC
This feature is mostly broken for me and I think it should be reverted. All URLs being used for sharing appear to be broken. Clicking the Facebook share button brings me to a page on Facebook where I'm sharing this link:

http://www.facebook.com/cgi-bin/koha/opac-detail.pl?biblionumber=453

On my Ubuntu machine the Twitter link brings me to a "share" page in which the body of the tweet is the correct URL, but on Windows I get a blank "share" box. We should be passing the title as well as the URL to Twitter.

Clicking the LinkedIn share button passes the correct title but not a full URL ("/cgi-bin/koha/opac-detail.pl?biblionumber=453").

The Delicious link passes the title but not a full URL.

The mailto: link seems to have extraneous markup and, again, does not pass a full URL.

This is not ready.
Comment 24 Owen Leonard 2012-03-28 20:01:31 UTC
The social networks stuff could be easily disentangled from the Babeltheque stuff. That would be a good option if the Babeltheque stuff is working well (which of course I can't test).
Comment 25 Jonathan Druart 2012-03-29 07:29:05 UTC
@Nicole: oups, it's only on the detail page. There is no link on the search page.

@Owen: Have you filled correctly your syspref OPACBaseURL ? It sound like the problem reported by Mathilde, please see my Comment 9.
Comment 26 Jonathan Druart 2012-03-30 07:18:06 UTC Comment hidden (obsolete)
Comment 27 Owen Leonard 2012-03-30 13:24:03 UTC
Okay, you're right: I didn't have the OPACBaseURL preference filled. This improves the behavior all around. I'm still getting inconsistent results with Twitter--sometimes nothing comes up in the share box. I wonder if anyone else sees the same thing?

I still think it would be good to pass the title of the item to Twitter along with the URL.

Delicious is working fine now that I have OPACBaseURL filled. I still have problems with the mailto link though. The "To:" field is pre-populated with the word "ADRESSE"--can it be left blank? The subject looks like this:

TO READ : Zero day :>

And the title like this:

Zero day :> localhost:88/cgi-bin/koha/opac-detail.pl?biblionumber=751

The ">" is not in the title. Seems to be coming from the link. So, some small follow-ups required but it is working properly for the most part.
Comment 28 Jonathan Druart 2012-03-30 13:55:43 UTC Comment hidden (obsolete)
Comment 29 Paul Poulain 2012-04-06 13:34:19 UTC
QA question about the follow-up: shouldn't the [% title %] be [% title|html %] ? Without |html, a title with a & would break the link validity, isn't it ?
Comment 30 Jonathan Druart 2012-04-06 13:46:07 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2012-04-23 07:33:20 UTC Comment hidden (obsolete)
Comment 32 Owen Leonard 2012-04-23 13:10:49 UTC
(In reply to comment #31)
> Bug 7470: Followup Move Babeltheque css into a specific fil

There's still no reference to the CSS file in the OPAC. Shouldn't it be conditionally included in opac-detail.tt if Babeltheque is enabled?
Comment 33 Jonathan Druart 2012-05-16 08:49:26 UTC
(In reply to comment #32)
> There's still no reference to the CSS file in the OPAC. Shouldn't it be
> conditionally included in opac-detail.tt if Babeltheque is enabled?

Hi Owen,
It is just an example. The good way to use these css properties is to declare them into the Babeltheque configuration.
Comment 34 Owen Leonard 2012-05-18 15:14:06 UTC
Created attachment 9643 [details] [review]
[SIGNED-OFF] Bug 7470: Followup Move Babeltheque css into a specific file

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 35 Owen Leonard 2012-05-18 15:24:54 UTC
(In reply to comment #28)
> - Change the subject for mailto

This is a good improvement.

> - Set url and text params for the twitter link (Appearly, OPACBaseURL must
> begin with http:// to make twitter happy)

In my test the link doesn't get passed to Twitter whether or not OPACBaseURL begins with "http://". That requirement is problematic since the preference instructs the user *not* to include "http://":

"The OPAC is located at http:// [ input ]"

I like the option to use a standard URL for the Twitter share option because it means the user doesn't need to have JavaScript turned on. But if we can't get it to work consistently I think my patch for Bug 7932 might be a better choice.
Comment 36 Jonathan Druart 2012-05-21 07:26:35 UTC Comment hidden (obsolete)
Comment 37 Owen Leonard 2012-07-17 15:26:41 UTC
Created attachment 10904 [details] [review]
[SIGNED-OFF] Bug 7470: Followup: Changes mailto links

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 38 Paul Poulain 2012-08-02 12:15:42 UTC
QA comment:
* the 1st patch remove css declarations that are useless when you don't use babeltheque, it's good. I just have added a follow-up with some doc to avoid removal of this babeltheque.css file !
* the 2nd patch is tiny

Passed QA (patches pushed on master branch, they don't apply correctly on new/bug_7470)