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

(-)a/Koha/Patron/Attribute.pm (+19 lines)
Lines 53-58 sub store { Link Here
53
    Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self )
53
    Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self )
54
        unless $self->repeatable_ok();
54
        unless $self->repeatable_ok();
55
55
56
    $self->do_trim_value_if_needed();
57
56
    Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self )
58
    Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self )
57
        unless $self->unique_ok();
59
        unless $self->unique_ok();
58
60
Lines 212-217 sub value_ok { Link Here
212
    return $ok;
214
    return $ok;
213
}
215
}
214
216
217
=head3 do_trim_value_if_needed
218
219
---
220
221
=cut
222
223
sub do_trim_value_if_needed {
224
225
    my ($self) = @_;
226
227
    if ( $self->type->trim_value ) {
228
        my $value_to_trim = $self->attribute;
229
        $value_to_trim =~ s/^\s+|\s+$//g;
230
        $self->attribute($value_to_trim);
231
    }
232
}
233
215
=head2 Internal methods
234
=head2 Internal methods
216
235
217
=head3 _type
236
=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 (+11 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 is_date
66
=head2 is_date
59
67
60
  data_type: 'tinyint'
68
  data_type: 'tinyint'
Lines 156-161 __PACKAGE__->add_columns( Link Here
156
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
164
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
157
  "unique_id",
165
  "unique_id",
158
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
166
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
167
  "trim_value",
168
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
159
  "is_date",
169
  "is_date",
160
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
170
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
161
  "opac_display",
171
  "opac_display",
Lines 271-276 __PACKAGE__->add_columns( Link Here
271
);
281
);
272
282
273
__PACKAGE__->add_columns(
283
__PACKAGE__->add_columns(
284
    '+trim_value' => { is_boolean => 1 },
274
    '+is_date' => { is_boolean => 1 },
285
    '+is_date' => { is_boolean => 1 },
275
);
286
);
276
287
(-)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 $is_date                   = $input->param('is_date') ? 1 : 0;
118
    my $is_date                   = $input->param('is_date') ? 1 : 0;
118
    my $opac_display              = $input->param('opac_display') ? 1 : 0;
119
    my $opac_display              = $input->param('opac_display') ? 1 : 0;
119
    my $opac_editable             = $input->param('opac_editable') ? 1 : 0;
120
    my $opac_editable             = $input->param('opac_editable') ? 1 : 0;
Lines 151-156 sub add_update_attribute_type { Link Here
151
        {
152
        {
152
            repeatable                => $repeatable,
153
            repeatable                => $repeatable,
153
            unique_id                 => $unique_id,
154
            unique_id                 => $unique_id,
155
            trim_value                => $trim_value,
154
            is_date                   => $is_date,
156
            is_date                   => $is_date,
155
            opac_display              => $opac_display,
157
            opac_display              => $opac_display,
156
            opac_editable             => $opac_editable,
158
            opac_editable             => $opac_editable,
Lines 240-251 sub edit_attribute_type_form { Link Here
240
        $can_be_set_to_unique = 0 if $@;
242
        $can_be_set_to_unique = 0 if $@;
241
        $attr_type->unique_id(0);
243
        $attr_type->unique_id(0);
242
    }
244
    }
245
    my $can_be_trimmed = 1;
246
    if ( $attr_type->trim_value == 0 ) {
247
        $attr_type->trim_value(1);
248
        eval {$attr_type->check_untrimmed_values};
249
        $can_be_trimmed = 0 if $@;
250
        $attr_type->trim_value(0);
251
    }
243
    $template->param(
252
    $template->param(
244
        attribute_type => $attr_type,
253
        attribute_type => $attr_type,
245
        attribute_type_form => 1,
254
        attribute_type_form => 1,
246
        edit_attribute_type => 1,
255
        edit_attribute_type => 1,
247
        can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable,
256
        can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable,
248
        can_be_set_to_unique => $can_be_set_to_unique,
257
        can_be_set_to_unique => $can_be_set_to_unique,
258
        can_be_trimmed => $can_be_trimmed,
249
        confirm_op => 'cud-edit_attribute_type_confirmed',
259
        confirm_op => 'cud-edit_attribute_type_confirmed',
250
        categories => $patron_categories,
260
        categories => $patron_categories,
251
    );
261
    );
(-)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 1241-1246 CREATE TABLE `borrower_attribute_types` ( Link Here
1241
  `description` varchar(255) NOT NULL COMMENT 'description for each custom field',
1241
  `description` varchar(255) NOT NULL COMMENT 'description for each custom field',
1242
  `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)',
1242
  `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)',
1243
  `unique_id` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)',
1243
  `unique_id` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)',
1244
  `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)',
1244
  `is_date` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is displayed as a date',
1245
  `is_date` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this field is displayed as a date',
1245
  `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)',
1246
  `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)',
1246
  `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)',
