Bug 21282 - Ordered/spent lists should use prices including tax for calculations
Summary: Ordered/spent lists should use prices including tax for calculations
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 21427
  Show dependency treegraph
 
Reported: 2018-08-28 21:56 UTC by Katrin Fischer
Modified: 2018-11-28 15:17 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Corrects the prices shown on the ordered/spent lists for each fund in acquisitions to show the price with taxes included. This will make the total shown on these pages match the total shown in the table on the acq start and fund pages.


Attachments
Bug 21282: Sync ordered and spent values with acqui home (3.22 KB, patch)
2018-08-29 20:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21282: Sync ordered and spent values with acqui home (4.43 KB, patch)
2018-10-23 12:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21282: Sync ordered and spent values with acqui home (4.49 KB, patch)
2018-11-01 11:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21282: Sync ordered and spent values with acqui home (4.55 KB, patch)
2018-11-13 09:50 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-08-28 21:56:02 UTC
The order list for each fund calculates using the ecost tax excl., but it should be tax incl.
At the moment this means that the sum on the start page table and the order list don't match up.
Comment 1 Jonathan Druart 2018-08-29 20:27:33 UTC
Created attachment 78293 [details] [review]
Bug 21282: Sync ordered and spent values with acqui home

The order list for each fund calculates using the ecost tax excl., but
it should be tax incl.
At the moment this means that the sum on the start page table and the
order list don't match up.

Test plan:
- Create and receive orders
- Values on acqui home and ordered/spent should be the same
Comment 2 Katrin Fischer 2018-08-30 06:01:00 UTC
Patch doesn't apply (should have a html filter?) and should be formatted. Currently displays with more than 2 decimals on ordered list.

Was not sure why it conflicted already.
Comment 3 Katrin Fischer 2018-10-23 08:43:06 UTC
Jonathan, could you have another look here?
Comment 4 Jonathan Druart 2018-10-23 12:01:54 UTC
Created attachment 81003 [details] [review]
Bug 21282: Sync ordered and spent values with acqui home

The order list for each fund calculates using the ecost tax excl., but
it should be tax incl.
At the moment this means that the sum on the start page table and the
order list don't match up.

Test plan:
- Create and receive orders
- Values on acqui home and ordered/spent should be the same
Comment 5 Katrin Fischer 2018-11-01 11:57:22 UTC
Created attachment 81794 [details] [review]
Bug 21282: Sync ordered and spent values with acqui home

The order list for each fund calculates using the ecost tax excl., but
it should be tax incl.
At the moment this means that the sum on the start page table and the
order list don't match up.

Test plan:
- Create and receive orders
- Values on acqui home and ordered/spent should be the same

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Martin Renvoize 2018-11-13 09:50:27 UTC
Created attachment 82252 [details] [review]
Bug 21282: Sync ordered and spent values with acqui home

The order list for each fund calculates using the ecost tax excl., but
it should be tax incl.
At the moment this means that the sum on the start page table and the
order list don't match up.

Test plan:
- Create and receive orders
- Values on acqui home and ordered/spent should be the same

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2018-11-13 09:51:17 UTC
I'm happy to pass qa on this.  It brings more consistency and reducing confusion and there are no regressions that I can see.
Comment 8 Martin Renvoize 2018-11-13 09:52:02 UTC
I think a note on the release notes wouldn't go amiss however.. as it could be a surprising/confusing change.
Comment 9 Katrin Fischer 2018-11-13 10:16:46 UTC
(In reply to Martin Renvoize from comment #8)
> I think a note on the release notes wouldn't go amiss however.. as it could
> be a surprising/confusing change.

I think it's a clear bug - the total on the acq home page is correct, the total on these pages is wrong. There is a clear mismatch that needs to be fixed.
Comment 10 Martin Renvoize 2018-11-13 20:42:55 UTC
Totally agree, thanks for adding the release note to clarify the change/correction.
Comment 11 Nick Clemens 2018-11-13 21:15:26 UTC
Awesome work all!

Pushed to master for 18.11
Comment 12 Martin Renvoize 2018-11-23 16:01:05 UTC
Pushed to 18.05.x for 18.05.06
Comment 13 Fridolin SOMERS 2018-11-28 14:48:47 UTC
I'm not sure it should be pushed to 17.11.x.
The change of display may not be wanted on a very stable branch.
Comment 14 Katrin Fischer 2018-11-28 15:12:45 UTC
Hi Frido, the amounts are wrong - this is a bug fix, not for prettiness! :)
Comment 15 Fridolin SOMERS 2018-11-28 15:17:00 UTC
The fields in the db are all there in 17.11 so I push.

Pushed to 17.11.x for 17.11.13