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] Add sale order lot selection #140

Closed
wants to merge 25 commits into from
Closed

[ADD] Add sale order lot selection #140

wants to merge 25 commits into from

Conversation

hurrinico
Copy link

This Module Allows you to select on sale order the production lot that will be delivered

Sale Order Lot Selection
========================

This Module Allow you to select on sale order the production lot that will be delivered
Copy link
Member

Choose a reason for hiding this comment

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

s/Allow/allows

@pedrobaeza
Copy link
Member

Please put an specific icon (and smaller) for the module, to not confuse with other functionality.

About functionality itself, I miss:

  • Check that the selected lot has stock available.
  • Reserve stock when confirming or saving the order.

'name': 'Sale Order Lot Selection',
'version': '1.0',
'category': 'Sales Management',
'author': "Agile Business Group",
Copy link
Member

Choose a reason for hiding this comment

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

Please add OCA as author

@hurrinico hurrinico changed the title [FIX] Add sale order dolder [FIX] Add sale order lot selection Apr 21, 2015
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.11%) to 80.13% when pulling ae02f1f on hurrinico:add_sale_order_lot_selection into a9ed64c on OCA:8.0.

@hurrinico
Copy link
Author

Hi @pedrobaeza in my last commit i've made some changings.
Now i reserve lot on sale order confirm, i'm thinking on how to check lot's availability on sale order but it's quite difficult; you have some suggestion about it?
Thank you in advance

@pedrobaeza
Copy link
Member

Why don't you reserve instead quants (with a many2many) in the same manner that Odoo works? That way, you only need to lock that quants. By standard, you can't do that, but you can use same mechanism than this: https://github.com/odoomrp/odoomrp-wip/blob/8.0/mrp_lock_lot/models/stock_quant.py to forbid to use that quants.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.96%) to 83.2% when pulling ec6f8f1 on hurrinico:add_sale_order_lot_selection into a9ed64c on OCA:8.0.

@hurrinico hurrinico changed the title [FIX] Add sale order lot selection [ADD] Add sale order lot selection Apr 29, 2015
@pedrobaeza pedrobaeza mentioned this pull request Jul 2, 2015
This was referenced Jul 2, 2015
@Viggor
Copy link

Viggor commented Jul 2, 2015

Have you done a similar thing for mrp like #176 because we have done a similar feature for the sale #175 and stock #177.

Viggor pushed a commit to akretion/sale-workflow that referenced this pull request Jul 3, 2015
Viggor pushed a commit to akretion/sale-workflow that referenced this pull request Jul 3, 2015
@Viggor Viggor mentioned this pull request Jul 3, 2015
@pedrobaeza
Copy link
Member

@hurrinico, please check the PR against your branch and move this to merge the module.

@hurrinico
Copy link
Author

@pedrobaeza @eLBati @Viggor i've made some changes to tests and now travis turns green

from openerp import fields, models, api


class procurement_order(models.Model):
Copy link
Member

Choose a reason for hiding this comment

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

Please use CamelCase for class names

@pedrobaeza
Copy link
Member

I also wonder if we should restrict the lot selection to the current available lots. If not, we should note in the README that the lot selection doesn't correspond with available lots in stock (it's only a priority), but I'm not sure that in this case, the module will be useful.

@eLBati
Copy link
Member

eLBati commented Aug 3, 2015

Afaik, the module prevents to use unavailable lots.
I suggest to add a test for the failing assignment
On 1 Aug 2015 21:06, "Pedro M. Baeza" notifications@github.com wrote:

I also wonder if we should restrict the lot selection to the current
available lots. If not, we should note in the README that the lot selection
doesn't correspond with available lots in stock (it's only a priority), but
I'm not sure that in this case, the module will be useful.


Reply to this email directly or view it on GitHub
#140 (comment).

@pedrobaeza
Copy link
Member

Indeed there's a warning programmed in an onchange, but it has 2 problems:

  • The warning is not preventing to select that lot.
  • You have to click on each lot to see which one is available, so it can be very tedious and not valid for a product with lots of serial numbers.

