Bug 31631 - Optionally choose for tax-exclusive values to be used for calculating fund values (spent, ordered)
Summary: Optionally choose for tax-exclusive values to be used for calculating fund va...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Buckley
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 31840
Blocks:
  Show dependency treegraph
 
Reported: 2022-09-27 03:50 UTC by Alex Buckley
Modified: 2023-11-29 19:40 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Public and tertiary libraries in New Zealand can claim tax back on purchases. This enhancement adds a new system preference `CalculateFundValuesIncludingTax`. When set to 'Exclude' these libraries can input order prices with tax included (so vendor invoice prices - which do contain tax - can be entered directly into Koha). However, the order prices removed from the Koha funds are the tax exclusive values (NZ libraries claim tax back so it should not be removed from their funds).
Version(s) released in:
23.11.00


Attachments
Bug 31631: Add new system preference (3.82 KB, patch)
2022-09-27 04:04 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Optionally use tax-exclusive values for calculating fund values (10.67 KB, patch)
2022-09-27 04:45 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Unit tests (5.49 KB, patch)
2022-09-27 04:45 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order (1.68 KB, patch)
2022-10-25 03:14 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Screenshot - Receiving order - formatting and default values (96.16 KB, image/png)
2022-10-26 02:56 UTC, David Nind
Details
Bug 31631: Add new system preference (3.87 KB, patch)
2022-11-03 23:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 31631: Optionally use tax-exclusive values for calculating fund values (10.72 KB, patch)
2022-11-03 23:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 31631: Unit tests (5.54 KB, patch)
2022-11-03 23:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order (1.72 KB, patch)
2022-11-03 23:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 31631: Add new system preference (3.97 KB, patch)
2023-02-21 04:49 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Optionally use tax-exclusive values for calculating fund values (11.00 KB, patch)
2023-02-21 04:50 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order (1.72 KB, patch)
2023-02-21 04:50 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Unit tests (6.87 KB, patch)
2023-02-21 04:50 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Unit tests (6.31 KB, patch)
2023-02-21 04:57 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Optionally use tax-exclusive values for calculating fund values (11.00 KB, patch)
2023-02-21 22:23 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order (1.72 KB, patch)
2023-02-21 22:23 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Unit tests (12.12 KB, patch)
2023-02-21 22:23 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Fixes for QA test tool (4.64 KB, patch)
2023-02-21 22:24 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Optionally use tax-exclusive values for calculating fund values (11.19 KB, patch)
2023-02-21 23:39 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order (1.72 KB, patch)
2023-02-21 23:39 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Unit tests (12.12 KB, patch)
2023-02-21 23:40 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Fixes for QA test tool (2.49 KB, patch)
2023-02-21 23:40 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: (follow-up) Fixes for failing unit test t/db_dependent/Budgets.t (1.22 KB, patch)
2023-04-24 00:20 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31631: Add new system preference (4.03 KB, patch)
2023-09-22 10:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: Add new system preference (4.03 KB, patch)
2023-09-22 10:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: Optionally use tax-exclusive values for calculating fund values (11.25 KB, patch)
2023-09-22 10:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order (1.78 KB, patch)
2023-09-22 10:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: Unit tests (12.17 KB, patch)
2023-09-22 10:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: (follow-up) Fixes for QA test tool (2.54 KB, patch)
2023-09-22 10:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: (follow-up) Fixes for failing unit test t/db_dependent/Budgets.t (1.27 KB, patch)
2023-09-22 10:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31631: (QA follow-up) Tidy code (9.90 KB, patch)
2023-09-22 10:58 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2022-09-27 03:50:29 UTC
Public and tertiary libraries in New Zealand can claim back tax on purchases - see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10461#c33

This means these NZ libraries want to input prices with tax included (as that is what is printed on their invoices), but want the amount removed from their Koha funds to be tax exclusive (as they will be claiming back the tax). 

Bug 13324 enforced that tax-included values were always used for calculating fund values (spent, ordered), for all supplier configurations. 

It would be useful for NZ libraries if they could globally choose for tax-exclusive values to be used for calculating fund values, with the behaviour introduced by 13324 continuing as the default.
Comment 1 Alex Buckley 2022-09-27 04:04:18 UTC
Created attachment 141017 [details] [review]
Bug 31631: Add new system preference

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 2 Alex Buckley 2022-09-27 04:45:10 UTC
Created attachment 141018 [details] [review]
Bug 31631: Optionally use tax-exclusive values for calculating fund values

