From 999a29dee64e0797fd437fb69ce55ea61f7f05a6 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 --- Koha/REST/V1/Patrons/Account.pm | 30 ++++++++----- api/v1/swagger/definitions/account_line.json | 2 +- api/v1/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 6ecaa86..2003bde 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 7f0df9e..ba1c717 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 52978b3..b8c0330 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 dc770f4..977890c 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.1.4