Bug 25951 - Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item
Summary: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nicolas Legrand
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 22407
  Show dependency treegraph
 
Reported: 2020-07-08 09:01 UTC by Nicolas Legrand
Modified: 2021-12-15 09:47 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item (7.97 KB, patch)
2020-07-08 09:07 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item (11.54 KB, patch)
2020-07-21 08:55 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 25951: Add tests for Holds and Item related Objects (3.77 KB, patch)
2020-07-21 09:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 25951: Koha::Objects using the DBIx::Class relationships in Koha::Hold and Koha::Item (8.15 KB, patch)
2020-07-21 09:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.