Test plan:
1. Apply patches
2. sudo koha-upgrade-schema <instance>
3. Restart services
4. Set TaxRates syspref = 0.15
5. Create a new fund with amount = 1000
6. Add an order to a basket with the following values:
- Fund = Choose the fund from step #5
- Quantity = 1
- Vendor price = 10
- Tax rate = 15%
- Discount = 20%
- Retail price = 10.00
- Replacement cost = 10.00
- Actual cost = 10.00
7. Go to the Acquisitions home page
8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
9. Click on the 9.20 link and confirm the ordered.pl page that loads
also contains 9.20 as the 'Subtotal'

10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
11. On the Acquisitions home page confirm the 'Ordered' column now
contains 8.00
12. Click on the 8.00 link and confirm the ordered.pl page also contains
8.00 as the 'Subtotal'

13. Close your basket and receive your order keeping all the default
values
14. Go to the Acquisitions home page and confirm the 'Spent' column now
contains 8.00
15. Click on the 8.00 link and confirm the spent.pl page also contains
8.00 as the 'Subtotal'

16. Change 'CalculateFundValuesIncludingTax' = 'Include'
17. Confirm the 'Spent' column on the Acquisitions home page now shows
9.20
18. Click on the 9.20 link and confirm the spent.pl page also contains
9.20 as the 'Subtotal'

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 3 Alex Buckley 2022-09-27 04:45:33 UTC
Created attachment 141019 [details] [review]
Bug 31631: Unit tests

Test plan:
1. Run tests in koha-shell
sudo koha-shell <instance>
prove t/db_dependent/Budgets.t

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 4 Aleisha Amohia 2022-10-25 03:14:43 UTC
Created attachment 142597 [details] [review]
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

This patch depends on Bug 31840.

To test:
1) Edit an order and increase the price so that you would expect it to
trigger a warning that you will exceed the amount allowed for this Fund
2) Confirm the warning is triggered
Comment 5 Aleisha Amohia 2022-10-25 03:15:09 UTC
Ready for testing.
Comment 6 David Nind 2022-10-26 02:56:12 UTC
Created attachment 142661 [details]
Screenshot - Receiving order - formatting and default values

Testing notes and issues/query (tested using koha-testing-docker).

Three things I noticed:

1. Tax Rate system preference warning message
---------------------------------------------

Even though I defined the Tax Rate system preference as 0.15, when adding an item to a basket: for tax rate there is a message saying "Tax rate not defined in system preference TaxRates!".

Despite the warning, everything is calculated correctly using the 15% tax rate. 

I've logged bug 31984 for this, as this was happening before the patches were applied (includes a screenshot to show what I mean).

2. Step 13 - Formatting things
------------------------------

When receiving order (see screenshot):

- For the Retail price field: no space after value and the opening bracket; no space between comma and tax exclusive

- For the Budgeted cost field: no space after value and the opening bracket

3. For step 13 - Query about using the default values
-----------------------------------------------------

I received the order with the default values (see the screenshot). As the actual cost defaulted to $10 (as entered in step 6), then the 10.00 is the value excluding tax, so:
- For step 14: spent = 10.00
- For step 15: subtotal = 10.00
- For step 17: spent = 11.50
- For step 18: subtotal = 11.50

These values are correctly calculated, but don't match up with the test plan. So should I have entered the actual costs as 8.00 (the $10 retail price less the 20% discount, tax exclusive)?
Comment 7 Aleisha Amohia 2022-11-03 22:11:15 UTC
Thanks for testing David

