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

(-)a/Koha/Exceptions/Category.pm (-17 lines)
Lines 1-17 Link Here
1
package Koha::Exceptions::Category;
2
3
use Modern::Perl;
4
5
use Exception::Class (
6
7
    'Koha::Exceptions::Category' => {
8
        description => 'Something went wrong!',
9
    },
10
    'Koha::Exceptions::Category::CategorycodeNotFound' => {
11
        isa => 'Koha::Exceptions::Category',
12
        description => "Category does not exist",
13
        fields => ["categorycode"],
14
    },
15
);
16
17
1;
(-)a/Koha/Exceptions/Library.pm (-17 lines)
Lines 1-17 Link Here
1
package Koha::Exceptions::Library;
2
3
use Modern::Perl;
4
5
use Exception::Class (
6
7
    'Koha::Exceptions::Library' => {
8
        description => 'Something went wrong!',
9
    },
10
    'Koha::Exceptions::Library::BranchcodeNotFound' => {
11
        isa => 'Koha::Exceptions::Library',
12
        description => "Library does not exist",
13
        fields => ["branchcode"],
14
    },
15
);
16
17
1;
(-)a/Koha/Exceptions/Patron.pm (-17 lines)
Lines 1-17 Link Here
1
package Koha::Exceptions::Patron;
2
3
use Modern::Perl;
4
5
use Exception::Class (
6
7
    'Koha::Exceptions::Patron' => {
8
        description => 'Something went wrong!',
9
    },
10
    'Koha::Exceptions::Patron::DuplicateObject' => {
11
        isa => 'Koha::Exceptions::Patron',
12
        description => "Patron cardnumber and userid must be unique",
13
        fields => ["conflict"],
14
    },
15
);
16
17
1;
(-)a/Koha/Patron.pm (-116 / +3 lines)
Lines 2-8 package Koha::Patron; Link Here
2
2
3
# Copyright ByWater Solutions 2014
3
# Copyright ByWater Solutions 2014
4
# Copyright PTFS Europe 2016
4
# Copyright PTFS Europe 2016
5
# Copyright Koha-Suomi Oy 2017
6
#
5
#
7
# This file is part of Koha.
6
# This file is part of Koha.
8
#
7
#
Lines 31-41 use Koha::Database; Link Here
31
use Koha::DateUtils;
30
use Koha::DateUtils;
32
use Koha::Holds;
31
use Koha::Holds;
33
use Koha::Old::Checkouts;
32
use Koha::Old::Checkouts;
34
use Koha::Exceptions;
35
use Koha::Exceptions::Category;
36
use Koha::Exceptions::Library;
37
use Koha::Exceptions::Patron;
38
use Koha::Libraries;
39
use Koha::Patron::Categories;
33
use Koha::Patron::Categories;
40
use Koha::Patron::HouseboundProfile;
34
use Koha::Patron::HouseboundProfile;
41
use Koha::Patron::HouseboundRole;
35
use Koha::Patron::HouseboundRole;
Lines 849-969 sub store { Link Here
849
    return $self->SUPER::store();
843
    return $self->SUPER::store();
850
}
844
}
851
845
852
=head3 type
853
854
=cut
855
856
sub _type {
857
    return 'Borrower';
858
}
859
860
=head2 Internal methods
846
=head2 Internal methods
861
847
862
=head3 _check_branchcode
848
=head3 _type
863
864
Checks the existence of patron's branchcode and throws
865
Koha::Exceptions::Library::BranchcodeNotFound if branchcode is not found.
866
867
=cut
868
869
sub _check_branchcode {
870
    my ($self) = @_;
871
872
    return unless $self->branchcode;
873
    unless (Koha::Libraries->find($self->branchcode)) {
874
        Koha::Exceptions::Library::BranchcodeNotFound->throw(
875
            error => "Library does not exist",
876
            branchcode => $self->branchcode,
877
        );
878
    }
879
    return 1;
880
}
881
882
=head3 _check_categorycode
883
884
Checks the existence of patron's categorycode and throws
885
Koha::Exceptions::Category::CategorycodeNotFound if categorycode is not found.
886
849
887
=cut
850
=cut
888
851
889
sub _check_categorycode {
852
sub _type {
890
    my ($self) = @_;
853
    return 'Borrower';
891
892
    return unless $self->categorycode;
893
    unless (Koha::Patron::Categories->find($self->categorycode)) {
894
        Koha::Exceptions::Category::CategorycodeNotFound->throw(
895
            error => "Patron category does not exist",
896
            categorycode => $self->categorycode,
897
        );
898
    }
899
    return 1;
900
}
901
902
=head3 _check_uniqueness
903
904
Checks patron's cardnumber and userid for uniqueness and throws
905
Koha::Exceptions::Patron::DuplicateObject if conflicting with another patron.
906
907
=cut
908
909
sub _check_uniqueness {
910
    my ($self) = @_;
911
912
    my $select = {};
913
    $select->{cardnumber} = $self->cardnumber if $self->cardnumber;
914
    $select->{userid} = $self->userid if $self->userid;
915
916
    return unless keys %$select;
917
918
    # Find conflicting patrons
919
    my $patrons = Koha::Patrons->search({
920
        '-or' => $select
921
    });
922
923
    if ($patrons->count) {
924
        my $conflict = {};
925
        foreach my $patron ($patrons->as_list) {
926
            # New patron $self: a conflicting patron $patron found.
927
            # Updating patron $self: first make sure conflicting patron $patron is
928
            #                        not this patron $self.
929
            if (!$self->in_storage || $self->in_storage &&
930
            $self->borrowernumber != $patron->borrowernumber) {
931
                # Populate conflict information to exception
932
                if ($patron->cardnumber && $self->cardnumber &&
933
                    $patron->cardnumber eq $self->cardnumber)
934
                {
935
                    $conflict->{cardnumber} = $self->cardnumber;
936
                }
937
                if ($patron->userid && $self->userid &&
938
                    $patron->userid eq $self->userid)
939
                {
940
                    $conflict->{userid} = $self->userid;
941
                }
942
            }
943
        }
944
945
        Koha::Exceptions::Patron::DuplicateObject->throw(
946
            error => "Patron data conflicts with another patron",
947
            conflict => $conflict
948
        ) if keys %$conflict;
949
    }
950
    return 1;
951
}
952
953
=head3 _validate
954
955
Performs a set of validations on this object and throws Koha::Exceptions if errors
956
are found.
957
958
=cut
959
960
sub _validate {
961
    my ($self) = @_;
962
963
    $self->_check_branchcode;
964
    $self->_check_categorycode;
965
    $self->_check_uniqueness;
966
    return $self;
967
}
854
}
968
855
969
=head1 AUTHOR
856
=head1 AUTHOR
(-)a/Koha/REST/V1/Patrons.pm (-29 / +24 lines)
Lines 90-98 sub add { Link Here
90
    my $c = shift->openapi->valid_input or return;
90
    my $c = shift->openapi->valid_input or return;
91
91
92
    return try {
92
    return try {
93
        my $body = $c->validation->param('body');
94
93
95
        Koha::Patron->new($body)->_validate;
94
        my $body = _to_model($c->validation->param('body'));
96
95
97
        # TODO: Use AddMember until it has been moved to Koha-namespace
96
        # TODO: Use AddMember until it has been moved to Koha-namespace
98
        my $borrowernumber = AddMember(%$body);
97
        my $borrowernumber = AddMember(%$body);
Lines 109-130 sub add { Link Here
109
                }
108
                }
110
            );
109
            );
111
        }
110
        }
112
        if ( $_->isa('Koha::Exceptions::Patron::DuplicateObject') ) {
111
        if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) {
113
            return $c->render(
112
            return $c->render(
114
                status  => 409,
113
                status  => 409,
115
                openapi => { error => $_->error, conflict => $_->conflict }
114
                openapi => { error => $_->error, conflict => $_->duplicate_id }
116
            );
115
            );
117
        }
116
        }
118
        elsif ( $_->isa('Koha::Exceptions::Library::BranchcodeNotFound') ) {
117
        elsif ( $_->isa('Koha::Exceptions::Object::FKConstraint') ) {
119
            return $c->render(
118
            return $c->render(
120
                status  => 400,
119
                status  => 400,
121
                openapi => { error => "Given branchcode does not exist" }
120
                openapi => { error => "Given " . $_->broken_fk . " does not exist" }
122
            );
121
            );
123
        }
122
        }
124
        elsif ( $_->isa('Koha::Exceptions::Category::CategorycodeNotFound') ) {
123
        elsif ( $_->isa('Koha::Exceptions::BadParameter') ) {
125
            return $c->render(
124
            return $c->render(
126
                status  => 400,
125
                status  => 400,
127
                openapi => { error => "Given categorycode does not exist" }
126
                openapi => { error => "Given " . $_->parameter . " does not exist" }
128
            );
127
            );
129
        }
128
        }
