Bug 8917 - Track clicks to links external to Koha
Summary: Track clicks to links external to Koha
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Chris Cormack
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-14 22:15 UTC by Chris Cormack
Modified: 2014-12-07 20:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8917 : Creating tables and system pref, no functional changes at all (4.81 KB, patch)
2012-10-15 19:28 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.76 KB, patch)
2012-10-15 19:28 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Script to log link clicks (3.00 KB, patch)
2012-10-15 19:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Link tracking, changes to XSLT (10.52 KB, patch)
2012-10-15 19:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Creating tables and system pref, no functional changes at all (4.80 KB, patch)
2012-10-22 21:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.75 KB, patch)
2012-10-22 21:13 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Script to log link clicks (2.99 KB, patch)
2012-10-22 21:13 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Link tracking, changes to XSLT (10.68 KB, patch)
2012-10-22 21:24 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Creating tables and system pref, no functional changes at all (4.90 KB, patch)
2012-12-12 20:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.75 KB, patch)
2012-12-12 20:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Script to log link clicks (3.00 KB, patch)
2012-12-12 20:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Link tracking, changes to XSLT (10.68 KB, patch)
2012-12-12 20:30 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Creating tables and system pref, no functional changes at all (4.67 KB, patch)
2013-02-26 08:06 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.68 KB, patch)
2013-02-26 08:06 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Script to log link clicks (3.00 KB, patch)
2013-02-26 08:06 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Link tracking, changes to XSLT (11.08 KB, patch)
2013-02-26 08:06 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Fixing a bug bgkriegel spotted and removing tabs (2.63 KB, patch)
2013-02-26 20:33 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Creating tables and system pref, no functional changes at all (4.74 KB, patch)
2013-02-26 21:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.76 KB, patch)
2013-02-26 21:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 8917 : Script to log link clicks (3.07 KB, patch)
2013-02-26 21:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 8917 : Link tracking, changes to XSLT (11.15 KB, patch)
2013-02-26 21:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Fixing a bug bgkriegel spotted and removing tabs (2.70 KB, patch)
2013-02-26 21:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 8917 : Creating tables and system pref, no functional changes at all (4.72 KB, patch)
2013-02-28 18:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.74 KB, patch)
2013-02-28 18:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Script to log link clicks (3.06 KB, patch)
2013-02-28 18:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Link tracking, changes to XSLT (10.93 KB, patch)
2013-02-28 18:57 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Fixing a bug bgkriegel spotted and removing tabs (2.69 KB, patch)
2013-02-28 18:57 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Shifting db dependent code to a module (4.28 KB, patch)
2013-03-01 20:52 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8917 : Follow up fixing bad behaviour that existed prior to this feature (2.49 KB, patch)
2013-03-01 21:15 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Creating tables and system pref, no functional changes at all (4.74 KB, patch)
2013-03-01 22:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.75 KB, patch)
2013-03-01 22:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Script to log link clicks (3.07 KB, patch)
2013-03-01 22:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Link tracking, changes to XSLT (10.95 KB, patch)
2013-03-01 22:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Fixing a bug bgkriegel spotted and removing tabs (2.70 KB, patch)
2013-03-01 22:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Shifting db dependent code to a module (4.36 KB, patch)
2013-03-01 22:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8917 : Follow up fixing bad behaviour that existed prior to this feature (2.67 KB, patch)
2013-03-01 22:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 8917 : [ Sign followup ] Track clicks to links external to Koha (1.46 KB, patch)
2013-03-01 22:55 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 8917 : Creating tables and system pref, no functional changes at all (4.79 KB, patch)
2013-03-04 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : Adding link tracking to the opac detail page, without xslt (3.80 KB, patch)
2013-03-04 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : Script to log link clicks (3.12 KB, patch)
2013-03-04 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : Link tracking, changes to XSLT (11.00 KB, patch)
2013-03-04 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : Fixing a bug bgkriegel spotted and removing tabs (2.75 KB, patch)
2013-03-04 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : Shifting db dependent code to a module (4.41 KB, patch)
2013-03-04 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : Follow up fixing bad behaviour that existed prior to this feature (2.72 KB, patch)
2013-03-04 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917 : [ Sign followup ] Track clicks to links external to Koha (1.52 KB, patch)
2013-03-04 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8917: replaces tab with spaces (4.53 KB, patch)
2013-03-04 16:01 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2012-10-14 22:15:41 UTC
It would be nice to be able to track if patrons clicked on links inside records to visit links external to Koha. (Ebooks, databases etc).

