Bugzilla – Attachment 71437 Details for
Bug 20144
Test suite is failing with new default SQL modes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20144: [sql_modes] Fix GROUP BY clause in GetLetters
Bug-20144-sqlmodes-Fix-GROUP-BY-clause-in-GetLette.patch (text/plain), 7.23 KB, created by
Julian Maurice
on 2018-02-12 11:02:45 UTC
(
hide
)
Description:
Bug 20144: [sql_modes] Fix GROUP BY clause in GetLetters
Filename:
MIME Type:
Creator:
Julian Maurice
Created:
2018-02-12 11:02:45 UTC
Size:
7.23 KB
patch
obsolete
>From c0e094faf0edf4b6a8a3a63ad1cc645a5125623e Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 5 Feb 2018 18:26:05 -0300 >Subject: [PATCH] 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 <josef.moravec@gmail.com> >Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> >--- > C4/Letters.pm | 7 +++++-- > t/db_dependent/Letters.t | 3 +-- > .../Letters/GetLettersAvailableForALibrary.t | 20 ++++++++++---------- > 3 files changed, 16 insertions(+), 14 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 42893f1bd3..219779bc3e 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -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 : () ) >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index e20cefb8b9..d8e96a6a7a 100644 >--- a/t/db_dependent/Letters.t >+++ b/t/db_dependent/Letters.t >@@ -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' ); >diff --git a/t/db_dependent/Letters/GetLettersAvailableForALibrary.t b/t/db_dependent/Letters/GetLettersAvailableForALibrary.t >index 4649a33742..8ebc4fea29 100644 >--- a/t/db_dependent/Letters/GetLettersAvailableForALibrary.t >+++ b/t/db_dependent/Letters/GetLettersAvailableForALibrary.t >@@ -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', >-- >2.14.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20144
:
71410
|
71411
|
71412
|
71413
|
71414
|
71415
|
71416
|
71417
|
71418
|
71419
|
71420
|
71421
|
71422
|
71423
|
71424
|
71425
|
71427
|
71428
|
71429
|
71430
|
71431
|
71432
|
71434
|
71435
|
71436
| 71437 |
71438
|
71440
|
71441
|
71442
|
71443
|
71444
|
71445
|
71446
|
71447
|
71448
|
71449
|
71450
|
71451
|
71452
|
71453
|
71454
|
71455
|
71456
|
71457
|
71458
|
71459
|
71460
|
71461
|
71462
|
71463
|
71467
|
71564