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 824-944 sub store { Link Here
824
    return $self->SUPER::store();
818
    return $self->SUPER::store();
825
}
819
}
826
820
827
=head3 type
828
829
=cut
830
831
sub _type {
832
    return 'Borrower';
833
}
834
835
=head2 Internal methods
821
=head2 Internal methods
836
822
837
=head3 _check_branchcode
823
=head3 _type
838
839
Checks the existence of patron's branchcode and throws
840
Koha::Exceptions::Library::BranchcodeNotFound if branchcode is not found.
841
842
=cut
843
844
sub _check_branchcode {
845
    my ($self) = @_;
846
847
    return unless $self->branchcode;
848
    unless (Koha::Libraries->find($self->branchcode)) {
849
        Koha::Exceptions::Library::BranchcodeNotFound->throw(
850
            error => "Library does not exist",
851
            branchcode => $self->branchcode,
852
        );
853
    }
854
    return 1;
855
}
856
857
=head3 _check_categorycode
858
859
Checks the existence of patron's categorycode and throws
860
Koha::Exceptions::Category::CategorycodeNotFound if categorycode is not found.
861
824
862
=cut
825
=cut
863
826
864
sub _check_categorycode {
827
sub _type {
865
    my ($self) = @_;
828
    return 'Borrower';
866
867
    return unless $self->categorycode;
868
    unless (Koha::Patron::Categories->find($self->categorycode)) {
869
        Koha::Exceptions::Category::CategorycodeNotFound->throw(
870
            error => "Patron category does not exist",
871
            categorycode => $self->categorycode,
872
        );
873
    }
874
    return 1;
875
}
876
877
=head3 _check_uniqueness
878
879
Checks patron's cardnumber and userid for uniqueness and throws
880
Koha::Exceptions::Patron::DuplicateObject if conflicting with another patron.
881
882
=cut
883
884
sub _check_uniqueness {
885
    my ($self) = @_;
886
887
    my $select = {};
888
    $select->{cardnumber} = $self->cardnumber if $self->cardnumber;
889
    $select->{userid} = $self->userid if $self->userid;
890
891
    return unless keys %$select;
892
893
    # Find conflicting patrons
894
    my $patrons = Koha::Patrons->search({
895
        '-or' => $select
896
    });
897
898
    if ($patrons->count) {
899
        my $conflict = {};
900
        foreach my $patron ($patrons->as_list) {
901
            # New patron $self: a conflicting patron $patron found.
902
            # Updating patron $self: first make sure conflicting patron $patron is
903
            #                        not this patron $self.
904
            if (!$self->in_storage || $self->in_storage &&
905
            $self->borrowernumber != $patron->borrowernumber) {
906
                # Populate conflict information to exception
907
                if ($patron->cardnumber && $self->cardnumber &&
908
                    $patron->cardnumber eq $self->cardnumber)
909
                {
910
                    $conflict->{cardnumber} = $self->cardnumber;
911
                }
912
                if ($patron->userid && $self->userid &&
913
                    $patron->userid eq $self->userid)
914
                {
915
                    $conflict->{userid} = $self->userid;
916
                }
917
            }
918
        }
919
920
        Koha::Exceptions::Patron::DuplicateObject->throw(
921
            error => "Patron data conflicts with another patron",
922
            conflict => $conflict
923
        ) if keys %$conflict;
924
    }
925
    return 1;
926
}
927
928
=head3 _validate
929
930
Performs a set of validations on this object and throws Koha::Exceptions if errors
931
are found.
932
933
=cut
934
935
sub _validate {
936
    my ($self) = @_;
937
938
    $self->_check_branchcode;
939
    $self->_check_categorycode;
940
    $self->_check_uniqueness;
941
    return $self;
942
}
829
}
943
830
944
=head1 AUTHOR
831
=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 (-95 / +1 lines)
Lines 19-25 Link Here
19
19
20
use Modern::Perl;
20
use Modern::Perl;
21
21
22
use Test::More tests => 25;
22
use Test::More tests => 24;
23
use Test::Warn;
23
use Test::Warn;
24
use Time::Fake;
24
use Time::Fake;
25
use DateTime;
25
use DateTime;
Lines 1071-1167 sub set_logged_in_user { Link Here
1071
        '',                      ''
1071
        '',                      ''
1072
    );
