Well, I've been struggling for almost 2 days with this issue now... I have taken as many ideas into account as I can, including the opinion of TR on the uc_currency_format page.

I have stayed up all night and cranked out some code that I think can knock this problem outta the park... I don't know if it's not complete enough, or maybe even if the code really sucks and I'm going about it totally wrong ^^

I hacked the theme registry, so we can use uc_product_price to be our theme function, instead of converting uc_currency_format. I felt like this was the best way to add on without hacking core too bad. From what I gather we don't want to overhaul so much right now, so the vast majority of the change is in uc_price_alter module.

The only changes in core are the new classes passed down in order to be able to include/exclude certain prices from any altering.

uc_price_alter lets you set a CA condition on the 'class' of the price, and actions are flat rate altering, applying any tax, and adding a prefix/suffix to the output. The internals of this aren't 100% clean, since CA has no way to return value ... hoever, I think my solution is pretty elegant. :)

At least I think we have something solid to work off of in our effort to knock this out. I have attached a patch that adds te uc_price_alter module and patches some currency_format calls into theme('uc_product_price' calls. I realize at this point that 'u_price' would be a better name for the theme, but I did throught all this keep an eye on compatibility with what's current.

There's a built-in predicate that comes with uc_price_alter... just enable it and watch what happens to prices around the site! By default, it's a blacklist, and excluding both kinds of prices in the cart block, so if you look on checkout or the catalog, you'll see they're modified, but on the cart block, they're not.

P.S. You need to apply this patch to make it work: http://www.ubercart.org/forum/support/9589/file_download_notification_no... since it uses the same CA predicate potentially multiple times.

Thanks, and please leave suggestions...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Status: Needs review » Needs work

Alrighty, chatted w/ cha0s about this and am generally excited about the solution but hesitant about the implementation. It seems like the hack through the theme layer was developed in an attempt to be as unintrusive as possible, but at this point I'm not sure if I'd want to introduce an official hack, make more sweeping changes to core modules, or just leave things be and continue to let these solutions live in contrib space. I'm going to try and lay out a vision for a similarly unintrusive plan that goes just a little farther than the current patch.

In general, it seems like what we need not just for VAT support but possibly for other systems as well is a system that accommodates the following workflow:

  1. We have a price that we want to use in a particular context, like a product price displayed on a node or a cart item price displayed in the cart block.
  2. At the point where the price is built for output, it is passed to uc_price()* with any necessary context information, including but not limited to some sort of key identifying the place of use and object data like a product node object or cart item object.
  3. uc_price() allows altering of the price itself, the altering of the arguments passed to uc_currency_format() (in step 4), and the addition/modification of any "options" that will be passed to the theme function if the price is displayed (see step 5).
  4. The price and the other arguments are passed through uc_currency_format().
  5. If being built for display (dependent on key for now? possibly registered by a hook?), the resultant formatted price and options array are passed to the theme function; at the very least we should include prefix and suffix in our default implementation.

*Note that uc_price() doesn't have to be the end name of the function; I just picked it because it's use case agnostic. In other words, the function doesn't presume a calculation will occur or the price is being altered for display. It might be modified and used in a calculation but never displayed, and it might be displayed as is!

As for the mechanism of alteration in step 3, cha0s' patch includes support for this in Conditional Actions, but it seems there are some limitations of that system that might cause us to roll out a less than ideal implementation. In light of that, I might favor an approach that simply uses a hook for now, and we can leave this price alteration system as a core feature that depends on a contributed UI (think fields in core on D7) to really make a reality. A contributed module could just as easily implement the CA code, and in fact cha0s might even want to pursue that himself. : ?

As for touching all the juicy bits of core this will require, I think we can make it work. I've already made it clear that I want to have core support at least at a low API level for VAT in the 2.0 release. I think this patch would get us there, but it would introduce code that we actually mean to rewrite later (and I feel we have enough that we didn't mean to rewrite but really need to as it is! x_X). So, I think perhaps even the same code in this patch can be repurposed to use uc_price() instead of the theme layer to accomplish the price modifications we're looking for.

I think we can shoot for making as few core changes as necessary... maybe this means for now we only run the prices through uc_price() that we know are essential for VAT support and then spec. out a full adoption for 3.x. One example Mike brought up is the fact that such a system could be hooked into by the attribute module to handle the price adjustments for cart items. Other places this might make sense is with an even more tightly integrated discount framework based on Lyle's work in uc_discount. But I digress...

Mike also recommended caching and a sort of revision history for the price, uc_currency_format() arguments, and options array. Caching for sure, though the revision history might need to be a 3.0 feature. I suppose if a base price and same context data are passed in, we should always return a cached version for now. cha0s recommended a hash on the context data, and that works fine for me.

Obviously this is going to need some testing, but I'd go so far as to say I'd be fine if every time the patch was rewritten we posted up a zipped version of the patched source somewhere on UC.org so we get as many eyes on the changes as possible.

mfer’s picture

my 2 cents... take it for what it's work (1 cent... really)

If you can call the solution a hack but think you have a solution that's clean... pursue the clean solution even if it takes a tad bit longer.

Seeing strip_tags being run on the output of a theme function makes me cringe a little bit. Why not use a hander of sorts or a hook?

cha0s’s picture

Thanks for posting, guys. =)

As Ryan guessed, the main point of the patch I made was to keep as much core unchanged as possible, and keep it compatible as much as possible. I do agree that the actual execution of the price altering here is a bit hackish, that is we shouldn't have that logic in the theme layer, it should be above that.

I'm glad to see that even though this might take some touching of the nether regions (:P) that we're willing to do what it takes to get it done. I started #drupal-ubercart-dev over on freenode, if you guys wanna get together so we can talk about any details you might want to discuss. =) I think following the flow Ryan outlined in his post will be good for all prices, especially as I've seen TR doing multicurrency, we would love your input, dude.

