Bugzilla – Attachment 106890 Details for
Bug 24379
Patron login attempts happen to be NULL instead of 0
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24379: Make login_attempts not nullable
Bug-24379-Make-loginattempts-not-nullable.patch (text/plain), 6.68 KB, created by
Katrin Fischer
on 2020-07-15 06:01:03 UTC
(
hide
)
Description:
Bug 24379: Make login_attempts not nullable
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2020-07-15 06:01:03 UTC
Size:
6.68 KB
patch
obsolete
>From b1437b58d839f19dc5f068f769395516ba6e1802 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 8 Jan 2020 20:59:38 +0000 >Subject: [PATCH] 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> >--- > .../make_login_attempts_not_nullable.perl | 8 ++++++++ > installer/data/mysql/kohastructure.sql | 4 ++-- > installer/data/mysql/updatedatabase.pl | 4 ++-- > t/db_dependent/Koha/Patron.t | 21 ++++++++++++++++++++- > 4 files changed, 32 insertions(+), 5 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/make_login_attempts_not_nullable.perl > >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 >index 0000000000..4fd6b399f2 >--- /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 4f6a9c399d..3c6ef33c45 100644 >--- 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 8b6f3e9a7e..f88606569b 100755 >--- 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 63b77c1573..3548e8ca78 100644 >--- 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; >+}; >-- >2.11.0
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 24379
:
97058
|
97073
|
105522
|
106846
|
106847
| 106890 |
107099