Using the OPAC, going into the public lists, I click one of them and get DBIx::Class::Storage::DBI::_dbh_execute(): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ASC LIMIT 10) `me` JOIN `biblio` `biblionumber` ON `biblionumber`.`biblionumber' THis is because the sortfield is empty and creates a SQL like GROUP BY ASC LIMIT 10. Missing the group by. In the intranet list, there's a default usage of 'title' that makes it all work. TESTING: 1) just create a public list with no sortfield. - virtualshelves.category = 2 - virtualshelves.sortfield = '' 2) click on the list in the OPAC's public list. Boom!
This also happens in 3.22, btw.
Created attachment 57010 [details] [review] Bug 17526 - OPAC lists require a sortfield Using the OPAC, going into the public lists, we click one of them and get DBIx::Class::Storage::DBI::_dbh_execute(): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ASC LIMIT 10) `me` JOIN `biblio` `biblionumber` ON `biblionumber`.`biblionumber' THis is because the sortfield is empty and creates a SQL like GROUP BY ASC LIMIT 10. Missing the group by <what?>. In the intranet list, there's a default usage of 'title' that makes it all work. TESTING: 1) just create a public list with no sortfield. In the DB: - virtualshelves.category = 2 - virtualshelves.sortfield = '' 2) click on the list in the OPAC's public list. Boom! This simple patch uses 'title' if nothing else is specified.
Created attachment 57104 [details] [review] Bug 17526 - OPAC lists require a sortfield Using the OPAC, going into the public lists, we click one of them and get DBIx::Class::Storage::DBI::_dbh_execute(): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ASC LIMIT 10) `me` JOIN `biblio` `biblionumber` ON `biblionumber`.`biblionumber' THis is because the sortfield is empty and creates a SQL like GROUP BY ASC LIMIT 10. Missing the group by <what?>. In the intranet list, there's a default usage of 'title' that makes it all work. TESTING: 1) just create a public list with no sortfield. In the DB: - virtualshelves.category = 2 - virtualshelves.sortfield = '' 2) click on the list in the OPAC's public list. Boom! This simple patch uses 'title' if nothing else is specified. Reproduced. Patch fixes issue as expected. Signed-off-by: Marc Véron <veron@veron.ch>
This should no longer be possible from the interface, so what about updating the bad data as well?
Cannot reproduce this from the interface. The request for a dbrev repairing bad data is a valid one. Changing status
But the data is there, the user doesn't really care that it won't happen anymore with the new interface. It seems that a simple default value is a very valid programming. In this case and in most others. This is not a failed QA. This is a "In Discussion" death sentence...
(In reply to Blou from comment #6) > But the data is there, the user doesn't really care that it won't happen > anymore with the new interface. It seems that a simple default value is a > very valid programming. In this case and in most others. > > This is not a failed QA. This is a "In Discussion" death sentence... Hum? Marcel is just asking the same thing as me: update the existing bad data. so set searchfield="title" where searchfield is null or "" and we are done.
(In reply to Blou from comment #6) > But the data is there, the user doesn't really care that it won't happen > anymore with the new interface. It seems that a simple default value is a > very valid programming. In this case and in most others. > > This is not a failed QA. This is a "In Discussion" death sentence... This is not a death sentence. Just add a small atomicupdate dbrev. I do not have a strong opinion about the line in opac-shelves. It should not be necessary, but it would not hurt either. Although Failed QA or In Discussion may look like the final destination of some patches, this generally speaking does not have to be so. (This one is not in that category at all.)
This patch is no longer relevant, it has been fixed by commit 45cffd874c62c7b090390c5fb3c955c31f524608 Bug 17901: Fix possible SQL injection in shelf editing However we could update the existing bad data anyway.
Repurposing... See also bug 22941 and bug 23084.
Created attachment 90584 [details] [review] Bug 17526: Change grep to deal with malformed sortfield Giving sortfield a malformed value when viewing lists results in Internal Server Error. eg. http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=view&shelfnumber=1&sortfield=title( Note the trailing ( Before Patch: kaboom apply patch restart After Patch: No kaboom
Created attachment 90641 [details] [review] Bug 17526: Change grep to deal with malformed sortfield Giving sortfield a malformed value when viewing lists results in Internal Server Error. eg. http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=view&shelfnumber=1&sortfield=title( Note the trailing ( Before Patch: kaboom apply patch restart After Patch: No kaboom Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Simple fix which works well for me, Signing Off
Created attachment 90695 [details] [review] Bug 17526: Change grep to deal with malformed sortfield Giving sortfield a malformed value when viewing lists results in Internal Server Error. eg. http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=view&shelfnumber=1&sortfield=title( Note the trailing ( Before Patch: kaboom apply patch restart After Patch: No kaboom Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Nice work! Pushed to master for 19.11.00
Pushed to 19.05.x for 19.05.01
Same fix as Bug 22941, nice catch