View | Details | Raw Unified | Return to bug 32767
Collapse All | Expand All

(-)a/Koha/Patron/Attribute.pm (+19 lines)
Lines 52-57 sub store { Link Here
52
    Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self )
52
    Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self )
53
        unless $self->repeatable_ok();
53
        unless $self->repeatable_ok();
54
54
55
    $self->do_trim_value_if_needed();
56
55
    Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self )
57
    Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self )
56
        unless $self->unique_ok();
58
        unless $self->unique_ok();
57
59
Lines 189-194 sub unique_ok { Link Here
189
    return $ok;
191
    return $ok;
190
}
192
}
191
193
194
=head3 do_trim_value_if_needed
195
196
---
197
198
=cut
199
200
sub do_trim_value_if_needed {
201
202
    my ($self) = @_;
203
204
    if ( $self->type->trim_value ) {
205
        my $value_to_trim = $self->attribute;
206
        $value_to_trim =~ s/^\s+|\s+$//g;
207
        $self->attribute($value_to_trim);
208
    }
209
}
210
192
=head2 Internal methods
211
=head2 Internal methods
193
212
194
=head3 _type
213
=head3 _type
(-)a/Koha/Patron/Attribute/Type.pm (+21 lines)
Lines 46-51 sub store { Link Here
46
46
47
    $self->check_repeatables;
47
    $self->check_repeatables;
48
    $self->check_unique_ids;
48
    $self->check_unique_ids;
49
    $self->check_untrimmed_values;
49
50
50
    return $self->SUPER::store();
51
    return $self->SUPER::store();
51
}
52
}
Lines 114-120 sub check_unique_ids { Link Here
114
    return $self;
115
    return $self;
115
}
116
}
116
117
118
=head3 check_untrimmed_values
117
119
120
=cut
121
122
sub check_untrimmed_values {
123
    my ($self) = @_;
124
125
    return $self unless $self->trim_value;
126
127
    my $count = $self->attributes->search(
128
        {
129
            attribute => { -regexp => '^\s+|\s+$' }
130
        }
131
    )->count;
132
133
        Koha::Exceptions::Patron::Attribute::Type::CannotChangeProperty->throw(
134
        property => 'trim_value' )
135
      if $count;
136
137
    return $self;
138
}
118
139
119
=head3 _type
140
=head3 _type
120
141
(-)a/Koha/Schema/Result/BorrowerAttributeType.pm (+14 lines)
Lines 55-60 defines whether one patron/borrower can have multiple values for this custom fie Link Here
55
55
56
defines if this value needs to be unique (1 for yes, 0 for no)
56
defines if this value needs to be unique (1 for yes, 0 for no)
57
57
58
=head2 trim_value
59
60
  data_type: 'tinyint'
61
  default_value: 0
62
  is_nullable: 0
63
64
defines if this value needs to be trimmed of whitespaces (1 for yes, 0 for no)
65
58
=head2 opac_display
66
=head2 opac_display
59
67
60
  data_type: 'tinyint'
68
  data_type: 'tinyint'
Lines 140-145 __PACKAGE__->add_columns( Link Here
140
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
148
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
141
  "unique_id",
149
  "unique_id",
142
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
150
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
151
  "trim_value",
152
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
143
  "opac_display",
153
  "opac_display",
144
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
154
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
145
  "opac_editable",
155
  "opac_editable",
Lines 251-256 __PACKAGE__->add_columns( Link Here
251
    '+mandatory' => { is_boolean => 1 },
261
    '+mandatory' => { is_boolean => 1 },
252
);
262
);
253
263
264
__PACKAGE__->add_columns(
265
    '+trim_value' => { is_boolean => 1 },
266
);
267
254
sub koha_object_class {
268
sub koha_object_class {
255
    'Koha::Patron::Attribute::Type';
269
    'Koha::Patron::Attribute::Type';
256
}
270
}
(-)a/admin/patron-attr-types.pl (+10 lines)
Lines 114-119 sub add_update_attribute_type { Link Here
114
    my $description               = $input->param('description');
114
    my $description               = $input->param('description');
115
    my $repeatable                = $input->param('repeatable') ? 1 : 0;
115
    my $repeatable                = $input->param('repeatable') ? 1 : 0;
116
    my $unique_id                 = $input->param('unique_id') ? 1 : 0;
116
    my $unique_id                 = $input->param('unique_id') ? 1 : 0;
117
    my $trim_value                = $input->param('trim_value') ? 1 : 0;
117
    my $opac_display              = $input->param('opac_display') ? 1 : 0;
118
    my $opac_display              = $input->param('opac_display') ? 1 : 0;
118
    my $opac_editable             = $input->param('opac_editable') ? 1 : 0;
119
    my $opac_editable             = $input->param('opac_editable') ? 1 : 0;
119
    my $staff_searchable          = $input->param('staff_searchable') ? 1 : 0;
120
    my $staff_searchable          = $input->param('staff_searchable') ? 1 : 0;
Lines 149-154 sub add_update_attribute_type { Link Here
149
        {
150
        {
150
            repeatable                => $repeatable,
151
            repeatable                => $repeatable,
151
            unique_id                 => $unique_id,
152
            unique_id                 => $unique_id,
153
            trim_value                => $trim_value,
152
            opac_display              => $opac_display,
154
            opac_display              => $opac_display,
153
            opac_editable             => $opac_editable,
155
            opac_editable             => $opac_editable,
154
            staff_searchable          => $staff_searchable,
156
            staff_searchable          => $staff_searchable,
Lines 236-247 sub edit_attribute_type_form { Link Here
236
        $can_be_set_to_unique = 0 if $@;
238
        $can_be_set_to_unique = 0 if $@;
237
        $attr_type->unique_id(0);
239
        $attr_type->unique_id(0);
238
    }
240
    }
241
    my $can_be_trimmed = 1;
242
    if ( $attr_type->trim_value == 0 ) {
243
        $attr_type->trim_value(1);
244
        eval {$attr_type->check_untrimmed_values};
245
        $can_be_trimmed = 0 if $@;
246
        $attr_type->trim_value(0);
247
    }
239
    $template->param(
248
    $template->param(
240
        attribute_type => $attr_type,
249
        attribute_type => $attr_type,
241
        attribute_type_form => 1,
250
        attribute_type_form => 1,
242
        edit_attribute_type => 1,
251
        edit_attribute_type => 1,
243
        can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable,
252
        can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable,
244
        can_be_set_to_unique => $can_be_set_to_unique,
253
        can_be_set_to_unique => $can_be_set_to_unique,
254
        can_be_trimmed => $can_be_trimmed,
245
        confirm_op => 'edit_attribute_type_confirmed',
255
        confirm_op => 'edit_attribute_type_confirmed',
246
        categories => $patron_categories,
256
        categories => $patron_categories,
247
    );
257
    );
(-)a/installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl (+14 lines)
Line 0 Link Here
1
use Modern::Perl;
2
3
return {
4
    bug_number => "32767",
5
    description => "New feature that trims attributes (if enabled) to avoid duplicates",
6
    up => sub {
7
        my ($args) = @_;
8
        my ($dbh, $out) = @$args{qw(dbh out)};
9
10
        unless ( column_exists( 'borrower_attribute_types', 'trim_value' ) ) {
11
            $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" );
12
        }
13
    },
14
};
(-)a/installer/data/mysql/kohastructure.sql (+1 lines)
Lines 1190-1195 CREATE TABLE `borrower_attribute_types` ( Link Here
1190
  `description` varchar(255) NOT NULL COMMENT 'description for each custom field',
1190
  `description` varchar(255) NOT NULL COMMENT 'description for each custom field',
1191
  `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)',
1191
  `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)',
1192
  `unique_id` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)',
1192
  `unique_id` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)',
1193
  `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)',
1193
  `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)',
1194
  `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)',
1194
  `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)',
1195
  `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)',
1195
  `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)',
