Merge lp:~wiz-keed/ocb-addons/ocb-7.0-fix-mrp-subproduct-unkink into lp:ocb-addons

[Paul Catinean]

Port to OCB of the following fix:

[FIX] Added ondelete='cascade' to mrp.subproduct when attatched bom is unlinked

https://code.launchpad.net/~wiz-keed/openobject-addons/7.0-fix-mrp-subproduct-unlink/+merge/213078

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

LGTM, thanks!

review: Approve

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

This change is touchy, as it modifies the underlying database schema. Probably not OK for standard addons.

I'd be OK for having this in OCB, though.

Having an automated test showing the bug is fixed would be sooooooo nice.

review: Approve (code review, no test)

[Pedro Manuel Baeza (pedro.baeza)]

Do you think it's better to have cascade instead of restricted, because with this change, we will have records missed when we delete BoMs.

Regards.

review: Needs Information

[Pedro Manuel Baeza (pedro.baeza)]

Sorry, I didn't see context of the cascade. Now I see that it makes sense, so I approve it.

IMHO a test for this is excessive, where we only have to set an attribute that it's not going to be missed again, but...

Regards.

review: Approve (code review)

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

[Paul Catinean (wiz-keed)]

Apologies, I have replied in the email client instead of writing comments here:

1st reply @rvalyi:

Glad to help out!

A local test showed no impact on a database that already had boms and mrp.subproducts created, automated tests would be more tedious instead of my isolated case though

2nd reply @pedro.baeza:

I kinda feel the same way to be honest since it's not a constraint that might be blocked by preexisting records.It will only affect delete statements after it has been placed on the table, and local tests show no problems

Up to you guys, cheers!

=== modified file 'mrp_byproduct/mrp_byproduct.py'
--- mrp_byproduct/mrp_byproduct.py	2014-03-10 08:54:20 +0000
+++ mrp_byproduct/mrp_byproduct.py	2014-03-27 15:11:23 +0000
@@ -35,7 +35,7 @@
   'Fixed' depicts a situation where the quantity of created byproduct is always equal to the quantity set on the BoM, regardless of how many are created in the production order.\
   By opposition, 'Variable' means that the quantity will be computed as\
     '(quantity of byproduct set on the BoM / quantity of manufactured product set on the BoM * quantity of manufactured product in the production order.)'"),
-        'bom_id': fields.many2one('mrp.bom', 'BoM'),
+        'bom_id': fields.many2one('mrp.bom', 'BoM', ondelete='cascade'),
     }
     _defaults={
         'subproduct_type': 'variable',