[11.0][MIG] mrp_mto_with_stock #255

bodedra · 2018-02-16T17:14:16Z

…e flexibility

guewen · 2018-02-19T13:26:47Z

mrp_mto_with_stock/demo/product.xml

+
+<odoo> <data noupdate="1">
+
+        <record id="product_product_manufacture_1" model="product.product">


could be the occasion to fix this 8 spaces indentation?

I have notice same. Somewhere I have read that formatting changes not allow. So ignore it. May we allow to fix it in XML files ?

Formatting changes are to avoid when you do a pull request for a change in an existing version. I'd say as we are doing a large diff anyway it could be the right moment to fix it. On the other hand, it can make the port of pull requests less easier. No strong opinion then.

Corresponded. Thank you.

guewen · 2018-02-19T13:29:30Z

mrp_mto_with_stock/models/mrp_production.py

+    @api.multi
+    def _adjust_procure_method(self):
+        # Si location => By pass method...
+        super(MrpProduction, self)._adjust_procure_method()


The code is the same in the 10.0 version, but I don't see the point of it

Yes. I am agree. We will remove it.

Corresponded. Thank you.

LoisRForgeFlow

Tested locally, does not work as expected.

LoisRForgeFlow · 2018-02-22T11:07:54Z

mrp_mto_with_stock/models/mrp_production.py

                    if move.group_id:
-                        domain.append(('group_id', '=', move.group_id.id))
-                    procurement = self.env['procurement.order'].search(domain)
+                        procurement = True


Why are you asuming that if you have group you don't have to procure?

@lreficent will improve these. Appreciate your feedback.

LoisRForgeFlow · 2018-02-22T11:09:43Z

mrp_mto_with_stock/tests/test_mrp_mto_with_stock.py

@@ -68,18 +67,11 @@ def test_manufacture_with_forecast_stock(self):

        self.assertEqual(self.production.availability, 'partially_available')

-        self.assertEquals(self.subproduct1.virtual_available, 0)
+        self.assertEquals(self.subproduct1.virtual_available, -3)


Well, I didn't check in deep but my first thought here is that this is not the way to migrate tests. If tests are failing change the logic, don't adapt the result to what you are getting.

@lreficent Here is explanation for -3 virtual quantity.

Update self.subproduct1 with 2 quantity and MO is generated with 5 quantity. So 2-5=-3

I will do more analysis on it and update you. Thanks for trigger.

Yes, of course but the module should use that 2 units and then create a procurement for the remaining 3. The previous 0 was there for a reason, that's what I meant.

I would recommend you to review the functionality in v10 and understand it a little bit more and then come back to v11 so you will know what to expect and could figure out what to fix

@bodedra In v10, the method action_assign in https://github.com/OCA/manufacture/blob/10.0/mrp_mto_with_stock/models/mrp_production.py#L20 is supposed to create a new procurement order to fulfill the missing quantity of the component.

In v11, you need to migrate the method to create a new purchase order or manufacturing order (depending on the product configuration). This will increase the forecast quantity and it will match the expected quantity set in the tests.

Not sure with the scope of this module, looking at the code I haven't seen regarding orderpoints on the procurement should we also need to consider it?

If an MO that triggered for a PO is cancelled what would happen to the PO? What if PO is on Draft/RFQ state? How about if PO is on confirmed state?

@lreficent @max3903 @agyamuta

I still have problem in following code.

qty_to_procure = move.remaining_qty - move.reserved_availability

Request to provide few guidance on it.

I have improved logic and test case. Would you please provide me feedback on it.

jgrandguillaume · 2018-02-22T15:29:49Z

I agree with @lreficent, please do not merge this branch before fixing this properly (maening not the test result). @max3903 You can drop a card at us to help here if required.

bodedra · 2018-03-06T12:23:51Z

Hi All,

Following screen-shots represents test case in Odoo 10. It seems have problem in it.

Current behavior

Create "P1" and "R1" product. Update "2" quantity of "R1" in location stock.

"R1" product manufacturing MTO/MTS location configuration.

Create bill of materials of "P1" product.

Create manufacturing order of "P1" with quantity 12 and save it.

It will trigger purchase order for "R1" with 12 quantity as per our product configuration.

"R1" product quantity on hand and forecasted quantity.

Expected behavior

First use 2 quantity in stock location and then create PO with 10 quantity to fulfil MO requirements.

Would you please correct me on these.

@lreficent May we do screen share via google hangout if possible for you?

JordiBForgeFlow · 2018-03-06T12:27:29Z