130
        else {
129
        else {
Lines 147-164 Controller function that handles updating a Koha::Patron object Link Here
147
sub update {
146
sub update {
148
    my $c = shift->openapi->valid_input or return;
147
    my $c = shift->openapi->valid_input or return;
149
148
150
    my $patron = Koha::Patrons->find( $c->validation->param('borrowernumber') );
149
    my $patron_id = $c->validation->param('borrowernumber');
150
    my $patron    = Koha::Patrons->find( $patron_id );
151
151
152
    return try {
152
    unless ($patron) {
153
        my $body = $c->validation->param('body');
153
         return $c->render(
154
             status  => 404,
155
             openapi => { error => "Patron not found" }
156
         );
157
     }
154
158
155
        $patron->set( _to_model($body) )->_validate;
159
    return try {
160
        my $body = _to_model($c->validation->param('body'));
156
161
157
        ## TODO: Use ModMember until it has been moved to Koha-namespace
162
        ## TODO: Use ModMember until it has been moved to Koha-namespace
158
        # Add borrowernumber to $body, as required by ModMember
163
        # Add borrowernumber to $body, as required by ModMember
159
        $body->{borrowernumber} = $patron->borrowernumber;
164
        $body->{borrowernumber} = $patron_id;
160
165
161
        if ( ModMember(%$body) ) {
166
        if ( ModMember(%$body) ) {
167
            # Fetch the updated Koha::Patron object
168
            $patron->discard_changes;
162
            return $c->render( status => 200, openapi => $patron );
169
            return $c->render( status => 200, openapi => $patron );
163
        }
170
        }
164
        else {
171
        else {
Lines 171-182 sub update { Link Here
171
        }
178
        }
172
    }
179
    }
173
    catch {
180
    catch {
174
        unless ($patron) {
175
            return $c->render(
176
                status  => 404,
177
                openapi => { error => "Patron not found" }
178
            );
179
        }
180
        unless ( blessed $_ && $_->can('rethrow') ) {
181
        unless ( blessed $_ && $_->can('rethrow') ) {
181
            return $c->render(
182
            return $c->render(
182
                status  => 500,
183
                status  => 500,
Lines 185-206 sub update { Link Here
185
                }
186
                }
186
            );
187
            );
187
        }
188
        }
188
        if ( $_->isa('Koha::Exceptions::Patron::DuplicateObject') ) {
189
        if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) {
189
            return $c->render(
190
            return $c->render(
190
                status  => 409,
191
                status  => 409,
191
                openapi => { error => $_->error, conflict => $_->conflict }
192
                openapi => { error => $_->error, conflict => $_->duplicate_id }
192
            );
193
        }
194
        elsif ( $_->isa('Koha::Exceptions::Library::BranchcodeNotFound') ) {
195
            return $c->render(
196
                status  => 400,
197
                openapi => { error => "Given branchcode does not exist" }
198
            );
193
            );
199
        }
194
        }
200
        elsif ( $_->isa('Koha::Exceptions::Category::CategorycodeNotFound') ) {
195
        elsif ( $_->isa('Koha::Exceptions::Object::FKConstraint') ) {
201
            return $c->render(
196
            return $c->render(
202
                status  => 400,
197
                status  => 400,
203
                openapi => { error => "Given categorycode does not exist" }
198
                openapi => { error => "Given " . $_->broken_fk . " does not exist" }
204
            );
199
            );
205
        }
200
        }
206
        elsif ( $_->isa('Koha::Exceptions::MissingParameter') ) {
201
        elsif ( $_->isa('Koha::Exceptions::MissingParameter') ) {
(-)a/t/db_dependent/Koha/Patrons.t (-94 lines)
Lines 1134-1230 sub set_logged_in_user { Link Here
1134
        '',                      ''
1134
        '',                      ''
1135
    );
1135
    );
1136
}
1136
}
1137
1138
subtest '_validate() tests' => sub {
1139
    plan tests => 4;
1140
1141
    $schema->storage->txn_begin;
1142
1143
    Koha::Patrons->delete;
1144
1145
    my $categorycode = $builder->build({ source => 'Category' })->{categorycode};
1146
    my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
1147
    my $patron = $builder->build({
1148
        source => 'Borrower',
1149
        value => {
1150
            branchcode   => $branchcode,
1151
            cardnumber   => 'conflict',
1152
            categorycode => $categorycode,
1153
        }
1154
    });
1155
1156
    ok(Koha::Patron->new({
1157
        surname      => 'Store test',
1158
        branchcode   => $branchcode,
1159
        categorycode => $categorycode
1160
    })->_validate->store, 'Stored a patron');
1161
1162
    subtest '_check_categorycode' => sub {
1163
        plan tests => 2;
1164
1165
        my $conflicting = $builder->build({
1166
            source => 'Borrower',
1167
            value => {
1168
                branchcode   => $branchcode,
1169
                categorycode => 'nonexistent',
1170
            }
1171
        });
1172
        delete $conflicting->{borrowernumber};
1173
1174
        eval { Koha::Patron->new($conflicting)->_validate };
1175
1176
        isa_ok($@, "Koha::Exceptions::Category::CategorycodeNotFound");
1177
        is($@->{categorycode}, $conflicting->{categorycode},
1178
           'Exception describes non-existent categorycode');
1179
    };
1180
1181
    subtest '_check_categorycode' => sub {
1182
        plan tests => 2;
1183
1184
        my $conflicting = $builder->build({
1185
            source => 'Borrower',
1186
            value => {
1187
                branchcode   => 'nonexistent',
1188
                categorycode => $categorycode,
1189
            }
1190
        });
1191
        delete $conflicting->{borrowernumber};
1192
1193
        eval { Koha::Patron->new($conflicting)->_validate };
1194
1195
        isa_ok($@, "Koha::Exceptions::Library::BranchcodeNotFound");
1196
        is($@->{branchcode}, $conflicting->{branchcode},
1197
           'Exception describes non-existent branchcode');
1198
    };
1199
1200
    subtest '_check_uniqueness() tests' => sub {
1201
        plan tests => 4;
1202
1203
        my $conflicting = $builder->build({
1204
            source => 'Borrower',
1205
            value => {
1206
                branchcode   => $branchcode,
1207
                categorycode => $categorycode,
1208
            }
1209
        });
1210
        delete $conflicting->{borrowernumber};
1211
        $conflicting->{cardnumber} = 'conflict';
1212
        $conflicting->{userid} = $patron->{userid};
1213
1214
        eval { Koha::Patron->new($conflicting)->_validate };
1215
1216
        isa_ok($@, "Koha::Exceptions::Patron::DuplicateObject");
1217
        is($@->{conflict}->{cardnumber}, $conflicting->{cardnumber},
1218
           'Exception describes conflicting cardnumber');
1219
        is($@->{conflict}->{userid}, $conflicting->{userid},
1220
           'Exception describes conflicting userid');
1221
1222
        $conflicting->{cardnumber} = 'notconflicting';
1223
        $conflicting->{userid}     = 'notconflicting';
1224
1225
        ok(Koha::Patron->new($conflicting)->_validate->store, 'After modifying'
1226
           .' cardnumber and userid to not conflict with others, no exception.');
1227
    };
1228
1229
    $schema->storage->txn_rollback;
1230
};
(-)a/t/db_dependent/api/v1/patrons.t (-29 / +31 lines)
Lines 19-24 use Modern::Perl; Link Here
19
19
20
use Test::More tests => 5;
20
use Test::More tests => 5;
21
use Test::Mojo;
21
use Test::Mojo;
22
use Test::Warn;
22
23
23
use t::lib::TestBuilder;
24
use t::lib::TestBuilder;
24
use t::lib::Mocks;
25
use t::lib::Mocks;
Lines 160-166 subtest 'add() tests' => sub { Link Here
160
    unauthorized_access_tests('POST', undef, $newpatron);
161
    unauthorized_access_tests('POST', undef, $newpatron);
161
162
162
    subtest 'librarian access tests' => sub {
163
    subtest 'librarian access tests' => sub {
163
        plan tests => 18;
164
        plan tests => 20;
164
165
165
        my ($borrowernumber, $sessionid) = create_user_and_session({
166
        my ($borrowernumber, $sessionid) = create_user_and_session({
166
            authorized => 1 });
167
            authorized => 1 });
Lines 168-176 subtest 'add() tests' => sub { Link Here
168
        $newpatron->{branchcode} = "nonexistent"; # Test invalid branchcode
169
        $newpatron->{branchcode} = "nonexistent"; # Test invalid branchcode
169
        my $tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron );
170
        my $tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron );
170
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
171
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
171
        $t->request_ok($tx)
172
        warning_like {
172
          ->status_is(400)
173
            $t->request_ok($tx)
173
          ->json_is('/error' => "Given branchcode does not exist");
174
              ->status_is(400)
175
              ->json_is('/error' => "Given branchcode does not exist"); }
176
            qr/^DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails/;
177
174
        $newpatron->{branchcode} = $branchcode;
178
        $newpatron->{branchcode} = $branchcode;
175
179
176
        $newpatron->{categorycode} = "nonexistent"; # Test invalid patron category
180
        $newpatron->{categorycode} = "nonexistent"; # Test invalid patron category
Lines 200-214 subtest 'add() tests' => sub { Link Here
200
204
201
        $tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
205
        $tx = $t->ua->build_tx(POST => "/api/v1/patrons" => json => $newpatron);
202
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
206
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
203
        $t->request_ok($tx)
207
        warning_like {
204
          ->status_is(409)
208
            $t->request_ok($tx)
205
          ->json_has('/error', 'Fails when trying to POST duplicate'.
209
              ->status_is(409)
206
                     ' cardnumber or userid')
210
              ->json_has( '/error', 'Fails when trying to POST duplicate cardnumber' )
207
          ->json_has('/conflict', {
211
              ->json_has( '/conflict', 'cardnumber' ); }
208
                        userid => $newpatron->{ userid },
212
            qr/^DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key 'cardnumber'/;
209
                        cardnumber => $newpatron->{ cardnumber }
210
                    }
211
            );
212
    };
213
    };
213
214
214
    $schema->storage->txn_rollback;
215
    $schema->storage->txn_rollback;
Lines 222-228 subtest 'update() tests' => sub { Link Here
222
    unauthorized_access_tests('PUT', 123, {email => 'nobody@example.com'});
223
    unauthorized_access_tests('PUT', 123, {email => 'nobody@example.com'});
223
224
224
    subtest 'librarian access tests' => sub {
225
    subtest 'librarian access tests' => sub {
225
        plan tests => 20;
226
        plan tests => 23;
226
227
227
        t::lib::Mocks::mock_preference('minPasswordLength', 1);
228
        t::lib::Mocks::mock_preference('minPasswordLength', 1);
228
        my ($borrowernumber, $sessionid) = create_user_and_session({ authorized => 1 });
229
        my ($borrowernumber, $sessionid) = create_user_and_session({ authorized => 1 });
Lines 243-259 subtest 'update() tests' => sub { Link Here
243
        $newpatron->{categorycode} = 'nonexistent';
244
        $newpatron->{categorycode} = 'nonexistent';
244
        $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron );
245
        $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron );
245
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
246
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
246
        $t->request_ok($tx)
247
        warning_like {
247
          ->status_is(400)
248
            $t->request_ok($tx)
248
          ->json_is('/error' => "Given categorycode does not exist");
249
              ->status_is(400)
250
              ->json_is('/error' => "Given categorycode does not exist"); }
251
            qr/^DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails/;
249
        $newpatron->{categorycode} = $patron_2->categorycode;
252
        $newpatron->{categorycode} = $patron_2->categorycode;
250
253
251
        $newpatron->{branchcode} = 'nonexistent';
254
        $newpatron->{branchcode} = 'nonexistent';
252
        $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron );
255
        $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron );
253
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
256
        $tx->req->cookies({name => 'CGISESSID', value => $sessionid});
254
        $t->request_ok($tx)
257
        warning_like {
255
          ->status_is(400)
258
            $t->request_ok($tx)
256
          ->json_is('/error' => "Given branchcode does not exist");
259
              ->status_is(400)
260
              ->json_is('/error' => "Given branchcode does not exist"); }
261
            qr/^DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails/;
257
        $newpatron->{branchcode} = $patron_2->branchcode;
262
        $newpatron->{branchcode} = $patron_2->branchcode;
258
263
259
        $newpatron->{falseproperty} = "Non existent property";
264
        $newpatron->{falseproperty} = "Non existent property";
Lines 271-284 subtest 'update() tests' => sub { Link Here
271
276
272
        $tx = $t->ua->build_tx( PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron );
277
        $tx = $t->ua->build_tx( PUT => "/api/v1/patrons/$borrowernumber2" => json => $newpatron );
273
        $tx->req->cookies({ name => 'CGISESSID', value => $sessionid });
278
        $tx->req->cookies({ name => 'CGISESSID', value => $sessionid });
274
        $t->request_ok($tx)->status_is(409)
279
        warning_like {
275
          ->json_has( '/error' => "Fails when trying to update to an existing cardnumber or userid")
280
            $t->request_ok($tx)
276
          ->json_is(  '/conflict',
281
              ->status_is(409)
277
                        {
282
              ->json_has( '/error' => "Fails when trying to update to an existing cardnumber or userid")
278
                            cardnumber => $newpatron->{cardnumber},
283
              ->json_is(  '/conflict', 'cardnumber' ); }
279
                            userid     => $newpatron->{userid}
284
            qr/^DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key 'cardnumber'/;
280
                        }
281
          );
282
285
283
        $newpatron->{ cardnumber } = $borrowernumber.$borrowernumber2;
286
        $newpatron->{ cardnumber } = $borrowernumber.$borrowernumber2;
284
        $newpatron->{ userid } = "user".$borrowernumber.$borrowernumber2;
287
        $newpatron->{ userid } = "user".$borrowernumber.$borrowernumber2;
285
- 

Return to bug 16330