[10.0][ADD] sale_mrp_link module #661

tafaRU · 2018-06-18T15:46:32Z

Migrated https://github.com/akretion/sale-workflow/blob/9-sale_mrp_link/sale_mrp_link/ to 10.0 cc @florian-dacosta

sale_mrp_link/__init__.py

hieulucky111 · 2018-06-22T04:02:50Z

sale_mrp_link/models/__init__.py

@@ -0,0 +1,3 @@
+from . import mrp_production
+from . import sale_order
+from . import procurement_order


Import order.

sale_mrp_link/models/__init__.py

hieulucky111 · 2018-06-22T04:03:22Z

sale_mrp_link/models/mrp_production.py

+# Copyright 2016-2018 Akretion
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from odoo import models, fields


import order.

hieulucky111 · 2018-06-22T04:03:55Z

sale_mrp_link/models/procurement_order.py

+# Copyright 2016-2018 Akretion
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from odoo import models, api, exceptions, _


import order.

hieulucky111 · 2018-06-22T04:06:49Z

sale_mrp_link/models/sale_order.py

+        mrp_production_model = self.env['mrp.production']
+        for sale in self:
+            domain = [('sale_order_id', '=', sale.id)]
+            self.production_count = mrp_production_model.search_count(domain)


it should be sale instead self.

hieulucky111

Please check my review.

florian-dacosta

Seems good overall, some minor remarks
Thanks for the migration!

florian-dacosta · 2018-06-22T08:54:48Z

sale_mrp_link/__manifest__.py

+    'category': 'Sales Management',
+    'website': 'https://github.com/OCA/sale-workflow',
+    'author': 'Agile Business Group, Akretion, '
+              'Odoo Community Association (OCA)',


I think authorship is normally in order of importance.
Also I do not think you should add an author for a standard migration. Adding it in contributor yes, but author?
Anyway, other opinions are welcome on the subject.

@florian-dacosta, no problem to change the order of authorship. Please note this is not just a standard migration: I added a new code and modified the original one. This is the reason I chose to add my company to author.

florian-dacosta · 2018-06-22T08:57:04Z

sale_mrp_link/readme/CONTRIBUTORS.rst

@@ -0,0 +1,2 @@
+* Alex Comba <alex.comba@agilebg.com>
+* Florian da Costa <florian.dacosta@akretion.com>


Wrong order, it should be the contrary

florian-dacosta · 2018-06-22T08:58:17Z

sale_mrp_link/readme/ROADMAP.rst

@@ -0,0 +1 @@
+The module is ncompatible with `sale_sourced_by_line <https://github.com/OCA/sale-workflow/tree/10.0/sale_sourced_by_line>`_ and `sale_quotation_sourcing <https://github.com/OCA/sale-workflow/blob/10.0/sale_quotation_sourcing>`_ (please note this latter is not migrated yet).


ncompatible => incompatible

tafaRU · 2018-06-22T13:00:02Z

@hieulucky111, @florian-dacosta, thanks a lot for your review! I should have addreessed all your comments, please check it.

florian-dacosta

Thanks

SimoRubi

Check runbot, I think it is saying you have to generate the README.
The rest of the code is great.

Tried in runbot, it is ok.

tafaRU · 2018-06-27T09:16:45Z

@SimoRubi, done at 55e441c

SimoRubi

Ok then

eLBati · 2018-07-03T11:00:36Z

I would avoid to create and write additional fields, as production order can be retrieved with a computed field , in sale.order, that search for linked procurements by procurement group , and thus production order by production_id field of procurement.order
In this way, you can also avoid to modify .travis.yml

eLBati · 2018-07-03T13:43:49Z

Even better, instead of searching for procurements by procurement group, searching using sale_line_id field of procurement.order

florian-dacosta · 2018-07-03T13:53:02Z

@eLBati I think you would have to search by procurement group anyway, because the sale_line_id is not propagated on all procurements, juste the first one created which usually is used to create the delivery order.

Having a new field of the Manufacture order is useful. It is not only used by the sale order button to show the manufacture order (which could be done with a computed field, like you say) but also on the side of the manufacturing order, A direct link to between Sale Order and the Manufacture Order allow you to apply filter, help for BI, see easily which SO is concernd in MO form view...

eLBati · 2018-07-06T08:13:17Z

ok then 👍

tafaRU · 2018-07-06T08:52:34Z

Thanks @florian-dacosta for the comment!
If that's okay with you, @eLBati, then you could merge it. Thanks.

eLBati · 2018-07-12T15:05:07Z

.travis.yml

