This bug affects all stable releases and seems to have been there for a long time. To recreate: - Launch a search at the OPAC - Add 2 elements to the cart - Remove one => Cart is empty Note that it does not exist on the staff interface
Created attachment 79852 [details] [review] Bug 21479: Fix remove from cart - OPAC This bug affects all stable releases and seems to have been there for a long time. To recreate: - Launch a search at the OPAC - Add 2 elements to the cart - Remove one => Cart is empty Note that it does not exist on the staff interface The code in opac-bottom is only for the detail page, but the evenement binding was done on the result search page as they have the same selector (.cartRemove) Using a more specific selector (already used for "add to shelf") fixes the problem. Test plan: Add and remove elements to/from your cart On both search and detail view
Created attachment 79857 [details] [review] Bug 21479: Fix remove from cart - OPAC This bug affects all stable releases and seems to have been there for a long time. To recreate: - Launch a search at the OPAC - Add 2 elements to the cart - Remove one => Cart is empty Note that it does not exist on the staff interface The code in opac-bottom is only for the detail page, where biblionumber is defined, but the event binding was done on the result search page as they have the same selector (.cartRemove) On the results page, the biblionumber is not defined and triggers the deletion of all the items as a result. Using a more specific selector (this solution is already used for "add to shelf") fixes the problem Test plan: Add and remove elements to/from your cart On both search and detail view Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
David reported on IRC he did not recreate the issue, if the problem does not exist for everybody we will need to understand why. Please do not push without knowing all the details.
Created attachment 79893 [details] [review] Bug 21479: (alternate) Removing from cart removes 2 items This patch changes the way biblionumbers are passed to the cart script for adding and removing single items. The title's biblionumber is now stored in a data-attribute on the links for adding and removing. This should be a more robust, unified way to handle these single-item operations. To test, apply the patch and enable the opacbookbag system preference. Test adding and removing items from the cart from various pages using various methods: - By clicking an "Add to your cart" / "Remove" link in search results - By clicking an "Add to your cart" / "Remove" link on a detail page - By checking boxes in search results and adding via the dropdown Each operation should work correctly.
Owen, with your patch: From the result page: Add Go to the detail page, "In your cart (remove)" The "In your cart" link does nothing => KO
Created attachment 79903 [details] [review] Bug 21479: (alternate) Removing from cart removes 2 items This patch changes the way biblionumbers are passed to the cart script for adding and removing single items. The title's biblionumber is now stored in a data-attribute on the links for adding and removing. This should be a more robust, unified way to handle these single-item operations. To test, apply the patch and enable the opacbookbag system preference. Test adding and removing items from the cart from various pages using various methods: - By clicking an "Add to your cart" / "Remove" link in search results - By clicking an "Add to your cart" / "Remove" link on a detail page - By checking boxes in search results and adding via the dropdown Each operation should work correctly.
Able to see the problem now - was using cart itself to remove items, rather than adding and removing from the search result list. Getting this message when trying to apply patch (git bz apply 21479): Applying: Bug 21479: Fix remove from cart - OPAC Applying: Bug 21479: (alternate) Removing from cart removes 2 items Using index info to reconstruct a base tree... M koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc error: Failed to merge in the changes. Patch failed at 0001 Bug 21479: (alternate) Removing from cart removes 2 items The copy of the patch that failed is found in: .git/rebase-apply/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-21479-alternate-Removing-from-cart-removes-2-i-Fk_Tok.patch
Created attachment 79950 [details] [review] Bug 21479: (alternate) Removing from cart removes 2 items This patch changes the way biblionumbers are passed to the cart script for adding and removing single items. The title's biblionumber is now stored in a data-attribute on the links for adding and removing. This should be a more robust, unified way to handle these single-item operations. To test, apply the patch and enable the opacbookbag system preference. Test adding and removing items from the cart from various pages using various methods: - By clicking an "Add to your cart" / "Remove" link in search results - By clicking an "Add to your cart" / "Remove" link on a detail page - By checking boxes in search results and adding via the dropdown Each operation should work correctly. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 80219 [details] [review] Bug 21479: (alternate) Removing from cart removes 2 items This patch changes the way biblionumbers are passed to the cart script for adding and removing single items. The title's biblionumber is now stored in a data-attribute on the links for adding and removing. This should be a more robust, unified way to handle these single-item operations. To test, apply the patch and enable the opacbookbag system preference. Test adding and removing items from the cart from various pages using various methods: - By clicking an "Add to your cart" / "Remove" link in search results - By clicking an "Add to your cart" / "Remove" link on a detail page - By checking boxes in search results and adding via the dropdown Each operation should work correctly. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I like Owens alternative.. as he says, it just feels more robust. QA Comment though.. could we possibly add a selenium test for this perhaps?
Created attachment 80291 [details] [review] Bug 21479: Fix remove from cart links at the OPAC (should remove only 1) This patch changes the way biblionumbers are passed to the cart script for adding and removing single items. The title's biblionumber is now stored in a data-attribute on the links for adding and removing. This should be a more robust, unified way to handle these single-item operations. To test, apply the patch and enable the opacbookbag system preference. Test adding and removing items from the cart from various pages using various methods: - By clicking an "Add to your cart" / "Remove" link in search results - By clicking an "Add to your cart" / "Remove" link on a detail page - By checking boxes in search results and adding via the dropdown Each operation should work correctly. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 80292 [details] [review] Bug 21479: Add regression tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Martin Renvoize from comment #10) > I like Owens alternative.. as he says, it just feels more robust. Agreed. > QA Comment though.. could we possibly add a selenium test for this perhaps? Done, the patch is waiting for a/your QA stamp.
Created attachment 80304 [details] [review] Bug 21479: Fix remove from cart links at the OPAC (should remove only 1) This patch changes the way biblionumbers are passed to the cart script for adding and removing single items. The title's biblionumber is now stored in a data-attribute on the links for adding and removing. This should be a more robust, unified way to handle these single-item operations. To test, apply the patch and enable the opacbookbag system preference. Test adding and removing items from the cart from various pages using various methods: - By clicking an "Add to your cart" / "Remove" link in search results - By clicking an "Add to your cart" / "Remove" link on a detail page - By checking boxes in search results and adding via the dropdown Each operation should work correctly. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 80305 [details] [review] Bug 21479: Add regression tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works well, no regressions found. Thankyou for the regression test Jonathan :). Passing QA
Created attachment 80557 [details] [review] Bug 21479: Remove the trace if the node does not exist Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed for 18.11. Thanks all!
Created attachment 80605 [details] [review] Bug 21479: Zebra index can return different result If the zebra index has been rebuilt by previous tests, the order/results will be different, and we will not have biblionumber 1, 3, 5 Let make this more flexible Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Jonathan Druart from comment #19) > Created attachment 80605 [details] [review] [review] > Bug 21479: Zebra index can return different result > > If the zebra index has been rebuilt by previous tests, the order/results > will be different, and we will not have biblionumber 1, 3, 5 > > Let make this more flexible > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Follow up pushed to master for 18.11
Created attachment 80625 [details] [review] Bug 21479: Mock SearchEngine to use Zebra Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 80643 [details] [review] Bug 21479: "Mock" SearchEngine to use Zebra Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to 18.05.x for 18.05.05
Pushed to 17.11.x for 17.11.12
Regression tests removed from 17.11.x, test suite not able to run selenium for this version.