Bug 17257

Summary: Cannot create a patron under MySQL 5.7
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: hblancoca, julian.maurice, katrin.fischer, kyle, mtj, srdjan
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16583
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17258    
Attachments: Bug 17257: Fix add/edit patrons under MySQL 5.7
[SIGNED-OFF] Bug 17257: Fix add/edit patrons under MySQL 5.7
Bug 17257: Fix add/edit patrons under MySQL 5.7

Description Jonathan Druart 2016-09-06 09:04:13 UTC
DBD::mysql::st execute failed: Incorrect integer value: '' for column 'guarantorid'
Comment 1 Jonathan Druart 2016-09-06 09:15:04 UTC
Created attachment 55231 [details] [review]
Bug 17257: Fix add/edit patrons under MySQL 5.7

If no guarantor is defined the patron won't be added/modified and an
error will be raised:

DBD::mysql::st execute failed: Incorrect integer value: '' for column
'guarantorid'

Test plan:
Using MySQL 5.7 (and/or sql_mode=STRICT_TRANS_TABLES)
Create a patron without guarantor
Comment 2 Jonathan Druart 2016-09-06 09:50:20 UTC
*** Bug 16094 has been marked as a duplicate of this bug. ***
Comment 3 Srdjan Jankovic 2017-03-21 02:17:48 UTC
I don't have mysql (running mariadb), and wondering how come DBIx::Class did not solve it?
Anyway, if it is a problem, I think Koha::Patron is the place to solve it.
Comment 4 Jonathan Druart 2017-03-21 10:44:07 UTC
(In reply to Srdjan Jankovic from comment #3)
> I don't have mysql (running mariadb), and wondering how come DBIx::Class did
> not solve it?

If the key exists, the default value defined at DB level is not used.

> Anyway, if it is a problem, I think Koha::Patron is the place to solve it.

I do not understand, we are not using Koha::Patron here.
Comment 5 Srdjan Jankovic 2017-03-22 00:39:53 UTC
(In reply to Jonathan Druart from comment #4)
> (In reply to Srdjan Jankovic from comment #3)
> > I don't have mysql (running mariadb), and wondering how come DBIx::Class did
> > not solve it?
> 
> If the key exists, the default value defined at DB level is not used.

Ah I see. Then maybe

delete $data{'dateofbirth'} if exists($data{'dateofbirth') && !$data{'dateofbirth'};

would make intention clearer.

> I do not understand, we are not using Koha::Patron here.

A couple of lines later:

my $patron = Koha::Patron->new( $new_member )->store;

Considering the line number mismatch, I'd say base for this patch is lagging a bit.
Comment 6 Jonathan Druart 2017-03-22 01:27:25 UTC
(In reply to Srdjan Jankovic from comment #5)
> (In reply to Jonathan Druart from comment #4)
> > (In reply to Srdjan Jankovic from comment #3)
> > > I don't have mysql (running mariadb), and wondering how come DBIx::Class did
> > > not solve it?
> > 
> > If the key exists, the default value defined at DB level is not used.
> 
> Ah I see. Then maybe
> 
> delete $data{'dateofbirth'} if exists($data{'dateofbirth') &&
> !$data{'dateofbirth'};
> 
> would make intention clearer.

I just mimic what existed before.

> > I do not understand, we are not using Koha::Patron here.
> 
> A couple of lines later:
> 
> my $patron = Koha::Patron->new( $new_member )->store;
> 
> Considering the line number mismatch, I'd say base for this patch is lagging
> a bit.

As I said, I just mimic what existed before.
The plan is to move C4::Members::AddMember to Koha::Patron, of course. But that patch is just supposed to be a bugfix.
Comment 7 Srdjan Jankovic 2017-03-22 01:40:05 UTC
Created attachment 61425 [details] [review]
[SIGNED-OFF] Bug 17257: Fix add/edit patrons under MySQL 5.7

If no guarantor is defined the patron won't be added/modified and an
error will be raised:

DBD::mysql::st execute failed: Incorrect integer value: '' for column
'guarantorid'

Test plan:
Using MySQL 5.7 (and/or sql_mode=STRICT_TRANS_TABLES)
Create a patron without guarantor

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Comment 8 Srdjan Jankovic 2017-03-22 01:50:16 UTC
(In reply to Jonathan Druart from comment #6)
> As I said, I just mimic what existed before.
> The plan is to move C4::Members::AddMember to Koha::Patron, of course. But
> that patch is just supposed to be a bugfix.
I understand. In that light I think it would be better to fix it in Koha::Patron rather than adding more code to a legacy module. Especially when you can add it elegantly to Patron::new(), bonus moving similar lines too - a micro-refactoring that will make moving AddMember to Patroms easier.
Comment 9 Jonathan Druart 2017-03-22 02:26:05 UTC
Thanks for volunteering :)
Comment 10 Julian Maurice 2017-05-11 11:09:56 UTC
Created attachment 63382 [details] [review]
Bug 17257: Fix add/edit patrons under MySQL 5.7

If no guarantor is defined the patron won't be added/modified and an
error will be raised:

DBD::mysql::st execute failed: Incorrect integer value: '' for column
'guarantorid'

Test plan:
Using MySQL 5.7 (and/or sql_mode=STRICT_TRANS_TABLES)
Create a patron without guarantor

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Tested with MariaDB 10.0.30 (sql_mode='STRICT_TRANS_TABLES')
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 11 Kyle M Hall 2017-05-12 12:59:59 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 12 Katrin Fischer 2017-05-14 10:02:00 UTC
This patch has been pushed to 16.11.x and will be in 16.11.08.
Comment 13 Julian Maurice 2017-05-22 10:56:27 UTC
Pushed to 3.22.x for 3.22.21
Comment 14 Mason James 2017-05-24 02:38:29 UTC
Pushed to 16.05.x, for 16.05.13 release