This should be stored in it's own database table, and a system preference should control if it is used, and also if it stores identifying information or not.

TrackClicks - No
            - Yes anonymously
            - Yes link to borrowers if logged in

Off by default. This data can then be retrieved by a crafted sql report.
Comment 1 Chris Cormack 2012-10-14 22:16:18 UTC
Development sponsored by Albany Senior High School
Comment 2 Chris Cormack 2012-10-15 19:28:39 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2012-10-15 19:28:47 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2012-10-15 19:29:00 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2012-10-15 19:29:25 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2012-10-15 19:32:44 UTC
To test

Apply patches, ... nothing should change.
Switch the TrackClicks syspref to anonymous.

View a result with a link, the link should be changed, clicking on it should take you to the real link, but a row should be inserted into the linktracker table.
The borrowernumber should be 0

Switch the syspref to Track
Try the link again, if you are not logged in borrower will still be 0
If you are logged in, it should have your borrowernumber logged.
Comment 7 Kyle M Hall 2012-10-19 13:08:10 UTC
When TrackClicks is set to track, clicking a link gives this error:

Can't use string ("") as a HASH ref while "strict refs" in use at /home/koha/kohaclone/C4/Auth.pm line 1624.

It does, however, work fine when set to track anonymously.
Comment 8 Chris Cormack 2012-10-22 21:12:33 UTC Comment hidden (obsolete)
Comment 9 Chris Cormack 2012-10-22 21:13:19 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2012-10-22 21:13:39 UTC Comment hidden (obsolete)
Comment 11 Chris Cormack 2012-10-22 21:24:32 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2012-10-22 21:25:54 UTC
(In reply to comment #7)
> When TrackClicks is set to track, clicking a link gives this error:
> 
> Can't use string ("") as a HASH ref while "strict refs" in use at
> /home/koha/kohaclone/C4/Auth.pm line 1624.
> 
> It does, however, work fine when set to track anonymously.

How exactly did you get this error? I can't recreate it, were you logged in at the time? Or not logged in?

I have rebased all the patches against the current HEAD of master, so if you can let me know how you got the error ill try to recreate and fix it.
Comment 13 Kyle M Hall 2012-10-25 16:29:55 UTC
> How exactly did you get this error? I can't recreate it, were you logged in
> at the time? Or not logged in?
> 
> I have rebased all the patches against the current HEAD of master, so if you
> can let me know how you got the error ill try to recreate and fix it.

I tried it both logged in and logged out iirc. I'll have to retest and see if I still get those errors.
Comment 14 Kyle M Hall 2012-10-25 16:35:34 UTC
Tested, I get the error when I am logged in. I tried it as two separate users and got the same error each time.

Can't use string ("") as a HASH ref while "strict refs" in use at /home/koha/kohaclone/C4/Auth.pm line 1625.

If I'm not logged in, it works fine.
Comment 15 Jonathan Druart 2012-12-07 10:11:12 UTC
The feature works for me. I can't reproduce the error describes by Kyle.
Given that Kyle get an error, I don't sign-off this patch.
Comment 16 Chris Cormack 2012-12-09 19:30:10 UTC
Could someone else please test, Kyle is getting an error that currently neither Jonathan or I can reproduce. If someone else could test as well that would be great.
Comment 17 Kyle M Hall 2012-12-12 17:30:02 UTC
I cannot apply the last patch. I'll attempt to retest the rest of them though.
Comment 18 Kyle M Hall 2012-12-12 18:11:48 UTC
I think I'm giving up on testing this one. I just did a full reinstall from master, with only the default data. I changed nothing except to enable TrackClicks, and catalog an item with a URL, and I *still* get the error:

Can't use string ("") as a HASH ref while "strict refs" in use at /home/koha/kohaclone/C4/Auth.pm line 1625.
Comment 19 Chris Cormack 2012-12-12 20:27:41 UTC Comment hidden (obsolete)
Comment 20 Chris Cormack 2012-12-12 20:29:31 UTC Comment hidden (obsolete)
Comment 21 Chris Cormack 2012-12-12 20:29:45 UTC Comment hidden (obsolete)
Comment 22 Chris Cormack 2012-12-12 20:30:01 UTC Comment hidden (obsolete)
Comment 23 Bernardo Gonzalez Kriegel 2013-02-26 00:44:33 UTC
Does not apply.
Comment 24 Chris Cormack 2013-02-26 08:06:23 UTC Comment hidden (obsolete)
Comment 25 Chris Cormack 2013-02-26 08:06:35 UTC Comment hidden (obsolete)
Comment 26 Chris Cormack 2013-02-26 08:06:47 UTC Comment hidden (obsolete)
Comment 27 Chris Cormack 2013-02-26 08:06:59 UTC Comment hidden (obsolete)
Comment 28 Bernardo Gonzalez Kriegel 2013-02-26 16:00:07 UTC
Well, I found Kyle's problem with logged in user.
Software error:
Can't use string ("") as a HASH ref while "strict refs" in use at /home/bgkriegel/kohaclone/C4/Auth.pm line 1682

Digging a little, I found:
1) in tracklinks.pl you call "checkauth( $cgi, 1, '', 'opac' );"
2) in C4/Auth.pm                                  ^ THIS

