|
Description
paxed
2014-01-29 07:40:47 UTC
Comment hidden (obsolete)
Created attachment 26996 [details] [review] [SIGNED-OFF]Improve AgeRestrictionMarker to consider the case where the marker is immediately followed by the age, eg. "K16" in Finland. How I tested: Configure Age Restricition (see Syspref AgeRestrictionMarker) and have a biblio record with e.g. PEGI 99 in age restriction field Try to check out to a patron with age < 99 Check out should be blocked Change entry in age restriction field to PEGI99 Checkout should be possible Apply patch Checkout schould now be blocked Patch behaves as expected http://bugs.koha-community.org/show_bug.cgi?id=11630 Signed-off-by: Marc Véron <veron@veron.ch> Would it be too onerous a request to move the code for parsing the age restriction marker to a separate subroutine and add unit tests? Kyle, I think it is a good idea to have a subroutine for the age restriction. Until now the code is used only at this place, but later we could expand the age restrictions e.g. to suppress titles in the catalogue if a kid is logged in etc. However, this is a small patch that makes an existing feature usable for Finland as well, and I can imagine that they need it immediately. That's why I kindly ask you to QA paxed's patch. In a next step we could open a bug to clean up the whole sub CanBookBeIssued in Circulation.pm (there is more functionality that could be moved in separate sbroutines). What do you think about? Thanks Marc Created attachment 27089 [details] [review] Bug 11630 [QA Followup] - Move code to subroutine, add unit tests Paxed, could you test and add your signoff to the Kyle's patch please? Created attachment 27156 [details] [review] [Signed-off] Bug 11630 [QA Followup] - Move code to subroutine, add unit tests Patch behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 27163 [details] [review] Improve AgeRestrictionMarker to consider the case where the marker is immediately followed by the age, eg. "K16" in Finland. How I tested: Configure Age Restricition (see Syspref AgeRestrictionMarker) and have a biblio record with e.g. PEGI 99 in age restriction field Try to check out to a patron with age < 99 Check out should be blocked Change entry in age restriction field to PEGI99 Checkout should be possible Apply patch Checkout schould now be blocked Patch behaves as expected http://bugs.koha-community.org/show_bug.cgi?id=11630 Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 27164 [details] [review] Bug 11630 [QA Followup] - Move code to subroutine, add unit tests Patch behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 27165 [details] [review] Bug 11630: Unit tests can be independent of DB Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Pushed to master. Thanks, Pasi! Thanks also to Kyle and Jonathan for working on the test cases. Nowadays, changes to routines in the C4 and Koha namespaces really ought to come with unit and/or regression tests. Just rebasing our production branch to 3.16.x and wanted to thank you guys for your great work in getting this through! |