From b4d6e64ba807b099e3f980c150eee5803ea050e6 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=UTF-8 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) --- Koha/REST/V1/Patrons/Account.pm | 101 ++++++++-- api/v1/swagger/definitions/debit.yaml | 1 - api/v1/swagger/paths/patrons_account.yaml | 47 +++++ t/db_dependent/api/v1/patrons_accounts.t | 235 ++++++++++++++++------ 4 files changed, 298 insertions(+), 86 deletions(-) diff --git a/Koha/REST/V1/Patrons/Account.pm b/Koha/REST/V1/Patrons/Account.pm index 2238ffc075..e6e4e44887 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,51 @@ 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->{type} = $data->{debit_type_code}; + $data->{cash_register} = $data->{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 4ffe2b8f4a..aa21ab9379 100644 --- a/api/v1/swagger/definitions/debit.yaml +++ b/api/v1/swagger/definitions/debit.yaml @@ -32,7 +32,6 @@ properties: type: - string - "null" - readOnly: true description: Account line description account_type: 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 cada141ef3..a6eaff9ec5 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 => 2; +use Test::More tests => 3; 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 ); @@ -268,3 +293,81 @@ subtest 'add_credit() tests' => sub { $schema->storage->txn_rollback; }; + +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.25.1