To be honest, the patch looks pretty good to me. I haven't thoroughly reviewed it but it appears to be a good implementation.
Ideally, I'd like some docs on how to implement a custom tax processor so we can get something in place. Other than that I'd like people to review but it looks like a reasonable approach. -Chris On Fri, Mar 18, 2011 at 9:28 PM, Nan <[email protected]> wrote: > > I think the best way to explain my patches to Satchmo is to actually > show them to you: > > https://bitbucket.org/ringemup/satchmo > > The patch is complete, is very simple, and passes all tests. The > additional processing is minimal; by_product_and_price acts > identically to by_price in the existing processors; and by_price is > used directly for shipping and other situations where there is no > product in question. > > My next step is to write the custom tax processor; I can use the code > in that to demonstrate to you what it accomplishes and why it needs > the product. > > > On Mar 18, 10:19 pm, hynekcer <[email protected]> wrote: > > OK. How to do it? > > > > I think that more people wants better performance in their situations > > (when you read this forum) and reliability than extending the > > complexity. > > > > The name by_product_and_price sounds redundant, hmm, but it is not.It > > can be useful because price calculation is an expensive operation and > > should not be unnecessarily repeated. It is also useful to keep tax > > modules and discount methods independent and preferably simple. Method > > by_product is not usable for undiscounted prices. Method get_rate > > looks universal and it is basic method for two complicated tax > > processors, but it should be internal method because it is not known > > out of tax processor which context parameters are mandatory for that > > processor. > > > > Why are too much tax.by_* methods? May have been attempts to solve > > rounding problems with 2 decimal places arithmetics with specialized > > methods for one and total. Finally, it was decided at commit 166 three > > years ago to save all 10 decimal places. Then it became less > > important. It was before issues history. > > > > > So patching Satchmo to change those calls shouldn't mean > > > any disruption to people using those processors. > > > > How do you ensure it? How will the new method by_product_and_price > > help you ? > > > > The big problem is with functions which depends on taxer and call only > > by_price: > > shop.models.OrderItem.update_tax > > payment.forms._get_shipping_choices > > product.utils.productvariation_details > > (All 3 templatetags satchmo_discounts.taxed_* can be eventually > > replaced without changing Satchmo or templates.) > > > > Some deep stored methods depends on it. > > shop.models.OrderItem.save > > > > product.modules.configurable.models.Configurable.add_template_context > > payment.forms.SimplePayShipForm.__init__ > > Also product itself depends on that. This does look nice at all. > > > > Normal fuction can be dirty patched in a private project by > > manipulation with sys.models['modelname'].__dict__['function_name'], > > but to do it with django.db.models children would be a harakiri. > > > > This is a mystery now > > if self.product.taxable: > > self.unit_tax = processor.by_price(taxclass, > > self.unit_price) > > self.tax = processor.by_orderitem(self) > > After you verify internals and simplify existing tax processors it can > > be probably also simplified without by_orderitem and make it > > deprecated. > > > > These approximately 6 lines with by_price can be replaced by: > > - .... taxer.by_price(product.taxClass, price) > > + if hasattr(taxer, 'need_product_detail'): > > + .... price * taxer.get_rate(product=product) > > + else: > > + .... taxer.by_price(product.taxClass, price) > > > > and you can use it soon. > > > > While it will not make any problems some time (exactly the same > > results, speed, database queries and memory requirements for normal > > taxed projects), it can be something done to be eventually unified in > > BaseProcessor and some redundant methods declared as deprecated and > > not used in a new development. > > > > On Mar 18, 9:52 pm, Nan <[email protected]> wrote: > > > > > To follow up, I think your idea of a base class is a really good one. > > > I'm going to actually try something similar: > > > > > 1) Create a new BaseProcessor and update the existing Tax Processors > > > to inherit from it > > > > > 2) Include a new by_product_and_price method in BaseProcessor, which > > > defaults to simply calling by_price > > > > > 3) In locations where by_price is currently called but product data is > > > available, change the calls to by_product_and_price > > > > > This likely could be accepted into Satchmo trunk without any > > > disruption of existing sites. > > > > > Then I can just write a custom processor that inherits from > > > tax.modules.area and overrides the by_product_and_price method and the > > > _get_location method. > > > > > > -- > You received this message because you are subscribed to the Google Groups > "Satchmo users" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/satchmo-users?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Satchmo users" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/satchmo-users?hl=en.