(In reply to David Nind from comment #6)
> Created attachment 142661 [details]
> Screenshot - Receiving order - formatting and default values
> 
> Testing notes and issues/query (tested using koha-testing-docker).
> 
> Three things I noticed:
> 
> 1. Tax Rate system preference warning message
> ---------------------------------------------
> 

Unrelated (exists before patches), see bug 31984 

> 
> 2. Step 13 - Formatting things
> ------------------------------
> 

Unrelated (exists before patches), maybe open another bug for this?

> 
> 3. For step 13 - Query about using the default values
> -----------------------------------------------------
> 
> These values are correctly calculated, but don't match up with the test
> plan. So should I have entered the actual costs as 8.00 (the $10 retail
> price less the 20% discount, tax exclusive)?

Yes I think that's right sorry!
Comment 8 David Nind 2022-11-03 23:22:45 UTC
Created attachment 143156 [details] [review]
Bug 31631: Add new system preference

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2022-11-03 23:22:50 UTC
Created attachment 143157 [details] [review]
Bug 31631: Optionally use tax-exclusive values for calculating fund values

Test plan:
1. Apply patches
2. sudo koha-upgrade-schema <instance>
3. Restart services
4. Set TaxRates syspref = 0.15
5. Create a new fund with amount = 1000
6. Add an order to a basket with the following values:
- Fund = Choose the fund from step #5
- Quantity = 1
- Vendor price = 10
- Tax rate = 15%
- Discount = 20%
- Retail price = 10.00
- Replacement cost = 10.00
- Actual cost = 10.00
7. Go to the Acquisitions home page
8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
9. Click on the 9.20 link and confirm the ordered.pl page that loads
also contains 9.20 as the 'Subtotal'

10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
11. On the Acquisitions home page confirm the 'Ordered' column now
contains 8.00
12. Click on the 8.00 link and confirm the ordered.pl page also contains
8.00 as the 'Subtotal'

13. Close your basket and receive your order keeping all the default
values
14. Go to the Acquisitions home page and confirm the 'Spent' column now
contains 8.00
15. Click on the 8.00 link and confirm the spent.pl page also contains
8.00 as the 'Subtotal'

16. Change 'CalculateFundValuesIncludingTax' = 'Include'
17. Confirm the 'Spent' column on the Acquisitions home page now shows
9.20
18. Click on the 9.20 link and confirm the spent.pl page also contains
9.20 as the 'Subtotal'

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2022-11-03 23:22:55 UTC
Created attachment 143158 [details] [review]
Bug 31631: Unit tests

Test plan:
1. Run tests in koha-shell
sudo koha-shell <instance>
prove t/db_dependent/Budgets.t

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2022-11-03 23:23:00 UTC
Created attachment 143159 [details] [review]
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

This patch depends on Bug 31840.

To test:
1) Edit an order and increase the price so that you would expect it to
trigger a warning that you will exceed the amount allowed for this Fund
2) Confirm the warning is triggered

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Kyle M Hall 2022-12-02 14:02:26 UTC
Unit tests are failing randomly:

root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 153/154 Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 515.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 526.
Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 515.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 526.

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1243.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1247.
    #          got: '169.44'
    #     expected: '157.6'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1270.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1272.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1276.
    #          got: '169.44'
    #     expected: '157.6'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1278.
    #          got: '169.44'
    #     expected: '157.6'
    # Looks like you failed 6 tests of 34.
t/db_dependent/Budgets.t .. 154/154
#   Failed test 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell FieldsForCalculatingFundValues tests'
#   at t/db_dependent/Budgets.t line 1290.
# Looks like you failed 1 test of 154.
t/db_dependent/Budgets.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/154 subtests

Test Summary Report
-------------------
t/db_dependent/Budgets.t (Wstat: 256 Tests: 154 Failed: 1)
  Failed test:  154
  Non-zero exit status: 1
Files=1, Tests=154,  4 wallclock secs ( 0.02 usr  0.00 sys +  3.43 cusr  0.65 csys =  4.10 CPU)
Result: FAIL
root@kohadevbox:koha(bug31631-qa)$ git log
commit 2a3c15c91467b01802c31a65f3436c9175ef1103 (HEAD -> bug31631-qa)
Author: Aleisha Amohia <aleisha@catalystacademy.net.nz>
Date:   Tue Oct 25 03:08:03 2022 +0000

    Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

    This patch depends on Bug 31840.

    To test:
    1) Edit an order and increase the price so that you would expect it to
    trigger a warning that you will exceed the amount allowed for this Fund
    2) Confirm the warning is triggered

    Signed-off-by: David Nind <david@davidnind.com>

commit cc6bcfe54ea55691bd4ed749b58fac2f9331f8fd
Author: Alex Buckley <alexbuckley@catalyst.net.nz>
Date:   Tue Sep 27 17:01:51 2022 +1300

    Bug 31631: Unit tests

    Test plan:
    1. Run tests in koha-shell
    sudo koha-shell <instance>
    prove t/db_dependent/Budgets.t

    Sponsored-by: Waikato Institute of Technology, New Zealand

    Signed-off-by: David Nind <david@davidnind.com>

