[16.0][ADD] quality_control_stock_oca: automatic scrap creation on Inspection failure #1611
Conversation
| super().button_validate() | ||
|
|
||
| if all(qc.product_id.auto_scrap for qc in self.qc_inspections_ids): | ||
| self.create_scraps() | ||
|
|
||
| return True |
There was a problem hiding this comment.
issue: tests of another module seem to be broken because of this. Can you test if this change fixes it?
| super().button_validate() | |
| if all(qc.product_id.auto_scrap for qc in self.qc_inspections_ids): | |
| self.create_scraps() | |
| return True | |
| res = super().button_validate() | |
| if all(qc.product_id.auto_scrap for qc in self.qc_inspections_ids): | |
| self.create_scraps() | |
| return res |
quirino95
force-pushed
the
16.0-receipts_inspection_failure
branch
2 times, most recently
from
9717b94 to
d6b5819
Compare
| super().action_approve() | ||
| if ( | ||
| self.picking_id.created_inspections == self.picking_id.done_inspections | ||
| and all(m.product_id.auto_scrap for m in self.picking_id.move_ids) | ||
| ): | ||
|
|
||
| self.picking_id.button_validate() | ||
|
|
||
| return True |
There was a problem hiding this comment.
suggestion:
| super().action_approve() | |
| if ( | |
| self.picking_id.created_inspections == self.picking_id.done_inspections | |
| and all(m.product_id.auto_scrap for m in self.picking_id.move_ids) | |
| ): | |
| self.picking_id.button_validate() | |
| return True | |
| res = super().action_approve() | |
| if ( | |
| self.picking_id.created_inspections == self.picking_id.done_inspections | |
| and all(m.product_id.auto_scrap for m in self.picking_id.move_ids) | |
| ): | |
| self.picking_id.button_validate() | |
| return res |
quirino95
force-pushed
the
16.0-receipts_inspection_failure
branch
from
d6b5819 to
8ca8b5f
Compare
| ) | ||
| self.inspection1.action_approve() | ||
|
|
||
| self.assertEqual(self.picking1.state, "done") |
There was a problem hiding this comment.
Observation: If there are two products in a receipt, one with scrap True and other with scrap False, picking is not in done and scrap not created for the product whose inspection Failed.
There was a problem hiding this comment.
Just fixed (see comment below)
There was a problem hiding this comment.
Suggestion: Can we add scrap creation validation as well for this testcase?
There was a problem hiding this comment.
Just added scrap creation validation, but not in that method since it tests inspection approval.
Later in the file there is test_auto_create_scraps, where i tested scrap creation, and now I added a check on scraps state.
|
|
||
| res = super().button_validate() | ||
|
|
||
| if all(qc.product_id.auto_scrap for qc in self.qc_inspections_ids): |
There was a problem hiding this comment.
Question: Why do we create scraps, if only all products in a receipt has 'scrap automatically' as True? shouldn't be any one of the products with scrap?
There was a problem hiding this comment.
This is a good suggestion: just fixed in a bit different way. Now every product is scrapped when needed, independently from the others
There was a problem hiding this comment.
It looks good when handled in create_scraps() for inspection failure with auto_scrap is checked.
| It also adds some shortcuts on picking and lots to these inspections. | ||
| Activating the "Remind Quality Control" flag on products, if inspections are not done (successfully or unsuccessfully), on picking confirmation a reminder popup appears. | ||
| Activating "Scrap Automatically" flag on products, if inspections fail, picking: | ||
| - Will be automatic validated if every product in picking has the flag checked; |
There was a problem hiding this comment.
Question: I see in description it says: "Moves should be done once set inspection as "failed" (so the "validation" should be automatic).", So I think whenever inspections are done and failed, Validation has to be completed (i.e. Pickings are in Done state) and also create scrap if any one of the products has 'scrap automatically' as True in a receipt- Is my understanding right?
If so, here mentioning validation is automatic only if every product has checked with scrap automatically is valid?
There was a problem hiding this comment.
Yes, is valid. This is a way to keep the default behavior, but also a chance to have a different behavior (checking flag auto_scrap) to speed up picking validation.
There was a problem hiding this comment.
Thank you for clarification. Implementation works intact for success scenarios but with respect to failed inspections after latest fix observed some improper behaviour as commented below. Could not figure out yet what makes this trigger of inspection when auto_scrap is checked and inspection is failed.
| if relative flag is on | ||
| """ | ||
| self.product.auto_scrap = True | ||
| self.picking1.move_ids[:1].quantity_done = 2 |
There was a problem hiding this comment.
Observation: Same observation with two products(one with scrap and other without scrap) under a receipt, validation is not done and scrap not created
There was a problem hiding this comment.
Based on the comments above: both products need to have auto_scrap flag checked in order to automatic validate picking. With last fix (#1611 (comment)), now needed scraps are created on validation.
There was a problem hiding this comment.
Agree validation based on auto_scrap flag and scrap creation on failed inspections once after validation handled manually. However, I see an observation as below where after validation completion, for failed inspection a new inspection is triggered with same use case above which was not seen before this latest fix. Both trigger with timing as before
Inspection view:
Transfer view:
There was a problem hiding this comment.
I think that behaviour is not related to this feature, since I was able to reproduce same issue with auto_scrap off on both products.
I've tested also with different products:
With auto_scrap set on both products, and no new Inspections were created. Maybe other products have different inventory rules, or Inspection settings.
There was a problem hiding this comment.
I tried to use runboat build instance on this PR to check issue reproduction. Agree, as you have mentioned auto_scrap as off on both products, auto_scrap as on on both products and inspection failed on any one of the products, it works as expected. No new inspection created.
However, when auto_scrap is set for one of the products and inspection fails for product whose auto_scrap=true, new inspection is created after validation.
RunBoat Instance view:
http://oca-manufacture-16-0-pr1611-c823028a1f8e.runboat.odoo-community.org/web#id=27&cids=1&menu_id=120&action=238&model=stock.picking&view_type=form
Also, when auto_scrap=false on products in the Pickings, i don't observe this new inspection creation, which is also verified in runboat instance. If its happening on runboat instance, it should not be settings related issue. What do you think?
There was a problem hiding this comment.
Based on discussion, it was identified that when a scrap is created for a product whose inspection is failed, there is a new inspection created after validation. This is an existing behaviour and no issues with any feature introduced with this PR.
As per first patchset, when one of the products with auto_scrap=true , scrap was not created, hence new inspection was not seen. This is the reason for difference in behaviour observed between patchsets.
quirino95
force-pushed
the
16.0-receipts_inspection_failure
branch
from
8ca8b5f to
9aa8e3b
Compare
quirino95
force-pushed
the
16.0-receipts_inspection_failure
branch
from
9aa8e3b to
c823028
Compare
|
Code and Functional Review: LGTM |
3 participants
Depends on #1584
New flag "Scrap Automatically" on products.
When an inspection related to a picking fails, on picking validation, scraps are automatically created for all products whose inspection has failed and have the flag active.
If all products in a picking have the flag active, and all inspections are completed (both success or fail), validation is completed automatically (and of course needed scraps are created).