Bug 5335 - define VAT/taxes at orderline level, with default value for supplier and list of valid tax rates
Summary: define VAT/taxes at orderline level, with default value for supplier and list...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Deadline: 2011-05-01
Assignee: Jonathan Druart
QA Contact: Bugs List
URL: http://wiki.koha-community.org/wiki/B...
Keywords:
: 5173 5350 8343 8788 (view as bug list)
Depends on:
Blocks: 5343 6965 7163 7180 7495 7686 8832 9506
  Show dependency treegraph
 
Reported: 2010-11-02 02:12 UTC by Paul Poulain
Modified: 2014-05-26 21:04 UTC (History)
14 users (show)

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


Attachments
Bug 5335 More granular VAT (90.38 KB, patch)
2012-03-21 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
screenshot about bug 5335 (20.59 KB, image/png)
2012-03-21 13:20 UTC, Pierre Angot
Details
Bug 5335 More granular VAT (91.71 KB, patch)
2012-03-22 10:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5335 More granular VAT (92.34 KB, patch)
2012-04-04 14:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5335 More granular VAT (92.88 KB, patch)
2012-04-05 08:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5335 More granular VAT (92.88 KB, patch)
2012-04-13 09:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5335 More granular VAT (92.97 KB, patch)
2012-05-15 14:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Rebased patch (92.76 KB, patch)
2012-05-24 10:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Rebased patch (92.39 KB, patch)
2012-06-07 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Screenshot of duplicated form (112.85 KB, image/jpeg)
2012-06-09 11:47 UTC, Marc Véron
Details
Rebased patch (92.76 KB, patch)
2012-06-11 08:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
bug with columns (77.56 KB, image/png)
2012-06-11 09:05 UTC, Pierre Angot
Details
Rebased patch (93.51 KB, patch)
2012-06-21 08:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ5335: More granular VAT (93.50 KB, patch)
2012-07-12 06:20 UTC, Chris Cormack
Details | Diff | Splinter Review
BZ5335: More granular VAT (94.14 KB, patch)
2012-08-03 14:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ5335: More granular VAT (94.20 KB, patch)
2012-08-04 07:51 UTC, Paul Poulain
Details | Diff | Splinter Review
screenshots about some tests (398.60 KB, application/zip)
2012-08-04 07:53 UTC, Pierre Angot
Details
Bug 5335 - More granular VAT (94.23 KB, patch)
2012-08-17 14:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5335 - More granular VAT (94.33 KB, patch)
2012-08-17 14:54 UTC, Larry Baerveldt
Details | Diff | Splinter Review
Bug 5335 - More granular VAT (94.32 KB, patch)
2012-08-17 14:55 UTC, Joy Nelson
Details | Diff | Splinter Review
Bug 5335 - More granular VAT (88.86 KB, patch)
2012-09-19 12:36 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 Paul Poulain 2010-11-02 02:12:19 UTC

    
Comment 1 Paul Poulain 2010-11-02 02:31:22 UTC
More granular VAT (all)

Sponsored by St Etienne University and developed by BibLibre, expected for/deadline: 2011-05-01
bugzilla entry: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5335
[edit] Description

the available vats will be a syspref (5.5%, 19.6% in france for example).
each supplier will have a default vat value, but for each order line the librarian will be able to choose the vat to apply.
all prices & totals will be displayed VATs excluded and VATs included where applicable in basket & parcel management
on parcel management, it will be possible to modify the applied VAT rate
Comment 2 Paul Poulain 2010-11-02 03:21:05 UTC
*** Bug 5350 has been marked as a duplicate of this bug. ***
Comment 3 Paul Poulain 2011-11-05 10:53:48 UTC
If you want to participate to the upstreaming:
* assign the bug to yourself
* retrieve the patch(es) on git.biblibre.com
* the branch name can be found using
git branch -a|grep NNNN
Usually, it should be 
  biblibre/customer/ft/MTNNNN
For example:
   remotes/origin/stetienne/ft/MT6522