commit a50a301eacde061376c6a3c0cafc03b56e8d1604
Author: Alex Buckley <alexbuckley@catalyst.net.nz>
Date:   Tue Sep 27 16:59:00 2022 +1300

    Bug 31631: Optionally use tax-exclusive values for calculating fund values

    Test plan:
    1. Apply patches
    2. sudo koha-upgrade-schema <instance>
    3. Restart services
    4. Set TaxRates syspref = 0.15
    5. Create a new fund with amount = 1000
    6. Add an order to a basket with the following values:
    - Fund = Choose the fund from step #5
    - Quantity = 1
    - Vendor price = 10
    - Tax rate = 15%
    - Discount = 20%
    - Retail price = 10.00
    - Replacement cost = 10.00
    - Actual cost = 10.00
    7. Go to the Acquisitions home page
    8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
    9. Click on the 9.20 link and confirm the ordered.pl page that loads
    also contains 9.20 as the 'Subtotal'

    10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 153/154 Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.
Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.
t/db_dependent/Budgets.t .. ok
All tests successful.
Files=1, Tests=154,  4 wallclock secs ( 0.02 usr  0.01 sys +  2.99 cusr  0.54 csys =  3.56 CPU)
Result: PASS
root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 153/154 Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 515.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 526.
Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 515.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 526.

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1243.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1247.
    #          got: '169.44'
    #     expected: '157.6'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1270.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1272.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1276.
    #          got: '169.44'
    #     expected: '157.6'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1278.
    #          got: '169.44'
    #     expected: '157.6'
    # Looks like you failed 6 tests of 34.
t/db_dependent/Budgets.t .. 154/154
#   Failed test 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell FieldsForCalculatingFundValues tests'
#   at t/db_dependent/Budgets.t line 1290.
# Looks like you failed 1 test of 154.
t/db_dependent/Budgets.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/154 subtests

Test Summary Report
-------------------
t/db_dependent/Budgets.t (Wstat: 256 Tests: 154 Failed: 1)
  Failed test:  154
  Non-zero exit status: 1
Files=1, Tests=154,  5 wallclock secs ( 0.02 usr  0.01 sys +  3.41 cusr  0.68 csys =  4.12 CPU)
Result: FAIL
root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 2/154 Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 515.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 526.
Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 515.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 526.

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1243.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1247.
    #          got: '169.44'
    #     expected: '157.6'
    # Looks like you failed 2 tests of 34.
t/db_dependent/Budgets.t .. 154/154
#   Failed test 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell FieldsForCalculatingFundValues tests'
#   at t/db_dependent/Budgets.t line 1290.
# Looks like you failed 1 test of 154.
t/db_dependent/Budgets.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/154 subtests

Test Summary Report
-------------------
t/db_dependent/Budgets.t (Wstat: 256 Tests: 154 Failed: 1)
  Failed test:  154
  Non-zero exit status: 1
Files=1, Tests=154,  2 wallclock secs ( 0.02 usr  0.00 sys +  2.17 cusr  0.24 csys =  2.43 CPU)
Result: FAIL
root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 100/154 Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.
Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1270.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1272.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1276.
    #          got: '169.44'
    #     expected: '157.6'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1278.
    #          got: '169.44'
    #     expected: '157.6'
    # Looks like you failed 4 tests of 34.
t/db_dependent/Budgets.t .. 154/154
#   Failed test 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell FieldsForCalculatingFundValues tests'
#   at t/db_dependent/Budgets.t line 1290.
# Looks like you failed 1 test of 154.
t/db_dependent/Budgets.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/154 subtests

Test Summary Report
-------------------
t/db_dependent/Budgets.t (Wstat: 256 Tests: 154 Failed: 1)
  Failed test:  154
  Non-zero exit status: 1
Files=1, Tests=154,  3 wallclock secs ( 0.03 usr  0.00 sys +  2.31 cusr  0.37 csys =  2.71 CPU)
Result: FAIL
root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 2/154 Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.
Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1270.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1272.
    #          got: '157.6'
    #     expected: '147.36'

    #   Failed test 'We expect this to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1276.
    #          got: '169.44'
    #     expected: '157.6'

    #   Failed test 'We expect this value to be the tax exclusive value'
    #   at t/db_dependent/Budgets.t line 1278.
    #          got: '169.44'
    #     expected: '157.6'
    # Looks like you failed 4 tests of 34.