It probably will be a better idea to do this outside of CA, especially if the locking thing isn't going to change. Should be cake to work that out.

Price caching is an interesting subject. I'd like to include that (and the history sounds interesting as well), not quite sure how that would work though I have an idea... (uc_price() just returns an array of strings instead of a string). However, even if it doesn't make it in, I think we'll still be doing good if everything else works.

Thanks everyone for the input, you guys know what you need best! ^^

P.S. @mfer, yeah... I know rendering is no place for this stuff, but I was trying to step lightly =P

Al01’s picture

It is a pleasure and an honour to work with you, guys, I'm really happy being part of this process. I see the elegance of the code and the clear will to go as far as necessary, so first a review of my issues with this patch:

  1. uc_price_alter:
    • A fixed rate for everything is not enough. Everything can be easily covered by CA, so here just 2 examples: In the EU we have 2 tax rates, the lower one for books, food and some others, the higher one for all other products/services and usually also shipping (which sometimes can be also 0%)
      Sell books with 7% and whatever with 19% won't work here.
    • If the user is from a different country than the store & verified Business customer (=> role), he doesn't pay any VAT.
    • => I cannot avoid that I have to check the settings in the powerful CA module.
    • The good news about is that I have working code, I could even maintain it if cha0s doesn't want to do it. I just need a bit more information (see #5) and a place where to inject it, then uc_vat can roll out.
  2. Rounding:
    • Example tax rate: 19%
    • I want to sell a nail for $0.09 (incl. VAT), so the DB holds 0.076
    • add 1000 nails into the shopping cart and you get $90.44. I'd like to see $90
      If the sell price should be $0.90, you get 899.64, but it should be $900.
      => this will affect anybody dealing with higher quantities.
    • The formula I need to inject is:
      $price = round($item-price * $tax_rate, variable_get('uc_currency_prec', '2')) * $qty;
  3. Just for completeness: Line items are both affected for multi currency and VAT. At least here we have no "Qty issue".
Al01’s picture

Notes to Ryan's post at #1:

  1. In the order pages is also affected at least what the customer sees.
  2. This would be sweet :) My favorite candidates for context information are:
    Products:
    • $nid (e.g. to get the class which might have a different rate)
    • $order_id or $_SESSION['cart_order']
    • $qty (can default to 1) would keep my code shorter, but with the above I will find a way around
    • $item_price - same situation like with $qty

    Line Items:

    • $type
    • $order_id or $_SESSION['cart_order']

    The user (and his role) I can get through global $user;

  3. sweet...
  4. sweet...
  5. If I have the necessary informations in uc_price() available, a key for the theme function is enough.
    Prefix/suffix stuff: While in usual EU environments I could survive without, it would be great to have this option for the actual Slovak issue, which will come back for a number of other countries within the next couple of years.

The next points I add for completeness, they are own issues:

  • Regarding orders I'd like to see in the DB both for the tables uc_order_products and uc_order_line_items an additional column called "tax_amount". I can provide a patch, but this might depend on what we do here.
  • /checkout : Calculate shipping Costs - the displayed values usually contain taxes, this might confuse uc_taxes. Not sure if what I have is good enough.
  • /checkout : Order Preview - we'd need a patch there or an alternative pane
  • Tax calculation: I need different formulas (they are not unique in the EU) to calculate the tax amount. Maybe also a hook?
cha0s’s picture

FileSize
10.4 KB

Got another draft here... this one is more of a complete refactoring of the price calculation.

As of now, I'm thinking a good way to implement this will be with a PriceHandler class that can extend Ubercart's and implement the functionality it needs. If you're allergic to OO, you probably won't like this. :P

Stuff that's here:
uc_store/include/uc_price.inc
theme_uc_price()
class UbercartPriceFormatter

Also, I think I solved the 'do we theme?' and caching stuff. Each step is cached, you can see this in UbercartPriceFormatter::calculate(). There's also a $revision parameter that specifies which of these cached data to return. If none is given, the themed one is returned. Caching is done using the hashed context, options, and price combined with the price handler's name and the $user->uid as a key into the cache table.

I'm sure there's stuff I forgot, and you might not agree with my specific implementation (extending a class to add your own price handling?), but the more important part to me is to see if there are use cases this won't cover. I just want this to be as extensible and powerful as possible... well ok, and elegant. :P

Here, I wrote a little sample code to show how you'd make some calls... setting the price handler as of right now is hackish, though. :)


class UbercartPriceFormatterTwo extends UbercartPriceFormatter {
  function __construct() {
    parent::__construct();
    $this->options = array(
      'sign' => '#',
    ) + $this->options;
  }
  
  function alter(&$price, &$context, &$options) {
    global $user;
    // Call the parent.
    parent::alter($price, $context, $options);
    if (!$user->uid) {
      // Add our own 2 cents.
      $options['prefixes'][] = t('Only ');
      $options['suffixes'][] = t(' excl. VAT');
      // Alter the price too.
      $price['value'] *= 1.25;
    }
    // Anonymous users get a fun message.
    else {
      $price['placeholder'] = t('Free for n00bies!');
    }
  }
}

$price = array(
  'value' => 69,
  'qty' => 10,
);

$context = array(
  'location' => 'catalog',
  'class' => array(
    'product',
    'sell-price',
  ),
  'subject' => array(
    'nid' => 1,
  ),
);

dvm(uc_price($price, $context));

// Hax a new class.
global $uc_price_handler;
$uc_price_handler = new UbercartPriceFormatterTwo;

dvm(check_plain(uc_price($price, $context, array())));
dvm(check_plain(uc_price($price, $context, array('label' => FALSE), 'altered')));
dvm(check_plain(uc_price($price, $context, array('label' => FALSE, 'sign' => ''))));

Enjoi... ^^

