Bug 5864 - Is C4::Serials::GetSubscriptions searching too generally?
Summary: Is C4::Serials::GetSubscriptions searching too generally?
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: PATCH-Sent (DO NOT USE) minor
Assignee: Julian Maurice
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-14 13:58 UTC by Magnus Enger
Modified: 2012-10-25 23:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Patch from BibLibre (3.23 KB, patch)
2011-03-15 08:18 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2011-03-14 13:58:42 UTC
After migrating subscription data from 2.2.9 to 3.2.1, 6 of about 222 subscriptions were showing two or three lists of "latest issues", lets say "Journal A" was displaying the list of "latest issues" for "Journal A", but also for "Journal X" and "Journal Y". 

The first theory was that an incorrect biblionumber had been put in e.g. the subscriptions table (they had to be updated since the biblionumbers changed from the old to the new database), but scrutiny of the data showed this not to be the case. "Jounal X" and "Journal Y" were displaying just fine on their own (looking up the biblionumber showed the expected results). 

The problem was solved by changing line 104 of opac/opac-detail.pl from this: 

my @subscriptions = GetSubscriptions( $dat->{title}, $dat->{issn}, $biblionumber );

to this: 

my @subscriptions = GetSubscriptions( '', $dat->{issn}, $biblionumber );

preventing C4::Serials::GetSubscriptions from using the title of the journal for finding subscription information. 

Looking at the code in C4::Serials::GetSubscriptions it looks like, if a title is provided, the function will look for it in all of these fields: 

biblio.title 
subscription.callnumber 
subscription.location 
subscription.notes 
subscription.internalnotes

returning any subscriptions where the title is found, including any where the string occurs in notes subscription.notes and subscription.internalnotes! 

This was indeed the case for our original problem, the journal that was displaying extra lists of issues was "Science" ("Journal A"), and the two extra journals having their issues erroneously displayed ("Journal X" and "Journal Y") had the word "science" mentioned in the subscription.notes field. 

Changing the call to GetSubscriptions worked in this case, but I'm not sure it would be an improvement for everyone? 

This was tested with 3.2.1, but the code seems to be same in master.
Comment 1 Paul Poulain 2011-03-14 14:02:24 UTC
Hi Magnus,

we already have fixed the problem (but not submitted it upstream, our fault...)
See:
http://git.biblibre.com/?p=koha;a=commit;h=80ba3e785101204419922d810b7a0cd3d44c9e07
Comment 2 Magnus Enger 2011-03-14 15:07:07 UTC
Cool, I'll just leave the bug report as it is then, so you have a bug number for your patch... ;-)
Comment 3 Julian Maurice 2011-03-15 08:18:24 UTC
Created attachment 3299 [details] [review]
Patch from BibLibre
Comment 4 Magnus Enger 2011-03-18 09:51:00 UTC
Tested on current master.
Comment 5 Chris Cormack 2011-03-20 23:01:19 UTC
Pushed and merged, please test and mark resolved
Comment 6 Magnus Enger 2011-03-21 22:02:23 UTC
Works in current master.
Comment 7 Frédérick Capovilla 2011-05-20 19:54:18 UTC
Comment on this problem :
The patch that was pushed fixes this particular problem by passing less filters to the GetSubscriptions() subroutine, but it doesn't fix the main problem :
GetSubscriptions() is still searching too generally.

I tried printing the final query created by GetSubscriptions and here is what it looks like :

SELECT subscription.*, subscriptionhistory.*, biblio.title,biblioitems.issn,biblio.biblionumber
            FROM   subscription
            LEFT JOIN subscriptionhistory USING(subscriptionid)
            LEFT JOIN biblio ON biblio.biblionumber = subscription.biblionumber
            LEFT JOIN biblioitems ON biblio.biblionumber = biblioitems.biblionumber
       WHERE biblio.biblionumber=? AND (biblio.title LIKE ? ) OR (subscription.callnumber LIKE ? ) OR (subscription.location LIKE ? ) OR (subscription.notes LIKE ? ) OR (subscription.internalnotes LIKE ? ) AND (biblioitems.issn LIKE ? ) OR (subscription.callnumber LIKE ? ) ORDER BY title

Example binds used : "47569" "%M%" "%M%" "%M%" "%M%" "%M%" "%1715-4820%" "%1715-4820%"

I think the query generated by GetSubscriptions needs to be fixed too...
Comment 8 Paul Poulain 2011-05-22 19:52:21 UTC
The fix does what it intended to do: when you want to find a subscription from biblionumber, it's an "exact" search.
The other parameter is for a kind of "approximative" search. Should it search on callnumber or not ? on title or not ? with % or not ?
I think it's another debate. Our libraries usually don't complain when there is a little bit too many results, they understand that the search is "large".

A fix could be to have the search explicitely done on "title", "callnumber", ... But that would require the user form to have more fields available (atm, there is only one field). I' not sure i've an opinion wether it's a good idea or not.
Comment 9 Frédérick Capovilla 2011-05-31 15:45:30 UTC
Here is the comment of this subroutine :
this function gets all subscriptions which have title like $title,ISSN like $ISSN and biblionumber like $biblionumber.

With this description, I thought that the subroutines would return all subscriptions with the specified title AND ISSN AND biblionumber.
Right now, it doesn't exactly returns that because the ANDs and ORs of the query are all mixed together without useful parentheses.

If I'm not wrong, with MySQL's operator precedences, the current query looks like this :
(biblio.biblionumber=? AND biblio.title LIKE ? ) OR
subscription.callnumber LIKE ? OR
subscription.location LIKE ? OR
subscription.notes LIKE ? OR 
(subscription.internalnotes LIKE ? AND biblioitems.issn LIKE ? ) OR subscription.callnumber LIKE ?

I think it was meant to be like this :
(biblio.biblionumber=?) AND 
(biblio.title LIKE ? OR subscription.callnumber LIKE ? OR subscription.location LIKE ? OR subscription.notes LIKE ? OR subscription.internalnotes LIKE ?) AND (biblioitems.issn LIKE ? OR subscription.callnumber LIKE ?)


I understand that the current fix does solve the problem, but I just want to bring to someone's attention that the GetSubscriptions subroutine might not work as intended.