1247
  `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)',
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt (+22 lines)
Lines 148-153 Link Here
148
            <span class="hint">If checked, attribute will be a unique identifier. If a value is given to a patron record, the same value
148
            <span class="hint">If checked, attribute will be a unique identifier. If a value is given to a patron record, the same value
149
                  cannot be given to a different record.</span>
149
                  cannot be given to a different record.</span>
150
        </li>
150
        </li>
151
152
        [% IF attribute_type AND attribute_type.trim_value %]
153
            <li aria-disabled="true">
154
        [% ELSE %]
155
            <li>
156
        [% END %]
157
            <label for="trim_value">Trim identifier: </label>
158
            [% IF attribute_type %]
159
                [% IF NOT can_be_trimmed %]
160
                    <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." />
161
                    <input type="hidden" name="trim_value" value="0" />
162
                    <span class="hint">At least one patron has attribute with whitespaces at the start or/end of it. Please fix that.</span>
163
                [% ELSIF attribute_type.trim_value %]
164
                    <input type="checkbox" id="trim_value" name="trim_value" checked="checked" />
165
                [% ELSE %]
166
                    <input type="checkbox" id="trim_value" name="trim_value" />
167
                [% END %]
168
            [% ELSE %]
169
                <input type="checkbox" id="trim_value" name="trim_value" />
170
            [% END %]
171
            <span class="hint">Check to always trim whitespaces around the identifier to avoid creating duplicates.</span>
172
        </li>
151
        <li><label for="is_date">Is a date: </label>
173
        <li><label for="is_date">Is a date: </label>
152
            [% IF attribute_type AND attribute_type.is_date %]
174
            [% IF attribute_type AND attribute_type.is_date %]
153
                <input type="checkbox" id="is_date" name="is_date" checked="checked" />
175
                <input type="checkbox" id="is_date" name="is_date" checked="checked" />
(-)a/members/memberentry.pl (+1 lines)
Lines 382-387 if ($op eq 'cud-save' || $op eq 'cud-insert'){ Link Here
382
      for my $attr ( @$extended_patron_attributes ) {
382
      for my $attr ( @$extended_patron_attributes ) {
383
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
383
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
384
          my $attribute = Koha::Patron::Attribute->new($attr);
384
          my $attribute = Koha::Patron::Attribute->new($attr);
385
          $attribute->do_trim_value_if_needed;
385
          if ( !$attribute->unique_ok ) {
386
          if ( !$attribute->unique_ok ) {
386
              push @errors, "ERROR_extended_unique_id_failed";
387
              push @errors, "ERROR_extended_unique_id_failed";
387
              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
388
              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
(-)a/opac/opac-memberentry.pl (+1 lines)
Lines 117-122 my $conflicting_attribute = 0; Link Here
117
117
118
foreach my $attr (@$attributes) {
118
foreach my $attr (@$attributes) {
119
    my $attribute = Koha::Patron::Attribute->new($attr);
119
    my $attribute = Koha::Patron::Attribute->new($attr);
120
    $attribute->do_trim_value_if_needed;
120
    if ( !$attribute->unique_ok ) {
121
    if ( !$attribute->unique_ok ) {
121
        my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
122
        my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
122
        $template->param(
123
        $template->param(
(-)a/t/db_dependent/Koha/Patron/Attribute.t (-1 / +73 lines)
Lines 232-237 subtest 'store() tests' => sub { Link Here
232
        $schema->storage->txn_rollback;
232
        $schema->storage->txn_rollback;
233
    };
233
    };
234
234
235
subtest 'trim identifier tests' => sub {
236
237
        plan tests => 5;
238
239
        $schema->storage->txn_begin;
240
241
        my $patron_1 = $builder->build( { source => 'Borrower' } )->{borrowernumber};
242
        my $patron_2 = $builder->build( { source => 'Borrower' } )->{borrowernumber};
243
244
        my $attribute_type_1 = $builder->build(
245
            {   source => 'BorrowerAttributeType',
246
                value  => { unique_id => 1, trim_value => 0 }
247
            }
248
        );
249
        Koha::Patron::Attribute->new(
250
            {   borrowernumber => $patron_1,
251
                code           => $attribute_type_1->{code},
252
                attribute      => '123'
253
            }
254
        )->store;
255
        Koha::Patron::Attribute->new(
256
            {   borrowernumber => $patron_2,
257
                code           => $attribute_type_1->{code},
258
                attribute      => ' 123'
259
            }
260
        )->store;
261
        my $attr_count
262
            = Koha::Patron::Attributes->search(
263
            { code => $attribute_type_1->{code} } )
264
            ->count;
265
        is( $attr_count, 2,
266
            'Two identical attributes stored and retrieved correctly because one of them had whitespace at the start' );
267
268
        my $attribute_type_2 = $builder->build(
269
            {   source => 'BorrowerAttributeType',
270
                value  => { unique_id => 1, trim_value => 1 }
271
            }
272
        );
273
274
        Koha::Patron::Attribute->new(
275
            {   borrowernumber => $patron_1,
276
                code           => $attribute_type_2->{code},
277
                attribute      => '123'
278
            }
279
        )->store;
280
        throws_ok {
281
            Koha::Patron::Attribute->new(
282
                {   borrowernumber => $patron_2,
283
                    code           => $attribute_type_2->{code},
284
                    attribute      => ' 123'
285
                }
286
            )->store;
287
        }
288
        'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint',
289
            'Exception thrown trying to store more than one unique attribute now that we enabled trim_value';
290
291
        is(
292
            "$@",
293
            "Your action breaks a unique constraint on the attribute. type="
294
            . $attribute_type_2->{code}
295
            . " value=123",
296
            'Exception stringified correctly, attribute passed correctly'
297
        );
298
299
        my $attributes = Koha::Patron::Attributes->search(
300
            { borrowernumber => $patron_1, code => $attribute_type_2->{code} } );
301
        is( $attributes->count, 1, '1 unique attribute stored' );
302
        is( $attributes->next->attribute,
303
            '123', 'our unique attribute remains unchanged' );
304
305
        $schema->storage->txn_rollback;
306
    };
307
235
    subtest 'invalid type tests' => sub {
308
    subtest 'invalid type tests' => sub {
236
309
237
        plan tests => 2;
310
        plan tests => 2;
238
- 

Return to bug 32767