Bug 25951

Summary: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item
Product: Koha Reporter: Nicolas Legrand <nicolas.legrand>
Component: Hold requestsAssignee: Nicolas Legrand <nicolas.legrand>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, severine.queune
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 22407    
Attachments: Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item
Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item
Bug 25951: Add tests for Holds and Item related Objects
Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item

Description Nicolas Legrand 2020-07-08 09:01:30 UTC
Those changes focus on intranet pages /reserve/request.pl, circ/waitingreserves.pl and opac page koha/opac-user.pl.
Comment 1 Nicolas Legrand 2020-07-08 09:07:56 UTC
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.
Comment 2 Nicolas Legrand 2020-07-13 13:51:22 UTC
mhmm... I should add some tests.
Comment 3 Nicolas Legrand 2020-07-21 08:55:26 UTC
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.
Comment 4 Nicolas Legrand 2020-07-21 09:03:56 UTC
Bad test plan, tests should be in a different patch...
Comment 5 Nicolas Legrand 2020-07-21 09:12:31 UTC
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
Comment 6 Nicolas Legrand 2020-07-21 09:12:34 UTC
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.
Comment 7 Jonathan Druart 2020-07-21 09:27:21 UTC
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.
Comment 8 Jonathan Druart 2021-12-15 09:47:45 UTC
Failing QA regarding last comment.