When cancelling an order, you now can give a reason. The breadcrumbs on that page don't have the bookseller or basket. They could be more complete.
Created attachment 64686 [details] [review] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT
The patch applies correctly : breadcrumbs is complete and both links to vendor and basket work. 'Aquisitions' is now ended by an 's'. Thanks Aleisha !
Patch tested with a sandbox, by severine.queune <severine.queune@bulac.fr>
Created attachment 64697 [details] [review] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT Signed-off-by: severine.queune <severine.queune@bulac.fr>
Created attachment 65176 [details] [review] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT Signed-off-by: severine.queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I would prefer to have new Koha::Acquisition::Basket[s] modules here.
Created attachment 65291 [details] [review] Bug 13208: [FOLLOW-UP] Creating and implementing new Koha::Acquisition::Basket[s] modules Test plan remains the same. Sponsored-by: Catalyst IT
This works well for me, but there is some noise from the QA tool: FAIL Koha/Acquisition/Basket.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK spelling OK valid
Marcel commented on this error from the qa script here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049#c11
Created attachment 65954 [details] [review] [SIGNED-OFF] Bug 13208: [FOLLOW-UP] Creating and implementing new Koha::Acquisition::Basket[s] modules Test plan remains the same. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to Owen Leonard from comment #8) > This works well for me, but there is some noise from the QA tool: > > FAIL Koha/Acquisition/Basket.pm > FAIL pod coverage > POD coverage was greater before, try perl > -MPod::Coverage=PackageName -e666 (In reply to Katrin Fischer from comment #9) > Marcel commented on this error from the qa script here: > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049#c11 It was indeed a false positive on bug 19049 (because it was a module in the t directory), but here it's a real failure. There is missing POD for methods in Koha::Acquisition::Basket.
Created attachment 66161 [details] [review] Bug 13208: [FOLLOW-UP] Perl documentation for modules
Personally I feel that the 'for <vendor>' is unecessary as the information is already visible in the breadcrumbs, but at the moment, this seems a common pattern in the module (Osiander being my vendor): Home › Acquisitions › Osiander › Basket Test bug 15347 (1) for Osiander
Created attachment 66499 [details] [review] [SIGNED OFF] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT Signed-off-by: severine.queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66500 [details] [review] [SIGNED OFF] Bug 13208: [FOLLOW-UP] Creating and implementing new Koha::Acquisition::Basket[s] modules Test plan remains the same. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66501 [details] [review] [SIGNED OFF] Bug 13208: [FOLLOW-UP] Perl documentation for modules Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Found another problem with the breadcrumbs in acq while testing this: bug 19180
Created attachment 66738 [details] [review] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT Signed-off-by: severine.queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66739 [details] [review] Bug 13208: [FOLLOW-UP] Creating and implementing new Koha::Acquisition::Basket[s] modules Test plan remains the same. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66740 [details] [review] Bug 13208: [FOLLOW-UP] Perl documentation for modules Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66741 [details] [review] Bug 13208: (QA follow-up) Remove blank breadcrumbs on successful deletion After order is deleted we don't have a vendor or basket so we get blank breadcrumbs, this removes them
Please provide tests for new modules (based on t/db_dependent/Koha/Cities.t).
Created attachment 66831 [details] [review] Bug 13208: [FOLLOW-UP] Tests for Koha::Acquisition::Baskets To test: prove -v t/db_dependent/Koha/Acquisition/Baskets.t
All tests successful. Files=1, Tests=4, 2 wallclock secs ( 0.02 usr 0.00 sys + 0.64 cusr 0.02 csys = 0.68 CPU) Result: PASS Signed off
Hi David, please add your sign-off line to the patches before you switch the status. It's quite easy with kohadevbox, but we can help you with whatever setup you have. See here: https://wiki.koha-community.org/wiki/Git_bz_configuration#Signing_off
Created attachment 67167 [details] [review] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT Signed-off-by: severine.queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 67168 [details] [review] Bug 13208: [FOLLOW-UP] Creating and implementing new Koha::Acquisition::Basket[s] modules Test plan remains the same. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 67169 [details] [review] Bug 13208: [FOLLOW-UP] Perl documentation for modules Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 67170 [details] [review] Bug 13208: (QA follow-up) Remove blank breadcrumbs on successful deletion After order is deleted we don't have a vendor or basket so we get blank breadcrumbs, this removes them Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 67171 [details] [review] Bug 13208: [FOLLOW-UP] Tests for Koha::Acquisition::Baskets To test: prove -v t/db_dependent/Koha/Acquisition/Baskets.t Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 67768 [details] [review] Bug 13208: Improving breadcrumbs for when cancelling an order To test: 1) Go to Acquisitions -> Find a vendor -> View a basket with orders in it (or make a new basket and add an order) 2) Click Cancel order 3) Notice incomplete breadcrumbs, and 'Acquisition' typo 4) Apply patch and refresh page 5) Breadcrumbs should be fixed. Confirm links to vendor and basket work as expected Sponsored-by: Catalyst IT Signed-off-by: severine.queune <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 67769 [details] [review] Bug 13208: [FOLLOW-UP] Creating and implementing new Koha::Acquisition::Basket[s] modules Test plan remains the same. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 67770 [details] [review] Bug 13208: [FOLLOW-UP] Perl documentation for modules Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 67771 [details] [review] Bug 13208: (QA follow-up) Remove blank breadcrumbs on successful deletion After order is deleted we don't have a vendor or basket so we get blank breadcrumbs, this removes them Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 67772 [details] [review] Bug 13208: [FOLLOW-UP] Tests for Koha::Acquisition::Baskets To test: prove -v t/db_dependent/Koha/Acquisition/Baskets.t Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 17.11, thanks to everybody involved!
Ok when cancelling from a basket page. But I found breadcrumbs are also broken when cancelling from a receipt summary. For example : /cgi-bin/koha/acqui/parcel.pl?invoiceid=1&op=cancelreceipt&ordernumber=1 The URL does not provide "basketno". Do we create a new bug ?
Yes, always a new bug after things are pushed (no confusion in release notes etc.)
(In reply to Katrin Fischer from comment #38) > Yes, always a new bug after things are pushed (no confusion in release notes > etc.) If it is not backported yet and the fix is trivial I prefer a follow-up on the same bug report and push it directly without the SO+QA steps.
Sorry, was a little too fast, not reading closely enough.