+  - TESTS="1" ODOO_REPO="odoo/odoo" LINT_CHECK="0" EXCLUDE=sale_sourced_by_line,sale_procurement_group_by_line
+  - TESTS="1" ODOO_REPO="OCA/OCB" LINT_CHECK="0" EXCLUDE=sale_sourced_by_line,sale_procurement_group_by_line
+  - TESTS="1" ODOO_REPO="odoo/odoo" LINT_CHECK="0" INCLUDE=sale_sourced_by_line
+  - TESTS="1" ODOO_REPO="OCA/OCB" LINT_CHECK="0" INCLUDE=sale_sourced_by_line


@tafaRU in this way, sale_procurement_group_by_line is never tested, right?

MiquelRForgeFlow · 2018-07-12T15:11:21Z

.travis.yml

-  - TESTS="1" ODOO_REPO="odoo/odoo" LINT_CHECK="0"
-  - TESTS="1" ODOO_REPO="OCA/OCB" LINT_CHECK="0"
+  - TESTS="1" ODOO_REPO="odoo/odoo" LINT_CHECK="0" EXCLUDE=sale_sourced_by_line,sale_procurement_group_by_line
+  - TESTS="1" ODOO_REPO="OCA/OCB" LINT_CHECK="0" EXCLUDE=sale_sourced_by_line,sale_procurement_group_by_line


Why excluding those modules?

Hi @mreficent,

as I wrote here, this module is incompatible with those modules. The reason for this is that sale.order and mrp.production are linked by the field sale_order_id and when the module sale_procurement_group_by_line is installed, the field procurement_group_id is only filled at sale.order.line [1] level not at sale.order level, as a standard way [2], and, consequently, here no sale.order is found.

After #573 the sale_procurement_group_by_line module is installed anyway, even though I excluded it. So that, I'd also to EXCLUDE the module sale_stock_picking_blocking_proc_group_by_line.

Would that be fine by you?

[1] https://github.com/OCA/sale-workflow/blob/10.0/sale_procurement_group_by_line/model/sale.py#L83
[2] https://github.com/odoo/odoo/blob/10.0/addons/sale/models/sale.py#L669

just seen #690 it also should work in my case!

sale_mrp_link/__manifest__.py

…e with sale_mrp_link See OCA#661 (comment) for further info

…patible with sale_mrp_link See OCA#661 (comment) for further info

tafaRU · 2018-09-11T15:39:04Z

@OCA/crm-sales-marketing-maintainers I think this PR is finally ready to be merged. Could you please check? Thanks

tafaRU · 2018-09-14T15:45:51Z

ping @OCA/crm-sales-marketing-maintainers !

…patible with sale_mrp_link See OCA#661 (comment) for further info

tafaRU added the work in progress label Jun 19, 2018

tafaRU changed the title ~~[ADD] sale_mrp_link module~~ [10.0][ADD] sale_mrp_link module Jun 19, 2018

tafaRU force-pushed the 10.0-mig-sale_mrp_link branch 5 times, most recently from a0e9d64 to 2d7ecba Compare June 19, 2018 16:20

tafaRU added needs review and removed work in progress labels Jun 19, 2018

hieulucky111 reviewed Jun 22, 2018

View reviewed changes

sale_mrp_link/__init__.py Show resolved Hide resolved

hieulucky111 reviewed Jun 22, 2018

View reviewed changes

sale_mrp_link/models/__init__.py Show resolved Hide resolved

hieulucky111 reviewed Jun 22, 2018

View reviewed changes

hieulucky111 suggested changes Jun 22, 2018

View reviewed changes

florian-dacosta suggested changes Jun 22, 2018

View reviewed changes

tafaRU added a commit to tafaRU/sale-workflow that referenced this pull request Jun 22, 2018

[FIX] sale_mrp_link: address comments review of OCA#661

6640dea

florian-dacosta approved these changes Jun 26, 2018

View reviewed changes

hieulucky111 approved these changes Jun 26, 2018

View reviewed changes

SimoRubi suggested changes Jun 26, 2018

View reviewed changes

SimoRubi approved these changes Jun 27, 2018

View reviewed changes

tafaRU requested a review from eLBati June 27, 2018 09:18

tafaRU added the approved label Jun 27, 2018

eLBati reviewed Jul 12, 2018

View reviewed changes

MiquelRForgeFlow suggested changes Jul 12, 2018

View reviewed changes

MiquelRForgeFlow approved these changes Aug 30, 2018

View reviewed changes

sale_mrp_link/__manifest__.py Outdated Show resolved Hide resolved

tafaRU force-pushed the 10.0-mig-sale_mrp_link branch from db936c7 to 97160bc Compare September 10, 2018 10:28