mikejoconnor’s picture

Here are a few thoughts

1. I like the code cleanup stuff, but it might be best to keep that as a separate patch.

2. At the most fundamental level, we are trying to give a module a chance to modify the price based on it's context. I don't think this passes that test, as it would require extending the UbercartPriceFormatter class, and overriding it's function. Since Ubercart will be invoking UbercartPriceFormatter, and not UbercartPriceFormatter2, a contrib module woudn't be able to change the price, without theme hackery.

rszrama’s picture

cha0s and I chatted about this earlier, and point 2 was brought up. My thought was I'm not necessarily against using a ucPrice class, though I don't like calling it a formatter when it's really doing much more. Also, I don't like the limitation of just having a single handler, because you'd have to write a custom handler any time you wanted to combine two or more alterations (VAT display and multi-currency, for example). My recommendation was to stick with hooks but have multiple points of alteration. Modules could then register callbacks to these various points (instead of having 4 different *_alter hooks), and we can have a thin UI that allows you to sort these callbacks in a tablesort. The context array looks like a good start, though I'm not sure I like the idea of putting qty in with the price array. A price should just be a float, imo, or at the very least the basic information pertinent to that price... perhaps price and currency. Quantity is more of a shopping cart contents feature than a price alteration feature, though I suppose for the cart display context you could always pass in the qty.

Anyways, cha0s was gonna have a go at revising the patch above, so we'll see what turns up next. : )

cha0s’s picture

Well, here we go, round 2 (or 4? :D)

This time around I've added:
A new hook, hook_uc_price_handler(), which is documented in hooks.php. I'll paste that here for easy readin'.

/**
 * Use this hook to define price handlers for your module. You may define one
 * price alterer and one price formatter. You may also define options that are
 * merged into the options array in order of each price alterer's weight.
 */
function hook_uc_price_handler() {
  return array(
    'alter' => array(
      'callback' => 'my_price_handler_alter',
    ),
    'format' => array(
      'callback' => 'my_price_handler_format',
    ),
    'options' => array(
      'sign' => variable_get('uc_currency_sign', '*'),
      'sign_after' => TRUE,
      'prec' => 4,
      'dec' => ',',
      'thou' => '.',
      'label' => FALSE,
      'my_option_that_my_formatter_recognizes' => 1337,
    )
  );
}

A new UI at admin/store/settings/prices, where you can selectively enable/disable price alterers, and you can select which price formatter to use (I tried, but I couldn't think of a use case where more than one formatting of currency would ever be needed). You can also tabledrag the alterers to define what order they fire in. They fire in descending order.

A default hook implementation by uc_store.

A new cache table for the price cache.

A small fix in the new calls to uc_price() regarding hiding the prefix if it's a teaser or not.

That's on top of everything from last patch. :P I got rid of the class, it would have worked very well if we ever only needed one implementation of a price handler, but tax systems + multicurrency being a very real scenario for international Ubercart stores ruled that model out. Oh well, we're not gonna try to drive a nail with a screwdriver. :)

TODO:
It'd be cool to have a preview on the price handler admin UI that showed what a sample price (like $1234.567) would look like after passing through each alterer, and the formatter. Extra slick would be using AJAX to do it as the selections change. Not holding my breath on that one!