I think that the problem is that you have selected the route "Make to order" for R1. And this overrides the MTO-MTS. Please remove the 'Make to order' route and re-try.

…

On Tue, Mar 6, 2018 at 1:23 PM, Bhavesh Odedra ***@***.***> wrote: Hi All, Following screen-shots represents test case in Odoo 10. It seems have problem in it. Current behavior 1. Create "P1" and "R1" product. Update "2" quantity of "R1" in location stock. [image: screenshot from 2018-03-06 17 19 40] <https://user-images.githubusercontent.com/14229503/37031370-821c25cc-2164-11e8-9bee-63258953ab79.png> [image: screenshot from 2018-03-06 17 19 58] <https://user-images.githubusercontent.com/14229503/37031378-8a28bef6-2164-11e8-9631-371fa6f0af7b.png> 1. "R1" product manufacturing MTO/MTS location configuration. [image: screenshot from 2018-03-06 17 45 14] <https://user-images.githubusercontent.com/14229503/37031847-6b93493c-2166-11e8-9bad-efa9579dbcb3.png> 1. Create bill of materials of "P1" product. [image: screenshot from 2018-03-06 17 20 25] <https://user-images.githubusercontent.com/14229503/37031429-b3bba0f8-2164-11e8-9366-09c66a03f95b.png> 1. Create manufacturing order of "P1" with quantity *12* and save it. [image: screenshot from 2018-03-06 17 20 54] <https://user-images.githubusercontent.com/14229503/37031492-f4dbf998-2164-11e8-8bcd-57fbaa47370b.png> [image: screenshot from 2018-03-06 17 21 23] <https://user-images.githubusercontent.com/14229503/37031497-fa9f02b2-2164-11e8-9831-48beaac70d0d.png> 1. It will trigger purchase order for "R1" with *12* quantity as per our product configuration. [image: screenshot from 2018-03-06 17 21 30] <https://user-images.githubusercontent.com/14229503/37031528-255409b2-2165-11e8-808f-70552d21bfb2.png> 1. "R1" product quantity on hand and forecasted quantity. [image: screenshot from 2018-03-06 17 21 41] <https://user-images.githubusercontent.com/14229503/37031569-4d2a5450-2165-11e8-8c54-a946d04eb0aa.png> Expected behavior First use *2* quantity in stock location and then create PO with *10* quantity to fulfil MO requirements. ------------------------------ Would you please correct me on these. @lreficent <https://github.com/lreficent> May we do screen share via google hangout if possible for you? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#255 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHU_Vj1JF0YigcPXu6q67pItk6KEAHiOks5tbn_bgaJpZM4SIppZ> .

-- Jordi Ballester Alomar CEO & Founder | Eficent (+34) 629530707 | jordi.ballester@eficent.com | http://www.eficent.com Twitter: https://twitter.com/jbeficent_erp | Linkedin: https://www.linkedin.com/in/jordiballesteralomar

bodedra · 2018-03-06T12:33:52Z

@jbeficent Appreciate your quick response. I have tested as per your suggested configuration and it creates PO with 10 quantity. I will start working on Odoo 11 to get same. Thank you.

max3903 · 2018-03-15T07:27:21Z

@jbeficent What do you think if @bodedra adds an onchange or check the values when writing to avoid this misconfiguration?

jgrandguillaume · 2018-03-26T09:08:44Z

Hi,

I think the best way to avoid this problem would be to refactor this module on this one:

https://github.com/OCA/stock-logistics-warehouse/tree/10.0/stock_mts_mto_rule

-> It means:

