Revert changes to computation of _immediately_usable_qty + distinguish product/variants

[clonedagain]

Commit 6c16913 changed the way we compute the immediately_usable_qty: instead of using the virtual stock, we used the sum of quants without reservations. But a quant may actually be reserved and still be available (for example it may be reserved for an internal move).
Fixes #79

Also fixes #73 by correctly computing template value as sum of variant values, and placing the fields on views of both the templates and variants.

[pedrobaeza]

You can put @api.one and avoid the loop

[clonedagain]

Indeed.

[clonedagain]

@gfcapalbo @gurneyalex @StefanRijnhart @laetitia-gangloff do you agree?

[laetitia-gangloff]

Why keep immediately_usable_qty if it is to set the same value than virtual_available ?

Other question, why stock_available_immediately is empty ?

[clonedagain]

stock_available only proposes a dummy implementation with configuration options to install the wanted features.
v8 only has the old stock_available_immediately but v7 has 2 more features ("exclude sales quotations" and "include production capability"), which I intend to port to v8 some time later.
Each feature is optional and can be cumulated with the others (for example we don't use stock_available_immediately but only the other two).

[florian-dacosta]

I also, I wonder why the module stock_available_immediately is empty!

I also agree with your explanation, that a quant can be reserved but still available but if you do that, the module stock_available will be useless for now since stock_available_immediately is empty and the other 2 art not ported yet.
So until then, it may be better to keep it this way no?

[clonedagain]

OK I hadn't understood your statement that stock_available_immediately is empty, indeed it should not be. I'll sort it out and update this PR.

You may mark "WIP".

[gfcapalbo]

Thank you @clonedagain for detecting problems post-merge. The initial concept of defining "available immediately" via quants was due to the fact that I had a client with internal moves to production that shouldn't be counted. In my case it works very well, but there are more use cases to consider.
As you say, a slight semantic shift. I hadn't thought about all the other internal moves that may influence this parameter.

What I don't understand is the alternative: self.immediately_usable_qty = self.virtual_available we are just duplicating information, doesn't seem to make sense.If the two values are the same just get rid of one.

virtual_available is defined as:

virtual_available = float_round(quants.get(id, 0.0) + moves_in.get(id, 0.0) - moves_out.get(id, 0.0), precision_rounding=product.uom_id.rounding)

Does it semantically may sense to define immediately_available as: quants - moves out ? In other words virtual_available + moves_in ? I propose docstrings to explain the semantics of each variable.
Would it be a welcome addition to extend my quant implementation taking in consideration the possible internal moves?

[clonedagain]

I've restored the computation in stock_available_immediatly so should make sense now.
Let's see if the tests run green.

@gfcapalbo yes "immediately_available as: quants - moves out" is basically what stock_available_immediatly does now.
Except than:
1/ it's written as virtual - incomming - it should be the same result but only uses data we have already computed
2/ it does not exclude reserved quants.
I don't see how we could introduce 2/ back, since outgoing and incoming moves may have reservations or not. If you have an idea about how we can do that, let's make an additional sub-module for that to make it modular/optional.

[gfcapalbo]

With your additions it makes sense.
I need to exclude reserved quants in my particular configuration. My client interprets this calculation of stock_available_immediately as "wrong", obviously it's a matter of interpretation.
It should be possible to do as a separate module, to offer the alternative.
Quick fix:
I could create new module. It would overwrite the calculation of "stock_available_immediatly" but would check only for companies with 1 internal location, therefore eliminating the internal move problem.

[clonedagain]

I think the travis failure is unrelated isn't it?

[gurneyalex]

👍

Ran the following test scenario on runbot

• install purchase
• create a new stockable product test1 with an initial inventory of 100
• create a draft SO for 10 test1
• create a draft PO for 5 test1
• virtual stock == 100, available to promise == 100 => OK
• confirm PO
• virtual stock == 105, available to promise == 100 => OK
• confirm SO
• virtual stock == 95, available to promise == 90 => OK

Similar test with warehouse configured in Pick + Ship

Also, this new implementation handles the context properly so it is possible by passing a modified to context to limit the stock to a given warehouse.

[charbeljc]

👍

[gurneyalex]

@clonedagain build should be fixed by #82

[clonedagain]

@gurneyalex I think travis now chokes on the unicode author "Numérigraphe".
https://travis-ci.org/OCA/stock-logistics-warehouse/jobs/80427466#L205
Do you want me to report this to maintainer-tools?

[gurneyalex]

@clonedagain yes and you can assign @moylop260 to the issue 😉

[moylop260]

@clonedagain
MQT fixed now real beta messages:

Start lint check just in modules changed
************* Module stock_available_immediately
stock_available_immediately/__init__.py:1: [C8201(no-utf8-coding-comment), ] No UTF-8 coding comment found: Use `# coding: utf-8` or `# -*- coding: utf-8 -*-`
stock_available_immediately/__init__.py:1: [W8201(incoherent-interpreter-exec-perm), ] Incoherent interpreter comment and executable permission. Interpreter: [] Exec perm: True
************* Module stock_available_immediately.models.__init__
stock_available_immediately/models/__init__.py:1: [C8201(no-utf8-coding-comment), ] No UTF-8 coding comment found: Use `# coding: utf-8` or `# -*- coding: utf-8 -*-`
stock_available_immediately/models/__init__.py:1: [W8201(incoherent-interpreter-exec-perm), ] Incoherent interpreter comment and executable permission. Interpreter: [] Exec perm: True
************* Module stock_available_immediately.tests.__init__
stock_available_immediately/tests/__init__.py:1: [C8201(no-utf8-coding-comment), ] No UTF-8 coding comment found: Use `# coding: utf-8` or `# -*- coding: utf-8 -*-`
************* Module stock_available_immediately.tests.test_stock_available_immediately
stock_available_immediately/tests/test_stock_available_immediately.py:23: [C8104(class-camelcase), testStockLogisticsWarehouse] Use `CamelCase` "TestStockLogisticsWarehouse" in class name "testStockLogisticsWarehouse". You can use oca-autopep8 of https://github.com/OCA/maintainer-tools to auto fix it.
Found 6 errors in modules changed.
These checks are still in beta: they won't affect your build status for now.

• Add coding in all py files.
• Run: chmod -x stock_available_immediately/models/__init__.py
• Use CapWords in class name

[guewen]

I don't think that coding: utf-8 is really necessary in __init__.py files when they contains only imports or nothing...

[clonedagain]

I'll check it now thanks.

[lbellier]

👍

[moylop260]

@guewen
Ok, good point!
Please, Could you create a PR to add __init__.py exception in Each python file should have # -*- coding: utf-8 -*- as first line?
https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#idioms

[moylop260]

👍