Bug 36868 - Add ability to automatically empty cart after placing holds ( staff interface and OPAC )
Summary: Add ability to automatically empty cart after placing holds ( staff interface...
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Lucas Gass (lukeg)
QA Contact: Victor Grousset/tuxayo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-15 15:22 UTC by Lucas Gass (lukeg)
Modified: 2025-02-26 07:37 UTC (History)
5 users (show)

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


Attachments
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced (2.41 KB, patch)
2024-05-16 15:57 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Logic for staff interface (2.40 KB, patch)
2024-05-16 15:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Logic for staff interface (4.89 KB, patch)
2024-06-06 21:59 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Add logic for OPAC (3.44 KB, patch)
2024-06-13 17:59 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced (3.63 KB, patch)
2024-06-13 21:32 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Logic for staff interface (4.89 KB, patch)
2024-06-13 21:32 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Add logic for OPAC (4.38 KB, patch)
2024-06-13 21:32 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: (follow-up) Fix staff interface code (2.91 KB, patch)
2024-06-18 14:16 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced (3.67 KB, patch)
2024-06-18 14:28 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 36868: Logic for staff interface (4.93 KB, patch)
2024-06-18 14:28 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 36868: Add logic for OPAC (4.42 KB, patch)
2024-06-18 14:28 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 36868: (follow-up) Fix staff interface code (2.95 KB, patch)
2024-06-18 14:28 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced (4.44 KB, patch)
2025-02-11 20:35 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Logic for staff interface (4.98 KB, patch)
2025-02-11 20:35 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: Add logic for OPAC (4.31 KB, patch)
2025-02-11 20:35 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: (follow-up) Fix staff interface code (2.68 KB, patch)
2025-02-11 20:35 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36868: (QA follow-up): fix exec permission (711 bytes, patch)
2025-02-25 05:07 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36868: (QA follow-up): fix typo in DBRev and DBRev skeleton (1.72 KB, patch)
2025-02-26 06:05 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36868: (QA follow-up): fix typo in DBRev and DBRev skeleton (1.73 KB, patch)
2025-02-26 06:06 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2024-05-15 15:22:20 UTC
Some librarians would like to have the cart automatically clear itself after placing holds on items in the current cart. 

This would behave similar to how online retail/e-commerce sites work after one has purchased items.
Comment 1 Lucas Gass (lukeg) 2024-05-16 15:57:58 UTC
Created attachment 166834 [details] [review]
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced
Comment 2 Lucas Gass (lukeg) 2024-05-16 15:58:00 UTC
Created attachment 166835 [details] [review]
Bug 36868: Logic for staff interface
Comment 3 Lucas Gass (lukeg) 2024-06-06 21:59:40 UTC
Created attachment 167558 [details] [review]
Bug 36868: Logic for staff interface
Comment 4 Lucas Gass (lukeg) 2024-06-13 17:59:10 UTC
Created attachment 167695 [details] [review]
Bug 36868: Add logic for OPAC
Comment 5 Lucas Gass (lukeg) 2024-06-13 21:32:00 UTC
Created attachment 167699 [details] [review]
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced
Comment 6 Lucas Gass (lukeg) 2024-06-13 21:32:02 UTC
Created attachment 167700 [details] [review]
Bug 36868: Logic for staff interface
Comment 7 Lucas Gass (lukeg) 2024-06-13 21:32:04 UTC
Created attachment 167701 [details] [review]
Bug 36868: Add logic for OPAC

1. APPLY PATCH, updatedatabase, restart_all
2. Search for the new AutoDeleteFromCartWhenHoldPlaced system preference
3. Turn it on for both staff and OPAC
4. From the staff interface put some items in your cart.
5. Place a hold on all of them.
6. Once the hold(s) have been successfully done those items should no longer be in your cart.
7. Put more items into your staff cart, place a hold on some of them but not all.
8. Only the items you have placed a hold on should be removed.
9. Go to the OPAC and put some items in your cart.
10. Place a hold on all of them, those items should now be deleted from the cart.
11. Place more items into your OPAC cart, place a hold on some of them but not all.
12. Only the items you have placed a hold on should be removed.
13. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'staff'.
14. Try steps 4-8 again.
15. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'opac'.
16. Try steps 9 -12 again.
Comment 8 Sam Lau 2024-06-17 20:41:13 UTC
After adding items to cart, from the cart if I select all and place a hold, after completion, it only removes one of the items from the cart and the others stay. 

Placing the holds one by one worked though.

Also, qa script didn't like the ordering placement of the sys. pref in installer/data/mysql/mandatory/sysprefs.sql
Comment 9 Lucas Gass (lukeg) 2024-06-18 14:16:44 UTC
Created attachment 167851 [details] [review]
Bug 36868: (follow-up) Fix staff interface code
Comment 10 Sam Lau 2024-06-18 14:28:44 UTC
Created attachment 167852 [details] [review]
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 11 Sam Lau 2024-06-18 14:28:46 UTC
Created attachment 167853 [details] [review]
Bug 36868: Logic for staff interface

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 12 Sam Lau 2024-06-18 14:28:48 UTC
Created attachment 167854 [details] [review]
Bug 36868: Add logic for OPAC

1. APPLY PATCH, updatedatabase, restart_all
2. Search for the new AutoDeleteFromCartWhenHoldPlaced system preference
3. Turn it on for both staff and OPAC
4. From the staff interface put some items in your cart.
5. Place a hold on all of them.
6. Once the hold(s) have been successfully done those items should no longer be in your cart.
7. Put more items into your staff cart, place a hold on some of them but not all.
8. Only the items you have placed a hold on should be removed.
9. Go to the OPAC and put some items in your cart.
10. Place a hold on all of them, those items should now be deleted from the cart.
11. Place more items into your OPAC cart, place a hold on some of them but not all.
12. Only the items you have placed a hold on should be removed.
13. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'staff'.
14. Try steps 4-8 again.
15. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'opac'.
16. Try steps 9 -12 again.

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 13 Sam Lau 2024-06-18 14:28:50 UTC
Created attachment 167855 [details] [review]
Bug 36868: (follow-up) Fix staff interface code

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 14 Jonathan Druart 2025-01-16 10:24:00 UTC
Why not a checkbox instead of a new syspref?

Seems more flexible.
Comment 15 Lucas Gass (lukeg) 2025-01-16 22:30:05 UTC
(In reply to Jonathan Druart from comment #14)
> Why not a checkbox instead of a new syspref?
> 
> Seems more flexible.

I like any solution that doesn't require a syspref. Can you explain what you mean? A checkbox where?
Comment 16 Jonathan Druart 2025-01-22 11:27:47 UTC
(In reply to Lucas Gass (lukeg) from comment #15)
> (In reply to Jonathan Druart from comment #14)
> > Why not a checkbox instead of a new syspref?
> > 
> > Seems more flexible.
> 
> I like any solution that doesn't require a syspref. Can you explain what you
> mean? A checkbox where?

On the "Place holds" page. If at least one record is in the cart we could have a "Remove from cart" checkbox that will behave like what you have done with the pref if checkbed.
Comment 17 Lucas Gass (lukeg) 2025-02-11 20:35:50 UTC
Created attachment 177760 [details] [review]
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 18 Lucas Gass (lukeg) 2025-02-11 20:35:53 UTC
Created attachment 177761 [details] [review]
Bug 36868: Logic for staff interface

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 19 Lucas Gass (lukeg) 2025-02-11 20:35:56 UTC
Created attachment 177762 [details] [review]
Bug 36868: Add logic for OPAC

1. APPLY PATCH, updatedatabase, restart_all
2. Search for the new AutoDeleteFromCartWhenHoldPlaced system preference
3. Turn it on for both staff and OPAC
4. From the staff interface put some items in your cart.
5. Place a hold on all of them.
6. Once the hold(s) have been successfully done those items should no longer be in your cart.
7. Put more items into your staff cart, place a hold on some of them but not all.
8. Only the items you have placed a hold on should be removed.
9. Go to the OPAC and put some items in your cart.
10. Place a hold on all of them, those items should now be deleted from the cart.
11. Place more items into your OPAC cart, place a hold on some of them but not all.
12. Only the items you have placed a hold on should be removed.
13. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'staff'.
14. Try steps 4-8 again.
15. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'opac'.
16. Try steps 9 -12 again.

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 20 Lucas Gass (lukeg) 2025-02-11 20:35:58 UTC
Created attachment 177763 [details] [review]
Bug 36868: (follow-up) Fix staff interface code

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 21 Victor Grousset/tuxayo 2025-02-24 23:24:35 UTC
Comment on attachment 177760 [details] [review]
Bug 36868: Add system preference AutoDeleteFromCartWhenHoldPlaced

Review of attachment 177760 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ +763,5 @@
>              - pref: HoldRatioDefault
>                class: integer
>              - .
> +        -
> +            - Automatically delete items from cart when placing on hold on the

Phrasing looks weird: «when placing on hold on the OPAC».
Typo or maybe it's just an uncommon phrasing?
Comment 22 Victor Grousset/tuxayo 2025-02-25 05:07:03 UTC
Created attachment 178640 [details] [review]
Bug 36868: (QA follow-up): fix exec permission
Comment 23 Victor Grousset/tuxayo 2025-02-25 05:31:12 UTC
The new syspref needs to be added to Hea (C4/UsageStats.pm)
https://wiki.koha-community.org/wiki/Database_updates#Additional_hints

-------

QA script:
They looks either false positives or have their enforcement suspended (tt_valid) so nothing to worry here.

FAIL   installer/data/mysql/mandatory/sysprefs.sql
   FAIL   semicolon
                simicolon found instead of comma at line 877
   FAIL   sysprefs_order
                Not blocker: Sysprefs hidelostitems is bad placed (see bug 10610)

 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
   FAIL   tt_valid
                lines 388, 937, 1096

 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
   FAIL   tt_valid
                lines 368

------

I'll continue with testing asap.
Comment 24 Victor Grousset/tuxayo 2025-02-26 06:05:08 UTC Comment hidden (obsolete)
Comment 25 Victor Grousset/tuxayo 2025-02-26 06:06:02 UTC
Created attachment 178701 [details] [review]
Bug 36868: (QA follow-up): fix typo in DBRev and DBRev skeleton

Hijacking this follow-up to also fix the source (skeleton) which lead
this typo to have 40 occurences!
Comment 26 Victor Grousset/tuxayo 2025-02-26 06:09:32 UTC
1. Apply and update DB
2. AutoDeleteFromCartWhenHoldPlaced = both or OPAC only
3. Go to OPAC and on search results add 3 records to cart
4. One one result, place hold
5. "item has been removed from cart", count = 2
6. From the current "Your summary" page, cancel the hold
7. Now we want to go back to the search results to test more case.
   So we will use the back button a few times.
8. Go back once
9. "item has been removed from cart", count = 1
   Not great but doesn't really matter if it's just there.
10. Go back more times to get back to the search results
11. cart count = 3
    Not really a problem, it's just the old value because we are going back.
12. Click on any link to go to any page
13. cart count = 1
    Oh no it was persistent! >_<
14. click on cart 
15. 2 items
    Confirmation that cart count is now desynced >_<
    (better than removing another of the remaining records)


Instead of cancelling the hold, I could have went to any page and hit back, it would have likely(speculation) done the same.

I didn't test on staff interface.
Comment 27 Victor Grousset/tuxayo 2025-02-26 07:37:08 UTC
Comment on attachment 177762 [details] [review]
Bug 36868: Add logic for OPAC

Review of attachment 177762 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
@@ +1077,5 @@
>      [% INCLUDE 'datatables.inc' %]
>      <script>
> +        [% IF successful_holds && Koha.Preference('AutoDeleteFromCartWhenHoldPlaced').grep('opac').size %]
> +            [% FOREACH success IN successful_holds.split('\|') %]
> +                delSingleRecord('[% success | html %]');

Having a function call dropped in a <script> of page it likely the cause.
Such technique need the call to be idempotent.
Like making the count always be calculated from the cart content instead a separate variable? Even without going as far as such of a refactor, it looks like a bug that passing an already removed record would trigger the correct message but still decrement the count.

Or somehow having the call only done once. (self deleting <script> tag? 😆)