The MTS / MTO become a route (not an outside configuration)
The route uses a new method run (as buy or manufacture, we now have mts_mto

I don't know if I'm clear enough, but the idea is to bring this behavior through routes & run_* methods instead of overriding it like it has been done here... I think this would be the "ideal" way to bring this feature IMO.

Then, if too complicated or too much work at this stage, I understand..

Regards,

Joël

florian-dacosta · 2018-03-29T23:48:31Z

I also think it could be more logical to use the routes/rules. But it seems kind of tricky to use route and pull rule... Hard to see a good solution here.

But, maybe we could use only the route.
This module could depend on stock_mts_mto_rule and override mrp.production's _adjust_procure_method function.
There, we can check if the product is configured with the mts+mto rule and then split each move into 2 if needed (depending on the quantity available).(one would be with procure_method 'make to order' and the other 'make to stock'
But it is weird to use a route for an other purpose than searching pull rules... In this solution, the route would be used like a pull rule in a way.
(I am not sure I am very clear!)

Anyway, I am not sure it is really better.

florian-dacosta

Thanks for the work!
Althought tests are green, I see 2 main problem.
One bug about mto_with_no_move_dest_id option, not yet showed by tests.
And the other is about useless procurement group creation, unless I miss understood this part.

florian-dacosta · 2018-03-29T23:53:22Z

mrp_mto_with_stock/models/mrp_production.py

+                    # Search procurement group which has created from here
+                    group_name = '{0}:{1}'.format(
+                        production.name, move.product_id.name)
+                    procurement = procurement_obj.search(


I don't get this part. Why are you (later) creating a new procurement group? I don't think it is needed.

florian-dacosta · 2018-03-29T23:55:41Z

mrp_mto_with_stock/models/mrp_production.py

+                            new_move = move
+                        pg_data = production._get_procurement_group_data(
+                            new_move)
+                        group = procurement_obj.create(pg_data)


Same thing than before, I see no reason to create a procurement group, as it used only un run_procurement to find the origin. In that case, you can simply define a char origin variable instead of creating a record in Odoo!

florian-dacosta · 2018-03-29T23:55:58Z

mrp_mto_with_stock/models/mrp_production.py

+                    qty_to_procure = production.get_mto_qty_to_procure(move)
+                    if qty_to_procure > 0.0:
+                        pg_data = production._get_procurement_group_data(move)
+                        group = procurement_obj.create(pg_data)


florian-dacosta · 2018-03-30T00:03:35Z

mrp_mto_with_stock/models/mrp_production.py

+    def run_procurement(self, move, group, qty):
+        self.ensure_one()
+        errors = []
+        values = move._prepare_procurement_values()


In case of mto_with_no_move_dest_id, you need to remove 'move_dest_ids' from values.
Else the 2 case handled in this module are the same.
Indeed, with mto_with_no_move_dest_id, we want to buy/manufacture to make the future qty (virtual qty) = 0
but we do not want to link the purchase/manufacture order to the stock move.

It would be nice to add a small test to show this issue.
It can be easily done, for instance, adding

self.production.action_assign() self.assertEquals(self.production.move_raw_ids[0].state, 'assigned')

after https://github.com/ursais/manufacture/blob/dab59bcc211b61bab0f67486818bdaed38539e2a/mrp_mto_with_stock/tests/test_mrp_mto_with_stock.py#L77
Indeed, if you add this test with the present code, it will fail because the raw moves have ancestor and they should not.
Of course, the rest of the test may need to be adapted.

… creation

florian-dacosta · 2018-04-09T20:40:58Z

Hello,
I have fixed the 2 issues I talked about here : ursais#1
All seems good to me now

[11.0][mrp_mto_with_stock] Migration

LoisRForgeFlow

Tested on runbot. This looks good to me now!

Thanks @florian-dacosta and @bodedra

bodedra · 2018-04-11T08:59:58Z

@florian-dacosta and @lreficent appreciate your support. Thank you.

michaeljohn32 and others added 8 commits February 16, 2018 22:31

[ADD] mrp_mto_with_stock - initial commit

42d3e24

[9.0][IMP] mrp_mto_with_stock: adapt to OCA and minor fixes.

7c02c01

[9.0][REW] mrp_mto_with_stock: Rework to remove dependency and enhanc…

ad94d92

…e flexibility

[IMP] should not auto-confirm the MO. Added test cases

9f1f676

add contributor email

94699f0

Migrate and improve mrp_to_mto_with_stock to version 10

6345f15

OCA Transbot updated translations from Transifex

5d8596e

OCA Transbot updated translations from Transifex

d8b0674

pedrobaeza mentioned this pull request Feb 16, 2018

Migration to version 11.0 #226

Closed

25 tasks

pedrobaeza added this to the 11.0 milestone Feb 16, 2018

bodedra force-pushed the 11.0-mig-mrp_mto_with_stock branch 2 times, most recently from 1d32e12 to 9ba29fc Compare February 19, 2018 07:12

bodedra changed the title ~~11.0 mig mrp mto with stock~~ [11.0][MIG] mrp_mto_with_stock Feb 19, 2018

bodedra mentioned this pull request Feb 19, 2018

Migration to version 11.0 ForgeFlow/ddmrp#28

Open

guewen approved these changes Feb 19, 2018

View reviewed changes

bodedra force-pushed the 11.0-mig-mrp_mto_with_stock branch from 77d6fde to e035400 Compare February 20, 2018 11:20

LoisRForgeFlow requested changes Feb 22, 2018

View reviewed changes

max3903 requested a review from agyamuta February 27, 2018 17:19

max3903 assigned bodedra Feb 27, 2018

max3903 added the needs fixing label Feb 27, 2018

bodedra force-pushed the 11.0-mig-mrp_mto_with_stock branch 3 times, most recently from b686d2a to f9a7832 Compare March 1, 2018 13:43

bodedra force-pushed the 11.0-mig-mrp_mto_with_stock branch from f9a7832 to fa27afd Compare March 7, 2018 12:32

[11.0][MIG] mrp_mto_with_stock

dab59bc

bodedra force-pushed the 11.0-mig-mrp_mto_with_stock branch from fa27afd to dab59bc Compare March 7, 2018 12:51

florian-dacosta suggested changes Mar 30, 2018

View reviewed changes

Fix mto_with_no_move_dest_id option + avoid useless procurement group…

af86bde

… creation

florian-dacosta mentioned this pull request Apr 9, 2018

[11.0][mrp_mto_with_stock] Migration ursais/manufacture#1

Merged

Merge pull request #1 from akretion/11-mig-mrp_mto_with_stock

334f206

[11.0][mrp_mto_with_stock] Migration

florian-dacosta approved these changes Apr 9, 2018

View reviewed changes

LoisRForgeFlow approved these changes Apr 10, 2018

View reviewed changes

LoisRForgeFlow removed the needs fixing label Apr 10, 2018

max3903 merged commit 205015f into OCA:11.0 Apr 10, 2018

max3903 deleted the 11.0-mig-mrp_mto_with_stock branch April 10, 2018 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[11.0][MIG] mrp_mto_with_stock #255

[11.0][MIG] mrp_mto_with_stock #255

bodedra commented Feb 16, 2018 •

edited by max3903

Loading

guewen Feb 19, 2018

bodedra Feb 19, 2018

guewen Feb 20, 2018

bodedra Feb 20, 2018 •

edited

Loading

guewen Feb 19, 2018

bodedra Feb 19, 2018

bodedra Feb 20, 2018

LoisRForgeFlow left a comment

LoisRForgeFlow Feb 22, 2018

bodedra Feb 23, 2018

LoisRForgeFlow Feb 22, 2018 •

edited

Loading

bodedra Feb 23, 2018 •

edited

Loading

LoisRForgeFlow Feb 26, 2018 •

edited

Loading

LoisRForgeFlow Feb 26, 2018

max3903 Feb 27, 2018

agyamuta Mar 1, 2018

agyamuta Mar 1, 2018

bodedra Mar 1, 2018

jgrandguillaume commented Feb 22, 2018

bodedra commented Mar 6, 2018

JordiBForgeFlow commented Mar 6, 2018 via email

bodedra commented Mar 6, 2018

max3903 commented Mar 15, 2018

jgrandguillaume commented Mar 26, 2018

florian-dacosta commented Mar 29, 2018

florian-dacosta left a comment

florian-dacosta Mar 29, 2018

florian-dacosta Mar 29, 2018

florian-dacosta Mar 29, 2018

florian-dacosta Mar 30, 2018

florian-dacosta commented Apr 9, 2018 •

edited

Loading

LoisRForgeFlow left a comment

bodedra commented Apr 11, 2018


		<odoo> <data noupdate="1">

		<record id="product_product_manufacture_1" model="product.product">

[11.0][MIG] mrp_mto_with_stock #255

[11.0][MIG] mrp_mto_with_stock #255

Conversation

bodedra commented Feb 16, 2018 • edited by max3903 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bodedra Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LoisRForgeFlow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LoisRForgeFlow Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

bodedra Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

LoisRForgeFlow Feb 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgrandguillaume commented Feb 22, 2018

bodedra commented Mar 6, 2018

Current behavior

Expected behavior

JordiBForgeFlow commented Mar 6, 2018 via email

bodedra commented Mar 6, 2018

max3903 commented Mar 15, 2018

jgrandguillaume commented Mar 26, 2018

florian-dacosta commented Mar 29, 2018

florian-dacosta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

florian-dacosta commented Apr 9, 2018 • edited Loading

LoisRForgeFlow left a comment

Choose a reason for hiding this comment

bodedra commented Apr 11, 2018

bodedra commented Feb 16, 2018 •

edited by max3903

Loading

bodedra Feb 20, 2018 •

edited

Loading

LoisRForgeFlow Feb 22, 2018 •

edited

Loading

bodedra Feb 23, 2018 •

edited

Loading

LoisRForgeFlow Feb 26, 2018 •

edited

Loading

florian-dacosta commented Apr 9, 2018 •

edited

Loading