From a9237380b51b297d4be2f9f9eb7c9bd6fda0ec26 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 25 Jun 2018 14:55:30 -0300 Subject: [PATCH] Bug 20942: Split debit and credit lines This patch splits the balance to match this object schema: { balance => #, outstanding_credits => { total => #, lines => [ credit_line_1, ..., credit_line_n ] }, outstanding_debits => { total => #, lines => [ debit_line_1, ..., debit_line_m ] } } This change is made to ease usage from the UI. Also because the outstanding credits need to be applied to outstanding debits in order to the balance value to make sense. So we still need to have each total. Tests are added for this change, and the schema files are adjusted as well. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_accounts.t => SUCCESS: Tests pass! - Sign off :-D staff_id is changed into user_id as voted on the dev meeting the RFC got approved. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Josef Moravec Signed-off-by: Kyle M Hall Signed-off-by: Nick Clemens (cherry picked from commit bb7c908dc06697a4ebbf8633897d24e26798cb6d) Signed-off-by: Martin Renvoize --- Koha/REST/V1/Patrons/Account.pm | 30 +++++---- api/v1/swagger/definitions/account_line.json | 2 +- .../swagger/definitions/patron_balance.json | 29 +++++++-- t/db_dependent/api/v1/patrons_accounts.t | 61 +++++++++++++++---- 4 files changed, 93 insertions(+), 29 deletions(-) diff --git a/Koha/REST/V1/Patrons/Account.pm b/Koha/REST/V1/Patrons/Account.pm index 6ecaa867cd..2003bde234 100644 --- a/Koha/REST/V1/Patrons/Account.pm +++ b/Koha/REST/V1/Patrons/Account.pm @@ -45,18 +45,26 @@ sub get { return $c->render( status => 404, openapi => { error => "Patron not found." } ); } + my $account = $patron->account; my $balance; - $balance->{balance} = $patron->account->balance; + $balance->{balance} = $account->balance; - my @outstanding_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->borrowernumber, - amountoutstanding => { '!=' => 0 } - } - ); - foreach my $line ( @outstanding_lines ) { - push @{ $balance->{outstanding_lines} }, _to_api($line->TO_JSON) - } + # get outstanding debits + my ( $debits_total, $debits ) = $account->outstanding_debits; + my ( $credits_total, $credits ) = $account->outstanding_credits; + + my @debit_lines = map { _to_api( $_->TO_JSON ) } @{ $debits->as_list }; + $balance->{outstanding_debits} = { + total => $debits_total, + lines => \@debit_lines + }; + + my @credit_lines = map { _to_api( $_->TO_JSON ) } @{ $credits->as_list }; + $balance->{outstanding_credits} = { + total => $credits_total, + lines => \@credit_lines + }; return $c->render( status => 200, openapi => $balance ); } @@ -135,7 +143,7 @@ our $to_api_mapping = { dispute => undef, issue_id => 'checkout_id', itemnumber => 'item_id', - manager_id => 'staff_id', + manager_id => 'user_id', note => 'internal_note', }; @@ -151,7 +159,7 @@ our $to_model_mapping = { internal_note => 'note', item_id => 'itemnumber', patron_id => 'borrowernumber', - staff_id => 'manager_id' + user_id => 'manager_id' }; 1; diff --git a/api/v1/swagger/definitions/account_line.json b/api/v1/swagger/definitions/account_line.json index 7f0df9e14d..ba1c7172d4 100644 --- a/api/v1/swagger/definitions/account_line.json +++ b/api/v1/swagger/definitions/account_line.json @@ -73,7 +73,7 @@ ], "description": "Internal note" }, - "staff_id": { + "user_id": { "type": "integer", "description": "Internal patron identifier for the staff member that introduced the account line" } diff --git a/api/v1/swagger/definitions/patron_balance.json b/api/v1/swagger/definitions/patron_balance.json index 52978b38d4..b8c0330ef1 100644 --- a/api/v1/swagger/definitions/patron_balance.json +++ b/api/v1/swagger/definitions/patron_balance.json @@ -5,10 +5,31 @@ "type": "number", "description": "Signed decimal number" }, - "outstanding_lines": { - "type": "array", - "items": { - "$ref": "account_line.json" + "outstanding_credits": { + "properties": { + "total": { + "type": "number" + }, + "lines": { + "type": "array", + "items": { + "$ref": "account_line.json" + } + } + } + }, + "outstanding_debits": { + "type": "object", + "properties": { + "total": { + "type": "number" + }, + "lines": { + "type": "array", + "items": { + "$ref": "account_line.json" + } + } } } }, diff --git a/t/db_dependent/api/v1/patrons_accounts.t b/t/db_dependent/api/v1/patrons_accounts.t index dc770f425e..977890c337 100644 --- a/t/db_dependent/api/v1/patrons_accounts.t +++ b/t/db_dependent/api/v1/patrons_accounts.t @@ -41,19 +41,23 @@ my $t = Test::Mojo->new('Koha::REST::V1'); subtest 'get_balance() tests' => sub { - plan tests => 9; + plan tests => 12; $schema->storage->txn_begin; my ( $patron_id, $session_id ) = create_user_and_session({ authorized => 0 }); - my $patron = Koha::Patrons->find($patron_id); + my $patron = Koha::Patrons->find($patron_id); + my $account = $patron->account; my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/$patron_id/account"); $tx->req->cookies({ name => 'CGISESSID', value => $session_id }); $tx->req->env({ REMOTE_ADDR => '127.0.0.1' }); - $t->request_ok($tx) - ->status_is(200) - ->json_is( { balance => 0.00 } ); + $t->request_ok($tx)->status_is(200)->json_is( + { balance => 0.00, + outstanding_debits => { total => 0, lines => [] }, + outstanding_credits => { total => 0, lines => [] } + } + ); my $account_line_1 = Koha::Account::Line->new( { @@ -85,16 +89,22 @@ subtest 'get_balance() tests' => sub { $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); $t->request_ok($tx)->status_is(200)->json_is( - { balance => 100.01, - outstanding_lines => [ - Koha::REST::V1::Patrons::Account::_to_api( $account_line_1->TO_JSON ), - Koha::REST::V1::Patrons::Account::_to_api( $account_line_2->TO_JSON ) - - ] + { balance => 100.01, + outstanding_debits => { + total => 100.01, + lines => [ + Koha::REST::V1::Patrons::Account::_to_api( $account_line_1->TO_JSON ), + Koha::REST::V1::Patrons::Account::_to_api( $account_line_2->TO_JSON ) + ] + }, + outstanding_credits => { + total => 0, + lines => [] + } } ); - Koha::Account->new({ patron_id => $patron_id })->pay( + $account->pay( { amount => 100.01, note => 'He paid!', description => 'Finally!', @@ -107,7 +117,32 @@ subtest 'get_balance() tests' => sub { $tx = $t->ua->build_tx( GET => "/api/v1/patrons/$patron_id/account" ); $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); - $t->request_ok($tx)->status_is(200)->json_is( { balance => 0 } ); + $t->request_ok($tx)->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 }); + # re-read from the DB + $credit_line->discard_changes; + $tx = $t->ua->build_tx( GET => "/api/v1/patrons/$patron_id/account" ); + $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); + $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); + $t->request_ok($tx)->status_is(200)->json_is( + { balance => -10, + outstanding_debits => { + total => 0, + lines => [] + }, + outstanding_credits => { + total => -10, + lines => [ Koha::REST::V1::Patrons::Account::_to_api( $credit_line->TO_JSON ) ] + } + } + ); $schema->storage->txn_rollback; }; -- 2.39.5