Bug 21603

Summary:	Incorrect GROUP BY clause in SearchCourses
Product:	Koha	Reporter:	Nick Clemens (kidclamp) <nick>
Component:	Course reserves	Assignee:	Jonathan Druart <jonathan.druart>
Status:	CLOSED FIXED	QA Contact:	Marcel de Rooy <m.de.rooy>
Severity:	major
Priority:	P5 - low	CC:	fridolin.somers, jonathan.druart, m.de.rooy, martin.renvoize
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Small patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Bug Depends on:
Bug Blocks:	17258
Attachments:	Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves

Description Nick Clemens (kidclamp) 2018-10-18 13:44:47 UTC

I am not certain we can simply remove here, but the GROUP by seems to stem form adding course instructors, though we fetch the instructors for each course after this query

 971     $query .= "
 972         LEFT JOIN course_instructors ci
 973             ON ( c.course_id = ci.course_id )
 974         LEFT JOIN borrowers b
 975             ON ( ci.borrowernumber = b.borrowernumber )
 976         LEFT JOIN authorised_values av
 977             ON ( c.department = av.authorised_value )
 978         WHERE
 979             ( av.category = 'DEPARTMENT' OR av.category = 'TERM' )
 980             AND
 981             (
 982                 department LIKE ? OR
 983                 course_number LIKE ? OR
 984                 section LIKE ? OR
 985                 course_name LIKE ? OR
 986                 term LIKE ? OR
 987                 public_note LIKE ? OR
 988                 CONCAT(surname,' ',firstname) LIKE ? OR
 989                 CONCAT(firstname,' ',surname) LIKE ? OR
 990                 lib LIKE ? OR
 991                 lib_opac LIKE ?
 992            )
 993            AND
 994            c.enabled LIKE ?
 995         GROUP BY c.course_id
 996     ";

Comment 1 Jonathan Druart 2018-10-18 20:42:48 UTC

'koha_kohadev.c.department' isn't in GROUP BY

then later:
Can't use an undefined value as an ARRAY reference at /usr/lib/x86_64-linux-gnu/perl5/5.20/DBI.pm line 2064

Comment 2 Jonathan Druart 2018-10-18 22:19:18 UTC

Created attachment 80884 [details] [review]
Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves

'koha_kohadev.c.department' isn't in GROUP BY

Test plan:
Prove that the test fail without this patch and pass with this patch
applied (switch on the SQL modes)

Comment 3 Andrew Isherwood 2018-10-25 14:13:33 UTC

Created attachment 81163 [details] [review]
Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves

'koha_kohadev.c.department' isn't in GROUP BY

Test plan:
Prove that the test fail without this patch and pass with this patch
applied (switch on the SQL modes)

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Comment 4 Marcel de Rooy 2018-10-26 10:00:38 UTC

Created attachment 81263 [details] [review]
Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves

'koha_kohadev.c.department' isn't in GROUP BY

Test plan:
Prove that the test fail without this patch and pass with this patch
applied (switch on the SQL modes)

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Comment 5 Marcel de Rooy 2018-10-26 10:01:33 UTC

> Prove that the test fail without this patch and pass with this patch
> applied (switch on the SQL modes)

Could not reproduce the warning with strict_sql_modes..
Lower severity ?

Comment 6 Jonathan Druart 2018-10-26 12:06:10 UTC

(In reply to Marcel de Rooy from comment #5)
> > Prove that the test fail without this patch and pass with this patch
> > applied (switch on the SQL modes)
> 
> Could not reproduce the warning with strict_sql_modes..
> Lower severity ?

The test plan is not cleared: you should
- apply the new test (so manually)
- prove the new test fails
- apply the pm changes
- prove the new test passes

I should have provided two patches.

Comment 7 Nick Clemens (kidclamp) 2018-10-26 17:19:55 UTC

Awesome work all!

Pushed to master for 18.11

Comment 8 Martin Renvoize (ashimema) 2018-11-06 11:56:31 UTC

Pushed to 18.05.x for 18.05.06

Comment 9 Fridolin Somers 2018-11-28 10:01:09 UTC

I pass for sql modes in 17.11.x