Bug 35906

Summary: Add bookable option on itemtypes
Product: Koha Reporter: Thibaud Guillot <thibaud.guillot>
Component: CirculationAssignee: Thibaud Guillot <thibaud.guillot>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, esther.melander, gmcharlt, kkrueger, kyle.m.hall, marie.hedbom, martin.renvoize, nick, r.delahunty, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29002
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 35248    
Bug Blocks: 36271    
Attachments: Bug 35906: Add bookable option on itemtype
Bug 35906: Add bookable option on itemtype
Bug 35906: Add bookable option on itemtype
Bug 35906: Add bookable option on itemtype
Bug 35906: Add bookable option on itemtype
Bug 35906 : Add unit test
Bug 35906: Add bookable option on itemtype
Bug 35906: Add unit test
Bug 35906: Remove preference and add override handling
Bug 35906: Schema dump
Bug 35906: Remove preference and add override handling
Bug 35906: Schema dump
Bug 35906: (QA follow-up) Fix item-type schema
Bug 35906: Add bookable option on itemtype
Bug 35906: Add unit test
Bug 35906: Remove preference and add override handling
Bug 35906: Schema dump
Bug 35906: (QA follow-up) Fix item-type schema
Bug 35906: Add bookable option on itemtype
Bug 35906: Add unit test
Bug 35906: Remove preference and add override handling
Bug 35906: Schema dump
Bug 35906: (QA follow-up) Fix item-type schema
Bug 35906: Add bookable option on itemtype
Bug 35906: Add unit test
Bug 35906: Remove preference and add override handling
Bug 35906: Schema dump
Bug 35906: (QA follow-up) Fix item-type schema
Bug 35906: (follow-up) fix missing bracket after rebase
Bug 35906: (follow-up) rebased it
Bug 35906: (follow-up) rebased it
Bug 35906: Add bookable option on itemtype
Bug 35906: Add unit test
Bug 35906: Remove preference and add override handling
Bug 35906: Schema dump
Bug 35906: (QA follow-up) Fix item-type schema
Bug 35906: (follow-up) rebased it

Description Thibaud Guillot 2024-01-25 13:11:24 UTC
Hello everyone,

In connection with bug 29002, which introduces the new "booking" feature, I'd like to suggest an improvement mentioned in the ticket: adding this option to an itemtype.
Comment 1 Thibaud Guillot 2024-01-25 13:31:37 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 2024-01-25 14:30:09 UTC
This uses the record level item type to determine bookability even if item-level_itypes is set to specific item. This means a record can be defined as bookable, then have no items that can be booked, so booking fails.

I also think the dropdown to set an item as bookable/not bookable should be disabled when the itemtype is set to 'Bookable'

The warning when an itemtype is not set to bookable and the item is seems strange to me too.

Maybe the option should be read "If checked, all items of this type will be enabled for future bookings, otherwise bookable status can be defined per item"
Comment 3 Thibaud Guillot 2024-01-26 10:49:35 UTC
Thank you nick for your feedback. I've changed the "visual" problems with the dropdown and the messages.


I was just wondering about the impact of itemtype on linked item bookings. I'd certainly change the checkbox for defining whether the itemtype is bookable by a select to have a behavior like : 
- Do not define => NULL, keeping the current operation with the possibility of defining each item as bookable
- Bookable => 1 all linked items are bookable
- Bookable => 0 items not bookable

which would then lead us to use Items->filter_by_bookable with the right filters to obtain bookable items by their direct bookable status or by the bookable status of their itemtype.

It would be necessary to create a foreign-key relationship between these tables, as this is not currently the case, and I wonder why... would there be any obstacle to doing so?
Comment 4 Thibaud Guillot 2024-01-30 14:07:22 UTC Comment hidden (obsolete)
Comment 5 Thibaud Guillot 2024-01-30 14:11:49 UTC
(In reply to Thibaud Guillot from comment #3)
> Thank you nick for your feedback. I've changed the "visual" problems with
> the dropdown and the messages.
> 

Sorry for previous mispelling Nick*

I've tried to come up with a workaround to avoid these table relationship problems.

Using a new system preference, we choose whether the future booking option attribute is activated on the item or on the itemtype. I've taken into account your comments on inconsistent messages and how to view the select on the item if the system preference is set to "itemtype".

I look forward to hearing from you :) Thanks a lot!
Comment 6 Thibaud Guillot 2024-02-05 09:48:16 UTC
Created attachment 161744 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Change new syspref 'item-level_booking' to 'itemtype'
6) Edit an itemtype, there is a new checkbox to add 'bookable' option
7) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
8) You can change syspref to 'item' to see the current behavior

Note: When item-level_booking is set on 'itemtype' you can change
dropdown option to 'No' on item bookable option.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 7 Thibaud Guillot 2024-02-09 10:29:17 UTC
Created attachment 161961 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Change new syspref 'item-level_booking' to 'itemtype'
6) Edit an itemtype, there is a new checkbox to add 'bookable' option
7) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
8) You can change syspref to 'item' to see the current behavior

Note: When item-level_booking is set on 'itemtype' you can change
dropdown option to 'No' on item bookable option.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 8 Martin Renvoize 2024-02-16 16:44:30 UTC
Created attachment 162242 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Change new syspref 'item-level_booking' to 'itemtype'
6) Edit an itemtype, there is a new checkbox to add 'bookable' option
7) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
8) You can change syspref to 'item' to see the current behavior

Note: When item-level_booking is set on 'itemtype' you can change
dropdown option to 'No' on item bookable option.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 9 Martin Renvoize 2024-02-16 16:45:24 UTC
Rebased on the tree..

We need unit test updates for the changes to filter_by_bookable too
Comment 10 Thibaud Guillot 2024-02-23 09:00:51 UTC
Created attachment 162365 [details] [review]
Bug 35906 : Add unit test
Comment 11 Martin Renvoize 2024-02-26 14:53:08 UTC
I'm considering this a little at the moment.

I'm wondering if we aught to be following the patterns already put in place by filter_by_for_hold a little more.

i.e. should this be in the circulation rules table rather than pushed into the item type management page.

Also.. I'm envisaging that we don't actually need the pref to force libaries to pick on or the other setting.  We could effectively build the query as 'item.bookable = 1 OR ( item.booking = undef AND item.itype NOT IN bookable_itypes )'
Comment 12 Martin Renvoize 2024-02-26 17:22:12 UTC
Created attachment 162465 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Change new syspref 'item-level_booking' to 'itemtype'
6) Edit an itemtype, there is a new checkbox to add 'bookable' option
7) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
8) You can change syspref to 'item' to see the current behavior

Note: When item-level_booking is set on 'itemtype' you can change
dropdown option to 'No' on item bookable option.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 13 Martin Renvoize 2024-02-26 17:22:15 UTC
Created attachment 162466 [details] [review]
Bug 35906: Add unit test
Comment 14 Martin Renvoize 2024-02-26 17:22:18 UTC
Created attachment 162467 [details] [review]
Bug 35906: Remove preference and add override handling

This patch updates the bookable nature of items to allow setting at the
itemtype level and then overriding that setting at item level should you
so wish to do so.

We also now properly handle the item_level-itypes preference such that
we look at item or biblioitem level appropriately.
Comment 15 Martin Renvoize 2024-02-26 17:22:21 UTC
Created attachment 162468 [details] [review]
Bug 35906: Schema dump
Comment 16 Martin Renvoize 2024-02-26 17:23:26 UTC
I still have a little more to do here.. Mostly to update the test.

Do you agree with the alternate approach proposed in the followup?  It allows for setting at itemtype level and optionally overriding at item level should one wish to do so.
Comment 17 Martin Renvoize 2024-02-26 19:19:03 UTC
Created attachment 162469 [details] [review]
Bug 35906: Remove preference and add override handling

This patch updates the bookable nature of items to allow setting at the
itemtype level and then overriding that setting at item level should you
so wish to do so.

We also now properly handle the item_level-itypes preference such that
we look at item or biblioitem level appropriately.
Comment 18 Martin Renvoize 2024-02-26 19:19:06 UTC
Created attachment 162470 [details] [review]
Bug 35906: Schema dump
Comment 19 Martin Renvoize 2024-02-26 19:28:33 UTC
Created attachment 162472 [details] [review]
Bug 35906: (QA follow-up) Fix item-type schema
Comment 20 Martin Renvoize 2024-02-29 11:58:34 UTC
I think with my follow-ups we can satisfy all interested parties... just looking for a signoff now.
Comment 21 Martin Renvoize 2024-03-07 12:14:13 UTC
Do you think this approach will satisfy your sponsors Thibaud?  If so, I'd love to see a signoff from them for the feature set as a whole then we can get it through QA :)
Comment 22 Thibaud Guillot 2024-03-07 16:17:07 UTC
Hello Martin,