This patch is in branch number 5836
Comment 4 Francois Charbonnier 2012-03-21 10:02:29 UTC
Here (http://wiki.koha-community.org/wiki/Better_VAT_RFC), you will find more information about how this development works.
If you need more details, tell me!
(francharb on IRC)
Comment 5 Jonathan Druart 2012-03-21 10:08:13 UTC Comment hidden (obsolete)
Comment 6 Pierre Angot 2012-03-21 13:18:42 UTC
Test on sandbox.

There is a problem with ordering. I've created a basket and add an order. Two columns of the "Order details" aren't fill (cf 5335_order.png).
Comment 7 Pierre Angot 2012-03-21 13:20:10 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2012-03-21 13:38:39 UTC
I don't know why, updatedatatable script was not execute on your sandbox. I have execute it manually (it creates aqorders.discount and renames aqorders.gst with aqorders.gstrate). Can you please retest in test8 ?
Comment 9 Pierre Angot 2012-03-21 14:03:28 UTC
It's ok. Maybe someone can make an other test.
Comment 10 Pierre Angot 2012-03-21 14:56:15 UTC
First : we can't select TVA (no "Accounting Details") when add order from a staged file.

Second : added an order from staged files, price are not correct.
Sandbox8, basket N°5 an 7. We used this file : 146498lecannetdesmaures.mrc (batch 224).
Comment 11 Jonathan Druart 2012-03-22 10:43:07 UTC Comment hidden (obsolete)
Comment 12 Pierre Angot 2012-03-22 14:55:03 UTC
It works ! \o/
Test on a sandbox (Unimarc).
Comment 13 Paul Poulain 2012-03-29 09:38:16 UTC
this patch does not apply anymore, and solving conflict is not trivial, sorry :\
Comment 14 Jonathan Druart 2012-04-04 14:40:04 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2012-04-04 19:09:49 UTC
Hi Jonathan,
I am sorry, this is conflicting again and I am not sure how to solve it. Can you perhaps rebase again?
Comment 16 Jonathan Druart 2012-04-05 08:06:44 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2012-04-13 09:37:26 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2012-04-13 09:39:26 UTC
This patch is already signed off by Pierre but another signer is welcome :)
Comment 19 Jonathan Druart 2012-05-15 14:41:10 UTC Comment hidden (obsolete)
Comment 20 Paul Poulain 2012-05-24 09:35:17 UTC
conflict again =
CONFLICT (content): Merge conflict in C4/Acquisition.pm
Auto-merging acqui/finishreceive.pl
CONFLICT (content): Merge conflict in acqui/finishreceive.pl
Auto-merging acqui/orderreceive.pl
CONFLICT (content): Merge conflict in acqui/orderreceive.pl
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl

This patch is quite large, I fear it will conflict a lot. If it has been signed-off by Pierre already, as it's a dev that is in production for one BibLibre customer, I think it's OK to mark it "signed-off" once you've rebased it.

[Katrin, if you can test it quickly, you'll be more than welcomed ;-) ]

(and I'll test once again before pushing)
Comment 21 Jonathan Druart 2012-05-24 10:07:15 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2012-06-07 11:07:22 UTC Comment hidden (obsolete)
Comment 23 Marc Véron 2012-06-09 11:46:31 UTC
Katrin and I found a bug while testing the patch during Kohacon 12 Hackfest.

If you add new items to a basket, the form "Item" is rpeated several times.

We double checked with and without patch applied and with and without ACQ framework defined.

Additionally, in the Items list table we had a line missing between the first and the second item.

See screenshot attached.

----------------

We found a misspelling in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref

If you want more than 1 value, please separe with |)
should read
If you want more than 1 value, please separate with |)

----------------

We found several Bugs on Bugzilla that probably could be related to or resolved by this one: 
Bug 7495 - Tax calculations problem for new orders.
Bug 7686 - Tax rate wrong calculation in reception summary page 
Bug 6965 - Tax and dicount rate should be formatted the same
Bug 3980 - Vendor GST being ignored
Comment 24 Marc Véron 2012-06-09 11:47:26 UTC
Created attachment 10017 [details]
Screenshot of duplicated form
Comment 25 Matthias Meusburger 2012-06-09 15:04:38 UTC
Apart from the bugs reported by Marc, I found a blocking bug too. Showing a basket (cgi-bin/koha/acqui/basket.pl?basketno=xxx) produces a software error:

