Bugzilla – Attachment 159891 Details for
Bug 32767
Optionally allow for trimming whitespace from patron attribute values
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates
Bug-32767-New-feature-that-trims-attributes-if-ena.patch (text/plain), 15.53 KB, created by
PTFS Europe Sandboxes
on 2023-12-15 12:46:23 UTC
(
hide
)
Description:
Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates
Filename:
MIME Type:
Creator:
PTFS Europe Sandboxes
Created:
2023-12-15 12:46:23 UTC
Size:
15.53 KB
patch
obsolete
>From 46f5b8ec10cff032230f12926b3887db2607e92f Mon Sep 17 00:00:00 2001 >From: Petro Vashchuk <stalkernoid@gmail.com> >Date: Wed, 18 Jan 2023 15:05:53 +0200 >Subject: [PATCH] Bug 32767: New feature that trims attributes (if enabled) to > avoid duplicates >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. > >This SQL query might help you find all untrimmed attributes & borrowers: >```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` > >Steps to reproduce: >1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) >2. Check the checkbox and save settings. >3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. >4. Check that the trailing whitespaces were trimmed correctly. > >Signed-off-by: Sam Lau <samalau@gmail.com> >Signed-off-by: Anneli Ãsterman <anneli.osterman@koha-suomi.fi> >--- > Koha/Patron/Attribute.pm | 19 +++++ > Koha/Patron/Attribute/Type.pm | 21 ++++++ > Koha/Schema/Result/BorrowerAttributeType.pm | 14 ++++ > admin/patron-attr-types.pl | 10 +++ > .../bug_32767-trim-attr-feature.pl | 14 ++++ > installer/data/mysql/kohastructure.sql | 1 + > .../en/modules/admin/patron-attr-types.tt | 22 ++++++ > members/memberentry.pl | 1 + > opac/opac-memberentry.pl | 1 + > t/db_dependent/Koha/Patron/Attribute.t | 75 ++++++++++++++++++- > 10 files changed, 177 insertions(+), 1 deletion(-) > create mode 100755 installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl > >diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm >index aaaefbcbb8..893f753ba0 100644 >--- a/Koha/Patron/Attribute.pm >+++ b/Koha/Patron/Attribute.pm >@@ -52,6 +52,8 @@ sub store { > Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self ) > unless $self->repeatable_ok(); > >+ $self->do_trim_value_if_needed(); >+ > Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self ) > unless $self->unique_ok(); > >@@ -189,6 +191,23 @@ sub unique_ok { > return $ok; > } > >+=head3 do_trim_value_if_needed >+ >+--- >+ >+=cut >+ >+sub do_trim_value_if_needed { >+ >+ my ($self) = @_; >+ >+ if ( $self->type->trim_value ) { >+ my $value_to_trim = $self->attribute; >+ $value_to_trim =~ s/^\s+|\s+$//g; >+ $self->attribute($value_to_trim); >+ } >+} >+ > =head2 Internal methods > > =head3 _type >diff --git a/Koha/Patron/Attribute/Type.pm b/Koha/Patron/Attribute/Type.pm >index 772cad3c43..7f32f1287a 100644 >--- a/Koha/Patron/Attribute/Type.pm >+++ b/Koha/Patron/Attribute/Type.pm >@@ -46,6 +46,7 @@ sub store { > > $self->check_repeatables; > $self->check_unique_ids; >+ $self->check_untrimmed_values; > > return $self->SUPER::store(); > } >@@ -114,7 +115,27 @@ sub check_unique_ids { > return $self; > } > >+=head3 check_untrimmed_values > >+=cut >+ >+sub check_untrimmed_values { >+ my ($self) = @_; >+ >+ return $self unless $self->trim_value; >+ >+ my $count = $self->attributes->search( >+ { >+ attribute => { -regexp => '^\s+|\s+$' } >+ } >+ )->count; >+ >+ Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty->throw( >+ property => 'trim_value' ) >+ if $count; >+ >+ return $self; >+} > > =head3 _type > >diff --git a/Koha/Schema/Result/BorrowerAttributeType.pm b/Koha/Schema/Result/BorrowerAttributeType.pm >index ad609f0a78..2f4bf10c86 100644 >--- a/Koha/Schema/Result/BorrowerAttributeType.pm >+++ b/Koha/Schema/Result/BorrowerAttributeType.pm >@@ -55,6 +55,14 @@ defines whether one patron/borrower can have multiple values for this custom fie > > defines if this value needs to be unique (1 for yes, 0 for no) > >+=head2 trim_value >+ >+ data_type: 'tinyint' >+ default_value: 0 >+ is_nullable: 0 >+ >+defines if this value needs to be trimmed of whitespaces (1 for yes, 0 for no) >+ > =head2 opac_display > > data_type: 'tinyint' >@@ -148,6 +156,8 @@ __PACKAGE__->add_columns( > { data_type => "tinyint", default_value => 0, is_nullable => 0 }, > "unique_id", > { data_type => "tinyint", default_value => 0, is_nullable => 0 }, >+ "trim_value", >+ { data_type => "tinyint", default_value => 0, is_nullable => 0 }, > "opac_display", > { data_type => "tinyint", default_value => 0, is_nullable => 0 }, > "opac_editable", >@@ -260,6 +270,10 @@ __PACKAGE__->add_columns( > '+staff_searchable' => { is_boolean => 1 }, > ); > >+__PACKAGE__->add_columns( >+ '+trim_value' => { is_boolean => 1 }, >+); >+ > sub koha_object_class { > 'Koha::Patron::Attribute::Type'; > } >diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl >index ca5d2fca47..d3d7e2d60b 100755 >--- a/admin/patron-attr-types.pl >+++ b/admin/patron-attr-types.pl >@@ -114,6 +114,7 @@ sub add_update_attribute_type { > my $description = $input->param('description'); > my $repeatable = $input->param('repeatable') ? 1 : 0; > my $unique_id = $input->param('unique_id') ? 1 : 0; >+ my $trim_value = $input->param('trim_value') ? 1 : 0; > my $opac_display = $input->param('opac_display') ? 1 : 0; > my $opac_editable = $input->param('opac_editable') ? 1 : 0; > my $staff_searchable = $input->param('staff_searchable') ? 1 : 0; >@@ -150,6 +151,7 @@ sub add_update_attribute_type { > { > repeatable => $repeatable, > unique_id => $unique_id, >+ trim_value => $trim_value, > opac_display => $opac_display, > opac_editable => $opac_editable, > staff_searchable => $staff_searchable, >@@ -238,12 +240,20 @@ sub edit_attribute_type_form { > $can_be_set_to_unique = 0 if $@; > $attr_type->unique_id(0); > } >+ my $can_be_trimmed = 1; >+ if ( $attr_type->trim_value == 0 ) { >+ $attr_type->trim_value(1); >+ eval {$attr_type->check_untrimmed_values}; >+ $can_be_trimmed = 0 if $@; >+ $attr_type->trim_value(0); >+ } > $template->param( > attribute_type => $attr_type, > attribute_type_form => 1, > edit_attribute_type => 1, > can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable, > can_be_set_to_unique => $can_be_set_to_unique, >+ can_be_trimmed => $can_be_trimmed, > confirm_op => 'edit_attribute_type_confirmed', > categories => $patron_categories, > ); >diff --git a/installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl b/installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl >new file mode 100755 >index 0000000000..9bda96d54e >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl >@@ -0,0 +1,14 @@ >+use Modern::Perl; >+ >+return { >+ bug_number => "32767", >+ description => "New feature that trims attributes (if enabled) to avoid duplicates", >+ up => sub { >+ my ($args) = @_; >+ my ($dbh, $out) = @$args{qw(dbh out)}; >+ >+ unless ( column_exists( 'borrower_attribute_types', 'trim_value' ) ) { >+ $dbh->do( "ALTER TABLE `borrower_attribute_types` ADD COLUMN `trim_value` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be trimmed of whitespaces (1 for yes, 0 for no)' AFTER unique_id" ); >+ } >+ }, >+}; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 0b59ad8d85..df460ca115 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -1232,6 +1232,7 @@ CREATE TABLE `borrower_attribute_types` ( > `description` varchar(255) NOT NULL COMMENT 'description for each custom field', > `repeatable` tinyint(1) NOT NULL DEFAULT 0 COMMENT '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 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)', >+ `trim_value` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be trimmed of whitespaces (1 for yes, 0 for no)', > `opac_display` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is visible to patrons on their account in the OPAC (1 for yes, 0 for no)', > `opac_editable` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is editable by patrons on their account in the OPAC (1 for yes, 0 for no)', > `staff_searchable` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is searchable via the patron search in the staff interface (1 for yes, 0 for no)', >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 fe792072b1..d533ca1cf0 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 >@@ -147,6 +147,28 @@ > cannot be given to a different record.</span> > </li> > >+ [% IF attribute_type AND attribute_type.trim_value %] >+ <li aria-disabled="true"> >+ [% ELSE %] >+ <li> >+ [% END %] >+ <label for="trim_value">Trim identifier: </label> >+ [% IF attribute_type %] >+ [% IF NOT can_be_trimmed %] >+ <input type="checkbox" id="trim_value" name="trim_value" disabled="disabled" title="At least one patron has attribute with whitespaces at the start or/end of it." /> >+ <input type="hidden" name="trim_value" value="0" /> >+ <span class="hint">At least one patron has attribute with whitespaces at the start or/end of it. Please fix that.</span> >+ [% ELSIF attribute_type.trim_value %] >+ <input type="checkbox" id="trim_value" name="trim_value" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" id="trim_value" name="trim_value" /> >+ [% END %] >+ [% ELSE %] >+ <input type="checkbox" id="trim_value" name="trim_value" /> >+ [% END %] >+ <span class="hint">Check to always trim whitespaces around the identifier to avoid creating duplicates.</span> >+ </li> >+ > <li><label for="opac_display">Display in OPAC: </label> > [% IF attribute_type AND attribute_type.opac_display %] > <input type="checkbox" id="opac_display" name="opac_display" checked="checked" /> >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 57e780cfec..f2351b9f0b 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -391,6 +391,7 @@ if ($op eq 'save' || $op eq 'insert'){ > for my $attr ( @$extended_patron_attributes ) { > $attr->{borrowernumber} = $borrowernumber if $borrowernumber; > my $attribute = Koha::Patron::Attribute->new($attr); >+ $attribute->do_trim_value_if_needed; > if ( !$attribute->unique_ok ) { > push @errors, "ERROR_extended_unique_id_failed"; > my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index a0f5a3ace2..3f87ac61c9 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -117,6 +117,7 @@ my $conflicting_attribute = 0; > > foreach my $attr (@$attributes) { > my $attribute = Koha::Patron::Attribute->new($attr); >+ $attribute->do_trim_value_if_needed; > if ( !$attribute->unique_ok ) { > my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); > $template->param( >diff --git a/t/db_dependent/Koha/Patron/Attribute.t b/t/db_dependent/Koha/Patron/Attribute.t >index a24de0fb2d..46c6d0912a 100755 >--- a/t/db_dependent/Koha/Patron/Attribute.t >+++ b/t/db_dependent/Koha/Patron/Attribute.t >@@ -33,7 +33,7 @@ my $builder = t::lib::TestBuilder->new; > > subtest 'store() tests' => sub { > >- plan tests => 4; >+ plan tests => 5; > > subtest 'repeatable attributes tests' => sub { > >@@ -179,6 +179,79 @@ subtest 'store() tests' => sub { > $schema->storage->txn_rollback; > }; > >+subtest 'trim identifier tests' => sub { >+ >+ plan tests => 5; >+ >+ $schema->storage->txn_begin; >+ >+ my $patron_1 = $builder->build( { source => 'Borrower' } )->{borrowernumber}; >+ my $patron_2 = $builder->build( { source => 'Borrower' } )->{borrowernumber}; >+ >+ my $attribute_type_1 = $builder->build( >+ { source => 'BorrowerAttributeType', >+ value => { unique_id => 1, trim_value => 0 } >+ } >+ ); >+ Koha::Patron::Attribute->new( >+ { borrowernumber => $patron_1, >+ code => $attribute_type_1->{code}, >+ attribute => '123' >+ } >+ )->store; >+ Koha::Patron::Attribute->new( >+ { borrowernumber => $patron_2, >+ code => $attribute_type_1->{code}, >+ attribute => ' 123' >+ } >+ )->store; >+ my $attr_count >+ = Koha::Patron::Attributes->search( >+ { code => $attribute_type_1->{code} } ) >+ ->count; >+ is( $attr_count, 2, >+ 'Two identical attributes stored and retrieved correctly because one of them had whitespace at the start' ); >+ >+ my $attribute_type_2 = $builder->build( >+ { source => 'BorrowerAttributeType', >+ value => { unique_id => 1, trim_value => 1 } >+ } >+ ); >+ >+ Koha::Patron::Attribute->new( >+ { borrowernumber => $patron_1, >+ code => $attribute_type_2->{code}, >+ attribute => '123' >+ } >+ )->store; >+ throws_ok { >+ Koha::Patron::Attribute->new( >+ { borrowernumber => $patron_2, >+ code => $attribute_type_2->{code}, >+ attribute => ' 123' >+ } >+ )->store; >+ } >+ 'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint', >+ 'Exception thrown trying to store more than one unique attribute now that we enabled trim_value'; >+ >+ is( >+ "$@", >+ "Your action breaks a unique constraint on the attribute. type=" >+ . $attribute_type_2->{code} >+ . " value=123", >+ 'Exception stringified correctly, attribute passed correctly' >+ ); >+ >+ my $attributes = Koha::Patron::Attributes->search( >+ { borrowernumber => $patron_1, code => $attribute_type_2->{code} } ); >+ is( $attributes->count, 1, '1 unique attribute stored' ); >+ is( $attributes->next->attribute, >+ '123', 'our unique attribute remains unchanged' ); >+ >+ $schema->storage->txn_rollback; >+ }; >+ > subtest 'invalid type tests' => sub { > > plan tests => 2; >-- >2.30.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=159891">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 32767
:
145846
|
151540
|
153651
|
153678
|
156985
|
159891
|
170815