t/db_dependent/Budgets.t .. 154/154
#   Failed test 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell FieldsForCalculatingFundValues tests'
#   at t/db_dependent/Budgets.t line 1290.
# Looks like you failed 1 test of 154.
t/db_dependent/Budgets.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/154 subtests

Test Summary Report
-------------------
t/db_dependent/Budgets.t (Wstat: 256 Tests: 154 Failed: 1)
  Failed test:  154
  Non-zero exit status: 1
Files=1, Tests=154,  2 wallclock secs ( 0.03 usr  0.00 sys +  2.09 cusr  0.28 csys =  2.40 CPU)
Result: FAIL
root@kohadevbox:koha(bug31631-qa)$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 2/154 Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.
Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 490.
t/db_dependent/Budgets.t .. ok
All tests successful.
Files=1, Tests=154,  2 wallclock secs ( 0.01 usr  0.00 sys +  2.15 cusr  0.23 csys =  2.39 CPU)
Result: PASS
Comment 13 Alex Buckley 2023-02-21 04:49:44 UTC
Created attachment 147011 [details] [review]
Bug 31631: Add new system preference

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 Alex Buckley 2023-02-21 04:50:00 UTC
Created attachment 147012 [details] [review]
Bug 31631: Optionally use tax-exclusive values for calculating fund values

Test plan:
1. Apply patches
2. sudo koha-upgrade-schema <instance>
3. Restart services
4. Set TaxRates syspref = 0.15
5. Create a new fund with amount = 1000
6. Add an order to a basket with the following values:
- Fund = Choose the fund from step #5
- Quantity = 1
- Vendor price = 10
- Tax rate = 15%
- Discount = 20%
- Retail price = 10.00
- Replacement cost = 10.00
- Actual cost = 10.00
7. Go to the Acquisitions home page
8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
9. Click on the 9.20 link and confirm the ordered.pl page that loads
also contains 9.20 as the 'Subtotal'

10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
11. On the Acquisitions home page confirm the 'Ordered' column now
contains 8.00
12. Click on the 8.00 link and confirm the ordered.pl page also contains
8.00 as the 'Subtotal'

13. Close your basket and receive your order keeping all the default
values
14. Go to the Acquisitions home page and confirm the 'Spent' column now
contains 8.00
15. Click on the 8.00 link and confirm the spent.pl page also contains
8.00 as the 'Subtotal'

16. Change 'CalculateFundValuesIncludingTax' = 'Include'
17. Confirm the 'Spent' column on the Acquisitions home page now shows
9.20
18. Click on the 9.20 link and confirm the spent.pl page also contains
9.20 as the 'Subtotal'

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 Alex Buckley 2023-02-21 04:50:17 UTC
Created attachment 147013 [details] [review]
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

This patch depends on Bug 31840.

To test:
1) Edit an order and increase the price so that you would expect it to
trigger a warning that you will exceed the amount allowed for this Fund
2) Confirm the warning is triggered

Signed-off-by: David Nind <david@davidnind.com>
Comment 16 Alex Buckley 2023-02-21 04:50:55 UTC
Created attachment 147014 [details] [review]
Bug 31631: Unit tests

Test plan:
1. Run tests in koha-shell
sudo koha-shell <instance>
prove t/db_dependent/Budgets.t

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 17 Alex Buckley 2023-02-21 04:57:35 UTC
Created attachment 147015 [details] [review]
Bug 31631: Unit tests

Test plan:
1. Run tests in koha-shell
sudo koha-shell <instance>
prove t/db_dependent/Budgets.t

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 18 Alex Buckley 2023-02-21 22:23:25 UTC
Created attachment 147098 [details] [review]
Bug 31631: Optionally use tax-exclusive values for calculating fund values

Test plan:
1. Apply patches
2. sudo koha-upgrade-schema <instance>
3. Restart services
4. Set TaxRates syspref = 0.15
5. Create a new fund with amount = 1000
6. Add an order to a basket with the following values:
- Fund = Choose the fund from step #5
- Quantity = 1
- Vendor price = 10
- Tax rate = 15%
- Discount = 20%
- Retail price = 10.00
- Replacement cost = 10.00
- Actual cost = 10.00
7. Go to the Acquisitions home page
8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
9. Click on the 9.20 link and confirm the ordered.pl page that loads
also contains 9.20 as the 'Subtotal'

10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
11. On the Acquisitions home page confirm the 'Ordered' column now
contains 8.00
12. Click on the 8.00 link and confirm the ordered.pl page also contains
8.00 as the 'Subtotal'