Thanks for your feedback, so I asked him to test it as soon as possible and SO (I hope so)
Comment 23 Martin Renvoize 2024-03-18 08:42:14 UTC
Created attachment 163291 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Change new syspref 'item-level_booking' to 'itemtype'
6) Edit an itemtype, there is a new checkbox to add 'bookable' option
7) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
8) You can change syspref to 'item' to see the current behavior

Note: When item-level_booking is set on 'itemtype' you can change
dropdown option to 'No' on item bookable option.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 24 Martin Renvoize 2024-03-18 08:42:16 UTC
Created attachment 163292 [details] [review]
Bug 35906: Add unit test
Comment 25 Martin Renvoize 2024-03-18 08:42:19 UTC
Created attachment 163293 [details] [review]
Bug 35906: Remove preference and add override handling

This patch updates the bookable nature of items to allow setting at the
itemtype level and then overriding that setting at item level should you
so wish to do so.

We also now properly handle the item_level-itypes preference such that
we look at item or biblioitem level appropriately.
Comment 26 Martin Renvoize 2024-03-18 08:42:22 UTC
Created attachment 163294 [details] [review]
Bug 35906: Schema dump
Comment 27 Martin Renvoize 2024-03-18 08:42:25 UTC
Created attachment 163295 [details] [review]
Bug 35906: (QA follow-up) Fix item-type schema
Comment 28 Martin Renvoize 2024-03-18 08:51:20 UTC
Just a quick rebase as it stopped applying when most fo the dependencies were pushed.
Comment 29 Thibaud Guillot 2024-03-20 14:54:19 UTC
Created attachment 163547 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Edit an itemtype, there is a new checkbox to add 'bookable' option
6) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
7) You now have a new setting on the item bookable option to 'Follow
   itemtype"

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 30 Thibaud Guillot 2024-03-20 14:54:22 UTC
Created attachment 163548 [details] [review]
Bug 35906: Add unit test
Comment 31 Thibaud Guillot 2024-03-20 14:54:26 UTC
Created attachment 163549 [details] [review]
Bug 35906: Remove preference and add override handling

This patch updates the bookable nature of items to allow setting at the
itemtype level and then overriding that setting at item level should you
so wish to do so.

We also now properly handle the item_level-itypes preference such that
we look at item or biblioitem level appropriately.
Comment 32 Thibaud Guillot 2024-03-20 14:54:29 UTC
Created attachment 163550 [details] [review]
Bug 35906: Schema dump
Comment 33 Thibaud Guillot 2024-03-20 14:54:32 UTC
Created attachment 163551 [details] [review]
Bug 35906: (QA follow-up) Fix item-type schema
Comment 34 Thibaud Guillot 2024-03-20 14:56:26 UTC
Test plan update (syspref removal)
Comment 35 Thibaud Guillot 2024-04-10 08:13:40 UTC
Created attachment 164579 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Edit an itemtype, there is a new checkbox to add 'bookable' option
6) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
7) You now have a new setting on the item bookable option to 'Follow
   itemtype"

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 36 Thibaud Guillot 2024-04-10 08:13:45 UTC
Created attachment 164580 [details] [review]
Bug 35906: Add unit test
Comment 37 Thibaud Guillot 2024-04-10 08:13:50 UTC
Created attachment 164581 [details] [review]
Bug 35906: Remove preference and add override handling

This patch updates the bookable nature of items to allow setting at the
itemtype level and then overriding that setting at item level should you
so wish to do so.

We also now properly handle the item_level-itypes preference such that
we look at item or biblioitem level appropriately.
Comment 38 Thibaud Guillot 2024-04-10 08:13:55 UTC
Created attachment 164582 [details] [review]
Bug 35906: Schema dump
Comment 39 Thibaud Guillot 2024-04-10 08:14:00 UTC
Created attachment 164583 [details] [review]
Bug 35906: (QA follow-up) Fix item-type schema
Comment 40 Thibaud Guillot 2024-04-10 08:16:19 UTC
Created attachment 164584 [details] [review]
Bug 35906: (follow-up) fix missing bracket after rebase
Comment 41 Thibaud Guillot 2024-04-10 08:22:26 UTC
Created attachment 164585 [details] [review]
Bug 35906: (follow-up) rebased it
Comment 42 ByWater Sandboxes 2024-04-23 18:48:32 UTC
Created attachment 165437 [details] [review]
Bug 35906: (follow-up) rebased it

