Hello, Linked to the booking feeture you can now transform a booking into a checkout directly from the booking list view.
Created attachment 166213 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking (due date is calculated from today + booking length period - new circulation rule - see bug 36271) 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Created attachment 167191 [details] [review] Bug 36789: Fix format when hours or minutes contain only one digit
Created attachment 167242 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking (due date is calculated from today + booking length period - new circulation rule - see bug 36271) 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Created attachment 167246 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking (due date is calculated from today + booking length period - new circulation rule - see bug 36271) 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Created attachment 167268 [details] [review] Bug 36789: Pass param to prevent notification on checkout transformation
Created attachment 167307 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Created attachment 167309 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Created attachment 167311 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Something in one of the dependencies cases bug 36915 to not apply (although 36915 applies OK by itself). Will re-test once dependencies fixed. git bz apply 36789 Bug 36789 Depends on bug 36271 (Needs Signoff) Follow? [(y)es, (n)o] y Bug 36271 Depends on bug 35906 (Signed Off) Follow? [(y)es, (n)o] y Bug 35906 - Add bookable option on itemtypes 166866 - Bug 35906: Add bookable option on itemtype 166867 - Bug 35906: Add unit test 166868 - Bug 35906: Remove preference and add override handling 166869 - Bug 35906: Schema dump 166870 - Bug 35906: (QA follow-up) Fix item-type schema 166871 - Bug 35906: (follow-up) rebased it 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 Applying: Bug 35906: Schema dump Applying: Bug 35906: (QA follow-up) Fix item-type schema Applying: Bug 35906: (follow-up) rebased it Bug 36271 - Bookings should respect circulation rules for max loan periods 166962 - Bug 36271: Add circulation rules for booking Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36271: Add circulation rules for booking Bug 36789 Depends on bug 36915 (Failed QA) Follow? [(y)es, (n)o] y Bug 36915 - Send email notification when a booking is cancelled 167013 - Bug 36915: Send email notification when a booking is cancelled Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36915: Send email notification when a booking is cancelled Using index info to reconstruct a base tree... M Koha/Booking.pm Falling back to patching base and 3-way merge... Auto-merging Koha/Booking.pm CONFLICT (content): Merge conflict in Koha/Booking.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 36915: Send email notification when a booking is cancelled
Created attachment 167368 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Created attachment 167369 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
(In reply to David Nind from comment #9) > Something in one of the dependencies cases bug 36915 to not apply (although > 36915 applies OK by itself). > > Will re-test once dependencies fixed. > > git bz apply 36789 > > Bug 36789 Depends on bug 36271 (Needs Signoff) > Follow? [(y)es, (n)o] y > > Bug 36271 Depends on bug 35906 (Signed Off) > Follow? [(y)es, (n)o] y > > Bug 35906 - Add bookable option on itemtypes > > 166866 - Bug 35906: Add bookable option on itemtype > 166867 - Bug 35906: Add unit test > 166868 - Bug 35906: Remove preference and add override handling > 166869 - Bug 35906: Schema dump > 166870 - Bug 35906: (QA follow-up) Fix item-type schema > 166871 - Bug 35906: (follow-up) rebased it > > 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 > Applying: Bug 35906: Schema dump > Applying: Bug 35906: (QA follow-up) Fix item-type schema > Applying: Bug 35906: (follow-up) rebased it > > Bug 36271 - Bookings should respect circulation rules for max loan periods > > 166962 - Bug 36271: Add circulation rules for booking > > Apply? [(y)es, (n)o, (i)nteractive] y > Applying: Bug 36271: Add circulation rules for booking > > Bug 36789 Depends on bug 36915 (Failed QA) > Follow? [(y)es, (n)o] y > > Bug 36915 - Send email notification when a booking is cancelled > > 167013 - Bug 36915: Send email notification when a booking is cancelled > > Apply? [(y)es, (n)o, (i)nteractive] y > Applying: Bug 36915: Send email notification when a booking is cancelled > Using index info to reconstruct a base tree... > M Koha/Booking.pm > Falling back to patching base and 3-way merge... > Auto-merging Koha/Booking.pm > CONFLICT (content): Merge conflict in Koha/Booking.pm > error: Failed to merge in the changes. > Patch failed at 0001 Bug 36915: Send email notification when a booking is > cancelled Hi David, Thanks for your feedback.. I fixed it (after changes there is only one dependency :)
I get a 403 error after clicking on "Transform to checkout": An error has occurred! Error 403 Programming error - No CSRF token passed for POST http://127.0.0.1:8081/intranet/circ/circulation.pl?borrowernumber=49 (referer: http://127.0.0.1:8081/cgi-bin/koha/bookings/list.pl?biblionumber=126&searchid=scs_1717528289710)
Created attachment 167602 [details] [review] Bug 36789: (follow-up) Fix CSRF Token missing
(In reply to David Nind from comment #13) > I get a 403 error after clicking on "Transform to checkout": > > An error has occurred! > Error 403 > Programming error - No CSRF token passed for POST > http://127.0.0.1:8081/intranet/circ/circulation.pl?borrowernumber=49 > (referer: > http://127.0.0.1:8081/cgi-bin/koha/bookings/list. > pl?biblionumber=126&searchid=scs_1717528289710) Hello David, Thanks for your feedback, It seems fixed now
Created attachment 167906 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS) Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167907 [details] [review] Bug 36789: (follow-up) Fix CSRF Token missing Signed-off-by: David Nind <david@davidnind.com>
Thanks Thibaud! Can confirm that the patches now apply and works as expected. Have added my sign-off as well.
This is the sort of workflow I was alluding to when I said in bug 36915 that perhaps we don't want to tie the cancellation notice to the delete method but rather introduce a cancel method instead. I'm also not really sure about removing bookings at fulfilment time.. I can see people wanting to report on those fulfilled and unfulfilled bookings down the line. I was considering instead, adding a status field to bookings to flag that a booking is in various states... waiting, fulfilled, overdue for pickup, cancelled.. etc. Then, adding to cleanup_database to ensure we don't have a runaway table of data. Thoughts?
The patches no longer apply 8-(... git bz apply 36789 Bug 36789 Depends on bug 36915 (Needs Signoff) Follow? [(y)es, (n)o] y Bug 36915 - Send email notification when a booking is cancelled 168149 - Bug 36915: Send email notification when a booking is cancelled 168150 - Bug 36915: (follow-up) Pass object to GetPreparedLetter 168155 - Bug 36915: (follow-up) Basic sample notice Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36915: Send email notification when a booking is cancelled Applying: Bug 36915: (follow-up) Pass object to GetPreparedLetter Applying: Bug 36915: (follow-up) Basic sample notice Bug 36789 - Transform a booking into checkout 167906 - Bug 36789: Transform a booking into checkout 167907 - Bug 36789: (follow-up) Fix CSRF Token missing Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36789: Transform a booking into checkout Using index info to reconstruct a base tree... M C4/Circulation.pm M Koha/Booking.pm M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging Koha/Booking.pm CONFLICT (content): Merge conflict in Koha/Booking.pm Auto-merging C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 36789: Transform a booking into checkout
Created attachment 170198 [details] [review] Bug 36789: (follow-up) rebase after using object directly in letter
(In reply to Martin Renvoize from comment #19) > This is the sort of workflow I was alluding to when I said in bug 36915 that > perhaps we don't want to tie the cancellation notice to the delete method > but rather introduce a cancel method instead. > > I'm also not really sure about removing bookings at fulfilment time.. I can > see people wanting to report on those fulfilled and unfulfilled bookings > down the line. > > I was considering instead, adding a status field to bookings to flag that a > booking is in various states... waiting, fulfilled, overdue for pickup, > cancelled.. etc. Then, adding to cleanup_database to ensure we don't have a > runaway table of data. > > Thoughts? Hello Yes, it might be useful to create a cancel method to mark them with a "status". Maybe we can also move bookings to "old_bookings" like holds/old_holds I mean.... For now I'll just quickly rebase the patch if anyone wants to test it and make up their own mind :)
Concerning the status from comment #22: There's bug 37601, where we want to do exactly that. Then we want to move the logic within delete into a cancel method in a follow-up to 37601 and 39615. This will require rebases on the children of this bug again, so let's get the root through first would be my recommendation.
Sorry meant bug 36915 in comment #23.
The patch no longer applies 8-(... git bz apply 36789 Bug 36789 - Transform a booking into checkout 167906 - Bug 36789: Transform a booking into checkout 167907 - Bug 36789: (follow-up) Fix CSRF Token missing 170198 - Bug 36789: (follow-up) rebase after using object directly in letter Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36789: Transform a booking into checkout Using index info to reconstruct a base tree... M C4/Circulation.pm M Koha/Booking.pm M circ/circulation.pl M koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt Auto-merging circ/circulation.pl Auto-merging Koha/Booking.pm CONFLICT (content): Merge conflict in Koha/Booking.pm Auto-merging C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 36789: Transform a booking into checkout
Created attachment 171553 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS) Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171554 [details] [review] Bug 36789: (follow-up) Fix CSRF Token missing Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171555 [details] [review] Bug 36789: (follow-up) rebase after using object directly in letter
Created attachment 172058 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 172059 [details] [review] Bug 36789: (follow-up) Fix CSRF Token missing Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 172060 [details] [review] Bug 36789: (follow-up) rebase after using object directly in letter Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Please rebase and then I'll qa ASAP.
Created attachment 176452 [details] [review] Bug 36789: Transform a booking into checkout Directly from the bookings list, you can perform a checkout with booked item. Test plan: 1) Got an item booked :) 2) Click on transform to checkout 3) Normally you will be redirected to circulation.pl checkout view with form infos from booking. 4) If issue is confirmed, the booking will be deleted. Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 176453 [details] [review] Bug 36789: (follow-up) Fix CSRF Token missing Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Hello :) Rebased and removal delete overload method due to implementation of _send_notices method.
FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt FAIL filters missing_filter at line 1074 ( var booking_id = [% booking_id.json %];) In order to prevent XSS vulnerabilities all the variables displayed in the templates must be escaped using an appropriate filter. See Bug 13618 for more information. Not all of them are concerned by the vulnerability but to be on the safe side we are going to escape them all. Usually you will want to use the filters provided by TT: html, uri, url, etc. And sometimes we will want to display the variable as it is, i.e. you will want to make the browser interpret the content (HTML or JavaScript), in that case you will explicitly use the raw filter, that does nothing. https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML9:_Filter_all_template_variables There's also 2 tidy warnings, one line each in C4/Circulation.pm & circ/circulation.pl It worked great, and once you add the filter I'm happy to pqa.
Created attachment 176489 [details] [review] Bug 36789: (follow-up) add JSON filter
Thanks Lisette for your time, I fixed this, sorry for missing ^^
1. +++ b/bookings/list.pl +use Koha::Biblios; +use Koha::Bookings; +use Koha::Patrons; +use Koha::Items; +use Koha::CirculationRules; Hum? -my $biblio = Koha::Biblios->find($biblionumber); +my $biblio = Koha::Biblios->find( { biblionumber => $biblionumber } ); Hum?? 2. Change in pm not covered by tests 3. 393 result += `<form name="checkout-transform" method="post" action="/cgi-bin/koha/circ/circulation.pl?borrowernumber=${row.patron_id}"><input type="hidden" name="op" value="cud-checkout"/><input type="hidden" name="borrowernumber" value="${row.patron_id}"/><input type="hidden" name="barcode" value="${row.item.external_id}"/><input type="hidden" name="duedatespec" value="${row.end_date}"/><input type="hidden" name="booking_id" value="${row.booking_id}"/><input type="hidden" name="csrf_token" value="${csrf_token}"/><button id="checkout_action" class="btn btn-default btn-xs" type="submit">${_("Transform to checkout")}</button></form>`; This line is 660 chars long, it's not readable. Additionally, all variables from the REST API must be escaped (using escape_str) I also think you will gain in readability using .format() and bind parameters. 4. JS error on circulation.pl Uncaught SyntaxError: expected expression, got ';'