Software error:

Undefined subroutine &main::get_infos called at /home/koha/versions/kohacommunity/acqui/basket.pl line 272.

Of course, it does not happen without the patch.
Comment 26 Matthias Meusburger 2012-06-09 15:14:51 UTC
About the items repetion, javascript fails on the page because a call function called calcNeworderTotal is triggered but never defined. That causes entered items fields never to be hidden. Hence the repetition.
Comment 27 Jonathan Druart 2012-06-11 08:22:40 UTC Comment hidden (obsolete)
Comment 28 Pierre Angot 2012-06-11 09:04:27 UTC
I've noticed a problem with the columns when I've saved a basket. They are shifted (cf bug_table.png).

Test on sandbox 1.
Comment 29 Pierre Angot 2012-06-11 09:05:05 UTC
Created attachment 10159 [details]
bug with columns
Comment 30 Jonathan Druart 2012-06-11 10:24:33 UTC
Hi Pierre, 
I can't reproduce your problem, could you explain the procedure to reproduce please ?
Comment 31 Pierre Angot 2012-06-11 10:48:03 UTC
Nothing spécial ^^

- manage suggest
- create basket
- add an order line from suggest

I have tested with Chromium. No problem with Firefox.
Maybe a bug with Chromium ?
Comment 32 Jonathan Druart 2012-06-11 12:44:00 UTC
Pierre, I have tested with FF >= 3.6 and chromium v.20 and the display is correct.

Could you update your browser and retest please ?

It seems there is a bug when we hide the th elements.
Comment 33 Pierre Angot 2012-06-11 17:58:57 UTC
Yep, that's what I wanted to do. But, I've got a big problem with my Ubuntu (10.04 -> 12.04 = crash). Many paquets are corrupted, and I can't upgrade anything ^^

I'll test thursday on (?) an other install.
Comment 34 Jonathan Druart 2012-06-21 08:47:54 UTC Comment hidden (obsolete)
Comment 35 Chris Cormack 2012-07-12 06:20:53 UTC Comment hidden (obsolete)
Comment 36 Chris Cormack 2012-07-12 06:22:32 UTC
Rebased to fix updatedatabase conflict
Comment 37 Jonathan Druart 2012-08-03 14:34:42 UTC Comment hidden (obsolete)
Comment 38 Paul Poulain 2012-08-04 07:51:10 UTC
Patch tested with a sandbox, by Pierre Angot <tredok.pierre@gmail.com>
Comment 39 Paul Poulain 2012-08-04 07:51:41 UTC Comment hidden (obsolete)
Comment 40 Pierre Angot 2012-08-04 07:53:57 UTC
Created attachment 11352 [details]
screenshots about some tests
Comment 41 Pierre Angot 2012-08-04 07:58:46 UTC
I've made some new tests.

syspref gist : 0.1|0.196|0.05|0.07

I've created 2 vendors : one with discount and one without.
- basket 1 : no discount, default tax rate 10%, add order with different VAT (5%)
- basket 2 : 2% discount.

Need to test list prices and invoices prices. New vendor.
- basket 4 : tax rate = 10%, no discount, list prices and invoice prices include tax.
- basket 5 : tax rate = 10%, no discount, invoice prices include tax.
- basket 6 : tax rate = 10%, no discount, list prices include tax.
Theses baskets have also an ACQ framework.

You can check calculations (correct words ?) with the screenshots ("screenshots about some tests" : 1 zip, 4 files).

I haven't noticed any problems.

Test on sandbox.
Comment 42 Kyle M Hall 2012-08-17 14:04:40 UTC Comment hidden (obsolete)
Comment 43 Larry Baerveldt 2012-08-17 14:54:35 UTC Comment hidden (obsolete)
Comment 44 Joy Nelson 2012-08-17 14:55:14 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2012-09-19 12:36:09 UTC
Created attachment 12340 [details] [review]
Bug 5335 - More granular VAT

