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
*** Bug 5350 has been marked as a duplicate of this bug. ***
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
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)
Created attachment 8390 [details] [review] Bug 5335 More granular VAT See the wiki page for more information
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).
Created attachment 8406 [details] screenshot about bug 5335
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 ?
It's ok. Maybe someone can make an other test.
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).
Created attachment 8488 [details] [review] Bug 5335 More granular VAT Rebased patch and adapts acqui/addorderiso2709.pl script
It works ! \o/ Test on a sandbox (Unimarc).
this patch does not apply anymore, and solving conflict is not trivial, sorry :\
Created attachment 8873 [details] [review] Bug 5335 More granular VAT Rebased patch
Hi Jonathan, I am sorry, this is conflicting again and I am not sure how to solve it. Can you perhaps rebase again?
Created attachment 8902 [details] [review] Bug 5335 More granular VAT Rebased patch
Created attachment 9171 [details] [review] Bug 5335 More granular VAT Rebased patch
This patch is already signed off by Pierre but another signer is welcome :)
Created attachment 9586 [details] [review] Bug 5335 More granular VAT Rebased patch
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)
Created attachment 9754 [details] [review] Rebased patch BZ5335: More granular VAT signed-off-by: Pierre Angot
Created attachment 9970 [details] [review] Rebased patch Rebased patch
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
Created attachment 10017 [details] Screenshot of duplicated form
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.
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.
Created attachment 10148 [details] [review] Rebased patch Rebased patch
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.
Created attachment 10159 [details] bug with columns
Hi Pierre, I can't reproduce your problem, could you explain the procedure to reproduce please ?
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 ?
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.
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.
Created attachment 10400 [details] [review] Rebased patch BZ5335: More granular VAT signed-off-by: Pierre Angot
Created attachment 10781 [details] [review] BZ5335: More granular VAT signed-off-by: Pierre Angot
Rebased to fix updatedatabase conflict
Created attachment 11332 [details] [review] BZ5335: More granular VAT signed-off-by: Pierre Angot
Patch tested with a sandbox, by Pierre Angot <tredok.pierre@gmail.com>
Created attachment 11351 [details] [review] BZ5335: More granular VAT signed-off-by: Pierre Angot Signed-off-by: Pierre Angot <tredok.pierre@gmail.com>
Created attachment 11352 [details] screenshots about some tests
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.
Created attachment 11662 [details] [review] Bug 5335 - More granular VAT Signed-off-by: Pierre Angot <tredok.pierre@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11668 [details] [review] Bug 5335 - More granular VAT Signed-off-by: Pierre Angot <tredok.pierre@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Larry Baerveldt <larry@bywatersolutions.com>
Created attachment 11669 [details] [review] Bug 5335 - More granular VAT Signed-off-by: Pierre Angot <tredok.pierre@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Joy Nelson <joy@bywatersolutions.com>
Created attachment 12340 [details] [review] Bug 5335 - More granular VAT Rebased patch (conflicts no easy to solved, please test while making QA)
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 !
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 !
*** Bug 8788 has been marked as a duplicate of this bug. ***
Hi Paul, can you please take a look at 8832? We found a bigger problem that was introduced by this feature. Thx!
Due to bug 8832 we think this patch needs to be reverted and thought out a bit more for the worldwide community.
(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.
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
(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.
(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.
*** Bug 5173 has been marked as a duplicate of this bug. ***
*** Bug 8343 has been marked as a duplicate of this bug. ***