Bug 21387 - Receive items from - form should include tax hints the same as the ordering form
Summary: Receive items from - form should include tax hints the same as the ordering form
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks: 21619 21929 22225
  Show dependency treegraph
 
Reported: 2018-09-20 15:35 UTC by Martin Renvoize (ashimema)
Modified: 2020-01-06 20:15 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 21387: Receive items from - form should include tax hints (2.12 KB, patch)
2018-09-21 10:45 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 21387: Receive items from - form should include tax hints (2.16 KB, patch)
2018-10-01 17:16 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 21387: Receive items from - form should include tax hints (3.38 KB, patch)
2018-10-01 17:32 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 21387: Receive items from - form should include tax hints (3.39 KB, patch)
2018-10-19 11:50 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 21387: Receive items from - form should include tax hints (3.50 KB, patch)
2018-10-30 20:31 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 21387: Receive items from - form should include tax hints (3.56 KB, patch)
2018-11-02 18:38 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 21387: Receive items from - form should include tax hints (3.64 KB, patch)
2018-11-06 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2018-09-20 15:35:08 UTC
On the 'New order' page in the 'Accounting details' section of the form there are clarifications on which amount (with/without tax) one is expected to enter.  These clarifications should be replicated to the 'Accounting details' form on the 'Receive items from: ' page.
Comment 1 Martin Renvoize (ashimema) 2018-09-21 10:45:32 UTC
Created attachment 79212 [details] [review]
Bug 21387: Receive items from - form should include tax hints
Comment 2 Martin Renvoize (ashimema) 2018-09-21 10:48:37 UTC
Would appreciate you're eye on the UI here Owen :)
Comment 3 Owen Leonard 2018-10-01 13:41:41 UTC
We usually use a "hint" class for adding additional information to a form or similar display. To my eye it looks good inline, so <span class="hint"> around the new text.

I think it would be better not to abbreviate "inc" and "exc."

One problem I saw: In my test system I got this:

Retail price:
18.58 Adjusted for , tax exc. 

I don't know why it isn't displaying the active currency.

Also: This will require a rebase on top of Bug 21454, Remove html filter for Price filtered variables.
Comment 4 Martin Renvoize (ashimema) 2018-10-01 17:16:52 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize (ashimema) 2018-10-01 17:32:43 UTC
Created attachment 79753 [details] [review]
Bug 21387: Receive items from - form should include tax hints
Comment 6 Martin Renvoize (ashimema) 2018-10-01 17:34:56 UTC
Thanks for the review Owen.

I've added the "hint" classes and correct the missing currency.. seems I forgot to commit the corresponding script change.. oops.

As for the abbreviation, I was trying to make it as close to the hints on the orderempty form as possible.  For consistency would you prefer I change both templates or leave hat to a second bug perhaps?
Comment 7 Owen Leonard 2018-10-18 17:33:37 UTC
I think a separate bug for expanding those abbreviations would be good. Unless there is a difficult space restraint my vote would be to avoid abbreviations whenever we can.
Comment 8 Martin Renvoize (ashimema) 2018-10-19 11:48:02 UTC
OK, I've created a dependent bug and asked for guidance on the approved wording at bug 21619
Comment 9 Martin Renvoize (ashimema) 2018-10-19 11:50:56 UTC
Created attachment 80930 [details] [review]
Bug 21387: Receive items from - form should include tax hints
Comment 10 Caroline Cyr La Rose 2018-10-19 18:06:45 UTC
I ran into a problem. Here's what I did:

1) Before starting, my vendor had both "list prices" and "invoice prices" set to "don't include tax".
2) I created an order line. The hints were "tax exc.", which is ok
3) I received an item. The hints were "tax exc.", which is ok
4) I went in my vendor and changed the "list prices" and "invoices princes" and set them to "include tax"
5) I created an order line. The hints, specifically "vendor price", "retail price", "budgeted cost" and "actual cost" were "tax inc.", which is ok
6) I received the same item. The hints, specifically "retail price", "budgeted cost" and "actual cost", were "tax exc." (but the same price was written as when I ordered, tax included).
Comment 11 Martin Renvoize (ashimema) 2018-10-24 09:47:38 UTC
Any thoughts on the above Jonathan?

I'm going to try to replicate this today, if I can it feels like a bug higher up the stack that this patch, but I'm not sure how it was intended to work.
Comment 12 Jonathan Druart 2018-10-24 15:27:49 UTC
(In reply to Caroline Cyr La Rose from comment #10)
> I ran into a problem. Here's what I did:

> 4) I went in my vendor and changed the "list prices" and "invoices princes"
> and set them to "include tax"

This step should not be done I would say, once they have been set you should not change these values.
Comment 13 Caroline Cyr La Rose 2018-10-24 17:29:59 UTC
So I should try with another vendor to see the different links?

Also, if we're not supposed to change the vendor tax settings, they shouldn't be editable.
Comment 14 Jonathan Druart 2018-10-24 17:40:49 UTC
(In reply to Caroline Cyr La Rose from comment #13)
> So I should try with another vendor to see the different links?
> 
> Also, if we're not supposed to change the vendor tax settings, they
> shouldn't be editable.

Yes I think they should not be editable unless there is no order yet.
Comment 15 Martin Renvoize (ashimema) 2018-10-25 07:46:01 UTC
Thanks for your comments Jonathan, my gut feeling was the same but as you wrote most of the current tax handling code I thought you'd be best to weight in :).

So perhaps a new bug to prevent editing of that setting once an order exists against the vendor... or a way to record which way the setting was set at the time of order.. either way, it feels like a new bug and shouldn't prevent this one moving forward in my opinion.
Comment 16 Caroline Cyr La Rose 2018-10-26 18:54:02 UTC
I created a new vendor and set the tax parameters to "include tax" for both invoice price and list price.

When I order an item from this vendor, hints are ok (tax inc.)

But when I receive the same item from this vendor, the tax hints are wrong (tax exc.)

Am I understanding what this is supposed to do?
Comment 17 Martin Renvoize (ashimema) 2018-10-30 20:31:43 UTC
Created attachment 81639 [details] [review]
Bug 21387: Receive items from - form should include tax hints
Comment 18 Martin Renvoize (ashimema) 2018-10-30 20:34:45 UTC
Good catch Caroline.. I'd completely missed passing a couple of variables through.

Should work as expected now.. you were completely right in your testing.
Comment 19 Caroline Cyr La Rose 2018-11-02 18:37:16 UTC
The hints in ordering and receiving form now match.
Comment 20 Caroline Cyr La Rose 2018-11-02 18:38:53 UTC
Created attachment 81917 [details] [review]
Bug 21387: Receive items from - form should include tax hints

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 21 Jonathan Druart 2018-11-06 17:43:46 UTC
Created attachment 81999 [details] [review]
Bug 21387: Receive items from - form should include tax hints

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Nick Clemens (kidclamp) 2018-11-07 13:10:42 UTC
Awesome work all!

Pushed to master for 18.11
Comment 23 Martin Renvoize (ashimema) 2018-11-16 12:43:37 UTC
Pushed to 18.05.x for 18.05.06
Comment 24 Fridolin Somers 2018-11-28 14:26:24 UTC
I prefer not to change display in 17.11.x