13. Close your basket and receive your order keeping all the default
values
14. Go to the Acquisitions home page and confirm the 'Spent' column now
contains 8.00
15. Click on the 8.00 link and confirm the spent.pl page also contains
8.00 as the 'Subtotal'

16. Change 'CalculateFundValuesIncludingTax' = 'Include'
17. Confirm the 'Spent' column on the Acquisitions home page now shows
9.20
18. Click on the 9.20 link and confirm the spent.pl page also contains
9.20 as the 'Subtotal'

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 19 Alex Buckley 2023-02-21 22:23:41 UTC
Created attachment 147099 [details] [review]
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

This patch depends on Bug 31840.

To test:
1) Edit an order and increase the price so that you would expect it to
trigger a warning that you will exceed the amount allowed for this Fund
2) Confirm the warning is triggered

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 Alex Buckley 2023-02-21 22:23:58 UTC
Created attachment 147100 [details] [review]
Bug 31631: Unit tests

Test plan:
Run amended Budgets.t test and all existing unchanged Acquisition tests in koha-shell
- sudo koha-shell <instance>
- prove t/db_dependent/Budgets.t
- prove t/db_dependent/Acquisition
- prove t/db_dependent/Acquisition.t
- prove t/db_dependent/Budgets

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 21 Alex Buckley 2023-02-21 22:24:21 UTC
Created attachment 147101 [details] [review]
Bug 31631: (follow-up) Fixes for QA test tool
Comment 22 Alex Buckley 2023-02-21 23:39:28 UTC
Created attachment 147102 [details] [review]
Bug 31631: Optionally use tax-exclusive values for calculating fund values

Test plan:
1. Apply patches
2. sudo koha-upgrade-schema <instance>
3. Restart services
4. Set TaxRates syspref = 0.15
5. Create a new fund with amount = 1000
6. Add an order to a basket with the following values:
- Fund = Choose the fund from step #5
- Quantity = 1
- Vendor price = 10
- Tax rate = 15%
- Discount = 20%
- Retail price = 10.00
- Replacement cost = 10.00
- Actual cost = 10.00
- Ensure the basket belongs to a vendor with ("List prices" = "Don't include tax" and "Invoice prices" = "Don't include tax")
7. Go to the Acquisitions home page
8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
9. Click on the 9.20 link and confirm the ordered.pl page that loads
also contains 9.20 as the 'Subtotal'

10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
11. On the Acquisitions home page confirm the 'Ordered' column now
contains 8.00
12. Click on the 8.00 link and confirm the ordered.pl page also contains
8.00 as the 'Subtotal'

13. Close your basket and receive your order - change the 'Actual costs'
= 8.00 (the $10 retail price less the 20% discount, tax exclusive)
14. Go to the Acquisitions home page and confirm the 'Spent' column now
contains 8.00
15. Click on the 8.00 link and confirm the spent.pl page also contains
8.00 as the 'Subtotal'

16. Change 'CalculateFundValuesIncludingTax' = 'Include'
17. Confirm the 'Spent' column on the Acquisitions home page now shows
9.20
18. Click on the 9.20 link and confirm the spent.pl page also contains
9.20 as the 'Subtotal'

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 Alex Buckley 2023-02-21 23:39:47 UTC
Created attachment 147103 [details] [review]
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

This patch depends on Bug 31840.

To test:
1) Edit an order and increase the price so that you would expect it to
trigger a warning that you will exceed the amount allowed for this Fund
2) Confirm the warning is triggered

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 Alex Buckley 2023-02-21 23:40:01 UTC
Created attachment 147104 [details] [review]
Bug 31631: Unit tests

Test plan:
Run amended Budgets.t test and all existing unchanged Acquisition tests in koha-shell
- sudo koha-shell <instance>
- prove t/db_dependent/Budgets.t
- prove t/db_dependent/Acquisition
- prove t/db_dependent/Acquisition.t
- prove t/db_dependent/Budgets

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 25 Alex Buckley 2023-02-21 23:40:21 UTC
Created attachment 147105 [details] [review]
Bug 31631: (follow-up) Fixes for QA test tool
Comment 26 Alex Buckley 2023-02-21 23:55:35 UTC
Setting this back to 'Signed off'. I have done the following:

1. Corrected the test plan on the second patch so that it reflects the point David raised in comment #6

