Attachment 145846 Details for Bug 32767 – Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates

[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), 11.86 KB, created by Peter Vashchuk on 2023-01-31 12:20:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Peter Vashchuk

Created: 2023-01-31 12:20:30 UTC

Size: 11.86 KB

patch

obsolete

>From 45ea3f7d4ad4e1133fb47eea9d456bc5ed68b663 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
>
>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.
>---
> Koha/Patron/Attribute.pm                      | 19 ++++++++++++++++
> Koha/Patron/Attribute/Type.pm                 | 21 ++++++++++++++++++
> Koha/Schema/Result/BorrowerAttributeType.pm   | 10 +++++++++
> 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 +
> 9 files changed, 99 insertions(+)
> 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..9e4f67b377 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 9a4b7125ce..2341f99094 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'
>@@ -140,6 +148,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",
>diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl
>index 2cf2bf6f3b..08df74cf6b 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;
>@@ -149,6 +150,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,
>@@ -236,12 +238,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 59607073ca..34f53e9e18 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -1123,6 +1123,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 3c26281a29..99a3e4207d 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
>@@ -154,6 +154,28 @@ Patron attribute types &rsaquo; Administration &rsaquo; Koha
>                   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 2ebf3b4335..5b0a11772f 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -394,6 +394,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 0115d78f98..1f071750d1 100755
>--- a/opac/opac-memberentry.pl
>+++ b/opac/opac-memberentry.pl
>@@ -111,6 +111,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(
>-- 
>2.38.1

Actions: View | Diff | Splinter Review

Attachments on bug 32767: 145846 | 151540 | 153651 | 153678 | 156985 | 159891 | 170815