Bug 17257 - Cannot create a patron under MySQL 5.7
Summary: Cannot create a patron under MySQL 5.7
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 16094 (view as bug list)
Depends on:
Blocks: 17258
  Show dependency treegraph
 
Reported: 2016-09-06 09:04 UTC by Jonathan Druart
Modified: 2017-12-07 22:20 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17257: Fix add/edit patrons under MySQL 5.7 (1.58 KB, patch)
2016-09-06 09:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17257: Fix add/edit patrons under MySQL 5.7 (1.13 KB, patch)
2017-03-22 01:40 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17257: Fix add/edit patrons under MySQL 5.7 (1.25 KB, patch)
2017-05-11 11:09 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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