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 148-153 __PACKAGE__->add_columns( Link Here
148
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
156
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
149
  "unique_id",
157
  "unique_id",
150
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
158
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
159
  "trim_value",
160
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
151
  "opac_display",
161
  "opac_display",
152
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
162
  { data_type => "tinyint", default_value => 0, is_nullable => 0 },
153
  "opac_editable",
163
  "opac_editable",
Lines 260-265 __PACKAGE__->add_columns( Link Here
260
    '+staff_searchable'          => { is_boolean => 1 },
270
    '+staff_searchable'          => { is_boolean => 1 },
261
);
271
);
262
272
273
__PACKAGE__->add_columns(
274
    '+trim_value' => { is_boolean => 1 },
275
);
276
263
sub koha_object_class {
277
sub koha_object_class {
264
    'Koha::Patron::Attribute::Type';
278
    'Koha::Patron::Attribute::Type';
265
}
279
}
(-)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 150-155 sub add_update_attribute_type { Link Here
150
        {
151
        {
151
            repeatable                => $repeatable,
152
            repeatable                => $repeatable,
152
            unique_id                 => $unique_id,
153
            unique_id                 => $unique_id,
154
            trim_value                => $trim_value,
153
            opac_display              => $opac_display,
155
            opac_display              => $opac_display,
154
            opac_editable             => $opac_editable,
156
            opac_editable             => $opac_editable,
155
            staff_searchable          => $staff_searchable,
157
            staff_searchable          => $staff_searchable,
Lines 238-249 sub edit_attribute_type_form { Link Here
238
        $can_be_set_to_unique = 0 if $@;
240
        $can_be_set_to_unique = 0 if $@;
239
        $attr_type->unique_id(0);
241
        $attr_type->unique_id(0);
240
    }
242
    }
243
    my $can_be_trimmed = 1;
244
    if ( $attr_type->trim_value == 0 ) {
245
        $attr_type->trim_value(1);
246
        eval {$attr_type->check_untrimmed_values};
247
        $can_be_trimmed = 0 if $@;
248
        $attr_type->trim_value(0);
249
    }
241
    $template->param(
250
    $template->param(
242
        attribute_type => $attr_type,
251
        attribute_type => $attr_type,
243
        attribute_type_form => 1,
252
        attribute_type_form => 1,
244
        edit_attribute_type => 1,
253
        edit_attribute_type => 1,
245
        can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable,
254
        can_be_set_to_nonrepeatable => $can_be_set_to_nonrepeatable,
246
        can_be_set_to_unique => $can_be_set_to_unique,
255
        can_be_set_to_unique => $can_be_set_to_unique,
256
        can_be_trimmed => $can_be_trimmed,
247
        confirm_op => 'edit_attribute_type_confirmed',
257
        confirm_op => 'edit_attribute_type_confirmed',
248
        categories => $patron_categories,
258
        categories => $patron_categories,
249
    );
259
    );
(-)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 1232-1237 CREATE TABLE `borrower_attribute_types` ( Link Here
1232
  `description` varchar(255) NOT NULL COMMENT 'description for each custom field',
1232
  `description` varchar(255) NOT NULL COMMENT 'description for each custom field',
1233
  `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)',
1233
  `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)',
1234
  `unique_id` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)',
1234
  `unique_id` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'defines if this value needs to be unique (1 for yes, 0 for no)',
1235
  `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)',
1235
  `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)',
1236
  `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)',
1236
  `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)',
1237
  `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)',
1237
  `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)',
1238
  `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 391-396 if ($op eq 'save' || $op eq 'insert'){ Link Here
391
      for my $attr ( @$extended_patron_attributes ) {
391
      for my $attr ( @$extended_patron_attributes ) {
392
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
392
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
393
          my $attribute = Koha::Patron::Attribute->new($attr);
393
          my $attribute = Koha::Patron::Attribute->new($attr);
394
          $attribute->do_trim_value_if_needed;
394
          if ( !$attribute->unique_ok ) {
395
          if ( !$attribute->unique_ok ) {
395
              push @errors, "ERROR_extended_unique_id_failed";
396
              push @errors, "ERROR_extended_unique_id_failed";
396
              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
397
              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 (-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