CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Enhancement] Work Order Material Consumption #13384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Let me quickly explain why we didn’t yet contribute documentation on this. We have not updated the documentation since the Frappe team is working on that currently due to the Production Order to Work Order transition. Whoever works on that could include this in the documentation if the design gets accepted or we can contribute the documentation after the updated documentation is available. |
Could you let us know if there is anything you need from us to get the PR merged ASAP? We are on a tight schedule. We are migrating on Friday this week and need this in. |
452c533
to
1041ac5
Compare
@nabinhait we bumped into an issue with this design since it records "issuance" accounting in the new purpose "Material Consumption for Manufacture". This creates a problem in making/calculating Valuation Rate of the Finish product. Can you suggest how to proceed so we can change code right away? Issue was pointed out in this discuss post https://discuss.erpnext.com/t/material-consumption-for-manufacture/35076. |
@nabinhait , here is my suggestion. We should have the idea of recording "WIP" accounts. So in the current design, we have these accounting entries for direct material: "Manufacture" Stock Entry: DEBIT: Finish Goods $ 1.00 If we will be adding the idea of "Material Consumption for Manufacture", we can have these accounting entries: "Material Consumption for Manufacture" Stock Entry: DEBIT: WIP Direct Material $ 1.00 "Manufacture" Stock Entry: DEBIT: Finish Goods $ 1.00 OR "Manufacture" Stock Entry: DEBIT: Finish Goods $ 1.00 So the components will be exhausted during the consumption using a WIP account. This means that we need to allow the "Manufacture" stock entry to record the "WIP" cost and use this to calculate the finish good's valuation rate. The WIP accounts should also show in the Balance sheet. We can add a column/field in the "Company" doctype as "WIP Direct Material" account to set-up the default account for this scenario. Same is true with Direct Material. Let us know what you think. |
Currently if the WIP warehouse has enough stock, "Finishing is allowed", is there is specific reason why you would like to disallow consumption ?
Can you should how the stock ledger will look like after consumption and completion? |
@djpalshikar can you pls put feedback from my recent suggestion about WIP accounts? |
This is the idea. If the WIP has enough stock it can consume. But if WIP only has 50 and consumption is 60 then this is not allowed.
In my recent comment, you can look at the accounting entries. For the Stock Ledger, here it is: "Material Consumption for Manufacture" Stock Entry "Manufacture" Stock Entry For partial consumption with "Manufacture" Stock Entry "Material Consumption for Manufacture" "Manufacture" Stock Entry |
|
Regarding accounting entry for "Material Consumption for Manufacture", let leave it to the users. Basically, user should be able to enter "Difference Account" manually and it can be either expense or asset account. |
if self.work_order and self.purpose == "Material Consumption for Manufacture": | ||
self.validate_work_order_status() | ||
else: | ||
self.update_stock_ledger() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update_stock_ledger
should be outside else
condition.
if self.purpose in manufacture_purpose and self.work_order: | ||
if not frappe.get_value('Work Order', self.work_order, 'skip_transfer'): | ||
item_code = [] | ||
for item in self.items: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loop should only be run for raw materials. Add a if condition if cstr(item.t_warehouse) = '':
if stock_qty > trans_qty: | ||
item_code.append(item.item_code) | ||
if item_code: | ||
frappe.throw(_("Consumption quantity cannot be more than transferred quantity for item/s {0}.").format(', '.join(item_code)), OverConsumptionError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do this validation? Because many cases, some unused raw materials (from earlier Work Orders) are there in WIP warehouse, which gets used. And it can lead to more consumption than transferred qty.
if (self.purpose == "Material Consumption for Manufacture" or self.purpose == "Manufacture"): | ||
for items in self.items: | ||
if self.docstatus == 1: | ||
frappe.db.sql("""Update `tabWork Order Item` set consumed_qty = consumed_qty + %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always avoid codes like this which does "+" and "- for updating qty. Use sum
of all consumed qty against the work order, similar to https://github.com/frappe/erpnext/blob/develop/erpnext/manufacturing/doctype/work_order/work_order.py#L498
@nabinhait , what about the calculation of the Finish product valuation rate ? Also, I wanted to confirm to you this:
This means the stock is consumed and deducted in the system but with zero accounting entries? And how do we make all the entries during finish? Should we get component cost from all the "Material Consumption for manufacture" for a work order? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some docstrings in this whole calculation, to explain the use case?
Yes, it will not going to work, it is not that simple. What about if we book normal expenses while consumptions? And while updating finished goods, raw material expenses will be reverted, if we use same difference account. And expenses will be booked only for additional costs. |
adb0791
to
0fc10b0
Compare
@nabinhait , made the changes you requested. And about below:
Currently, it is done like this now. "Material Consumption for Manufacture" behaves like a "Material Issue" only with tagged Work Order.
Can't be done since there is an option to do consumption in the "Manufacture" stock entry too when booked "Material Consumption for manufacture" is not done fully for the fg_completed_qty. And yes it will be challenging in the valuation rate calculation that is why I have suggested a WIP account that will be reverted in the "Manufacture" stock entry. |
@dottenbr @creamdory Documentation is updated for renaming Production Order to Work Order. You can start documenting this changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire functionality should be hidden by default. And can be enabled via a setting in Manufacturing Settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same calculation is applicable if "Backflush Raw Materials Based On" is set as "BOM" in Manufacturing Settings. It should consider Consumption entry and pull only remaining items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nabinhait this is what this code do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I think it does not work if "Backflush Raw Materials Based On" is set as "BOM" in Manufacturing Settings. I think get_transfered_raw_materials
is not called for that use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not be shown if all items are consumed
0fc10b0
to
dd39201
Compare
dccf9c8
to
53068a6
Compare
@creamdory Change the label "Material Consumption" to "Allow Multiple Material Consumption". |
53068a6
to
0894896
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nabinhait Added this to trigger get_transfered_raw_materials()
without looking at the backlash_raw_materials_base_on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@creamdory It is not right. If "backflush_raw_materials_based_on" is based on BOM, then it should not consider transferred materials. It will simply assume all the items in the BOM is in WIP warehouses.
47864ca
to
381e0e4
Compare
@nabinhait requested changes made. |
3b342b7
to
1988562
Compare
@nabinhait currently, documentation is broken. https://discuss.frappe.io/t/bug-documentation-broken/601 |
1988562
to
98bcffe
Compare
@nabinhait made the documentation. Please check. |
@creamdory is there decision on the valuation of the finished item ? |
@djpalshikar currently not automated. Nabin says to change accounts during Stock Entries. Though we will think about it and make some enhancements. Currently, this feature can be enabled or disabled for this case. |
…acture Hi @nabinhait , Issue : Manufacturing setting > Backflush Raw Material Based on “Material Transferred for Manufacture” doesn't fetch the actual raw material transferred qty. It fetches qty based on "BOM" The issue is because @creamdory in PR #frappe#13384 commit : https://github.com/frappe/erpnext/pull/13384/files#diff-91f0ed661ef4b6e1f167fc7961b1a79b ``` changed from: if trans_qty and manufacturing_qty >= (produced_qty + flt(self.fg_completed_qty)): to : if trans_qty and manufacturing_qty > (produced_qty + flt(self.fg_completed_qty)): ``` **'='** was added by her in the condition, which was not there before her commit. Kindly except the fix for the issue. https://github.com/frappe/erpnext/blob/develop/erpnext/stock/doctype/stock_entry/stock_entry.py#L1057 frappe#13384 https://github.com/frappe/erpnext/pull/13384/files#diff-91f0ed661ef4b6e1f167fc7961b1a79b **before fix gif** : Stock Entry = Manufacture shows raw material quantity as per BOM.  **after fix gi**f : Stock Entry = "Manufacture" shows raw material quantity as per "Material Transfer for Manufacture". 
Made the enhancements from these issues:
#13368
#13369
#13368
BOM:
Usecases:
Usecase:
1 transfer, 1 consumption and 1 Finish from a Work Order
Usecase:
Multiple Transfer, Consumption and Finish from a Work Order.
Work Order

Partial Stock Transfer suggest only "Required Qty" * "Qty for Material Transfer for Manufacture". If we click "Finish" before any "Material Consumption", must suggest Qty to consumed in the Manufacture Stock Entry. If we create "Material Consumption for Manufacture" before "Finish", must take into account the "Consumed Qty" during Finish.
Usecase:
Work Order with Partial transfer, consumption and completion.
Validations
Notes