Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] sale_packaging_price: New module to assign a package price. #221

Merged
merged 7 commits into from
Dec 21, 2015
Merged

[ADD] sale_packaging_price: New module to assign a package price. #221

merged 7 commits into from
Dec 21, 2015

Conversation

sergio-teruel
Copy link
Contributor

This module was written to extends the functionality of sale workflow and allow you to assign a selling price of a complete package.
This price is split for having the price unit according the number of pieces that fits in that packaging, so there can be rounding issues to get the exact price that are warned if happens.

@rafaelbn
Copy link
Member

rafaelbn commented Oct 8, 2015

I cannot test it, something wrong in runbot. Please could you review? Thanks


To use this module, you need to:

* go to warehause config and check 'Use packages: pallets, boxes, ...' and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/warehause/warehouse

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be on Configuration section

@pedrobaeza
Copy link
Member

Remove sale_packaging_price/i18n/.gitkeep

action="action_product_packaging_form"
parent="base.menu_product" sequence="99" />
</data>
</openerp>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra spaces and put a line feed

@pedrobaeza
Copy link
Member

You need to add product_packaging field to the form view.

@@ -0,0 +1,30 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why name it sale_stock.py instead sale_order.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rewrite function product_packaging_change that is located into sale_stock module in sale_stock.py file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's not wrong. You should rename it to sale_order (model name, not module name).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting from there:

File naming: [...]
models/< inherited_main_model >.py

And class name is correct in CamelCase, also as guidelines say.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't know what's wrong with my proposal: rename it to sale_order.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it was a mistake, you said:

You should rename it to sale_order (model name, not module name).

The model name is SaleOrderLine, correct.

Indeed the module should be sale_order.py.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the main model is sale.order, not sale.order.line, that's why I ask for sale_order.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK, I was understanding that you wanted him to rename the model! We agree then 😉

@pedrobaeza
Copy link
Member

You have to also intercept product_id_change for handling changes on quantity. You should also prevent when packaging is selected, non multiple quantities.

@sergio-teruel
Copy link
Contributor Author

@rafaelbn @pedrobaeza Changes made

* go to warehouse config and check 'Use packages: pallets, boxes, ...' and
'Allow to define several packaging methods on products'

* go to product and into procurement tab you can add new packages with a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is Usage section. I was talking about the other paragraph

@sergio-teruel
Copy link
Contributor Author

@rafaelbn @pedrobaeza Chnges made

@pedrobaeza
Copy link
Member

👍

Bug Tracker
===========

Bugs are tracked on `GitHub Issues <https://github.com/OCA/167/issues>`_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

167?

@yajo
Copy link
Member

yajo commented Oct 20, 2015

Testing in runbot, I created a package with 6 wine bottles (individual price would be 20€, pack price 100€).

See the results:
captura de pantalla de 2015-10-20 17-06-36

100.02€ is not the price I set.

To avoid this kind of problems, add a constraint that will not let the user to set an icompatible unit + package price combination. It will hassle them, but better hassle the user than the client.


To use this module, you need to:

* go to product and into procurement tab you can add new packages with a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in the inventory tab.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.
About the previous comment(100.02) you can increment the price decimal precision.

@sergio-teruel
Copy link
Contributor Author

@yajo @pedrobaeza @rafaelbn Changes made.

@yajo
Copy link
Member

yajo commented Oct 23, 2015

Much better, but still getting #221 (comment) though. Fix it and test it please.

@sergio-teruel
Copy link
Contributor Author

@yajo about #221 (comment) you can increment the price decimal precision.

@yajo
Copy link
Member

yajo commented Oct 27, 2015

Not sure how to do that. Can you explain?

Anyway, independently of the decimal precision, if you set a final price for a bundle of products, that should always be the final price. If the final price cannot be exactly matched because of the mix of quantity, price and decimal precision, then it should not allow to put that price in the first place.

@carlosdauden
Copy link
Contributor

cc @pedrobaeza @yajo @rafaelbn @sergio-incaser

@yajo
Copy link
Member

yajo commented Nov 10, 2015

The code looks good, but there is a new bug.

  1. Add a new product.
  2. Go to Inventory > Packaging > Configurations.
  3. Add new.

Error:

