Description
Alex Arnaud
2015-05-06 12:43:59 UTC
Created attachment 38884 [details] [review] Add maxreserves and alreadyreserved checks when placing hold via ILSDI Comment on attachment 38884 [details] [review] Add maxreserves and alreadyreserved checks when placing hold via ILSDI Review of attachment 38884 [details] [review]: ----------------------------------------------------------------- Alex, I think the following : ::: C4/Reserves.pm @@ +1117,5 @@ > + return 1 if $borrowernumber eq $res->{borrowernumber}; > + } > + > + return 0; > +} needs added test in t/db_dependent/Reserves.t @@ +1139,5 @@ > + return 1; > + } > + > + return 0; > +} needs added test in t/db_dependent/Reserves.t (In reply to Indranil Das Gupta from comment #2) > Comment on attachment 38884 [details] [review] [review] > Add maxreserves and alreadyreserved checks when placing hold via ILSDI > > Review of attachment 38884 [details] [review] [review]: > ----------------------------------------------------------------- > > Alex, > > I think the following : > > ::: C4/Reserves.pm C4::Reserves::AlreadyReserved() C4::Reserves::ReachedMaxReserves() needs added test in t/db_dependent/Reserves.t Created attachment 40170 [details] [review] [Follow-up] Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), (In reply to Indranil Das Gupta from comment #3) > (In reply to Indranil Das Gupta from comment #2) > > Comment on attachment 38884 [details] [review] [review] [review] > > Add maxreserves and alreadyreserved checks when placing hold via ILSDI > > > > Review of attachment 38884 [details] [review] [review] [review]: > > ----------------------------------------------------------------- > > > > Alex, > > > > I think the following : > > > > ::: C4/Reserves.pm > > C4::Reserves::AlreadyReserved() > C4::Reserves::ReachedMaxReserves() > > needs added test in t/db_dependent/Reserves.t Hello, Thank for having had a look at this bug. Just added tests for ttthis 2 sub? Created attachment 40174 [details] [review] Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> Created attachment 40175 [details] [review] Add maxreserves and alreadyreserved checks when placing hold via ILSDI http://bugs.koha-community.org/show_bug.cgi?id=14155 Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> Created attachment 40176 [details] [review] Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> Created attachment 40208 [details] [review] (fix typo) AddBiblio call in Reserves.t Fix for the typo, it read : my ($broken_id, $title, $bibitemnum) = AddBiblio($miniaturist, ''); it should read : my ($broken_id, $title, $bibitemnum) = AddBiblio($broken, ''); Created attachment 40210 [details] [review] (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Setting this back to "Needs Signoff" Created attachment 40239 [details] [review] (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Signed-off-by: Alex Arnaud (Biblibre) <alex.arnaud@biblibre.com> Created attachment 40240 [details] [review] (fix typo) AddBiblio call in Reserves.t Fix for the typo, it read : my ($broken_id, $title, $bibitemnum) = AddBiblio($miniaturist, ''); it should read : my ($broken_id, $title, $bibitemnum) = AddBiblio($broken, ''); Signed-off-by: Alex Arnaud (Biblibre) <alex.arnaud@biblibre.com> Created attachment 40241 [details] [review] (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Signed-off-by: Alex Arnaud (Biblibre) <alex.arnaud@biblibre.com> Be care, AddReserve prototype changed by bug 14526 (contrainst param removed). I meant bug 9809. I have some problems with the conflict in the test files - can you please take a look? If I just fix the number of tests, I get some output like this: bumblebee:~/kohaclone (14155-ilsdi|AM 1/1) $ perl t/db_dependent/Reserves.t 1..71 ok 1 - use C4::Reserves; "my" variable $title masks earlier declaration in same scope at t/db_dependent/Reserves.t line 85. "my" variable $title masks earlier declaration in same scope at t/db_dependent/Reserves.t line 98. "my" variable $bibitemnum masks earlier declaration in same scope at t/db_dependent/Reserves.t line 98. ... Created attachment 44047 [details] [review] Add maxreserves and alreadyreserved checks when placing hold via ILSDI http://bugs.koha-community.org/show_bug.cgi?id=14155 Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> Created attachment 44048 [details] [review] Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> Created attachment 44049 [details] [review] (fix typo) AddBiblio call in Reserves.t Fix for the typo, it read : my ($broken_id, $title, $bibitemnum) = AddBiblio($miniaturist, ''); it should read : my ($broken_id, $title, $bibitemnum) = AddBiblio($broken, ''); Signed-off-by: Alex Arnaud (Biblibre) <alex.arnaud@biblibre.com> Created attachment 44050 [details] [review] (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Signed-off-by: Alex Arnaud (Biblibre) <alex.arnaud@biblibre.com> Created attachment 44064 [details] [review] Bug 14155 - Update tests and fix failures Hi, I'm happy to have a guess at how to test this, please in the future may we have a test plan? It makes things go a lot faster. :) Thanks very much, Liz *** This bug has been marked as a duplicate of bug 11999 *** |