Bug 21002

Summary: Add Koha::Account::add_debit
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, jonathan.druart, josef.moravec, kyle, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20978
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21678
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Part of the ongoing effort to improve the maintainability of our codebase. This enhancement adds the `add_debit` method to Koha::Account as a parallel to the existing `add_credit` method. This method should be used from now on for any code dealing with the addition of debts to a patrons account.
Version(s) released in:
Bug Depends on: 19066, 21980    
Bug Blocks: 21043, 21747    
Attachments: Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account
Bug 21002: Add tests for Koha::Account::add_debit
Bug 21002: Add ->add_debit method to Koha::Account

Description Tomás Cohen Arazi 2018-06-26 17:34:09 UTC
There should be a high-level method for creating new debits, similar to Koha::Account::add_credit.

It will handle creating the line, and the offset.
Comment 1 Martin Renvoize 2018-10-30 10:23:47 UTC
Created attachment 81614 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account
Comment 2 Martin Renvoize 2018-10-30 10:24:55 UTC
Uploaded a 'Work In Progress' patch.. my intention is to start building the dependent bugs and iterate on this patch as I find new requirements when building the dependant bugs.
Comment 3 Martin Renvoize 2018-10-30 12:49:41 UTC
Created attachment 81620 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account
Comment 4 Josef Moravec 2018-10-31 08:15:47 UTC
Comment on attachment 81620 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account

Review of attachment 81620 [details] [review]:
-----------------------------------------------------------------

::: Koha/Account.pm
@@ +435,5 @@
> +  - 'lost'
> +  - 'processing'
> +  - 'management'
> +  - 'sundry'
> +  - 'card'

You are missing some types from $account_types_debit here

@@ +454,5 @@
> +    }
> +
> +    my $description  = $params->{description} // q{};
> +    my $note         = $params->{note} // q{};
> +    my $user_id      = $params->{user_id};

Should not user_id be taken from C4::Context->userenv when not available in params?

@@ +488,5 @@
> +                    amount            => $amount,
> +                    description       => $description,
> +                    accounttype       => $account_type,
> +                    amountoutstanding => $amount,
> +                    invoice_type      => $invoice_type,

There is no invoice_type column in accountlines

@@ +498,5 @@
> +
> +            # Record the account offset
> +            my $account_offset = Koha::Account::Offset->new(
> +                {   debit_id => $line->id,
> +                    type      => $Koha::Account::offset_type->{$type},

This will not work, values from $type are not defined in offset_type
Comment 5 Tomás Cohen Arazi 2018-10-31 09:56:14 UTC
(In reply to Josef Moravec from comment #4)
> > +    my $user_id      = $params->{user_id};
> 
> Should not user_id be taken from C4::Context->userenv when not available in
> params?

No, that's the caller's responability.
Comment 6 Martin Renvoize 2018-11-02 10:58:16 UTC
Created attachment 81880 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account
Comment 7 Jonathan Druart 2018-11-02 12:19:37 UTC
+$debit_type can be any of:

=> I would not repeat that. Move the global vars at the top of the file instead, maybe should be defined with constant (see C4::Serials for an existing case).
Comment 8 Martin Renvoize 2018-11-02 16:33:02 UTC
Going to rebase this little tree against 19066 as it's close to going in.
Comment 9 Martin Renvoize 2018-11-30 10:28:18 UTC
Created attachment 82772 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit
Comment 10 Martin Renvoize 2018-11-30 10:28:21 UTC
Created attachment 82773 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account
Comment 11 Martin Renvoize 2018-11-30 13:34:56 UTC
Created attachment 82785 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit
Comment 12 Martin Renvoize 2018-11-30 13:34:59 UTC
Created attachment 82786 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account
Comment 13 Martin Renvoize 2018-12-10 12:10:05 UTC
Created attachment 83009 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit
Comment 14 Martin Renvoize 2018-12-10 12:10:08 UTC
Created attachment 83010 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account
Comment 15 Martin Renvoize 2018-12-12 09:02:33 UTC
Created attachment 83086 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit

Sponsored-by: PTFS Europe
Comment 16 Martin Renvoize 2018-12-12 09:02:37 UTC
Created attachment 83087 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account

Sponsored-by: PTFS Europe
Comment 17 Martin Renvoize 2018-12-12 09:03:04 UTC
Added sponsorship lines.
Comment 18 Josef Moravec 2018-12-12 11:52:57 UTC
Created attachment 83099 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Josef Moravec 2018-12-12 11:53:01 UTC
Created attachment 83100 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 20 Jonathan Druart 2018-12-13 20:18:51 UTC
At first glance the logs will be different, how bad could be?
Comment 21 Martin Renvoize 2018-12-14 16:19:40 UTC
Created attachment 83228 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 22 Martin Renvoize 2018-12-14 16:19:44 UTC
Created attachment 83229 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 23 Martin Renvoize 2018-12-17 10:47:01 UTC
Created attachment 83273 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 24 Martin Renvoize 2018-12-17 10:47:05 UTC
Created attachment 83274 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 25 Martin Renvoize 2018-12-17 12:13:38 UTC
(In reply to Jonathan Druart from comment #20)
> At first glance the logs will be different, how bad could be?

You are completely right.. I've noted the differences in each of the dependant bugs.. as it happens I was going to try and catch you to discuss at some point today as my own opinion is now varying.
Comment 26 Tomás Cohen Arazi 2019-01-08 09:54:20 UTC
Created attachment 83706 [details] [review]
Bug 21002: Add tests for Koha::Account::add_debit

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 27 Tomás Cohen Arazi 2019-01-08 09:54:28 UTC
Created attachment 83707 [details] [review]
Bug 21002: Add ->add_debit method to Koha::Account

Sponsored-by: PTFS Europe

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 28 Nick Clemens (kidclamp) 2019-01-25 20:52:01 UTC
Awesome work all!

Pushed to master for 19.05
Comment 29 Martin Renvoize 2019-01-31 17:08:50 UTC
Enhancement, will not be backported to 18.11.x series at this time.