2. Submitted a fifth patch fixing QA test tool failures. I am still getting one failure for the template on ordered.tt, but I think that might just be my qa test tools. 

3. Fixed the unit tests and expanded the unit test coverage - we now test both when vendors have listincgst/invoiceincgst set to 0 and 1.

The unit tests now consistently pass:

"
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 153/156 Use of uninitialized value in addition (+) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 495.
t/db_dependent/Budgets.t .. 154/156 Use of uninitialized value in multiplication (*) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 520.
Use of uninitialized value in numeric eq (==) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 531.
t/db_dependent/Budgets.t .. 155/156 Use of uninitialized value in addition (+) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 495.
t/db_dependent/Budgets.t .. ok       
All tests successful.
Files=1, Tests=156, 12 wallclock secs ( 0.05 usr  0.01 sys +  9.22 cusr  2.01 csys = 11.29 CPU)
Result: PASS
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 153/156 Use of uninitialized value in multiplication (*) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 520.
Use of uninitialized value in numeric eq (==) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 531.
t/db_dependent/Budgets.t .. 154/156 Use of uninitialized value in multiplication (*) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 520.
Use of uninitialized value in numeric eq (==) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 531.
t/db_dependent/Budgets.t .. 155/156 Use of uninitialized value in addition (+) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 495.
t/db_dependent/Budgets.t .. ok       
All tests successful.
Files=1, Tests=156, 11 wallclock secs ( 0.04 usr  0.02 sys +  9.10 cusr  1.50 csys = 10.66 CPU)
Result: PASS
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Budgets.t
t/db_dependent/Budgets.t .. 153/156 Use of uninitialized value in multiplication (*) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 520.
Use of uninitialized value in numeric eq (==) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 531.
t/db_dependent/Budgets.t .. 154/156 Use of uninitialized value in multiplication (*) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 520.
Use of uninitialized value in numeric eq (==) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 531.
t/db_dependent/Budgets.t .. 155/156 Use of uninitialized value in addition (+) at /home/vagrant/kohaclone/Koha/Acquisition/Order.pm line 495.
t/db_dependent/Budgets.t .. ok       
All tests successful.
Files=1, Tests=156, 10 wallclock secs ( 0.02 usr  0.02 sys +  7.34 cusr  1.27 csys =  8.65 CPU)
Result: PASS
"
Comment 27 Marcel de Rooy 2023-04-14 07:46:11 UTC
# Looks like you failed 4 tests of 156.

#   Failed test 'total spent for budget1 is 160'
#   at t/db_dependent/Budgets.t line 436.
#          got: '0'
#     expected: '160'

#   Failed test 'total spent for budget11 is 100'
#   at t/db_dependent/Budgets.t line 437.
#          got: '0'
#     expected: '100'

#   Failed test 'total spent for budget111 is 20'
#   at t/db_dependent/Budgets.t line 438.
#          got: '0'
#     expected: '20'

#   Failed test 'total ordered price is 20'
#   at t/db_dependent/Budgets.t line 519.
#          got: '0'
#     expected: '20'
Comment 28 Alex Buckley 2023-04-24 00:20:24 UTC
Created attachment 150146 [details] [review]
Bug 31631: (follow-up) Fixes for failing unit test t/db_dependent/Budgets.t

Test plan:
1. Set CalculateFundValuesIncludingTax = 'Include'

2. Run Budgets.t unit test
sudo koha-shell kohadev
prove t/db_dependent/Budgets.t

3. Set CalculateFundValuesIncludingTax = 'Exclude'

4. Run Budgets.t unit test
sudo koha-shell kohadev
prove t/db_dependent/Budgets.t