I suggest you to pre-calculate in a many2many field the available lots using quants, calling the method for getting the preferred domain and mapping the lots of these quants.

@hurrinico
Copy link
Author

@pedrobaeza I've added a test case in order to dimostrate that if a lot on a sale order is already restricted, you can't confirm the order.

@pedrobaeza
Copy link
Member

Yeah, I know, but my remark is about the usability. Can you read it again?

@hurrinico
Copy link
Author

Ok @pedrobaeza we have understand the problem, and we agree with you.
Have you some idea about the fix? (this link doesn't work anymore https://github.com/odoomrp/odoomrp-wip/blob/8.0/mrp_lock_lot/models/stock_quant.py )
I was thinking about a new module that add a computed fields (boolean) to identify an available's lot?

@pedrobaeza
Copy link
Member

The idea is to have a many2many computed field available lots in the sale.order.line, that depends on the warehouse, product and the quantity. This field is calculated calling the self.env['stock.quant'].quants_get_prefered_domain with the corresponding arguments, and applying that domain to make a search. Then, make a mapped function to retrieve the lots associated (this doesn't remove duplicates).

@hurrinico
Copy link
Author

Hi @pedrobaeza , we have find this solution; we have replaced the onchange on lot_id with an override to the method "product_id_change_with_wh" where we have called (as you suggest) quants_get_prefered_domain and extended the domain. Could be this a solution?

@hurrinico
Copy link
Author

@eLBati you are right!! i have made the requested changes.

@hurrinico
Copy link
Author

@eLBati i've fixed the indentation

@eLBati
Copy link
Member

eLBati commented Aug 26, 2015

Tried on runbot:

  • install module
  • create new SO
  • add a new line, selecting 'Ice Cream' product
  • go to lot field and click on arrow

get

Traceback (most recent call last):
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/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/3113776-140-40501d/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/3113776-140-40501d/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/3113776-140-40501d/openerp/service/model.py", line 113, in wrapper
    return f(dbname, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/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/3113776-140-40501d/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/3113776-140-40501d/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/3113776-140-40501d/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/3113776-140-40501d/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/3113776-140-40501d/openerp/api.py", line 241, in wrapper
    return old_api(self, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/api.py", line 336, in old_api
    result = method(recs, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/models.py", line 1725, in name_search
    return self._name_search(name, args, operator, limit=limit)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/api.py", line 239, in wrapper
    return new_api(self, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/api.py", line 463, in new_api
    result = method(self._model, cr, uid, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/models.py", line 1737, in _name_search
    ids = self._search(cr, user, args, limit=limit, context=context, access_rights_uid=access_rights_uid)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/api.py", line 241, in wrapper
    return old_api(self, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/models.py", line 4654, in _search
    query = self._where_calc(cr, user, args, context=context)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/api.py", line 241, in wrapper
    return old_api(self, *args, **kwargs)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/models.py", line 4465, in _where_calc
    e = expression.expression(cr, user, domain, self, context)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/osv/expression.py", line 646, in __init__
    self.parse(cr, uid, context=context)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/osv/expression.py", line 749, in parse
    self.stack = [ExtendedLeaf(leaf, self.root_model) for leaf in self.expression]
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/osv/expression.py", line 532, in __init__
    self.check_leaf(internal)
  File "/srv/openerp/instances/openerp-oca-runbot/parts/odoo-extra/runbot/static/build/3113776-140-40501d/openerp/osv/expression.py", line 588, in check_leaf
    raise ValueError("Invalid leaf %s" % str(self.leaf))
ValueError: Invalid leaf id

@hurrinico
Copy link
Author

@eLBati i've fixed with my last commit

@eLBati
Copy link
Member

eLBati commented Aug 28, 2015

I'm closing this in favor of #202 (I fixed last issues and squashed some commits)

Recent updates, which should fix the remaining problems, are
eLBati@57eb332
eLBati@e671c1d

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