Bug 24379: Make login_attempts not nullable

While the column defaults to 0 in Koha::Object->store we set to NULL if NULLABLE

When trying to reset a patrons password we check that the account is not administratively locked:
login_attempts != -1

This query does not return rows where login_attempts IS NULL. It will return accounts where login_attempts = 0

Let's default to 0 like we intend

To test:
1 - Create a new patron
2 - Note their login_attempts is NULL
    SELECT login_attempts FROM borrowers ORDER BY borrowernumber DESC LIMIT 1
3 - Enable  OpacResetPassword
4 - Attempt to reset password before logging in, you cannot
5 - Apply patch, updatedatabase, restart_all, update schema
6 - Create another patron
7 - Their login attempts should be 0
8 - Attempt to reset password, it works!

Bug 24379: Fix the test

First we create a patron using TestBuilder to get a hashref of valid
info. Then we delete it and create a new patron using Koha::Patron->new
Once stored, we should call discard_changes to make the calculated
values available in the currenct object.

Bug 24379: Don't drop default of 0 for login attempts

When moving the column we drop the default, this means that DBs upgraded form earlier versions
get the wrong values set

To test:
1 - Checkout 16.11.x
2 - Reset all
3 - Checkout master
4 - updatedatabase
5 - SHOW CREATE TABLE borrowers;
6 - Note the column login_attempts defaults to NULL
7 - Apply patch(es)
8 - Repeat
9 - Now it defaults ot 0 (and has NOT NULL if applied all)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/installer/data/mysql/atomicupdate/make_login_attempts_not_nullable.perl b/installer/data/mysql/atomicupdate/make_login_attempts_not_nullable.perl
new file mode 100644 (file)
index 0000000..4fd6b39
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/make_login_attempts_not_nullable.perl
@@ -0,0 +1,8 @@
+if( CheckVersion( $DBversion ) ) {
+    $dbh->do( "UPDATE borrowers SET login_attempts=0 WHERE login_attempts IS NULL" );
+    $dbh->do( "ALTER TABLE borrowers MODIFY COLUMN login_attempts int(4) NOT NULL DEFAULT 0" );
+    $dbh->do( "UPDATE deletedborrowers SET login_attempts=0 WHERE login_attempts IS NULL" );
+    $dbh->do( "ALTER TABLE deletedborrowers MODIFY COLUMN login_attempts int(4) NOT NULL DEFAULT 0" );
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug XXXXX - Set login_attempts NOT NULL)\n";
+}

diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index 4f6a9c3..3c6ef33 100644 (file)
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -602,7 +602,7 @@ CREATE TABLE `deletedborrowers` ( -- stores data related to the patrons/borrower
   `updated_on` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, -- time of last change could be useful for synchronization with external systems (among others)
   `lastseen` datetime default NULL, -- last time a patron has been seen (connected at the OPAC or staff interface)
   `lang` varchar(25) NOT NULL default 'default', -- lang to use to send notices to this patron
-  `login_attempts` int(4) default 0, -- number of failed login attemps
+  `login_attempts` int(4) NOT NULL default 0, -- number of failed login attemps
   `overdrive_auth_token` MEDIUMTEXT default NULL, -- persist OverDrive auth token
   `anonymized` TINYINT(1) NOT NULL DEFAULT 0, -- flag for data anonymization
   `autorenew_checkouts` TINYINT(1) NOT NULL DEFAULT 1, -- flag for allowing auto-renewal
@@ -1522,7 +1522,7 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons
   `updated_on` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, -- time of last change could be useful for synchronization with external systems (among others)
   `lastseen` datetime default NULL, -- last time a patron has been seen (connected at the OPAC or staff interface)
   `lang` varchar(25) NOT NULL default 'default', -- lang to use to send notices to this patron
-  `login_attempts` int(4) default 0, -- number of failed login attemps
+  `login_attempts` int(4) NOT NULL default 0, -- number of failed login attemps
   `overdrive_auth_token` MEDIUMTEXT default NULL, -- persist OverDrive auth token
   `anonymized` TINYINT(1) NOT NULL DEFAULT 0, -- flag for data anonymization
   `autorenew_checkouts` TINYINT(1) NOT NULL DEFAULT 1, -- flag for allowing auto-renewal

diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index 58708f1..eb3ce36 100755 (executable)
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -14662,10 +14662,10 @@ if( CheckVersion( $DBversion ) ) {
 $DBversion = '17.06.00.009';
 if( CheckVersion( $DBversion ) ) {
     $dbh->do(q{
-        ALTER TABLE borrowers MODIFY COLUMN login_attempts int(4) AFTER lang;
+        ALTER TABLE borrowers MODIFY COLUMN login_attempts int(4) DEFAULT 0 AFTER lang;
     });
     $dbh->do(q{
-        ALTER TABLE deletedborrowers MODIFY COLUMN login_attempts int(4) AFTER lang;
+        ALTER TABLE deletedborrowers MODIFY COLUMN login_attempts int(4) DEFAULT 0 AFTER lang;
     });
 
     SetVersion( $DBversion );

diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
index 63b77c1..3548e8c 100644 (file)
--- a/t/db_dependent/Koha/Patron.t
+++ b/t/db_dependent/Koha/Patron.t
@@ -19,7 +19,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 3;
+use Test::More tests => 4;
 use Test::Exception;
 
 use Koha::Database;
@@ -192,3 +192,22 @@ subtest 'to_api() tests' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'login_attempts tests' => sub {
+    plan tests => 1;
+
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+        }
+    );
+    my $patron_info = $patron->unblessed;
+    $patron->delete;
+    delete $patron_info->{login_attempts};
+    my $new_patron = Koha::Patron->new($patron_info)->store;
+    is( $new_patron->discard_changes->login_attempts, 0, "login_attempts defaults to 0 as expected");
+
+    $schema->storage->txn_rollback;
+};