|
Description
Thibaud Guillot (thibaud_g)
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 Created attachment 170187 [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 170188 [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 170189 [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 170190 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 170191 [details] [review] Bug 35906: Schema dump Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 170582 [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 170583 [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 170584 [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 170585 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 170586 [details] [review] Bug 35906: Schema dump Signed-off-by: Esther <esther@bywatersolutions.com> Corrected rebase.. now there's just a small number of QA script fails we need to look into rather than outright breakage :) Great! Privated the comment so it doesn't fill up the entire page. Created attachment 172127 [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 172128 [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 172129 [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 172130 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 172131 [details] [review] Bug 35906: Schema dump Signed-off-by: Esther <esther@bywatersolutions.com> Created attachment 172132 [details] [review] Bug 35906: (QA follow-up) Add bookable option for API Looks like this one has been signed off. Can it be updated to a signed off status? We probably aught to add a unit test for effective_bookable here Created attachment 172941 [details] [review] Bug 35906: (QA follow-up) Ensure we return effective_bookable The API wasn't including the effective_bookable value in responses Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> We should support the ?bookable filter on the plain /api/v1/items endpoint too. Also.. I'm tempted to rename it 'effective_bookable' to more clearly distinguish been effective bookable and localised item bookable fields. I'd love to see this also taken account of within q= as well as the local param option.. that could be a future follow-up though. I agree, that would be clearer. Created attachment 172945 [details] [review] Bug 35906: (QA follow-up) Unit tests for effective_bookable Created attachment 172950 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172951 [details] [review] Bug 35906: Add unit test Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172952 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172953 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172954 [details] [review] Bug 35906: Schema dump Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172955 [details] [review] Bug 35906: (QA follow-up) Add bookable option for API Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172956 [details] [review] Bug 35906: (QA follow-up) Ensure we return effective_bookable The API wasn't including the effective_bookable value in responses Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172957 [details] [review] Bug 35906: (QA follow-up) Tidy admin/itemtypes.pl, Items.t Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172958 [details] [review] Bug 35906: (QA follow-up) Refactor (slightly) to reduce complexity in Koha/Item(s).pm, catalogue/updateitem Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172959 [details] [review] Bug 35906: (QA follow-up) Tidy block in TestBuilder.pm Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172960 [details] [review] Bug 35906: (QA follow-up) Correct type on added bookable column Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 172961 [details] [review] Bug 35906: (QA follow-up) Unit tests for effective_bookable Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Please have another look, but I think this is PQA! I am sorry, this currently no longer applies. Please rebase ASAP! Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 35906: Add bookable option on itemtype Applying: Bug 35906: Add unit test Applying: Bug 35906: Remove preference and add override handling error: sha1 information is lacking or useless (Koha/Items.pm). error: could not build fake ancestor Patch failed at 0001 Bug 35906: Remove preference and add override handling hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-35906-Remove-preference-and-add-override-handl-7ip612hr.patch Created attachment 173324 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173325 [details] [review] Bug 35906: Add unit test Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173326 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173327 [details] [review] Bug 35906: (QA follow-up) Fix item-type schema Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173328 [details] [review] Bug 35906: Schema dump Signed-off-by: Esther <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173329 [details] [review] Bug 35906: (QA follow-up) Add bookable option for API Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173330 [details] [review] Bug 35906: (QA follow-up) Ensure we return effective_bookable The API wasn't including the effective_bookable value in responses Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173331 [details] [review] Bug 35906: (QA follow-up) Tidy admin/itemtypes.pl, Items.t Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173332 [details] [review] Bug 35906: (QA follow-up) Refactor (slightly) to reduce complexity in Koha/Item(s).pm, catalogue/updateitem Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173333 [details] [review] Bug 35906: (QA follow-up) Tidy block in TestBuilder.pm Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173334 [details] [review] Bug 35906: (QA follow-up) Correct type on added bookable column Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 173335 [details] [review] Bug 35906: (QA follow-up) Unit tests for effective_bookable Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> 1) Documentation Test plan is really outdated: 5) Change new syspref 'item-level_booking' to 'itemtype' 6) Edit an itemtype, there is a new checkbox to add 'bookable' option There is no such system preference (which is good). Please provide release notes and some documentation step by step for the documentation team. 2) Database update Whenever adding a new column, you need to use AFTER or BEFORE to make sure the sequence matches in updated and new installations. (Fixed in a follow-up) Pushed for 24.11! Well done everyone, thank you! It turns out that the "IF NOT EXISTS" pattern for ALTER TABLE doesn't play well with MySQL8 - can you please replace with our usual check? ALTER TABLE itemtypes ADD IF NOT EXISTS bookable INT(1) DEFAULT 0 AFTER automatic_checkin Created attachment 173893 [details] [review] Bug 35906: (QA follow-up) Remove use of IF NOT EXISTS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Thibaud, can you provide a follow-up for the bug 35906 please? (In reply to Katrin Fischer from comment #118) > Thibaud, can you provide a follow-up for the bug 35906 please? Sorry, wrong bug number. It's Bug 38434 - (Bug 35906 follow-up) dbrev different from kohastructure.sql Enhancement, no 24.05.x backport |