1072
    );
1073
}
1073
}
1074
1075
subtest '_validate() tests' => sub {
1076
    plan tests => 4;
1077
1078
    $schema->storage->txn_begin;
1079
1080
    Koha::Patrons->delete;
1081
1082
    my $categorycode = $builder->build({ source => 'Category' })->{categorycode};
1083
    my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
1084
    my $patron = $builder->build({
1085
        source => 'Borrower',
1086
        value => {
1087
            branchcode   => $branchcode,
1088
            cardnumber   => 'conflict',
1089
            categorycode => $categorycode,
1090
        }
1091
    });
1092
1093
    ok(Koha::Patron->new({
1094
        surname      => 'Store test',
1095
        branchcode   => $branchcode,
1096
        categorycode => $categorycode
1097
    })->_validate->store, 'Stored a patron');
1098
1099
    subtest '_check_categorycode' => sub {
1100
        plan tests => 2;
1101
1102
        my $conflicting = $builder->build({
1103
            source => 'Borrower',
1104
            value => {
1105
                branchcode   => $branchcode,
1106
                categorycode => 'nonexistent',
1107
            }
1108
        });
1109
        delete $conflicting->{borrowernumber};
1110
1111
        eval { Koha::Patron->new($conflicting)->_validate };
1112
1113
        isa_ok($@, "Koha::Exceptions::Category::CategorycodeNotFound");
1114
        is($@->{categorycode}, $conflicting->{categorycode},
1115
           'Exception describes non-existent categorycode');
1116
    };
1117
1118
    subtest '_check_categorycode' => sub {
1119
        plan tests => 2;
1120
1121
        my $conflicting = $builder->build({
1122
            source => 'Borrower',
1123
            value => {
1124
                branchcode   => 'nonexistent',
1125
                categorycode => $categorycode,
1126
            }
1127
        });
1128
        delete $conflicting->{borrowernumber};
1129
1130
        eval { Koha::Patron->new($conflicting)->_validate };
1131
1132
        isa_ok($@, "Koha::Exceptions::Library::BranchcodeNotFound");
1133
        is($@->{branchcode}, $conflicting->{branchcode},
1134
           'Exception describes non-existent branchcode');
1135
    };
1136
1137
    subtest '_check_uniqueness() tests' => sub {
1138
        plan tests => 4;
1139
1140
        my $conflicting = $builder->build({
1141
            source => 'Borrower',
1142
            value => {
1143
                branchcode   => $branchcode,
1144
                categorycode => $categorycode,
1145
            }
1146
        });
1147
        delete $conflicting->{borrowernumber};
1148
        $conflicting->{cardnumber} = 'conflict';
1149
        $conflicting->{userid} = $patron->{userid};
1150
1151
        eval { Koha::Patron->new($conflicting)->_validate };
1152
1153
        isa_ok($@, "Koha::Exceptions::Patron::DuplicateObject");
1154
        is($@->{conflict}->{cardnumber}, $conflicting->{cardnumber},
1155
           'Exception describes conflicting cardnumber');
1156
        is($@->{conflict}->{userid}, $conflicting->{userid},
1157
           'Exception describes conflicting userid');
1158
1159
        $conflicting->{cardnumber} = 'notconflicting';
1160
        $conflicting->{userid}     = 'notconflicting';
1161
1162
        ok(Koha::Patron->new($conflicting)->_validate->store, 'After modifying'
1163
           .' cardnumber and userid to not conflict with others, no exception.');
1164
    };
1165
1166
    $schema->storage->txn_rollback;
1167
};
(-)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