From ab10759c5061ccbd957b667ddf345a4bc174769c Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Mon, 27 Feb 2023 12:51:16 -0300 Subject: [PATCH] Bug 21043: Add debit REST endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This patch adds an endpoint to create a debit for a patron. Testplan 1. Create a new account debit type (Administration > Debit types) 2. Add a fee with this debit type to a patron’s account via API 3. Make sure that this fee is shown in the accounting overview in the patron’s account in the staff interface 4. Make sure that it is possible to make a payment for this fee Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Patrons/Account.pm | 100 ++++++++-- api/v1/swagger/definitions/debit.yaml | 1 - api/v1/swagger/paths/patrons_account.yaml | 47 +++++ t/db_dependent/api/v1/patrons_accounts.t | 232 ++++++++++++++++------ 4 files changed, 294 insertions(+), 86 deletions(-) diff --git a/Koha/REST/V1/Patrons/Account.pm b/Koha/REST/V1/Patrons/Account.pm index 2238ffc075..ca2850752c 100644 --- a/Koha/REST/V1/Patrons/Account.pm +++ b/Koha/REST/V1/Patrons/Account.pm @@ -44,7 +44,10 @@ sub get { my $patron = Koha::Patrons->find($patron_id); unless ($patron) { - return $c->render( status => 404, openapi => { error => "Patron not found." } ); + return $c->render( + status => 404, + openapi => { error => "Patron not found." } + ); } return try { @@ -57,7 +60,7 @@ sub get { return $c->render( status => 200, openapi => { - balance => $account->balance, + balance => $account->balance, outstanding_debits => { total => $debits->total_outstanding, lines => $debits->to_api @@ -85,14 +88,17 @@ sub list_credits { my $patron = Koha::Patrons->find($patron_id); unless ($patron) { - return $c->render( status => 404, openapi => { error => "Patron not found." } ); + return $c->render( + status => 404, + openapi => { error => "Patron not found." } + ); } return try { my $account = $patron->account; my $credits_set = $account->credits; - my $credits = $c->objects->search( $credits_set ); + my $credits = $c->objects->search($credits_set); return $c->render( status => 200, openapi => $credits ); } catch { @@ -113,19 +119,23 @@ sub add_credit { my $patron = Koha::Patrons->find($patron_id); my $user = $c->stash('koha.user'); - unless ($patron) { - return $c->render( status => 404, openapi => { error => "Patron not found." } ); + return $c->render( + status => 404, + openapi => { error => "Patron not found." } + ); } my $account = $patron->account; my $body = $c->validation->param('body'); return try { - my $credit_type = $body->{credit_type} || 'PAYMENT'; # default to 'PAYMENT' - my $amount = $body->{amount}; # mandatory, validated by openapi + my $credit_type = + $body->{credit_type} || 'PAYMENT'; # default to 'PAYMENT' + my $amount = $body->{amount}; # mandatory, validated by openapi - unless ( $amount > 0 ) { # until we support newer JSON::Validator and thus minimumExclusive + unless ( $amount > 0 ) + { # until we support newer JSON::Validator and thus minimumExclusive Koha::Exceptions::BadParameter->throw( { parameter => 'amount' } ); } @@ -136,7 +146,8 @@ sub add_credit { my $library_id = $body->{library_id}; my $credit = $account->add_credit( - { amount => $amount, + { + amount => $amount, type => $credit_type, payment_type => $payment_type, description => $description, @@ -149,22 +160,24 @@ sub add_credit { $credit->discard_changes; my $date = $body->{date}; - $credit->date( $date )->store - if $date; + $credit->date($date)->store + if $date; my $debits_ids = $body->{account_lines_ids}; my $debits; - $debits = Koha::Account::Lines->search({ accountlines_id => { -in => $debits_ids } }) - if $debits_ids; + $debits = Koha::Account::Lines->search( + { accountlines_id => { -in => $debits_ids } } ) + if $debits_ids; if ($debits) { + # pay them! - $credit = $credit->apply({ debits => [ $debits->as_list ] }); + $credit = $credit->apply( { debits => [ $debits->as_list ] } ); } - if ($credit->amountoutstanding != 0) { + if ( $credit->amountoutstanding != 0 ) { my $outstanding_debits = $account->outstanding_debits; - $credit->apply({ debits => [ $outstanding_debits->as_list ] }); + $credit->apply( { debits => [ $outstanding_debits->as_list ] } ); } $credit->discard_changes; @@ -190,14 +203,17 @@ sub list_debits { my $patron = Koha::Patrons->find($patron_id); unless ($patron) { - return $c->render( status => 404, openapi => { error => "Patron not found." } ); + return $c->render( + status => 404, + openapi => { error => "Patron not found." } + ); } return try { my $account = $patron->account; my $debits_set = $account->debits; - my $debits = $c->objects->search( $debits_set ); + my $debits = $c->objects->search($debits_set); return $c->render( status => 200, openapi => $debits ); } catch { @@ -205,4 +221,50 @@ sub list_debits { }; } +=head3 add_debit + +=cut + +sub add_debit { + my $c = shift->openapi->valid_input or return; + + my $patron_id = $c->validation->param('patron_id'); + my $patron = Koha::Patrons->find($patron_id); + + unless ($patron) { + return $c->render( + status => 404, + openapi => { error => "Patron not found." } + ); + } + + return try { + my $data = + Koha::Account::Debit->new_from_api( $c->validation->param('body') ) + ->unblessed; + + $data->{library_id} = $data->{branchcode}; + $data->{cash_register} = $data->{cash_register_id}; + $data->{item_id} = $data->{itemnumber}; + $data->{interface} = 'api' + ; # Should this always be API, or should we allow the API consumer to choose? + $data->{user_id} = $patron->borrowernumber + ; # Should this be API user OR staff the API may be acting on behalf of? + + my $debit = $patron->account->add_debit($data); + + $c->res->headers->location( + $c->req->url->to_string . '/' . $debit->id ); + $debit->discard_changes; + + return $c->render( + status => 201, + openapi => $debit->to_api + ); + } + catch { + $c->unhandled_exception($_); + }; +} + 1; diff --git a/api/v1/swagger/definitions/debit.yaml b/api/v1/swagger/definitions/debit.yaml index 596588aa44..c3cc06096f 100644 --- a/api/v1/swagger/definitions/debit.yaml +++ b/api/v1/swagger/definitions/debit.yaml @@ -31,7 +31,6 @@ properties: type: - string - "null" - readOnly: true description: Account line description interface: type: diff --git a/api/v1/swagger/paths/patrons_account.yaml b/api/v1/swagger/paths/patrons_account.yaml index 7c4e033826..9b05e11572 100644 --- a/api/v1/swagger/paths/patrons_account.yaml +++ b/api/v1/swagger/paths/patrons_account.yaml @@ -177,3 +177,50 @@ permissions: borrowers: edit_borrowers updatecharges: remaining_permissions + post: + x-mojo-to: Patrons::Account#add_debit + operationId: addPatronDebit + tags: + - patrons + summary: Add debit to a patron's account + parameters: + - $ref: "../swagger.yaml#/parameters/patron_id_pp" + - name: body + in: body + description: A JSON object containing debit information + required: true + schema: + $ref: "../swagger.yaml#/definitions/debit" + produces: + - application/json + responses: + "201": + description: Debit added + schema: + $ref: "../swagger.yaml#/definitions/account_line" + "401": + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Patron not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: | + Internal server error. Possible `error_code` attribute values: + + * `internal_server_error` + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + updatecharges: remaining_permissions diff --git a/t/db_dependent/api/v1/patrons_accounts.t b/t/db_dependent/api/v1/patrons_accounts.t index 3118171b7a..770e8a1af3 100755 --- a/t/db_dependent/api/v1/patrons_accounts.t +++ b/t/db_dependent/api/v1/patrons_accounts.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 4; +use Test::More tests => 5; use Test::Mojo; @@ -42,35 +42,37 @@ subtest 'get_balance() tests' => sub { # Enable AccountAutoReconcile t::lib::Mocks::mock_preference( 'AccountAutoReconcile', 1 ); - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { flags => 1 } - }); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 1 } + } + ); my $password = 'thePassword123'; - $patron->set_password({ password => $password, skip_validation => 1 }); + $patron->set_password( { password => $password, skip_validation => 1 } ); my $userid = $patron->userid; - my $library = $builder->build_object({ class => 'Koha::Libraries' }); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $patron_id = $patron->id; my $account = $patron->account; $t->get_ok("//$userid:$password@/api/v1/patrons/$patron_id/account") - ->status_is(200) - ->json_is( - { balance => 0.00, + ->status_is(200)->json_is( + { + balance => 0.00, outstanding_debits => { total => 0, lines => [] }, outstanding_credits => { total => 0, lines => [] } } - ); + ); my $debit_1 = $account->add_debit( { - amount => 50, - description => "A description", - type => "NEW_CARD", - user_id => $patron->borrowernumber, - library_id => $library->id, - interface => 'test', + amount => 50, + description => "A description", + type => "NEW_CARD", + user_id => $patron->borrowernumber, + library_id => $library->id, + interface => 'test', } )->store(); $debit_1->discard_changes; @@ -79,7 +81,7 @@ subtest 'get_balance() tests' => sub { { amount => 50.01, description => "A description", - type => "NEW_CARD", # New card + type => "NEW_CARD", # New card user_id => $patron->borrowernumber, library_id => $library->id, interface => 'test', @@ -88,25 +90,23 @@ subtest 'get_balance() tests' => sub { $debit_2->discard_changes; $t->get_ok("//$userid:$password@/api/v1/patrons/$patron_id/account") - ->status_is(200) - ->json_is( - { balance => 100.01, + ->status_is(200)->json_is( + { + balance => 100.01, outstanding_debits => { total => 100.01, - lines => [ - $debit_1->to_api, - $debit_2->to_api - ] + lines => [ $debit_1->to_api, $debit_2->to_api ] }, outstanding_credits => { total => 0, lines => [] } } - ); + ); $account->pay( - { amount => 100.01, + { + amount => 100.01, note => 'He paid!', description => 'Finally!', library_id => $patron->branchcode, @@ -115,24 +115,31 @@ subtest 'get_balance() tests' => sub { ); $t->get_ok("//$userid:$password@/api/v1/patrons/$patron_id/account") - ->status_is(200) - ->json_is( - { balance => 0, + ->status_is(200)->json_is( + { + balance => 0, outstanding_debits => { total => 0, lines => [] }, outstanding_credits => { total => 0, lines => [] } } - ); + ); # add a credit my $credit_line = $account->add_credit( - { amount => 10, user_id => $patron->id, library_id => $library->id, interface => 'test' } ); + { + amount => 10, + user_id => $patron->id, + library_id => $library->id, + interface => 'test' + } + ); + # re-read from the DB $credit_line->discard_changes; $t->get_ok("//$userid:$password@/api/v1/patrons/$patron_id/account") - ->status_is(200) - ->json_is( - { balance => -10, + ->status_is(200)->json_is( + { + balance => -10, outstanding_debits => { total => 0, lines => [] @@ -142,14 +149,14 @@ subtest 'get_balance() tests' => sub { lines => [ $credit_line->to_api ] } } - ); + ); # Accountline without manager_id (happens with fines.pl cron for example) my $debit_3 = $account->add_debit( { amount => 50, description => "A description", - type => "NEW_CARD", # New card + type => "NEW_CARD", # New card user_id => undef, library_id => $library->id, interface => 'test', @@ -158,21 +165,19 @@ subtest 'get_balance() tests' => sub { $debit_3->discard_changes; $t->get_ok("//$userid:$password@/api/v1/patrons/$patron_id/account") - ->status_is(200) - ->json_is( - { balance => 40.00, + ->status_is(200)->json_is( + { + balance => 40.00, outstanding_debits => { total => 50.00, - lines => [ - $debit_3->to_api - ] + lines => [ $debit_3->to_api ] }, outstanding_credits => { total => -10, lines => [ $credit_line->to_api ] } } - ); + ); $schema->storage->txn_rollback; }; @@ -183,34 +188,44 @@ subtest 'add_credit() tests' => sub { $schema->storage->txn_begin; - my $patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { flags => 1 } - }); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 1 } + } + ); my $password = 'thePassword123'; - $patron->set_password({ password => $password, skip_validation => 1 }); + $patron->set_password( { password => $password, skip_validation => 1 } ); my $userid = $patron->userid; - my $library = $builder->build_object({ class => 'Koha::Libraries' }); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $patron_id = $patron->id; my $account = $patron->account; - is( $account->outstanding_debits->count, 0, 'No outstanding debits for patron' ); - is( $account->outstanding_credits->count, 0, 'No outstanding credits for patron' ); + is( $account->outstanding_debits->count, + 0, 'No outstanding debits for patron' ); + is( $account->outstanding_credits->count, + 0, 'No outstanding credits for patron' ); my $credit = { amount => 100 }; - my $ret = $t->post_ok("//$userid:$password@/api/v1/patrons/$patron_id/account/credits" => json => $credit) - ->status_is(201)->tx->res->json; + my $ret = $t->post_ok( + "//$userid:$password@/api/v1/patrons/$patron_id/account/credits" => + json => $credit )->status_is(201)->tx->res->json; - is_deeply( $ret, Koha::Account::Lines->find( $ret->{account_line_id} )->to_api, 'Line returned correctly' ); + is_deeply( + $ret, + Koha::Account::Lines->find( $ret->{account_line_id} )->to_api, + 'Line returned correctly' + ); my $outstanding_credits = $account->outstanding_credits; is( $outstanding_credits->count, 1 ); is( $outstanding_credits->total_outstanding, -100 ); my $debit_1 = $account->add_debit( - { amount => 10, + { + amount => 10, description => "A description", type => "NEW_CARD", user_id => $patron->borrowernumber, @@ -218,7 +233,8 @@ subtest 'add_credit() tests' => sub { } )->store(); my $debit_2 = $account->add_debit( - { amount => 15, + { + amount => 15, description => "A description", type => "NEW_CARD", user_id => $patron->borrowernumber, @@ -229,11 +245,15 @@ subtest 'add_credit() tests' => sub { is( $account->outstanding_debits->total_outstanding, 25 ); $credit->{library_id} = $library->id; - $ret = $t->post_ok("//$userid:$password@/api/v1/patrons/$patron_id/account/credits" => json => $credit) - ->status_is(201) - ->tx->res->json; + $ret = $t->post_ok( + "//$userid:$password@/api/v1/patrons/$patron_id/account/credits" => + json => $credit )->status_is(201)->tx->res->json; - is_deeply( $ret, Koha::Account::Lines->find( $ret->{account_line_id} )->to_api, 'Line returned correctly' ); + is_deeply( + $ret, + Koha::Account::Lines->find( $ret->{account_line_id} )->to_api, + 'Line returned correctly' + ); my $account_line_id = $t->tx->res->json->{account_line_id}; is( Koha::Account::Lines->find($account_line_id)->branchcode, @@ -243,7 +263,8 @@ subtest 'add_credit() tests' => sub { 0, "Debits have been cancelled automatically" ); my $debit_3 = $account->add_debit( - { amount => 100, + { + amount => 100, description => "A description", type => "NEW_CARD", user_id => $patron->borrowernumber, @@ -256,11 +277,15 @@ subtest 'add_credit() tests' => sub { account_lines_ids => [ $debit_1->id, $debit_2->id, $debit_3->id ] }; - $ret = $t->post_ok("//$userid:$password@/api/v1/patrons/$patron_id/account/credits" => json => $credit) - ->status_is(201) - ->tx->res->json; + $ret = $t->post_ok( + "//$userid:$password@/api/v1/patrons/$patron_id/account/credits" => + json => $credit )->status_is(201)->tx->res->json; - is_deeply( $ret, Koha::Account::Lines->find( $ret->{account_line_id} )->to_api, 'Line returned correctly' ); + is_deeply( + $ret, + Koha::Account::Lines->find( $ret->{account_line_id} )->to_api, + 'Line returned correctly' + ); my $outstanding_debits = $account->outstanding_debits; is( $outstanding_debits->total_outstanding, 65 ); @@ -342,6 +367,81 @@ subtest 'list_debits() test' => sub { ->tx->res->json; is(140, $ret->[0]->{amount} + $ret->[1]->{amount}, 'Total debits are 140'); +}; + +subtest 'add_debit() tests' => sub { + + plan tests => 13; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 1 } + } + ); + my $password = 'thePassword123'; + $patron->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $patron->userid; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron_id = $patron->id; + my $account = $patron->account; + + is( $account->outstanding_debits->count, + 0, 'No outstanding debits for patron' ); + is( $account->outstanding_credits->count, + 0, 'No outstanding credits for patron' ); + + my $debit = { + amount => 100, + description => "A description", + debit_type => "NEW_CARD", + user_id => $patron->borrowernumber, + interface => 'test', + library_id => $library->id, + }; + + my $ret = $t->post_ok( + "//$userid:$password@/api/v1/patrons/$patron_id/account/debits" => + json => $debit )->status_is(201)->tx->res->json; + my $account_line = Koha::Account::Lines->find( $ret->{account_line_id} ); + + is_deeply( $ret, $account_line->to_api, 'Line returned correctly' ); + + is( $account_line->branchcode, + $library->id, 'Library id is sored correctly' ); + + my $outstanding_debits = $account->outstanding_debits; + is( $outstanding_debits->count, 1 ); + is( $outstanding_debits->total_outstanding, 100 ); + + my $credit_1 = $account->add_credit( + { + amount => 10, + interface => 'test', + } + )->store()->apply( { debits => [$account_line] } ); + my $credit_2 = $account->add_credit( + { + amount => 15, + interface => 'test' + } + )->store()->apply( { debits => [$account_line] } ); + + is( $account->outstanding_credits->total_outstanding, 0 ); + is( $account->outstanding_debits->total_outstanding, + 75, "Credits partially cancelled debit" ); + + my $account_line_id = $ret->{account_line_id}; + + $t->post_ok( + "//$userid:$password@/api/v1/patrons/$patron_id/account/debits" => + json => $debit )->status_is(201); + + is( $account->outstanding_debits->total_outstanding, + 175, "Debit added to total outstanding debits" ); $schema->storage->txn_rollback; }; -- 2.39.5