Actually replacing all the uc_currency_format() and theme('uc_product_price, etc calls with uc_price(). I haven't done that yet because I want to get a solid core that everyone likes first. (I figure that's higher priority)

Determining if we do in fact need to pass in the item price as well as the quantity. Ryan, I only did it this way because Al said that for certain tax calculations, it would be favorable to have that information. My strong point is definitely not the actual VAT/GST/whatever rules, so I'll defer to him on that one, but figured I'd let you know it wasn't a random choice. :)

cha0s’s picture

Status: Needs work » Needs review
Al01’s picture

The Qty issue by examples:

Tax rate: 19%
Qty.: 1000
Product 1: $0.09 (incl. VAT)
Product 2: $0.90 (incl. VAT)

Database:
Product 1: $0.076 (excl. VAT)
Product 2: $0.756 (excl. VAT)

Without rounding I get for Qty = 1000:
Product 1: $90.44 (incl. VAT) // ! should be $90
Product 2: $899.64 (incl. VAT) // ! should be $900

=> For products we calculate:
$price = round($item->price * (1+$tax_rate), variable_get('uc_currency_prec', '2')) * $qty;

Accordingly taxes are calculated on the item price, where the situation can be even harder as there could be a different VAT currency precision (e.g. Switzerland or Czech Republic sure) or fancy rules like "round up/down/mathematically/on 3 decimals", but there I have still not enough information. That's why I still didn't open a thread about.

Maybe somebody of the other Europeans would like to help out here?

imo the uc_taxes issue should run through a hook so that Europeans care for it.

VinceW’s picture

In reply on #11

=> For products we calculate:
$price = round($item->price * (1+$tax_rate), variable_get('uc_currency_prec', '2')) * $qty;

When doing so (putting $qty outside the brakes) you multiply also the rounding differences.

In Europe it's normal to calculate VAT on the whole package of sold items/products. So if you sell 1000 nails for 0.09 eurocent a piece with a vat of 19% the formula should be:

$price = ($qty * $item->price) + ( ($qty * $item->price) * $tax_rate )

(1000 * 0.09) + ( (1000 * 0.09) * 0.19) = 90 + 17.10 = 107.10

When doing so it doesn't matter if the rounding is done on the VAT-amount or on the total amount. The formula then looks like:

$price = ($qty * $item->price) + round( ( ($qty * $item->price) * $tax_rate ), variable_get('uc_currency_prec', '2') )

or

$price = round( ($qty * $item->price) + ( ($qty * $item->price) * $tax_rate ), variable_get('uc_currency_prec', '2') )

Best
VinceW

PS: didn't know where to post best, here or in: http://www.ubercart.org/forum/development/6995/vatgst_overview As you can see i decided for this thread. Let me know if there's a better spot for it.

rszrama’s picture

Status: Needs review » Needs work

Hey Vince, this was the perfect place to reply, so we can keep all development discussion in one place. Are you saying then that passing in the quantity is not necessary?

@cha0s, I'm certainly not opposed to passing in a qty. I think I'd prefer to move it to its own parameter, though, since $price really implies you're just passing in the price. Perhaps we can then pass $qty in as the second parameter w/ a default of 1? I think this will simplify the various uc_price() calls, like in your code demoing the new functionality in the product module. If I'm not mistaken, this patch moved the price above the image whereas it used to sit below the product image. Any way to move it back down? (See the 2.x Livetest.)

Also, I notice you added your own weight sorting function. There's actually a func you should be able to use called uc_weight_sort() that is defined in uc_store.module.

Sweet ideas on the admin UI. You rule those tablesorts. : )

cha0s’s picture

Very strange about the price being above the image. Same thing happened here, but I noticed that field weight was set to -10. Setting it back to the normal -1 fixed it...

Anywho this patch allows you to pass the price as the first arg. I renamed the param $price_info. I wasn't just trying to be contradictory, it seems like a strange design to me to put price, qty, context, when context will pretty much always be specified. That means qty will always have to be as well, so there's no point making it optional. Putting qty after context seems even weirder... I'd like to hear your thoughts about it, anyway.

VinceW’s picture

@Ryan #13

No, not quite.... You'll always need a quantity to calculate the total amount. I was only refering to the position of the quantity in the formula. By doing it the way I indicated, you only get a rounding on the final (incl. vat) amount. So when using tax-rates with 3 decimals, you only get on the second decimal of the total amount a rounding. This implies a max difference of 0.5 cents, a difference tax agencies can live with :-)

Willing to explain more, when I haven't elaborated enough...

Best
VinceW

rszrama’s picture

Issue tags: +dc2009 code sprint
rszrama’s picture

@VinceW, no further explanation needed. I'm blissfully ignorant here and think I just misunderstood your post. ; )

@cha0s, I see, I thought $context was optional b/c it had a default value in the func definition. I think your current solution is ok, in that other core funcs like node_load() can take a scalar or array argument, but I'm also not opposed to either adding in $qty or possibly making it part of the $context array. I'd have to see more examples of that I suppose, but if you have the slightest bit of hesitation, let's just leave it how it is at this point. This last patch leaves a dpm() in the product module... and weird on the weight thing. Not sure what to think about that. :-/

Agileware’s picture

Subscribing

cialog’s picture

Subscribing

Al01’s picture

@VinceW #12 Most UC installs will be about business to end user and I know quite a lot people who'd like to avoid calculations like
(1.02 * 1.19) + (1.02 * 1.19)
which renders like:

  $1.21
+ $1.21
______
  $2.43

But back to the patch:
My CA tests are working again on cha0s' latest patches #14 as far as it goes. So I went on and patched by the example of uc_product.module uc_flatrate.module. This works basically also, but I have troubles with the caching as the country condition and with it the tax rules can change in checkout. I'm sure I'm missing something...

I would need some advice where and how to force $options = array('rebuild_handlers' => TRUE,);

TIA

robertDouglass’s picture

As per mikejoconnor's request:

The following should always be true:
$total_price = $qty * $display_price
where
$display_price = ($base_price * (1 + $tax_rate))

cha0s’s picture

I'm not sure if we should cache the prices if it's going to cause problems. Perhaps there's a smarter way to cache the prices than what I'm doing now. However, if it's going to end up presenting a huge problem to implement VAT or multicurrency stuff, then it has to be changed.

Thoughts?

robertDouglass’s picture

Then you can calculate what the base price should be from the display price because the second equation only has one variable:

$tax_rate = 0.19; // for Germany
$display_price = 5.00; // Will be Euros

// $display_price = ($base_price * (1 + $tax_rate))
$base_price = $display_price / (1 + $tax_rate);
// $base_price = €4.2017

So I don't know either what big businesses do to find out how much tax they owe the government, but I do know that in retail, all base prices are set to make the display price look good. In other words, merchants decide what the display price should be and wrap the base price and tax to fit, probably erring on the side of safety when paying the gov. back their VAT.

But I'm no expert... where's the freakin' accountant?

rszrama’s picture

Hmm... I guess I'd say it all depends on what's happening during price calculation... are the DB queries? Are we going to be calculating a lot of prices at once? For example, displaying a catalog that lists the sell price and list price of 25 products might add 50 * number of queries for each price... that could be bad. But maybe we don't have caching at our level and write a specification for now for those modules implementing these hooks to enable some sort of caching on their end?

Al01’s picture

What I need for VAT is building a fake order containing
* user
* country (defaults to store country)
* one product / line item
With this data I ask CA which conditions apply. As long as I did this with WF_ng I didn't run into performance issues.

mikejoconnor’s picture

There seem to be two major ways of calculating taxes, although there are variations of these based on class of product, it seems to come down to these two

a. $total = ($base * $tax) * $qty
b. $total = ($base * $qty) * $tax

Overall, I believe our current tax system supports b, the purpose of this patch is to support a.

That being said, I don't think qty hurts, I Just don't think it's necessary, and I think we have other functions available to handle this situation.

mikejoconnor’s picture

Al01,

The idea of this system is that you can provide the same calculations without creating a fake order. In the past the fake order scenario was simply a method of triggering the workflow logic. With this patch, you will be able to write a price handler that will fire the conditional actions triggers, allowing you to perform the calculations you were previously performing by faking the order.

parrottvision’s picture

subscribing

VinceW’s picture

@robertDouglass #23

But I'm no expert... where's the freakin' accountant?

My wife is an accountant, and she ain't no freak :-)

@mikejoconnor #26

Overall, I believe our current tax system supports b, the purpose of this patch is to support a.

In relation to the topic of this thread (solving the VAT display stuf) I think it should maybe considered that different regions have different options, and therefore Ubercart should present those different options as a choice to the end user.

Best,
VinceW

prodosh’s picture

In many European countries (certainly Switzerland, Germany, Austria) the following VAT related rules are relevant for retail sales, purchases and VAT declarations:

- The Price of an item in a retail (B2C) sale usually refers to Price = ListPrice * (1 + VATrate) where VATrate is a percentage. So the price are generally shown per item including VAT.

- Often there is a requirement to show prices per unit e.g. for soap powder the price per 100 grams including VAT. This is just a store labelling requirement.

- While there usually is a general VAT rate that applies to most things (in Switzerland 7.6%, in Austria 20%), there are special rates (usually lower) for certain categories of goods and services. For example rent is VAT exempt. In some countries e.g. Restaurants get to charge a lower VAT rate.

- Sometimes there may be other local or excise taxes. These may be a fixed amount per person, per invoice, per room (tourism) or a percentage of the Total Amount etc. Basically you need to allow for a few additional arbitrary charges per line item and per invoice / bill / receipt to provide full flexibility to cover every eventuality.
Receipts:

A. On the receipt the net price of each line item is shown along with the % VATrate

B. The TotalPrice is shown as the sum of the Line Items on the receipt. This includes VAT. 

C. If there are local or special charges they are now individually listed

D. The Grand Total of B and C is calculated and shown and is usually what the customer has to pay.

E. Below the GrandTotal (D), the total amount of VAT charged is shown grouped by the different VATrates. For example "VAT @ 20%   EUR 1.40" followed by "VAT @ 10%  EUR 5.50"

F. In Switzerland it is common to show the GrandTotal (D) in EUR too below the Grand Total in Swiss Francs. This is common in many countries like those that are candidates to join the Eurozone or who have many tourists from other parts of Europe.  
Calculations:

- Price = ListPrice * (1 + VATrate)

- Therefore: ListPrice = Price / (1 + VATrate)

- The amounts in E (above) are calculated using the formula:   LineItemPrice = Price * Quantity where Price includes the VAT. 

- LineItemVAT = (Price - ListPrice) * Quantity

- or LineItemVAT = Price (1 - 1/(1+VATrate)) * Quantity    

Because Price, VATrate and Quantity are usually the known variables, the last line above is usually the one used for calculations.
Data and Record Keeping:

1. Each vendor must periodically make a VAT declaration to the VAT authority. This could be monthly, quarterly, bi-monthly or simply for an arbitrary range of dates (from, to)

2. Declaration of Sales and VAT collected: For this they will need (and are usually required to keep records of) the sales for the period usually showing the amount of sales per VAT Rate category and the corresponding total amount of VAT obtained by summing up the VAT in all categories. 

3. Declaration of total amounts of goods and services purchased (e.g. for inventory, expenses) and the VAT paid on these purchases. 

4. Any adjustments requested or carry overs from the previous declarations or unpaid amounts.

5. The difference between 2 and 3 +/- 4 is the amount the vendor owes the VAT Tax Authority or is owed from the VAT Tax Authorities. 

The general case would be to maintain a VAT account and debit and credit all the relevant VAT amounts to the VAT account. The amount of VAT owed should show up as a liability (or asset in case of a refund) in the accounting system and it should be possible to calculate / identigy this amount on a given date by checking the balance of the VAT account. 

The VAT system needs to maintain sales and purchase records, usually in the form of Sales and Purchases (or Expenses) account to be able to handle the periodic VAT declarations.  The associated transaction lists, cash flows and Balance Sheet entries need to be made available (exported) to the accounting system periodically. 

If the VAT declaration is handled by a separate acounting system, then UC needs to regularly export a list of all transactions (since the last export) to the accounting system. Each exported transaction needs to include the Total sales price (including VAT ) and the VATrate applied per line item, the item sold (item nr and short description) and quantity. A CSV file is great for this export because most accounting systems can import it.

There may be more rules in addition to the above, but this set should cover the vast majority of the cases for most European Countries.

HTH and thanks to everyone who is helping to make this happen.

Prodosh

prodosh’s picture

In the wholesale / B2B situation, most of the above would still work. However:

- The purchaser would like to see price without VAT displayed more prominently, because this is the amount that is relevant for their internal costing.
- The VAT paid while purchasing gets automatically adjusted against the VAT collected from clients in the VAT account of the accounting system as outlined above
- For cash flow purposes, the total amount of an invoice including the VAT is relevant, because this is what gets paid out. The VAT adjustment is reflected in the cashflow on the VAT account where VAT paid to third parties and VAT collected from clients come together and balance each other out.

robertDouglass’s picture

Thank you Prodosh. Awesome attention to detail and completeness, as always =)

robertDouglass’s picture

From Alex Langer:

Can't even squeeze out some hard facts with Google, so I guess curious me is gonna phone his accountant tomorrow. Stuff I read a moment ago was that usually you go with display prices including VAT for end customers => "gross / (1 + tax_rate) = net". So you make sure you don't have to turn to your calculator just set up new "fancy" prices like the famous 0.99ers.
For commercial customers you start with net prices as VAT gets carried away by the government anyway and calculate gross prices with "net * (1 + tax_rate) = gross".
Let me think about it for a moment...
... looking at some bills ...
In the end each bill has only one total amount to pay. That's the point where the included tax is calculated. For end customers you have gross total and calculate included tax from there, while for business customers you have a net total and add the tax for that value.
So in the end I'd say you have two prices attached to each product. You give one value and the other may be automatically calculated. What type of customer gets the bill then decides how totals are calculated and that it may make some little difference when comparing the two possibilities .. well.. it's the way things are I guess.
But okay, I'm gonna phone tomorrow and the questions still up and I'm backed up by my accountant I'll post it on D.org, too.
P.S.:
If in doubt, the gov's gonna tell the business how much it owes, not the other way around ;-)

http://www.drupalcenter.de/node/17713#comment-61798

cha0s’s picture

FileSize
18.23 KB

Alright, next patch... this one fixed that sloppy dpm() I left in, and also I added a mechanism to force a price to not be cached. You do that by passing in $options['cache'] to uc_price() as FALSE. It defaults to TRUE.

Not sure what happened there with #weight, seems like a burp; I created a new branch and applied my patch, looks fine.

Also, I think this is the most sensible and cleanest way to do $qty. i'm not sure I like mixing the item info into $context. We could think about adding the price/qty into context and removing the need for $price altogether, but it seemed like a good idea to me, to keep those lines drawn.

Anonymous’s picture

I´m not using Ubercart right now, but I cerainly will in near future.
Therefore be gentle if I don´t know the details.

Adding to Prodosh´s comment:

  • It is common in European union countries to pay no tax at all if the customer comes from another country.
    So we need to distinguish a home country and a customers country (depending on some sort of delivery address, not address of the invoice).
    But if somebody knows the US VAT system he´ll know what I´m talking about ;-)
  • The VAT delcaration itself will be and has to be created by the accounting system because:
    • you will always have some invoices not posted in Ubercart (Toilet paper, pens, gas, water supply,...)
    • you would have to recalculate taxes for discounts granted[1]
    • nevertheless you will need the ability to export data for the periods mentioned
  • Base for the tax declaration in most European countries is some tax point, a date where the customer becomes owner of the good. This is usually based on the terms of delivery (incoterms). This usually will be the day you send the goods (not the day the customer receives the goods).

Thats all I could think of right now at my extended break.
If some things need more explenation just ask.

Cheers

Stefan

[1]
Invoice (net)  $100
Tax             $19
Total   (gros) $119

Customer gets a discount of 2%, so he´s paying: $119 * 0.98 = $116.62
Tax (in Germany, Italy, Poland,...) would now be calculated:

net amount ($116.62 / 1.19)  = $98
Tax             $98 * 0.19   = $18.62
=> $18.62 would be the new taxbase

BUT: Your not allowed to do this in the UK.
In the UK your allowed to reduce tax immediately:

Invoice (net)   $100.32
Tax             $18.68
Total   (gros)  $119

Discount allowed: 2% will reduce the VAT at the time of issuing the invoice.
In other European countries this happens at the date of payment.

Other example:
Netherlands:
As far as I know if there is a discount granted the invoice is issued at 100% and the value increases after a certain period to 102% and with it the tax base.
erem’s picture

Subscribing

shiroitatsu’s picture

subscribing

Island Usurper’s picture

FileSize
1.07 KB

As a way to review this, I've implemented the price handlers for uc_discount: #430022: Keep following Ubercart code that allows product discounts to work. I also implemented uc_price() on the cart page, which you can see in the patch.

I made a couple of changes, which bring me to the main part of this post. As uc_price() is implemented throughout the codebase, I think we need to be deliberate and thorough about the subjects that are provided through $context. One of the changes I made was to use the entire node instead of just the nid as the subject. Even though node_load() caches its results, it seems odd that every handler that deals with a node has to load it for itself.

On the cart page, I'm using both the item from the cart, and the node that it came from as the subject. The discount price handler already needed a node to pass along to its Conditional Actions, so the cart item wasn't suitable for use there. Fortunately, all cart items have a node id already, so the node can be loaded then to put it into $context. I'm just trying to predict whether we should keep doing this in all of the other places uc_price() will be used, or require the price handlers to switch() off of $context['location'], or some other marker.

I don't yet have a recommendation, and the decision will probably be determined by how things pan out later. Just be thinking about it as we work it out.

Finally, please note that uc_store_price_handler_alter() needs to check that $context['class'] is an array before using it in in_array().

Island Usurper’s picture

Separating out the implementation of uc_price() from the code that supports it. These include the changes that I mentioned in my previous post. Still "needs work" on the implementation to cover the rest of Ubercart.

rszrama’s picture

FileSize
17.82 KB

Alrighty, I've updated the price patch with some bugfixes, usability improvements, and comments. Here's a mini-changelog, though a few other things might've been tweaked that I didn't add to the list:

  • Changed 'value' key in $price_info array to 'price'.
  • Changed the use of some temporary variables in uc_price().
  • Changed $cache array to $values.
  • Only allow $price_info['placeholder'] to override the formatted value.
  • Fixed formatter allocation; previously it stored only the module name, not the callback name, so it never passed the function_exists() check.
  • Added default value to formatter element on price handler settings form and changed it to a radio selection.
  • Added titles and descriptions to the alterers and formatters on the settings form.

(Edit: Per Lyle's feedback, I've already fixed the comment for the $price_info argument for uc_price().)

cha0s’s picture

Looking good guys! ^^

Island Usurper’s picture

Here's the implementations in uc_product, uc_cart, and uc_order, that should work with #40. I think the checkout page breaks the discount module, but I'm not too sure. It might just be the predicate locking doing it.

Island Usurper’s picture

In order to get the discounts to work, I think we really do need to use the patch in #369742: CA predicates maybe don't need to be prevented from being evaluated more than once.. uc_discount_price_handler_alter() now keeps track of the nodes and their prices that it has already discounted so it won't have to do the same trigger on the same arguments multiple times on a page.

Added a 'field' value to the context subject in uc_product_view() because the discount price alterer needs to only run when the sell_price is being displayed. Otherwise, the list price and the cost would look the same value as the sell price.

rszrama’s picture

FileSize
18.89 KB

