XISBN API uses normalized ISBN of a biblio to get a list of ISBNs, then searches via SQL in database for notices with those ISBNs. I noticed that if input ISBN has hyphens then returned ISBNs have hyphens else they don't have. So SQL query will not find if ISBNs have hyphens in database. Also, if biblio has several ISBN, only first one can be found with actual SQL query (isbn LIKE '$xisbn%'). So I propose to use a simple search "nb=$xisbn". This will find biblio from ISBN with or without hyphen.
Created attachment 15763 [details] [review] Proposed patch See commit comment
This appears to be working, but I'm not sure what it means in the test plan, "Perform a simple search on thoses ISBNs : nb:1234567890." The description of the problem describes issues with hyphens and multiple ISBNs. If the patch addresses those problems, how can I test those aspects of it?
> Perform a simple search on thoses ISBNs : nb:1234567890 I mean perform a search on the previously found ISBNs. You can use the simple search field with a CCL query : "nb:1234567890" (replace 1234567890 with the ISBN) or the advanced search.
(In reply to comment #2) > The description of the problem describes issues with hyphens and multiple > ISBNs. If the patch addresses those problems, how can I test those aspects of it? To test this, edit a matched record : - Multiple ISBN : add an new ISBN field after and before matching ISBN field and retest - Hyphens : edit matched ISBN to remove or add hyphens and retest
Patch tested with a sandbox, by jmbroust <jean-manuel.broust@univ-lyon2.fr>
Created attachment 16633 [details] [review] Bug 9728: XISBN use simple search instead of SQL XISBN API uses normalized ISBN of a biblio to get a list of ISBNs, then searches via SQL in database for notices with those ISBNs. I noticed that if input ISBN has hyphens then returned ISBNs have hyphens else they don't have. So SQL query will not find if ISBNs have hyphens in database. Also, if biblio has several ISBN, only first one can be found with actual SQL query (isbn LIKE '$xisbn%'). This patch replaces SQL query by a simple search "nb=$xisbn". This will find biblio from ISBN with or without hyphen. Test plan : - Activate FRBRizeEditions and XISBN sysprefs - Go to a biblio witch has several editions - Note its normalized ISBN (you may look in amazon links) - Replace [ISBN] by biblio normalized ISBN in this URL : http://xisbn.worldcat.org/webservices/xid/isbn/[ISBN]?method=getEditions&format=xml&fl=form,year,lang,ed - Go to this URL and see which ISBNs are returned - Perform a simple search on thoses ISBNs : nb:1234567890 - Look at "Editions" tab => Check that diplayed biblios are the same you found by simple search Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr>
Created attachment 18650 [details] [review] Bug 9728: XISBN use simple search instead of SQL XISBN API uses normalized ISBN of a biblio to get a list of ISBNs, then searches via SQL in database for notices with those ISBNs. I noticed that if input ISBN has hyphens then returned ISBNs have hyphens else they don't have. So SQL query will not find if ISBNs have hyphens in database. Also, if biblio has several ISBN, only first one can be found with actual SQL query (isbn LIKE '$xisbn%'). This patch replaces SQL query by a simple search "nb=$xisbn". This will find biblio from ISBN with or without hyphen. Test plan : - Activate FRBRizeEditions and XISBN sysprefs - Go to a biblio witch has several editions - Note its normalized ISBN (you may look in amazon links) - Replace [ISBN] by biblio normalized ISBN in this URL : http://xisbn.worldcat.org/webservices/xid/isbn/[ISBN]?method=getEditions&format=xml&fl=form,year,lang,ed - Go to this URL and see which ISBNs are returned - Perform a simple search on thoses ISBNs : nb:1234567890 - Look at "Editions" tab => Check that diplayed biblios are the same you found by simple search Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Pushed to master. Thanks, Fridolyn!
Seems to have broken Unit Test. I work on a followup to fix this.
Created attachment 19122 [details] [review] Bug 9728: XISBN unit test update Follow-up to update XISBN unit tests. Since now XISBN uses search engin, I added a mock on C4::Search::SimpleSearch.pm. A test for ThingISBN connexion already existed, I added on XISBN connexion.
(In reply to comment #9) > Seems to have broken Unit Test. > I work on a followup to fix this. Oops, I should have caught that. I've pushed the follow-up to master. Thanks, Fridolyn!
The UT updates have been pushed to 3.12.x, will be in 3.12.9. Thanks Fridolin!