Skip to content

refactor!: dynamically compute bom_level - #29522

Merged
rohitwaghchaure merged 2 commits into
frappe:developfrom
ankush:drop_bom_level
Jan 31, 2022
Merged

refactor!: dynamically compute bom_level#29522
rohitwaghchaure merged 2 commits into
frappe:developfrom
ankush:drop_bom_level

Conversation

@ankush

@ankush ankush commented Jan 30, 2022

Copy link
Copy Markdown
Member

#26240 introduced a field called "BOM level" to properly sort subassemblies in the production plan. This however relied on value present in BOM.

Since a BOM's level is not a static value but depends on context, this has to be computed dynamically. Thankfully indent is exactly that, starting with the 0th level and recursively adding the level as it goes. (another alternative was to use BOMTree class which generates this on the fly too)

Breaking Change:

  • The BOM level shown now is opposite of what it used to be and hence sorted in a reverse manner. The point is to make work order for the lowest level of subassemblies first and work upwards in the tree so you always have subcomponents available.

bom_data = sorted(bom_data, key = lambda i: i.bom_level, reverse=True)

Secondary fix:

  • Sort subassembly items globally instead of doing it for a single production item. (refer test for more info)

Example:

Suppose we are manufacturing 2 items with nested tree like this:

Assembly
┣━ SubAssembly1
┃  ┣━ ChildPart1
┃  ┣━ ChildPart2
┣━ SubAssembly2
┃  ┣━ ChildPart3
┣━ SubAssembly3
┃  ┣━ SubSubAssy1
┃  ┃  ┣━ ChildPart4
┣━ ChildPart5
┣━ ChildPart6
┣━ SubAssembly4
┃  ┣━ SubSubAssy2
┃  ┃  ┣━ ChildPart7


MegaDeepAssy
┣━ SecretSubassy
┃  ┣━ SecretPart
┃  ┃  ┣━ VerySecret
┃  ┃  ┃  ┣━ SuperSecret
┃  ┃  ┃  ┃  ┣━ Classified

Then generated list of subassembly should be like this:

SuperSecret
VerySecret
SubSubAssy1
SubSubAssy2
SecretPart
SubAssembly1
SubAssembly2
SubAssembly3
SubAssembly4
SecretSubassy

Basically, items that are at the lowest level in BOM tree should appear first. Similar to Work Order fix: #25886

@ankush ankush added the needs-tests This PR needs automated unit-tests. label Jan 30, 2022
@ankush
ankush marked this pull request as draft January 30, 2022 16:41
@ankush ankush added this to the v14.0 milestone Jan 30, 2022
@codecov

codecov Bot commented Jan 30, 2022

Copy link
Copy Markdown

Codecov Report

Merging #29522 (d38fd86) into develop (0f7c2a1) will decrease coverage by 0.35%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #29522      +/-   ##
===========================================
- Coverage    58.47%   58.12%   -0.36%     
===========================================
  Files         1091     1091              
  Lines        68013    68003      -10     
===========================================
- Hits         39770    39526     -244     
- Misses       28243    28477     +234     
Impacted Files Coverage Δ
erpnext/manufacturing/doctype/bom/bom.py 86.91% <ø> (-0.07%) ⬇️
.../manufacturing/report/bom_explorer/bom_explorer.py 94.44% <ø> (ø)
...production_plan_summary/production_plan_summary.py 0.00% <ø> (ø)
...cturing/doctype/production_plan/production_plan.py 69.24% <100.00%> (+0.05%) ⬆️
...eport/item_variant_details/item_variant_details.py 31.32% <0.00%> (-56.63%) ⬇️
...pnext/accounts/report/gross_profit/gross_profit.py 40.85% <0.00%> (-41.64%) ⬇️
...t/accounts/report/sales_register/sales_register.py 36.36% <0.00%> (-36.97%) ⬇️
...xt/stock/report/stock_analytics/stock_analytics.py 80.19% <0.00%> (-12.88%) ⬇️
...rial_no_valuation/incorrect_serial_no_valuation.py 85.96% <0.00%> (-10.53%) ⬇️
...it_and_loss_statement/profit_and_loss_statement.py 71.42% <0.00%> (-3.90%) ⬇️
... and 23 more

@ankush ankush removed the needs-tests This PR needs automated unit-tests. label Jan 30, 2022
@ankush
ankush marked this pull request as ready for review January 30, 2022 17:31
@rohitwaghchaure
rohitwaghchaure merged commit f026ec6 into frappe:develop Jan 31, 2022
@ankush
ankush deleted the drop_bom_level branch January 31, 2022 10:31
@rohitwaghchaure

Copy link
Copy Markdown
Collaborator

@Mergifyio backport version-13-hotfix

@mergify

mergify Bot commented Jan 31, 2022

Copy link
Copy Markdown
Contributor

backport version-13-hotfix

✅ Backports have been created

Details

ankush added a commit that referenced this pull request Jan 31, 2022
* refactor!: dynamically compute bom_level

(cherry picked from commit 157b4b3)

# Conflicts:
#	erpnext/manufacturing/doctype/bom/bom.py
#	erpnext/patches.txt

* fix(ux): sort multi-production item plans correctly

(cherry picked from commit d38fd86)

* fix: conflicts

* fix: removed set_bom_level

* fix: conflict

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: rohitwaghchaure <rohitw1991@gmail.com>
@djpalshikar

Copy link
Copy Markdown
Contributor

@ankush how will the his change affect the BOM update tool ?

@ankush

ankush commented Mar 2, 2022

Copy link
Copy Markdown
Member Author

@ankush how will this change affect the BOM update tool?

It doesn't affect the BOM update tool.

We were using this field only for sorting production plan subassemblies and nothing else.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jan 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants