k-t-d 1) visit a biblio: http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=230 2) Tick 2 checkboxes on the left most column of the items table 3) Click the " Delete selected items" action. 4) Notice you are taken to http://localhost:8081/cgi-bin/koha/tools/batchMod.pl?op=show&del=1&itemnumber=507&itemnumber=508&biblionumber=230&src=CATALOGUING 5) Notice the page is blank
Created attachment 163227 [details] [review] Bug 36326: Fix batch item mod/del access from biblio detail page Allow both $op eq "show" and "cud-show". We need to keep the POST when we upload a file, but we can simply allow GET with "show". Test plan: Go to the biblio detail page, select an item and test both tools via the links "Delete selected items" and "Modify selected items"
Not the nicest fix, but the easiest. Other places have been fixed using a POST: commit 74d7ae7672a4b82568e57eaddefbded4bf5b05e6 Bug 34478: (follow-up) batchMod commit 925eb73a279e18194f05b92069d78fd93c88a58e Bug 34478: (follow-up) batch_record_modification
Created attachment 163238 [details] [review] Bug 36326: PoC Joubu, whats your opinion on this approach?
Replacing "build link" logic with "build invisible form" logic.
With bug 33568 in the queue, I really would prefer to limit the changes we add to this template.
Otherwise the way to do would be to have a form with empty hidden inputs that we populate when the form is submitted. like what we do on commit 1415fee322a497a3dc7e511a61854a4099ea40a6 Bug 36193: cud- treatment for cash_registers.pl or commit c706fc9e41583f593bda7a555764f61116e11fbb Bug 34478: Replace delete links with form - smart-rules
Created attachment 163250 [details] [review] Bug 36326: PoC followup I've moved the static elements to the form instead of appending them. This does not overlap 100% with bug 36193 or 34478 as in those cases we're not adding inputs to the form dynamically, so we should/have to keep that in here. Let me know what you think.
This second patch is causing a major conflict with bug 33568 while my single line patch is fixing the original problem without any conflicts. I would prefer to prevent change to this template until 33568 is pushed. I will be happy to provide a better fix on top of it once in master.
Created attachment 163276 [details] [review] Bug 36326: Fix batch item mod/del access from biblio detail page Allow both $op eq "show" and "cud-show". We need to keep the POST when we upload a file, but we can simply allow GET with "show". Test plan: Go to the biblio detail page, select an item and test both tools via the links "Delete selected items" and "Modify selected items" Signed-off-by: David Nind <david@davidnind.com>
I've signed off Jonathan's patch. Feel free to add me to the bug once bug 33568 is pushed.
Looking here
Created attachment 163661 [details] [review] Bug 36326: Fix batch item mod/del access from biblio detail page Allow both $op eq "show" and "cud-show". We need to keep the POST when we upload a file, but we can simply allow GET with "show". Test plan: Go to the biblio detail page, select an item and test both tools via the links "Delete selected items" and "Modify selected items" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 163662 [details] [review] Bug 36326: PoC followup I've moved the static elements to the form instead of appending them. This does not overlap 100% with bug 36193 or 34478 as in those cases we're not adding inputs to the form dynamically, so we should/have to keep that in here. Let me know what you think. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 163663 [details] [review] Bug 36326: Fix batch item mod/del access from biblio detail page Allow both $op eq "show" and "cud-show". We need to keep the POST when we upload a file, but we can simply allow GET with "show". Test plan: Go to the biblio detail page, select an item and test both tools via the links "Delete selected items" and "Modify selected items" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Only the second patch should be needed here.
Waiting to push this one until discussion is resolved - please tidy commit message if this is the one to be pushed.
Comment on attachment 163662 [details] [review] Bug 36326: PoC followup Yes, we can also push the other one on itself.
(In reply to Jonathan Druart from comment #15) > Only the second patch should be needed here. Confirmed
Pushed for 24.05! Well done everyone, thank you!
Depends on Bug 34478 not in 23.11.x
(In reply to Jonathan Druart from comment #8) > This second patch is causing a major conflict with bug 33568 while my single > line patch is fixing the original problem without any conflicts. > I would prefer to prevent change to this template until 33568 is pushed. > > I will be happy to provide a better fix on top of it once in master. Did we get the 'better fix' submitted anywhere?
(In reply to Martin Renvoize from comment #21) > (In reply to Jonathan Druart from comment #8) > > This second patch is causing a major conflict with bug 33568 while my single > > line patch is fixing the original problem without any conflicts. > > I would prefer to prevent change to this template until 33568 is pushed. > > > > I will be happy to provide a better fix on top of it once in master. > > Did we get the 'better fix' submitted anywhere? I totally forgot about it, thanks for the reminder. I will work on it tomorrow, see bug 37309.