1196
  `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)',
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt (+22 lines)
Lines 147-152 Link Here
147
                  cannot be given to a different record.</span>
147
                  cannot be given to a different record.</span>
148
        </li>
148
        </li>
149
149
150
        [% IF attribute_type AND attribute_type.trim_value %]
151
            <li aria-disabled="true">
152
        [% ELSE %]
153
            <li>
154
        [% END %]
155
            <label for="trim_value">Trim identifier: </label>
156
            [% IF attribute_type %]
157
                [% IF NOT can_be_trimmed %]
158
                    <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." />
159
                    <input type="hidden" name="trim_value" value="0" />
160
                    <span class="hint">At least one patron has attribute with whitespaces at the start or/end of it. Please fix that.</span>
161
                [% ELSIF attribute_type.trim_value %]
162
                    <input type="checkbox" id="trim_value" name="trim_value" checked="checked" />
163
                [% ELSE %]
164
                    <input type="checkbox" id="trim_value" name="trim_value" />
165
                [% END %]
166
            [% ELSE %]
167
                <input type="checkbox" id="trim_value" name="trim_value" />
168
            [% END %]
169
            <span class="hint">Check to always trim whitespaces around the identifier to avoid creating duplicates.</span>
170
        </li>
171
150
       <li><label for="opac_display">Display in OPAC: </label>
172
       <li><label for="opac_display">Display in OPAC: </label>
151
          [% IF attribute_type AND attribute_type.opac_display %]
173
          [% IF attribute_type AND attribute_type.opac_display %]
152
            <input type="checkbox" id="opac_display" name="opac_display" checked="checked" />
174
            <input type="checkbox" id="opac_display" name="opac_display" checked="checked" />
(-)a/members/memberentry.pl (+1 lines)
Lines 354-359 if ($op eq 'save' || $op eq 'insert'){ Link Here
354
      for my $attr ( @$extended_patron_attributes ) {
354
      for my $attr ( @$extended_patron_attributes ) {
355
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
355
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
356
          my $attribute = Koha::Patron::Attribute->new($attr);
356
          my $attribute = Koha::Patron::Attribute->new($attr);
357
          $attribute->do_trim_value_if_needed;
357
          if ( !$attribute->unique_ok ) {
358
          if ( !$attribute->unique_ok ) {
358
              push @errors, "ERROR_extended_unique_id_failed";
359
              push @errors, "ERROR_extended_unique_id_failed";
359
              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
360
              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
(-)a/opac/opac-memberentry.pl (+1 lines)
Lines 114-119 my $conflicting_attribute = 0; Link Here
114
114
115
foreach my $attr (@$attributes) {
115
foreach my $attr (@$attributes) {
116
    my $attribute = Koha::Patron::Attribute->new($attr);
116
    my $attribute = Koha::Patron::Attribute->new($attr);
117
    $attribute->do_trim_value_if_needed;
117
    if ( !$attribute->unique_ok ) {
118
    if ( !$attribute->unique_ok ) {
118
        my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
119
        my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
119
        $template->param(
120
        $template->param(
(-)a/t/db_dependent/Koha/Patron/Attribute.t (-2 / +74 lines)
Lines 33-39 my $builder = t::lib::TestBuilder->new; Link Here
33
33
34
subtest 'store() tests' => sub {
34
subtest 'store() tests' => sub {
35
35
36
    plan tests => 4;
36
    plan tests => 5;
37
37
38
    subtest 'repeatable attributes tests' => sub {
38
    subtest 'repeatable attributes tests' => sub {
39
39
Lines 179-184 subtest 'store() tests' => sub { Link Here
179
        $schema->storage->txn_rollback;
179
        $schema->storage->txn_rollback;
180
    };
180
    };
181
181
182
subtest 'trim identifier tests' => sub {
183
184
        plan tests => 5;
185
186
        $schema->storage->txn_begin;
187
188
        my $patron_1 = $builder->build( { source => 'Borrower' } )->{borrowernumber};
189
        my $patron_2 = $builder->build( { source => 'Borrower' } )->{borrowernumber};
190
191
        my $attribute_type_1 = $builder->build(
192
            {   source => 'BorrowerAttributeType',
193
                value  => { unique_id => 1, trim_value => 0 }
194
            }
195
        );
196
        Koha::Patron::Attribute->new(
197
            {   borrowernumber => $patron_1,
198
                code           => $attribute_type_1->{code},
199
                attribute      => '123'
200
            }
201
        )->store;
202
        Koha::Patron::Attribute->new(
203
            {   borrowernumber => $patron_2,
204
                code           => $attribute_type_1->{code},
205
                attribute      => ' 123'
206
            }
207
        )->store;
208
        my $attr_count
209
            = Koha::Patron::Attributes->search(
210
            { code => $attribute_type_1->{code} } )
211
            ->count;
212
        is( $attr_count, 2,
213
            'Two identical attributes stored and retrieved correctly because one of them had whitespace at the start' );
214
215
        my $attribute_type_2 = $builder->build(
216
            {   source => 'BorrowerAttributeType',
217
                value  => { unique_id => 1, trim_value => 1 }
218
            }
219
        );
220
221
        Koha::Patron::Attribute->new(
222
            {   borrowernumber => $patron_1,
223
                code           => $attribute_type_2->{code},
224
                attribute      => '123'
225
            }
226
        )->store;
227
        throws_ok {
228
            Koha::Patron::Attribute->new(
229
                {   borrowernumber => $patron_2,
230
                    code           => $attribute_type_2->{code},
231
                    attribute      => ' 123'
232
                }
233
            )->store;
234
        }
235
        'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint',
236
            'Exception thrown trying to store more than one unique attribute now that we enabled trim_value';
237
238
        is(
239
            "$@",
240
            "Your action breaks a unique constraint on the attribute. type="
241
            . $attribute_type_2->{code}
242
            . " value=123",
243
            'Exception stringified correctly, attribute passed correctly'
244
        );
245
246
        my $attributes = Koha::Patron::Attributes->search(
247
            { borrowernumber => $patron_1, code => $attribute_type_2->{code} } );
248
        is( $attributes->count, 1, '1 unique attribute stored' );
249
        is( $attributes->next->attribute,
250
            '123', 'our unique attribute remains unchanged' );
251
252
        $schema->storage->txn_rollback;
253
    };
254
182
    subtest 'invalid type tests' => sub {
255
    subtest 'invalid type tests' => sub {
183
256
184
        plan tests => 2;
257
        plan tests => 2;
185
- 

Return to bug 32767