I've added some skeleton documentation to the branch: https://bitbucket.org/ringemup/satchmo
On Mar 18, 11:28 pm, Nan <[email protected]> wrote: > Heh, I'd love to have some documentation to work from, too! I'll > mostly be duplicating tax.modules.area and working from [1], but will > jot some notes in the process, and hopefully at least be able to add > to what exists. I'll be starting in on that some time Monday. > > If you'd like documentation for the BaseProcessor in my patch, I'll > try, but TBH, I'm not entirely clear yet on what each of those methods > is used for, or where it's called from. > > [1]http://groups.google.com/group/satchmo-users/browse_thread/thread/6bb... > > On Mar 18, 11:12 pm, Chris Moffitt <[email protected]> wrote: > > > 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.
