Commit 666cbed5 authored by joubu's avatar joubu

Bug 20144: [sql_modes] Fix GROUP BY clause in GetLetters

This subroutine is wrong and must be rewritten using
Koha::Notice::Templates.
Mainly because the DB structure is bad.
Meanwhile we remove the branchcode from the SELECT to get correct
results, it was not used by callers anyway.

Fix for:
'koha_kohadev.letter.module' isn't in GROUP BY

t/db_dependent/Letters.t
Signed-off-by: Josef Moravec's avatarJosef Moravec <josef.moravec@gmail.com>
Signed-off-by: default avatarJulian Maurice <julian.maurice@biblibre.com>
Signed-off-by: joubu's avatarJonathan Druart <jonathan.druart@bugs.koha-community.org>
parent 2decad7b
......@@ -70,6 +70,9 @@ C4::Letters - Give functions for Letters management
returns informations about letters.
if needed, $module filters for letters given module
DEPRECATED - You must use Koha::Notice::Templates instead
The group by clause is confusing and can lead to issues
=cut
sub GetLetters {
......@@ -80,14 +83,14 @@ sub GetLetters {
my $dbh = C4::Context->dbh;
my $letters = $dbh->selectall_arrayref(
q|
SELECT module, code, branchcode, name
SELECT code, module, name
FROM letter
WHERE 1
|
. ( $module ? q| AND module = ?| : q|| )
. ( $code ? q| AND code = ?| : q|| )
. ( defined $branchcode ? q| AND branchcode = ?| : q|| )
. q| GROUP BY code ORDER BY name|, { Slice => {} }
. q| GROUP BY code, module, name ORDER BY name|, { Slice => {} }
, ( $module ? $module : () )
, ( $code ? $code : () )
, ( defined $branchcode ? $branchcode : () )
......
......@@ -18,7 +18,7 @@
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Test::More tests => 77;
use Test::More tests => 76;
use Test::MockModule;
use Test::Warn;
......@@ -176,7 +176,6 @@ Look at this wonderful biblio timestamp: <<biblio.timestamp>>.
$dbh->do( q|INSERT INTO letter(branchcode,module,code,name,is_html,title,content,message_transport_type) VALUES (?,'my module','my code','my name',1,?,?,'email')|, undef, $library->{branchcode}, $title, $content );
$letters = C4::Letters::GetLetters();
is( @$letters, 1, 'GetLetters returns the correct number of letters' );
is( $letters->[0]->{branchcode}, $library->{branchcode}, 'GetLetters gets the branch code correctly' );
is( $letters->[0]->{module}, 'my module', 'GetLetters gets the module correctly' );
is( $letters->[0]->{code}, 'my code', 'GetLetters gets the code correctly' );
is( $letters->[0]->{name}, 'my name', 'GetLetters gets the name correctly' );
......
......@@ -15,7 +15,7 @@ my $letters = [
module => 'circulation',
code => 'code1',
branchcode => '',
name => 'B default name for code1 circ',
name => 'B name for code1 circ',
is_html => 0,
title => 'default title for code1 email',
content => 'default content for code1 email',
......@@ -25,7 +25,7 @@ my $letters = [
module => 'circulation',
code => 'code1',
branchcode => '',
name => 'B default name for code1 circ',
name => 'B name for code1 circ',
is_html => 0,
title => 'default title for code1 sms',
content => 'default content for code1 sms',
......@@ -35,7 +35,7 @@ my $letters = [
module => 'circulation',
code => 'code2',
branchcode => '',
name => 'A default name for code2 circ',
name => 'A name for code2 circ',
is_html => 0,
title => 'default title for code2 email',
content => 'default content for code2 email',
......@@ -45,7 +45,7 @@ my $letters = [
module => 'circulation',
code => 'code3',
branchcode => '',
name => 'C default name for code3 circ',
name => 'C name for code3 circ',
is_html => 0,
title => 'default title for code3 email',
content => 'default content for code3 email',
......@@ -56,7 +56,7 @@ my $letters = [
module => 'cataloguing',
code => 'code1',
branchcode => '',
name => 'default name for code1 cat',
name => 'D name for code1 cat',
is_html => 0,
title => 'default title for code1 cat email',
content => 'default content for code1 cat email',
......@@ -67,7 +67,7 @@ my $letters = [
module => 'circulation',
code => 'code1',
branchcode => 'CPL',
name => 'B CPL name for code1 circ',
name => 'B name for code1 circ',
is_html => 0,
title => 'CPL title for code1 email',
content => 'CPL content for code1 email',
......@@ -77,17 +77,17 @@ my $letters = [
module => 'circulation',
code => 'code2',
branchcode => 'CPL',
name => 'A CPL name for code1 circ',
name => 'A name for code2 circ',
is_html => 0,
title => 'CPL title for code1 sms',
content => 'CPL content for code1 sms',
title => 'CPL title for code2 sms',
content => 'CPL content for code2 sms',
message_transport_type => 'sms',
},
{
module => 'circulation',
code => 'code1',
branchcode => 'MPL',
name => 'B MPL name for code1 circ',
name => 'B name for code1 circ',
is_html => 0,
title => 'MPL title for code1 email',
content => 'MPL content for code1 email',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment