Directly from LMSCloud's requirements: > As a Koha library, we want to be able to use configurable additional fields for bookings in order to enable patrons to provide additional information with bookings like the persons who need to be informed when the material is available or a contact person of the institution. > The additional fields will be editable and stored with the booking and should be also available in booking notifications. I think this is interesting because these use-cases seems like a perfect fit for the bookings feature. For example when working with schools or something of the like.
Created attachment 181034 [details] [review] Bug 37829: Module, Controller, DBIx::Class changes
Created attachment 181035 [details] [review] Bug 37829: API specification changes
Created attachment 181036 [details] [review] Bug 37829: Add bookings as config option in additional-fields.tt
Created attachment 181037 [details] [review] Bug 37829: Expose additional fields API client, add needed methods
Created attachment 181038 [details] [review] Bug 37829: Add additional fields module
Created attachment 181039 [details] [review] Bug 37829: Add entry point for additional fields in place_booking.inc
Created attachment 181040 [details] [review] Bug 37829: Add additional fields support to modal, tables To test: 1) Setup: a) Apply the db seed provided: koha-mysql < test_bug_37829.sql --- or --- b) Setup authorised values and additional fields manually: i) Navigate to Administration -> Authorized Values and click '+ New Category'. ii) Define a new authorised value category (e.g., prefix with BOOKINGS_ for easy identification). iii) Click '+ Add a new authorized value'. iv) Add a value, filling in at least 'Authorized Value' and 'Description'. v) Save the value. vi) Navigate to Administration -> Additional Fields. vii) Click 'Bookings (bookings)'. viii)Click '+ New field'. ix) Add a plain text field (fill 'Name' and save). x) Add a repeatable text field. xi) Add a field tied to your previously created authorized value category. xii) Add another field tied to any authorised value (e.g., YES_NO) and make it repeatable. 2) Item Configuration: Make an item bookable. Either: a) Navigate to Administration -> Item types, select an item type (e.g., BK), and check 'bookable'. b) Navigate to the 'Items' view for a specific item (catalogue/moredetail.pl) and set Priority -> Bookable -> 'Yes'. 3) Create Booking: Click the 'Place booking' button on the item's record. The configured additional fields should load dynamically in the modal. 4) Fill and Save Booking: Fill out the default booking fields and then the additional fields. Use the 'Remove' and 'New' buttons for repeatable fields. Save the booking. 5) Verify Display in Tables: Confirm that the additional fields for the booking are displayed correctly in the following locations: a) The bookings table within the bibliographic record's detail view. b) The 'Check out' view (circ/circulation.pl -> Bookings tab). c) The 'Patron's details' view (members/moremember.pl -> Bookings tab). d) The 'Bookings to collect' view (circ/pendingbookings.pl) (adjust filters if necessary). 6) Verify Edit/Move Functionality: Return to the bookings view of the bibliographic record. a) Verify the additional fields are correctly pre-filled and saved when you move the booking using the timeline component. b) Verify the additional fields are correctly pre-filled and saved when you edit the booking using the edit button in the bookings table. 7) Repeat Testing: Repeat steps 4 through 6.b with additional bookings to ensure consistency. 8) Sign-off and/or give your feedback. Here's some things I'd would still like to improve/try: - Incorporate the neighboring additional-fields-entry.js to have a single library for additional fields - Restructure the buttons to be consistent with other usage of additional fields in Koha (noticed this way to late) - Test this with the freshly added support for additional fields on branches (libraries) - Make the additional field values searchable in the table - Clean up and DRY some of the JSDoc types (but this could easily become its own tree of bugs if done from the source :D) - Use mockData.js for the mocha tests instead of hardcoding them I actually need this for a customer and the current state will be alright for that, but now it's good enough to work and improve it together.
Created attachment 181041 [details] [review] Bug 37829: Add unit tests (mocha) and testing dependencies This also acts as a proposal on how to easily test our assets without having to turn them into modules. Not that I am against modules in any way, but rewire also allows us to add tests for older JS in Koha :)
Created attachment 181042 [details] [review] Bug 37829: Isolated yarn.lock changes
Created attachment 181043 [details] [review] Bug 37829: [DO NOT PUSH] Add db seed for SO + QA This file can just be run against the db to: - Make books bookable in general - Add some authorised values targeted at testing the different types - Add some additional fields, some of which using the aforementioned authorised values
Addendum to the test plan: Setup: 1) Run 'yarn api:bundle' to rebuild the api spec with the new changes. 2) Reload plack to load the new spec: 'koha-plack --reload kohadev'.
Just another note, with '+ // @ts-check' at the top of the file, you can also observe that there are currently no type errors in additional-fields.js except for complaining about __ not being defined (which can be ignored).
This is lovely. 1) "Bookings" should be alphabetically placed in cgi-bin/koha/admin/additional-fields.pl 2) Is there absolutely no way we use/extend the current established pattern of additional-fields-entry.inc and additional-fields-entry.js? This patchset is now introducing a 3rd implementation of additional fields into Koha (vanilla tt, VueJS, now this JS). Some things work differently here to the vanilla Koha implementation e.g. repeatable fields start hidden here where in vanilla Koha it always start with showing at least one. Repeatable AV fields in vanilla Koha are shown as checkboxes, here they're shown as multiple dropdowns.
Hm, please remember alphabetic has no meaning with translations...
> 1) "Bookings" should be alphabetically placed in cgi-bin/koha/admin/additional-fields.pl Yes, will fix that shortly. > 2) Is there absolutely no way we use/extend the current established pattern of additional-fields-entry.inc and additional-fields-entry.js? I actually wanted it to be the other way around. Since additional-fields-entry.js is so small in scope, I just wanted to keep it as an interface to the new additional-fields.js, and cover that functionality as well. > This patchset is now introducing a 3rd implementation of additional fields into Koha (vanilla tt, VueJS, now this JS). That's true, for the time being I'd like to move to vanilla.tt + one consolidated js library + Vue. And in the future move to a singular implementation based on optional tt + vue islands. > Some things work differently here to the vanilla Koha implementation e.g. repeatable fields start hidden here where in vanilla Koha it always start with showing at least one. I had that initially, will reintroduce that behaviour to be consistent. > Repeatable AV fields in vanilla Koha are shown as checkboxes, here they're shown as multiple dropdowns. Do you have a prominent spot I can look at that you would describe as some sort of reference implementation? Thanks for taking the time to look at this :)
> That's true, for the time being I'd like to move to vanilla.tt + one consolidated js library + Vue. And in the future move to a singular implementation based on optional tt + vue islands. In addition to keep using the native Vue component in Vue apps of course.
(In reply to Paul Derscheid from comment #15) > Do you have a prominent spot I can look at that you would describe as some > sort of reference implementation? Yeah I just discovered bug 39664 while testing this, the steps there provide you with an example of the current implementation. (In reply to Paul Derscheid from comment #15) > I actually wanted it to be the other way around. Since > additional-fields-entry.js is so small in scope, I just wanted to keep it as > an interface to the new additional-fields.js, and cover that functionality > as well. That works just the same imo, this is why I said use/extend the current implementation. As long as we don't add a new standalone implementation of additional fields.
(In reply to Katrin Fischer from comment #14) > Hm, please remember alphabetic has no meaning with translations... Fair point.
Yes, that's also not my intention as you might be able to tell :D I will make a round of adjustments and try consolidating the diverging approaches. Thanks for the pointers.