sub checkauth {
    my $query = shift;
    $debug and warn "Checking Auth";
    # $authnotrequired will be set for scripts which will run without authentication
    my $authnotrequired = shift;
    my $flagsrequired   = shift; <----------- HERE
    my $type            = shift;
    my $persona         = shift;
    $type = 'opac' unless $type;

3) some lines later, $flagsrequired is used in  
$flags = haspermission($userid, $flagsrequired);

4) more lines later, in haspermission()
(1682)    foreach my $module ( keys %$flagsrequired ) {

And that's the error's line.

5) $flagsrequired need to be a hashref, you send ''

6) If I change 1) to 

              checkauth( $cgi, 1, {}, 'opac' );

now Koha is happy and logs the correct user in linktracker table.
(Perl (v5.14.2) built for x86_64-linux-gnu-thread-multi)
Perhaps declaring args and then using them in checkout()
would be cleaner and more understandable.

7) Not related, koha-qa complains of tabulations in updatedatabase.pl, I could fix that.
Comment 29 Bernardo Gonzalez Kriegel 2013-02-26 16:02:11 UTC
(In reply to comment #28)
> Perhaps declaring args and then using them in checkout()
Sorry, I mean checkauth()
Comment 30 Chris Cormack 2013-02-26 20:33:11 UTC Comment hidden (obsolete)
Comment 31 Bernardo Gonzalez Kriegel 2013-02-26 21:03:07 UTC Comment hidden (obsolete)
Comment 32 Bernardo Gonzalez Kriegel 2013-02-26 21:03:15 UTC Comment hidden (obsolete)
Comment 33 Bernardo Gonzalez Kriegel 2013-02-26 21:03:24 UTC Comment hidden (obsolete)
Comment 34 Bernardo Gonzalez Kriegel 2013-02-26 21:03:33 UTC Comment hidden (obsolete)
Comment 35 Bernardo Gonzalez Kriegel 2013-02-26 21:03:41 UTC Comment hidden (obsolete)
Comment 36 Bernardo Gonzalez Kriegel 2013-02-26 21:09:25 UTC
Signed-off.
Work as described. No errors.

Tests
1) Syspref disabled, no tracking
2) Syspref enabled to anonymous, tracking with borrowernumber 0, anonymous search or logged in.
3) Syspref enabled to track, tracking, borrowernumber depends, 0 for anonymous search, borrowernumber for logged in user.
Comment 37 Jonathan Druart 2013-02-28 11:16:36 UTC
Comment on attachment 15713 [details] [review]
[SIGNED-OFF] Bug 8917 : Adding link tracking to the opac detail page, without xslt

Review of attachment 15713 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
@@ +716,4 @@
>              <br />[% END %] 
>          <!-- here you might do a tmpl_if name="toc" and use greybox or equivalent for table of contents -->
>          
> +        [% IF ( MARCurl.OPACurlOpenInNewWindow ) %]

This is always false. The variable to test is OPACurlOpenInNewWindow.
Comment 38 Jonathan Druart 2013-02-28 11:20:11 UTC
Comment on attachment 15714 [details] [review]
[SIGNED_OFF] Bug 8917 : Script to log link clicks

Review of attachment 15714 [details] [review]:
-----------------------------------------------------------------