tafaRU added a commit to tafaRU/sale-workflow that referenced this pull request Sep 10, 2018

[FIX] sale_mrp_link: address comments review of OCA#661

5a126f6

[ADD] sale_mrp_link module

0ef3145

tafaRU force-pushed the 10.0-mig-sale_mrp_link branch from fa74fbe to 0ef3145 Compare September 11, 2018 14:53

tafaRU added a commit to tafaRU/sale-workflow that referenced this pull request Sep 11, 2018

[FIX] sale_mrp_link: separate tests for those modules are incompatibl…

325b2db

…e with sale_mrp_link See OCA#661 (comment) for further info

[FIX] sale_mrp_link: separate tests for those modules which are incom…

a2c21a4

…patible with sale_mrp_link See OCA#661 (comment) for further info

tafaRU force-pushed the 10.0-mig-sale_mrp_link branch from 325b2db to a2c21a4 Compare September 11, 2018 15:07

tafaRU removed the needs review label Sep 11, 2018

MiquelRForgeFlow approved these changes Sep 12, 2018

View reviewed changes

tafaRU mentioned this pull request Sep 17, 2018

[9.0] Add module sale_mrp_link #401

Closed

eLBati merged commit b3a14ea into OCA:10.0 Sep 21, 2018

tafaRU deleted the 10.0-mig-sale_mrp_link branch September 21, 2018 07:39

tafaRU mentioned this pull request Oct 16, 2018

[10.0] base branch fails sale_mrp_link tests #717

Closed

sylvainvh pushed a commit to camptocamp/sale-workflow that referenced this pull request Feb 5, 2019

[FIX] sale_mrp_link: separate tests for those modules which are incom…

e652bf0

…patible with sale_mrp_link See OCA#661 (comment) for further info

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.0][ADD] sale_mrp_link module #661

[10.0][ADD] sale_mrp_link module #661

tafaRU commented Jun 18, 2018 •

edited

Loading

hieulucky111 Jun 22, 2018

hieulucky111 Jun 22, 2018

hieulucky111 Jun 22, 2018

hieulucky111 Jun 22, 2018

hieulucky111 left a comment

florian-dacosta left a comment

florian-dacosta Jun 22, 2018

tafaRU Jun 22, 2018

florian-dacosta Jun 22, 2018

florian-dacosta Jun 22, 2018

tafaRU commented Jun 22, 2018

florian-dacosta left a comment

SimoRubi left a comment •

edited

Loading

tafaRU commented Jun 27, 2018

SimoRubi left a comment

eLBati commented Jul 3, 2018

eLBati commented Jul 3, 2018

florian-dacosta commented Jul 3, 2018

eLBati commented Jul 6, 2018

tafaRU commented Jul 6, 2018

eLBati Jul 12, 2018

MiquelRForgeFlow Jul 12, 2018

tafaRU Aug 30, 2018 •

edited

Loading

tafaRU Aug 30, 2018

tafaRU commented Sep 11, 2018

tafaRU commented Sep 14, 2018

		@@ -0,0 +1,2 @@
		* Alex Comba <alex.comba@agilebg.com>
		* Florian da Costa <florian.dacosta@akretion.com>

		@@ -0,0 +1 @@
		The module is ncompatible with `sale_sourced_by_line <https://github.com/OCA/sale-workflow/tree/10.0/sale_sourced_by_line>`_ and `sale_quotation_sourcing <https://github.com/OCA/sale-workflow/blob/10.0/sale_quotation_sourcing>`_ (please note this latter is not migrated yet).

[10.0][ADD] sale_mrp_link module #661

[10.0][ADD] sale_mrp_link module #661

Conversation

tafaRU commented Jun 18, 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

hieulucky111 left a comment

Choose a reason for hiding this comment

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

tafaRU commented Jun 22, 2018

florian-dacosta left a comment

Choose a reason for hiding this comment

SimoRubi left a comment • edited Loading

Choose a reason for hiding this comment

tafaRU commented Jun 27, 2018

SimoRubi left a comment

Choose a reason for hiding this comment

eLBati commented Jul 3, 2018

eLBati commented Jul 3, 2018

florian-dacosta commented Jul 3, 2018

eLBati commented Jul 6, 2018

tafaRU commented Jul 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tafaRU Aug 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tafaRU commented Sep 11, 2018

tafaRU commented Sep 14, 2018

tafaRU commented Jun 18, 2018 •

edited

Loading

SimoRubi left a comment •

edited

Loading

tafaRU Aug 30, 2018 •

edited

Loading