Bugzilla – Attachment 50122 Details for
Bug 12267
Allow password option in Patron Attribute non functional
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12267: Remove borrower_attributes.password
Bug-12267-Remove-borrowerattributespassword.patch (text/plain), 21.57 KB, created by
Jonathan Druart
on 2016-04-11 09:19:00 UTC
(
hide
)
Description:
Bug 12267: Remove borrower_attributes.password
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2016-04-11 09:19:00 UTC
Size:
21.57 KB
patch
obsolete
>From 3b406fcf55498307d8a89506dedbb20c8e282396 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 11 Apr 2016 10:05:01 +0100 >Subject: [PATCH] Bug 12267: Remove borrower_attributes.password > >When creating a patron attribute type, there is a "Allow password" >checkbox. If checked, the librarian will be able to enter a password for >this patron attribute when editing a patron. >The goal was to allow a patron to log in with a secondary password. >However, this feature has never been implemented. > >""" >commit 6fc62bcd321eddb0fd3ae46903e9ab6c8b1db2cd > CommitDate: Mon May 12 09:03:00 2008 -0500 > extended patron attributes tables & syspref (DB rev 081) > >- password_allowed (if set, staff patron editor will > allow a password to be associated with a value; this > is mostly a hook for functionality to be implemented > in the future. >""" > >To decrease maintainability, this patch suggest to remove the 2 DB fields >borrower_attributes.password and >borrower_attribute_types.password_allowed >If they have not used by the library. > >Test plan: >- Edit a patron attribute type and select "allow password" >- Edit a patron and defined a password for this attribute >- Execute the DB entry >- Note that you get a warning >- Empty the password field >- Execute the DB entry >- You do not get the warning and the 2 DB fields have been removed >--- > C4/Members/AttributeTypes.pm | 35 +++------------------- > C4/Members/Attributes.pm | 17 ++++------- > admin/patron-attr-types.pl | 8 ----- > installer/data/mysql/atomicupdate/bug_12267.perl | 18 +++++++++++ > installer/data/mysql/kohastructure.sql | 2 -- > .../prog/en/modules/admin/patron-attr-types.tt | 8 ----- > .../prog/en/modules/members/memberentrygen.tt | 4 --- > members/memberentry.pl | 6 +--- > t/Members_AttributeTypes.t | 6 ++-- > t/Members_Attributes.t | 6 ---- > 10 files changed, 31 insertions(+), 79 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_12267.perl > >diff --git a/C4/Members/AttributeTypes.pm b/C4/Members/AttributeTypes.pm >index eb4346d..be236f0 100644 >--- a/C4/Members/AttributeTypes.pm >+++ b/C4/Members/AttributeTypes.pm >@@ -37,7 +37,6 @@ C4::Members::AttributeTypes - mananage extended patron attribute types > $attr_type->repeatable($repeatable); > $attr_type->unique_id($unique_id); > $attr_type->opac_display($opac_display); >- $attr_type->password_allowed($password_allowed); > $attr_type->staff_searchable($staff_searchable); > $attr_type->authorised_value_category($authorised_value_category); > $attr_type->store(); >@@ -123,7 +122,6 @@ sub new { > $self->{'repeatable'} = 0; > $self->{'unique_id'} = 0; > $self->{'opac_display'} = 0; >- $self->{'password_allowed'} = 0; > $self->{'staff_searchable'} = 0; > $self->{'display_checkout'} = 0; > $self->{'authorised_value_category'} = ''; >@@ -165,7 +163,6 @@ sub fetch { > $self->{'repeatable'} = $row->{'repeatable'}; > $self->{'unique_id'} = $row->{'unique_id'}; > $self->{'opac_display'} = $row->{'opac_display'}; >- $self->{'password_allowed'} = $row->{'password_allowed'}; > $self->{'staff_searchable'} = $row->{'staff_searchable'}; > $self->{'display_checkout'} = $row->{'display_checkout'}; > $self->{'authorised_value_category'} = $row->{'authorised_value_category'}; >@@ -206,7 +203,6 @@ sub store { > repeatable = ?, > unique_id = ?, > opac_display = ?, >- password_allowed = ?, > staff_searchable = ?, > authorised_value_category = ?, > display_checkout = ?, >@@ -215,23 +211,12 @@ sub store { > WHERE code = ?"); > } else { > $sth = $dbh->prepare_cached("INSERT INTO borrower_attribute_types >- (description, repeatable, unique_id, opac_display, password_allowed, >+ (description, repeatable, unique_id, opac_display, > staff_searchable, authorised_value_category, display_checkout, category_code, class, code) >- VALUES (?, ?, ?, ?, ?, >+ VALUES (?, ?, ?, ?, > ?, ?, ?, ?, ?, ?)"); > } >- $sth->bind_param(1, $self->{'description'}); >- $sth->bind_param(2, $self->{'repeatable'}); >- $sth->bind_param(3, $self->{'unique_id'}); >- $sth->bind_param(4, $self->{'opac_display'}); >- $sth->bind_param(5, $self->{'password_allowed'}); >- $sth->bind_param(6, $self->{'staff_searchable'}); >- $sth->bind_param(7, $self->{'authorised_value_category'}); >- $sth->bind_param(8, $self->{'display_checkout'}); >- $sth->bind_param(9, $self->{'category_code'} || undef); >- $sth->bind_param(10, $self->{'class'}); >- $sth->bind_param(11, $self->{'code'}); >- $sth->execute; >+ $sth->execute( $self->{description}, $self->{repeatable}, $self->{unique_id}, $self->{opac_display}, $self->{staff_searchable}, $self->{authorised_value_category}, $self->{display_checkout}, $self->{category_code} || undef, $self->{class}, $self->{code} ); > > if ( defined $$self{branches} ) { > $sth = $dbh->prepare("DELETE FROM borrower_attribute_types_branches WHERE bat_code = ?"); >@@ -323,6 +308,7 @@ sub unique_id { > my $self = shift; > @_ ? $self->{'unique_id'} = ((shift) ? 1 : 0) : $self->{'unique_id'}; > } >+ > =head2 opac_display > > my $opac_display = $attr_type->opac_display(); >@@ -337,20 +323,7 @@ sub opac_display { > my $self = shift; > @_ ? $self->{'opac_display'} = ((shift) ? 1 : 0) : $self->{'opac_display'}; > } >-=head2 password_allowed >- >- my $password_allowed = $attr_type->password_allowed(); >- $attr_type->password_allowed($password_allowed); >- >-Accessor. The C<$password_allowed> argument >-is interpreted as a Perl boolean. > >-=cut >- >-sub password_allowed { >- my $self = shift; >- @_ ? $self->{'password_allowed'} = ((shift) ? 1 : 0) : $self->{'password_allowed'}; >-} > =head2 staff_searchable > > my $staff_searchable = $attr_type->staff_searchable(); >diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm >index eedaae1..1036e58 100644 >--- a/C4/Members/Attributes.pm >+++ b/C4/Members/Attributes.pm >@@ -59,7 +59,6 @@ code (attribute type code) > description (attribute type description) > value (attribute value) > value_description (attribute value description (if associated with an authorised value)) >-password (password, if any, associated with attribute > > If the C<$opac_only> parameter is present and has a true value, only the attributes > marked for OPAC display are returned. >@@ -71,7 +70,7 @@ sub GetBorrowerAttributes { > my $opac_only = @_ ? shift : 0; > > my $dbh = C4::Context->dbh(); >- my $query = "SELECT code, description, attribute, lib, password, display_checkout, category_code, class >+ my $query = "SELECT code, description, attribute, lib, display_checkout, category_code, class > FROM borrower_attributes > JOIN borrower_attribute_types USING (code) > LEFT JOIN authorised_values ON (category = authorised_value_category AND attribute = authorised_value) >@@ -87,7 +86,6 @@ sub GetBorrowerAttributes { > description => $row->{'description'}, > value => $row->{'attribute'}, > value_description => $row->{'lib'}, >- password => $row->{'password'}, > display_checkout => $row->{'display_checkout'}, > category_code => $row->{'category_code'}, > class => $row->{'class'}, >@@ -205,7 +203,7 @@ sub CheckUniqueness { > > =head2 SetBorrowerAttributes > >- SetBorrowerAttributes($borrowernumber, [ { code => 'CODE', value => 'value', password => 'password' }, ... ] ); >+ SetBorrowerAttributes($borrowernumber, [ { code => 'CODE', value => 'value' }, ... ] ); > > Set patron attributes for the patron identified by C<$borrowernumber>, > replacing any that existed previously. >@@ -221,11 +219,10 @@ sub SetBorrowerAttributes { > > DeleteBorrowerAttributes( $borrowernumber, $no_branch_limit ); > >- my $sth = $dbh->prepare("INSERT INTO borrower_attributes (borrowernumber, code, attribute, password) >- VALUES (?, ?, ?, ?)"); >+ my $sth = $dbh->prepare("INSERT INTO borrower_attributes (borrowernumber, code, attribute) >+ VALUES (?, ?, ?)"); > foreach my $attr (@$attr_list) { >- $attr->{password} = undef unless exists $attr->{password}; >- $sth->execute($borrowernumber, $attr->{code}, $attr->{value}, $attr->{password}); >+ $sth->execute($borrowernumber, $attr->{code}, $attr->{value}); > if ($sth->err) { > warn sprintf('Database returned the following error: %s', $sth->errstr); > return; # bail immediately on errors >@@ -301,10 +298,6 @@ sub UpdateBorrowerAttribute { > my $dbh = C4::Context->dbh; > my $query = "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?"; > my @params = ( $attribute->{attribute}, $attribute->{code}, $borrowernumber ); >- if ( defined $attribute->{password} ) { >- $query .= ", password = ?"; >- push @params, $attribute->{password}; >- } > my $sth = $dbh->prepare( $query ); > > $sth->execute( @params ); >diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl >index b233854..3c978e9 100755 >--- a/admin/patron-attr-types.pl >+++ b/admin/patron-attr-types.pl >@@ -113,9 +113,6 @@ sub error_add_attribute_type_form { > if ($input->param('unique_id')) { > $template->param(unique_id_checked => 1); > } >- if ($input->param('password_allowed')) { >- $template->param(password_allowed_checked => 1); >- } > if ($input->param('opac_display')) { > $template->param(opac_display_checked => 1); > } >@@ -167,8 +164,6 @@ sub add_update_attribute_type { > $attr_type->staff_searchable($staff_searchable); > my $authorised_value_category = $input->param('authorised_value_category'); > $attr_type->authorised_value_category($authorised_value_category); >- my $password_allowed = $input->param('password_allowed'); >- $attr_type->password_allowed($password_allowed); > my $display_checkout = $input->param('display_checkout'); > $attr_type->display_checkout($display_checkout); > $attr_type->category_code($input->param('category_code')); >@@ -242,9 +237,6 @@ sub edit_attribute_type_form { > $template->param(unique_id_checked => 1); > } > $template->param(unique_id_disabled => 1); >- if ($attr_type->password_allowed()) { >- $template->param(password_allowed_checked => 1); >- } > if ($attr_type->opac_display()) { > $template->param(opac_display_checked => 1); > } >diff --git a/installer/data/mysql/atomicupdate/bug_12267.perl b/installer/data/mysql/atomicupdate/bug_12267.perl >new file mode 100644 >index 0000000..c9139a7 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_12267.perl >@@ -0,0 +1,18 @@ >+my $dbh = C4::Context->dbh; >+my ( $column_has_been_used ) = $dbh->selectrow_array(q| >+ SELECT COUNT(*) >+ FROM borrower_attributes >+ WHERE password IS NOT NULL >+|); >+ >+if ( $column_has_been_used ) { >+ warn q|WARNING: The columns borrower_attribute_types.password_allowed and borrower_attributes.column have been removed from the Koha codebase. There were not used. However your installation has at least 1 borrower_attributes.password defined. In order not to alter your data, the columns have been kept, please remove them manually if you do not use them this feature.|; >+} else { >+ $dbh->do(q| >+ ALTER TABLE borrower_attribute_types DROP column password_allowed >+ |); >+ $dbh->do(q| >+ ALTER TABLE borrower_attributes DROP column password; >+ |); >+} >+ >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index e6e3142..242755d 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -292,7 +292,6 @@ CREATE TABLE `borrower_attribute_types` ( -- definitions for custom patron field > `repeatable` tinyint(1) NOT NULL default 0, -- defines whether one patron/borrower can have multiple values for this custom field (1 for yes, 0 for no) > `unique_id` tinyint(1) NOT NULL default 0, -- defines if this value needs to be unique (1 for yes, 0 for no) > `opac_display` tinyint(1) NOT NULL default 0, -- defines if this field is visible to patrons on their account in the OPAC (1 for yes, 0 for no) >- `password_allowed` tinyint(1) NOT NULL default 0, -- defines if it is possible to associate a password with this custom field (1 for yes, 0 for no) > `staff_searchable` tinyint(1) NOT NULL default 0, -- defines if this field is searchable via the patron search in the staff client (1 for yes, 0 for no) > `authorised_value_category` varchar(32) default NULL, -- foreign key from authorised_values that links this custom field to an authorized value category > `display_checkout` tinyint(1) NOT NULL default 0,-- defines if this field displays in checkout screens >@@ -311,7 +310,6 @@ CREATE TABLE `borrower_attributes` ( -- values of custom patron fields known as > `borrowernumber` int(11) NOT NULL, -- foreign key from the borrowers table, defines which patron/borrower has this attribute > `code` varchar(10) NOT NULL, -- foreign key from the borrower_attribute_types table, defines which custom field this value was entered for > `attribute` varchar(255) default NULL, -- custom patron field value >- `password` varchar(64) default NULL, -- password associated with this field > KEY `borrowernumber` (`borrowernumber`), > KEY `code_attribute` (`code`, `attribute`), > CONSTRAINT `borrower_attributes_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt >index 3ddb09c..bb6ed53 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt >@@ -139,14 +139,6 @@ function CheckAttributeTypeForm(f) { > <span>If checked, attribute will be a unique identifier — if a value is given to a patron record, the same value > cannot be given to a different record. This setting cannot be changed after an attribute is defined.</span> > </li> >- <li><label for="password_allowed">Allow password: </label> >- [% IF ( password_allowed_checked ) %] >- <input type="checkbox" id="password_allowed" name="password_allowed" checked="checked" /> >- [% ELSE %] >- <input type="checkbox" id="password_allowed" name="password_allowed" /> >- [% END %] >- <span>Check to make it possible to associate a password with this attribute.</span> >- </li> > <li><label for="opac_display">Display in OPAC: </label> > [% IF ( opac_display_checked ) %] > <input type="checkbox" id="opac_display" name="opac_display" checked="checked" /> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >index d54c29e..badb1a5 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >@@ -1060,10 +1060,6 @@ function select_user(borrowernumber, borrower) { > [% ELSE %] > <textarea rows="2" cols="30" id="[% patron_attribute.form_id %]" name="[% patron_attribute.form_id %]">[% patron_attribute.value %]</textarea> > [% END %] >- [% IF ( patron_attribute.password_allowed ) %] >- (<label class="yesno" for="[% patron_attribute.form_id %]_password">Password:</label> <input type="password" maxlength="64" value="[% patron_attribute.password %]" >- id="[% patron_attribute.form_id %]_password" name="[% patron_attribute.form_id %]_password" />) >- [% END %] > <a href="#" onclick="clear_entry(this); return false;"><i class="fa fa-fw fa-trash"></i> Clear</a> > [% IF ( patron_attribute.repeatable ) %] > <a href="#" onclick="clone_entry(this); return false;"><i class="fa fa-fw fa-plus"></i> New</a> >diff --git a/members/memberentry.pl b/members/memberentry.pl >index a190740..b899a26 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -718,11 +718,10 @@ sub parse_extended_patron_attributes { > foreach my $key (@patron_attr) { > my $value = $input->param($key); > next unless defined($value) and $value ne ''; >- my $password = $input->param("${key}_password"); > my $code = $input->param("${key}_code"); > next if exists $dups{$code}->{$value}; > $dups{$code}->{$value} = 1; >- push @attr, { code => $code, value => $value, password => $password }; >+ push @attr, { code => $code, value => $value }; > } > return \@attr; > } >@@ -756,16 +755,13 @@ sub patron_attributes_form { > code => $attr_type->code(), > description => $attr_type->description(), > repeatable => $attr_type->repeatable(), >- password_allowed => $attr_type->password_allowed(), > category => $attr_type->authorised_value_category(), > category_code => $attr_type->category_code(), >- password => '', > }; > if (exists $attr_hash{$attr_type->code()}) { > foreach my $attr (@{ $attr_hash{$attr_type->code()} }) { > my $newentry = { %$entry }; > $newentry->{value} = $attr->{value}; >- $newentry->{password} = $attr->{password}; > $newentry->{use_dropdown} = 0; > if ($attr_type->authorised_value_category()) { > $newentry->{use_dropdown} = 1; >diff --git a/t/Members_AttributeTypes.t b/t/Members_AttributeTypes.t >index 6bb58ae..4b4d5a1 100755 >--- a/t/Members_AttributeTypes.t >+++ b/t/Members_AttributeTypes.t >@@ -48,13 +48,13 @@ fixtures_ok [ > [ > 'code', 'description', > 'repeatable', 'unique_id', >- 'opac_display', 'password_allowed', >+ 'opac_display', > 'staff_searchable', 'authorised_value_category', > 'display_checkout', 'category_code', > 'class' > ], >- [ 'one', 'ISBN', '1', '1', '1', '1', '1', 'red', '1', 'orange', 'green' ], >- [ 'two', 'ISSN', '0', '0', '0', '0', '0', 'blue', '0', 'yellow', 'silver' ] >+ [ 'one', 'ISBN', '1', '1', '1', '1', 'red', '1', 'orange', 'green' ], >+ [ 'two', 'ISSN', '0', '0', '0', '0', 'blue', '0', 'yellow', 'silver' ] > > ], > ], 'add fixtures'; >diff --git a/t/Members_Attributes.t b/t/Members_Attributes.t >index aafd9a9..205d611 100755 >--- a/t/Members_Attributes.t >+++ b/t/Members_Attributes.t >@@ -17,7 +17,6 @@ INIT { > 'opac_display' => '1', > 'staff_searchable' => '1', > 'description' => 'Grade level', >- 'password_allowed' => '0', > 'authorised_value_category' => '', > 'repeatable' => '0', > 'code' => 'grade', >@@ -27,7 +26,6 @@ INIT { > 'opac_display' => '0', > 'staff_searchable' => '1', > 'description' => 'Deans List (annual)', >- 'password_allowed' => '0', > 'authorised_value_category' => '', > 'repeatable' => '1', > 'code' => 'deanslist', >@@ -37,7 +35,6 @@ INIT { > 'opac_display' => '0', > 'staff_searchable' => '0', > 'description' => 'Some Ext. Attribute', >- 'password_allowed' => '0', > 'authorised_value_category' => '', > 'repeatable' => '0', > 'code' => 'somedata', >@@ -47,7 +44,6 @@ INIT { > 'opac_display' => '0', > 'staff_searchable' => '0', > 'description' => 'Another Ext. Attribute', >- 'password_allowed' => '0', > 'authorised_value_category' => '', > 'repeatable' => '0', > 'code' => 'extradata', >@@ -57,7 +53,6 @@ INIT { > 'opac_display' => '1', > 'staff_searchable' => '1', > 'description' => 'School ID Number', >- 'password_allowed' => '0', > 'authorised_value_category' => '', > 'repeatable' => '0', > 'code' => 'school_id', >@@ -67,7 +62,6 @@ INIT { > 'opac_display' => '1', > 'staff_searchable' => '1', > 'description' => 'Homeroom', >- 'password_allowed' => '0', > 'authorised_value_category' => '', > 'repeatable' => '0', > 'code' => 'homeroom', >-- >2.7.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=50122">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12267
:
50122
|
50140
|
50573
|
50574