| Summary: | Remove GROUP BY clause in GetCourses | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Julian Maurice <julian.maurice> |
| Component: | Test Suite | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Julian Maurice <julian.maurice> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | martin.renvoize, mtompset, nick |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20144 | ||
| 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, 20521 | ||
| Attachments: |
Bug 20177: [sql_modes] Remove GROUP BY clause in GetCourses
Bug 20177: [sql_modes] Fix GROUP BY clause in GetCourses Bug 20177: [sql_modes] Fix GROUP BY clause in GetCourses Bug 20177: [sql_modes] Fix GROUP BY clause in GetCourses |
||
|
Description
Julian Maurice
2018-02-12 13:45:20 UTC
Created attachment 71468 [details] [review] Bug 20177: [sql_modes] Remove GROUP BY clause in GetCourses Fix for: 'koha_kohadev.courses.department' isn't in GROUP BY t/db_dependent/CourseReserves.t no need to group by here Signed-off-by: Josef Moravec <josef.moravec@gmail.com> The GROUP BY is needed here. Otherwise you can see the same course multiple times in /cgi-bin/koha/course_reserves/course-reserves.pl But maybe the joins are useless ? Created attachment 71620 [details] [review] Bug 20177: [sql_modes] Fix GROUP BY clause in GetCourses Fix for: 'koha_kohadev.courses.department' isn't in GROUP BY t/db_dependent/CourseReserves.t A better fix would be to remove the joins and only return values from the courses table. But more work is needed to acchieve that goal. (In reply to Julian Maurice from comment #3) > But maybe the joins are useless ? I'm thinking the idea was to let courses exist, but if there are no reserves or items, then why list the course? (In reply to Jonathan Druart from comment #4) > A better fix would be to remove the joins and only return values from > the courses table. But more work is needed to acchieve that goal. This sub is only used in one place. What "more work" is needed? A refactor to Objects? The GROUP BY and JOINs make no sense in the one template it is used. It's also called from GetCourseReserves How can I trigger the bug ?
> mysql> SELECT @@SQL_MODE;
> ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
But no errors on master while creating/editing courses and course reserves
prove t/db_dependent/CourseReserves.t pass on master too.
(In reply to Julian Maurice from comment #8) > How can I trigger the bug ? > > > mysql> SELECT @@SQL_MODE; > > ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION > But no errors on master while creating/editing courses and course reserves > > prove t/db_dependent/CourseReserves.t pass on master too. Yes, since bug 20229 is pushed, the sql mode are set for the connection. Try with bug 20521 applied. Created attachment 79405 [details] [review] Bug 20177: [sql_modes] Fix GROUP BY clause in GetCourses Fix for: 'koha_kohadev.courses.department' isn't in GROUP BY t/db_dependent/CourseReserves.t A better fix would be to remove the joins and only return values from the courses table. But more work is needed to acchieve that goal. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test passes and the behavior of Course reserves appears to be unchanged Created attachment 80080 [details] [review] Bug 20177: [sql_modes] Fix GROUP BY clause in GetCourses Fix for: 'koha_kohadev.courses.department' isn't in GROUP BY t/db_dependent/CourseReserves.t A better fix would be to remove the joins and only return values from the courses table. But more work is needed to acchieve that goal. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test passes and the behavior of Course reserves appears to be unchanged Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.05 |