::: opac/tracklinks.pl
@@ +65,5 @@
> +    print $cgi->redirect("/cgi-bin/koha/errors/404.pl");    # escape early
> +    exit;
> +}
> +
> +sub trackclick {

Don't we want to have this routine into a new module (in order to prevent sql code into pl file)?
Comment 39 Jonathan Druart 2013-02-28 11:21:42 UTC
Marked as Failed QA for the test on the wrong variable.
Comment 40 Chris Cormack 2013-02-28 18:34:36 UTC
Ahh someone must have fixed that since the patch was first made http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12832&action=diff 

I'll change the patch and resubmit
Comment 41 Chris Cormack 2013-02-28 18:56:19 UTC Comment hidden (obsolete)
Comment 42 Chris Cormack 2013-02-28 18:56:33 UTC Comment hidden (obsolete)
Comment 43 Chris Cormack 2013-02-28 18:56:46 UTC Comment hidden (obsolete)
Comment 44 Chris Cormack 2013-02-28 18:57:01 UTC Comment hidden (obsolete)
Comment 45 Chris Cormack 2013-02-28 18:57:19 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2013-03-01 09:35:49 UTC
(In reply to comment #38)
> Comment on attachment 15714 [details] [review] [review]
> [SIGNED_OFF] Bug 8917 : Script to log link clicks
> 
> Review of attachment 15714 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: opac/tracklinks.pl
> @@ +65,5 @@
> > +    print $cgi->redirect("/cgi-bin/koha/errors/404.pl");    # escape early
> > +    exit;
> > +}
> > +
> > +sub trackclick {
> 
> Don't we want to have this routine into a new module (in order to prevent
> sql code into pl file)?

Hi Chris,
You didn't address this remark, isn't it relevant?
Comment 47 Chris Cormack 2013-03-01 09:44:08 UTC
(In reply to comment #46)
> (In reply to comment #38)
> > Comment on attachment 15714 [details] [review] [review] [review]
> > [SIGNED_OFF] Bug 8917 : Script to log link clicks
> > 
> > Review of attachment 15714 [details] [review] [review] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: opac/tracklinks.pl
> > @@ +65,5 @@
> > > +    print $cgi->redirect("/cgi-bin/koha/errors/404.pl");    # escape early
> > > +    exit;
> > > +}
> > > +
> > > +sub trackclick {
> > 
> > Don't we want to have this routine into a new module (in order to prevent
> > sql code into pl file)?
> 
> Hi Chris,
> You didn't address this remark, isn't it relevant?

I didn't think it was, no.
Comment 48 Jonathan Druart 2013-03-01 09:55:05 UTC
(In reply to comment #47)
> (In reply to comment #46)
> > (In reply to comment #38)
> > > Comment on attachment 15714 [details] [review] [review] [review] [review]
> > > [SIGNED_OFF] Bug 8917 : Script to log link clicks
> > > 
> > > Review of attachment 15714 [details] [review] [review] [review] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: opac/tracklinks.pl
> > > @@ +65,5 @@
> > > > +    print $cgi->redirect("/cgi-bin/koha/errors/404.pl");    # escape early
> > > > +    exit;
> > > > +}
> > > > +
> > > > +sub trackclick {
> > > 
> > > Don't we want to have this routine into a new module (in order to prevent
> > > sql code into pl file)?
> > 
> > Hi Chris,
> > You didn't address this remark, isn't it relevant?
> 
> I didn't think it was, no.

So please set out your argument, mine is the coding guidelines : 
http://wiki.koha-community.org/wiki/Coding_Guidelines#SQL8:_SQL_code_in_.pl_scripts
Comment 49 Chris Cormack 2013-03-01 10:02:16 UTC
(In reply to comment #48)
> (In reply to comment #47)
> > (In reply to comment #46)
> > > (In reply to comment #38)
> > > > Comment on attachment 15714 [details] [review] [review] [review] [review] [review]
> > > > [SIGNED_OFF] Bug 8917 : Script to log link clicks
> > > > 
> > > > Review of attachment 15714 [details] [review] [review] [review] [review] [review]:
> > > > -----------------------------------------------------------------
> > > > 
> > > > ::: opac/tracklinks.pl
> > > > @@ +65,5 @@
> > > > > +    print $cgi->redirect("/cgi-bin/koha/errors/404.pl");    # escape early
> > > > > +    exit;
> > > > > +}
> > > > > +
> > > > > +sub trackclick {
> > > > 
> > > > Don't we want to have this routine into a new module (in order to prevent
> > > > sql code into pl file)?
> > > 
> > > Hi Chris,
> > > You didn't address this remark, isn't it relevant?
> > 
> > I didn't think it was, no.
> 
> So please set out your argument, mine is the coding guidelines : 
> http://wiki.koha-community.org/wiki/Coding_Guidelines#SQL8:_SQL_code_in_.
> pl_scripts

I no longer care, someone else can work on this
Comment 50 Jonathan Druart 2013-03-01 11:37:30 UTC
Chris, I feel like you are upset. Did I offense you? If it is the case, I am sorry, I didn't mean it. I am just trying to do my QA job, and this job consists in following the coding guidelines.

I just would like to understand why do you think this remark is no relevant, because I though i was.
Comment 51 Chris Cormack 2013-03-01 20:52:12 UTC Comment hidden (obsolete)
Comment 52 Chris Cormack 2013-03-01 21:01:54 UTC
Shifted the db insert to a new module. Spotted a bug with the behaviour of the links that existed before this new feature, even if we dont have a uri, we are putting an empty href in.

Will do a follow up to fix this bug.
Comment 53 Chris Cormack 2013-03-01 21:15:28 UTC Comment hidden (obsolete)
Comment 54 Bernardo Gonzalez Kriegel 2013-03-01 22:53:19 UTC Comment hidden (obsolete)
Comment 55 Bernardo Gonzalez Kriegel 2013-03-01 22:53:27 UTC Comment hidden (obsolete)
Comment 56 Bernardo Gonzalez Kriegel 2013-03-01 22:53:36 UTC Comment hidden (obsolete)
Comment 57 Bernardo Gonzalez Kriegel 2013-03-01 22:53:46 UTC Comment hidden (obsolete)
Comment 58 Bernardo Gonzalez Kriegel 2013-03-01 22:53:55 UTC Comment hidden (obsolete)
Comment 59 Bernardo Gonzalez Kriegel 2013-03-01 22:54:07 UTC Comment hidden (obsolete)
Comment 60 Bernardo Gonzalez Kriegel 2013-03-01 22:54:18 UTC Comment hidden (obsolete)
Comment 61 Bernardo Gonzalez Kriegel 2013-03-01 22:55:59 UTC Comment hidden (obsolete)
Comment 62 Jonathan Druart 2013-03-04 16:00:02 UTC
QA comment:

This patch add a link tracker at the OPAC for the result and detail pages.
The feature works as expected without XSLT (3 options tested). I didn't test the pages with XSLT, I am a unimarc user and this patch does not modify the unimarc XSLTs.

Marked as Passed QA.

Note: the update database message is not correct ("Adds feature Bug 8917, the ability to track links clicked").
Comment 63 Jonathan Druart 2013-03-04 16:00:35 UTC
Created attachment 15850 [details] [review]
Bug 8917 : Creating tables and system pref, no functional changes at all

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 64 Jonathan Druart 2013-03-04 16:00:43 UTC
Created attachment 15851 [details] [review]
Bug 8917 : Adding link tracking to the opac detail page, without xslt

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 65 Jonathan Druart 2013-03-04 16:00:47 UTC
Created attachment 15852 [details] [review]
Bug 8917 : Script to log link clicks

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 66 Jonathan Druart 2013-03-04 16:00:51 UTC
Created attachment 15853 [details] [review]
Bug 8917 : Link tracking, changes to XSLT

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 67 Jonathan Druart 2013-03-04 16:00:54 UTC
Created attachment 15854 [details] [review]
Bug 8917 : Fixing a bug bgkriegel spotted and removing tabs

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 68 Jonathan Druart 2013-03-04 16:00:58 UTC
Created attachment 15855 [details] [review]
Bug 8917 : Shifting db dependent code to a module

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 69 Jonathan Druart 2013-03-04 16:01:04 UTC
Created attachment 15856 [details] [review]
Bug 8917 : Follow up fixing bad behaviour that existed prior to this feature

But the bug only becomes visible with link tracking on
Prior to this patch it would put an empty link in the html

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

General comment: Work as described. No errors.
Tested all preference values with and without logged in user.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 70 Jonathan Druart 2013-03-04 16:01:08 UTC
Created attachment 15857 [details] [review]
Bug 8917 : [ Sign followup ] Track clicks to links external to Koha

Remove empty pod section in Koha/Linktracker.pm to
make koha-qa happy.

Add missing "\n" in installer/data/mysql/updatedatabase.pl

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 71 Jonathan Druart 2013-03-04 16:01:12 UTC
Created attachment 15858 [details] [review]
Bug 8917: replaces tab with spaces

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 72 Jared Camins-Esakov 2013-03-07 16:17:13 UTC
This patch has been pushed to master.
Comment 73 Mathieu Saby 2013-05-20 21:49:58 UTC
Is somebody planning to write a followup bug for UNIMARC XSLT?

M. Saby