Traceback (most recent call last):
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/http.py", line 537, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/http.py", line 574, in dispatch
    result = self._call_function(**self.params)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/http.py", line 310, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/service/model.py", line 118, in wrapper
    return f(dbname, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/http.py", line 307, in checked_call
    return self.endpoint(*a, **kw)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/http.py", line 803, in __call__
    return self.method(*args, **kw)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/http.py", line 403, in response_wrap
    response = f(*args, **kw)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/addons/web/controllers/main.py", line 944, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/addons/web/controllers/main.py", line 936, in _call_kw
    return getattr(request.registry.get(model), method)(request.cr, request.uid, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/api.py", line 250, in wrapper
    return old_api(self, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/api.py", line 372, in old_api
    result = method(recs, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/models.py", line 5929, in onchange
    record._onchange_eval(name, field_onchange[name], result)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/models.py", line 5787, in _onchange_eval
    method_res = method(self)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3119204-221-bd055f/openerp/addons/sale_packaging_price/models/product.py", line 23, in _onchange_list_price
    round(self.list_price / self.qty, price_precision) *
ZeroDivisionError: float division by zero

@yajo
Copy link
Member

yajo commented Nov 10, 2015

Seems to happen when qty = 0. When saving a wrong package price, the warning works as expected.

@carlosdauden
Copy link
Contributor

@pedrobaeza @yajo @rafaelbn Changes made

@carlosdauden
Copy link
Contributor

@pedrobaeza Changes made

@carlosdauden
Copy link
Contributor

cc @yajo @rafaelbn

@rafaelbn
Copy link
Member

About this: #221 (comment) I agree with @yajo . Check: Setting -> Technical -> Database structure -> Decimal accuracy

@rafaelbn
Copy link
Member

To solve problem of decimal precision please add in usage (even-though this is not the solution):

  • Go to Setting -> Technical -> Database structure -> Decimal accuracy, select Product Price and set Digits to 3, if you get better accuracy set 4 digits of higher.

Been exactly: (maybe I need some illustration here)

sale_packaging_price: New module to assign a package price

  • A package in Odoo is object stock.quant.package - Go To Warehouse -> Products -> Packages.
  • A Logistic Unit in Odoo is object product.ul - Go To Warehouse -> Configuration -> Products -> Logistic Units. Also here I cannot see price.
  • A product packaging in Odoo is object product.packaging - Go To Warehouse -> Products -> Products packaging. In the product form (tab Inventory) this is called Packaging configuration.

Having this clear (I'm not sure if I have) the objective of module:

  • To sell 1 product in packages, then if you make a quotation or invoice you are not selling 6 units of individual product, you are selling 1 unit of the package. Thanks why you set the price in the package and not in the product.
    • With this point of view you can guess that the problem of decimal precision disappears as if you say a package/box cost 100 and you sell 1 unit of a package.
    • So you buy individual units or packages/boxes but you store units
      For example:
    • You buy 1 package of 12 bottles of cola (6€) and you stock is 12 bottles of cola, each one cost 0.50€
    • You sell packages of 2 bottles of cola, each package has sell price 2€.

@rafaelbn
Copy link
Member

What do you think about this? #221 (comment)
cc @liebana

@rafaelbn
Copy link
Member

@sergio-incaser runbot is failing, could you please check it? thanks

@liebana
Copy link

liebana commented Nov 17, 2015

We have something for one customer, we based in the old product-pack of v6.1. You first define a service product as a bundle:

seleccion_999 076

and then you select which are their components:

seleccion_999 077

Then, because the first product is a service and there can be different VATs on the components in our case, we detail everything in the sale order

image

but only the stock of the components is carried to the delivery order.

But this module is not finished, because is intended to work with Magento connector and right now it's not possible to create from the ERP the sale order itself. We can upload the code anyway if you want.

This is the way we would eventually solve your requirement, but I haven't look your solution.

@yajo
Copy link
Member

yajo commented Nov 20, 2015

IMHO this solution is better integrated with current core Odoo. The notice of decimal precision seems fair enough to me. If you want 1.99€/pack, you will be warned, but not disabled to do it.

@rafaelbn rafaelbn added this to the 8.0 milestone Dec 2, 2015
@rafaelbn
Copy link
Member

rafaelbn commented Dec 2, 2015

Hi @liebana could you make a PR in https://github.com/OCA/product-attribute/tree/8.0 with that functionality? Module name could be product_pack, this could be better to compare. Ping us when done please, thanks.

@yajo
Copy link
Member

yajo commented Dec 2, 2015

👍 to this PR, but @liebana's implementation is interesting too. Could you send it please?

@pedrobaeza
Copy link
Member

That module is in 8.0 in https://github.com/ingadhoc/odoo-addons/tree/8.0/product_pack, but it has the problems I mention here: OCA/product-attribute#6 (comment)

@rafaelbn
Copy link
Member

Thanks! Tested in runbot 👍

rafaelbn added a commit that referenced this pull request Dec 21, 2015
[ADD] sale_packaging_price: New module to assign a package price.
@rafaelbn rafaelbn merged commit 4e078a5 into OCA:8.0 Dec 21, 2015
@sergio-teruel sergio-teruel deleted the sale_packaging_price branch December 24, 2015 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants