Summary: | Add syspref to control location for use of AllowRenewalIfOtherItemsAvailable | ||
---|---|---|---|
Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
Component: | Circulation | Assignee: | Aleisha Amohia <aleisha> |
Status: | Failed QA --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | david, elizabethjohn6654, gmcharlt, jonathan.druart, kyle.m.hall, lucas, m.de.rooy, wainuiwitikapark |
Version: | unspecified | Keywords: | Sandbox |
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | Sponsored | |
Patch complexity: | Medium patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: |
This enhancement adds a new system preference AllowRenewalIfOtherItemsAvailableLocation which works in conjunction with the AllowRenewalIfOtherItemsAvailable system preference. It has two options - allow renewal if other items are available to fill holds at any library, or only at the hold's pickup library. This way, if items cannot be shared between libraries, the AllowRenewalIfOtherItemsAvailable system preference will consider this when deciding if renewal is allowed.
|
|
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref
Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25224: Show large print format in search results XSLT Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref |
Description
Aleisha Amohia
2020-04-19 22:30:16 UTC
Ignore the description, I had thought about this wrong - would like to edit the AllowRenewalIfOtherItemsAvailable system preference so the options are: - Don't allow - Allow if items available at reserve patron's branch - Allow always The second option would mean that Patron A can only renew their item if there are other items available at Patron B's (patron who has placed a reserve on this biblio) branch. The following scenarios should explain this a bit better: NZ is one branch, AU is another branch. There is a Biblio A with four items - Item NZa, Item NZb, Item AUa and Item AUb. There are two patrons - Patron NZa and Patron NZb. Situation 1: Patron NZa has Item NZa checked out. Item NZb is lost/damaged/checked out/not available. Items AUa and AUb are both available. Patron NZb has reserved Biblio A. AllowRenewalIfOtherItemsAvailable is enabled. Patron NZa should NOT be able to renew Item NZa because other NZ items are not available, even though AU items are available. Situation 2: Patron NZa has Item NZa checked out. Item NZb is lost/damaged/checked out/not available. Items AUa and AUb are both available. Patron NZb has reserved Biblio A. AllowRenewalIfOtherItemsAvailable is NOT enabled. Patron NZa should NOT be able to renew Item NZa because other NZ items are not available, even though AU items are available. Situation 3: Patron NZa has Item NZa checked out. Item NZb is available. Items AUa and AUb are both available. Patron NZb has reserved Biblio A. AllowRenewalIfOtherItemsAvailable is enabled. Patron NZa SHOULD be able to renew Item NZa. Situation 4: Patron NZa has Item NZa checked out. Item NZb is available. Items AUa and AUb are both available. Patron NZb has reserved Biblio A. AllowRenewalIfOtherItemsAvailable is NOT enabled. Patron NZa should NOT be able to renew Item NZa because AllowRenewalIfOtherItemsAvailable is not enabled, even though NZb is available. Please ignore all earlier comments, they are a mess! Patch and test plan coming that will explain this feature better Created attachment 103318 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any branch". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any branch". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup branch". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Also: confirm tests pass t/db_dependent/Circulation.t Created attachment 103322 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any branch". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any branch". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup branch". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 103516 [details] [review] Bug 25224: Show large print format in search results XSLT To test: 1: find a bib with 008 type Book (Leader position 6 = 'a' and leader position 7 = 'm' -- use bib number 1 in master data) 2: set 008 position 23 to 'd' for large print 3: check your book record in opac and intranet search results, note that your change is not reflected in the "format" note in line with material type 4: find a bib with 008 type Continuing Resource (Leader position 6 = 'a' and leader position 7 = 's' -- use bib number 44 in master data) 5: set 008 position 23 to 'd' for large print 6: check your continuing resource record in opac and intranet search results, note that your change is not reflected in the "format" note in line with material type 7: apply patch, restart_all 8: reload search results from steps 3 and 6, confirm they now say "format: large print" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Aleisha, Have some concern about the interaction of both sysprefs and if the interface now helps us to understand it well.. AllowRenewalIfOtherItemsAvailable / AllowRenewalIfOtherItemsAvailableLocation Dont allow / Allow a patron to renew an item with unfilled holds if other available items at any branch / at the hold's pickup branch can fill that hold. If the pref is set to Allow, we may examine the other pref. But if we say Do not allow we now suggest that it is conditional depending on the second pref. But that is not the case: if ( $resfound && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) { I wonder if we confuse people? How could we improve that? Another question: Would holding branch be a better wording? Changing status for feedback. Created attachment 107293 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any branch". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any branch". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup branch". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> [EDIT] Changed number of tests to 50. For wording, please avoid branch in favor of library (see terminology list). We are trying to be more consistent, which will help people get less confused, including translators who struggle with the synonyms. Created attachment 110083 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 110415 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 110437 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, borrowernumber => $patron->borrowernumber })->next; next unless $item->holdingbranch eq $res->branchcode; What if a given patron has several holds on the same record? Should not we loop on the different holds instead of testing only the first one? (In reply to Jonathan Druart from comment #13) > my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, > borrowernumber => $patron->borrowernumber })->next; > next unless $item->holdingbranch eq $res->branchcode; > > What if a given patron has several holds on the same record? Should not we > loop on the different holds instead of testing only the first one? I guess we could put another condition in the search params like: my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, borrowernumber => $patron->borrowernumber, branchcode => $item->holdingbranch }, { order_by => { -asc => 'priority' } })->next; What do you think? (In reply to Aleisha Amohia from comment #14) > (In reply to Jonathan Druart from comment #13) > > my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, > > borrowernumber => $patron->borrowernumber })->next; > > next unless $item->holdingbranch eq $res->branchcode; > > > > What if a given patron has several holds on the same record? Should not we > > loop on the different holds instead of testing only the first one? > > I guess we could put another condition in the search params like: > > my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, > borrowernumber => $patron->borrowernumber, branchcode => > $item->holdingbranch }, { order_by => { -asc => 'priority' } })->next; > > What do you think? My concern was about the other holds a patron could have on the same bibliographic record. I don't know if it makes sense but, reading the code: 2826 if ( C4::Context->preference('AllowRenewalIfOtherItemsAvailableLocation') eq 'holdbranch' ){ 2827 my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, borrowernumber => $patron->borrowernumber })->next; 2828 next unless $item->holdingbranch eq $res->branchcode; 2829 } We skip the patron if they have their first hold on this biblio in the holding library of the item. Should not we do this test on all the holds this patron has on the bibliographic record? Katrin, what do you think? Waiting for an answer here, adjusting status. (In reply to Jonathan Druart from comment #15) > (In reply to Aleisha Amohia from comment #14) > > (In reply to Jonathan Druart from comment #13) > > > my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, > > > borrowernumber => $patron->borrowernumber })->next; > > > next unless $item->holdingbranch eq $res->branchcode; > > > > > > What if a given patron has several holds on the same record? Should not we > > > loop on the different holds instead of testing only the first one? > > > > I guess we could put another condition in the search params like: > > > > my $res = Koha::Holds->search({ biblionumber => $item->biblionumber, > > borrowernumber => $patron->borrowernumber, branchcode => > > $item->holdingbranch }, { order_by => { -asc => 'priority' } })->next; > > > > What do you think? > > My concern was about the other holds a patron could have on the same > bibliographic record. > I don't know if it makes sense but, reading the code: > > 2826 if ( > C4::Context->preference('AllowRenewalIfOtherItemsAvailableLocation') eq > 'holdbranch' ){ > 2827 my $res = Koha::Holds->search({ biblionumber => > $item->biblionumber, borrowernumber => $patron->borrowernumber })->next; > 2828 next unless $item->holdingbranch eq > $res->branchcode; > 2829 } > > We skip the patron if they have their first hold on this biblio in the > holding library of the item. > Should not we do this test on all the holds this patron has on the > bibliographic record? > > Katrin, what do you think? Ugh, sorry, I just came across this. I understand that the goal is that a renewal should be prevented, if there is a another pending hold where the pickup library matches the holding branch (checkout library) of the item to be renewed. Is that correct? I think it might be unlikely for a patron to place multiple holds on a record for pick-up at different libraries, but it would be possible. So looping through all existing holds seems logical to me. But I am feeling unsure here about why we are interested in the holds of a specific patron... are we not interested in looking at the holds for this specific record in general? (In reply to Elizabeth John from comment #18) Spam Created attachment 161126 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> (In reply to Katrin Fischer from comment #17) > (In reply to Jonathan Druart from comment #15) > I understand that the goal is that a renewal should be prevented, if there > is a another pending hold where the pickup library matches the holding > branch (checkout library) of the item to be renewed. Is that correct? > > I think it might be unlikely for a patron to place multiple holds on a > record for pick-up at different libraries, but it would be possible. So > looping through all existing holds seems logical to me. > > But I am feeling unsure here about why we are interested in the holds of a > specific patron... are we not interested in looking at the holds for this > specific record in general? Thank you both for your thoughts. I ended up taking out the loop entirely because at that point we are already looping through the fillable holds on this biblio. Hopefully that will suffice. The patch no longer applies 8-(.... git bz apply 25193 Bug 25193 - Add syspref to control location for use of AllowRenewalIfOtherItemsAvailable 161126 - Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Using index info to reconstruct a base tree... M C4/Circulation.pm M installer/data/mysql/mandatory/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref M t/db_dependent/Circulation.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Circulation.t CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging installer/data/mysql/mandatory/sysprefs.sql Auto-merging C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref Created attachment 171168 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Everything worked fine up until step 9 in the test plan. Step 9 - For the details section, the table of checkouts for Patron A doesn't load and says "Loading... you may continue scanning". The error in web development tools console: XHRGET http://127.0.0.1:8081/cgi-bin/koha/svc/checkouts?borrowernumber=49 [HTTP/1.1 500 Internal Server Error 76ms] Errir in /var/log/koha/kohadev/plack-error.log: 655 Starman: Accepting connections at unix://localhost:/var/run/koha/kohadev/plack.sock/ 656 Can't call method "branchcode" on unblessed reference at /kohadevbox/koha/C4/Circulation.pm line 3116 657 C4::Circulation::CanBookBeRenewed('Koha::Patron=HASH(0x5c765b745670)', 'Koha::Checkout=HASH(0x5c765b83a1f8)') called at /kohadevbox/koha/svc/checkouts line 161 658 eval {...} at /kohadevbox/koha/svc/checkouts line 2 659 CGI::Compile::ROOT::kohadevbox_koha_svc_checkouts::__ANON__('CGI::Compile=HASH(0x5c765b87ce88)', '', '/kohadevbox/koha/svc/checkouts', '/kohadevbox/koha/svc', 'ARRAY(0x5c765ba4f568)') called at /usr/shar e/perl5/CGI/Compile.pm line 151 660 CGI::Compile::ROOT::kohadevbox_koha_svc_checkouts::checkouts at /usr/share/perl5/CGI/Emulate/PSGI.pm line 30 661 CGI::Emulate::PSGI::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/App/WrapCGI.pm line 95 662 Plack::App::WrapCGI::call('Plack::App::WrapCGI=HASH(0x5c765be07230)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 663 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/App/CGIBin.pm line 50 664 Plack::App::CGIBin::serve_path('Plack::App::CGIBin=HASH(0x5c7650770098)', 'HASH(0x5c765bae8fa8)', '/kohadevbox/koha/svc/checkouts') called at /usr/share/perl5/Plack/App/File.pm line 34 665 Plack::App::File::call('Plack::App::CGIBin=HASH(0x5c7650770098)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 666 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /kohadevbox/koha/Koha/Middleware/CSRF.pm line 92 667 Koha::Middleware::CSRF::call('Koha::Middleware::CSRF=HASH(0x5c765b7707e8)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 668 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Middleware/LogWarn.pm line 22 669 Plack::Middleware::LogWarn::call('Plack::Middleware::LogWarn=HASH(0x5c765b84c0d0)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 670 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Middleware/Log4perl.pm line 30 671 Plack::Middleware::Log4perl::call('Plack::Middleware::Log4perl=HASH(0x5c765b8041c0)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 672 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/App/URLMap.pm line 71 673 Plack::App::URLMap::call('Plack::App::URLMap=HASH(0x5c765a091da8)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 674 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /kohadevbox/koha/Koha/Middleware/RealIP.pm line 64 675 Koha::Middleware::RealIP::call('Koha::Middleware::RealIP=HASH(0x5c765b7c57a8)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 676 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /kohadevbox/koha/Koha/Middleware/SetEnv.pm line 106 677 Koha::Middleware::SetEnv::call('Koha::Middleware::SetEnv=HASH(0x5c765b804388)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 678 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /kohadevbox/koha/Koha/Middleware/UserEnv.pm line 27 679 Koha::Middleware::UserEnv::call('Koha::Middleware::UserEnv=HASH(0x5c765b809e00)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 680 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Middleware/Static.pm line 18 681 Plack::Middleware::Static::call('Plack::Middleware::Static=HASH(0x5c765b908d88)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 682 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Middleware/ReverseProxy.pm line 68 683 Plack::Middleware::ReverseProxy::call('Plack::Middleware::ReverseProxy=HASH(0x5c765b908f38)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 684 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Middleware/Lint.pm line 24 685 Plack::Middleware::Lint::call('Plack::Middleware::Lint=HASH(0x5c76507685a0)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 686 Plack::Middleware::StackTrace::try {...} at /usr/share/perl5/Try/Tiny.pm line 100 687 eval {...} at /usr/share/perl5/Try/Tiny.pm line 93 688 Plack::Middleware::StackTrace::call('Plack::Middleware::StackTrace=HASH(0x5c765a0699e0)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 689 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Middleware/AccessLog.pm line 27 690 Plack::Middleware::AccessLog::call('Plack::Middleware::AccessLog=HASH(0x5c765a079818)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Component.pm line 50 691 Plack::Component::__ANON__('HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Plack/Util.pm line 145 692 eval {...} at /usr/share/perl5/Plack/Util.pm line 145 693 Plack::Util::run_app('CODE(0x5c765a079728)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Starman/Server.pm line 195 694 Starman::Server::dispatch_request('Starman::Server=HASH(0x5c765013acd0)', 'HASH(0x5c765bae8fa8)') called at /usr/share/perl5/Starman/Server.pm line 299 695 Starman::Server::process_request('Starman::Server=HASH(0x5c765013acd0)', 'Net::Server::Proto::UNIX=GLOB(0x5c765b7eb820)') called at /usr/share/perl5/Net/Server.pm line 72 696 Net::Server::run_client_connection('Starman::Server=HASH(0x5c765013acd0)') called at /usr/share/perl5/Net/Server/PreFork.pm line 226 697 eval {...} at /usr/share/perl5/Net/Server/PreFork.pm line 226 698 Net::Server::PreFork::run_child('Starman::Server=HASH(0x5c765013acd0)') called at /usr/share/perl5/Net/Server/PreFork.pm line 184 699 Net::Server::PreFork::run_n_children('Starman::Server=HASH(0x5c765013acd0)', 2) called at /usr/share/perl5/Net/Server/PreFork.pm line 107 700 Net::Server::PreFork::loop('Starman::Server=HASH(0x5c765013acd0)') called at /usr/share/perl5/Net/Server.pm line 58 701 Net::Server::run('Starman::Server=HASH(0x5c765013acd0)', 'port', 'ARRAY(0x5c76506f51b8)', 'host', '*', 'proto', 'tcp', 'serialize', 'none', 'min_servers', 2, 'min_spare_servers', 1, 'max_spare_servers', 1, 'max_servers', 2, 'max_requests', 50, 'user', 'kohadev-koha', 'group', 'kohadev-koha', 'listen', 1024, 'check_for_waiting', 1, 'no_client_stdout', 1, 'background', 1, 'log_file', '/var/log/koha/kohadev/pl ack-error.log', 'pid_file', '/var/run/koha/kohadev/plack.pid', 'setsid', 1) called at /usr/share/perl5/Starman/Server.pm line 106 702 Starman::Server::run('Starman::Server=HASH(0x5c765013acd0)', 'CODE(0x5c765013ab80)', 'HASH(0x5c765013a928)') called at /usr/share/perl5/Plack/Handler/Starman.pm line 25 703 Plack::Handler::Starman::run('Plack::Handler::Starman=HASH(0x5c765013ab98)', 'CODE(0x5c765013ab80)') called at /usr/share/perl5/Plack/Loader/Delayed.pm line 20 704 Plack::Loader::Delayed::run('Plack::Loader::Delayed=HASH(0x5c764fe81250)', 'Plack::Handler::Starman=HASH(0x5c765013ab98)') called at /usr/share/perl5/Plack/Runner.pm line 279 705 Plack::Runner::run('Plack::Runner=HASH(0x5c764fe28e68)') called at /bin/starman line 38 Created attachment 172554 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Aleisha and I have fixed up the patch so it's ready to be signed off again please :) Created attachment 175482 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177845 [details] [review] Bug 25193: Add AllowRenewalIfOtherItemsAvailableLocation syspref This syspref works in conjunction with AllowRenewalIfOtherItemsAvailable syspref. It has two options - allow renewal if other items are available to fill holds at any branch, or only at the hold's pickup branch. To test: 1) Apply patch, update database, restart services 2) Go to Administration -> system preferences. Search for AllowRenewalIfOtherItemsAvailable. Notice the two sysprefs exist in the same row. By default they should be set to "Don't allow" and "at any library". 3) Set up a biblio with two items. Have both items held at Branch A. 4) Check out Item A to Borrower A. 5) Place a biblio-level reserve for Borrower B with pickup branch at Branch A. 6) View Borrower A's checkouts. Confirm you cannot renew Item A ('On hold' message). 7) Edit Item B, set the current location to Branch B. The items should now have different locations. 8) Enable the AllowRenewalIfOtherItemsAvailable syspref. Leave the AllowRenewalIfOtherItemsAvailableLocation syspref set to "at any library". 9) View Borrower A's checkouts. Confirm you CAN renew Item A (because Item B can fill Borrower B's reserve). 10) Set the AllowRenewalIfOtherItemsAvailableLocation syspref to "at the hold's pickup library". Keep AllowRenewalIfOtherItemsAvailable syspref enabled. 11) View Borrower A's checkouts. Confirm you CANNOT renew Item A (because Item B cannot fill Borrower B's reserve due to being at a different branch). 12) Edit Item B, set the current location to Branch A. The items should have the same location now. 13) View Borrower A's checkouts. Confirm you CAN renew Item A. Sponsored-by: Chartered Accountants Australia & New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Nind <david@davidnind.com> Used the auto_rebase.pl script to rebase this patch $ export BUG_NUMBER=25193; perl auto_rebase.pl --bugzilla $BUG_NUMBER Creating a temporary branch 'bug_25193-tmp'... Guessing a date to use from bugzilla's bug 25193 First patch from this bug was applied at 2024-12-15T19:19:58Z Resetting HEAD to where origin/main was at 2024-12-15T19:19:58Z (commit 1a4820322e528fdea4ac6dd212254ca3021d30a2) Applying patches using git bz Rebasing the temporary branch up to before the tidy commits... Creating a new branch 'bug_25193-rebased' starting at the end of the tidy commits... Getting commit list from branch 'bug_25193-tmp'... No number of commits passed, trying to guess it... Found 1 commits to process Processing 1 commits... Processing commit 1ae15053a9b... (1/1) Processing file C4/Circulation.pm... (1/5) Tidying file 1/1 (C4/Circulation.pm) Processing file installer/data/mysql/atomicupdate/bug_25193-add_AllowRenewalIfOtherItemsAvailableLocation_syspref.pl... (2/5) Tidying file 1/1 (installer/data/mysql/atomicupdate/bug_25193-add_AllowRenewalIfOtherItemsAvailableLocation_syspref.pl) Processing file installer/data/mysql/mandatory/sysprefs.sql... (3/5) Tidying file 1/1 (installer/data/mysql/mandatory/sysprefs.sql) Cannot guess filetype for installer/data/mysql/mandatory/sysprefs.sql at /tmp/FKRVdWhW6W line 191. Processing file koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref... (4/5) Tidying file 1/1 (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref) Cannot guess filetype for koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref at /tmp/FKRVdWhW6W line 191. Processing file t/db_dependent/Circulation.t... (5/5) Tidying file 1/1 (t/db_dependent/Circulation.t) Committing changes... All commits from the source branch have been applied to 'bug_25193-rebased'! Rebasing on top of main... Everything applied successfully! FAIL installer/data/mysql/atomicupdate/bug_25193-add_AllowRenewalIfOtherItemsAvailableLocation_syspref.pl FAIL file permissions File must have the exec flag FAIL installer/data/mysql/mandatory/sysprefs.sql FAIL semicolon simicolon found instead of comma at line 878 @Joubu Jonathan: See the simicolon? :) (In reply to Marcel de Rooy from comment #30) > FAIL installer/data/mysql/mandatory/sysprefs.sql > FAIL semicolon > simicolon found instead of comma at line 878 > > > @Joubu Jonathan: See the simicolon? :) It could be considered false-positive, but actually I think it highlights another problem: you are no supposed to change the "newline at end of file". It is an editor configuration issue. |