Sponsored-By: Waikato Institute of Technology, New Zealand
Comment 29 Alex Buckley 2023-04-24 00:21:25 UTC
(In reply to Marcel de Rooy from comment #27)
> # Looks like you failed 4 tests of 156.
> 
> #   Failed test 'total spent for budget1 is 160'
> #   at t/db_dependent/Budgets.t line 436.
> #          got: '0'
> #     expected: '160'
> 
> #   Failed test 'total spent for budget11 is 100'
> #   at t/db_dependent/Budgets.t line 437.
> #          got: '0'
> #     expected: '100'
> 
> #   Failed test 'total spent for budget111 is 20'
> #   at t/db_dependent/Budgets.t line 438.
> #          got: '0'
> #     expected: '20'
> 
> #   Failed test 'total ordered price is 20'
> #   at t/db_dependent/Budgets.t line 519.
> #          got: '0'
> #     expected: '20'

Thanks Marcel. 

I've attached a followup fixing the Budgets.t unit test when  CalculateFundValuesIncludingTax is set to 'Include' or 'Exclude'.
Comment 30 Kyle M Hall 2023-09-22 10:57:33 UTC
Created attachment 156062 [details] [review]
Bug 31631: Add new system preference

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 31 Kyle M Hall 2023-09-22 10:57:50 UTC
Created attachment 156063 [details] [review]
Bug 31631: Add new system preference

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Kyle M Hall 2023-09-22 10:57:56 UTC
Created attachment 156064 [details] [review]
Bug 31631: Optionally use tax-exclusive values for calculating fund values

Test plan:
1. Apply patches
2. sudo koha-upgrade-schema <instance>
3. Restart services
4. Set TaxRates syspref = 0.15
5. Create a new fund with amount = 1000
6. Add an order to a basket with the following values:
- Fund = Choose the fund from step #5
- Quantity = 1
- Vendor price = 10
- Tax rate = 15%
- Discount = 20%
- Retail price = 10.00
- Replacement cost = 10.00
- Actual cost = 10.00
- Ensure the basket belongs to a vendor with ("List prices" = "Don't include tax" and "Invoice prices" = "Don't include tax")
7. Go to the Acquisitions home page
8. Confirm the 'Ordered' column for the step #5 fund contains 9.20
9. Click on the 9.20 link and confirm the ordered.pl page that loads
also contains 9.20 as the 'Subtotal'

10. Change 'CalculateFundValuesIncludingTax' = 'Exclude'
11. On the Acquisitions home page confirm the 'Ordered' column now
contains 8.00
12. Click on the 8.00 link and confirm the ordered.pl page also contains
8.00 as the 'Subtotal'

13. Close your basket and receive your order - change the 'Actual costs'
= 8.00 (the $10 retail price less the 20% discount, tax exclusive)
14. Go to the Acquisitions home page and confirm the 'Spent' column now
contains 8.00
15. Click on the 8.00 link and confirm the spent.pl page also contains
8.00 as the 'Subtotal'

16. Change 'CalculateFundValuesIncludingTax' = 'Include'
17. Confirm the 'Spent' column on the Acquisitions home page now shows
9.20
18. Click on the 9.20 link and confirm the spent.pl page also contains
9.20 as the 'Subtotal'

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2023-09-22 10:57:58 UTC
Created attachment 156065 [details] [review]
Bug 31631: (follow-up) Use CalculateFundValuesIncludingTax when modifying existing order

This patch depends on Bug 31840.

To test:
1) Edit an order and increase the price so that you would expect it to
trigger a warning that you will exceed the amount allowed for this Fund
2) Confirm the warning is triggered

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2023-09-22 10:58:01 UTC
Created attachment 156066 [details] [review]
Bug 31631: Unit tests

Test plan:
Run amended Budgets.t test and all existing unchanged Acquisition tests in koha-shell
- sudo koha-shell <instance>
- prove t/db_dependent/Budgets.t
- prove t/db_dependent/Acquisition
- prove t/db_dependent/Acquisition.t
- prove t/db_dependent/Budgets

Sponsored-by: Waikato Institute of Technology, New Zealand

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2023-09-22 10:58:03 UTC
Created attachment 156067 [details] [review]
Bug 31631: (follow-up) Fixes for QA test tool

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2023-09-22 10:58:05 UTC
Created attachment 156068 [details] [review]
Bug 31631: (follow-up) Fixes for failing unit test t/db_dependent/Budgets.t

Test plan:
1. Set CalculateFundValuesIncludingTax = 'Include'

2. Run Budgets.t unit test
sudo koha-shell kohadev
prove t/db_dependent/Budgets.t

3. Set CalculateFundValuesIncludingTax = 'Exclude'

4. Run Budgets.t unit test
sudo koha-shell kohadev
prove t/db_dependent/Budgets.t

Sponsored-By: Waikato Institute of Technology, New Zealand

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Kyle M Hall 2023-09-22 10:58:08 UTC
Created attachment 156069 [details] [review]
Bug 31631: (QA follow-up) Tidy code

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Tomás Cohen Arazi 2023-10-18 18:43:54 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 39 Fridolin Somers 2023-10-25 08:01:58 UTC
Enhancement not pushed to 23.05.x