Description
Thibaud Guillot
2024-01-25 13:11:24 UTC
Created attachment 161397 [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, go to edit item view ("Booking" tab is already in biblio-view-menu even if there is no booking on item, due to bookable option on itemtype) 7) Next to the bookable item option you can see a warn with a message if both options (item/itemtype) are not the same. 8) Test it Sponsored by: BibLibre 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" 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? Created attachment 161638 [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: BibLibre (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! 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) 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) 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) Rebased on the tree.. We need unit test updates for the changes to filter_by_bookable too Created attachment 162365 [details] [review] Bug 35906 : Add unit test 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 )' 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) Created attachment 162466 [details] [review] Bug 35906: Add unit test 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. Created attachment 162468 [details] [review] Bug 35906: Schema dump 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. 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. Created attachment 162470 [details] [review] Bug 35906: Schema dump Created attachment 162472 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema I think with my follow-ups we can satisfy all interested parties... just looking for a signoff now. 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 :) Hello Martin, Thanks for your feedback, so I asked him to test it as soon as possible and SO (I hope so) 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) Created attachment 163292 [details] [review] Bug 35906: Add unit test 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. Created attachment 163294 [details] [review] Bug 35906: Schema dump Created attachment 163295 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Just a quick rebase as it stopped applying when most fo the dependencies were pushed. 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) Created attachment 163548 [details] [review] Bug 35906: Add unit test 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. Created attachment 163550 [details] [review] Bug 35906: Schema dump Created attachment 163551 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Test plan update (syspref removal) 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) Created attachment 164580 [details] [review] Bug 35906: Add unit test 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. Created attachment 164582 [details] [review] Bug 35906: Schema dump Created attachment 164583 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Created attachment 164584 [details] [review] Bug 35906: (follow-up) fix missing bracket after rebase Created attachment 164585 [details] [review] Bug 35906: (follow-up) rebased it Created attachment 165437 [details] [review] Bug 35906: (follow-up) rebased it Signed-off-by: Esther <esther@bywatersolutions.com> 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. 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 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) Created attachment 166867 [details] [review] Bug 35906: Add unit test 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. Created attachment 166869 [details] [review] Bug 35906: Schema dump Created attachment 166870 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Created attachment 166871 [details] [review] Bug 35906: (follow-up) rebased it Signed-off-by: Esther <esther@bywatersolutions.com> Patch now applies again, so back to ready for QA. Patch no longer applies. Created attachment 168120 [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) Created attachment 168121 [details] [review] Bug 35906: Schema dump Created attachment 168122 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Created attachment 168160 [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) Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 168161 [details] [review] Bug 35906: Add unit test Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 168162 [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. Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 168163 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 168164 [details] [review] Bug 35906: Schema dump Signed-off-by: Esther <esther@bywatersolutions.com> I restored the signoff lines in my rebase.. you also removed a function in your rebase in error I believe. (In reply to Martin Renvoize from comment #61) > I restored the signoff lines in my rebase.. you also removed a function in > your rebase in error I believe. Oh sorry it was my mistake, I had squashed some commits about adding then removing the syspref and I must have forgotten some lines... thanks |