Rebased patch (conflicts no easy to solved, please test while making QA)
Comment 46 Paul Poulain 2012-09-20 16:29:49 UTC
QA comment:
 * passes koha-qa.pl
 * a big patch, but not so much code. A lot is made of cleaning / change in indentation/... like
-        $line{booksellerid}   = $booksellerid;
+        $line{booksellerid} = $booksellerid;

 * I'm not sure the following change
-            $biblionumber, $ordernumber, $quantityrec, $user, $unitprice,
-            $invoiceid, $replacement, undef, $datereceived, \@received_items);
+            $biblionumber,
+            $ordernumber,
+            $quantityrec,
+            $user,
+            $order->{unitprice},
+            $order->{ecost},
+            $invoiceno,
+            $order->{rrp},
+            undef,
+            $datereceived,
+            \@received_items,
+        );
 is very desirable, but I won't fail QA for that

* I'm not sure that t/db_dependent/lib/KohaTest/* is used, so the (tiny) changes here are probably useless.

Conclusion = Passed QA !
Comment 47 Paul Poulain 2012-09-21 09:48:31 UTC
Patch pushed to master

I made a very intensive testing of this patch, and could not find any problem either.

3 other signoffs... patch pushed !
Comment 48 Mark Tompsett 2012-09-21 15:33:48 UTC
*** Bug 8788 has been marked as a duplicate of this bug. ***
Comment 49 Katrin Fischer 2012-09-27 11:56:47 UTC
Hi Paul,

can you please take a look at 8832? We found a bigger problem that was introduced by this feature. 

Thx!
Comment 50 Nicole C. Engard 2012-09-27 12:06:22 UTC
Due to bug 8832 we think this patch needs to be reverted and thought out a bit more for the worldwide community.
Comment 51 Kyle M Hall 2012-09-27 12:11:04 UTC
(In reply to comment #50)
> Due to bug 8832 we think this patch needs to be reverted and thought out a
> bit more for the worldwide community.

Agreed. Nobody realized what ramifications this patch would cause. We need to revert it until an upgrade patch can be found and added.
Comment 52 Colin Campbell 2012-10-01 14:41:01 UTC
Patch breaks subscription-detail.pl ( it calls non-existant subroutines GetLastOrderNotReceivedFromSubscriptionId
and
GetLastOrderReceivedFromSubscriptionId

it also does not put brackets around the parameters for these generating another error
Comment 53 Paul Poulain 2012-10-01 16:46:02 UTC
(In reply to comment #52)
> Patch breaks subscription-detail.pl ( it calls non-existant subroutines
> GetLastOrderNotReceivedFromSubscriptionId
> and
> GetLastOrderReceivedFromSubscriptionId

Yikes !

This sub is here since the 1st patch submitted, it seems that no-one spotted that subscription-detail.pl was changed, and this patch has 4 signoffs+ my tests !!!

* I've digged, and can't find any reference to this sub anywhere outside from subscription-detail.pl
* it's used to display a link from subscription to acquisitions when applicable
* Jonathan is not here atm (holiday for 2 more weeks)
* I don't want to revert this patch, because reverting an updatedatabase-d patch is a nightmare

To fix the problem described, I've commented the lines calling those sub. The behaviour on subscription is back to normal. I'll push this problem on top of Jonathan's list when he comes back.
Comment 54 Jonathan Druart 2012-10-15 12:18:31 UTC
(In reply to comment #53)
> (In reply to comment #52)
> > Patch breaks subscription-detail.pl ( it calls non-existant subroutines
> > GetLastOrderNotReceivedFromSubscriptionId
> > and
> > GetLastOrderReceivedFromSubscriptionId

> To fix the problem described, I've commented the lines calling those sub.
> The behaviour on subscription is back to normal. I'll push this problem on
> top of Jonathan's list when he comes back.

I think it is the best way to fix it. In fact these 2 routines will be add by the Bug 5343 and should not be present with this patch.
Comment 55 Katrin Fischer 2013-04-14 07:34:11 UTC
*** Bug 5173 has been marked as a duplicate of this bug. ***
Comment 56 Katrin Fischer 2013-04-14 07:38:15 UTC
*** Bug 8343 has been marked as a duplicate of this bug. ***