|
Description
Aleisha Amohia
2022-08-18 05:38:54 UTC
Created attachment 139442 [details] [review] Issue 1: RecallsInterface system preference (cherry picked from commit fed2dcd820335395f6478754e76c3e0d751bee57) Created attachment 139443 [details] [review] Issue 1: Implement RecallsInterface on OPAC (cherry picked from commit 75eb449b369255c3288bdc55ed274e38bf29c5c2) Created attachment 139444 [details] [review] Issue 1: Convert reserves to recalls on staff client (cherry picked from commit 1c6c784e4c392ca5d12bd003559146fce60dcaed) Issue 1: Convert recalls and place staff-side recalls (cherry picked from commit 3a02cb1c0769213d9a5988600fadf7e305cc3637) This is a work in progress and is not ready for testing Created attachment 139446 [details] [review] Issue 1: Convert reserves to recalls on staff client (cherry picked from commit 1c6c784e4c392ca5d12bd003559146fce60dcaed) Issue 1: Convert recalls and place staff-side recalls (cherry picked from commit 3a02cb1c0769213d9a5988600fadf7e305cc3637) Created attachment 139642 [details] [review] Bug 31391: RecallsInterface system preference Created attachment 139643 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Created attachment 139644 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Created attachment 139645 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Patch no longer applies 8-(.. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 31391: RecallsInterface system preference Applying: Bug 31391: Recalled authorised value for hold cancellation Applying: Bug 31391: Implement RecallsInterface on OPAC Applying: Bug 31391: Request recalls and convert reserves on staff interface Using index info to reconstruct a base tree... M Koha/Item.pm M koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt M koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt M reserve/request.pl Falling back to patching base and 3-way merge... Auto-merging reserve/request.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt Auto-merging Koha/Item.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 31391: Request recalls and convert reserves on staff interface Created attachment 141474 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Hi Aleisha.
Everything works as per the test plan EXCEPT for step 7.
I cannot cancel a recall and existing or new recall from the OPAC after changing the system preference from 'the OPAC only' to 'the staff interface only'.
For step 7 (have an existing recall, hcnage system preference):
- Can no longer place recalls (as expected).
- Cannot cancel a recall (in OPAC from either your summary > recalls tab or your recalls history). Message is: "Recalls have not been enabled. Please contact your library."
David
---------------------
Testing notes (using koha-testing-docker):
---------------------
Patrons used:
- Patron A: Mary Burton (Midway) (changed password and set username to first name)
- Patron B: Henry Acevedo (Midway)
Step 2: Circulation rules for recalls
- Recalls allowed (total): 5
- Recalls per record (count): 1
- On shelf recalls allowed: If any unavailable
- Recall due date interval (day): 5
- Recall overdue fine amount:5
- Recall pickup period (day):2
Step 5:
- Placed and cancelled a recall from:
. the patron's OPAC account (your summary > recalls > cancel)
. in the staff interface:
. recalls tab for the record
. patron's account - details section > holds tab
. patron's account - recalls history section
. circulation > recalls > recalls queue
- Placed and fulfilled the recall
. place recall
. check item in (and transfer if required)
. check item is listed under circulation > recalls > recalls awaiting pickup
. check under patron's account that recall status is 'Ready for pickup'
. checkout out item to recall requestor
. check in item again so that it is available
Step 6: before changing the system preference to staff interface only, checkout an item to Patron B and place a recall again for Patron A so that you can view in their OPAC account and cancel
- should still be a recalls tab in the your summary section
- your recalls history should still be visible
- can cancel from both places
Step 8: check out Item to Patron B first
Created attachment 142594 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Hi Aleisha. Apologies for Failing QA on this again! The previous issue reported is fixed (step 7 - can't cancel a recall from OPAC when RecallsInterface = staff interface only). I've identified a new issue with step 5 - cancelling a recall from the recalls section for a record - record doesn't exist after cancelling a recall using the 'Cancel selected recalls' button and selecting the normal section. To replicate: 1. Place a recall 2. Go to the recalls section for the record on the staff interface 3. Using Actions > Cancel works as expected 4. Repeat steps 1 and 2. 5. Cancel the recall by ticking the box next to the item and then selecting 'Cancel selected recalls' button doesn't work as expected: . The recall is cancelled . URL = http://127.0.0.1:8081/cgi-bin/koha/recalls/request.pl . Breadcrumb = Home > Catalog > No title > Place a recall . Click on the normal section: message = The record you requested does not exist (). URL = http://127.0.0.1:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber= I'm not sure if I tested this last time. I'm also sure I've seen this when testing something else a while ago, but can't remember or find what bug it was. Also, for step 13 it is possible to have more than one recall for a record or item. Whose recall takes priority, and what is the workflow libraries would use to address this? - If a recall is already on the record (from the previous step), there are now two recalls (a record level one, and an item level one) - On the record page is shows "recalled by X on [date]" - for X it shows the person who made the latest recall, not the first person who place a recall. - Is this an edge case to be resolved in a separate bug, or a non-issue? - It seems logical that multiple patrons could make a recall request for the same record. What would the workflow be for libraries if they were in this situation, and how do they resolve this? David Created attachment 143155 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox (In reply to David Nind from comment #14) > Hi Aleisha. > > Apologies for Failing QA on this again! > > The previous issue reported is fixed (step 7 - can't cancel a recall from > OPAC when RecallsInterface = staff interface only). > > I've identified a new issue with step 5 - cancelling a recall from the > recalls section for a record - record doesn't exist after cancelling a > recall using the 'Cancel selected recalls' button and selecting the normal > section. > > To replicate: > 1. Place a recall > 2. Go to the recalls section for the record on the staff interface > 3. Using Actions > Cancel works as expected > 4. Repeat steps 1 and 2. > 5. Cancel the recall by ticking the box next to the item and then selecting > 'Cancel selected recalls' button doesn't work as expected: > . The recall is cancelled > . URL = http://127.0.0.1:8081/cgi-bin/koha/recalls/request.pl > . Breadcrumb = Home > Catalog > No title > Place a recall > . Click on the normal section: > message = The record you requested does not exist (). > URL = > http://127.0.0.1:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber= > > I'm not sure if I tested this last time. I'm also sure I've seen this when > testing something else a while ago, but can't remember or find what bug it > was. Fixed in latest follow-up > > Also, for step 13 it is possible to have more than one recall for a record > or item. Whose recall takes priority, and what is the workflow libraries > would use to address this? > - If a recall is already on the record (from the previous step), there are > now two recalls (a record level one, and an item level one) > - On the record page is shows "recalled by X on [date]" - for X it shows the > person who made the latest recall, not the first person who place a recall. > - Is this an edge case to be resolved in a separate bug, or a non-issue? > - It seems logical that multiple patrons could make a recall request for the > same record. What would the workflow be for libraries if they were in this > situation, and how do they resolve this? > I think that is outside of the scope of this particular ticket (because fulfilling recalls via the staff interface came from the original Bug 19532) but at the moment the oldest/earliest placed recall will be picked first, and priority is given to item-level recalls over biblio-level recalls. A follow-up to all of this would be to introduce a priority field like reserves has, but that can come after we bring in functionality to place recalls via the staff interface I think > > David (In reply to Aleisha Amohia from comment #16) > (In reply to David Nind from comment #14) > > Hi Aleisha. > > > > Apologies for Failing QA on this again! > > > > The previous issue reported is fixed (step 7 - can't cancel a recall from > > OPAC when RecallsInterface = staff interface only). > > > > I've identified a new issue with step 5 - cancelling a recall from the > > recalls section for a record - record doesn't exist after cancelling a > > recall using the 'Cancel selected recalls' button and selecting the normal > > section. > > > > To replicate: > > 1. Place a recall > > 2. Go to the recalls section for the record on the staff interface > > 3. Using Actions > Cancel works as expected > > 4. Repeat steps 1 and 2. > > 5. Cancel the recall by ticking the box next to the item and then selecting > > 'Cancel selected recalls' button doesn't work as expected: > > . The recall is cancelled > > . URL = http://127.0.0.1:8081/cgi-bin/koha/recalls/request.pl > > . Breadcrumb = Home > Catalog > No title > Place a recall > > . Click on the normal section: > > message = The record you requested does not exist (). > > URL = > > http://127.0.0.1:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber= > > > > I'm not sure if I tested this last time. I'm also sure I've seen this when > > testing something else a while ago, but can't remember or find what bug it > > was. > > Fixed in latest follow-up This is still not working for me and doesn't appear to be fixed. If it helps, there is an entry in /var/log/koha/kohadev/plack-intranet-error.log: [2022/11/04 06:36:44] [WARN] Use of uninitialized value $biblionumber in concatenation (.) or string at /kohadevbox/koha/recalls/request.pl line 49. Everything else continues to work as per the test plan. Created attachment 144104 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Created attachment 144105 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox (In reply to David Nind from comment #17) > This is still not working for me and doesn't appear to be fixed. > > If it helps, there is an entry in > /var/log/koha/kohadev/plack-intranet-error.log: > [2022/11/04 06:36:44] [WARN] Use of uninitialized value $biblionumber in > concatenation (.) or string at /kohadevbox/koha/recalls/request.pl line > 49. > > Everything else continues to work as per the test plan. Sorry you were right - amended in most recent patch Created attachment 144131 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David <david@davidnind.com> Created attachment 144132 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Created attachment 144133 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Created attachment 144134 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Created attachment 144135 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Created attachment 144136 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> (In reply to Aleisha Amohia from comment #20) > (In reply to David Nind from comment #17) > > This is still not working for me and doesn't appear to be fixed. > > > > If it helps, there is an entry in > > /var/log/koha/kohadev/plack-intranet-error.log: > > [2022/11/04 06:36:44] [WARN] Use of uninitialized value $biblionumber in > > concatenation (.) or string at /kohadevbox/koha/recalls/request.pl line > > 49. > > > > Everything else continues to work as per the test plan. > > Sorry you were right - amended in most recent patch Thanks Aleisha - now signed off! I added a follow-up to change the number of tests so that tests pass, and change staff client to staff interface. David (In reply to David Nind from comment #27) > Thanks Aleisha - now signed off! > > I added a follow-up to change the number of tests so that tests pass, and > change staff client to staff interface. > > David Good catch! Thank you for your awesome testing. Doing a first round of testing here: 1) QA test tools FAIL installer/data/mysql/atomicupdate/bug_31391_-_add_HOLD_CANCELLATION-RECALLED_authorised_value.pl FAIL file permissions File must have the exec flag FAIL installer/data/mysql/atomicupdate/bug_31391_-_add_RecallsInterface_syspref.pl FAIL file permissions File must have the exec flag 2) Unit tests There is a new warn that is not there when running the test on master, that should be fixed to give a clean result: t/db_dependent/Holds.t .. 76/77 An appropriate email address for borrower myfirstname.whateversurname1 could not be found. at /kohadevbox/koha/C4/Message.pm line 192. No reserves HOLD_CANCELLATION letter transported by email at /kohadevbox/koha/C4/Letters.pm line 584. t/db_dependent/Holds.t .. ok All tests successful. 3) Database update * is idempotent * installer/data/mysql/atomicupdate/bug_31391_-_add_HOLD_CANCELLATION-RECALLED_authorised_value. could be updated to also use the new style 3) Code review + <th>Last location</th> a) This might be a little confusing with the 'location' in item. Could we use 'Last library' or even 'holding library' here? + [% IF ( item.itemlost || item.withdrawn ) %] + <span class="lost">Unavailable (lost or missing)</span> + [% END %] b) This looks like it should read "(lost or withdrawn)" + Patron has already placed a <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% patron.borrowernumber | uri %]#reserves">reserve</a> on this item. c) Terminology: 'reserve' should be 'hold'. 4) GUI testing a) The system preference RecallsInterface appears in the Curbside pickup section instead of in recalls. Maybe a rebase issue. b) Could be a separate bug: when placing a recall from the detail page, the list of existing recalls needs the 'page-section' class to appear correctly in the new staff interface design with white background. c) Could be a separate bug: Cancel recalls on the same page should be a yellow primary button. d) Should I be able to to place a record level hold and a recall level hold for the same patron at the same time? It seems like maybe we should have a warning or a message there, maybe recommending to convert the existing hold. e) When converting the hold, the recall action is 'Hiding' in the priorities. I feel like a link or icon might be a little nicer as I almost missed it there. Not blocker. Stopping here for now, but I feel this will make the feature much more flexible! Created attachment 147581 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page Created attachment 147582 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page Created attachment 150485 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David Nind <david@davidnind.com> Created attachment 150486 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150487 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150488 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150489 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150490 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150491 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page Signed-off-by: David Nind <david@davidnind.com> IMO we are too far in the release cycle to push this one for 23.05. Additionally there are several bugs opened for the OPAC part that are still waiting to be fixed. Bug 31415 Depends on bug 31391 (Signed Off) Follow? [(y)es, (n)o] y The patch no longer applies (discovered when testing bug 31415): Bug 31391 - Staff-side recalls 150485 - Bug 31391: RecallsInterface system preference 150486 - Bug 31391: Recalled authorised value for hold cancellation 150487 - Bug 31391: Implement RecallsInterface on OPAC 150488 - Bug 31391: Request recalls and convert reserves on staff interface 150489 - Bug 31391: (follow-up) Fix cancel recalls using checkbox 150490 - Bug 31391: (follow-up) Fix number of tests and terminology 150491 - Bug 31391: Interface and wording fixes Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 31391: RecallsInterface system preference Applying: Bug 31391: Recalled authorised value for hold cancellation Applying: Bug 31391: Implement RecallsInterface on OPAC Using index info to reconstruct a base tree... M koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc M koha-tmpl/opac-tmpl/bootstrap/en/includes/title-actions-menu.inc Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/title-actions-menu.inc CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/includes/title-actions-menu.inc Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc error: Failed to merge in the changes. Patch failed at 0001 Bug 31391: Implement RecallsInterface on OPAC Created attachment 156995 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 156996 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 156997 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 156998 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 156999 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 157000 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 157001 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page - Include changes to patron search dropdown Signed-off-by: David Nind <david@davidnind.com> Everything still works, however the tests are failing for me: prove t/db_dependent/Holds.t t/db_dependent/Holds.t .. 68/74 Use of uninitialized value in numeric le (<=) at /kohadevbox/koha/C4/Circulation.pm line 3002. Use of uninitialized value in numeric le (<=) at /kohadevbox/koha/C4/Circulation.pm line 3002. Use of uninitialized value in numeric le (<=) at /kohadevbox/koha/C4/Circulation.pm line 3002. t/db_dependent/Holds.t .. 73/74 # No tests run! # Failed test 'No tests run for subtest "ModReserve to convert a hold to a recall"' # at t/db_dependent/Holds.t line 1902. Can't call method "categorycode" on unblessed reference at /kohadevbox/koha/C4/Circulation.pm line 3830. # Looks like your test exited with 11 just after 74. t/db_dependent/Holds.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 1/74 subtests Test Summary Report ------------------- t/db_dependent/Holds.t (Wstat: 2816 Tests: 74 Failed: 1) Failed test: 74 Non-zero exit status: 11 Files=1, Tests=74, 7 wallclock secs ( 0.04 usr 0.01 sys + 3.62 cusr 1.28 csys = 4.95 CPU) Result: FAIL Created attachment 158691 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158692 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158693 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158694 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page - Include changes to patron search dropdown - add 'mark hold for conversion to recall' column to columns settings Signed-off-by: David Nind <david@davidnind.com> Created attachment 158695 [details] [review] Bug 31391: Test fixes Created attachment 158825 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158826 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158827 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158828 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158829 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158830 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 158831 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page - Include changes to patron search dropdown - add 'mark hold for conversion to recall' column to columns settings Signed-off-by: David Nind <david@davidnind.com> Created attachment 158832 [details] [review] Bug 31391: Test fixes Signed-off-by: David Nind <david@davidnind.com> Thanks Aleisha! The tests now pass, and everything seems to be working as expected. David Test plan works as described and qa script seems happy, but I don't feel comfortable passing or failing QA here, so I'll leave that to someone else and provide some observations instead: Instead of adding RecallsInterface, should we consider updating UseRecalls sys pref to: 0 - Don't use 1 - Use on OPAC only 2 - Use on staff interface only 3 - Use on both OPAC and staff interface This would address the use-cases being covered here and not be YASP. I understand it's more work but it seems partially redundant to have both UseRecalls and RecallsInterface. I believe using the values I suggest above would ease the work, since the current UseRecalls = 1 is equivalent to the would be RecallsInterface = 'opac', although some current checks for truthy UseRecalls would also now encompass '2' and '3', maybe not critical but just something to consider. -------------- Some improvements to error messaging could be considered: Attempt to place a recall on an item not checked out, you get: "Unable to place a recall. Check your circulation rules." When you should get something like: "Unable to place a recall on an item not checked out." Attempt to place a recall on the same patron that the item is checked out to, you get: "Unable to place a recall. Check your circulation rules." When you should get something like "Unable to place recall. Item is already checked out to user X" I agree with the preference suggestion here. We also need the error messages improveing as highlighting, Failing QA Thanks all I agree with your suggestions and am working on implementing these. Created attachment 172166 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 172167 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 172168 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 172169 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 172170 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 172171 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 172172 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page - Include changes to patron search dropdown - add 'mark hold for conversion to recall' column to columns settings Signed-off-by: David Nind <david@davidnind.com> Created attachment 172173 [details] [review] Bug 31391: Test fixes Signed-off-by: David Nind <david@davidnind.com> Created attachment 172174 [details] [review] Bug 31391: Update the UseRecalls system preference to have choices Options are: * off * opac * staff * opac_and_staff This patch implements this all over the place. All recalls things should still work including the above test plan. Tests should pass: * t/db_dependent/Circulation.t * t/db_dependent/Circulation/transferbook.t * t/db_dependent/Holds.t * t/db_dependent/Koha/Biblio.t * t/db_dependent/Koha/Item.t * t/db_dependent/SIP/Transaction.t * t/db_dependent/Search.t * t/db_dependent/XSLT.t Sponsored-by: Auckland University of Technology Sponsored-by: Catalyst IT Created attachment 172175 [details] [review] Bug 31391: [WIP] Better recalls error messages Hi Matrin, Pedro and David I have: - rebased the patches - implemented UseRecalls to have choices so that RecallsInterface isn't needed - begun working on the recalls error messages This ends up causing significant changes to the existing recalls logic so I wanted to check my approach before I get too deep. Could you please look at the latest WIP patch and let me know if this is what you had in mind for implementing those error messages? Something I'm concerned about in particular is that included more detailed error messages means you can no longer do `if ( $biblio->can_be_recalled )` as it returns a string which Perl interprets at `true`. Would appreciate any thoughts or ideas. I am not sure I can help much with the questions on development side, but please let me know once this is ready for testing again and happy to help with that. Hi Aleisha, providing my input here. Patch doesn't apply (sorry!). I'll try to come back faster next you're available to rebase. I've taken a look at the WIP patch. At first glance, it looks good, but it also looks like it may be a bit out of scope of the original intent here, which is to provide staff-side recalls, so I don't think it should be reason enough to block the whole thing because of it. Your call if you think we should tackle it here or perhaps better on a follow-up bug. Having said that, I'll look again once it applies again. (In reply to Pedro Amorim from comment #78) > Hi Aleisha, providing my input here. > Patch doesn't apply (sorry!). I'll try to come back faster next you're > available to rebase. > > I've taken a look at the WIP patch. At first glance, it looks good, but it > also looks like it may be a bit out of scope of the original intent here, > which is to provide staff-side recalls, so I don't think it should be reason > enough to block the whole thing because of it. Your call if you think we > should tackle it here or perhaps better on a follow-up bug. > Having said that, I'll look again once it applies again. Thanks Pedro. It does feel wider than the original scope of the bug. I'll see what I can do to address the earlier comments whilst keeping this within the scope. Created attachment 173839 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 173840 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 173841 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 173842 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 173843 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 173844 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 173845 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page - Include changes to patron search dropdown - add 'mark hold for conversion to recall' column to columns settings Signed-off-by: David Nind <david@davidnind.com> Created attachment 173846 [details] [review] Bug 31391: Test fixes Signed-off-by: David Nind <david@davidnind.com> Created attachment 173847 [details] [review] Bug 31391: Update the UseRecalls system preference to have choices Options are: * off * opac * staff * opac_and_staff This patch implements this all over the place. All recalls things should still work including the above test plan. Tests should pass: * t/db_dependent/Circulation.t * t/db_dependent/Circulation/transferbook.t * t/db_dependent/Holds.t * t/db_dependent/Koha/Biblio.t * t/db_dependent/Koha/Item.t * t/db_dependent/SIP/Transaction.t * t/db_dependent/Search.t * t/db_dependent/XSLT.t Sponsored-by: Auckland University of Technology Sponsored-by: Catalyst IT I've rebased patches and hopefully improved the styling of the error message, but I would like to work on improving the content of the error message in a follow-up bug. (In reply to Aleisha Amohia from comment #89) > I've rebased patches and hopefully improved the styling of the error > message, but I would like to work on improving the content of the error > message in a follow-up bug. Hi Aleisha. Thanks for rebasing! I've changed the status to Failed QA as: 1. I can't place a recall using the staff interface (with staff interface or both selected for placing recalls): 1.1 Patron search box doesn't work as expected, so can't select a patron to place a recall for: autocomplete doesn't work, search brings you to an empty record, browse by last name does nothing (the autocomplete still works on the holds section). 1.2 Couldn't test steps 12-14, 17-19. Testing notes (using KTD): 1. Test plan: with follow-up patches, there is now only one system preference UseRecalls, you can select which interfaces to allow recalls to be placed from Other notes: 1. With recalls allowed from the staff interface only and an existing recall: 1.1 In the OPAC, the status for the record as a "Recall" in a button, under "Checked out". 1.2 If you click "Recall" you get "Recalls have not been enabled. Please contact your library." on the account's "Recalls history" section. 1.3 Not sure if that is what is expected or not. Created attachment 174396 [details] [review] Bug 31391: (follow-up) Include changed requirements of patron autocomplete, datatable Hey looking here, some minor observations (these may not be issues and are possibly caused by my lack of understanding - follow-up as you see fit):
In koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
We have
IF Koha.Preference('UseRecalls') != "off"
Should it not be
IF Koha.Preference('UseRecalls').match('opac)
i.e. do we want to show this on this template if UseRecalls = 'staff'?
In opac-recall.pl:
Shouldn't the check for if($op eq 'cud-cancel) be inside the if ( C4::Context->preference('UseRecalls') =~ m/opac/ ) check?
---------
Code looks good and QA script is happy. I'll follow the test plan soon and provide a SO if all is well.
Created attachment 174822 [details] [review] Bug 31391: RecallsInterface system preference Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174823 [details] [review] Bug 31391: Recalled authorised value for hold cancellation Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174824 [details] [review] Bug 31391: Implement RecallsInterface on OPAC Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174825 [details] [review] Bug 31391: Request recalls and convert reserves on staff interface This enhancement adds the ability to place recalls via the staff interface by introducing a system preference, RecallsInterface, to configure where recalls can be placed. Recalls can either be placed via the OPAC, or the staff interface, or both. This is set to OPAC by default to be consistent with current behaviour. This enhancement also adds the ability to convert existing reserves to recalls easily via the staff interface. The reserve will be cancelled and a recall placed using the reserve's information. To test: 1) Update database and restart services 2) Enable UseRecalls and set the relevant recalls circulation rules 3) View the new RecallsInterface system preference. Confirm it is set to OPAC only by default 4) Check out Item A to Patron B. 5) Log into the OPAC as Patron A. Confirm you can place, view and cancel recalls as normal. 6) In the staff interface, set RecallsInterface to Staff interface only. 7) In the OPAC, confirm you can still view and cancel recalls, but can no longer place recalls. 8) In the staff interface, search for Item A and go to the Reserves tab. Place a reserve for Patron C. 9) Under the Priority column, select the dropdown and choose the recall option next to your reserve. 10) Click Update holds. Confirm when the page refreshes that your reserve has disappeared. 11) Go to the recalls tab. Confirm your reserve has been converted to a recall, and details like the patron, expiration, and pickup location have carried across to the recall. 12) On this recalls page, use the patron search to find Patron D. Test placing a record-level recall by keeping the 'recall next available item' checkbox. 13) When the page refreshes, confirm the recall was successfully placed with the correct details stored. 14) Repeat steps 12 and 13 with Patron E, this time place an item-level recall by choosing an item in the 'place a recall on a specific item' table. When selecting one of these items, the 'recall next available item' checkbox should de-select. 15) Set RecallsInterface back to OPAC only. Go back to the record and view the Recalls tab. 16) Confirm you cannot search for a patron to place a recall, but can still view and cancel recalls. Confirm you also cannot convert reserves to recalls. 17) Set RecallsInterface to both staff interface and OPAC. Confirm you can place recalls on both interfaces. 18) On the staff interface, test placing recalls that your circulation rules do not allow. For example, if 'recalls per record' is 1, ensure you're blocked from placing a second recall on a record for a patron. 19) Place a reserve for one of the patron's that already has a recall. Once complete, try to convert this reserve into a recall. Confirm you are blocked and shown a message that the hold cannot be converted to a recall. 20) Ensure tests pass t/db_dependent/Holds.t Sponsored-by: Auckland University of Technology Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174826 [details] [review] Bug 31391: (follow-up) Fix cancel recalls using checkbox Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174827 [details] [review] Bug 31391: (follow-up) Fix number of tests and terminology Fixed two items: - Number of tests: changed to 77 (from 76), tests now pass - Terminology: changed message when attempting to place recalls from the staff interface when RecallsInterface in set to the OPAC only (staff client should be staff interface) Signed-off-by: David <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174828 [details] [review] Bug 31391: Interface and wording fixes This patch includes the following fixes: - Making the atomic updates executable - Preventing warns when running tests - Updating db updates - Fixing terminology - Make 'cancel selected recalls' button yellow - Fix HTML makeup of recalls request page - Show a warning when placing a recall if there is already a record-level hold by this patron - Show a link to mark a hold for conversion which selects the recall option on holds page - Include changes to patron search dropdown - add 'mark hold for conversion to recall' column to columns settings Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174829 [details] [review] Bug 31391: Test fixes Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174830 [details] [review] Bug 31391: Update the UseRecalls system preference to have choices Options are: * off * opac * staff * opac_and_staff This patch implements this all over the place. All recalls things should still work including the above test plan. Tests should pass: * t/db_dependent/Circulation.t * t/db_dependent/Circulation/transferbook.t * t/db_dependent/Holds.t * t/db_dependent/Koha/Biblio.t * t/db_dependent/Koha/Item.t * t/db_dependent/SIP/Transaction.t * t/db_dependent/Search.t * t/db_dependent/XSLT.t Sponsored-by: Auckland University of Technology Sponsored-by: Catalyst IT Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 174831 [details] [review] Bug 31391: (follow-up) Include changed requirements of patron autocomplete, datatable Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Have signed-off as the observations I'm listing below are minor and quickly fixable (adding the ones from comment #92 here to have them all in a single comment): 1) In koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt We have IF Koha.Preference('UseRecalls') != "off" Should it not be IF Koha.Preference('UseRecalls').match('opac) i.e. do we want to show this on this template if UseRecalls = 'staff'? 2) In opac-recall.pl: Shouldn't the check for if($op eq 'cud-cancel) be inside the if ( C4::Context->preference('UseRecalls') =~ m/opac/ ) check? 3) Occurrences of: <input type="submit" class="btn btn-default" value="Place recall"> Should be using .btn-primary 4) I believe occurrences of: div class="dialog alert" Should be div class="alert alert-warning" 5) At first it seems recalls_per_record was buggy as I had it configured as 5 but was placing 10 recalls on a biblio record. I then realized 'record' in recalls_per_record refers to the patron record (?) and is consistent with the other circulation rules. 6) Bug 23269 should be rebased on this or vice-versa. I believe recalls API endpoints will be needed for bug 23269. |