From a61eafa6631f3a0a926b2a8f5405209b57c968cc Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 17 Apr 2019 11:20:43 -0300 Subject: [PATCH] Bug 21336: (QA follow-up) Rename flgAnonymized column for anonymized This patch renames the flgAnonymized column for anonymized, and it also sets it to - TINYINT(1) - NOT NULL The ALTER TABLE updatedatabase entries are adjusted to make sure the column will be added after overdrive_... always, and preventing backporting issues. Just in case. Tests are adjusted to reflect the changes, as well as Koha::Patrons Signed-off-by: Tomas Cohen Arazi --- Koha/Patron.pm | 4 ++-- Koha/Patrons.pm | 4 ++-- Koha/Schema/Result/Borrower.pm | 20 +++++++------------ Koha/Schema/Result/Deletedborrower.pm | 18 ++++++----------- .../atomicupdate/bug_21336_followup.perl | 20 +++++++++++++++++++ installer/data/mysql/kohastructure.sql | 4 ++-- installer/data/mysql/updatedatabase.pl | 15 ++++++++++++-- t/db_dependent/Koha/Patrons.t | 10 +++++----- 8 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_21336_followup.perl diff --git a/Koha/Patron.pm b/Koha/Patron.pm index c3326285b9..6a25d717c1 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1362,12 +1362,12 @@ sub anonymize { split /\s*\|\s*/, C4::Context->preference('BorrowerMandatoryField') }; $mandatory->{userid} = 1; # needed since sub store does not clear field my @columns = $self->_result->result_source->columns; - @columns = grep { !/borrowernumber|branchcode|categorycode|^date|password|flags|updated_on|lastseen|lang|login_attempts|flgAnonymized/ } @columns; + @columns = grep { !/borrowernumber|branchcode|categorycode|^date|password|flags|updated_on|lastseen|lang|login_attempts|anonymized/ } @columns; push @columns, 'dateofbirth'; # add this date back in foreach my $col (@columns) { $self->_anonymize_column($col, $mandatory->{lc $col} ); } - $self->flgAnonymized(1)->store; + $self->anonymized(1)->store; } sub _anonymize_column { diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index b35e6cc4a8..7510093185 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -293,7 +293,7 @@ sub search_anonymize_candidates { my $dt = dt_from_string()->subtract( days => $delay ); my $str = $parser->format_datetime($dt); $cond->{dateexpiry} = { '<=' => $str }; - $cond->{flgAnonymized} = [ undef, 0 ]; # not yet done + $cond->{anonymized} = 0; # not yet done if( $params->{locked} ) { my $fails = C4::Context->preference('FailedLoginAttempts'); $cond->{login_attempts} = [ -and => { '!=' => undef }, { -not_in => [0, 1..$fails-1 ] } ]; # -not_in does not like undef @@ -324,7 +324,7 @@ sub search_anonymized { my $dt = dt_from_string()->subtract( days => $delay ); my $str = $parser->format_datetime($dt); $cond->{dateexpiry} = { '<=' => $str }; - $cond->{flgAnonymized} = 1; + $cond->{anonymized} = 1; return $class->search( $cond ); } diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index f2fa3bff38..2c3e8540ba 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -451,12 +451,11 @@ __PACKAGE__->table("borrowers"); data_type: 'mediumtext' is_nullable: 1 -=head2 flgAnonymized +=head2 anonymized - accessor: 'flg_anonymized' data_type: 'tinyint' default_value: 0 - is_nullable: 1 + is_nullable: 0 =cut @@ -660,13 +659,8 @@ __PACKAGE__->add_columns( { data_type => "integer", default_value => 0, is_nullable => 1 }, "overdrive_auth_token", { data_type => "mediumtext", is_nullable => 1 }, - "flgAnonymized", - { - accessor => "flg_anonymized", - data_type => "tinyint", - default_value => 0, - is_nullable => 1, - }, + "anonymized", + { data_type => "tinyint", default_value => 0, is_nullable => 0 }, ); =head1 PRIMARY KEY @@ -1535,8 +1529,8 @@ Composing rels: L -> ordernumber __PACKAGE__->many_to_many("ordernumbers", "aqorder_users", "ordernumber"); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2019-04-17 11:11:33 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ZkPU/EUsZVXiRoQ1E8NMbw +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2019-04-16 18:53:11 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ehheuLbVpMKvKtLpSI73DA __PACKAGE__->belongs_to( "guarantor", @@ -1545,7 +1539,7 @@ __PACKAGE__->belongs_to( ); __PACKAGE__->add_columns( - '+lost' => { is_boolean => 1 }, + '+lost' => { is_boolean => 1 }, '+gonenoaddress' => { is_boolean => 1 } ); diff --git a/Koha/Schema/Result/Deletedborrower.pm b/Koha/Schema/Result/Deletedborrower.pm index dc20a65d4e..6671cd3af7 100644 --- a/Koha/Schema/Result/Deletedborrower.pm +++ b/Koha/Schema/Result/Deletedborrower.pm @@ -448,12 +448,11 @@ __PACKAGE__->table("deletedborrowers"); data_type: 'mediumtext' is_nullable: 1 -=head2 flgAnonymized +=head2 anonymized - accessor: 'flg_anonymized' data_type: 'tinyint' default_value: 0 - is_nullable: 1 + is_nullable: 0 =cut @@ -645,18 +644,13 @@ __PACKAGE__->add_columns( { data_type => "integer", default_value => 0, is_nullable => 1 }, "overdrive_auth_token", { data_type => "mediumtext", is_nullable => 1 }, - "flgAnonymized", - { - accessor => "flg_anonymized", - data_type => "tinyint", - default_value => 0, - is_nullable => 1, - }, + "anonymized", + { data_type => "tinyint", default_value => 0, is_nullable => 0 }, ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2019-04-17 11:11:33 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:9XbNY+E7YqzsiUldTFg6Zg +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2019-04-16 18:53:12 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:d/vqm936LpPrgj9fvpyDUQ # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/installer/data/mysql/atomicupdate/bug_21336_followup.perl b/installer/data/mysql/atomicupdate/bug_21336_followup.perl new file mode 100644 index 0000000000..880d83205e --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_21336_followup.perl @@ -0,0 +1,20 @@ +$DBversion = 'XXX'; +if( CheckVersion( $DBversion ) ) { + + if ( column_exists( 'borrowers', 'flgAnonymized' ) ) { + $dbh->do(q{ + ALTER TABLE borrowers + CHANGE `flgAnonymized` `anonymized` TINYINT(1) NOT NULL DEFAULT 0 + }); + } + + if ( column_exists( 'deletedborrowers', 'flgAnonymized' ) ) { + $dbh->do(q{ + ALTER TABLE deletedborrowers + CHANGE `flgAnonymized` `anonymized` TINYINT(1) NOT NULL DEFAULT 0 + }); + } + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 21336 - (follow-up) Rename flgAnonymized column)\n"; +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index b01fb66af2..929da6163c 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -609,7 +609,7 @@ CREATE TABLE `deletedborrowers` ( -- stores data related to the patrons/borrower `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 `overdrive_auth_token` MEDIUMTEXT default NULL, -- persist OverDrive auth token - `flgAnonymized` tinyint DEFAULT 0, -- flag for data anonymization + `anonymized` TINYINT(1) NOT NULL DEFAULT 0, -- flag for data anonymization KEY borrowernumber (borrowernumber), KEY `cardnumber` (`cardnumber`), KEY `sms_provider_id` (`sms_provider_id`) @@ -1644,7 +1644,7 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons `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 `overdrive_auth_token` MEDIUMTEXT default NULL, -- persist OverDrive auth token - `flgAnonymized` tinyint DEFAULT 0, -- flag for data anonymization + `anonymized` TINYINT(1) NOT NULL DEFAULT 0, -- flag for data anonymization UNIQUE KEY `cardnumber` (`cardnumber`), PRIMARY KEY `borrowernumber` (`borrowernumber`), KEY `categorycode` (`categorycode`), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 1d2feb4baf..3bb31fa4c3 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -18037,8 +18037,19 @@ if( CheckVersion( $DBversion ) ) { $DBversion = '18.12.00.049'; if( CheckVersion( $DBversion ) ) { - $dbh->do( "ALTER TABLE borrowers ADD COLUMN flgAnonymized tinyint DEFAULT 0" ) if !column_exists('borrowers', 'flgAnonymized'); - $dbh->do( "ALTER TABLE deletedborrowers ADD COLUMN flgAnonymized tinyint DEFAULT 0" ) if !column_exists('deletedborrowers', 'flgAnonymized'); + + $dbh->do(q{ + ALTER TABLE borrowers + ADD COLUMN flgAnonymized tinyint DEFAULT 0 + AFTER `overdrive_auth_token` + }) if !column_exists('borrowers', 'flgAnonymized'); + + $dbh->do(q{ + ALTER TABLE deletedborrowers + ADD COLUMN flgAnonymized tinyint DEFAULT 0 + AFTER `overdrive_auth_token` + }) if !column_exists('deletedborrowers', 'flgAnonymized'); + SetVersion( $DBversion ); print "Upgrade to $DBversion done (Bug 21336 - Add field flgAnonymized)\n"; } diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index b248729de0..7e4a535f9e 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1663,9 +1663,9 @@ subtest 'search_anonymize_candidates' => sub { plan tests => 5; my $patron1 = $builder->build_object({ class => 'Koha::Patrons' }); my $patron2 = $builder->build_object({ class => 'Koha::Patrons' }); - $patron1->flgAnonymized(0); + $patron1->anonymized(0); $patron1->dateexpiry( dt_from_string->add(days => 1) )->store; - $patron2->flgAnonymized(undef); + $patron2->anonymized(0); $patron2->dateexpiry( dt_from_string->add(days => 1) )->store; t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', q{} ); @@ -1713,9 +1713,9 @@ subtest 'search_anonymized' => sub { t::lib::Mocks::mock_preference( 'PatronRemovalDelay', 1 ); $patron1->dateexpiry( dt_from_string ); - $patron1->flgAnonymized(0)->store; + $patron1->anonymized(0)->store; my $cnt = Koha::Patrons->search_anonymized->count; - $patron1->flgAnonymized(1)->store; + $patron1->anonymized(1)->store; is( Koha::Patrons->search_anonymized->count, $cnt, 'Number unchanged' ); $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store; is( Koha::Patrons->search_anonymized->count, $cnt+1, 'Found patron1' ); @@ -1775,7 +1775,7 @@ subtest 'anonymize' => sub { my $surname = $patron1->surname; # expect change, no clear my $branchcode = $patron1->branchcode; # expect skip $patron1->anonymize; - is($patron1->flgAnonymized, 1, 'Check flag' ); + is($patron1->anonymized, 1, 'Check flag' ); is( $patron1->dateofbirth, undef, 'Birth date cleared' ); is( $patron1->firstname, undef, 'First name cleared' ); -- 2.21.0