Those changes focus on intranet pages /reserve/request.pl, circ/waitingreserves.pl and opac page koha/opac-user.pl.
Created attachment 106649 [details] [review] Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item Those changes focus on intranet pages /reserve/request.pl, circ/waitingreserves.pl and opac page koha/opac-user.pl. Plan test: 1. make a hold on a checkouted item and check it in. It should be in waiting reserves now. 2. go have a look to /reserve/request.pl, circ/waitingreserves.pl, koha/opac-user.pl (while logged as the user having the hold) pages. 3. it should be errorless, you should see the library names and the forms should prompt the correct branchcode.
mhmm... I should add some tests.
Created attachment 107129 [details] [review] Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item Those changes focus on intranet pages /reserve/request.pl, circ/waitingreserves.pl and opac page koha/opac-user.pl. Plan test: 1. run `prove t/db_dependent/Koha/Item/RelatedObjects.t ; prove t/db_dependent/Koha/Holds.pm`, all should be ok 2. apply patch 3. rerun `prove t/db_dependent/Koha/Item/RelatedObjects.t ; prove t/db_dependent/Koha/Holds.pm`, all should be ok 4. make a hold on a checkouted item and check it in. It should be in waiting reserves now. 5. go have a look to /reserve/request.pl, circ/waitingreserves.pl, koha/opac-user.pl (while logged as the user having the hold) pages. 6. it should be errorless, you should see the library names and the forms should prompt the correct branchcode.
Bad test plan, tests should be in a different patch...
Created attachment 107130 [details] [review] Bug 25951: Add tests for Holds and Item related Objects Test plan: 1. apply patch 2. `prove t/db_dependent/Koha/Holds.t` and `t/db_dependent/Koha/Item/RelatedObjects.t` 3. all should be OK
Created attachment 107131 [details] [review] Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item Those changes focus on intranet pages /reserve/request.pl, circ/waitingreserves.pl and opac page koha/opac-user.pl. Test plan: 1. Apply the tests patch and do its test plan 2. apply this patch 3. rerun `prove t/db_dependent/Koha/Item/RelatedObjects.t ; prove t/db_dependent/Koha/Holds.pm`, all should be ok 4. make a hold on a checkouted item and check it in. It should be in waiting reserves now. 5. go have a look to /reserve/request.pl, circ/waitingreserves.pl, koha/opac-user.pl (while logged as the user having the hold) pages. 6. it should be errorless, you should see the library names and the forms should prompt the correct branchcode.
1. I would not create a separate test file RelatedObjects.t code can be moved to a subtest of Koha/Items.t 2. On your way, you could create a Koha::Hold->library and ->patron. ->branch and ->borrower will call ->library and ->patron branch and borrower could be marked as deprecated 3. In holds-table.inc, + Item in transit to <strong> [% HOLD.branch.branchname | html %]</strong> <input type="hidden" name="pickup" value="[% HOLD.branchcode | html %]" /> It's weird to access HOLD.branch.branchname and HOLD.branchcode (better HOLD.branch.branchcode to highlight it's the same object we access). I think it make more sense to have a "library" variable, [% SET library = HOLD.branch %] then reuse library for branchcode and branchname attributes.
Failing QA regarding last comment.