Signed-off-by: Esther <esther@bywatersolutions.com>
Comment 43 Esther Melander 2024-04-23 18:50:24 UTC
Works according to test plan. Just a few notes:

I tested this by changing the Book item type to allow for bookings. However, when I searched for Books, the Bookable dropdown for the item was not updated for existing items. I still had to go in and manually update the option to Follow item type. This is probably correct for existing items. There is not a way to update this via batch item modification for existing items (Bug 35671).

However, adding a new item does default to Follow item type in the Bookable dropdown. I was able to place a booking for the item.

Library limitations appear to work. In a consortium, if one library wants an item type like Books to be bookable and another does not, they will have to create separate item types with library limitations.

If I change the item type like Books to be bookable by item type, book some items, and then go and change those items to not be bookable, the bookings/bookings tab disappears from the bibliographic detail view. In this scenario, the bookings should not disappear even though the item type is no longer bookable. Perhaps a separate bug.

I did not understand step 8, other than I could see the behavior difference in the item detail page with the bookable option by item type enabled versus not.
Comment 44 David Nind 2024-05-16 20:09:52 UTC
The patch no longer applies 8-(...

git bz apply 35906

Bug 35906 - Add bookable option on itemtypes

164579 - Bug 35906: Add bookable option on itemtype
164580 - Bug 35906: Add unit test
164581 - Bug 35906: Remove preference and add override handling
164582 - Bug 35906: Schema dump
164583 - Bug 35906: (QA follow-up) Fix item-type schema
165437 - Bug 35906: (follow-up) rebased it

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 35906: Add bookable option on itemtype
Using index info to reconstruct a base tree...
M	Koha/Items.pm
M	Koha/REST/V1/Biblios.pm
M	installer/data/mysql/kohastructure.sql
M	installer/data/mysql/mandatory/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
M	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging installer/data/mysql/mandatory/sysprefs.sql
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging Koha/REST/V1/Biblios.pm
CONFLICT (content): Merge conflict in Koha/REST/V1/Biblios.pm
Auto-merging Koha/Items.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 35906: Add bookable option on itemtype
Comment 45 Thibaud Guillot 2024-05-17 07:38:05 UTC
Created attachment 166866 [details] [review]
Bug 35906: Add bookable option on itemtype

Actually new "booking" feature can be set on an item but no directly on
an itemtype. This patch adds this possibility.

Test plan:
1) Test this new feature on an item as it were currently working.
2) Apply this patch
3) Run updatedatabase.pl
4) Reload Schema by running update_dbix_class_files.pl
5) Edit an itemtype, there is a new checkbox to add 'bookable' option
6) Test it with item with this itemtype, if there is 1 item at least you
   will see 'Booking' tab.
7) You now have a new setting on the item bookable option to 'Follow
   itemtype"

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 46 Thibaud Guillot 2024-05-17 07:38:08 UTC
Created attachment 166867 [details] [review]
Bug 35906: Add unit test
Comment 47 Thibaud Guillot 2024-05-17 07:38:11 UTC
Created attachment 166868 [details] [review]
Bug 35906: Remove preference and add override handling

This patch updates the bookable nature of items to allow setting at the
itemtype level and then overriding that setting at item level should you
so wish to do so.

We also now properly handle the item_level-itypes preference such that
we look at item or biblioitem level appropriately.
Comment 48 Thibaud Guillot 2024-05-17 07:38:13 UTC
Created attachment 166869 [details] [review]
Bug 35906: Schema dump
Comment 49 Thibaud Guillot 2024-05-17 07:38:16 UTC
Created attachment 166870 [details] [review]
Bug 35906: (QA follow-up) Fix item-type schema
Comment 50 Thibaud Guillot 2024-05-17 07:38:19 UTC
Created attachment 166871 [details] [review]
Bug 35906: (follow-up) rebased it

Signed-off-by: Esther <esther@bywatersolutions.com>
Comment 51 David Nind 2024-05-19 05:08:46 UTC
Patch now applies again, so back to ready for QA.