Hi, In acquisition advanced search, it will be great that isbn index use system preference SearchWithISBNVariations. Regards, Marjorie
Created attachment 67355 [details] [review] Bug 17457 - Adv. Acquisition search with ISBN variations Advanced acquisition search will now follow the "SearchWithISBNVariations" system preference. Test plan : 0) Make sure you have orders pending or completed 1) Enable SearchWithISBNVariations if it is not enabled 2) Search for one of your orders by its ISBN, it should appear 3) Search for the same order by a variation of its ISBN, I used this website to find it : http://www.hahnlibrary.net/libraries/isbncalc.html You should not get a result. 4) Apply patch 5) Repeat step 2-3. You should get a hit both times.
Created attachment 67416 [details] [review] Bug 17457 - Add tests in Acquisition.t Adds tests.
To test : 0) Previous test plan 6) prove t/db_dependent/Acquisition.t
Created attachment 67417 [details] [review] Bug 17457 - Small fix to prevent variable name collision
Created attachment 67418 [details] [review] Bug 17457 - Adv. Acquisition search with ISBN variations New patch : I did some git manipulation mistakes and previous patches wouldn't not apply, sorry :( Advanced acquisition search will now follow the "SearchWithISBNVariations" system preference. Test plan : 0) Make sure you have orders pending or completed 1) Enable SearchWithISBNVariations if it is not enabled 2) Search for one of your orders by its ISBN, it should appear 3) Search for the same order by a variation of its ISBN, I used this website to find it : http://www.hahnlibrary.net/libraries/isbncalc.html You should not get a result. 4) Apply patch 5) Repeat step 2-3. You should get a hit both times. 6) prove t/db_dependent/Acquisition.t
Please don't forget to assign your bugs to yourself :)
- QA tools pass - Tests pass Nice little feature!
Created attachment 67820 [details] [review] Bug 17457 - Adv. Acquisition search with ISBN variations Advanced acquisition search will now follow the "SearchWithISBNVariations" system preference. Test plan : 0) Make sure you have orders pending or completed 1) Enable SearchWithISBNVariations if it is not enabled 2) Search for one of your orders by its ISBN, it should appear 3) Search for the same order by a variation of its ISBN, I used this website to find it : http://www.hahnlibrary.net/libraries/isbncalc.html You should not get a result. 4) Apply patch 5) Repeat step 2-3. You should get a hit both times. 6) prove t/db_dependent/Acquisition.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 70279 [details] [review] Bug 17457 - Adv. Acquisition search with ISBN variations Advanced acquisition search will now follow the "SearchWithISBNVariations" system preference. Test plan : 0) Make sure you have orders pending or completed 1) Enable SearchWithISBNVariations if it is not enabled 2) Search for one of your orders by its ISBN, it should appear 3) Search for the same order by a variation of its ISBN, I used this website to find it : http://www.hahnlibrary.net/libraries/isbncalc.html You should not get a result. 4) Apply patch 5) Repeat step 2-3. You should get a hit both times. 6) prove t/db_dependent/Acquisition.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tests are MARC21 specific, please provide a version for UNIMARC. I would suggest to create a MARC::Record manually instead of using a MARC XML. See what is done in t/db_dependent/Biblio.t
Created attachment 70910 [details] [review] Bug 17457 - Add tests for each marc flavour + Added a `run_flavoured_tests` sub that runs for each MARC flavour * Moved SearchWithISBNVariation tests to the new sub I copied a lot of the necessary utility functions over Biblio.t (as of Jonathan's suggestion). I also tried to name and organize the code in such a way that it can be reused if anyone else needs to run "flavoured" tests in the future. prove t/db_dependant/Acquisition.t OK QA tool green
(In reply to Jonathan Druart from comment #10) > Tests are MARC21 specific, please provide a version for UNIMARC. > I would suggest to create a MARC::Record manually instead of using a MARC > XML. > See what is done in t/db_dependent/Biblio.t Imho adding a sample UNIMARC record with a ISBN would have been much simpler than following Biblio.t. This is unnecessary code complexity/duplication (120 lines extra for a UNIMARC ISBN). David is to be commended for this efforts. But personally I would prefer to push without the second patch. I will pass QA without additional signoff to let Jonathan handle it.
Pushed to master for 18.05, thanks to everybody involved!
Enhancement, not backported for 17.11