Minor update to add an entire account object to the context array instead of just a uid. I also changed the cache key to not include the full account, since it changes every pageload. The only parts of the account in the context array that's being used to generate the key are the uid, name, mail, and roles.

I also added a setting to turn price caching on or off. I made it clear that cached prices only last until the next cron run but still warn about the possible performance hit for high traffic sites if caching is not turned on.

Island Usurper’s picture

Lots of new places have uc_price() implemented now.

I believe the only major section left is everything in the shipping folder. We're on the home stretch!

rszrama’s picture

Just committed uc_price_2.patch. The implementation work is remaining in a patch, as some things need to be changed per Lyle's developer experience. The only thing that may change in the core handler stuff itself is the price info array based on roadblocks Lyle's hitting as he implements it all.

Hooray, progress!

cha0s’s picture

Hm, reviewing the implementation, I suggest a couple of changes to the API.

I think we should pass the desired price revision in through $context. This will prevent crappy looking function calls (e.g. passing 'formatted' as it stands) I have a patch for that, along with some more option docs in the doxygen.

Also, since you mentioned the shipping/* hadn't been done, I did it... I implemented it assuming we change the API, so you can take a look at it... I think it's a lot cleaner. :)

Finally, I'm wondering if we should really be using 'themed' as the default. Perhaps we should use 'formatted'. All depends on what you guys think is most convenient from a DX perspective, but it seems like there's an awful lot of those ugly 'formatted' call variations atm.

cha0s’s picture

Yarrr, 2 patches, mateys.

rszrama’s picture

Ahh, options docs are awesome. Also, I think we'd be in agreement about making "formatted" the default. Let's see what Lyle says; I'm sure he'd concur. I think he wanted to suggest another change or two, too. Thanks for the review and the shipping patch! We're sooo close. ^_^

cha0s’s picture

I was looking around a little more and we've still got lots of uc_currency_format calls going on... Won't we need to allow a formatter to change those for different currencies?

Another thing, I've been looking at the revisions we have, and I think it may be smartest to do it as a bitmask... We can default it so that it won't be a lot of craziness everywhere, but for instance:

define('UC_PRICE_ORIGINAL', 0);
define('UC_PRICE_ALTERED', 1);
define('UC_PRICE_FORMATTED', 2);
define('UC_PRICE_THEMED', 4);

define('UC_PRICE_DEFAULT', UC_PRICE_ALTERED | UC_PRICE_FORMATTED);

Cause I was thinking like, what if you wanna format the price, but you don't wanna alter it? This seems to be a flexible enough way to handle that, plus any other possible combination.

Really, I'm mostly wondering if we're gonna cover all those uc_currency_format calls with at least the *opportunity* to run these transformations though uc_price(). I think it's a good idea...

Island Usurper’s picture

I'd been wondering how it was going to work when there were prices I needed to format but didn't think should be altered. Most of the time I would describe what the price was in the location, but didn't put a subject in the $context.

I was about to argue that sites with multi-currency would want to alter every price that's displayed, but I realized that the value of the prices don't change. Currency switching is a formatting issue, then. There are still some people who wanted to give products different prices in different currencies, but I think that's a bad idea to do on the same website.

Stuff I'd thought of while doing the implementation:
The discount handler really only needs to affect the sell price of nodes. However, even if uc_price() is modifying the node's cost, or not even a node field at all, like an attribute option's price adjustment, the discount handler will still alter the product sell price. Does allowing that to happen make sense? It might not matter since uc_discount_price_handler_alter() has a static variable to keep track of the nodes' new prices.

If we think it does matter, we can put more information in the $price_info, saying what "class" the main subject is, and the field to look at. This morning, I'm starting to think it's a bit more trouble than it's worth. I had thought I might need it for attributes, but since the product sell price has already been altered by the time uc_price() is called for the option adjustments, I decided not to include the product in the subject context.

I guess now I'll finish out attribute and catalog prices, and maybe we'll actually have this thing done!

Island Usurper’s picture

Status: Needs work » Needs review
FileSize
61.97 KB

OK, I'm going to try something a little different here. Keeping track of all these changes is getting a bit unwieldy, so I started committing to my local test branch. I've broken them out into logical pieces, starting with changing the locking mechanism for CA predicates. Awesomely, Bazaar lets me pass around merge directives without actually having to commit them to the main repository.

If you have Bazaar, all you need to do is

bzr update
bzr merge uc_price-399586.merge.txt

This will modify the files as if you had merged from my branch directly, but nothing will be committed yet. You can review changes with bzr log or qlog and inspect each commit.

If you don't have Bazaar, don't fret because the merge bundle contains patch instructions as well. I haven't tried it, but you may be able to apply the file as a regular patch as-is. If it doesn't work, take out the last part of the file containing the base64-encoded merge directive.

As we make changes, commit using the --local flag, and create merge directives with the send command:

bzr send -o [filename] http://bazaar.ubercart.org/drupal6/ubercart

Speaking of changes, I haven't made 'formatted' the default revision yet. It seemed like the uses of 'formatted' and 'themed' were about half. Instead, I figured that the real reason we wanted to use 'formatted' instead of 'themed' is because the <div> in theme_uc_price() was causing things to look funny. The simple solution was to change it to a <span>. Now it's automatically displayed inline, which probably means we can take out most of the times we use 'formatted'. I've also added some CSS to prevent word wrapping in .uc-price, which will fix another long-standing issue.

Marked as needing review because it feels complete. But if there's something missing, now is the time to make changes. I'm feeling really good about it, though. This is going to open up a lot of new opportunities for module writers.

Island Usurper’s picture

Reminder: #430022: Keep following Ubercart code that allows product discounts to work is the only patch that has a price alterer that's used very much. I suggest making a predicate that causes all of your products to have a 10% discount or price increase. This will help make sure that it's working with uc_price() well. It's also the reason I've included the CA predicate locking changes in this patch.

cha0s’s picture

FileSize
62.88 KB

Sorry if I sound like a broken record ;)

It didn't feel like you answered either of my questions;

Should we convert over the uc_currency_format()'s to uc_price() to allow other modules to do their thing with those prices?

Are we gong to implement some system to do just formatting but not altering? If so, what do you think of using bitmasks?

I made a change and bzr send'ed it... but wouldn't a patch be easier?

Island Usurper’s picture

I should have said to commit the merge locally before making more changes. Then we could just make patches that are a lot smaller.

But I think we're to a point where things won't break horribly, so I've just committed all the changes that have been made so far. bzr update and we'll just continue on as we have been.

I don't think we need to have a system that just formats prices without altering. Right now, none of the alters have a very wide scope, so they have to have rather specific things in the $context or they won't alter the price. That sounds correct, and we should encourage other price alterer authors to do it in the same way. If later on we find that we really do need to prevent altering in some places, then bitmasks sound like the way to go.

Just about all of the uc_currency_format()s should be changed. The only ones I might consider leaving in are the ones formatting prices in a particular way with XML for payment gateways. Those XML schemas can be rather picky. I grepped the code for all the remaining uc_currency_format()s, and it's still a daunting task.

Island Usurper’s picture

Here's a second round of implementing uc_price in Ubercart modules. This gets all of the rest of the uses of uc_currency_format() in the modules that were touched previously. I believe all that remains are the payment methods and gateways and uc_reports.

Even though I changed the views price field handler, I'm only going to guarantee that it will be formatted. I tried to fake a node object, but alterers won't have everything available to them that they may expect. It works with the Discount module on the default products view. Your mileage may vary.

Island Usurper’s picture

Assigned: cha0s » Unassigned
FileSize
9.8 KB

Here's the changed uc_report module. Ryan's going to get the payment methods done. We're getting closer to a release with each patch. Let's knock it out of the park!

Island Usurper’s picture

FileSize
32.75 KB

And finally, the payment methods.

I'm starting to rethink my decision to make the default theme use a <span>. Doing that makes the product page look bad when you have more than the sell-price enabled. Either their theme functions there need to put <div>s around them or we should make 'formatted' the default revision as was suggested earlier.

Thoughts, opinions, comments?

rszrama’s picture

This patch was committed and utilized for the latest revisions to the implementation patches. It adds formatted and themed original prices to the possible revisions.

rszrama’s picture

Nerf. Forgot to add the new revisions to the list of allowed revisions.

rszrama’s picture

One more try, this time with a fix to the original value!

Island Usurper’s picture

And committed!

This patch implements the *-original revisions and fixes some other implementations.

And there was much rejoicing.

"yay"

Island Usurper’s picture

Status: Needs review » Fixed
FileSize
45.63 KB

*facepalm*

IckZ’s picture

I'm a little bit confused. Is the last patch from #63 including all others posted before or which of them should I "activate"?

Island Usurper’s picture

Get 6.x-2.0-rc1, since it has all of the changes already. I realized after I posted that patch that I had screwed up whatever naming scheme I had for these patches (my excuse is that it was so late), so I don't remember which patches should be applied to beta6 anymore either.

IckZ’s picture

thanks, using the rc1 is also a more cleaner way to get this problem solved. thanks for that great work!

jurgenhaas’s picture

Am I going right assuming that the rc1 contains a framework for improved price-handling but no additional functionality yet? What's needed next is a module which makes use of this framework and comes with support for european vat handling. Or have I missed something?

Island Usurper’s picture

That's correct, jurgenhaas. Someone needs to take the ideas and techniques presented in this issue and make some code out of it.

shiroitatsu’s picture

Is it now possible to add a new pricehandler to show prices so that it includes fields for price without tax and amount of tax (or amount of tax and price with tax)? How would that happen? It would be the only thing that i would need to move on with this thing...

Al01’s picture

@Ryan, Lyle, Cha0s and Mike: Big thank you for all your hard work!

I just figured out that between uc_price-399586.merge_.txt and uc_price_implementation_3_0.patch the whole patch for uc_product.module got lost.

Island Usurper’s picture

Are you sure? uc_price-399586.merge_.txt, plus the comment.txt after it were committed to the repository. Anything after that should apply on top of it.

If I really did miss something, make a new patch for me so that it will apply correctly to the latest version.

Al01’s picture

FileSize
4.59 KB

New hardware and old issues in generating patches are back for the moment... Just copied now from comment.txt, so the patch will not apply even the line numbers didn't change. Might be also a windows issue.

Island Usurper’s picture

Crap. It looks like I screwed up committing those changes to CVS somehow. It's there in the Bazaar repository. I guess this means there will be a rc2 before too long.

Island Usurper’s picture

FileSize
12.59 KB

There are more files than just uc_product.module that didn't get committed right. Here's a patch that should be put on rc1 to make it what I intended it to be. I feel very lucky that none of these changes are essential for any other code to work.

robertDouglass’s picture

I think this is the type of "oops" that a test suite is good at catching.

Island Usurper’s picture

I want to direct people following this issue to #449688: Price adjustments not carrying through to checkout. There's a question in it about how the data in $context should be organized, and I'd like more opinions than just mine. :)

reglogge’s picture

Hi,

it's really great to see that this feature (different ways of displaying and processing taxes) finally seems to be within reach. Not being a real coder myself, I'm wondering whether anyone is actually working on a module that implements a capability to set the various options from the administration interface. I mean options like how taxes are being calculated and displayed to the user?

Any news on this front? Thanks again to alle the members here for putting so much effort into this issue!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

grub3’s picture

I am a little bit lost. What should I download to display prices with VAT included?

cha0s’s picture

@jmpoure: You'll want to look up http://drupal.org/project/uc_vat